Simon Glass wrote at Sunday, January 22, 2012 11:03 AM:
On Wed, Jan 18, 2012 at 4:16 PM, Stephen Warren swarren@nvidia.com wrote:
Document a binding for the Tegra20 CAR (Clock And Reset) Controller, add it to tegra20.dtsi, and configure it for the board in tegra- seaboard.dts.
...
A comment on the shared enable issue:
When enabling/disabling clocks, Tegra requires you to reset the HW module that the clock is routed to. Both reset and clock enable registers are part of the CAR. U-Boot currently has an API to do the reset based on a "peripheral ID", which simply means a bit number in a set of 3 "reset" registers. The bits in those registers share the same numbering as the "clock enable" registers. Hence, to make life easy for U-Boot, I've defined the clock IDs in this binding to be equal to these bit numbers. However, this breaks down where there isn't a 1:1 mapping between reset and clock enable bits, and clocks. For example, there's a single reset and clock enable bit for the SPDIF controller, yet this applies to two clocks; spdif_in and spdif_out. Hence, this simplification for U-Boot breaks down. I think the correct solution is to fix U-Boot not to require this simplification, renumber the clocks in the CAR binding in a completely arbitrary fashion, and require U-Boot to contain a mapping table between CAR's DT clock IDs and any other information related to those clocks. Do you see any alternative? We really need the two SPDIF clocks to be different clock IDs in the binding, since each has a completely independant mux for the clock source/parent, and the two clocks obviously can't share the same clock ID (well, hmm, perhaps they could if we used 2 cells for the specifier, 1 for the bit number, and one more to differentiate clocks that use the same bit...). I'm still inclined to simply push back on U-Boot to do it right.
Are SPDIF and VI the only examples of this?
I think so. I should double-check to be sure though before committing to a final binding.
If so, perhaps just having a special extra clock ID for those two and mapping in a special way would be more efficient. So two IDs would map to one clock/reset bit but different clocks.
I thought about that initially, but it doesn't make semantic sense; there isn't a 1:1 mapping between clock ID and reset bit, so I don't think we should pretend there is for some clocks, and special-case others.
Also I note that one is an input and one is an output clock. So we could name them this way, and use the separate ID for the input clocks perhaps.
I don't think that solves the problem; HW modules and drivers use both clocks, so special-casing the input clocks doesn't make the problem less relevant.
If you do add an arbitrary mapping, what would it be? It might as well follow along with the registers so far as it can, since the device tree should describe the hardware. Then the mapping becomes a few lines of code in the driver instead of YAT (yet another table).
The mapping would be that the clock IDs are unrelated to the bit numbers in the CAR's reset/clock-enable registers.
In practice, this means that to find the reset bit number, you need a table indexed by the .dts's clock number, with the bit number being retrieved from that table.
So instead of bit = of_get_u32(1), we'd have bit = table[of_get_u32(1)].
In practice, I believe we'll need such a table anyway, since each clock has many more parameters than just the reset bit ID; some clocks have no reset bit at all, some clocks have a mux but some don't, the inputs to the mux are different for each clock, some have a divider but some don't, where there's a divider or mux, you need to know the register address to configure them, each clock has a different max rate, etc.
Having thought a little more about this, I'm definitely leaning towards this way; making the clock ID and register bit completely unrelated just to make it really obvious that you can't use the clock ID as a register bit.