[U-Boot] [PATCH] spi: Tegra: add device tree binding doc for SPI and QSPI
This patch adds the device tree binding doc for the Tegra114 SPI controller and the Tegra210 QSPI controller.
Signed-off-by: Tom Warren twarren@nvidia.com --- doc/device-tree-bindings/spi/spi-tegra.txt | 47 ++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 doc/device-tree-bindings/spi/spi-tegra.txt
diff --git a/doc/device-tree-bindings/spi/spi-tegra.txt b/doc/device-tree-bindings/spi/spi-tegra.txt new file mode 100644 index 0000000..e215efe --- /dev/null +++ b/doc/device-tree-bindings/spi/spi-tegra.txt @@ -0,0 +1,47 @@ +NVIDIA Tegra114 SPI controller. + +Required properties: +- compatible : should be "nvidia,tegra114-spi". +- reg: Should contain SPI registers location and length. +- interrupts: Should contain SPI interrupts. +- clocks : Should contain an entry for SPI clock. + +Recommended properties: +- spi-max-frequency: Definition as per + doc/device-tree-bindings/spi/spi-bus.txt +Example: + +spi@7000d600 { + compatible = "nvidia,tegra114-spi"; + reg = <0x7000d600 0x200>; + interrupts = <0 82 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 44>; + status = "disabled"; + spi-max-frequency = <25000000>; +}; + +NVIDIA Tegra210 QSPI controller. + +Required properties: +- compatible : should be "nvidia,tegra210-qspi". +- reg: Should contain QSPI registers location and length. +- interrupts: Should contain QSPI interrupts. +- clocks : Should contain an entry for QSPI clock. + +Recommended properties: +- spi-max-frequency: Definition as per + doc/device-tree-bindings/spi/spi-bus.txt +Example: + +spi@70410000 { + compatible = "nvidia,tegra210-qspi"; + reg = <0x0 0x70410000 0x0 0x1000>; + interrupts = <0 10 0x04>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&tegra_car 211>; + status = "disabled"; + spi-max-frequency = <24000000>; +};
On 10/23/2015 11:11 AM, Tom Warren wrote:
This patch adds the device tree binding doc for the Tegra114 SPI controller and the Tegra210 QSPI controller.
Initially, this should be sent as a Linux kernel patch, since the kernel currently holds the definitive repository for DT bindings.
The binding should be based on the Tegra SPI binding present there, not on the non-standard binding for Tegra114 SPI that's evidently in the U-Boot tree.
That would imply sending the patch to the people/lists listed in the following Linux kernel MAINTAINERS entry for DT bindings, plus at least the Tegra mailing list and maintainers too:
OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS M: Rob Herring robh+dt@kernel.org M: Pawel Moll pawel.moll@arm.com M: Mark Rutland mark.rutland@arm.com M: Ian Campbell ijc+devicetree@hellion.org.uk M: Kumar Gala galak@codeaurora.org L: devicetree@vger.kernel.org
diff --git a/doc/device-tree-bindings/spi/spi-tegra.txt b/doc/device-tree-bindings/spi/spi-tegra.txt new file mode 100644 index 0000000..e215efe --- /dev/null +++ b/doc/device-tree-bindings/spi/spi-tegra.txt @@ -0,0 +1,47 @@ +NVIDIA Tegra114 SPI controller.
Isn't this intended to be a binding for the QSPI controller?
+Required properties: +- compatible : should be "nvidia,tegra114-spi".
This should be "qspi" not "spi", assuming the HW really is different. This is a separate HW module, right?
+- reg: Should contain SPI registers location and length. +- interrupts: Should contain SPI interrupts. +- clocks : Should contain an entry for SPI clock.
Reset- and DMA-related properties are missing here.
You could mark the DMA properties optional, and leave it up to drivers to support PIO mode if the DMA properties are missing.
+Recommended properties: +- spi-max-frequency: Definition as per
doc/device-tree-bindings/spi/spi-bus.txt
That should use a relative path ("spi-bus.txt"), so that the same text applies irrespective of whether the file is contained within the Linux kernel or U-Boot source trees.
+NVIDIA Tegra210 QSPI controller.
The binding for Tegra114 and Tegra210 should be (and appears to be) identical. There's no need to duplicate the text. Instead, simply say something like the following for the compatible value in the one copy of the text:
- compatible : should be one of the following: "nvidia,tegra114-qspi" (for Tegra114) "nvidia,tegra210-qspi" (for Tegra210)
However, if a driver that supports Tegra114 could operate correctly on Tegra210 without knowledge that it was running on different HW, the following compatible values area appropriate:
- compatible : should be one of the following: "nvidia,tegra114-qspi" (for Tegra114) "nvidia,tegra210-qspi", "nvidia,tegra114-qspi" (for Tegra210)
Stephen,
-----Original Message----- From: Stephen Warren [mailto:swarren@wwwdotorg.org] Sent: Friday, October 23, 2015 10:26 AM To: Tom Warren TWarren@nvidia.com Cc: u-boot@lists.denx.de; jteki@openedev.com; Stephen Warren swarren@nvidia.com; tomcwarren3959@gmail.com Subject: Re: [U-Boot] [PATCH] spi: Tegra: add device tree binding doc for SPI and QSPI
On 10/23/2015 11:11 AM, Tom Warren wrote:
This patch adds the device tree binding doc for the Tegra114 SPI controller and the Tegra210 QSPI controller.
Initially, this should be sent as a Linux kernel patch, since the kernel currently holds the definitive repository for DT bindings.
The binding should be based on the Tegra SPI binding present there, not on the non-standard binding for Tegra114 SPI that's evidently in the U-Boot tree.
This is a copy of the 'nvidia,tegra114-spi.txt' binding in the kernel (Documentation/devicetree/bindings/spi/). I removed the dma and reset fields, since they aren't required (or used) in U-Boot. I then added QSPI for T210, and named the file spi-tegra.txt. There wasn't a U-Boot SPI binding doc in U-Boot to start with.
That would imply sending the patch to the people/lists listed in the following Linux kernel MAINTAINERS entry for DT bindings, plus at least the Tegra mailing list and maintainers too:
OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS M: Rob Herring robh+dt@kernel.org M: Pawel Moll pawel.moll@arm.com M: Mark Rutland mark.rutland@arm.com M: Ian Campbell ijc+devicetree@hellion.org.uk M: Kumar Gala galak@codeaurora.org L: devicetree@vger.kernel.org
Since this is basically a copy of a kernel binding doc, I didn't know that was necessary. Is that policy for U-Boot binding docs? Jagan - you have a few of these in the SPI bindings - did you have them reviewed by kernel/DT folk first?
Regardless, I'll resend with those people/lists in CC. Which Tegra ML/maintainers did you also want in there?
diff --git a/doc/device-tree-bindings/spi/spi-tegra.txt b/doc/device-tree-bindings/spi/spi-tegra.txt new file mode 100644 index 0000000..e215efe --- /dev/null +++ b/doc/device-tree-bindings/spi/spi-tegra.txt @@ -0,0 +1,47 @@ +NVIDIA Tegra114 SPI controller.
Isn't this intended to be a binding for the QSPI controller?
Since there was no SPI binding, I included the Tegra114 SPI binding here. Note that other QSPI bindings exist here, for instance spi-cadence.txt. If you'd like two separate binding docs, I can do that, but this seemed more efficient.
+Required properties: +- compatible : should be "nvidia,tegra114-spi".
This should be "qspi" not "spi", assuming the HW really is different. This is a separate HW module, right?
Again, this is for the SPI controller, not QSPI. QSPI binding follows @ line 25 below.
+- reg: Should contain SPI registers location and length. +- interrupts: Should contain SPI interrupts. +- clocks : Should contain an entry for SPI clock.
Reset- and DMA-related properties are missing here.
You could mark the DMA properties optional, and leave it up to drivers to support PIO mode if the DMA properties are missing.
Will do.
+Recommended properties: +- spi-max-frequency: Definition as per
doc/device-tree-bindings/spi/spi-bus.txtThat should use a relative path ("spi-bus.txt"), so that the same text applies irrespective of whether the file is contained within the Linux kernel or U-Boot source trees.
Linux binding uses an absolute path (Documentation/devicetree/bindings/spi/spi-bus.txt), so I copied their usage to point to our spi-bus.txt. I can change to a relative path.
+NVIDIA Tegra210 QSPI controller.
The binding for Tegra114 and Tegra210 should be (and appears to be) identical. There's no need to duplicate the text. Instead, simply say something like the following for the compatible value in the one copy of the text:
- compatible : should be one of the following: "nvidia,tegra114-qspi" (for Tegra114) "nvidia,tegra210-qspi" (for Tegra210)
Will do.
However, if a driver that supports Tegra114 could operate correctly on Tegra210 without knowledge that it was running on different HW, the following compatible values area appropriate:
- compatible : should be one of the following: "nvidia,tegra114-qspi" (for Tegra114) "nvidia,tegra210-qspi", "nvidia,tegra114-qspi" (for Tegra210)
Thanks,
Tom -- nvpublic
On 10/23/2015 11:52 AM, Tom Warren wrote:
Stephen,
Stephen Warren wrote at Friday, October 23, 2015 10:26 AM:
On 10/23/2015 11:11 AM, Tom Warren wrote:
This patch adds the device tree binding doc for the Tegra114 SPI controller and the Tegra210 QSPI controller.
Initially, this should be sent as a Linux kernel patch, since the kernel currently holds the definitive repository for DT bindings.
The binding should be based on the Tegra SPI binding present there, not on the non-standard binding for Tegra114 SPI that's evidently in the U-Boot tree.
This is a copy of the 'nvidia,tegra114-spi.txt' binding in the kernel (Documentation/devicetree/bindings/spi/). I removed the dma and reset fields, since they aren't required (or used) in U-Boot. I then added QSPI for T210, and named the file spi-tegra.txt. There wasn't a U-Boot SPI binding doc in U-Boot to start with.
Which kernel release did you use? The content in kernel git tag next-20151002, has been cleaned up quite a bit I suspect.
(Note that in that tag, the existing nvidia,tegra114-spi binding has one layout issue; I'd expect the clocks property to be documented immediately after the clock-names property. I'm not sure why it's not right now. This is just an FYI if you copy that file.)
Re "since they aren't required (or used) in U-Boot". A binding should be a description of the HW, and not influence by any particular OS or SW stack's design or implementation. If some feature exists in HW, it should be described in the binding, even if some particular SW stack isn't going to immediately make use of it. There's some leeway for optional features; if any arbitrary SW stack could make useful use of the HW without a particular feature, a simple binding could be submitted first without describing that feature, then enhanced later to add it. This avoids bike-shedding re: the design of some esoteric feature from blocking a simple binding being accepted. However, basic features that are simple, common, well-understood, and widely used (e.g. resets, DMA), should invariably be described right from the start.
That would imply sending the patch to the people/lists listed in the following Linux kernel MAINTAINERS entry for DT bindings, plus at least the Tegra mailing list and maintainers too:
OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS M: Rob Herring robh+dt@kernel.org M: Pawel Moll pawel.moll@arm.com M: Mark Rutland mark.rutland@arm.com M: Ian Campbell ijc+devicetree@hellion.org.uk M: Kumar Gala galak@codeaurora.org L: devicetree@vger.kernel.org
Since this is basically a copy of a kernel binding doc, I didn't know that was necessary. Is that policy for U-Boot binding docs?
All bindings need to be reviewed by the DT binding review process. That currently involves submitting a Linux kernel patch to add the documentation of the binding to the kernel source tree. There have been discussions about moving the DT binding docs into a separate standalone repo with a seprate review process, but that hasn't happened yet. This is all policy for DT, so nothing to do with U-Boot or Linux or other SW.
Jagan - you have a few of these in the SPI bindings - did you have them reviewed by kernel/DT folk first?
I don't know about those specifically, but historical usage of DT in U-Boot has been extremely lax about standards, and has tended to diverge from the official set of bindings, currently stored in the kernel git tree.
Regardless, I'll resend with those people/lists in CC. Which Tegra ML/maintainers did you also want in there?
Generally, Linux kernel patches should be sent to whatever ./scripts/get_maintainers.pl says, although in some cases you'd want to strip out some irrelevant people/lists so as not to make your CC too large. Here's the entry for Tegra:
TEGRA ARCHITECTURE SUPPORT M: Stephen Warren swarren@wwwdotorg.org M: Thierry Reding thierry.reding@gmail.com M: Alexandre Courbot gnurou@gmail.com L: linux-tegra@vger.kernel.org
You should probably add in the ARM linux kernel mailing list but not bother with the main Linux kernel mailing list or general documentation people (the DT-binding-specific reviewers should be enough for this).
diff --git a/doc/device-tree-bindings/spi/spi-tegra.txt b/doc/device-tree-bindings/spi/spi-tegra.txt new file mode 100644 index 0000000..e215efe --- /dev/null +++ b/doc/device-tree-bindings/spi/spi-tegra.txt @@ -0,0 +1,47 @@ +NVIDIA Tegra114 SPI controller.
Isn't this intended to be a binding for the QSPI controller?
Since there was no SPI binding, I included the Tegra114 SPI binding here. Note that other QSPI bindings exist here, for instance spi-cadence.txt.If you'd like two separate binding docs, I can do that, but this seemed more efficient.
Any DT binding docs that get imported into the U-Boot source tree should be exactly identical to the official docs that are currently located in the kernel git tree. This means all of content, directory layout, and filenames.
For the existing SPI bindings, you could just copy the existing file from the kernel tree to the U-Boot tree. However, where the kernel and U-Boot use identical bindings (which should be everywhere but currently isn't), I don't see the point of duplicating the doc in the U-Boot tree at all, since it just leaves two separate copies that likely won't be kept in sync.
Generally, there is a separate DT binding doc per type of incompatible HW module. So, if the Tegra SPI and QSPI HW modules need to be programmed differently, there would be two bindings. However, if the programming for e.g. Tegra114 and Tegra124 (regular) SPI HW modules are identical or backwards-compatible or a superset, then those two HW modules can share a binding definition, e.g. with a note in the compatible value definition describing a different value for each version of that HW module.
+Recommended properties: +- spi-max-frequency: Definition as per
doc/device-tree-bindings/spi/spi-bus.txtThat should use a relative path ("spi-bus.txt"), so that the same text applies irrespective of whether the file is contained within the Linux kernel or U-Boot source trees.
Linux binding uses an absolute path (Documentation/devicetree/bindings/spi/spi-bus.txt), so I copied their usage to point to our spi-bus.txt. I can change to
a relative path.
Unfortunately, there are plenty of bad examples that were written before we realized some things didn't work well:-( The more bindings get fixed when first submitted, the lower the chance of randomly picking the wrong example:-)
Sorry there's a lot of tribal knowledge re: DT bindings. The first is the hardest; the next hopefully much simpler.
participants (3)
-
Stephen Warren -
Tom Warren -
Tom Warren