Re: [PATCH v2 4/4] hwmon: (ina2xx) Decouple in0 and curr1 alarms

Hi Guenter,

On Fri, Aug 21, 2026 at 09:58:14AM -0700, Guenter Roeck wrote:
> Jared,
> 
> On 8/20/26 06:23, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] Writing 0 to an inactive alert limit unintentionally clears the currently active alert, breaking the newly introduced alarm decoupling.
> > [snip]
> > 
> > [Severity: High]
> > Will this unintentionally disable active alerts if a 0 is written to an
> > inactive alert?
> > 
> > Because the read side was updated in this patch to return 0 for inactive
> > alerts, state restoration tools might now read 0 for an inactive alert and
> > subsequently write 0 back to it during initialization.
> > 
> > When ina226_alert_limit_write() is called with val == 0 for that inactive
> > alert, the code just before this hunk unconditionally clears the hardware
> > mask and the newly added line sets data->active_alert = INA2XX_ALERT_NONE.
> > 
> > Since val is 0, the if (val) block is bypassed, leaving all alerts
> > disabled. This seems to allow writes to an inactive alert to silently
> > disable the currently active alert.
> > 
> > Does this function need an early return or guard to ignore writes of 0
> > if the alert being written to is already inactive?
> > 
> Is that a valid concern ?

I don't believe so; this is documented behavior in
Documentation/hwmon/ina2xx.rst:

    Writing 0 to any `*crit` setting clears all `*crit` settings and
    alarms.

I might be gazing into the AI crystal ball a bit too deeply, but I think
a secondary concern suggested in the report is that a tool could read
all limits and later write them back in a way that ends with a zero
write, which would disable an active alert. This is pre-existing
behavior though, and I think that would be more of a script bug due to
the current documentation.

Best,
Jared

> 
> Thanks,
> Guenter
>