Re: [PATCH v10 03/10] iio: accel: mma8452: Fix runtime PM bugs in mma8452_read()

[Esben Haabendal <esben@geanix.com>]

"Jonathan Cameron" <jic23@kernel.org> writes:

> On Mon, 28 Sep 2026 10:26:19 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> When runtime PM support was added [1], the implementation in mma8452_read()
>> was done incorrectly, as it calls mma8452_drdy() before ensuring runtime pm
>> has set the device active, so it can cause read from STATUS register while
>> device is suspended/off. At the same time, the return value from
>> i2c_smbus_read_i2c_block_data() was discarded, and the function would
>> return the value returned from pm_runtime_put_autosuspend() instead.
>>
>> When the latter bug described above was fixed [2], another bug was
>> introduced, as mma8452_read() would now leak the runtime PM reference
>> counter when i2c_smbus_read_i2c_block_data() failed.
>>
>> This commit corrects this mess.
>>
>> [1] commit 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
>> [2] commit 5bdff291d20c ("iio: accel: mma8452: handle I2C read error(s) in mma8452_read()")
>>
>> Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>
> I suppose not directly relevant to this patch but that
> set_runtime_pm_state() helper is definitely not helping.
> I'd follow up with ripping that out and calling the two runtimme
> pm calls directly.  Then you would be trivially able to use
> ACQUIRE_RUNTIME_PM() etc to reduce complexity in places like this.

I have this queued up pending the merge of this series.

> In more recent drivers I've always pushed back against that
> sort of helper precisely because it stops other sensible refactors.
>
> In general that also follows the common pattern of not checking for
> errors in the put as that trips up on things like !CONFIG_PM and
> is generally not that useful!
>
> Jonathan

/Esben