[RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Rajendra Nayak]

Most users of PM clocks do the exact same thing in runtime callbacks.
Provide default callbacks and cleanup the existing users (keystone/davinci/omap1/sh)

Rajendra Nayak (5):
  PM / clock_ops: Provide default runtime ops to users
  arm: keystone: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
  arm: omap1: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
  arm: davinci: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
  drivers: sh: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS

 arch/arm/mach-davinci/pm_domain.c  | 32 +-------------------------
 arch/arm/mach-keystone/pm_domain.c | 33 +-------------------------
 arch/arm/mach-omap1/pm_bus.c       | 37 ++----------------------------
 drivers/base/power/clock_ops.c     | 38 ++++++++++++++++++++++++++++++
 drivers/sh/pm_runtime.c            | 47 ++------------------------------------
 include/linux/pm_clock.h           | 10 ++++++++
 6 files changed, 54 insertions(+), 143 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[RFC 1/5] PM / clock_ops: Provide default runtime ops to users

[Rajendra Nayak]

Most users of PM clocks do the extact same things in the runtime
suspend/resume callbacks. Provide them USE_PM_CLK_RUNTIME_OPS so
as to avoid/remove boilerplate code.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/base/power/clock_ops.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/pm_clock.h       | 10 ++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 7fdd017..8abea66 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -15,6 +15,7 @@
 #include <linux/clkdev.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/pm_runtime.h>
 
 #ifdef CONFIG_PM
 
@@ -367,6 +368,43 @@ static int pm_clk_notify(struct notifier_block *nb,
 	return 0;
 }
 
