On 10/14, Thomas Petazzoni wrote:
> On Wed, 14 Oct 2015 11:21:38 -0700, Stephen Boyd wrote:
> 
> > > Stephen, what do you suggest to fix this issue?
> > 
> > Simplest fix is to revert this hunk.
> 
> Indeed, this is the simplest fix. But it's going backward with what you
> were trying to do originally, so it isn't really nice either.
> 
> > Longer term, we should look into making of_clk_get_parent_name() use
> > whatever string has been used when registering the clock,
> 
> I am not sure how of_clk_get_parent_name() can know the name of the
> clock without clock-output-names in the DT. Without clock-output-names,
> the only way to know the name of the parent clock is to actually ask
> the driver of the parent clock, no?

Yes that's the plan. We would try to resolve the name with
of_clk_get() and __clk_get_name() if there isn't a
clock-output-names property. If #clock-cells > 1 and of_clk_get()
fails, we would return NULL, otherwise for #clock-cells == 0 we
could fallback on the node name as a last resort. The change here
is that we don't use the node name for providers with
#clock-cells > 1 because that doesn't make any sense. We'll also
try a clk_get() in the case with #clock-cells = 0 and no
clock-output-names.

Eventually once we add the ability for OF clk providers to
register clk_hw pointers instead of clk pointers we'll be able to
"simplify" the of_clk_get() + __clk_get_name() path with a
provider clk_hw lookup and clk_hw_get_name() combination, which
should be faster and avoid any allocations.

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;

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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/

Reply via email to