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