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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Returning -ENODATA does not mask unsupported PMBus limit registers, incorrectly exposing unscaled limit attributes if the hardware responds.
--

--- Patch [2]: [PATCH v5 2/4] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support ---
commit 852e7cf36da1bb5ec1c29eff09bb747755762b5b
Author: Alexis Czezar Torreno <[email protected]>

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

This commit adds support for external voltage divider scaling in the max20830 
driver. It reads resistor values from device tree properties and applies a 
scaling formula to VOUT readings to report the actual output voltage.

> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
[ ... ]
> @@ -14,6 +15,56 @@
[ ... ]
> +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;
> +	u64 temp;
> +
> +	switch (reg) {
> +	case PMBUS_READ_VOUT:
[ ... ]
> +		return ret;
> +	default:
> +		return -ENODATA;

[Severity: Medium]
Does returning -ENODATA here actually mask the unsupported limit registers as
intended?

In the PMBus core framework, returning -ENODATA from the driver's read
callback is treated as a signal to fall back to a standard I2C read. It
does not block the attribute.

If the hardware responds to the limit register reads over standard I2C, the
PMBus core will still create the sysfs attributes. Because scaling is only
applied to PMBUS_READ_VOUT in this patch, these exposed limit attributes
(e.g., in1_max) would report raw, unscaled feedback voltages, while in1_input
reports scaled actual voltages, creating an inconsistent interface.

Should this return -ENXIO instead, which explicitly tells the PMBus core that
the register is unsupported and prevents the sysfs attributes from being
created?

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2