Re: [PATCH v2] hwmon: (pmbus/lm25066) Fix PMBus coefficients for LM5064/5066/5066i

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Multiplication of `info->m` with `shunt` can overflow a 32-bit unsigned integer, leading to drastically incorrect PMBus coefficients.
--

commit afaf15dfc17bb11eae6bb630c7de46aab3eda6b1
Author: Potin Lai <potin.lai.pt@gmail.com>

hwmon: (pmbus/lm25066) Fix PMBus coefficients for LM5064/5066/5066i

This commit swaps the high and low setting coefficients in the lm25066_coeff
table for the LM5064, LM5066, and LM5066i. It also dynamically assigns the
exponent (R) registers inside the LM25066_DEV_SETUP_CL check in the probe
function.

> diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
> --- a/drivers/hwmon/pmbus/lm25066.c
> +++ b/drivers/hwmon/pmbus/lm25066.c
[ ... ]
> @@ -519,18 +519,20 @@ static int lm25066_probe(struct i2c_client *client)
>  	} else {
>  		info->m[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].m;
>  		info->b[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].b;
> +		info->R[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].R;
>  		info->m[PSC_POWER] = coeff[PSC_POWER].m;
>  		info->b[PSC_POWER] = coeff[PSC_POWER].b;
> +		info->R[PSC_POWER] = coeff[PSC_POWER].R;
>  	}
>  
>  	/*

[Severity: High]
This is a pre-existing issue, but could this multiplication overflow a 32-bit
unsigned integer?

Looking at the lines immediately following this block in lm25066_probe():

	info->m[PSC_CURRENT_IN] = info->m[PSC_CURRENT_IN] * shunt / 1000;
	info->m[PSC_POWER] = info->m[PSC_POWER] * shunt / 1000;

The calculations evaluate the multiplication using 32-bit arithmetic because
info->m is an int and shunt is a u32. For large coefficients like 26882 
(LM25056) or 15076 (LM5066i), a device tree shunt-resistor-micro-ohms value
exceeding roughly 159,000 causes the intermediate product to exceed UINT_MAX
(4,294,967,295), silently wrapping around before the division by 1000. 

If the wrapped value has the most significant bit set, converting it back to
the signed int info->m results in negative coefficients. This leads to
incorrect current and power readings, which could cause erratic thermal or
power management behavior.

Should these calculations be cast to a 64-bit integer before multiplication to
avoid the overflow?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-lm25066-driver-fix-v2-1-5d28861b0d8e@quantatw.com?part=1