On Wed, 17 Mar 2010, Kevin Hilman wrote:
> >> In my case, the driver's runtime_suspend and runtime_resume hooks are
> >> not where the clocks are managed. The actual hardware enable/disable
> >> is done in the bus-level runtime PM hooks, in this case platform_bus.
> >> So having the system PM methods directly call the drivers runtime PM
> >> methods doesn't help. In fact, because we handle the hardware at the
> >> bus level, most drivers can live without any runtime PM methods, and
> >> simply use get/put.
> >>
> >> I've worked around this temporarily by calling the
> >> bus->pm->runtime_suspend() and ->runtime_resume() methods from the
> >> system PM methods, but am curious if that is an acceptable solution.
> >
> > If the platform bus manages the clocks from within its runtime-PM
> > routines, then it ought to provide a similar service from within its
> > system-PM routines.
>
> Hmm, good point. Currently the platform bus code allows overriding
> the runtime PM methods via weak functions (drivers/base/platform.c)
> but not the system PM methods. Below is a patch that allows platforms
> to extend the system PM methods of the platform bus as well.
>
> > You could do it by calling the bus's runtime-PM
> > routines indirectly through the method pointers (as you do now), or by
> > calling the runtime-PM routines directly, or by making the runtime-PM
> > routines and the system-PM routines both call a separate common
> > function responsible for managing the clocks.
>
> Using the patch below, I am able to add custom system PM hooks and then
> use common code to manage the clocks for runtime PM and system PM.
>
> Comments?
>
> Kevin
>
>
> commit ca2173923bae3ba631e12698401ef0b59ec0433c
> Author: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Date: Wed Mar 17 09:36:10 2010 -0700
>
> platform_bus: allow custom extensions to system PM methods
>
> When runtime PM for platform_bus was added, it allowed for platforms
> to customize the runtime PM methods since they are defined as weak
> symbols.
>
> This patch allows platforms to extend the system PM methods with
> custom hooks as well so runtime PM and system PM extensions can be
> managed together.
>
> Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 1ba9d61..a30f850 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -729,6 +729,26 @@ static void platform_pm_complete(struct device *dev)
>
> #ifdef CONFIG_SUSPEND
This probably should be CONFIG_SLEEP.
>
> +int __weak platform_pm_suspend_hook(struct device *dev)
> +{
> + return 0;
> +}
> +
> +int __weak platform_pm_suspend_noirq_hook(struct device *dev)
> +{
> + return 0;
> +}
> +
> +int __weak platform_pm_resume_hook(struct device *dev)
> +{
> + return 0;
> +}
> +
> +int __weak platform_pm_resume_noirq_hook(struct device *dev)
> +{
> + return 0;
> +}
> +
> static int platform_pm_suspend(struct device *dev)
> {
> struct device_driver *drv = dev->driver;
> @@ -744,6 +764,8 @@ static int platform_pm_suspend(struct device *dev)
> ret = platform_legacy_suspend(dev, PMSG_SUSPEND);
> }
>
> + platform_pm_suspend_hook(dev);
> +
> return ret;
> }
>
> @@ -760,6 +782,8 @@ static int platform_pm_suspend_noirq(struct device *dev)
> ret = drv->pm->suspend_noirq(dev);
> }
>
> + platform_pm_suspend_noirq_hook(dev);
> +
> return ret;
> }
>
> @@ -768,6 +792,8 @@ static int platform_pm_resume(struct device *dev)
> struct device_driver *drv = dev->driver;
> int ret = 0;
>
> + platform_pm_resume_hook(dev);
> +
> if (!drv)
> return 0;
>
> @@ -786,6 +812,8 @@ static int platform_pm_resume_noirq(struct device *dev)
> struct device_driver *drv = dev->driver;
> int ret = 0;
>
> + platform_pm_resume_noirq_hook(dev);
> +
> if (!drv)
> return 0;
>
It looks reasonable to me, but I'm not actively involved in PM for the
platform bus. Magnus Damm might have some suggestions.
Alan Stern
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
