[PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
most clocks within Tegra20. The device tree binding models this as a
single monolithic clock provider, which exports ~130 clocks. This reduces
the number of nodes needed in device tree to represent these clocks.

This binding is only useful for Tegra20; the set of clocks that exists on
Tegra30 is sufficiently different to merit its own binding.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v3:
* Re-order clock ID list so that IDs 0..95 match the CLK_OUT_ENB register
  layout where possible. For register bits that affect multiple clocks,
  assign those clocks IDs outside the range to make this clear.
* Double-checked the documentation, and added a couple more cases where
  a single CLK_OUT_ENB bit affects multiple clocks.
* Split the binding example into separate SoC and board file sections.
v2:
* Remove clock-names, clock-output-names properties; Tegra will solely
  use integer IDs for clocks in DT.
* Fixed typo in compatible flag
* 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.
* 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 conceptually relies on Grant Likely's common clock binding patch
series. However, since that seems pretty stable and people agree with it, I
think we can check in this patch without waiting for Grant's.
---
 .../bindings/clock/nvidia,tegra20-car.txt          |  207 ++++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     |    6 +
 2 files changed, 213 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
new file mode 100644
index 0000000..5c07fca
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
@@ -0,0 +1,207 @@
+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,tegra20-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-cells : Should be 1.
+  In clock consumers, this cell represents the clock ID exposed by the CAR.
+
+  The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
+  registers. These IDs often match those in the CAR's RST_DEVICES registers,
+  but not in all cases. Some bits in CLK_OUT_ENB affect multiple clocks. In
+  this case, those clocks are assigned IDs above 95 in order to highlight
+  this issue. Implementations that interpret these clock IDs as bit values
+  within the CLK_OUT_ENB or RST_DEVICES registers should be careful to
+  explicitly handle these special cases.
+
+  The balance of the clocks controlled by the CAR are assigned IDs of 96 and
+  above.
+
+  0	cpu
+  1	unassigned
+  2	unassigned
+  3	ac97
+  4	rtc
+  5	tmr
+  6	uart1
+  7	unassigned	(register bit affects uart2 and vfir)
+  8	gpio
+  9	sdmmc2
+  10	unassigned	(register bit affects spdif_in and spdif_out)
+  11	i2s1
+  12	i2c1
+  13	ndflash
+  14	sdmmc1
+  15	sdmmc4
+  16	twc
+  17	pwm
+  18	i2s2
+  19	epp
+  20	unassigned	(register bit affects vi and vi_sensor)
+  21	2d
+  22	usbd
+  23	isp
+  24	3d
+  25	ide
+  26	disp2
+  27	disp1
+  28	host1x
+  29	vcp
+  30	unassigned
+  31	cache2
+
+  32	mem
+  33	ahbdma
+  34	apbdma
+  35	unassigned
+  36	kbc
+  37	stat_mon
+  38	pmc
+  39	fuse
+  40	kfuse
+  41	sbc1
+  42	snor
+  43	spi1
+  44	sbc2
+  45	xio
+  46	sbc3
+  47	dvc
+  48	dsi
+  49	unassigned	(register bit affects tvo and cve)
+  50	mipi
+  51	hdmi
+  52	csi
+  53	tvdac
+  54	i2c2
+  55	uart3
+  56	unassigned
+  57	emc
+  58	usb2
+  59	usb3
+  60	mpe
+  61	vde
+  62	bsea
+  63	bsev
+
+  64	speedo
+  65	uart4
+  66	uart5
+  67	i2c3
+  68	sbc4
+  69	sdmmc3
+  70	pcie
+  71	owr
+  72	afi
+  73	csite
+  74	unassigned
+  75	avpucq
+  76	la
+  77	unassigned
+  78	unassigned
+  79	unassigned
+  80	unassigned
+  81	unassigned
+  82	unassigned
+  83	unassigned
+  84	irama
+  85	iramb
+  86	iramc
+  87	iramd
+  88	cram2
+  89	audio_2x	a/k/a audio_2x_sync_clk
+  90	clk_d
+  91	unassigned
+  92	sus
+  93	cdev1
+  94	cdev2
+  95	unassigned
+
+  96	uart2
+  97	vfir
+  98	spdif_in
+  99	spdif_out
+  100	vi
+  101	vi_sensor
+  102	tvo
+  103	cve
+  104	osc
+  105	clk_32k		a/k/a clk_s
+  106	clk_m
+  107	sclk
+  108	cclk
+  109	hclk
+  110	pclk
+  111	blink
+  112	pll_a
+  113	pll_a_out0
+  114	pll_c
+  115	pll_c_out1
+  116	pll_d
+  117	pll_d_out0
+  118	pll_e
+  119	pll_m
+  120	pll_m_out1
+  121	pll_p
+  122	pll_p_out1
+  123	pll_p_out2
+  124	pll_p_out3
+  125	pll_p_out4
+  126	pll_s
+  127	pll_u
+  128	pll_x
+  129	cop		a/k/a avp
+  130	audio		a/k/a audio_sync_clk
+
+Example SoC include file:
+
+/ {
+	tegra_car: clock@60006000 {
+		compatible = "nvidia,tegra20-car";
+		reg = <0x60006000 0x1000>;
+		#clock-cells = <1>;
+	};
+
+	usb@c5004000 {
+		clocks = <&tegra_car 58>; /* usb2 */
+	};
+};
+
+Example board file:
+
+/ {
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		osc: clock {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <12000000>;
+		};
+	};
+
+	i2c@7000d000 {
+		pmic@34 {
+			compatible = "ti,tps6586x";
+			reg = <0x34>;
+
+			clk_32k: clock {
+				compatible = "fixed-clock";
+				#clock-cells = <0>;
+				clock-frequency = <32768>;
+			};
+		};
+	};
+
+	&tegra_car {
+		clocks = <&clk_32k> <&osc>;
+	};
+};
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index ec1f010..550da98 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -4,6 +4,12 @@
 	compatible = "nvidia,tegra20";
 	interrupt-parent = <&intc>;
 
