Re: [hwmon PATCH v3 5/6] hwmon: (adt7470) Fix temperature alarm logic in hwmon_temp_read()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The bitmask calculation `BIT(channel)` for temperature alarms incorrectly maps channels 7, 8, and 9 due to a discontinuity at bit 7.
--

--- Patch [5]: [hwmon PATCH v3 5/6] hwmon: (adt7470) Fix temperature alarm logic in hwmon_temp_read() ---
commit ab0ed15b014e653c238da0d91110d0699630d8f7
Author: Luiz Angelo Daros de Luca <[email protected]>

hwmon: (adt7470) Fix temperature alarm logic in hwmon_temp_read()

This commit changes the temperature alarm read logic to use a bitmask based
on the channel index, attempting to fix an issue where the channel index
was previously interpreted directly as an alarm bitmask.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index a34635526aaa2..f1150f0fe2172 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -569,7 +569,7 @@ static int adt7470_temp_read(struct device *dev, u32 attr, int channel, long *va
>  		*val = 1000 * data->temp_max[channel];
>  		break;
>  	case hwmon_temp_alarm:
> -		*val = !!(data->alarm & channel);
> +		*val = !!(data->alarm & BIT(channel));

[Severity: High]
Does this bitwise calculation map correctly for higher channel indexes?

If bit 7 of the alarm state is the Out-of-Limit (OOL) global flag, it
appears channel 7 will erroneously report the OOL flag instead of its actual
alarm state.

Could this also cause an off-by-one mapping for the remaining channels,
where channel 8 reports channel 7's alarm and channel 9 reports channel 8's
alarm?

>  		break;
>  	default:
>  		return -EOPNOTSUPP;

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