[PATCH 0/2] hwmon: (pmbus/tps53679) Select page 0 for single-page parts

[PATCH 0/2] hwmon: (pmbus/tps53679) Select page 0 for single-page parts

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

For single-page TPS53676 and TPS536C7 configurations the PAGE register is
never normalised: pmbus_set_page() only writes PAGE when info->pages > 1,
so a page left selected by the boot firmware persists and telemetry is
read from the wrong page. Select page 0 explicitly (with readback) in the
two identify routines.

Patch 1 is a pre-existing TPS53676 fix (Fixes/stable). Patch 2 fixes the
TPS536C7 support and applies on top of the TPS53622/TPS53659/TPS536C7
series already applied to hwmon-next; it is not yet in a released kernel,
so it carries no stable tag.

These two patches apply on top of the TPS536C7 series ("hwmon:
(pmbus/tps53679) Add support for TPS536C7") already in hwmon-next, so
there is no separate base-commit line.

Link: https://lore.kernel.org/linux-hwmon/20260915164823.160977-1-sanman.pradhan@hpe.com/T/#t

Sanman Pradhan (2):
  hwmon: (pmbus/tps53679) Select page 0 for single-page TPS53676
  hwmon: (pmbus/tps53679) Select page 0 for single-page TPS536C7

 drivers/hwmon/pmbus/tps53679.c | 48 ++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

-- 
2.34.1

[PATCH 1/2] hwmon: (pmbus/tps53679) Select page 0 for single-page TPS53676

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

tps53676_identify() derives the number of PMBus pages but does not
ensure that page 0 is selected for single-page configurations.
pmbus_set_page() does not update the PAGE register when info->pages is
1, so if boot firmware leaves PAGE set to another value subsequent
register accesses may target the wrong page.

For single-page devices, select page 0 explicitly and verify that the
PAGE register was updated.

Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/tps53679.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index dcd4250b679d..f38c19b8cc43 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -248,6 +248,30 @@ static int tps53676_identify(struct i2c_client *client,
 		info->phases[1] = phases_b;
 	}
 
+	/*
+	 * pmbus_set_page() does not update the PAGE register on single-page
+	 * devices, so select page 0 explicitly and verify it in case the
+	 * boot firmware left the device on another page.
+	 */
+	if (info->pages == 1) {
+		ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+		if (ret < 0)
+			return ret;
+		if (ret != 0) {
+			ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+			if (ret < 0)
+				return ret;
+			ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+			if (ret < 0)
+				return ret;
+			if (ret != 0) {
+				dev_err(&client->dev,
+					"failed to select page 0\n");
+				return -EIO;
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 1/2] hwmon: (pmbus/tps53679) Select page 0 for single-page TPS53676

[Guenter Roeck]

Hi,

On 9/16/26 13:04, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> tps53676_identify() derives the number of PMBus pages but does not
> ensure that page 0 is selected for single-page configurations.
> pmbus_set_page() does not update the PAGE register when info->pages is
> 1, so if boot firmware leaves PAGE set to another value subsequent
> register accesses may target the wrong page.
> 
> For single-page devices, select page 0 explicitly and verify that the
> PAGE register was updated.
> 

Why that complexity ? We don't read back other registers. Why would it be
necessary or even make sense to do it here ? Following that logic one could
argue that every single write has to be read back to verify it.

> Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> ---
>   drivers/hwmon/pmbus/tps53679.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
> index dcd4250b679d..f38c19b8cc43 100644
> --- a/drivers/hwmon/pmbus/tps53679.c
> +++ b/drivers/hwmon/pmbus/tps53679.c
> @@ -248,6 +248,30 @@ static int tps53676_identify(struct i2c_client *client,
>   		info->phases[1] = phases_b;
>   	}
>   

A much simpler
	} else {
		/*
		 * pmbus_set_page() does not update the PAGE register on
		 * single-page devices, so select page 0 explicitly in case
		 * the boot firmware left the device on another page.
		 */
		ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
		if (ret < 0)
			return ret;
	}

should do the trick. Yes, the write may be unnecessary, but it
is cheaper than a read followed by an optional write.

Thanks,
Guenter
> +	/*
> +	 * pmbus_set_page() does not update the PAGE register on single-page
> +	 * devices, so select page 0 explicitly and verify it in case the
> +	 * boot firmware left the device on another page.
> +	 */
> +	if (info->pages == 1) {
> +		ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> +		if (ret < 0)
> +			return ret;
> +		if (ret != 0) {
> +			ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
> +			if (ret < 0)
> +				return ret;
> +			ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> +			if (ret < 0)
> +				return ret;
> +			if (ret != 0) {
> +				dev_err(&client->dev,
> +					"failed to select page 0\n");
> +				return -EIO;
> +			}
> +		}
> +	}
> +
>   	return 0;
>   }
>   

Re: [PATCH 1/2] hwmon: (pmbus/tps53679) Select page 0 for single-page TPS53676

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

Thanks for the review.

My thought was that read-back should be there
to catch a write-protected PAGE write.

Your approach makes sense, I'll drop the read-back
and just write PAGE 0 in v2.

Thank you.

Regards,
Sanman Pradhan

[PATCH 2/2] hwmon: (pmbus/tps53679) Select page 0 for single-page TPS536C7

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

tps536c7_identify() sets info->pages to 1 for a single-channel part and
then accesses page 0 (writing PMBUS_PHASE) without ensuring PAGE is
actually 0. pmbus_set_page() does not update the PAGE register when
info->pages is 1, so if boot firmware left PAGE set to another value the
PHASE writes and subsequent telemetry may target the wrong page.

Select page 0 explicitly and verify it before configuring PHASE.

Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/tps53679.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index f38c19b8cc43..b68313485511 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -299,6 +299,30 @@ static int tps536c7_identify(struct i2c_client *client,
 	 */
 	info->pages = phases_b ? 2 : 1;
 
+	/*
+	 * pmbus_set_page() does not update the PAGE register on single-page
+	 * devices, so select page 0 explicitly and verify it in case the
+	 * boot firmware left the device on another page.
+	 */
+	if (info->pages == 1) {
+		ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+		if (ret < 0)
+			return ret;
+		if (ret != 0) {
+			ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+			if (ret < 0)
+				return ret;
+			ret = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+			if (ret < 0)
+				return ret;
+			if (ret != 0) {
+				dev_err(&client->dev,
+					"failed to select page 0\n");
+				return -EIO;
+			}
+		}
+	}
+
 	/*
 	 * With info->phases[] left unset the PMBus core never programs the
 	 * PHASE selector, so make sure each page reports the aggregate
-- 
2.34.1