| Message ID | 1395334473-27600-1-git-send-email-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Ben, Thank you for the patch. On Thursday 20 March 2014 17:54:33 Ben Dooks wrote: > If we override the platform bus calls for pm_runtime then we end up > with the calls to the devices' suspend and resume methods ignored > in favour of the bus ones. > > Change to calling the pm_runtime calls to suspend and resume the > devices specifically in the drivers/sh/pm_runtime.c implementation > to allow any device that may want to run power management to do so. > > Note, all the current sh driver implementations do not use their > own power management code so this is not a major implementation > issues. > > This also brings the implementation into line with the versions > used by the Davinci and Keystone PM domain code, so once fully > tested these implementations could be merged together. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > drivers/sh/pm_runtime.c | 37 +++++++++++++++++++++++++++++++++++-- > 1 file changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c > index f4f8851..ced5307 100644 > --- a/drivers/sh/pm_runtime.c > +++ b/drivers/sh/pm_runtime.c > @@ -21,10 +21,43 @@ > #include <linux/slab.h> > > #ifdef CONFIG_PM_RUNTIME > +static int sh_pm_runtime_suspend(struct device *dev) > +{ > + int ret; > + > + ret = pm_generic_runtime_suspend(dev); > + if (ret) { > + dev_err(dev, "failed to suspend device\n"); > + return ret; > + } > + > + ret = pm_clk_suspend(dev); > + if (ret) { > + dev_err(dev, "failed to suspend clock\n"); Despite their return type, pm_clk_suspend and pm_clk_resume never fail (in their current implementation). I'm thus wondering if the dev_err calls are really worth it, especially given that the other implementations you mention in the commit message don't have any error logging. > + pm_generic_runtime_resume(dev); > + return ret; > + } > + > + return 0; > +} > + > +static int sh_pm_runtime_resume(struct device *dev) > +{ > + int ret; > + > + ret = pm_clk_resume(dev); > + if (ret) { > + dev_err(dev, "failed to resume clock\n"); > + return ret; > + } > + > + return pm_generic_runtime_resume(dev); If you want to keep the error messages you should probably add one here as well. > +} > + > static struct dev_pm_domain default_pm_domain = { > .ops = { > - .runtime_suspend = pm_clk_suspend, > - .runtime_resume = pm_clk_resume, > + .runtime_suspend = sh_pm_runtime_suspend, > + .runtime_resume = sh_pm_runtime_resume, > USE_PLATFORM_PM_SLEEP_OPS > }, > };
On Thu, Mar 20, 2014 at 5:54 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > If we override the platform bus calls for pm_runtime then we end up > with the calls to the devices' suspend and resume methods ignored > in favour of the bus ones. > > Change to calling the pm_runtime calls to suspend and resume the > devices specifically in the drivers/sh/pm_runtime.c implementation > to allow any device that may want to run power management to do so. > > Note, all the current sh driver implementations do not use their > own power management code so this is not a major implementation > issues. > > This also brings the implementation into line with the versions > used by the Davinci and Keystone PM domain code, so once fully > tested these implementations could be merged together. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c index f4f8851..ced5307 100644 --- a/drivers/sh/pm_runtime.c +++ b/drivers/sh/pm_runtime.c @@ -21,10 +21,43 @@ #include <linux/slab.h> #ifdef CONFIG_PM_RUNTIME +static int sh_pm_runtime_suspend(struct device *dev) +{ + int ret; + + ret = pm_generic_runtime_suspend(dev); + if (ret) { + dev_err(dev, "failed to suspend device\n"); + return ret; + } + + ret = pm_clk_suspend(dev); + if (ret) { + dev_err(dev, "failed to suspend clock\n"); + pm_generic_runtime_resume(dev); + return ret; + } + + return 0; +} + +static int sh_pm_runtime_resume(struct device *dev) +{ + int ret; + + ret = pm_clk_resume(dev); + if (ret) { + dev_err(dev, "failed to resume clock\n"); + return ret; + } + + return pm_generic_runtime_resume(dev); +} + static struct dev_pm_domain default_pm_domain = { .ops = { - .runtime_suspend = pm_clk_suspend, - .runtime_resume = pm_clk_resume, + .runtime_suspend = sh_pm_runtime_suspend, + .runtime_resume = sh_pm_runtime_resume, USE_PLATFORM_PM_SLEEP_OPS }, };
If we override the platform bus calls for pm_runtime then we end up with the calls to the devices' suspend and resume methods ignored in favour of the bus ones. Change to calling the pm_runtime calls to suspend and resume the devices specifically in the drivers/sh/pm_runtime.c implementation to allow any device that may want to run power management to do so. Note, all the current sh driver implementations do not use their own power management code so this is not a major implementation issues. This also brings the implementation into line with the versions used by the Davinci and Keystone PM domain code, so once fully tested these implementations could be merged together. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- drivers/sh/pm_runtime.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)