Re: [PATCH v2 2/2] hwmon: (pmbus/ina233) Handle sign extension for negative shunt voltage

[Guenter Roeck]

On 3/18/26 13:20, Guenter Roeck wrote:
> On 3/18/26 12:40, Pradhan, Sanman wrote:
> > From: Sanman Pradhan <psanman@xxxxxxxxxxx>
> >
> > ina233_read_word_data() reads MFR_READ_VSHUNT, which is a 16-bit
> > two's complement value. Because pmbus_read_word_data() returns an
> > integer, negative voltages (values > 32767) are currently treated as
> > large positive values, leading to incorrect scaling in DIV_ROUND_CLOSEST().
> >
> > Add a cast to (s16) to ensure negative shunt voltages are correctly
> > sign-extended before the scaling calculation is performed.
> >
> > Fixes: b64b6cb163f16 ("hwmon: Add driver for TI INA233 Current and Power Monitor")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>
> > ---
> > v2:
> >    - Added (s16) cast to fix sign-extension for negative shunt voltages,
> >      complementing the error check fix applied in v1
> > ---
> >   drivers/hwmon/pmbus/ina233.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c
> > index dde1e16783943..819f4e8aeab61 100644
> > --- a/drivers/hwmon/pmbus/ina233.c
> > +++ b/drivers/hwmon/pmbus/ina233.c
> > @@ -70,7 +70,7 @@ static int ina233_read_word_data(struct i2c_client *client, int page,
> >           /* Adjust returned value to match VIN coefficients */
> >           /* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */
> > -        ret = DIV_ROUND_CLOSEST(ret * 25, 12500);
> > +        ret = DIV_ROUND_CLOSEST((s16)ret * 25, 12500);
>
> This may end up reporting a negative error value to the caller.
> Should the result be masked against 0xffff ?

Also, as Sashiko reports, "ret" can be a negative error code which needs
to be checked and handled first. And the mask would be wrong - I think it
needs a clamp_val().

Thanks,
Guenter