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

[Stephen Boyd]

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/