[PATCH v2 0/5] hwmon: (pmbus) Fix bugs in MPS MP2869, MP29502, MP2925, and MP9945 drivers

Pradhan, Sanman posted 5 patches 1 week, 3 days ago
drivers/hwmon/pmbus/mp2869.c  |  8 ++--
drivers/hwmon/pmbus/mp2925.c  |  4 +-
drivers/hwmon/pmbus/mp29502.c | 80 +++++++++++------------------------
drivers/hwmon/pmbus/mp9945.c  | 21 +++------
4 files changed, 37 insertions(+), 76 deletions(-)
[PATCH v2 0/5] hwmon: (pmbus) Fix bugs in MPS MP2869, MP29502, MP2925, and MP9945 drivers
Posted by Pradhan, Sanman 1 week, 3 days ago
From: Sanman Pradhan <psanman@juniper.net>

This series fixes several bugs in the MPS PMBus drivers (mp2869,
mp29502, mp2925, mp9945) recently added to hwmon-next.

Patch 1 fixes the default return value in read_word_data/write_word_data
callbacks. These callbacks return -EINVAL for unhandled registers, but
the PMBus core interprets any negative value other than -ENODATA as
"register does not exist", silently hiding valid standard PMBus
registers. Affects mp2869, mp29502, and mp2925. 

Note: While I do not have physical access to every variant in this series,
the safety justification for the -ENODATA fallback is based on the PMBus 
interface of these MPS families and the existing driver constraints
(func[] flags).

Patch 2 fixes a return type truncation bug in the reg2data_linear11()
helper used by mp2869 and mp29502. The function computes a signed 64-bit
intermediate but returns u16, silently truncating negative or large
values. The fix changes the return type to int and clamps to [0, 0xFFFF],
keeping the conversion semantics local to the helper so all callers get
the same bounded result.

Patches 3-4 replace raw i2c_smbus_*() calls with their corresponding
PMBus core API helpers in mp9945 and mp29502. The raw writes to
PMBUS_PAGE desynchronize the core's internal page cache, causing
subsequent pmbus_read_word_data() calls to skip the page-select and
potentially read from the wrong page. As a secondary benefit, the
switch also routes post-probe accesses through the update_lock mutex.

Patch 5 adds zero-value guards for hardware-derived divisors in mp29502.
These divisors feed DIV_ROUND_CLOSEST() calculations in the
PMBUS_READ_VOUT, PMBUS_READ_POUT, and PMBUS_VOUT_UV_FAULT_LIMIT
handlers as well as the OV-limit helpers. If the hardware returns zero,
a division-by-zero exception occurs at runtime.

Sanman Pradhan (5):
  hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers
  hwmon: (pmbus) Fix return type truncation in MPS reg2data_linear11()
  hwmon: (pmbus/mp9945) Replace raw I2C calls with PMBus core API
  hwmon: (pmbus/mp29502) Replace raw I2C calls with PMBus core API
  hwmon: (pmbus/mp29502) Prevent division by zero from hardware register

 drivers/hwmon/pmbus/mp2869.c  |  8 ++--
 drivers/hwmon/pmbus/mp2925.c  |  4 +-
 drivers/hwmon/pmbus/mp29502.c | 80 +++++++++++------------------------
 drivers/hwmon/pmbus/mp9945.c  | 21 +++------
 4 files changed, 37 insertions(+), 76 deletions(-)

v2:
- Patch 1: Folded explicit per-register -ENODATA cases into the
  default case per review feedback. Added paragraph explaining why
  returning -ENODATA is safe for these chips.
- Patches 2-5: No changes.

-- 
2.34.1

