Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Joshua Crofts]

On Mon, 07 Sep 2026 18:36:56 +0200
Esben Haabendal <esben@xxxxxxxxxx> wrote:

> "Joshua Crofts" <joshua.crofts1@xxxxxxxxx> writes:
> 
> > On Mon, 07 Sep 2026 16:51:04 +0200
> > Esben Haabendal <esben@xxxxxxxxxx> 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.
> >>
> >> 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.
> >>
> >> Signed-off-by: Esben Haabendal <esben@xxxxxxxxxx>
> >> ---  
> >
> > ...
> >  
> >> @@ -1784,29 +1796,62 @@ static void mma8452_remove(struct i2c_client *client)
> >>  #ifdef CONFIG_PM
> >>  static int mma8452_runtime_suspend(struct device *dev)
> >>  {
> >> -	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> >> +	struct i2c_client *client = to_i2c_client(dev);
> >> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >>  	struct mma8452_data *data = iio_priv(indio_dev);
> >>  	int ret;
> >>
> >> -	scoped_guard(mutex, &data->lock)
> >> -		ret = mma8452_standby(data);
> >> +	guard(mutex)(&data->lock);
> >> +  
> >
> > Hmm, this patch might benefit from breaking it into multiple patches
> > (the change from scoped_guard to guard, the local pointers etc. along
> > the fact that these aren't mentioned in the commit message), but
> > that's up to Jonathan.  
> 
> I will try and improve the commit message. I don't think that breaking
> it up makes a lot of sense.
> 
> The change from guard() to scoped_guard() is closely tied to the rest of
> the changes. At least I am not sure if it is even sane to make that
> change without the rest of this commit. It might be fine. Probably is,
> but is it worth to risk causing trouble for older kernels?
> 
> Regarding the local pointers, I assume you mean the added struct
> i2c_client and struct device pointers. They are only used in the added
> code, so I don't see how I can split that out in a meaningful way.
> 

Ah fair enough, no need to split it then :)

-- 
Kind regards,
Joshua Crofts