Re: [PATCH v10 10/10] iio: accel: mma8452: Support interrupt sharing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iio: mma8452: interrupt storm during runtime resume and suspend
- [Medium] iio: mma8452: unbalanced regulator disable on removal when runtime suspended
--

commit f941a16ef4ad24cff1f76a14b03513af5d8ea4cd
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Support interrupt sharing

Adding support for sharing interrupt line with other device requires the
interrupt handler to handle runtime PM suspension properly, ignoring the
irq if the device is suspended (maybe even off).

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index dc8031e14c292..f05dd936b7f37 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1086,15 +1088,32 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>  {
>  	struct iio_dev *indio_dev = p;
>  	struct mma8452_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
>  	irqreturn_t ret = IRQ_NONE;
> +	int pm_status;
>  	int src;
>  
> +	pm_status = pm_runtime_get_if_active(dev);
> +	if (pm_status == 0)
> +		/* device is powered down */
> +		return IRQ_NONE;
> +	if (IS_ENABLED(CONFIG_PM) && pm_status < 0)
> +		/* runtime PM was disabled, possibly suspending */
> +		return IRQ_HANDLED;

[Severity: High]
Does returning IRQ_NONE here without clearing the hardware interrupt source
risk causing an interrupt storm?

The driver registers a threaded IRQ handler with IRQF_ONESHOT | IRQF_SHARED.
If an interrupt fires while pm_status is 0, returning IRQ_NONE without
clearing the interrupt means the core will unmask the still-asserted line,
causing an immediate refire and potentially an infinite interrupt storm that
hangs the CPU.

Additionally, doesn't returning IRQ_HANDLED when pm_status < 0 incorrectly
claim shared interrupts raised by other devices, hiding them from the
spurious interrupt detector?

[ ... ]

> @@ -1787,29 +1811,67 @@ static void mma8452_remove(struct i2c_client *client)
>  #ifdef CONFIG_PM
>  static int mma8452_runtime_suspend(struct device *dev)
>  {

[ ... ]

> +	ret = regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
>  	if (ret) {
>  		dev_err(dev, "failed to disable regulators\n");
> -		return ret;
> +		goto out_restore_ctrl_reg4;
>  	}

[Severity: Medium]
Since regulator_bulk_disable() is now called here during runtime suspend,
does this cause unbalanced regulator disables in mma8452_remove()?

In mma8452_remove(), the driver unconditionally calls mma8452_standby() and
regulator_bulk_disable() without checking the runtime PM state or waking up
the device (e.g. via pm_runtime_get_sync()).

If the device was already runtime suspended on removal, its regulators are
off. Calling mma8452_standby() performs an I2C write to an unpowered chip,
resulting in an I2C error, and calling regulator_bulk_disable() a second time
drops the regulator usage count below zero, triggering a kernel warning.

[ ... ]

> @@ -1823,6 +1885,10 @@ static int mma8452_runtime_resume(struct device *dev)
>  	if (ret)
>  		goto runtime_resume_failed;
>  
> +	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4);
> +	if (ret)
> +		goto runtime_resume_failed;
> +
>  	ret = mma8452_active(data);
>  	if (ret < 0)
>  		goto runtime_resume_failed;

[Severity: High]
Does re-enabling hardware interrupts here risk an interrupt storm?

During mma8452_runtime_resume(), the device state is RPM_RESUMING, so
pm_runtime_get_if_active() inside mma8452_interrupt() will return 0.

If an interrupt fires after writing to MMA8452_CTRL_REG4 and before the
resume completes (for example, during the sleep later in this function), the
interrupt handler will return IRQ_NONE without clearing it. As mentioned
above, this can lead to an infinite interrupt storm.

A similar issue appears to exist during mma8452_runtime_suspend() before
CTRL_REG4 is disabled, since the state is RPM_SUSPENDING.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-mma8452-open-drain-v10-0-b906fb408386@geanix.com?part=10