On Wednesday, December 04, 2013 12:15:13 AM Rafael J. Wysocki wrote:
> On Wednesday, November 27, 2013 04:34:56 PM Ulf Hansson wrote:
> > To put devices into low power state during sleep, it sometimes makes
> > sense at subsystem-level to re-use the runtime PM callbacks.
> >
> > The PM core will at device_suspend_late disable runtime PM, after that
> > we can safely operate on these callbacks. At suspend_late the device
> > will be put into low power state by invoking the runtime_suspend
> > callbacks, unless the runtime status is already suspended. At
> > resume_early the state is restored by invoking the runtime_resume
> > callbacks. Soon after the PM core will re-enable runtime PM before
> > returning from device_resume_early.
> >
> > The new pm_generic functions are named pm_generic_suspend_late_runtime
> > and pm_generic_resume_early_runtime, they are supposed to be used in
> > pairs.
> >
> > Do note that these new generic callbacks will work smothly even with
> > and without CONFIG_PM_RUNTIME, as long as the runtime PM callbacks are
> > implemented inside CONFIG_PM instead of CONFIG_PM_RUNTIME.
> >
> > A special thanks to Alan Stern who came up with this idea.
> >
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Alan Stern <[email protected]>
> > Signed-off-by: Ulf Hansson <[email protected]>
> > ---
> > drivers/base/power/generic_ops.c | 86
> > ++++++++++++++++++++++++++++++++++++++
> > include/linux/pm.h | 2 +
> > 2 files changed, 88 insertions(+)
> >
> > diff --git a/drivers/base/power/generic_ops.c
> > b/drivers/base/power/generic_ops.c
> > index 5ee030a..8e78ad1 100644
> > --- a/drivers/base/power/generic_ops.c
> > +++ b/drivers/base/power/generic_ops.c
> > @@ -93,6 +93,49 @@ int pm_generic_suspend_late(struct device *dev)
> > EXPORT_SYMBOL_GPL(pm_generic_suspend_late);
>
> After a bit of reconsideration it appears to me that the only benefit of that
> would be to make it possible for drivers to work around incomplete PM domains
> implementations. Which would be of questionable value.
>
> > /**
> > + * pm_generic_suspend_late_runtime - Generic suspend_late callback for
> > drivers
> > + * @dev: Device to suspend.
> > + * Use to invoke runtime_suspend callbacks at suspend_late.
> > + */
> > +int pm_generic_suspend_late_runtime(struct device *dev)
> > +{
> > + int (*callback)(struct device *);
> > + int ret = 0;
> > +
> > + /*
> > + * PM core has disabled runtime PM in device_suspend_late, thus we can
> > + * safely check the device's runtime status and decide whether
> > + * additional actions are needed to put the device into low power state.
> > + * If so, we invoke the runtime_suspend callbacks.
> > + * For the !CONFIG_PM_RUNTIME case, pm_runtime_status_suspended() always
> > + * returns false and therefore the runtime_suspend callback will be
> > + * invoked.
> > + */
> > + if (pm_runtime_status_suspended(dev))
> > + return 0;
> > +
> > + if (dev->pm_domain)
> > + callback = dev->pm_domain->ops.runtime_suspend;
>
> First of all, for this to work you need to assume that the type/bus/class will
> not exercise hardware in any way by itself (or you can look at the PCI bus
> type
> for an example why it won't generally work otherwise), so you could simply
> skip
> the rest of this conditional.
>
> For the bus types you are concerned about (platform and AMBA) that actually is
> the case as far as I can say.
>
> > + else if (dev->type && dev->type->pm)
> > + callback = dev->type->pm->runtime_suspend;
> > + else if (dev->class && dev->class->pm)
> > + callback = dev->class->pm->runtime_suspend;
> > + else if (dev->bus && dev->bus->pm)
> > + callback = dev->bus->pm->runtime_suspend;
> > + else
> > + callback = NULL;
> > +
> > + if (!callback && dev->driver && dev->driver->pm)
> > + callback = dev->driver->pm->runtime_suspend;
> > +
> > + if (callback)
> > + ret = callback(dev);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pm_generic_suspend_late_runtime);
>
> After that you're left with something like this:
>
> int prototype_suspend_late(struct device *dev)
> {
> int (*callback)(struct device *);
>
> if (pm_runtime_status_suspended(dev))
> return 0;
>
> if (dev->pm_domain)
> callback = dev->pm_domain->ops.runtime_suspend;
> else if (dev->driver && dev->driver->pm)
> callback = dev->driver->pm->runtime_suspend;
> else
> callback = NULL;
>
> return callback ? callback(dev) : 0;
> }
>
> Moreover, if a driver's .suspend_late is pointed to the above, it can be
> called
> in two ways. The first way is via type/bus/class or the PM core itself which
> means that all PM handling at this stage is going to be done by
> prototype_suspend_late(). The other way is via a PM domain, in which case
> there may be some additional PM handling in the domain code in principle
> (before or after calling the driver's .suspend_late()). However, in the
> latter case it generally wouldn't be OK to call PM domain's
> .runtime_suspend(),
> because that may interfere with the PM handling in its .suspend_late(). So
> again, this pretty much requires that the PM domain's .suspend_late pointer be
> NULL or point to a routine that simply executes the driver's callback and does
> noting in addition to that.
>
> Now, I suspect that if the driver's runtime_suspend callback is
> driver_runtime_suspend() and the PM domain's runtime_suspend callback is
> pm_domain_runtime_suspend(), the code you actually want to be executed at the
> "late suspend" stage will be
>
> if (!pm_runtime_status_suspended(dev))
> pm_domain_runtime_suspend()
> driver_runtime_suspend()
>
> (where the assumption is that pm_domain_runtime_suspend() will call
> driver_runtime_suspend() for you). Clearly, prototype_suspend_late() will
> lead
> to that effective result in the conditions in which it makes sense to use it.
>
> So, instead of pointing the driver's .suspend_late() to
> prototype_suspend_late(),
> why don't you point the .suspend_late of the *PM* *domain* to the following
> slightly modified version of that function:
>
> int pm_domain_simplified_suspend_late(struct device *dev)
> {
> int (*callback)(struct device *) = NULL;
>
> if (pm_runtime_status_suspended(dev))
> return 0;
>
> /* Try to execute our own .runtime_suspend() callback. */
> if (dev->pm_domain)
> callback = dev->pm_domain->ops.runtime_suspend;
>
> if (WARN_ON(!callback) && dev->driver && dev->driver->pm)
> callback = dev->driver->pm->runtime_suspend;
>
> return callback ? callback(dev) : 0;
> }
>
> This will cause exactly the code you need to be executed too (with
> a fallback to direct execution of the driver's callback in broken
> cases where the PM domain's own .runtime_suspend() is not implemented),
> but in a much cleaner way (no going back to the code layer that has just
> called you etc.). And you *can* point the PM domain's .suspend_late
> to this, because it has to be either empty of trivial if the
> prototype_suspend_late() above is supposed to work as the driver's
> .suspend_late() callback.
>
> So in my opinion, if you point the .suspend_late callback pointers of the
> PM domains in question to the pm_domain_simplified_suspend_late() above
> and their .resume_early callback pointers to the following function:
>
> int pm_domain_simplified_resume_early(struct device *dev)
> {
> int (*callback)(struct device *) = NULL;
>
> if (pm_runtime_status_suspended(dev))
> return 0;
>
> if (dev->pm_domain)
> callback = dev->pm_domain->ops.runtime_resume;
>
> if (WARN_ON(!callback) && dev->driver && dev->driver->pm)
> callback = dev->driver->pm->runtime_resume;
>
> return callback ? callback(dev) : 0;
> }
>
> it will solve your problems without that horrible jumping between code
> layers.
And in addition to that you can point the drivers' .suspend_late callbacks to
something like:
int pm_simplified_suspend_late(struct device *dev)
{
if (pm_runtime_status_suspended(dev))
return 0;
return dev->driver->pm->runtime_suspend ?
dev->driver->pm->runtime_suspend(dev) : 0;
}
(and analogously for the "early resume") which will cause their
.runtime_suspend()
to be executed even if the PM domain doesn't have a .suspend_late (or there's
no PM domain at all).
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
