[PATCH v2 0/5] Use of_clk_get_parent_count() instead of open coding

[PATCH v2 0/5] Use of_clk_get_parent_count() instead of open coding

[Geert Uytterhoeven]

	Hi Mike, Stephen, et al,

This patch series replaces open-coded versions of
of_clk_get_parent_count() by calls to the helper function.

Changes compared to v1:
  - Move OF clock helpers (a.o. of_clk_get_parent_count()) to their own
    include file <linux/of_clk.h>, as suggested by Stephen,
  - Update includes for the above change,
  - Fix timer-sp804, now a dummy is available in the !CONFIG_COMMON_CLK
    case,
  - Fix a new occurrence.

Note that patches 2-5 depend on the first patch.  To simplify
dependencies, perhaps the first patch can still make it in v4.17-rc1?

Thanks!

Geert Uytterhoeven (5):
  clk: Extract OF clock helpers in <linux/of_clk.h>
  ARM: timer-sp: Use of_clk_get_parent_count() instead of open coding
  soc: rockchip: power-domain: Use of_clk_get_parent_count() instead of
    open coding
  soc/tegra: pmc: Use of_clk_get_parent_count() instead of open coding
  pinctrl: sunxi: Use of_clk_get_parent_count() instead of open coding

 drivers/clocksource/timer-sp804.c     |  3 ++-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c |  4 ++--
 drivers/soc/rockchip/pm_domains.c     |  4 ++--
 drivers/soc/tegra/pmc.c               |  3 ++-
 include/linux/clk-provider.h          | 14 +-------------
 include/linux/of_clk.h                | 29 +++++++++++++++++++++++++++++
 6 files changed, 38 insertions(+), 19 deletions(-)
 create mode 100644 include/linux/of_clk.h

-- 
2.7.4

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

[PATCH v2 1/5] clk: Extract OF clock helpers in <linux/of_clk.h>

[Geert Uytterhoeven]

The use of of_clk_get_parent_{count,name}() and of_clk_init() is not
limited to clock providers.

Hence move these helpers into their own header file, so callers that are
not clock providers no longer have to include <linux/clk-provider.h>.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.

I have't added an SPDX-License-Identifier line, as
<linux/clk-provider.h> also doesn't have one yet.

Other candidates to be moved here later?
  - of_clk_get(),
  - of_clk_get_by_name(),
  - of_clk_get_from_provider().
---
 include/linux/clk-provider.h | 14 +-------------
 include/linux/of_clk.h       | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/of_clk.h

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 210a890008f9e129..61cb4729f22acd6b 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -13,6 +13,7 @@
 
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_clk.h>
 
 #ifdef CONFIG_COMMON_CLK
 
