lkml.org 
[lkml]   [2015]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 14/26] clk: mvebu: Convert to clk_hw based provider APIs
Stephen,

On Wed, 14 Oct 2015 14:08:59 -0700, Stephen Boyd wrote:

> Here's that untested patch, which we can throw into clk-next for
> v4.4
>
> -----8<----
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b005f666e3a1..16b86a551bcb 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(np, index);
> + 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;

It almost worked, but not completely. The issue is that by the time you
call of_clk_get(np, index), index is no longer equal to the value
passed as argument to of_clk_get_parent_name(), it has been modified to
indicate the index of the *parent* clock.

So, after changing your patch to:

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0ebcf44..60d2f62 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3052,9 +3052,11 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
struct property *prop;
const char *clk_name;
const __be32 *vp;
+ int parent_index;
u32 pv;
int rc;
int count;
+ struct clk *clk;

if (index < 0)
return NULL;
@@ -3064,14 +3066,14 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
if (rc)
return NULL;

- index = clkspec.args_count ? clkspec.args[0] : 0;
+ parent_index = clkspec.args_count ? clkspec.args[0] : 0;
count = 0;

/* if there is an indices property, use it to transfer the index
* specified into an array offset for the clock-output-names property.
*/
of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
- if (index == pv) {
+ if (parent_index == pv) {
index = count;
break;
}
@@ -3079,9 +3081,26 @@ 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;
+ parent_index,
+ &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(np, index);
+ 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;
It does work properly for me, on top of 4.3-rc5, without any change to
my Device Tree files.

So, what is the plan now ?

- Have the minimal fix in drivers/clk/mvebu/clk-cpu.c for 4.3.

- Have the patch improving the of_clk_get_parent_name() logic merged
in 4.4 + a revert of the fix done for 4.3 in clk-cpu.c. Unless maybe
you don't want to clutter the core of the clock framework to handle
this specific case?

- Add the clock-output-names to the Marvell EBU Device Tree files, so
that in a couple of kernel releases we can remove the hacks and rely
on the generic logic of of_clk_get_parent_name().

Thanks,

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


\
 
 \ /
  Last update: 2015-10-15 11:01    [W:0.119 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site