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
