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

[Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>]

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().

What you do is this:

1. Have property names in the device node like:

	clock_<connection-id> = <&provider-node output>

   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.

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.