[PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Paul Walmsley]

Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.

Signed-off-by: Paul Walmsley <pwalmsley-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Matthew Longnecker <mlongnecker-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 .../bindings/clock/nvidia,tegra114-dfll.txt        |   43 ++++++++++++++++++++
 arch/arm/boot/dts/tegra114.dtsi                    |   10 +++++
 2 files changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt
new file mode 100644
index 000000000000..b868bf97bc3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt
@@ -0,0 +1,43 @@
+NVIDIA Tegra114 DFLL FCPU clocksource data in the SoC DTS file:
+
+Required properties:
+
+- compatible : "nvidia,tegra114-dfll-fcpu"
+
+- reg : Must contain the starting physical address and length for the
+        DFLL's MMIO register space, including the DFLL-to-I2C
+        controller interface and the DFLL's I2C controller.
+
+- clocks : Must contain an array of two-cell arrays, one per clock.
+           DFLL source clocks.  At minimum this should include the
+           reference clock source and the IP block's main clock
+           source.  Also it should contain the DFLL's I2C controller
+           clock source.  The format is <&clock-provider-phandle
+           clock-id>.
+
+- clock-names : Must contain an array of strings, one per 'clocks'
+                two-cell array.  The position in the array of these
+                strings must correspond to the position in the 'clocks'
+                array (see above).  The DFLL driver currently requires
+                the "soc", "ref", and "i2c" clock names to be populated.
+
+
+Optional properties:
+
+- status : device availability -- managed by the DT integration code, not
+           the DFLL driver.  Should be set to "disabled" in the SoC
+           DTS file.
+
+
+Example:
+
+dfll@70110000 {
+        compatible = "nvidia,tegra114-dfll-fcpu";
+        reg = <0x70110000 0x400>;
+        clocks = <&tegra_car TEGRA114_CLK_DFLL_SOC>,
+                 <&tegra_car TEGRA114_CLK_DFLL_REF>,
+                 <&tegra_car TEGRA114_CLK_I2C5>;
+        clock-names = "soc", "ref", "i2c";
+        status = "disabled";
+};
+
diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index ae855ec60bbd..1cd59d79e67c 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -480,6 +480,16 @@
 		};
 	};
 
+	dfll@70110000 {
+		compatible = "nvidia,tegra114-dfll-fcpu";
+		reg = <0x70110000 0x400>;
+		clocks = <&tegra_car TEGRA114_CLK_DFLL_SOC>,
+			 <&tegra_car TEGRA114_CLK_DFLL_REF>,
+			 <&tegra_car TEGRA114_CLK_I2C5>;
+		clock-names = "soc", "ref", "i2c";
+		status = "disabled";
+	};
+
 	sdhci@78000000 {
 		compatible = "nvidia,tegra114-sdhci", "nvidia,tegra30-sdhci";
 		reg = <0x78000000 0x200>;

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

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Stephen Warren]

On 12/19/2013 05:49 AM, Paul Walmsley wrote:
> Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.

> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt

> +- clocks : Must contain an array of two-cell arrays, one per clock.
> +           DFLL source clocks.  At minimum this should include the
> +           reference clock source and the IP block's main clock
> +           source.  Also it should contain the DFLL's I2C controller
> +           clock source.  The format is <&clock-provider-phandle
> +           clock-id>.

Entries in "clocks" aren't two cells, they're a phandle plus as many
cells as the node referenced by the phandle specifies.

> +
> +- clock-names : Must contain an array of strings, one per 'clocks'
> +                two-cell array.  The position in the array of these

clock-names defines the set of entries in clocks, not the other way around.

> +                strings must correspond to the position in the 'clocks'
> +                array (see above).  The DFLL driver currently requires
> +                the "soc", "ref", and "i2c" clock names to be populated.

The standard wording used by all the Tegra clock client bindings is now:

- clocks : Must contain an entry for each entry in clock-names.
  See clock-bindings.txt for details.
- clock-names : Must include the following entries:
  - soc
  - ref
  - i2c

For consistency, it'd be nice to adopt the same style here.

> +Optional properties:
> +
> +- status : device availability -- managed by the DT integration code, not
> +           the DFLL driver.  Should be set to "disabled" in the SoC
> +           DTS file.

That's such a core property that it's not worth documenting in every
single binding.

> +

Blank line at EOF.

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Paul Walmsley]

(Resending, and revising - the original wasn't fully baked in several 
regards..)

Hello Stephen,

thanks for your review.

On Thu, 19 Dec 2013, Stephen Warren wrote:

> On 12/19/2013 05:49 AM, Paul Walmsley wrote:
>> Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.
>
>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt
>
>> +- clocks : Must contain an array of two-cell arrays, one per clock.
>> +           DFLL source clocks.  At minimum this should include the
>> +           reference clock source and the IP block's main clock
>> +           source.  Also it should contain the DFLL's I2C controller
>> +           clock source.  The format is <&clock-provider-phandle
>> +           clock-id>.
>
> Entries in "clocks" aren't two cells, they're a phandle plus as many
> cells as the node referenced by the phandle specifies.

It's worth noting that the clock binding documentation itself refers to 
pairs:

----

clocks:		List of phandle and clock specifier pairs, one pair
 		for each clock input to the device.  Note: if the
 		clock provider specifies '0' for #clock-cells, then
 		only the phandle portion of the pair will appear.

----

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/clock-bindings.txt#n50

But given the ambiguity of that documentation, I basically agree, so have 
changed it to:

- clocks : Must contain an array of clock specifiers, one per clock.
            DFLL source clocks.  At minimum this should include the
            reference clock source and the IP block's main clock
            source.  Also it should contain the DFLL's I2C controller
            clock source.  The format is <&clock-provider-phandle
            clock-id>.

>> +
>> +- clock-names : Must contain an array of strings, one per 'clocks'
>> +                two-cell array.  The position in the array of these
>
> clock-names defines the set of entries in clocks, not the other way around.

Hmm.  Referring to the DT clock binding documentation, it lists the 
'clock-names' property as optional, and the 'clocks' property as 
mandatory:

-----

==Clock consumers==

Required properties:
clocks:		List of phandle and clock specifier pairs, one pair
 		for each clock input to the device.  Note: if the
 		clock provider specifies '0' for #clock-cells, then
 		only the phandle portion of the pair will appear.

Optional properties:
clock-names:	List of clock input name strings sorted in the same
 		order as the clocks property.  Consumers drivers
 		will use clock-names to match clock input names
 		with clocks specifiers.

-----

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/clock-bindings.txt#n47

Considering that of_clk_get() doesn't require names, the situation appears 
to be the way that the original patch described it (modulo the part about 
"two cells.")

>> +                strings must correspond to the position in the 'clocks'
>> +                array (see above).  The DFLL driver currently requires
>> +                the "soc", "ref", and "i2c" clock names to be populated.
>
> The standard wording used by all the Tegra clock client bindings is now:
>
> - clocks : Must contain an entry for each entry in clock-names.
>  See clock-bindings.txt for details.
> - clock-names : Must include the following entries:
>  - soc
>  - ref
>  - i2c
>
> For consistency, it'd be nice to adopt the same style here.

I've altered the 'clock-names' format along the lines of what you've 
suggested, but have not changed the 'clocks' format, per the above 
discussion:

-----

- clocks : Must contain an array of clock specifiers, one per clock.
            DFLL source clocks.  At minimum this should include the
            reference clock source and the IP block's main clock
            source.  Also it should contain the DFLL's I2C controller
            clock source.  The format is <&clock-provider-phandle
            clock-id>.

- clock-names : Must contain an array of strings, one per 'clocks'
                 cell.  The position in the array of these strings must
                 correspond to the position in the 'clocks' array (see
                 above).
   - soc
   - ref
   - i2c

-----


>> +Optional properties:
>> +
>> +- status : device availability -- managed by the DT integration code, not
>> +           the DFLL driver.  Should be set to "disabled" in the SoC
>> +           DTS file.
>
> That's such a core property that it's not worth documenting in every
> single binding.

That's fine.  Removed.

>
>> +
>
> Blank line at EOF.


Hehe.  Removed.

- Paul

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Paul Walmsley]

On 01/13/2014 10:27 PM, Paul Walmsley wrote:
> The format is <&clock-provider-phandle clock-id>.

Ah, one other change: the above string has been dropped.  Pesky examples ;-)

- Paul

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Gerhard Sittig]

On Mon, Jan 13, 2014 at 22:27 -0800, Paul Walmsley wrote:
> 
> On Thu, 19 Dec 2013, Stephen Warren wrote:
> 
> >On 12/19/2013 05:49 AM, Paul Walmsley wrote:
> >>Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.
> >
> >>diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt
> >
> >>+- clocks : Must contain an array of two-cell arrays, one per clock.
> >>+           DFLL source clocks.  At minimum this should include the
> >>+           reference clock source and the IP block's main clock
> >>+           source.  Also it should contain the DFLL's I2C controller
> >>+           clock source.  The format is <&clock-provider-phandle
> >>+           clock-id>.
> >
> >Entries in "clocks" aren't two cells, they're a phandle plus as many
> >cells as the node referenced by the phandle specifies.
> 
> It's worth noting that the clock binding documentation itself refers
> to pairs:
> 
> ----
> 
> clocks:		List of phandle and clock specifier pairs, one pair
> 		for each clock input to the device.  Note: if the
> 		clock provider specifies '0' for #clock-cells, then
> 		only the phandle portion of the pair will appear.
> 
> ----
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/clock-bindings.txt#n50
> 
> But given the ambiguity of that documentation, I basically agree, so
> have changed it to:

Please note that there neither is an ambiguity nor a conflict
here, and that you actually acknowledge what Stephen said:

Clocks get referenced by a pair which consists of a phandle for
the clock provider and a clock specifier.  The clock specifier is
made of as many cells as the clock provider's #clock-cells
property defines (including none, or any other number than 1).

This is exactly what Stephen said:  A "clocks" item does not need
to have two cells.  The pair of phandle and clock specifier don't
necessarily translate into two cells, instead the number of cells
depends on the clock provider.

Also note that the phandle is specifically _not_ part of the
clock specifier.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Paul Walmsley]

On 01/15/2014 11:50 AM, Gerhard Sittig wrote:
> On Mon, Jan 13, 2014 at 22:27 -0800, Paul Walmsley wrote:
>> On Thu, 19 Dec 2013, Stephen Warren wrote:
>>
>>> On 12/19/2013 05:49 AM, Paul Walmsley wrote:
>>>> Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.
>>>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt
>>>> +- clocks : Must contain an array of two-cell arrays, one per clock.
>>>> +           DFLL source clocks.  At minimum this should include the
>>>> +           reference clock source and the IP block's main clock
>>>> +           source.  Also it should contain the DFLL's I2C controller
>>>> +           clock source.  The format is <&clock-provider-phandle
>>>> +           clock-id>.
>>> Entries in "clocks" aren't two cells, they're a phandle plus as many
>>> cells as the node referenced by the phandle specifies.
>> It's worth noting that the clock binding documentation itself refers
>> to pairs:
>>
>> ----
>>
>> clocks:		List of phandle and clock specifier pairs, one pair
>> 		for each clock input to the device.  Note: if the
>> 		clock provider specifies '0' for #clock-cells, then
>> 		only the phandle portion of the pair will appear.
>>
>> ----
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/clock-bindings.txt#n50
>>
>> But given the ambiguity of that documentation, I basically agree, so
>> have changed it to:
> Please note that there neither is an ambiguity nor a conflict
> here, and that you actually acknowledge what Stephen said:

I do not agree that the Documentation is unambiguous.

It is not correct to refer to a "pair" without a second item as a "pair."

> This is exactly what Stephen said:  A "clocks" item does not need
> to have two cells.  The pair of phandle and clock specifier don't
> necessarily translate into two cells, instead the number of cells
> depends on the clock provider.

I do agree with this, and have updated the documentation accordingly.


- Paul

Re: [PATCH 4/6] ARM: DTS: tegra: add the DFLL IP block to the T114 SoC file

[Stephen Warren]

On 01/13/2014 11:03 PM, Paul Walmsley wrote:
> On 12/19/2013 04:05 PM, Stephen Warren wrote:
>> On 12/19/2013 05:49 AM, Paul Walmsley wrote:
>>> Add basic DT bindings for the DFLL IP block for the NVIDIA Tegra114 SoC.
>>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra114-dfll.txt

>>> +- clock-names : Must contain an array of strings, one per 'clocks'
>>> +                two-cell array.  The position in the array of these
>> clock-names defines the set of entries in clocks, not the other way around.
> 
> Hmm.  If one refers to the DT clock binding documentation, it lists the
> 'clock-names' property as optional, and the 'clocks' property as mandatory:
> 
> -----
> 
> |==Clock consumers==
> 
> Required properties:
> clocks:		List of phandle and clock specifier pairs, one pair
> 		for each clock input to the device.  Note: if the
> 		clock provider specifies '0' for #clock-cells, then
> 		only the phandle portion of the pair will appear.
> 
> Optional properties:
> clock-names:	List of clock input name strings sorted in the same
> 		order as the clocks property.  Consumers drivers
> 		will use clock-names to match clock input names
> 		with clocks specifiers.
> 
> -----
> |
> 
> 
>  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/clock-bindings.txt#n47
> 
> Considering that of_clk_get() doesn't require names, it certainly
> appears to be the way that the original patch described it (modulo the
> bit about 'cells'; which has been fixed.)

If the binding defines that clock must exist at certain indices in the
clock property, and clock-names isn't used, then the clocks property
defines which clocks must exist.

However, if the binding is defining clock names, then the clock-names
property is the master that defines which clocks exist. In this case,
the index of entries in the clocks property is undefined by the binding;
it must simply match the clock-names property, which can be arbitrarily
ordered by the user.

Since this is a new binding, we should use the clock-names style to
ensure easy future backwards-compatible changes.