@@ -890,13 +891,10 @@ struct clk_hw *of_clk_hw_simple_get(struct of_phandle_args *clkspec,
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
 struct clk_hw *of_clk_hw_onecell_get(struct of_phandle_args *clkspec,
 				     void *data);
-unsigned int of_clk_get_parent_count(struct device_node *np);
 int of_clk_parent_fill(struct device_node *np, const char **parents,
 		       unsigned int size);
-const char *of_clk_get_parent_name(struct device_node *np, int index);
 int of_clk_detect_critical(struct device_node *np, int index,
 			    unsigned long *flags);
-void of_clk_init(const struct of_device_id *matches);
 
 #else /* !CONFIG_OF */
 
@@ -943,26 +941,16 @@ of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
 {
 	return ERR_PTR(-ENOENT);
 }
-static inline unsigned int of_clk_get_parent_count(struct device_node *np)
-{
-	return 0;
-}
 static inline int of_clk_parent_fill(struct device_node *np,
 				     const char **parents, unsigned int size)
 {
 	return 0;
 }
-static inline const char *of_clk_get_parent_name(struct device_node *np,
-						 int index)
-{
-	return NULL;
-}
 static inline int of_clk_detect_critical(struct device_node *np, int index,
 					  unsigned long *flags)
 {
 	return 0;
 }
-static inline void of_clk_init(const struct of_device_id *matches) {}
 #endif /* CONFIG_OF */
 
 /*
diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h
new file mode 100644
index 0000000000000000..ba3c0e6983ec1543
--- /dev/null
+++ b/include/linux/of_clk.h
@@ -0,0 +1,29 @@
+/*
+ * OF clock helpers
+ */
+
+#ifndef __LINUX_OF_CLK_H
+#define __LINUX_OF_CLK_H
+
+#if defined(CONFIG_COMMON_CLK) && defined(CONFIG_OF)
+
+unsigned int of_clk_get_parent_count(struct device_node *np);
+const char *of_clk_get_parent_name(struct device_node *np, int index);
+void of_clk_init(const struct of_device_id *matches);
+
+#else /* !CONFIG_COMMON_CLK || !CONFIG_OF */
+
+static inline unsigned int of_clk_get_parent_count(struct device_node *np)
+{
+	return 0;
+}
+static inline const char *of_clk_get_parent_name(struct device_node *np,
+						 int index)
+{
+	return NULL;
+}
+static inline void of_clk_init(const struct of_device_id *matches) {}
+
+#endif /* !CONFIG_COMMON_CLK || !CONFIG_OF */
+
+#endif /* __LINUX_OF_CLK_H */
-- 
2.7.4

Re: [PATCH v2 1/5] clk: Extract OF clock helpers in <linux/of_clk.h>

[Heiko Stuebner]

Am Dienstag, 10. April 2018, 14:51:37 CEST schrieb Geert Uytterhoeven:
> The use of of_clk_get_parent_{count,name}() and of_clk_init() is not
> limited to clock providers.
> 
> Hence move these helpers into their own header file, so callers that are
> not clock providers no longer have to include <linux/clk-provider.h>.
> 
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Really cool move,
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v2 1/5] clk: Extract OF clock helpers in <linux/of_clk.h>

[Rob Herring]

On Tue, Apr 10, 2018 at 02:51:37PM +0200, Geert Uytterhoeven wrote:
> The use of of_clk_get_parent_{count,name}() and of_clk_init() is not
> limited to clock providers.
> 
> Hence move these helpers into their own header file, so callers that are
> not clock providers no longer have to include <linux/clk-provider.h>.
> 
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - New.
> 
> I have't added an SPDX-License-Identifier line, as
> <linux/clk-provider.h> also doesn't have one yet.

Should that matter?

> 
> Other candidates to be moved here later?
>   - of_clk_get(),
>   - of_clk_get_by_name(),
>   - of_clk_get_from_provider().
> ---
>  include/linux/clk-provider.h | 14 +-------------
>  include/linux/of_clk.h       | 29 +++++++++++++++++++++++++++++

Please update MAINTAINERS so I am not the maintainer. :)

>  2 files changed, 30 insertions(+), 13 deletions(-)
>  create mode 100644 include/linux/of_clk.h

Re: [PATCH v2 1/5] clk: Extract OF clock helpers in <linux/of_clk.h>

[Stephen Boyd]

Quoting Geert Uytterhoeven (2018-04-10 05:51:37)
> The use of of_clk_get_parent_{count,name}() and of_clk_init() is not
> limited to clock providers.
> 
> Hence move these helpers into their own header file, so callers that are
> not clock providers no longer have to include <linux/clk-provider.h>.
> 
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - New.
> 
> I have't added an SPDX-License-Identifier line, as
> <linux/clk-provider.h> also doesn't have one yet.
> 
> Other candidates to be moved here later?
>   - of_clk_get(),
>   - of_clk_get_by_name(),
>   - of_clk_get_from_provider().

The clk_get APIs should stay in the consumer header file (clk.h) because
they're consumer APIs. Sure they have "of" in the name, but whatever.

