Re: [PATCH] hwmon: adm1177: fix sysfs ABI violation and current unit conversion

[Guenter Roeck]

On 3/24/26 11:22, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@xxxxxxxxxxx>
>
> The adm1177 driver exposes the current alert threshold using
> hwmon_curr_max_alarm. Per the hwmon sysfs ABI, *_alarm attributes
> are read-only status flags; the writable threshold should use
> hwmon_curr_max instead.
>
> Additionally, the threshold is stored internally in microamps
> (alert_threshold_ua) but the ABI requires milliamps for currN_max.
> Convert appropriately on both the read and write paths, and
> propagate the return value of adm1177_write_alert_thr() which was
> previously discarded.
>
> Clamp write values to the range the hardware can represent rather
> than rejecting out-of-range input, and use DIV_ROUND_CLOSEST on the
> read path to minimise rounding error during the uA-to-mA conversion.
>
> Fixes: 09b08ac9e8d5 ("hwmon: (adm1177) Add ADM1177 Hot Swap Controller and Digital Power Monitor driver")
> Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>

AI: https://sashiko.dev/#/patchset/20260324182231.228195-1-sanman.pradhan%40hpe.com

Various possible under/overflows.

Thanks,
Guenter

> ---
>   drivers/hwmon/adm1177.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c
> index 8b2c965480e3f..8742b8b5314b6 100644
> --- a/drivers/hwmon/adm1177.c
> +++ b/drivers/hwmon/adm1177.c
> @@ -10,6 +10,7 @@
>   #include <linux/hwmon.h>
>   #include <linux/i2c.h>
>   #include <linux/init.h>
> +#include <linux/minmax.h>
>   #include <linux/module.h>
>   #include <linux/regulator/consumer.h>
>   
> @@ -91,8 +92,8 @@ static int adm1177_read(struct device *dev, enum hwmon_sensor_types type,
>   			*val = div_u64((105840000ull * dummy),
>   				       4096 * st->r_sense_uohm);
>   			return 0;
> -		case hwmon_curr_max_alarm:
> -			*val = st->alert_threshold_ua;
> +		case hwmon_curr_max:
> +			*val = DIV_ROUND_CLOSEST(st->alert_threshold_ua, 1000);
>   			return 0;
>   		default:
>   			return -EOPNOTSUPP;
> @@ -126,9 +127,10 @@ static int adm1177_write(struct device *dev, enum hwmon_sensor_types type,
>   	switch (type) {
>   	case hwmon_curr:
>   		switch (attr) {
> -		case hwmon_curr_max_alarm:
> -			adm1177_write_alert_thr(st, val);
> -			return 0;
> +		case hwmon_curr_max:
> +			val = clamp_val(val, 0,
> +					div_u64(105840000ULL, st->r_sense_uohm));
> +			return adm1177_write_alert_thr(st, val * 1000);
>   		default:
>   			return -EOPNOTSUPP;
>   		}
> @@ -156,7 +158,7 @@ static umode_t adm1177_is_visible(const void *data,
>   			if (st->r_sense_uohm)
>   				return 0444;
>   			return 0;
> -		case hwmon_curr_max_alarm:
> +		case hwmon_curr_max:
>   			if (st->r_sense_uohm)
>   				return 0644;
>   			return 0;
> @@ -170,7 +172,7 @@ static umode_t adm1177_is_visible(const void *data,
>   
>   static const struct hwmon_channel_info * const adm1177_info[] = {
>   	HWMON_CHANNEL_INFO(curr,
> -			   HWMON_C_INPUT | HWMON_C_MAX_ALARM),
> +			   HWMON_C_INPUT | HWMON_C_MAX),
>   	HWMON_CHANNEL_INFO(in,
>   			   HWMON_I_INPUT),
>   	NULL