generic runtime pm callbacks

generic runtime pm callbacks

[Ohad Ben-Cohen]

I'm adding Runtime PM support to the SDIO subsystem, and was planning
to use the generic runtime pm handlers.

Those handlers return -EINVAL if the driver didn't explicitly define
the runtime pm handlers:

That may result in some drivers defining nop handlers, just to return
0 (in case there's nothing else they need to do).

Do we want that ?

Alternatively, we may want to allow drivers to enable Runtime PM (by
taking the appropriate action for their subsystem, e.g. calling
put_noidle in probe and get_noresume in remove), but still not define
any runtime pm handlers (implicitly always returning a success), with
something like:

diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.
index 304c831..531762a 100644
--- a/drivers/base/power/generic_ops.c
+++ b/drivers/base/power/generic_ops.c
@@ -49,7 +49,7 @@ int pm_generic_runtime_suspend(struct device *dev)
       const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
       int ret;

-       ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : -EINVAL;
+       ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;

       return ret;
 }
@@ -68,7 +68,7 @@ int pm_generic_runtime_resume(struct device *dev)
       const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
       int ret;

-       ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : -EINVAL;
+       ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0;

       return ret;
 }

What do you think ?

Please note that the generic pm_generic_runtime_idle handler does
allow drivers not to explicitly define a runtime_idle handler.

Thanks,
Ohad.

Re: [linux-pm] generic runtime pm callbacks

[Mark Brown]

On Mon, Sep 06, 2010 at 03:32:09PM +0300, Ohad Ben-Cohen wrote:

> That may result in some drivers defining nop handlers, just to return
> 0 (in case there's nothing else they need to do).

> Do we want that ?

Funnily enough I was about to report this issue too - it doesn't look
great in the driver code.  My use case is using runtime PM in an MFD to
communicate status to the parent devices.  The subdevices are just
indicating that they are idle to the parent and have no reason to do
anything in a suspend or resume callback.

> Alternatively, we may want to allow drivers to enable Runtime PM (by
> taking the appropriate action for their subsystem, e.g. calling
> put_noidle in probe and get_noresume in remove), but still not define
> any runtime pm handlers (implicitly always returning a success), with
> something like:

This would be my preferred solution.

Re: [linux-pm] generic runtime pm callbacks

[Alan Stern]

On Mon, 6 Sep 2010, Mark Brown wrote:

> On Mon, Sep 06, 2010 at 03:32:09PM +0300, Ohad Ben-Cohen wrote:
> 
> > That may result in some drivers defining nop handlers, just to return
> > 0 (in case there's nothing else they need to do).
> 
> > Do we want that ?
> 
> Funnily enough I was about to report this issue too - it doesn't look
> great in the driver code.  My use case is using runtime PM in an MFD to
> communicate status to the parent devices.  The subdevices are just
> indicating that they are idle to the parent and have no reason to do
> anything in a suspend or resume callback.
> 
> > Alternatively, we may want to allow drivers to enable Runtime PM (by
> > taking the appropriate action for their subsystem, e.g. calling
> > put_noidle in probe and get_noresume in remove), but still not define
> > any runtime pm handlers (implicitly always returning a success), with
> > something like:
> 
> This would be my preferred solution.

Here's an excerpt from a message I posted recently:

-------------------------------------------------------------------
The real issue with USB interfaces is that they are only logical
sub-devices; they can't be power-managed separately from their parent.  
Indeed, the runtime_suspend and runtime_resume callbacks for interfaces
always return 0 immediately without doing anything, and the
runtime_idle callback always calls pm_runtime_suspend.

So why not tell the PM core about this behavior?  Let's put a
"no_callbacks" flag in dev_pm_info.  When this flag is set the core
won't bother invoking the runtime callbacks; it can always assume
success.  More importantly, an asynchronous request may never need to
use the workqueue:

	__pm_request_idle can directly call __pm_request_suspend.

	If the device's state is ACTIVE then __pm_request_suspend
	can switch the state directly to SUSPENDED.

	If the device's state is SUSPENDED and the parent's state
	is ACTIVE then __pm_request_resume can switch the device
	directly to ACTIVE.  (If the parent isn't ACTIVE then we
	need to proceed as we do now.)

With those shortcuts added there won't be any need to use delayed 
autosuspend for interfaces.  I don't know if any other subsystems would 
want to use the no_callbacks flag, but it's possible.
-------------------------------------------------------------------

Does that look like what you're talking about?

Alan Stern

Re: [linux-pm] generic runtime pm callbacks

[Mark Brown]

On Mon, Sep 06, 2010 at 10:30:05AM -0400, Alan Stern wrote:

> The real issue with USB interfaces is that they are only logical
> sub-devices; they can't be power-managed separately from their parent.  
> Indeed, the runtime_suspend and runtime_resume callbacks for interfaces
> always return 0 immediately without doing anything, and the
> runtime_idle callback always calls pm_runtime_suspend.


> So why not tell the PM core about this behavior?  Let's put a
> "no_callbacks" flag in dev_pm_info.  When this flag is set the core
> won't bother invoking the runtime callbacks; it can always assume
> success.  More importantly, an asynchronous request may never need to
> use the workqueue:

...

> Does that look like what you're talking about?

That'd cover my use case, though it would still be nice to have the
flexibility to just omit individual callbacks in the same way that it's
possible to do so with regular suspend and resume (eg, if you only have
to restore some state after the bus has suspended your device).

Re: [linux-pm] generic runtime pm callbacks

[Rafael J. Wysocki]

On Monday, September 06, 2010, Mark Brown wrote:
> On Mon, Sep 06, 2010 at 03:32:09PM +0300, Ohad Ben-Cohen wrote:
> 
> > That may result in some drivers defining nop handlers, just to return
> > 0 (in case there's nothing else they need to do).
> 
> > Do we want that ?
> 
> Funnily enough I was about to report this issue too - it doesn't look
> great in the driver code.  My use case is using runtime PM in an MFD to
> communicate status to the parent devices.  The subdevices are just
> indicating that they are idle to the parent and have no reason to do
> anything in a suspend or resume callback.
> 
> > Alternatively, we may want to allow drivers to enable Runtime PM (by
> > taking the appropriate action for their subsystem, e.g. calling
> > put_noidle in probe and get_noresume in remove), but still not define
> > any runtime pm handlers (implicitly always returning a success), with
> > something like:
> 
> This would be my preferred solution.

OK

Please submit the patch with appropriate changelog and I'll apply it.

Thanks,
Rafael