[PATCH v2 2/5] ARM: timer-sp: Use of_clk_get_parent_count() instead of open coding

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
<linux/of_clk.h>".

v2:
  - of_clk_get_parent_count() was moved to <linux/of_clk.h>,
  - Dropped RFC, as a dummy is now available in the !CONFIG_COMMON_CLK
    case.
---
 drivers/clocksource/timer-sp804.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index 3ac9dec9a03852f9..e01222ea888ff9d2 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_clk.h>
 #include <linux/of_irq.h>
 #include <linux/sched_clock.h>
 
@@ -245,7 +246,7 @@ static int __init sp804_of_init(struct device_node *np)
 		clk1 = NULL;
 
 	/* Get the 2nd clock if the timer has 3 timer clocks */
-	if (of_count_phandle_with_args(np, "clocks", "#clock-cells") == 3) {
+	if (of_clk_get_parent_count(np) == 3) {
 		clk2 = of_clk_get(np, 1);
 		if (IS_ERR(clk2)) {
 			pr_err("sp804: %s clock not found: %d\n", np->name,
-- 
2.7.4

Re: [PATCH v2 2/5] ARM: timer-sp: Use of_clk_get_parent_count() instead of open coding

[Daniel Lezcano]

On Tue, Apr 10, 2018 at 02:51:38PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
> <linux/of_clk.h>".
> 
> v2:
>   - of_clk_get_parent_count() was moved to <linux/of_clk.h>,
>   - Dropped RFC, as a dummy is now available in the !CONFIG_COMMON_CLK
>     case.
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Re: [PATCH v2 2/5] ARM: timer-sp: Use of_clk_get_parent_count() instead of open coding

[Linus Walleij]

On Tue, Apr 10, 2018 at 2:51 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
> <linux/of_clk.h>".
>
> v2:
>   - of_clk_get_parent_count() was moved to <linux/of_clk.h>,
>   - Dropped RFC, as a dummy is now available in the !CONFIG_COMMON_CLK
>     case.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v2 3/5] soc: rockchip: power-domain: Use of_clk_get_parent_count() instead of open coding

[Geert Uytterhoeven]

As of_clk_get_parent_count() returns zero on failure, while
of_count_phandle_with_args() might return a negative error code, this
also fixes the issue of possibly using a negative number in the
allocation below.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
<linux/of_clk.h>".

v2:
  - of_clk_get_parent_count() was moved to <linux/of_clk.h>.
---
 drivers/soc/rockchip/pm_domains.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 53efc386b1ada8cf..13913d40c8213e36 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -14,6 +14,7 @@
 #include <linux/pm_clock.h>
 #include <linux/pm_domain.h>
 #include <linux/of_address.h>
+#include <linux/of_clk.h>
 #include <linux/of_platform.h>
 #include <linux/clk.h>
 #include <linux/regmap.h>
@@ -400,8 +401,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	pd->info = pd_info;
 	pd->pmu = pmu;
 
-	pd->num_clks = of_count_phandle_with_args(node, "clocks",
-						  "#clock-cells");
+	pd->num_clks = of_clk_get_parent_count(node);
 	if (pd->num_clks > 0) {
 		pd->clks = devm_kcalloc(pmu->dev, pd->num_clks,
 					sizeof(*pd->clks), GFP_KERNEL);
-- 
2.7.4

Re: [PATCH v2 3/5] soc: rockchip: power-domain: Use of_clk_get_parent_count() instead of open coding

[Heiko Stuebner]

Am Dienstag, 10. April 2018, 14:51:39 CEST schrieb Geert Uytterhoeven:
> As of_clk_get_parent_count() returns zero on failure, while
> of_count_phandle_with_args() might return a negative error code, this
> also fixes the issue of possibly using a negative number in the
> allocation below.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Depending on how we plan on merging this series, I could either pick
this patch for 4.19, after patch1 makes it into 4.18 or otherwise
someone could pick the whole series together with my

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

for this patch.


Heiko

[PATCH v2 4/5] soc/tegra: pmc: Use of_clk_get_parent_count() instead of open coding

[Geert Uytterhoeven]

As of_clk_get_parent_count() returns zero on failure, while
of_count_phandle_with_args() might return a negative error code, this
also fixes the issue of possibly using a very big number in the
allocation below.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
<linux/of_clk.h>".

v2:
  - of_clk_get_parent_count() was moved to <linux/of_clk.h>.
---
 drivers/soc/tegra/pmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index d9fcdb592b3966a5..d8cb48a4b8eb1b78 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -31,6 +31,7 @@
 #include <linux/iopoll.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_clk.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -725,7 +726,7 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 	unsigned int i, count;
 	int err;
 
-	count = of_count_phandle_with_args(np, "clocks", "#clock-cells");
+	count = of_clk_get_parent_count(np);
 	if (count == 0)
 		return -ENODEV;
 
-- 
2.7.4

Re: [PATCH v2 4/5] soc/tegra: pmc: Use of_clk_get_parent_count() instead of open coding

[Jon Hunter]

On 10/04/18 13:51, Geert Uytterhoeven wrote:
> As of_clk_get_parent_count() returns zero on failure, while
> of_count_phandle_with_args() might return a negative error code, this
> also fixes the issue of possibly using a very big number in the
> allocation below.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
> <linux/of_clk.h>".
> 
> v2:
>   - of_clk_get_parent_count() was moved to <linux/of_clk.h>.
> ---
>  drivers/soc/tegra/pmc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index d9fcdb592b3966a5..d8cb48a4b8eb1b78 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -31,6 +31,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_clk.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> @@ -725,7 +726,7 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
>  	unsigned int i, count;
>  	int err;
>  
> -	count = of_count_phandle_with_args(np, "clocks", "#clock-cells");
> +	count = of_clk_get_parent_count(np);
>  	if (count == 0)
>  		return -ENODEV;
>  
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

[PATCH v2 5/5] pinctrl: sunxi: Use of_clk_get_parent_count() instead of open coding

[Geert Uytterhoeven]

A new open coder has crept in since 470b73a38470e8ba ("pinctrl: sunxi:
Use of_clk_get_parent_count() instead of open coding"), replace it.

of_clk_get_parent_count() was moved to <linux/of_clk.h>, so include that
instead of <linux/clk-provider.h>.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH v2 1/5] clk: Extract OF clock helpers in
<linux/of_clk.h>".

v2:
  - New.
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 020d6d84639ca002..25e80a5370ca02f6 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -12,12 +12,12 @@
 
 #include <linux/io.h>
 #include <linux/clk.h>
-#include <linux/clk-provider.h>
 #include <linux/gpio/driver.h>
 #include <linux/irqdomain.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/export.h>
 #include <linux/of.h>
+#include <linux/of_clk.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
@@ -1361,7 +1361,7 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
 			goto gpiochip_error;
 	}
 
-	ret = of_count_phandle_with_args(node, "clocks", "#clock-cells");
+	ret = of_clk_get_parent_count(node);
 	clk = devm_clk_get(&pdev->dev, ret == 1 ? NULL : "apb");
 	if (IS_ERR(clk)) {
 		ret = PTR_ERR(clk);
-- 
2.7.4

Re: [PATCH v2 5/5] pinctrl: sunxi: Use of_clk_get_parent_count() instead of open coding

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

On Tue, Apr 10, 2018 at 02:51:41PM +0200, Geert Uytterhoeven wrote:
> A new open coder has crept in since 470b73a38470e8ba ("pinctrl: sunxi:
> Use of_clk_get_parent_count() instead of open coding"), replace it.
> 
> of_clk_get_parent_count() was moved to <linux/of_clk.h>, so include that
> instead of <linux/clk-provider.h>.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]