2018-04-10 12:58:27

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2 0/5] Use of_clk_get_parent_count() instead of open coding

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


2018-04-10 12:55:09

by Geert Uytterhoeven

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

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


2018-04-10 12:56:05

by Geert Uytterhoeven

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

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


2018-04-10 12:56:22

by Geert Uytterhoeven

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

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
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


2018-04-10 12:56:41

by Geert Uytterhoeven

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

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 <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
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


2018-04-10 12:57:36

by Geert Uytterhoeven

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

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


2018-04-10 13:10:25

by Maxime Ripard

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

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

Acked-by: Maxime Ripard <[email protected]>

Maxime

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


Attachments:
(No filename) (581.00 B)
signature.asc (849.00 B)
Download all attachments

2018-04-10 13:20:53

by Heiko Stuebner

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

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 <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>

Really cool move,
Reviewed-by: Heiko Stuebner <[email protected]>


2018-04-10 13:25:54

by Heiko Stuebner

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

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

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

for this patch.


Heiko


2018-04-11 09:11:45

by Jon Hunter

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


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

Cheers
Jon

2018-04-13 21:18:14

by Rob Herring (Arm)

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

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 <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> 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

2018-04-16 16:06:34

by Stephen Boyd

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

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 <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> 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.


2018-04-17 08:51:51

by Daniel Lezcano

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

On Tue, Apr 10, 2018 at 02:51:38PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> 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 <[email protected]>

2018-04-26 11:57:48

by Linus Walleij

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

On Tue, Apr 10, 2018 at 2:51 PM, Geert Uytterhoeven
<[email protected]> wrote:

> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> 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 <[email protected]>

Yours,
Linus Walleij