Simon Glass wrote at Wednesday, January 25, 2012 1:14 PM:
On Wed, Jan 25, 2012 at 11:57 AM, 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.
...
v2:
...
- Resolve FIXME re: multiple clocks with the same "reset ID"; give each
unique clock an ID, and ignore the reset bits, since this is purely a clock binding. Code (e.g. U-Boot) that wants to use this to determine CAR reset bit numbers would need a table to convert from the clock IDs in this binding to the related reset bit number, if any. In general, I think that's true, and the U-Boot code that handles "peripheral IDs" should be reworked to handle "clocks", the peripheral clocks being a subset of all clocks.
The clock enable and reset enable bits use the same numbering. I think you have invented a third which is an arbitrary number which doesn't correspond to anything in hardware. That makes sense for pin mux function perhaps, since the indirection provides a useful concept of function number, but here I can't see the benefit.
I didn't do it because there was specific benefit, but simply because we have no choice.
There are clocks that don't have reset bits or clock enable bits.
There are some clocks that are really different clocks (different mux selection or divider control registers) yet share the same bit for reset and clock enable.
Therefore it simply isn't true that there's a 1:1 mapping between clocks and clock-enable/reset bits.
I deliberately made this updated binding have a different numbering scheme to the clock-enable/reset bits to make this clear, so that no one would accidentally confuse the two concepts.
- Define clock IDs for all the non-peripheral clocks too; inputs, PLLs,
etc.
- Separate tegra-seaboard.dts usage example into a separate patch.
This patch semantically relies on Grant Likely's common clock binding patch series. Once that's finalized, this patch could be checked into the kernel provided there are no relevant changes to Grant's patches. I believe that Simon Glass wants to start using this within U-Boot ASAP though.
As I may have said I am really not keen on the idea of having a table just to use it in U-Boot.
I don't see any alternative given the way the HW is designed.
We could ignore the way the HW works and assume that clock ID == clock- enable/reset bits is true for many clocks. However, it's not true for all, so I think that'd be too error-prone.
Equally, I know that you will need a table sometime in U-Boot to map from clock ID to clock-enable/reset bit and many other per-clock parameters if you're going to be initializing stuff in U-Boot from DT rather than hard-coding it, so I think you may as well just add it now.
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
...
- The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
- registers. Later, subsequent IDs will be assigned to all other clocks the
- CAR controls; mainly the PLLs.
Are you sure? The ordering doesn't seem to fit with U-Boot's enum periph_id in arch/arm/include/asm/arch-tegra2/clock.h. That file follows along with the hardware.
No, that paragraph is wrong. I simply forgot to remove it.