Re: calling runtime PM from system PM methods

On Tuesday, June 07, 2011, Kevin Hilman wrote:

"Rafael J. Wysocki" [off-list ref] writes:

[..]

quoted

While it is tempting to try to get away with only two PM callbacks per
driver instead of four (or even more), it generally is not doable, simply
because driver callbacks are not executed directly by the core.

The only way to address the problem of code duplication between .suspend()
and .runtime_suspend() callbacks (and analogously for resume) I see at the
moment is to make those callbacks execute common routines.

Makes sense if the "common routines" are in the driver.  The problem
arises when the common routines are not actually in the driver, but are
instead at the subsystem (or in this case, device power domain) level.

As Alan said, I'm not sure why that is a problem, because device power
domain can (and most likely should) provide system suspend callbacks as well
as runtime PM callbacks.  Those callbacks can be designed to do whatever is
needed.

Considering the OMAP I2C driver, it doesn't have (or need) runtime PM
callbacks.  It does runtime PM get/put per-xfer, and has no context to
save/restore, so it needs no runtime PM callbacks, or no special PM code
other than runtime PM get/put calls.  The device power domain level code
is managing the device clocks, power states, etc..

So what the system suspend needs to do is something like:

1. block any new xfers from starting
2. wait for any outstanding xfers
3. if device is already runtime suspended
    - nothing to do
4. trigger idle transition (at device power domain level)

If runtime PM has not been disabled from userspace, it should always end
at step 3, since the last xfer will always trigger a runtime suspend.

However, if runtime PM has been disabled from
userspace/pm_runtime_forbid(), we need some way to do step 4.

Well, it looks like my previous message wasn't clear enough. :-)

Whether or not user space has disabled runtime PM _doesn't_ _matter_ for
system suspend, because _you_ _can't_ call pm_runtime_suspend(), or
pm_runtime_put_sunc(), from a driver's .suspend() callback _anyway_.
The reason is that doing that would cause the subsystem's (or power
domain's in this case) .runtime_suspend() callback to be invoked and
that's incorrect.  Namely, it would require the subsystem (power domain)
to expect that its .runtime_suspend() would always be executed indirectly
as a result of calling its .suspend() (through the driver's callback)
and that expectation may or may not be met (depending on the driver's
design).

The appropriate way to handle system suspend is to provide subsystem
(or power domain) .suspend()/.resume() callbacks that will do whatever is
needed at the subsystem level and will call the corresponding driver
callbacks.  There is no need whatsoever to involve runtime PM into this
in any way.

It's this last 'trigger' step that I'm trying to figure out how a clean way of
implementing, particularily for drivers with no runtime PM callbacks.

Unless I'm missing something else, if runtime PM was not prevented via
userspace/pm_runtime_forbid(), we would not need this last 'trigger'
step.  That's why a solution where any pending runtime PM transitions
would be allowed during system PM is the ideal solution (to me.)  It
avoids having to call runtime PM methods from system PM all together.

Well, again.  There's nothing to avoid, because all the thing you'd like to do
is incorrect in the first place.

The current OMAP I2C driver in mainline does this extra "trigger" step
by directly calling the subsystem (bus) callbacks. (It's also missing
the first two steps, which is a known bug and will be fixed once I
figure out the rest of this problem.)

However, now that we have device power domains, I was planning to extend
that to call device power domain callbacks first if they exist.  Since
that was starting to duplicate callback precedence assumptions in the
runtime PM core, I was thinking about ways to avoid that by simply using
runtime PM directly, that's what got me to start this thread in the
first place.

So, I see 2 ways forward

1. Having some per-device option/flag that would allow pending runtime
   PM transitions to happen during system PM, thus removing the need
   for step 4 above.

2. Decide the "right" way to trigger device power domain (or subsytem)
   transitions from the driver.   Directly calling the subsystem
   callbacks from the driver?   Any other options?

Yes, see above.

I have a rather strong preference for the first one, but am realizing
that I may be in the minority.  So what is the recommended solution for
2?

Well, let me repeat: you need to separate the system suspend handling from
runtime PM.  Each of them requires a different approach, because the goal
is really different in both cases (basically, runtime PM triggers when
the device is _known_ to be idle, while system suspend may trigger while
it's actually being used).

P.S.  I'm glad you got to discuss this with Paul & Magnus at LinuxCon
Japan.  I wish I could've been there too.

There is a good opportunity to discuss those things during the Linux
Plumbers Conference in September.  We are going to have a Linux PM
miniconference during that event which you are welcome to attend. :-)

Hope your return trip went well and your internet is back working soon.

Yes, it did and the internet is already working, thanks!

Take care,
Rafael