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@xxxxxxxxxxxxxxxxxxx>
>> ---
>>   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;
>