Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error

[Jonathan Cameron]

On Mon, 12 Apr 2021 00:38:41 +0200
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> On Sun, Apr 11, 2021 at 5:07 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > On Wed,  7 Apr 2021 11:49:27 +0800
> > Dinghao Liu <dinghao.liu@xxxxxxxxxx> wrote:
> >  
> > > When devm_request_threaded_irq() fails, we should decrease the
> > > runtime PM counter to keep the counter balanced. But when
> > > iio_device_register() fails, we need not to decrease it because
> > > we have already decreased it before.  
> >
> > Whilst agree with your assessment that the code is wrong, I'm not
> > totally sure why we need to do the pm_runtime_get_noresume() in
> > the first place.   Why do we need to hold the reference for
> > the operations going on here?  What can race against this that
> > might care about that reference count?  
> 
> pm_runtime_get_noresume() is increasing the runtime PM
> reference without calling the pm_runtime_resume() callback.
> 
> It is often called in sequence like this:
> 
>     pm_runtime_get_noresume(dev);
>     pm_runtime_set_active(dev);
>     pm_runtime_enable(dev);
> 
> This increases the reference, sets the device as active
> and enables runtime PM.
> 
> The reason that probe() has activated resources such as
> enabling two regulators, and want to leave them on so that
> later on pm_runtime_suspend() will disable them, i.e.
> handover to runtime PM with the device in resumed state.
> 
> I hope this is answering the question, not sure.

There are drivers that look the same except they aren't
holding the reference.  Are those immediately disabling the power?
I can't see the path by which that happens, but perhaps I'm just
missing something?   Maybe this is just paranoid locking in
a probe path (before we are in a position where races can occur)?

An example would be the bmc150_magn driver which does exactly the
same call sequence as this one, but without the reference count increment
and decrement.  Basically I want to know if there is a problem in
those other drivers that is being protected against here!

> 
> Yours,
> Linus Walleij