From: Sanman Pradhan <psanman@juniper.net>

ina233_read_word_data() reads MFR_READ_VSHUNT via pmbus_read_word_data()
but has two issues:

1. The return value is not checked for errors before being used in
   arithmetic. A negative error code from a failed I2C transaction is
   passed directly to DIV_ROUND_CLOSEST(), producing garbage data.

2. MFR_READ_VSHUNT is a 16-bit two's complement value. Negative shunt
   voltages (values with bit 15 set) are treated as large positive
   values since pmbus_read_word_data() returns them zero-extended in an
   int. This leads to incorrect scaling in the VIN coefficient
   conversion.

Fix both issues by adding an error check, casting to s16 for proper
sign extension, and clamping the result to a valid non-negative range.
The clamp is necessary because read_word_data callbacks must return
non-negative values on success (negative values indicate errors to the
pmbus core).

Fixes: b64b6cb163f16 ("hwmon: Add driver for TI INA233 Current and Power Monitor")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/ina233.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c
index dde1e16783943..1f7170372f243 100644
--- a/drivers/hwmon/pmbus/ina233.c
+++ b/drivers/hwmon/pmbus/ina233.c
@@ -67,10 +67,13 @@ static int ina233_read_word_data(struct i2c_client *client, int page,
 	switch (reg) {
 	case PMBUS_VIRT_READ_VMON:
 		ret = pmbus_read_word_data(client, 0, 0xff, MFR_READ_VSHUNT);
+		if (ret < 0)
+			return ret;
 
 		/* Adjust returned value to match VIN coefficients */
 		/* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */
-		ret = DIV_ROUND_CLOSEST(ret * 25, 12500);
+		ret = clamp_val(DIV_ROUND_CLOSEST((s16)ret * 25, 12500),
+				0, 0x7FFF);
 		break;
 	default:
 		ret = -ENODATA;
-- 
2.34.1

On Thu, Mar 19, 2026 at 05:31:19PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> ina233_read_word_data() reads MFR_READ_VSHUNT via pmbus_read_word_data()
> but has two issues:
> 
> 1. The return value is not checked for errors before being used in
>    arithmetic. A negative error code from a failed I2C transaction is
>    passed directly to DIV_ROUND_CLOSEST(), producing garbage data.
> 
> 2. MFR_READ_VSHUNT is a 16-bit two's complement value. Negative shunt
>    voltages (values with bit 15 set) are treated as large positive
>    values since pmbus_read_word_data() returns them zero-extended in an
>    int. This leads to incorrect scaling in the VIN coefficient
>    conversion.
> 
> Fix both issues by adding an error check, casting to s16 for proper
> sign extension, and clamping the result to a valid non-negative range.
> The clamp is necessary because read_word_data callbacks must return
> non-negative values on success (negative values indicate errors to the
> pmbus core).
> 
> Fixes: b64b6cb163f16 ("hwmon: Add driver for TI INA233 Current and Power Monitor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> ---
>  drivers/hwmon/pmbus/ina233.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c
> index dde1e16783943..1f7170372f243 100644
> --- a/drivers/hwmon/pmbus/ina233.c
> +++ b/drivers/hwmon/pmbus/ina233.c
> @@ -67,10 +67,13 @@ static int ina233_read_word_data(struct i2c_client *client, int page,
>  	switch (reg) {
>  	case PMBUS_VIRT_READ_VMON:
>  		ret = pmbus_read_word_data(client, 0, 0xff, MFR_READ_VSHUNT);
> +		if (ret < 0)
> +			return ret;
>  
>  		/* Adjust returned value to match VIN coefficients */
>  		/* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */
> -		ret = DIV_ROUND_CLOSEST(ret * 25, 12500);
> +		ret = clamp_val(DIV_ROUND_CLOSEST((s16)ret * 25, 12500),
> +				0, 0x7FFF);

The clamp should be to 0xffff, not 0x7fff. That is still a positive return
value, but does not drop the sign bit (bit 15).

Thanks,
Guenter

>  		break;
>  	default:
>  		ret = -ENODATA;

On Sun, Mar 22, 2026 at 07:20:43AM -0700, Guenter Roeck wrote:
> On Thu, Mar 19, 2026 at 05:31:19PM +0000, Pradhan, Sanman wrote:
> > From: Sanman Pradhan <psanman@juniper.net>
> > 
> > ina233_read_word_data() reads MFR_READ_VSHUNT via pmbus_read_word_data()
> > but has two issues:
> > 
> > 1. The return value is not checked for errors before being used in
> >    arithmetic. A negative error code from a failed I2C transaction is
> >    passed directly to DIV_ROUND_CLOSEST(), producing garbage data.
> > 
> > 2. MFR_READ_VSHUNT is a 16-bit two's complement value. Negative shunt
> >    voltages (values with bit 15 set) are treated as large positive
> >    values since pmbus_read_word_data() returns them zero-extended in an
> >    int. This leads to incorrect scaling in the VIN coefficient
> >    conversion.
> > 
> > Fix both issues by adding an error check, casting to s16 for proper
> > sign extension, and clamping the result to a valid non-negative range.
> > The clamp is necessary because read_word_data callbacks must return
> > non-negative values on success (negative values indicate errors to the
> > pmbus core).
> > 
> > Fixes: b64b6cb163f16 ("hwmon: Add driver for TI INA233 Current and Power Monitor")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> > ---
> >  drivers/hwmon/pmbus/ina233.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/ina233.c b/drivers/hwmon/pmbus/ina233.c
> > index dde1e16783943..1f7170372f243 100644
> > --- a/drivers/hwmon/pmbus/ina233.c
> > +++ b/drivers/hwmon/pmbus/ina233.c
> > @@ -67,10 +67,13 @@ static int ina233_read_word_data(struct i2c_client *client, int page,
> >  	switch (reg) {
> >  	case PMBUS_VIRT_READ_VMON:
> >  		ret = pmbus_read_word_data(client, 0, 0xff, MFR_READ_VSHUNT);
> > +		if (ret < 0)
> > +			return ret;
> >  
> >  		/* Adjust returned value to match VIN coefficients */
> >  		/* VIN: 1.25 mV VSHUNT: 2.5 uV LSB */
> > -		ret = DIV_ROUND_CLOSEST(ret * 25, 12500);
> > +		ret = clamp_val(DIV_ROUND_CLOSEST((s16)ret * 25, 12500),
> > +				0, 0x7FFF);
> 
> The clamp should be to 0xffff, not 0x7fff. That is still a positive return
> value, but does not drop the sign bit (bit 15).
> 
Never mind though, I'll fix that up myself.

Applied.

Thanks,
Guenter

Thank you for catching that. That's right, clamping to 0xFFFF 
is the correct approach. Appreciate the fix-up and 
applying the series.

Thank you.

Regards,
Sanman Pradhan