[PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup

[Rob Herring]

On 08/21/2012 07:27 AM, Russell King - ARM Linux wrote:
> On Mon, Aug 20, 2012 at 12:22:56PM -0500, Matt Sealey wrote:
>> You're going to have to define these clocks as a tree with parents and
>> leaf nodes anyway in the clock subsystem. Why not define these in the
>> device tree in total and reference them by handle when you build the
>> entire clock tree from the ground up? Or will it just be all the
>> clocks defined in Linux, but the lookups (which is what I see here)
>> moved into the DT? Why not form the lookups as part of the definition
>> of the clock tree?
> 
> Well, IMHO the DT conversion of the clk lookup stuff has been done
> completely wrong.
> 
> What should have been done is rather than invent a totally new bloody
> lookup interface that drivers have to use instead of clk_get(), is to
> embed the OF lookup _inside_ clk_get().

That is exactly what was done. Drivers only use clk_get. Only if you
don't have a struct device, then you can use of_clk_get.

Internally, you still need a conversion of clk provider node and cell to
a struct clk. It is up to each clk provider how to do this. The lookup
done here by Shawn is using the struct clk name and using the existing
clk framework lookup.

> What you do is this:
> 
> 1. Have property names in the device node like:
> 
> 	clock_<connection-id> = <&provider-node output>

The connection id is defined by the position in the list and
supplemented with "clock-names" property.

> 
>    In the case of a NULL connection id:
> 
> 	clock = <&provider-node output>
> 
>    Remember that the connection ID is _supposed_ to be something that
>    is described by the hardware (like - for the AACI primecell, the
>    clock which runs the functional side is called "AACICLK" by the TRM,
>    and for the MMCI primecell, it's "MMCICLK" - even though these two
>    clocks may be fed by the same source in an implementation.)
> 
> 2. clkdev's lookup is then modified to look at the struct device, and
>    check for a DT node.  If there is a DT node, it formats a property
>    string:
> 
> 	if (dev->of_node) {
> 		char *propname, *clk_prop = NULL;
> 		struct property *p;
> 
> 		if (conn_id) {
> 			clk_prop = kasprintf("clock_%s", conn_id);
> 			propname = clk_prop;
> 		} else {
> 			propname = "clock";
> 		}
> 
> 		p = of_find_property(dev->of_node, propname, NULL);
> 		if (clk_prop)
> 			kfree(clk_prop);
> 
> 		if (p) {
> 			clk = clk_get_from_of_property(p);
> 			if (clk)
> 				return clk;
> 		}
> 
> 		/* Fallthrough to clkdev table lookup */
> 	}
> 
> So now, you're not dealing with inventing a whole load of names for clocks
> on a platform, instead what you're doing is describing _where_ the clock
> comes from in the system for a particular device by device node and index
> into it - just like we do for interrupts.

That is what we're doing. The names are optional for DT, but happen to
be required for struct clk now. If we don't put something in DT, then
the clock names will have to be something generic like ccm-1..ccm-185.

Rob

> 
> This means there's no need for huge tables and such like of clock names.
> 
> I did mention this idea long ago but got ignored.
>