[V3] ARM: tegra: Define Tegra20 CAR binding
diff mbox

Message ID 1329329667-26725-1-git-send-email-swarren@nvidia.com
State Superseded, archived
Headers show

Commit Message

Stephen Warren Feb. 15, 2012, 6:14 p.m. UTC
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@nvidia.com>
---
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

Comments

Stephen Warren Feb. 15, 2012, 7:14 p.m. UTC | #1
Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren@nvidia.com> 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.
Simon Glass Feb. 15, 2012, 8:25 p.m. UTC | #2
Hi Stephen,

On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
>> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren@nvidia.com> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Feb. 15, 2012, 9:30 p.m. UTC | #3
Simon Glass wrote at Wednesday, February 15, 2012 1:25 PM:
> On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
> >> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren@nvidia.com> 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.
Simon Glass Feb. 15, 2012, 9:42 p.m. UTC | #4
Hi Stephen,

On Wed, Feb 15, 2012 at 1:30 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Wednesday, February 15, 2012 1:25 PM:
>> On Wed, Feb 15, 2012 at 11:14 AM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Simon Glass wrote at Wednesday, February 15, 2012 11:45 AM:
>> >> On Wed, Feb 15, 2012 at 10:14 AM, Stephen Warren <swarren@nvidia.com> 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@chromium.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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

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>;