"Rafael J. Wysocki" <rjw@xxxxxxx> 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 \ > } > > /** _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm