[PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> > +* NVIDIA Tegra20 Clock And Reset Controller
> > +
> > +This binding uses the common clock binding:
> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> > +for muxing and gating Tegra's clocks, and setting their rates.
> > +
> > +Required properties :
> > +- compatible : Should be "nvidia,<chip>-car"
> > +- reg : Should contain CAR registers location and length
> > +- clocks : Should contain phandle and clock specifiers for two clocks:
> > +  the 32 KHz "32k_in", and the board-specific oscillator "osc".
> > +- clock-names : Should contain a list of strings, with values "32k_in",
> > +  and "osc".
> 
> Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
> where it isn't strictly necessary. Just because some vendors don't want
> to define an order between their clocks doesn't mean it's a good idea
> for everybody to use that model. In this case, just declaring that the
> two clocks refs have to be to those two clocks in that order should
> be sufficient.

OK, that seems reasonable. I'm happy using of_clk_get() rather than
of_clk_get_by_name(). I guess that means we should just avoid any
discussion of clock-output-names too.

> > +- #clock-cells : Should be 1.
> > +  In clock consumers, this cell represents the clock ID exposed by the CAR.
> > +  For a list of valid values, see the clock-output-names property.
> > +
> > +Optional properties :
> > +- clock-output-names : Should contain a list of strings defining the clocks
> > +  that the CAR provides. The list of names and clock IDs is below. The IDs
> > +  may be used in clock specifiers. The names should be listed in this clock-
> > +  output-names property, sorted in ID order, if this property is present.
> > +
> > +  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.
> 
> Aren't these names hardcoded per SoC? If so, they can be derived from the
> compatible field + output number without having a table in the device tree.
>
> If anything, you might want to enumerate the allowed/expected values, but
> actually putting them in a property seems overkill.

Yes, the set of clocks is hard-coded per SoC, and the clock is uniquely
identified by compatible+id.

clock-output-names doesn't actually serve any functional purpose in
Grant's common clock bindings patches; it's more of a documentation
thing. As such, yes, I can remove the suggestion to actually put the
table into the device tree, and rework the binding to simply define
what the mapping is independently.

...
> > +Example:
> > +
> > +clocks {
> > +	clk_32k: oscillator at 0 {
> 
> If it has a unit addres (@<x>) it needs a reg property with the same base
> address.

I thought everything needed a unit address period? Is the rule more like
the unit address is optional, but if present must match reg, so I can
simply remove @0 and @1 here? I guess that makes a lot more sense.

> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <32768>;
> > +	};
> > +
> > +	osc: oscillator at 1 {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <12000000>;
> > +	};
> > +};

-- 
nvpublic