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

[Esben Haabendal <esben@geanix.com>]

"Joshua Crofts" <joshua.crofts1@gmail.com> writes:

> On Mon, 07 Sep 2026 16:51:04 +0200
> Esben Haabendal <esben@geanix.com> 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@geanix.com>
>> ---
>
> ...
>
>> @@ -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.

I don't mind making such changes, I just don't see what I am expected to
do.

/Esben