PM / clock_ops: provide default runtime ops and cleanup users

RFC In review Rajendra Nayak 4 participants 11 messages Apr 14, 2015 – Apr 21, 2015

Subsystems (12)

Arm (32-bit) Arm sub-architectures Arm Qualcomm mailing list SuperH +8 more subsystems

lore

Share & tools

lore

Rajendra Nayak

author

2015-04-14 13:34 UTC #0 lore raw

cover PM / clock_ops: provide default runtime ops and cleanup users

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

Signature · 2 lines

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

Rajendra Nayak

author

2015-04-14 13:34 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/base/power/clock_ops.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/pm_clock.h       | 10 ++++++++++
 2 files changed, 48 insertions(+)

drivers/base/power/clock_ops.c +38 -0

index 7fdd017..8abea66 100644
@@ -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 */
 
 /**

include/linux/pm_clock.h +10 -0

index 0b00396..25266c6 100644
@@ -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)
 {

Signature · 2 lines

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

Rajendra Nayak

author

2015-04-14 13:34 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 arch/arm/mach-keystone/pm_domain.c | 33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

arch/arm/mach-keystone/pm_domain.c +1 ~~-32~~

index 41bebfd..edea697 100644
@@ -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
 	},
 };

Signature · 2 lines

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

Rajendra Nayak

author

2015-04-14 13:34 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 arch/arm/mach-omap1/pm_bus.c | 37 ++-----------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

arch/arm/mach-omap1/pm_bus.c +2 ~~-35~~

index c40e209..667c163 100644
@@ -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, },
 };

Signature · 2 lines

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

Rajendra Nayak

author

2015-04-14 13:34 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 arch/arm/mach-davinci/pm_domain.c | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

arch/arm/mach-davinci/pm_domain.c +1 ~~-31~~

index 641edc3..78eac2c 100644
@@ -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
 	},
 };

Signature · 2 lines

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

Rajendra Nayak

author

2015-04-14 13:34 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/sh/pm_runtime.c | 47 ++---------------------------------------------
 1 file changed, 2 insertions(+), 45 deletions(-)

drivers/sh/pm_runtime.c +2 ~~-45~~

index cd4c293..e0fd1e0 100644
@@ -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, },
 };

Signature · 2 lines

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

Kevin Hilman

maintainer Reviewed-by on the v1 series

2015-04-20 23:21 UTC #6 lore raw

In reply to Rajendra Nayak

Quoted 1 line · Rajendra Nayak

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

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

Quoted 1 line

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

Very nice cleanup, Thanks!

For the series:

Reviewed-by Kevin Hilman <[email protected]>

Kevin

santosh shilimkar

Acked-by

2015-04-20 23:25 UTC #7 lore raw

Quoted 6 lines · Kevin Hilman

Quoted 1 line · Rajendra Nayak
Most users of PM clocks do the exact same thing in runtime callbacks.
Probably because they were all copied from mach-davinci. ;)

Yep. ;-)

Quoted 4 lines

Quoted 1 line
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.

Quoted 4 lines

For the series:
Reviewed-by Kevin Hilman <[email protected]>

Acked-by Santosh Shilimkar <[email protected]>

Geert Uytterhoeven

Acked-by

2015-04-21 08:01 UTC #8 lore raw

Quoted 10 lines · santosh shilimkar

Quoted 8 lines · Kevin Hilman
Quoted 1 line · Rajendra Nayak
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

Quoted 5 lines

Quoted 5 lines
Quoted 2 lines
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...

Quoted 5 lines

Quoted 4 lines
For the series:
Reviewed-by Kevin Hilman <[email protected]>
Acked-by Santosh Shilimkar <[email protected]>

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 <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Rajendra Nayak

author

2015-04-21 08:56 UTC #9 lore raw

Quoted 21 lines

Quoted 10 lines · santosh shilimkar
Quoted 8 lines · Kevin Hilman
Quoted 1 line · Rajendra Nayak
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
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Provide default callbacks and cleanup the existing users (keystone/davinci/omap1/sh)

Quoted 5 lines

Quoted 1 line
Quoted 1 line
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.

Quoted 10 lines

I don't think that matters much, as we seem to be having more and more systems that rely on CONFIG_PM=y...
Quoted 5 lines
Quoted 4 lines
For the series:
Reviewed-by Kevin Hilman <[email protected]>
Acked-by Santosh Shilimkar <[email protected]>

Quoted 8 lines

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 <[email protected]>

Thanks,
Rajendra

Quoted 15 lines

Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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

Signature · 2 lines

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

Geert Uytterhoeven

2015-04-21 09:04 UTC #10 lore raw

Quoted 6 lines · Rajendra Nayak

Quoted 3 lines
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 -- [email protected]

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