(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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