Re: [PATCH v2 0/5] hwmon: (pmbus) Fix bugs in MPS MP2869, MP29502, MP2925, and MP9945 drivers
Posted by Guenter Roeck 1 week, 3 days ago
On 3/23/26 16:33, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> This series fixes several bugs in the MPS PMBus drivers (mp2869,
> mp29502, mp2925, mp9945) recently added to hwmon-next.
> 
> Patch 1 fixes the default return value in read_word_data/write_word_data
> callbacks. These callbacks return -EINVAL for unhandled registers, but
> the PMBus core interprets any negative value other than -ENODATA as
> "register does not exist", silently hiding valid standard PMBus
> registers. Affects mp2869, mp29502, and mp2925.
> 
> Note: While I do not have physical access to every variant in this series,
> the safety justification for the -ENODATA fallback is based on the PMBus
> interface of these MPS families and the existing driver constraints
> (func[] flags).
> 
> Patch 2 fixes a return type truncation bug in the reg2data_linear11()
> helper used by mp2869 and mp29502. The function computes a signed 64-bit
> intermediate but returns u16, silently truncating negative or large
> values. The fix changes the return type to int and clamps to [0, 0xFFFF],
> keeping the conversion semantics local to the helper so all callers get
> the same bounded result.
> 
> Patches 3-4 replace raw i2c_smbus_*() calls with their corresponding
> PMBus core API helpers in mp9945 and mp29502. The raw writes to
> PMBUS_PAGE desynchronize the core's internal page cache, causing
> subsequent pmbus_read_word_data() calls to skip the page-select and
> potentially read from the wrong page. As a secondary benefit, the
> switch also routes post-probe accesses through the update_lock mutex.
> 
> Patch 5 adds zero-value guards for hardware-derived divisors in mp29502.
> These divisors feed DIV_ROUND_CLOSEST() calculations in the
> PMBUS_READ_VOUT, PMBUS_READ_POUT, and PMBUS_VOUT_UV_FAULT_LIMIT
> handlers as well as the OV-limit helpers. If the hardware returns zero,
> a division-by-zero exception occurs at runtime.
> 

Looks like the series does more harm than good. Please check

https://sashiko.dev/#/patchset/20260323233244.201294-1-sanman.pradhan%40hpe.com

Thanks,
Guenter

> Sanman Pradhan (5):
>    hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers
>    hwmon: (pmbus) Fix return type truncation in MPS reg2data_linear11()
>    hwmon: (pmbus/mp9945) Replace raw I2C calls with PMBus core API
>    hwmon: (pmbus/mp29502) Replace raw I2C calls with PMBus core API
>    hwmon: (pmbus/mp29502) Prevent division by zero from hardware register
> 
>   drivers/hwmon/pmbus/mp2869.c  |  8 ++--
>   drivers/hwmon/pmbus/mp2925.c  |  4 +-
>   drivers/hwmon/pmbus/mp29502.c | 80 +++++++++++------------------------
>   drivers/hwmon/pmbus/mp9945.c  | 21 +++------
>   4 files changed, 37 insertions(+), 76 deletions(-)
> 
> v2:
> - Patch 1: Folded explicit per-register -ENODATA cases into the
>    default case per review feedback. Added paragraph explaining why
>    returning -ENODATA is safe for these chips.
> - Patches 2-5: No changes.
>
Re: [PATCH v2 0/5] hwmon: (pmbus) Fix bugs in MPS MP2869, MP29502, MP2925, and MP9945 drivers
Posted by Pradhan, Sanman 1 week, 2 days ago
From: Sanman Pradhan <psanman@juniper.net>

Thank you for the detailed review, I'll stop this series for now
and re-evaluate the arithmetic, page-handling, and sign-extension
assumptions for each driver individually.

I can revisit sending back targeted, standalone fixes once I've verified
they don't introduce telemetry regressions or break page selection.

Thank you.

Regards,
Sanman Pradhan
Re: [PATCH v2 0/5] hwmon: (pmbus) Fix bugs in MPS MP2869, MP29502, MP2925, and MP9945 drivers
Posted by Guenter Roeck 1 week, 2 days ago
On 3/24/26 09:37, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> Thank you for the detailed review, I'll stop this series for now
> and re-evaluate the arithmetic, page-handling, and sign-extension
> assumptions for each driver individually.
> 
> I can revisit sending back targeted, standalone fixes once I've verified
> they don't introduce telemetry regressions or break page selection.
> 

Makes sense.

Thanks,
Guenter