Re: [PATCH] PM: add dpm_use_runtime_{suspend, resume} helper functions

Kevin Hilman
2010-03-01T22:25:35Z

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Saturday 27 February 2010, Mark Brown wrote:
>> On Fri, Feb 26, 2010 at 05:06:59PM -0500, Alan Stern wrote:
>> > On Fri, 26 Feb 2010, Rafael J. Wysocki wrote:
>>
>> > > I have one problem with the design. Namely, dpm_invoke_runtime_*() can
>> > > run a callback from another subsystem. Say you are a device class and you
>> > > decide to use dpm_invoke_runtime_*(), but the device's bus type implements
>> > > the runtime PM callbacks, so they will be run as device class suspend and
>> > > resume callbacks. That doesn't look particularly clean to me.
>>
>> > That is a valid point. I suppose there could be separate bus-type,
>> > device-type, and device-class versions of these functions, but that
>> > seems like excessive complication with little real benefit.
>>
>> I do agree that it'd be good to avoid adding any further complexity here
>> - the use case I have is devices that only really have one suspend type
>> and don't want or need to know if it's a runtime, disk or memory suspend.
>
> What about the patch below, then (untested)?
>
> Use GENERIC_SUBSYS_PM_OPS for the subsystem and then
> UNIVERSAL_DEV_PM_OPS for drivers, possibly passing NULL as idle_fn.

This looks good to me for what I'm tinkering with too, but have not
tested it.

One thing missing from the orignal patch is a publicly availble
version of pm_is_runtime_suspended(). I found this useful in at least
one driver where I needed a slightly different suspend hook from he
runtime hook.

Kevin

