Messages in this thread | | | Date | Mon, 12 Apr 2021 11:15:06 +0100 | From | Jonathan Cameron <> | Subject | Re: [PATCH] iio: light: gp2ap002: Fix rumtime PM imbalance on error |
| |
On Mon, 12 Apr 2021 00:38:41 +0200 Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Apr 11, 2021 at 5:07 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 7 Apr 2021 11:49:27 +0800 > > Dinghao Liu <dinghao.liu@zju.edu.cn> 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
| |