Re: [PATCH v4 2/6] iio: accel: mma8452: Optimize struct mm8452_data member orders

"Jonathan Cameron" <[email protected]> writes:

> On Mon, 17 Aug 2026 19:10:46 +0200
> Esben Haabendal <[email protected]> wrote:
>
>> "Jonathan Cameron" <[email protected]> writes:
>>
>> > On Wed, 12 Aug 2026 16:30:33 +0200
>> > Esben Haabendal <[email protected]> wrote:
>> >
>> >> Reorder struct mma8452_data members to avoid holes.
>> > Trivial but sashiko pointed out mm[a]_8452 in the title.
>> > I'll tidy that up if nothing significant comes up.
>> >
>> > Interestingly Sashiko also thinks it found a deadlock.
>> > Given you are working with this driver if you have time could
>> > you take a look at that.
>> >
>> > https://sashiko.dev/#/patchset/20260812-mma8452-open-drain-v4-0-bfca15d02b59%40geanix.com
>> >
>> > I'm rather surprised to see the lock taken in the runtime
>> > pm suspend callback. It is probably there to close a race
>> > where the device is being suspended and the sampling
>> > frequency is being written.  I'm not immediately sure what
>> > the best way to fix it is. One thing that would work is to
>> > do pm_runtime_get* to raise the reference counter and stop
>> > there being any chance of an autosuspend.
>>
>> The mma8452_change_config() function is grabbing &data->lock, and is
>> forcing chip in stand mode while applying change. This needs to be
>> synchronized with mma8452_runtime_suspend() switching to standby mode,
>> to avoid a race condition where mma8452_change_config() would end up
>> undoing the change made by runtime PM, due to it keeping the old state
>> in is_active local variable. So far it makes sense.
>>
>> But I don't see the reason for mma8452_read_raw(IIO_CHAN_INFO_RAW)
>> grabbing the lock for the call to mma8452_read(). And when
>> mma8452_trigger_handler() calls mma8452_read() it does so without
>> data->lock held.
>>
>> So maybe we we can simply drop the &data->lock from mma8452_read_raw()?
>>
> That does look safe to do to me as well. Everything used is
> either reference counted (runtime pm in particular) or local variables
> so there doesn't seem to be any reason to take the lock for it.

I can add a patch with this change if another revision is needed, or
will send it separately otherwise.

/Esben