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...@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?
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 <swar...@wwwdotorg.org>
M: Thierry Reding <thierry.red...@gmail.com>
M: Alexandre Courbot <gnu...@gmail.com>
L: linux-te...@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.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.
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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
