Simon Glass wrote at Sunday, January 22, 2012 11:03 AM: > On Wed, Jan 18, 2012 at 4:16 PM, Stephen Warren <swarren@xxxxxxxxxx> 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. -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
