[PATCH v4 5/6] clocksource: armada-370-xp: Fix device-tree binding

[Stephen Warren]

On 08/15/2013 10:27 AM, Ezequiel Garcia wrote:
...
> Armada XP
> ---------
> 
> Two clock sources are available for timer and watchdog counters:
> 
> Just as explained for the Armada 370, the timer and watchdog counters decrement
> rate is a configurable ratio of the L2/coherency fabric clock.
> The current clocksource driver implementation chooses an abritrary ratio.
> 
> In addition to this, both timer and watchdog counter rate can be configured
> to use an (internal) 25 MHz fixed clock.

So there are clearly two clocks fed into the HW block here. The DT
should reflect that.

> However, and as explained in this patchset, using the letter fixed clock is in
> practice the only choice. Once CPUFreq support is implemented for the
> Armada XP SoC the L2/coherency fabric clock will change its rate, and handling
> such change will be problematic.

It's not mandatory for an OS to implement cpufreq, so it'd be quite
possible to still use that variable-rate clock on some HW. What SW
may-or-may-not do on HW isn't something that should feed into the DT
binding design.

> Maybe the most accurate representation would be something like this...
> 
> timer {
> 	compatible = "marvell,armada-xp-timer";
> 	reg = ...
> 	interrupts = ...
> 	clocks = ... /* either 25 MHz fixed clock
> 	              * or L2/coherency fabric clock */
> };

I think better would be:

clocks = <&xxx ...> <&yyy ...>;
clock-names = "fabric", "fixed";

In the documentation for this HW block, there's a register that switches
between the two clock sources. What names are used for the clocks in the
documentation of that register? Those names should be used for
clock-names entries.

Is the fact that one of "fabric" and "fixed" is variable and one
fixed-rate more a facet of the integration of the IP block into the SoC,
and not forced by the design of the IP block itself? If so, perhaps
you'd want some additional properties indicating which of those clocks
was fixed and which was variable, which would allow SW to make
intelligent decisions re: which to use. Perhaps it can be assumed this
information can be queried from the source of the clock though?