From: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@codeaurora.org>,
"Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
Cc: linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
"Gregory CLEMENT <gregory.clement@free-electrons.com>,
Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
geert@linux-m68k.org, magnus.damm@gmail.com
Subject: Re: [PATCH] clk: Make of_clk_get_parent_name() robust with #clock-cells = 1
Date: Fri, 16 Oct 2015 05:55:33 -0700 [thread overview]
Message-ID: <20151016125533.20687.23387@quantum> (raw)
In-Reply-To: <1444951178-18733-1-git-send-email-sboyd@codeaurora.org>
Cc'ing Geert.
Quoting Stephen Boyd (2015-10-15 16:19:38)
> If a clock provider has #clock-cells =3D 1 and we call
> of_clk_get_parent_name() on it we may end up returning the name
> of the provider node if the provider doesn't have a
> clock-output-names property. This doesn't make sense, especially
> when you consider that calling of_clk_get_parent_name() on such a
> node with different indices will return the same name each time.
> =
> Let's try getting the clock from the framework via of_clk_get()
> instead, and only fallback to the node name if we have a provider
> with #clock-cells =3D 0. This way, we can't hand out the same name
> for different clocks when we don't actually know their names.
> =
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Geert,
Does this approach fix the issues you had removing clock-output-names
from your clock provider DTS?
Regards,
Mike
> ---
> drivers/clk/clk.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
> =
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b005f666e3a1..a0fe9ca1f0af 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3055,6 +3055,7 @@ const char *of_clk_get_parent_name(struct device_no=
de *np, int index)
> u32 pv;
> int rc;
> int count;
> + struct clk *clk;
> =
> if (index < 0)
> return NULL;
> @@ -3080,8 +3081,25 @@ const char *of_clk_get_parent_name(struct device_n=
ode *np, int index)
> =
> if (of_property_read_string_index(clkspec.np, "clock-output-names=
",
> index,
> - &clk_name) < 0)
> - clk_name =3D clkspec.np->name;
> + &clk_name) < 0) {
> + /*
> + * Best effort to get the name if the clock has been
> + * registered with the framework. If the clock isn't
> + * registered, we return the node name as the name of
> + * the clock as long as #clock-cells =3D 0.
> + */
> + clk =3D of_clk_get_from_provider(&clkspec);
> + if (IS_ERR(clk)) {
> + if (clkspec.args_count =3D=3D 0)
> + clk_name =3D clkspec.np->name;
> + else
> + clk_name =3D NULL;
> + } else {
> + clk_name =3D __clk_get_name(clk);
> + clk_put(clk);
> + }
> + }
> +
> =
> of_node_put(clkspec.np);
> return clk_name;
> -- =
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>=20
WARNING: multiple messages have this Message-ID (diff)
From: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@codeaurora.org>,
"Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
Cc: linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
"Gregory CLEMENT <gregory.clement@free-electrons.com>,
Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
geert@linux-m68k.org, magnus.damm@gmail.com
Subject: Re: [PATCH] clk: Make of_clk_get_parent_name() robust with #clock-cells = 1
Date: Fri, 16 Oct 2015 05:55:33 -0700 [thread overview]
Message-ID: <20151016125533.20687.23387@quantum> (raw)
In-Reply-To: <1444951178-18733-1-git-send-email-sboyd@codeaurora.org>
Cc'ing Geert.
Quoting Stephen Boyd (2015-10-15 16:19:38)
> If a clock provider has #clock-cells = 1 and we call
> of_clk_get_parent_name() on it we may end up returning the name
> of the provider node if the provider doesn't have a
> clock-output-names property. This doesn't make sense, especially
> when you consider that calling of_clk_get_parent_name() on such a
> node with different indices will return the same name each time.
>
> Let's try getting the clock from the framework via of_clk_get()
> instead, and only fallback to the node name if we have a provider
> with #clock-cells = 0. This way, we can't hand out the same name
> for different clocks when we don't actually know their names.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Geert,
Does this approach fix the issues you had removing clock-output-names
from your clock provider DTS?
Regards,
Mike
> ---
> drivers/clk/clk.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b005f666e3a1..a0fe9ca1f0af 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3055,6 +3055,7 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
> u32 pv;
> int rc;
> int count;
> + struct clk *clk;
>
> if (index < 0)
> return NULL;
> @@ -3080,8 +3081,25 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
>
> if (of_property_read_string_index(clkspec.np, "clock-output-names",
> index,
> - &clk_name) < 0)
> - clk_name = clkspec.np->name;
> + &clk_name) < 0) {
> + /*
> + * Best effort to get the name if the clock has been
> + * registered with the framework. If the clock isn't
> + * registered, we return the node name as the name of
> + * the clock as long as #clock-cells = 0.
> + */
> + clk = of_clk_get_from_provider(&clkspec);
> + if (IS_ERR(clk)) {
> + if (clkspec.args_count == 0)
> + clk_name = clkspec.np->name;
> + else
> + clk_name = NULL;
> + } else {
> + clk_name = __clk_get_name(clk);
> + clk_put(clk);
> + }
> + }
> +
>
> of_node_put(clkspec.np);
> return clk_name;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
next prev parent reply other threads:[~2015-10-16 12:55 UTC|newest]
Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-31 17:03 [PATCH 00/26] Remove struct clk based provider APIs Stephen Boyd
2015-07-31 17:03 ` [PATCH 01/26] clk: Add clk_hw_get_num_parents() Stephen Boyd
2015-07-31 17:03 ` [PATCH 02/26] clk: Replace __clk_get_num_parents with clk_hw_get_num_parents() Stephen Boyd
2015-07-31 18:46 ` Boris Brezillon
2015-08-06 8:09 ` Sylwester Nawrocki
2015-08-07 22:40 ` Stephen Boyd
2015-09-18 0:18 ` Scott Wood
2015-09-18 15:56 ` Stephen Boyd
2015-09-18 18:27 ` Scott Wood
2015-09-18 23:18 ` Stephen Boyd
2015-09-20 2:03 ` Scott Wood
2015-07-31 17:03 ` [PATCH 03/26] clk: Remove __clk_get_num_parents() Stephen Boyd
2015-07-31 17:03 ` [PATCH 04/26] clk: Add clk_hw_get_flags() Stephen Boyd
2015-07-31 17:03 ` [PATCH 05/26] clk: Convert __clk_get_flags() to clk_hw_get_flags() Stephen Boyd
2015-08-10 21:00 ` Sebastian Hesselbarth
2015-07-31 17:03 ` [PATCH 06/26] clk: Add clk_hw_*() API for use by providers Stephen Boyd
2015-07-31 17:03 ` [PATCH 07/26] clk: ti: Remove CLK_IS_BASIC check Stephen Boyd
2015-07-31 17:03 ` [PATCH 08/26] ARM: OMAP: Convert __clk_get_rate() to provider/consumer APIs Stephen Boyd
2015-07-31 17:03 ` [PATCH 09/26] MIPS: alchemy: Convert to clk_hw based provider APIs Stephen Boyd
2015-07-31 17:03 ` [PATCH 10/26] clk: at91: " Stephen Boyd
2015-07-31 18:47 ` Boris Brezillon
2015-07-31 17:03 ` [PATCH 11/26] clk: bcm: " Stephen Boyd
2015-07-31 17:33 ` Alex Elder
2015-07-31 17:03 ` [PATCH 12/26] clk: Convert basic types " Stephen Boyd
2015-07-31 17:03 ` [PATCH 13/26] clk: mmp: Convert " Stephen Boyd
2015-07-31 17:03 ` [PATCH 14/26] clk: mvebu: " Stephen Boyd
2015-10-14 15:09 ` Thomas Petazzoni
2015-10-14 18:21 ` Stephen Boyd
2015-10-14 20:17 ` Thomas Petazzoni
2015-10-14 21:08 ` Stephen Boyd
2015-10-15 8:43 ` Thomas Petazzoni
2015-10-15 18:09 ` Stephen Boyd
2015-10-15 19:56 ` Thomas Petazzoni
2015-10-15 23:19 ` [PATCH] clk: Make of_clk_get_parent_name() robust with #clock-cells = 1 Stephen Boyd
2015-10-16 12:55 ` Michael Turquette [this message]
2015-10-16 12:55 ` Michael Turquette
2015-10-16 13:02 ` Geert Uytterhoeven
2015-10-21 8:41 ` Thomas Petazzoni
2015-10-15 8:22 ` [PATCH 14/26] clk: mvebu: Convert to clk_hw based provider APIs Thomas Petazzoni
2015-10-15 18:02 ` Stephen Boyd
2015-07-31 17:03 ` [PATCH 15/26] clk: stm32f4: " Stephen Boyd
2015-07-31 17:03 ` [PATCH 16/26] clk: qcom: " Stephen Boyd
2015-07-31 17:03 ` [PATCH 17/26] clk: rockchip: " Stephen Boyd
2015-08-04 14:12 ` Heiko Stübner
2015-08-07 23:45 ` Stephen Boyd
2015-07-31 17:03 ` [PATCH 18/26] clk: samsung: " Stephen Boyd
2015-08-06 8:15 ` Sylwester Nawrocki
2015-07-31 17:03 ` [PATCH 19/26] clk: sirf: " Stephen Boyd
2015-07-31 17:04 ` [PATCH 20/26] clk: spear: " Stephen Boyd
2015-08-01 11:36 ` Viresh Kumar
2015-07-31 17:04 ` [PATCH 21/26] clk: sunxi: " Stephen Boyd
2015-07-31 17:04 ` [PATCH 22/26] clk: tegra: " Stephen Boyd
2015-07-31 17:04 ` [PATCH 23/26] " Stephen Boyd
2015-08-03 8:17 ` Tero Kristo
2015-08-03 18:08 ` Stephen Boyd
2015-07-31 17:04 ` [PATCH 24/26] clk: versatile: " Stephen Boyd
2015-07-31 17:08 ` Pawel Moll
2015-07-31 17:40 ` Stephen Boyd
2015-07-31 23:44 ` [PATCH 0/3] Move clk-sp810 to assigned clock parents Stephen Boyd
2015-07-31 23:44 ` Stephen Boyd
2015-07-31 23:44 ` [PATCH 1/3] clk: versatile: Switch " Stephen Boyd
2015-07-31 23:44 ` Stephen Boyd
2015-08-03 14:01 ` Pawel Moll
2015-08-03 14:01 ` Pawel Moll
2015-08-03 17:55 ` Stephen Boyd
2015-08-03 17:55 ` Stephen Boyd
2015-08-05 10:29 ` Pawel Moll
2015-08-05 10:29 ` Pawel Moll
2015-08-05 17:56 ` Stephen Boyd
2015-08-05 17:56 ` Stephen Boyd
2015-08-06 15:05 ` Pawel Moll
2015-08-06 15:05 ` Pawel Moll
2015-08-07 22:28 ` Stephen Boyd
2015-08-07 22:28 ` Stephen Boyd
2015-07-31 23:44 ` [PATCH 2/3] ARM: dts: vexpress: Use assigned-clock-parents for sp810 Stephen Boyd
2015-07-31 23:44 ` Stephen Boyd
2015-07-31 23:44 ` [PATCH 3/3] ARM64: " Stephen Boyd
2015-07-31 23:44 ` Stephen Boyd
2015-08-03 10:18 ` [PATCH 0/3] Move clk-sp810 to assigned clock parents Sudeep Holla
2015-08-03 10:18 ` Sudeep Holla
2015-07-31 17:04 ` [PATCH 25/26] drm/msm/dsi: Convert to clk_hw based provider APIs Stephen Boyd
2015-07-31 17:04 ` [PATCH 26/26] clk: Remove unused " Stephen Boyd
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151016125533.20687.23387@quantum \
--to=mturquette@baylibre.com \
--cc=geert@linux-m68k.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=sboyd@codeaurora.org \
--cc=sebastian.hesselbarth@gmail.com \
--cc=thomas.petazzoni@free-electrons.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.