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

Rafael J. Wysocki
2010-02-26T21:24:02Z

On Friday 26 February 2010, Alan Stern wrote:
...
> Index: usb-2.6/drivers/base/power/power.h
> ===================================================================
> --- usb-2.6.orig/drivers/base/power/power.h
> +++ usb-2.6/drivers/base/power/power.h
> @@ -72,3 +72,14 @@ static inline void dpm_sysfs_remove(stru
> }
>
> #endif
> +
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)

CONFIG_PM_OPS can be used for that, it means exactly the above.

> +/*
> + * invoke_runtime.c
> + */
> +extern void dpm_invoke_runtime_idle(struct device *dev);
> +extern int dpm_invoke_runtime_suspend(struct device *dev);
> +extern int dpm_invoke_runtime_resume(struct device *dev);
> +
> +#endif
> Index: usb-2.6/drivers/base/power/Makefile
> ===================================================================
> --- usb-2.6.orig/drivers/base/power/Makefile
> +++ usb-2.6/drivers/base/power/Makefile
> @@ -1,6 +1,6 @@
> obj-$(CONFIG_PM) += sysfs.o
> -obj-$(CONFIG_PM_SLEEP) += main.o
> -obj-$(CONFIG_PM_RUNTIME) += runtime.o
> +obj-$(CONFIG_PM_SLEEP) += main.o invoke_runtime.o
> +obj-$(CONFIG_PM_RUNTIME) += runtime.o invoke_runtime.o

And here I think.

There seem to be a few more places where the comment applies.

> obj-$(CONFIG_PM_TRACE_RTC) += trace.o
>
> ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
> Index: usb-2.6/drivers/base/power/main.c
> ===================================================================
> --- usb-2.6.orig/drivers/base/power/main.c
> +++ usb-2.6/drivers/base/power/main.c
> @@ -936,3 +936,51 @@ void __suspend_report_result(const char
> printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
> }
> EXPORT_SYMBOL_GPL(__suspend_report_result);
> +
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
> +
> +/* Convenience routines for drivers that want to use the same functions
> + * for system suspend/resume and runtime suspend/resume:
> + * Set driver->pm_ops->suspend = dpm_use_runtime_suspend and
> + * driver->pm_ops->resume = dpm_use_runtime_resume.
> + */

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.

Rafael