[PATCH v0 0/2] Add support for tps546e25

[PATCH v0 0/2] Add support for tps546e25

[Aryan Srivastava]

Add support to existing generic pmbus driver for tps546e25. 

Aryan Srivastava (2):
  hwmon: pmbus: Fix vout_mode parsing
  hwmon: pmbus: Add tps546e25 to generic pmbus

 drivers/hwmon/pmbus/pmbus.c      | 3 ++-
 drivers/hwmon/pmbus/pmbus_core.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.55.0

[PATCH v0 1/2] hwmon: pmbus: Fix vout_mode parsing

[Aryan Srivastava]

Currently this parsing assumes the top-most bit of the register is
unset, allowing it to match against all 2bit numbers. In the case where
the top bit is set, the parsing fails as the extracted value is always a
3 bit number.

AND the read value after the bit shift to ensure only 2 bits are being
parsed out.

Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
 drivers/hwmon/pmbus/pmbus.c      | 2 +-
 drivers/hwmon/pmbus/pmbus_core.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index 7a7eac90d62e..b629acbffbfb 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -121,7 +121,7 @@ static int pmbus_identify(struct i2c_client *client,
 
 		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
 		if (vout_mode >= 0 && vout_mode != 0xff) {
-			switch (vout_mode >> 5) {
+			switch ((vout_mode >> 5) & 0x3) {
 			case 0:
 				break;
 			case 1:
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 5f69c1420b4e..55391db5b414 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2753,7 +2753,7 @@ static int pmbus_identify_common(struct i2c_client *client,
 		 * Not all chips support the VOUT_MODE command,
 		 * so a failure to read it is not an error.
 		 */
-		switch (vout_mode >> 5) {
+		switch ((vout_mode >> 5) & 0x3) {
 		case 0:	/* linear mode      */
 			if (data->info->format[PSC_VOLTAGE_OUT] != linear)
 				return -ENODEV;
-- 
2.55.0

[PATCH v0 2/2] hwmon: pmbus: Add tps546e25 to generic pmbus

[Aryan Srivastava]

Similar to other TI DCDC devices. Operates over SMBus and is comapatible
with the generic pmbus driver.

Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
 drivers/hwmon/pmbus/pmbus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index b629acbffbfb..96a72950e7d3 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -245,6 +245,7 @@ static const struct i2c_device_id pmbus_id[] = {
 	{ .name = "tps544b25", .driver_data = (kernel_ulong_t)&pmbus_info_one },
 	{ .name = "tps544c20", .driver_data = (kernel_ulong_t)&pmbus_info_one },
 	{ .name = "tps544c25", .driver_data = (kernel_ulong_t)&pmbus_info_one },
+	{ .name = "tps546e25", .driver_data = (kernel_ulong_t)&pmbus_info_zero },
 	{ .name = "udt020", .driver_data = (kernel_ulong_t)&pmbus_info_one },
 	{ }
 };
-- 
2.55.0

Re: [PATCH v0 1/2] hwmon: pmbus: Fix vout_mode parsing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch truncates the PMBus VOUT_MODE mode field from 3 bits to 2 bits, violating the PMBus specification, wrongly accepting reserved modes, and introducing dead code.
--

commit b7e943858dbfad31b0cd1d29753290823efc6361
Author: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>

hwmon: pmbus: Fix vout_mode parsing

This patch masks the vout_mode parsing in the PMBus core with 0x3, intending
to handle cases where devices unexpectedly set the top-most bit of the
VOUT_MODE register.

> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index 7a7eac90d62e5..b629acbffbfb6 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -121,7 +121,7 @@ static int pmbus_identify(struct i2c_client *client,
>  
>  		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
>  		if (vout_mode >= 0 && vout_mode != 0xff) {
> -			switch (vout_mode >> 5) {
> +			switch ((vout_mode >> 5) & 0x3) {

[Severity: High]
Does this change incorrectly alias reserved VOUT_MODE values (4-7) to standard
modes (0-3)?

By masking the top 3 bits down to 2 bits, if a PMBus chip reports a reserved or
manufacturer-specific mode (such as mode 4), it will now be parsed as a
standard mode (like linear mode 0) instead of being properly rejected.
Misinterpreting the voltage format could cause erroneous voltage calculations,
which might lead to system instability or physical hardware damage from
overvoltage.

Should device-specific quirks (such as a specific device erroneously setting
bit 7) be handled via a custom .read_byte_data callback or a quirk in a
dedicated device driver, rather than breaking PMBus protocol compliance for
all devices in the generic pmbus_identify() logic?

>  			case 0:
>  				break;
>  			case 1:

[ ... ]

> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 806c9a4913bb0..60ddc49afb3f7 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2751,7 +2751,7 @@ static int pmbus_identify_common(struct i2c_client *client,
>  		 * Not all chips support the VOUT_MODE command,
>  		 * so a failure to read it is not an error.
>  		 */
> -		switch (vout_mode >> 5) {
> +		switch ((vout_mode >> 5) & 0x3) {

[Severity: High]
Does masking with 0x3 here introduce dead code in pmbus_identify_common()?

Because the evaluated value is strictly restricted to 0, 1, 2, or 3, and all
four of those cases are explicitly handled below this switch, the default case
and its error handling (return -ENODEV;) can never be reached.

>  		case 0:	/* linear mode      */
>  			if (data->info->format[PSC_VOLTAGE_OUT] != linear)
>  				return -ENODEV;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904005351.2136374-1-aryan.srivastava@alliedtelesis.co.nz?part=1

Re: [PATCH v0 2/2] hwmon: pmbus: Add tps546e25 to generic pmbus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] The new `tps546e25` device entry incorrectly uses `pmbus_info_zero` instead of `pmbus_info_one`, which forces unnecessary dynamic page probing.
- [Low] The new compatible string is not documented in the device tree bindings.
- [Low] The new device is missing from the generic PMBus driver documentation.

Pre-existing issues:
- [High] Potential NULL pointer dereference of `device_info` in `pmbus_probe()`.
--

commit fc4eef8dbc6c89c58d89c8feeedc224bbb325745
Author: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
Subject: hwmon: pmbus: Add tps546e25 to generic pmbus

This commit adds support for the tps546e25 device to the generic PMBus driver.
It operates over SMBus and is similar to other TI DCDC devices.

> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index b629acbffbfb6..96a72950e7d36 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -245,6 +245,7 @@ static const struct i2c_device_id pmbus_id[] = {
>  	{ .name = "tps544b25", .driver_data = (kernel_ulong_t)&pmbus_info_one },
>  	{ .name = "tps544c20", .driver_data = (kernel_ulong_t)&pmbus_info_one },
>  	{ .name = "tps544c25", .driver_data = (kernel_ulong_t)&pmbus_info_one },
> +	{ .name = "tps546e25", .driver_data = (kernel_ulong_t)&pmbus_info_zero },

[Severity: Medium]
Since this is similar to other TI DCDC devices, should this use pmbus_info_one
instead of pmbus_info_zero?

Using pmbus_info_zero forces dynamic page probing, which causes unnecessary
I2C/SMBus traffic. If the device ignores the data but ACKs out-of-range page
writes, it could expose 32 identical phantom pages in sysfs.

>  	{ .name = "udt020", .driver_data = (kernel_ulong_t)&pmbus_info_one },
>  	{ }
>  };


[Severity: Low]
Is it intentional that the new device is omitted from the device tree bindings?

Adding ti,tps546e25 to Documentation/devicetree/bindings/trivial-devices.yaml
would prevent dtbs_check warnings when this compatible string is used.


[Severity: Low]
Could we also update the supported hardware documentation in
Documentation/hwmon/pmbus.rst?

Adding the TPS546E25 to the list of Texas Instruments devices and its prefix
'tps546e25' would ensure users know it is supported by this driver.


[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer dereference
in pmbus_probe().

drivers/hwmon/pmbus/pmbus.c:pmbus_probe() {
    ...
	device_info = (struct pmbus_device_info *)i2c_get_match_data(client);
	if (device_info->flags) {
    ...
}

If the driver is bound manually via sysfs (e.g., using driver_override)
without an I2C ID that matches the driver's id_table, i2c_get_match_data()
will return NULL. The code immediately dereferences device_info->flags,
which will cause a kernel crash.

Does this need to be checked before dereferencing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904005351.2136374-1-aryan.srivastava@alliedtelesis.co.nz?part=2

Re: [PATCH v0 1/2] hwmon: pmbus: Fix vout_mode parsing

[Guenter Roeck]

On 9/3/26 17:53, Aryan Srivastava wrote:
> Currently this parsing assumes the top-most bit of the register is
> unset, allowing it to match against all 2bit numbers. In the case where
> the top bit is set, the parsing fails as the extracted value is always a
> 3 bit number.
> 
> AND the read value after the bit shift to ensure only 2 bits are being
> parsed out.
> 

This isn't that easy. Bit 7 is for "relative mode" starting with
PMBus v1.3. Relative mode is not currently supported by the driver,
and ignoring the bit could potentially be fatal.

A patch adding support for it was submitted a while ago.
https://patchwork.kernel.org/project/linux-hwmon/patch/20240315151855.377627-2-lars.petter.mostad@appear.net/
Unfortunately I never got to test it. This or something similar will
be needed. Again, we can not just ignore the "relative mode" bit.

Thanks,
Guenter
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
> ---
>   drivers/hwmon/pmbus/pmbus.c      | 2 +-
>   drivers/hwmon/pmbus/pmbus_core.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index 7a7eac90d62e..b629acbffbfb 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -121,7 +121,7 @@ static int pmbus_identify(struct i2c_client *client,
>   
>   		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
>   		if (vout_mode >= 0 && vout_mode != 0xff) {
> -			switch (vout_mode >> 5) {
> +			switch ((vout_mode >> 5) & 0x3) {
>   			case 0:
>   				break;
>   			case 1:
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 5f69c1420b4e..55391db5b414 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2753,7 +2753,7 @@ static int pmbus_identify_common(struct i2c_client *client,
>   		 * Not all chips support the VOUT_MODE command,
>   		 * so a failure to read it is not an error.
>   		 */
> -		switch (vout_mode >> 5) {
> +		switch ((vout_mode >> 5) & 0x3) {
>   		case 0:	/* linear mode      */
>   			if (data->info->format[PSC_VOLTAGE_OUT] != linear)
>   				return -ENODEV;

Re: [PATCH v0 1/2] hwmon: pmbus: Fix vout_mode parsing

[Guenter Roeck]

On 9/3/26 17:53, Aryan Srivastava wrote:
> Currently this parsing assumes the top-most bit of the register is
> unset, allowing it to match against all 2bit numbers. In the case where
> the top bit is set, the parsing fails as the extracted value is always a
> 3 bit number.
> 
> AND the read value after the bit shift to ensure only 2 bits are being
> parsed out.
> 

Another note: Why "v0" ? What is this supposed to mean ?

Guenter

> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
> ---
>   drivers/hwmon/pmbus/pmbus.c      | 2 +-
>   drivers/hwmon/pmbus/pmbus_core.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
> index 7a7eac90d62e..b629acbffbfb 100644
> --- a/drivers/hwmon/pmbus/pmbus.c
> +++ b/drivers/hwmon/pmbus/pmbus.c
> @@ -121,7 +121,7 @@ static int pmbus_identify(struct i2c_client *client,
>   
>   		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
>   		if (vout_mode >= 0 && vout_mode != 0xff) {
> -			switch (vout_mode >> 5) {
> +			switch ((vout_mode >> 5) & 0x3) {
>   			case 0:
>   				break;
>   			case 1:
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 5f69c1420b4e..55391db5b414 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2753,7 +2753,7 @@ static int pmbus_identify_common(struct i2c_client *client,
>   		 * Not all chips support the VOUT_MODE command,
>   		 * so a failure to read it is not an error.
>   		 */
> -		switch (vout_mode >> 5) {
> +		switch ((vout_mode >> 5) & 0x3) {
>   		case 0:	/* linear mode      */
>   			if (data->info->format[PSC_VOLTAGE_OUT] != linear)
>   				return -ENODEV;

Re: [PATCH v0 1/2] hwmon: pmbus: Fix vout_mode parsing

[Aryan Srivastava]

On 05/09/2026 3:04 AM, Guenter Roeck wrote:
> On 9/3/26 17:53, Aryan Srivastava wrote:
>> Currently this parsing assumes the top-most bit of the register is
>> unset, allowing it to match against all 2bit numbers. In the case where
>> the top bit is set, the parsing fails as the extracted value is always a
>> 3 bit number.
>>
>> AND the read value after the bit shift to ensure only 2 bits are being
>> parsed out.
>>
> 
> This isn't that easy. Bit 7 is for "relative mode" starting with
> PMBus v1.3. Relative mode is not currently supported by the driver,
> and ignoring the bit could potentially be fatal.
> 
> A patch adding support for it was submitted a while ago.
> https://scanmail.trustwave.com/?c=20988&d=h96a6tb7GtM2aFa8R9tAfvF3itFMYHQzcKLB8ZK4kA&u=https%3a%2f%2fpatchwork%2ekernel%2eorg%2fproject%2flinux-hwmon%2fpatch%2f20240315151855%2e377627-2-lars%2epetter%2emostad%40appear%2enet%2f
> Unfortunately I never got to test it. This or something similar will
> be needed. Again, we can not just ignore the "relative mode" bit.
> 
> Thanks,
> Guenter

Hi Guenter,

Thank you for your reply. I agree simply ignoring the top bit is not an option.
My initial patch was short-sighted.

I will test the above patch on my HW. I also looked into the driver for the 
tps546d24, but on the tps546e25 (my HW) the relative mode bit is read only.

Thanks,
Aryan.
>> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
>> ---
>>   drivers/hwmon/pmbus/pmbus.c      | 2 +-
>>   drivers/hwmon/pmbus/pmbus_core.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
>> index 7a7eac90d62e..b629acbffbfb 100644
>> --- a/drivers/hwmon/pmbus/pmbus.c
>> +++ b/drivers/hwmon/pmbus/pmbus.c
>> @@ -121,7 +121,7 @@ static int pmbus_identify(struct i2c_client *client,
>>             vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
>>           if (vout_mode >= 0 && vout_mode != 0xff) {
>> -            switch (vout_mode >> 5) {
>> +            switch ((vout_mode >> 5) & 0x3) {
>>               case 0:
>>                   break;
>>               case 1:
>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
>> index 5f69c1420b4e..55391db5b414 100644
>> --- a/drivers/hwmon/pmbus/pmbus_core.c
>> +++ b/drivers/hwmon/pmbus/pmbus_core.c
>> @@ -2753,7 +2753,7 @@ static int pmbus_identify_common(struct i2c_client *client,
>>            * Not all chips support the VOUT_MODE command,
>>            * so a failure to read it is not an error.
>>            */
>> -        switch (vout_mode >> 5) {
>> +        switch ((vout_mode >> 5) & 0x3) {
>>           case 0:    /* linear mode      */
>>               if (data->info->format[PSC_VOLTAGE_OUT] != linear)
>>                   return -ENODEV;
>