Re: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The voltage divider scaling calculation suffers from a 32-bit addition overflow and an unsafe 64-bit to 32-bit signed integer truncation before clamping.
--

--- Patch [2]: [PATCH v3 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support ---
commit ac88893286286c633f117369db91c2cffd163afd
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support

This commit implements support for external voltage divider scaling using
the adi,vout-rfb1-ohms and adi,vout-rfb2-ohms device tree properties. The
driver reads these values and applies the scaling formula to the raw VOUT
reading.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d..7a5e67bd33c0a 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c

[ ... ]

> +static int max20830_read_word_data(struct i2c_client *client, int page,
> +				   int phase, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	const struct max20830_data *data = container_of(info, struct max20830_data, info);
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_READ_VOUT:
> +		ret = pmbus_read_word_data(client, page, phase, reg);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Apply voltage divider scaling if resistors are non-zero */
> +		if (data->vout_rfb1 && data->vout_rfb2) {
> +			ret = DIV_ROUND_CLOSEST_ULL((u64)ret * (data->vout_rfb1 +
> +						    data->vout_rfb2), data->vout_rfb2);
> +			ret = clamp_val(ret, 0, 0xFFFF);

[Severity: Medium]
Can this calculation suffer from a 32-bit addition overflow and an unsafe
64-bit to 32-bit signed integer truncation?

If data->vout_rfb1 and data->vout_rfb2 sum to more than 2^32 - 1, the 32-bit
addition will wrap around before the 64-bit multiplication.

Additionally, since ret is a signed 32-bit integer, if the 64-bit result
returned from DIV_ROUND_CLOSEST_ULL exceeds 2^31 - 1, the assignment to ret
will overflow into a negative integer. The clamp_val() macro will then
evaluate this negative number and incorrectly clamp it to 0 instead of
0xFFFF.

> +		}
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-dev-max20830c-v3-0-9d06d27d6da3@analog.com?part=2