[PATCH 1/2] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Simon Glass wrote at Wednesday, January 25, 2012 1:14 PM:
> On Wed, Jan 25, 2012 at 11:57 AM, Stephen Warren <swarren at 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.

-- 
nvpublic