On Sat, Jun 17, 2017 at 11:37:11AM +0300, Andy Shevchenko wrote: > On Sat, Jun 17, 2017 at 9:32 AM, Tomasz Figa <[email protected]> wrote: > > On Sat, Jun 17, 2017 at 9:00 AM, Zhi, Yong <[email protected]> wrote: > >>> On Thu, Jun 15, 2017 at 1:19 AM, Yong Zhi <[email protected]> wrote: > > >>> > + /* Set Power */ > >>> > + r = pm_runtime_get_sync(dev); > >>> > + if (r < 0) { > >>> > + dev_err(dev, "failed to set imgu power\n"); > >>> > >>> > + pm_runtime_put(dev); > >>> > >>> I'm not sure it's a right thing to do. > >>> How did you test runtime PM counters in this case? > >>> > >>> > + return r; > >>> > + } > > >> Actually I have not tested the error case, what the right way to do in > >> your opinion? there is no checking of this function return in lot of the > >> driver code, or simply returning the error code, I also saw examples to > >> call either pm_runtime_put() or pm_runtime_put_noidle() in this case. > > > > Instead of speculating, if we inspect pm_runtime_get_sync() [1], we > > can see that it always causes the runtime PM counter to increment, but > > it never decrements it, even in case of error. So to keep things > > balanced, you need to call pm_runtime_put() in error path. > > > > It shouldn't matter if it's pm_runtime_put() or > > pm_runtime_put_noidle(), because of runtime PM semantics, which are > > explicitly specified [2] that after an error, no hardware state change > > is attempted until the state is explicitly reset by the driver with > > either pm_runtime_set_active() or pm_runtime_set_suspended(). > > > > So, as far as I didn't miss some even more obscure bits of the runtime > > PM framework, current code is fine. > > Indeed. Thanks for explanation. PM runtime is hard :-) > Previously I didn't meet (and actually never used) check for returning > code of pm_runtime_get*().
Yeah, depending on what is actually done it might fail. pm_runtime_put() isn't wrong but pn_runtime_put_noidle() is sufficient: powering the device on just failed so it's already off. -- Sakari Ailus e-mail: [email protected] XMPP: [email protected]
