Re: Support for trees of clocks served by one clock manager

[Aymeric Vincent <aymericvincent%free.fr@localhost>]

Jared McNeill <jmcneill%invisible.ca@localhost> writes:

> In the generic binding docs, there is no mention of searching for
> parent clock controllers. The cross reference from clock consumer
> contains the phandle of the clock controller and optional specifier.

Well, it mentions a "clock provider" as a clock with possibly multiple
outputs, and the current users (e.g. tegra) in netbsd blur the two
concepts because their architecture/dtb make it so. (i.e. one clock
provider with lots of subclocks)

I agree with the fact that climbing the tree is inelegant but I would
like to advocate another solution: another way of doing this could be to
have very few registered "clock managers" (that's how I understood
"clock controller") and when a clock has to be decoded, its
phandle(+index) would be provided to these clock managers until one
accepts it. This would remove the weird part of looking for the magic
method directly inside each clock provider, and it's safe because each
phandle is unique and each clock will be claimed by only one clock
manager.

Creating "clock controllers" for each clock in the system will end up as
a bunch of small data bits whose role will only be to grab often the
same method which will provide the associated struct clk *. It becomes a
~useless concept and yet each has to be registered globally.  Please
note that simply because of the way the tegra dtb is written, this
architecture is not plagued by this design defect. It actually looks by
chance very close to the way I describe things here.

Also, I don't buy into having a (struct) device per clock; matching the
fdt with the autoconf tree looks nice on the paper, but it actually
complicates things uselessly for clocks (they come by trees of dozens on
"modern" socs). IMHO the right level of abstraction is exactly "clock
managers".

More generally, clock managers have to play a central role in many
cases: clocks are inter-dependent in many ways and this has to be
centralised somewhere. Ideally, setting a clock should update a
constraints system for which a global solution would have to be found.
[ If you request a rate for a clock, it might be possible to satisfy
  this request at the cost of having to set the rate of many other
  clocks, not just parents. ]

In summary, agreed, don't commit the loop, but please give access to the
clock phandle in decode() in any way you see fit. For the time being I
will register bunches of "clock controllers" until, I hope, you agree
with the "few clock managers" approach. :-)

Regards,
 Aymeric