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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
