Stephen,
> -----Original Message-----
> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
> Sent: Friday, October 23, 2015 10:26 AM
> To: Tom Warren <twar...@nvidia.com>
> Cc: u-boot@lists.denx.de; jt...@openedev.com; Stephen Warren
> <swar...@nvidia.com>; tomcwarren3...@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...@kernel.org>
> M: Pawel Moll <pawel.m...@arm.com>
> M: Mark Rutland <mark.rutl...@arm.com>
> M: Ian Campbell <ijc+devicet...@hellion.org.uk>
> M: Kumar Gala <ga...@codeaurora.org>
> L: devicet...@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.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.
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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
