Re: [PATCH 14/26] clk: mvebu: Convert to clk_hw based provider APIs

From: Stephen Boyd
Date: Wed Oct 14 2015 - 17:09:08 EST


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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/