Re: [PATCH v4 05/11] power: supply: max17042_battery: Propagate status register errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Probe failure leaves device interrupts actively enabled on the hardware, potentially causing an interrupt storm.
- [High] Multiple hardware configuration functions ignore the return value of regmap_read, using uninitialized stack variables to write arbitrary values to hardware registers.
- [High] The threaded IRQ handler returns IRQ_HANDLED upon failing to read the device status, defeating the kernel's spurious interrupt detector and risking a hard interrupt storm.
--

--- Patch [5]: [PATCH v4 05/11] power: supply: max17042_battery: Propagate status register errors ---
commit bced78410601fcdc0467a660acdbfd2bb2e4527e
Author: Vincent Cloutier <vincent@cloutier.co>

power: supply: max17042_battery: Propagate status register errors

The driver adds error checking for the initial STATUS read and final POR
clear during initialization. A failed read will now propagate the error
instead of ignoring it, and the POR-clear result is returned directly.

> diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
> index 38abd3604643b..9210bdb4d67eb 100644
> --- a/drivers/power/supply/max17042_battery.c
> +++ b/drivers/power/supply/max17042_battery.c
> @@ -908,8 +908,7 @@ static int max17042_init_chip(struct max17042_chip *chip)
>  	}
>  
>  	/* Init complete, Clear the POR bit */
> -	regmap_update_bits(map, MAX17042_STATUS, STATUS_POR_BIT, 0x0);
> -	return 0;
> +	return regmap_clear_bits(map, MAX17042_STATUS, STATUS_POR_BIT);
>  }
>  
>  static void max17042_set_soc_threshold(struct max17042_chip *chip, u16 off)

[Severity: High]
This is a pre-existing issue, but functions like max17042_set_soc_threshold
and max17042_load_new_capacity_params ignore the return value of regmap_read.

If the I2C read fails, can this lead to uninitialized stack variables
(soc, vfSoc, full_cap0) being bit-shifted and written directly back
to critical hardware registers?

For example:
drivers/power/supply/max17042_battery.c:max17042_set_soc_threshold() {
    ...
    regmap_read(map, MAX17042_RepSOC, &soc);
    soc_tr = (soc + off) << 8;
    ...
}

[ ... ]

> @@ -1248,7 +1247,10 @@ static int max17042_probe(struct i2c_client *client, struct device *dev, int irq
>  
>  	chip->irq = irq;
>  
> -	regmap_read(chip->regmap, MAX17042_STATUS, &val);
> +	ret = regmap_read(chip->regmap, MAX17042_STATUS, &val);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to read status\n");
> +

[Severity: High]
This isn't a bug introduced by this patch, but returning here exposes a missing
cleanup step.

If this new read fails, we return an error and the devm framework frees
the IRQ handler. However, earlier in max17042_probe, we enable hardware alerts
with regmap_update_bits(chip->regmap, MAX17042_CONFIG, CFG_ALRT_BIT_ENBL, ...).

Since there is no devm-managed cleanup or manual unwind for this alert
enablement, and the IRQ is requested with IRQF_SHARED, could this leave the
device actively asserting the interrupt line without a registered handler,
resulting in an interrupt storm?

>  	if (val & STATUS_POR_BIT) {
>  		ret = devm_delayed_work_autocancel(dev, &chip->work,
>  						   max17042_init_worker);

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
max17042_thread_handler():

drivers/power/supply/max17042_battery.c:max17042_thread_handler() {
    ...
    ret = regmap_read(chip->regmap, MAX17042_STATUS, &val);
    if (ret)
        return IRQ_HANDLED;
    ...
}

If regmap_read fails (e.g., due to a transient I2C bus error), the handler
returns IRQ_HANDLED without clearing the interrupt bits on the device.

Since the handler reports success (IRQ_HANDLED) while the physical device
is still asserting the shared interrupt line, does this bypass the kernel's
spurious interrupt detector and risk a hard interrupt storm?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727011319.621794-1-vincent.cloutier@icloud.com?part=5