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

[Joshua Crofts]

On Mon, 28 Sep 2026 12:26:12 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Mon, Sep 28, 2026 at 10:26:26AM +0200, Esben Haabendal wrote:
> > 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). And while at it, we use
> > the PM reference to ensure we do not get suspended while processing an irq.
> > 
> > In order to prevent the chip from raising irq while suspended (that is when
> > using fixed regulator, where suspend just means setting the device in
> > STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
> > then restores the value again when resuming.
> > 
> > The scoped_guard in mma8452_runtime_suspend() is changed to a plain
> > mutex_lock() instead, both to prevent mixing guards and goto, but also to
> > ensure that we stay in STANDBY mode while writing to CTRL_REG4 and all the
> > way up to disabling the device as much as possible.
> > 
> > With that in place, it is safe to add the IRQF_SHARED flag.
> > 
> > Keep in mind that the device by default is using push-pull for the irq pin,
> > which might require additional hardware design to allow interrupt sharing.  
> 
> ...
> 
> > +	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;  
> 
> This is very interesting part. I bet this will be the first driver using this.
> A big question "why?"
> 
> > +	/*
> > +	 * pm_status is now 1 or -EINVAL (with CONFIG_PM not enabled). If
> > +	 * pm_status==1, runtime PM is enabled and device is RPM_ACTIVE. If
> > +	 * pm_status==-EINVAL, runtime PM is build-time disabled (i.e. CONFIG_PM
> > +	 * not enabled), and we can/must assume device is active.
> > +	 */
> > +
> >  	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
> >  	if (src < 0)
> > -		return IRQ_NONE;
> > +		goto out_runtime_put;
> >  
> >  	if (!(src & (data->chip_info->enabled_events | MMA8452_INT_DRDY)))
> > -		return IRQ_NONE;
> > +		goto out_runtime_put;
> >  
> >  	if (src & MMA8452_INT_DRDY) {
> >  		iio_trigger_poll_nested(indio_dev->trig);
> > @@ -1120,6 +1139,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
> >  		ret = IRQ_HANDLED;
> >  	}
> >  
> > +out_runtime_put:
> > +	if (pm_status > 0)
> > +		pm_runtime_put_autosuspend(dev);
> > +
> >  	return ret;
> >  }  
> 
> ...
> 
> > +	pm_runtime_enable(dev);
> > +	pm_runtime_set_autosuspend_delay(dev, MMA8452_AUTO_SUSPEND_DELAY_MS);
> > +	pm_runtime_use_autosuspend(dev);  
> 
> I don't see the respective _dont_use_autosuspend() call anywhere.

FWIW, starting Linux 7.4 you won't need to add _dont_use_autosuspend()
calls, driver core will just handle it on unbind [1].

If anyone would like to join me in purging the kernel of redundant
dont_use_autosuspend() calls once the patch hits mainline, feel free
to do so :)

[1] https://lore.kernel.org/all/20260919-move-dont-use-autosuspend-v1-1-f6e2d1315c23@xxxxxxxxx/

-- 
Kind regards,
Joshua Crofts