>
> ---
> drivers/base/power/Makefile | 1
> drivers/base/power/generic_ops.c | 119 +++++++++++++++++++++++++++++++++++++++
> include/linux/pm.h | 67 +++++++++++++++++++--
> 3 files changed, 181 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/drivers/base/power/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/Makefile
> +++ linux-2.6/drivers/base/power/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_PM) += sysfs.o
> obj-$(CONFIG_PM_SLEEP) += main.o
> obj-$(CONFIG_PM_RUNTIME) += runtime.o
> +obj-$(CONFIG_PM_OPS) += generic_ops.o
> obj-$(CONFIG_PM_TRACE_RTC) += trace.o
>
> ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
> Index: linux-2.6/drivers/base/power/generic_ops.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/generic_ops.c
> +++ linux-2.6/drivers/base/power/generic_ops.c
> @@ -6,3 +6,122 @@
> * This file is released under the GPLv2.
> */
>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +
> +#ifdef CONFIG_PM_RUNTIME
> +/**
> + * pm_generic_runtime_idle - Generic runtime idle callback for subsystems.
> + * @dev: Device to handle.
> + *
> + * If PM operations are defined for the driver of @dev and they include
> + * ->runtime_idle(), execute it and return the error code returned by it if
> + * nonzero. Otherwise, execute pm_runtime_suspend() for the device.
> + */
> +int pm_generic_runtime_idle(struct device *dev)
> +{
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> + if (pm && pm->runtime_idle) {
> + int ret = pm->runtime_idle(dev);
> + if (ret)
> + return ret;
> + }
> +
> + pm_runtime_suspend(dev);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pm_generic_runtime_idle);
> +
> +/**
> + * pm_generic_runtime_suspend - Generic runtime suspend callback for subsystems.
> + * @dev: Device to suspend.
> + *
> + * If PM operations are defined for the driver of @dev and they include
> + * ->runtime_suspend(), execute it and return the error code returned by it.
> + * Otherwise, return zero.
> + */
> +int pm_generic_runtime_suspend(struct device *dev)
> +{
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int ret = 0;
> +
> + if (pm && pm->runtime_suspend)
> + ret = pm->runtime_suspend(dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
> +
> +/**
> + * pm_generic_runtime_resume - Generic runtime resume callback for subsystems.
> + * @dev: Device to resume.
> + *
> + * If PM operations are defined for the driver of @dev and they include
> + * ->runtime_resume(), execute it and return the error code returned by it.
> + * Otherwise, return zero.
> + */
> +int pm_generic_runtime_resume(struct device *dev)
> +{
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int ret = 0;
> +
> + if (pm && pm->runtime_resume)
> + ret = pm->runtime_resume(dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pm_generic_runtime_resume);
> +#endif /* CONFIG_PM_RUNTIME */
> +
> +#define CONFIG_PM_SLEEP
> +/**
> + * pm_generic_suspend - Generic suspend callback for subsystems.
> + * @dev: Device to suspend.
> + *
> + * If the device has not been suspended at run time, execute the suspend
> + * callback provided by its driver, if defined, and return the error code
> + * returned by it. Otherwise, return zero.
> + */
> +int pm_generic_suspend(struct device *dev)
> +{
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int ret = 0;
> +
> + if (dev->power.runtime_status == RPM_SUSPENDED)
> + return 0;
> +
> + if (pm && pm->suspend)
> + ret = pm->suspend(dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pm_generic_suspend);
> +
> +/**
> + * pm_generic_resume - Generic resume callback for subsystems.
> + * @dev: Device to resume.
> + *
> + * Execute the resume callback provided by the driver of @dev, if defined.
> + * If it returns 0, change the device's runtime PM status to 'active'. Return
> + * the error code returned by it.
> + */
> +int pm_generic_resume(struct device *dev)
> +{
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int ret;
> +
> + if (!pm || !pm->resume)
> + return 0;
> +
> + ret = pm->resume(dev);
> + if (!ret) {
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pm_generic_resume);
> +#endif /* CONFIG_PM_SLEEP */
> Index: linux-2.6/include/linux/pm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pm.h
> +++ linux-2.6/include/linux/pm.h
> @@ -215,18 +215,73 @@ struct dev_pm_ops {
> int (*runtime_idle)(struct device *dev);
> };
>
> +#ifdef CONFIG_PM_SLEEP
> +#define SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> + .suspend = suspend_fn, \
> + .resume = resume_fn, \
> + .freeze = suspend_fn, \
> + .thaw = resume_fn, \
> + .poweroff = suspend_fn, \
> + .restore = resume_fn,
> +#else
> +#define SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> +#endif
> +
> +#ifdef CONFIG_PM_RUNTIME
> +#define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> + .runtime_suspend = suspend_fn, \
> + .runtime_resume = resume_fn, \
> + .runtime_idle = idle_fn,
> +#else
> +#define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> +#endif
> +
> /*
> * Use this if you want to use the same suspend and resume callbacks for suspend
> * to RAM and hibernation.
> */
> #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> const struct dev_pm_ops name = { \
> - .suspend = suspend_fn, \
> - .resume = resume_fn, \
> - .freeze = suspend_fn, \
> - .thaw = resume_fn, \
> - .poweroff = suspend_fn, \
> - .restore = resume_fn, \
> +SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +}
> +
> +/*
> + * Use this for defining a set of PM operations to be used in all situations
> + * (system suspend, hibernation or runtime PM).
> + */
> +#define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> +const struct dev_pm_ops name = { \
> +SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +#define GENERIC_SYSTEM_SLEEP_PM_OPS \
> + .suspend = pm_generic_suspend, \
> + .resume = pm_generic_resume,
> +#else
> +#define GENERIC_SYSTEM_SLEEP_PM_OPS
> +
> +#endif
> +
> +#ifdef CONFIG_PM_RUNTIME
> +#define GENERIC_RUNTIME_PM_OPS \
> + .runtime_suspend = pm_generic_runtime_suspend, \
> + .runtime_resume = pm_generic_runtime_resume, \
> + .runtime_idle = pm_generic_runtime_idle,
> +#else
> +#define GENERIC_RUNTIME_PM_OPS
> +#endif
> +
> +/*
> + * Use this for subsystems (bus types, device types, device classes) that only
> + * need to invoke PM callbacks provided by device drivers supporting both the
> + * system sleep PM and runtime PM.
> + */
> +#define GENERIC_SUBSYS_PM_OPS(name) \
> +const struct dev_pm_ops name = { \
> +GENERIC_SYSTEM_SLEEP_PM_OPS \
> +GENERIC_RUNTIME_PM_OPS \
> }
>
> /**