Re: [PATCH 1/4] hwmon: (ina2xx) Replace regmap in ina226_alert_read() with ina2xx_data
Jared Kangas <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-hwmon |
|---|---|
| Message-ID | <aoYVS8htMnz8vLQb@rh-jkangas-kernel> |
On Wed, Jul 29, 2026 at 03:52:53PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > [snip] > > > -static int ina226_alert_read(struct regmap *regmap, u32 mask, long *val) > > +static int ina226_alert_read(struct ina2xx_data *data, u32 mask, long *val) > > { > > unsigned int regval; > > int ret; > > > > - ret = regmap_read_bypassed(regmap, INA226_MASK_ENABLE, ®val); > > + ret = regmap_read_bypassed(data->regmap, INA226_MASK_ENABLE, ®val); > > [Severity: High] > This is a pre-existing issue, but I have a question about how the hardware > alert state is managed here in ina226_alert_read(). > > Since the INA226_MASK_ENABLE register clears the Alert Function Flag when > read, does this bypassed hardware read clear the alert flag unintentionally > if an inactive alarm attribute is polled? > > For example, if userspace utilities sequentially read sysfs alarm attributes, > could checking an inactive alarm trigger this clear-on-read behavior before > the actually active alarm is polled? It seems this sequence would cause the > active alarm to falsely report as inactive (0). > > > if (ret) > > return ret; Yes, the current behavior of the driver is to clear alerts even when an inactive alarm is polled. This is addressed in patch 4/4. > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260729-upstream-ina2xx-in0-curr1-alarms-v1-0-349f7b2f1df8@redhat.com?part=1