[PATCHv5 01/31] CLK: clkdev: add support for looking up clocks from DT

Hi Russell,

On Saturday 03 of August 2013 19:35:43 Russell King - ARM Linux wrote:

On Sat, Aug 03, 2013 at 04:02:36PM +0200, Tomasz Figa wrote:

quoted

quoted

+	if (cl)
+		return cl;
+
+	/* If clock was not found, attempt to look-up from DT */
+	node = of_find_node_by_name(NULL, con_id);

Why are we introducing the "lookup by name" brokenness to the yet
(mostly) sane DT world?

We already have a good way of binding things together in DT, which is
using phandles.

Not even saying that this (or something this patch relies on) breaks
the ePAPR recommendation about node naming, which states that node
names should not be used to convey platform-specific data, but
instead should be as generic as possible to show what kind of
hardware is represented by the node.

Tell me this: many devices name their clock inputs.  You can find these
input names in data sheets and the like.  These are the names defined
by the hardware.  What is wrong about using those names in DT?

Yes, clock inputs. But this is about lookup by clock name, not clock input 
name, as far as I can tell from the patch, based on looking for a node 
with given name over the whole DT.

Remember that the second argument to clk_get() is the _clock_ _input_
_name_ on the _device_ passed in the first argument.  It is not *ever*
some random name of the clock producer unless someone's fscked up their
use of this API (in which case they're the ones with the problem.)

This is perfectly fine and this is how the generic common clock framework 
DT bindings work - clock-names property is a list of clock input names and 
clocks property contain specifiers of respective clocks.

Best regards,
Tomasz