ARM clock API to PowerPC

[Benjamin Herrenschmidt]

On Wed, 2009-08-12 at 22:44 +0100, Mark Brown wrote:
> On Thu, Aug 13, 2009 at 07:34:07AM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2009-08-12 at 13:35 +0100, Mark Brown wrote:
> 
> > > What happens if another clock gets added or the list gets reordered for
> > > some reason?
> 
> > The device-tree is mostly static in that regard. I'm not sure what you
> > mean. Clocks are referenced by IDs so if you want to add more clocks you
> > can just add them to the end. The "NULL" case is typically for things
> > that have only one clock (which seems to be reasonably common in ARM
> > land).
> 
> The issue I'm worrying about here is what happens if the device has only
> one clock in current revisions of the hardware but another revision of
> the same hardware is produced which adds another clock.  When that
> happens your NULL lookup gets confused.

Well, then don't use clk_get() with NULL :-) I mean, ARM devices do that
today (ie, calling with NULL), it makes sense in the context of a
device-tree binding to provide a defined behaviour for it (pick the
first clock in the order it appears in the device-tree is one way to do
it), but we can also recommend to driver writers not to do it :-)

But if one was to use an existing driver that does it, then we need to
cater for it.

Note that adding a new clock in the future isn't a big deal. As long as
the first one in the list is the one the driver that calls clk_get with
NULL expects to obtain.

Maybe we can make clock-names non-optional though in the DT as an
incentive not to use the simple 1-clock "NULL" path.

> > I think it's reasonable to ask whoever produces the device-tree to keep
> > the two properties in sync.
> 
> It's not just the device tree, it's also the drivers which have to be
> able to cope with whatever random device tree that's thrown at them.

Well, the clocks are named. At some stage, the binding for a given
device will define what clock names it expects. I don't see that
differing from what the ARM folks do.

> This is less of a problem on ARM where it's fairly straightforward to
> adjust the users at the same time as the driver but if you've got a
> device tree delivered via a completely different distribution mechanism
> it gets more sticky.

Possibly yes, and that's a reason why we tend to discourage that ;-) But
again, it boils down to settling with a naming convention for a given
device. If clocks are added later on, the driver will have to cope with
the new clocks not existing, of course, or the platform can cater for
it.

That's also one of the reason why I want to make clk_get() go through
ppc_md. first, so that the platform can always override the behaviour,
and for example, fill-in in case the device-tree is incomplete or
incorrect.

> The problem with the API at the minute in this regard is that there is
> no standard way of registering new clocks.  Only something that knows
> about the magic sauce for a given architecture (if any) is able to add
> clocks.  My concern here is that if PowerPC moves in this direction
> without some general agreement from elsewhere then there may be problems
> for drivers.

Well, the idea is to allow to register clock-providers associated with
device nodes. I think we should keep the rest in the hand of platforms
for now, and possibly make it evolve later.

Do you have maybe a precise example scenario where your above statement
about the lack of facility for registering new clocks is a problem ? I'm
curious to see a real life example so I can think better about how it
can be solved (or whether it needs to be solved).

Cheers,
Ben.