+	tegra_car: clock@60006000 {
+		compatible = "nvidia,tegra20-car";
+		reg = <0x60006000 1000>;
+		#clock-cells = <1>;
+	};
+
 	pmc@7000f400 {
 		compatible = "nvidia,tegra20-pmc";
 		reg = <0x7000e400 0x400>;
-- 
1.7.0.4

Re: [PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Simon Glass]

[-- Attachment #1.1: Type: text/plain, Size: 9029 bytes --]

Hi Stephen,

On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

> The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
> most clocks within Tegra20. The device tree binding models this as a
> single monolithic clock provider, which exports ~130 clocks. This reduces
> the number of nodes needed in device tree to represent these clocks.
>
> This binding is only useful for Tegra20; the set of clocks that exists on
> Tegra30 is sufficiently different to merit its own binding.
>

It seems there is a large common element - they are almost backwards
compatible. Should we not at least look at this now?


>
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v3:
> * Re-order clock ID list so that IDs 0..95 match the CLK_OUT_ENB register
>  layout where possible. For register bits that affect multiple clocks,
>  assign those clocks IDs outside the range to make this clear.
>

Great!


> * Double-checked the documentation, and added a couple more cases where
>  a single CLK_OUT_ENB bit affects multiple clocks.
> * Split the binding example into separate SoC and board file sections.
> v2:
> * Remove clock-names, clock-output-names properties; Tegra will solely
>  use integer IDs for clocks in DT.
> * Fixed typo in compatible flag
> * 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.
> * 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 conceptually relies on Grant Likely's common clock binding patch
> series. However, since that seems pretty stable and people agree with it, I
> think we can check in this patch without waiting for Grant's.
> ---
>  .../bindings/clock/nvidia,tegra20-car.txt          |  207
> ++++++++++++++++++++
>  arch/arm/boot/dts/tegra20.dtsi                     |    6 +
>  2 files changed, 213 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>
> diff --git
> a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> new file mode 100644
> index 0000000..5c07fca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> @@ -0,0 +1,207 @@
> +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,tegra20-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-cells : Should be 1.
> +  In clock consumers, this cell represents the clock ID exposed by the
> CAR.
> +
> +  The first 96 clocks are numbered to match the bits in the CAR's
> CLK_OUT_ENB
> +  registers. These IDs often match those in the CAR's RST_DEVICES
> registers,
> +  but not in all cases. Some bits in CLK_OUT_ENB affect multiple clocks.
> In
> +  this case, those clocks are assigned IDs above 95 in order to highlight
> +  this issue. Implementations that interpret these clock IDs as bit values
> +  within the CLK_OUT_ENB or RST_DEVICES registers should be careful to
> +  explicitly handle these special cases.
> +
> +  The balance of the clocks controlled by the CAR are assigned IDs of 96
> and
> +  above.
> +
> +  0    cpu
> +  1    unassigned
> +  2    unassigned
> +  3    ac97
> +  4    rtc
> +  5    tmr
> +  6    uart1
> +  7    unassigned      (register bit affects uart2 and vfir)
> +  8    gpio
> +  9    sdmmc2
> +  10   unassigned      (register bit affects spdif_in and spdif_out)
> +  11   i2s1
> +  12   i2c1
> +  13   ndflash
> +  14   sdmmc1
> +  15   sdmmc4
> +  16   twc
> +  17   pwm
> +  18   i2s2
> +  19   epp
> +  20   unassigned      (register bit affects vi and vi_sensor)
> +  21   2d
> +  22   usbd
> +  23   isp
> +  24   3d
> +  25   ide
> +  26   disp2
> +  27   disp1
> +  28   host1x
> +  29   vcp
> +  30   unassigned
> +  31   cache2
> +
> +  32   mem
> +  33   ahbdma
> +  34   apbdma
> +  35   unassigned
> +  36   kbc
> +  37   stat_mon
> +  38   pmc
> +  39   fuse
> +  40   kfuse
> +  41   sbc1
> +  42   snor
> +  43   spi1
> +  44   sbc2
> +  45   xio
> +  46   sbc3
> +  47   dvc
> +  48   dsi
> +  49   unassigned      (register bit affects tvo and cve)
> +  50   mipi
> +  51   hdmi
> +  52   csi
> +  53   tvdac
> +  54   i2c2
> +  55   uart3
> +  56   unassigned
> +  57   emc
> +  58   usb2
> +  59   usb3
> +  60   mpe
> +  61   vde
> +  62   bsea
> +  63   bsev
> +
> +  64   speedo
> +  65   uart4
> +  66   uart5
> +  67   i2c3
> +  68   sbc4
> +  69   sdmmc3
> +  70   pcie
> +  71   owr
> +  72   afi
> +  73   csite
>

Would 'coresight' be better given that you have csi also?


> +  74   unassigned
> +  75   avpucq
> +  76   la
>

What is this one?


> +  77   unassigned
> +  78   unassigned
> +  79   unassigned
> +  80   unassigned
> +  81   unassigned
> +  82   unassigned
> +  83   unassigned
> +  84   irama
> +  85   iramb
> +  86   iramc
> +  87   iramd
> +  88   cram2
> +  89   audio_2x        a/k/a audio_2x_sync_clk
> +  90   clk_d
> +  91   unassigned
> +  92   sus
> +  93   cdev1
> +  94   cdev2
> +  95   unassigned
> +
> +  96   uart2
> +  97   vfir
> +  98   spdif_in
> +  99   spdif_out
> +  100  vi
> +  101  vi_sensor
> +  102  tvo
> +  103  cve
>

These clocks follow on directly from the above numbers. What is your plan
for T30? I think this has another 64 clocks. Should we reserve those spaces
now so that the binding are compatible between the two?


> +  104  osc

+  105  clk_32k         a/k/a clk_s
> +  106  clk_m
> +  107  sclk
> +  108  cclk
> +  109  hclk
> +  110  pclk
> +  111  blink
> +  112  pll_a
> +  113  pll_a_out0
> +  114  pll_c
> +  115  pll_c_out1
> +  116  pll_d
> +  117  pll_d_out0
> +  118  pll_e
> +  119  pll_m
> +  120  pll_m_out1
> +  121  pll_p
> +  122  pll_p_out1
> +  123  pll_p_out2
> +  124  pll_p_out3
> +  125  pll_p_out4
> +  126  pll_s
> +  127  pll_u
> +  128  pll_x
> +  129  cop             a/k/a avp
> +  130  audio           a/k/a audio_sync_clk
> +
> +Example SoC include file:
> +
> +/ {
> +       tegra_car: clock@60006000 {
> +               compatible = "nvidia,tegra20-car";
> +               reg = <0x60006000 0x1000>;
> +               #clock-cells = <1>;
> +       };
> +
> +       usb@c5004000 {
> +               clocks = <&tegra_car 58>; /* usb2 */
> +       };
> +};
> +
> +Example board file:
> +
> +/ {
> +       clocks {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               osc: clock {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <12000000>;
> +               };
> +       };
> +
> +       i2c@7000d000 {
> +               pmic@34 {
> +                       compatible = "ti,tps6586x";
> +                       reg = <0x34>;
> +
> +                       clk_32k: clock {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <32768>;
> +                       };
> +               };
> +       };
> +
> +       &tegra_car {
> +               clocks = <&clk_32k> <&osc>;
> +       };
> +};
> diff --git a/arch/arm/boot/dts/tegra20.dtsi
> b/arch/arm/boot/dts/tegra20.dtsi
> index ec1f010..550da98 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -4,6 +4,12 @@
>        compatible = "nvidia,tegra20";
>        interrupt-parent = <&intc>;
>
> +       tegra_car: clock@60006000 {
> +               compatible = "nvidia,tegra20-car";
> +               reg = <0x60006000 1000>;
> +               #clock-cells = <1>;
> +       };
> +
>        pmc@7000f400 {
>                compatible = "nvidia,tegra20-pmc";
>                reg = <0x7000e400 0x400>;
> --
> 1.7.0.4
>

Regards,
Simon


>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #1.2: Type: text/html, Size: 11732 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
> > most clocks within Tegra20. The device tree binding models this as a
> > single monolithic clock provider, which exports ~130 clocks. This reduces
> > the number of nodes needed in device tree to represent these clocks.
> > 
> > This binding is only useful for Tegra20; the set of clocks that exists on
> > Tegra30 is sufficiently different to merit its own binding.
> 
> It seems there is a large common element - they are almost backwards
> compatible. Should we not at least look at this now?

I don't believe there's any need for the clock IDs for the two chips to
align in any way. These are two different chips, with different sets of
clocks, different tegra*.dtsi files, different clock "drivers" that define
the available clocks, etc.

And while as you say, there are a lot of similarities, there are also a
number of differences within these first 96 clocks which make having
things completely aligned impractical. I imagine you'd rather that
Tegra30's binding follow Tegra30's CLK_OUT_ENB register layouts exactly,
rather than attempting to align with Tegra20 even in the face of
differences in HW.

> > +  73   csite
> 
> Would 'coresight' be better given that you have csi also?

The clock is named "csite" in the TRM; renaming it would make it harder
to correlate the binding with the TRM.

> > +  76   la
> 
> What is this one?

It's in the kernel's tegra2_clocks.c. It's undocumented, but I've
confirmed that it does exist.

> +  96   uart2
> +  97   vfir
> +  98   spdif_in
> +  99   spdif_out
> +  100  vi
> +  101  vi_sensor
> +  102  tvo
> +  103  cve
> 
> These clocks follow on directly from the above numbers. What is your
> plan for T30? I think this has another 64 clocks. Should we reserve
> those spaces now so that the binding are compatible between the two?

For Tegra30, I imagine the first 160 clock IDs would be assigned in a
way that matches the CLK)OUT_ENB registers (since there are 160 bits in
them), and any other clocks would be assigned IDs starting with 160.

P.S. Your HTML mail was a little hard to quote.

-- 
nvpublic

Re: [PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Simon Glass]

Hi Stephen,

On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
>> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> > The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
>> > most clocks within Tegra20. The device tree binding models this as a
>> > single monolithic clock provider, which exports ~130 clocks. This reduces
>> > the number of nodes needed in device tree to represent these clocks.
>> >
>> > This binding is only useful for Tegra20; the set of clocks that exists on
>> > Tegra30 is sufficiently different to merit its own binding.
>>
>> It seems there is a large common element - they are almost backwards
>> compatible. Should we not at least look at this now?
>
> I don't believe there's any need for the clock IDs for the two chips to
> align in any way. These are two different chips, with different sets of
> clocks, different tegra*.dtsi files, different clock "drivers" that define
> the available clocks, etc.
>
> And while as you say, there are a lot of similarities, there are also a
> number of differences within these first 96 clocks which make having
> things completely aligned impractical. I imagine you'd rather that
> Tegra30's binding follow Tegra30's CLK_OUT_ENB register layouts exactly,
> rather than attempting to align with Tegra20 even in the face of
> differences in HW.

OK my question was really more about how you deal with multi-arch in
Linux / U-Boot. Does it make sense to ignore the commonality. Perhaps
instead the range from 96 to 160 should be reserved?

>
>> > +  73   csite
>>
>> Would 'coresight' be better given that you have csi also?
>
> The clock is named "csite" in the TRM; renaming it would make it harder
> to correlate the binding with the TRM.

OK

>
>> > +  76   la
>>
>> What is this one?
>
> It's in the kernel's tegra2_clocks.c. It's undocumented, but I've
> confirmed that it does exist.

OK

>
>> +  96   uart2
>> +  97   vfir
>> +  98   spdif_in
>> +  99   spdif_out
>> +  100  vi
>> +  101  vi_sensor
>> +  102  tvo
>> +  103  cve
>>
>> These clocks follow on directly from the above numbers. What is your
>> plan for T30? I think this has another 64 clocks. Should we reserve
>> those spaces now so that the binding are compatible between the two?
>
> For Tegra30, I imagine the first 160 clock IDs would be assigned in a
> way that matches the CLK)OUT_ENB registers (since there are 160 bits in
> them), and any other clocks would be assigned IDs starting with 160.
>
> P.S. Your HTML mail was a little hard to quote.

Sorry, I turned it on to send something odd, and forgot to turn it off.

Regards,
Simon

>
> --
> nvpublic
>

RE: [PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Simon Glass wrote at Wednesday, February 15, 2012 1:25 PM:
> On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
> >> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> > The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
> >> > most clocks within Tegra20. The device tree binding models this as a
> >> > single monolithic clock provider, which exports ~130 clocks. This reduces
> >> > the number of nodes needed in device tree to represent these clocks.
> >> >
> >> > This binding is only useful for Tegra20; the set of clocks that exists on
> >> > Tegra30 is sufficiently different to merit its own binding.
> >>
> >> It seems there is a large common element - they are almost backwards
> >> compatible. Should we not at least look at this now?
> >
> > I don't believe there's any need for the clock IDs for the two chips to
> > align in any way. These are two different chips, with different sets of
> > clocks, different tegra*.dtsi files, different clock "drivers" that define
> > the available clocks, etc.
> >
> > And while as you say, there are a lot of similarities, there are also a
> > number of differences within these first 96 clocks which make having
> > things completely aligned impractical. I imagine you'd rather that
> > Tegra30's binding follow Tegra30's CLK_OUT_ENB register layouts exactly,
> > rather than attempting to align with Tegra20 even in the face of
> > differences in HW.
> 
> OK my question was really more about how you deal with multi-arch in
> Linux / U-Boot. Does it make sense to ignore the commonality. Perhaps
> instead the range from 96 to 160 should be reserved?

I'm having a hard time seeing the problem here; the correct mapping from
device tree clock ID values to clock objects will be selected based on
the SoC version you're running on; there's no need to try and tie the
clock IDs for the two SoCs together, and there's always a way to know
which SoC version's numbering you should be dealing with at runtime.

-- 
nvpublic

Re: [PATCH V3] ARM: tegra: Define Tegra20 CAR binding

[Simon Glass]

Hi Stephen,

On Wed, Feb 15, 2012 at 1:30 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Simon Glass wrote at Wednesday, February 15, 2012 1:25 PM:
>> On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> > Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
>> >> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> >> > The Tegra20 CAR (Clock And Reset) Controller controls most aspects of
>> >> > most clocks within Tegra20. The device tree binding models this as a
>> >> > single monolithic clock provider, which exports ~130 clocks. This reduces
>> >> > the number of nodes needed in device tree to represent these clocks.
>> >> >
>> >> > This binding is only useful for Tegra20; the set of clocks that exists on
>> >> > Tegra30 is sufficiently different to merit its own binding.
>> >>
>> >> It seems there is a large common element - they are almost backwards
>> >> compatible. Should we not at least look at this now?
>> >
>> > I don't believe there's any need for the clock IDs for the two chips to
>> > align in any way. These are two different chips, with different sets of
>> > clocks, different tegra*.dtsi files, different clock "drivers" that define
>> > the available clocks, etc.
>> >
>> > And while as you say, there are a lot of similarities, there are also a
>> > number of differences within these first 96 clocks which make having
>> > things completely aligned impractical. I imagine you'd rather that
>> > Tegra30's binding follow Tegra30's CLK_OUT_ENB register layouts exactly,
>> > rather than attempting to align with Tegra20 even in the face of
>> > differences in HW.
>>
>> OK my question was really more about how you deal with multi-arch in
>> Linux / U-Boot. Does it make sense to ignore the commonality. Perhaps
>> instead the range from 96 to 160 should be reserved?
>
> I'm having a hard time seeing the problem here; the correct mapping from
> device tree clock ID values to clock objects will be selected based on
> the SoC version you're running on; there's no need to try and tie the
> clock IDs for the two SoCs together, and there's always a way to know
> which SoC version's numbering you should be dealing with at runtime.

At present in U-Boot we have exactly the same clock code for T20 and
T30, and the header file differences are in the reserved bits. I am
pointing this out in case it is of interest. But I think you are aware
of it, so please go ahead with the binding as you have it. So:

Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>


diff -u ./arch/arm/include/asm/arch-tegra2/clock-tables.h
./arch/arm/include/asm/arch-tegra3/clock-tables.h
 /* The clocks supported by the hardware */
 enum periph_id {
 	PERIPH_ID_FIRST,
 	PERIPH_ID_CPU = PERIPH_ID_FIRST,
-	PERIPH_ID_RESERVED1,
-	PERIPH_ID_RESERVED2,
-	PERIPH_ID_AC97,
-	PERIPH_ID_RTC,
+	PERIPH_ID_COP,
+	PERIPH_ID_TRIGSYS,
+	PERIPH_ID_RESERVED3,
+	PERIPH_ID_RESERVED4,
 	PERIPH_ID_TMR,
 	PERIPH_ID_UART1,
 	PERIPH_ID_UART2,
@@ -50,7 +53,7 @@
 	PERIPH_ID_SDMMC4,

 	/* 16 */
-	PERIPH_ID_TWC,
+	PERIPH_ID_RESERVED16,
 	PERIPH_ID_PWM,
 	PERIPH_ID_I2S2,
 	PERIPH_ID_EPP,
@@ -61,12 +64,12 @@

 	/* 24 */
 	PERIPH_ID_3D,
-	PERIPH_ID_IDE,
+	PERIPH_ID_RESERVED24,
 	PERIPH_ID_DISP2,
 	PERIPH_ID_DISP1,
 	PERIPH_ID_HOST1X,
 	PERIPH_ID_VCP,
-	PERIPH_ID_RESERVED30,
+	PERIPH_ID_I2S0,
 	PERIPH_ID_CACHE2,

 	/* Middle word: 63:32 */
@@ -83,9 +86,9 @@
 	PERIPH_ID_KFUSE,
 	PERIPH_ID_SBC1,
 	PERIPH_ID_SNOR,
-	PERIPH_ID_SPI1,
+	PERIPH_ID_RESERVED43,
 	PERIPH_ID_SBC2,
-	PERIPH_ID_XIO,
+	PERIPH_ID_RESERVED45,
 	PERIPH_ID_SBC3,
 	PERIPH_ID_DVC_I2C,

@@ -122,17 +125,17 @@
 	/* 72 */
 	PERIPH_ID_AFI,
 	PERIPH_ID_CORESIGHT,
-	PERIPH_ID_RESERVED74,
+	PERIPH_ID_PCIEXCLK,
 	PERIPH_ID_AVPUCQ,
 	PERIPH_ID_RESERVED76,
 	PERIPH_ID_RESERVED77,
 	PERIPH_ID_RESERVED78,
-	PERIPH_ID_RESERVED79,
+	PERIPH_ID_DTV,

 	/* 80 */
-	PERIPH_ID_RESERVED80,
-	PERIPH_ID_RESERVED81,
-	PERIPH_ID_RESERVED82,
+	PERIPH_ID_NANDSPEED,
+	PERIPH_ID_I2CSLOW,
+	PERIPH_ID_DSIB,
 	PERIPH_ID_RESERVED83,
 	PERIPH_ID_IRAMA,
 	PERIPH_ID_IRAMB,
@@ -141,7 +144,76 @@

 	/* 88 */
 	PERIPH_ID_CRAM2,
...extra T30 things

Regards,
Simon

>
> --
> nvpublic
>