+int pm_clk_runtime_suspend(struct device *dev)
+{
+	int ret;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	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;
+}
+
+int pm_clk_runtime_resume(struct device *dev)
+{
+	int ret;
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	ret = pm_clk_resume(dev);
+	if (ret) {
+		dev_err(dev, "failed to resume clock\n");
+		return ret;
+	}
+
+	return pm_generic_runtime_resume(dev);
+}
+
 #else /* !CONFIG_PM */
 
 /**
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 0b00396..25266c6 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -20,6 +20,16 @@ struct pm_clk_notifier_block {
 
 struct clk;
 
+#ifdef CONFIG_PM
+extern int pm_clk_runtime_suspend(struct device *dev);
+extern int pm_clk_runtime_resume(struct device *dev);
+#define USE_PM_CLK_RUNTIME_OPS \
+	.runtime_suspend = pm_clk_runtime_suspend, \
+	.runtime_resume = pm_clk_runtime_resume,
+#else
+#define USE_PM_CLK_RUNTIME_OPS
+#endif
+
 #ifdef CONFIG_PM_CLK
 static inline bool pm_clk_no_clocks(struct device *dev)
 {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[RFC 2/5] arm: keystone: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS

[Rajendra Nayak]

USE_PM_CLK_RUNTIME_OPS is introduced so we don't repeat the same code
to do runtime_suspend and runtime_resume across users of PM clocks.
Use it to remove the boilerplate code.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm/mach-keystone/pm_domain.c | 33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index 41bebfd..edea697 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -19,40 +19,9 @@
 #include <linux/clk-provider.h>
 #include <linux/of.h>
 
-#ifdef CONFIG_PM
-static int keystone_pm_runtime_suspend(struct device *dev)
-{
-	int ret;
-
-	dev_dbg(dev, "%s\n", __func__);
-
-	ret = pm_generic_runtime_suspend(dev);
-	if (ret)
-		return ret;
-
-	ret = pm_clk_suspend(dev);
-	if (ret) {
-		pm_generic_runtime_resume(dev);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int keystone_pm_runtime_resume(struct device *dev)
-{
-	dev_dbg(dev, "%s\n", __func__);
-
-	pm_clk_resume(dev);
-
-	return pm_generic_runtime_resume(dev);
-}
-#endif
-
 static struct dev_pm_domain keystone_pm_domain = {
 	.ops = {
-		SET_RUNTIME_PM_OPS(keystone_pm_runtime_suspend,
-				   keystone_pm_runtime_resume, NULL)
+		USE_PM_CLK_RUNTIME_OPS
 		USE_PLATFORM_PM_SLEEP_OPS
 	},
 };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[RFC 3/5] arm: omap1: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS

[Rajendra Nayak]

USE_PM_CLK_RUNTIME_OPS is introduced so we don't repeat the same code
to do runtime_suspend and runtime_resume across users of PM clocks.
Use it to remove the boilerplate code.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm/mach-omap1/pm_bus.c | 37 ++-----------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/arch/arm/mach-omap1/pm_bus.c b/arch/arm/mach-omap1/pm_bus.c
index c40e209..667c163 100644
--- a/arch/arm/mach-omap1/pm_bus.c
+++ b/arch/arm/mach-omap1/pm_bus.c
@@ -21,48 +21,15 @@
 
 #include "soc.h"
 
-#ifdef CONFIG_PM
-static int omap1_pm_runtime_suspend(struct device *dev)
-{
-	int ret;
-
-	dev_dbg(dev, "%s\n", __func__);
-
-	ret = pm_generic_runtime_suspend(dev);
-	if (ret)
-		return ret;
-
-	ret = pm_clk_suspend(dev);
-	if (ret) {
-		pm_generic_runtime_resume(dev);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int omap1_pm_runtime_resume(struct device *dev)
-{
-	dev_dbg(dev, "%s\n", __func__);
-
-	pm_clk_resume(dev);
-	return pm_generic_runtime_resume(dev);
-}
-
 static struct dev_pm_domain default_pm_domain = {
 	.ops = {
-		.runtime_suspend = omap1_pm_runtime_suspend,
-		.runtime_resume = omap1_pm_runtime_resume,
+		USE_PM_CLK_RUNTIME_OPS
 		USE_PLATFORM_PM_SLEEP_OPS
 	},
 };
-#define OMAP1_PM_DOMAIN (&default_pm_domain)
-#else
-#define OMAP1_PM_DOMAIN NULL
-#endif /* CONFIG_PM */
 
 static struct pm_clk_notifier_block platform_bus_notifier = {
-	.pm_domain = OMAP1_PM_DOMAIN,
+	.pm_domain = &default_pm_domain,
 	.con_ids = { "ick", "fck", NULL, },
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[RFC 4/5] arm: davinci: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS

[Rajendra Nayak]

USE_PM_CLK_RUNTIME_OPS is introduced so we don't repeat the same code
to do runtime_suspend and runtime_resume across users of PM clocks.
Use it to remove the boilerplate code.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm/mach-davinci/pm_domain.c | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/arch/arm/mach-davinci/pm_domain.c b/arch/arm/mach-davinci/pm_domain.c
index 641edc3..78eac2c 100644
--- a/arch/arm/mach-davinci/pm_domain.c
+++ b/arch/arm/mach-davinci/pm_domain.c
@@ -14,39 +14,9 @@
 #include <linux/pm_clock.h>
 #include <linux/platform_device.h>
 
-#ifdef CONFIG_PM
-static int davinci_pm_runtime_suspend(struct device *dev)
-{
-	int ret;
-
-	dev_dbg(dev, "%s\n", __func__);
-
-	ret = pm_generic_runtime_suspend(dev);
-	if (ret)
-		return ret;
-
-	ret = pm_clk_suspend(dev);
-	if (ret) {
-		pm_generic_runtime_resume(dev);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int davinci_pm_runtime_resume(struct device *dev)
-{
-	dev_dbg(dev, "%s\n", __func__);
-
-	pm_clk_resume(dev);
-	return pm_generic_runtime_resume(dev);
-}
-#endif
-
 static struct dev_pm_domain davinci_pm_domain = {
 	.ops = {
-		SET_RUNTIME_PM_OPS(davinci_pm_runtime_suspend,
-				   davinci_pm_runtime_resume, NULL)
+		USE_PM_CLK_RUNTIME_OPS
 		USE_PLATFORM_PM_SLEEP_OPS
 	},
 };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[RFC 5/5] drivers: sh: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS

[Rajendra Nayak]

USE_PM_CLK_RUNTIME_OPS is introduced so we don't repeat the same code
to do runtime_suspend and runtime_resume across users of PM clocks.
Use it to remove the boilerplate code.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/sh/pm_runtime.c | 47 ++---------------------------------------------
 1 file changed, 2 insertions(+), 45 deletions(-)

diff --git a/drivers/sh/pm_runtime.c b/drivers/sh/pm_runtime.c
index cd4c293..e0fd1e0 100644
--- a/drivers/sh/pm_runtime.c
+++ b/drivers/sh/pm_runtime.c
@@ -20,58 +20,15 @@
 #include <linux/bitmap.h>
 #include <linux/slab.h>
 
-#ifdef CONFIG_PM
-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 = sh_pm_runtime_suspend,
-		.runtime_resume = sh_pm_runtime_resume,
+		USE_PM_CLK_RUNTIME_OPS
 		USE_PLATFORM_PM_SLEEP_OPS
 	},
 };
 
-#define DEFAULT_PM_DOMAIN_PTR	(&default_pm_domain)
-
-#else
-
-#define DEFAULT_PM_DOMAIN_PTR	NULL
-
-#endif /* CONFIG_PM */
-
 static struct pm_clk_notifier_block platform_bus_notifier = {
-	.pm_domain = DEFAULT_PM_DOMAIN_PTR,
+	.pm_domain = &default_pm_domain,
 	.con_ids = { NULL, },
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Kevin Hilman]

Rajendra Nayak <rnayak@codeaurora.org> writes:

> Most users of PM clocks do the exact same thing in runtime callbacks.

Probably because they were all copied from mach-davinci. ;)

