Re: [PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

"Andy Shevchenko" <andriy.shevchenko@xxxxxxxxx> writes:

> On Mon, Aug 31, 2026 at 05:43:06PM +0200, Esben Haabendal wrote:
>> "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxx> writes:
>> > On Mon, Aug 31, 2026 at 02:17:07PM +0200, Esben Haabendal wrote:
>
> ...
>
>> >> +	WRITE_ONCE(data->suspended, false);
>> >> +
>> >>  	ret = mma8452_active(data);
>> >>  	if (ret < 0)
>> >>  		goto runtime_resume_failed;
>> >
>> >>  	return 0;
>> >>
>> >>  runtime_resume_failed:
>> >> +	WRITE_ONCE(data->suspended, true);
>> >>  	regulator_disable(data->vddio_reg);
>> >>  	regulator_disable(data->vdd_reg);
>> >
>> > But with this, what's the point in having WRITE_ONCE()? It can be read
>> > just in the middle as true and be immediately changed afterwards. It
>> > may be that I am missing something, but I think WRITE_ONCE() should be
>> > done once in this function.
>>
>> Yes, there does look like there is still a race condition after adding
>> this data->suspended flag. An irq handler could just have read
>> data->suspended, gotten false, and thereafter proceeeded with handling
>> the irq, and then we write data->suspended=true and the irq handler
>> would just continue with accessing the device, even though we are now
>> (if possible) powering down the device.
>>
>> I did go through all the pre-existing runtime pm and other race
>> condition issues raised by sashiko-bot during this review, and worked
>> through it all. The result is a quite a bit larger than what I would
>> like to add on top of this series. Among other things, it converts the
>> driver to use regmap for accessing the i2c registers, and after various
>> fixes the data->suspended flag is removed again.
>>
>> So I am a bit hesitant to pull all those changes into this series, the
>> combined series would blow up quite a bit. But if required, I guess I
>> can do that, although I fear that it will not make reviewing easier to
>> mix things more than maybe needed.
>
> Personally I would go with it, or as a compromise, publish somewhere in Git
> (like on GitHub) and share the link to the repo (in a format that one can run
> with `git fetch ...`).

Ok. Let me bite the bullet, and try to rebase the part of the next
series that relates to the data->suspended.

I will try and send a new version to this series with the minimal
changes to get rid the data->suspended handling, so we can avoid
introducing it just to throw it away the next day.

>> Could we find a way to merge this series first in some way, or should I
>> post a new version with all the other fixes added on top?
>
> It's a Q to Jonathan.

/Esben