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

> > +             *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?

Sashiko is right here:

       err = regmap_read(data->regmap, ADT7470_REG_ALARM1, &val);
       if (err < 0)
               return err;
       data->alarm = val;
       if (data->alarm & ADT7470_OOL_ALARM) {
               err = regmap_read(data->regmap, ADT7470_REG_ALARM2, &val);
               if (err < 0)
                       return err;
               data->alarm |= ALARM2(val);
       }

The BIT(7) is the hint to read ADT7470_REG_ALARM2. So the bitmap is
not a direct BIT(channel) for channels 8, 9, 10.
The data->alarm is also used for fan alarm:

        case hwmon_fan_alarm:
               *val = !!(data->alarm & (1 << (12 + channel)));
               break;

It is too confusing to use the same variable for two different uses
and it is exposing the HW bitmap instead of translating it.
I'll introduce temp_alarm and a fan_alarm and keep the bitmap simpler.

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

Regards,

Luiz