> Provide default callbacks and cleanup the existing users (keystone/davinci/omap1/sh)

Very nice cleanup, Thanks!

For the series:

Reviewed-by: Kevin Hilman <khilman@linaro.org>

Kevin

Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[santosh shilimkar]

On 4/20/2015 4:21 PM, Kevin Hilman wrote:
> Rajendra Nayak <rnayak@codeaurora.org> writes:
>
>> Most users of PM clocks do the exact same thing in runtime callbacks.
>
> Probably because they were all copied from mach-davinci. ;)
>
Yep. ;-)

>> Provide default callbacks and cleanup the existing users (keystone/davinci/omap1/sh)
>
> Very nice cleanup, Thanks!
>
Indeed !!
I can't test it out but from the series, I don't expect anything
to break. So looks good to me as well.

> For the series:
>
> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>
Acked-by: Santosh Shilimkar <ssantosh@kernel.org>

Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Geert Uytterhoeven]

On Tue, Apr 21, 2015 at 1:25 AM, santosh shilimkar
<santosh.shilimkar@oracle.com> wrote:
> On 4/20/2015 4:21 PM, Kevin Hilman wrote:
>>
>> Rajendra Nayak <rnayak@codeaurora.org> writes:
>>
>>> Most users of PM clocks do the exact same thing in runtime callbacks.
>>
>>
>> Probably because they were all copied from mach-davinci. ;)
>>
> Yep. ;-)

If you're interested in the history, I did some digging last year:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/242352.html

>>> Provide default callbacks and cleanup the existing users
>>> (keystone/davinci/omap1/sh)
>>
>>
>> Very nice cleanup, Thanks!

Note that the new code always has a dev_pm_domain, while the old code had it
conditionally on CONFIG_PM.

I don't think that matters much, as we seem to be having more and more
systems that rely on CONFIG_PM=y...

>> For the series:
>>
>> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>>
> Acked-by: Santosh Shilimkar <ssantosh@kernel.org>

Looks good to me, and works fine on (pre-Clock Domain) r8a7791/koelsch
and r8a7740/armadillo-legacy (with PM Domains, but still relying on the
drivers/sh/pm_runtime.c hack for devices in the C5 "always on" domain).

This code is no longer used in multiplatform kernels on shmobile boards
with real PM Domains.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Rajendra Nayak]

> On Tue, Apr 21, 2015 at 1:25 AM, santosh shilimkar
> <santosh.shilimkar@oracle.com> wrote:
>> On 4/20/2015 4:21 PM, Kevin Hilman wrote:
>>>
>>> Rajendra Nayak <rnayak@codeaurora.org> writes:
>>>
>>>> Most users of PM clocks do the exact same thing in runtime callbacks.
>>>
>>>
>>> Probably because they were all copied from mach-davinci. ;)
>>>
>> Yep. ;-)
>
> If you're interested in the history, I did some digging last year:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/242352.html
>
>>>> Provide default callbacks and cleanup the existing users
>>>> (keystone/davinci/omap1/sh)
>>>
>>>
>>> Very nice cleanup, Thanks!
>
> Note that the new code always has a dev_pm_domain, while the old code had
> it
> conditionally on CONFIG_PM.

Right, but since USE_PM_CLK_RUNTIME_OPS is actually a nop with
!CONFIG_PM, we don't really need the checks anymore.

>
> I don't think that matters much, as we seem to be having more and more
> systems that rely on CONFIG_PM=y...
>
>>> For the series:
>>>
>>> Reviewed-by: Kevin Hilman <khilman@linaro.org>
>>>
>> Acked-by: Santosh Shilimkar <ssantosh@kernel.org>
>
> Looks good to me, and works fine on (pre-Clock Domain) r8a7791/koelsch
> and r8a7740/armadillo-legacy (with PM Domains, but still relying on the
> drivers/sh/pm_runtime.c hack for devices in the C5 "always on" domain).
>
> This code is no longer used in multiplatform kernels on shmobile boards
> with real PM Domains.
>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks,
Rajendra

>
> 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
>


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Geert Uytterhoeven]

On Tue, Apr 21, 2015 at 10:56 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>> Note that the new code always has a dev_pm_domain, while the old code had
>> it
>> conditionally on CONFIG_PM.
>
> Right, but since USE_PM_CLK_RUNTIME_OPS is actually a nop with
> !CONFIG_PM, we don't really need the checks anymore.

What I mean is that it uses more memory, to store the dev_pm_domain struct.

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