Re: [PATCH v2 1/2] dt-bindings: serial: add documentation for the SiFive UART driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 24 Oct 2018, Rob Herring wrote:

On Tue, Oct 23, 2018 at 10:05:40AM -0700, Paul Walmsley wrote:
On 10/20/18 7:21 AM, Rob Herring wrote:
On Fri, Oct 19, 2018 at 5:06 PM Paul Walmsley <paul.walmsley@xxxxxxxxxx> wrote:
On 10/19/18 1:45 PM, Rob Herring wrote:
On Fri, Oct 19, 2018 at 1:48 PM Paul Walmsley <paul.walmsley@xxxxxxxxxx> wrote:
Add DT binding documentation for the Linux driver for the SiFive
asynchronous serial IP block.  Nothing too exotic.

Cc: linux-serial@xxxxxxxxxxxxxxx
Cc: devicetree@xxxxxxxxxxxxxxx
Cc: linux-riscv@xxxxxxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Rob Herring <robh+dt@xxxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Palmer Dabbelt <palmer@xxxxxxxxxx>
Reviewed-by: Palmer Dabbelt <palmer@xxxxxxxxxx>
Signed-off-by: Paul Walmsley <paul.walmsley@xxxxxxxxxx>
Signed-off-by: Paul Walmsley <paul@xxxxxxxxx>
---
   .../bindings/serial/sifive-serial.txt         | 21 +++++++++++++++++++
   1 file changed, 21 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/serial/sifive-serial.txt

diff --git a/Documentation/devicetree/bindings/serial/sifive-serial.txt b/Documentation/devicetree/bindings/serial/sifive-serial.txt
new file mode 100644
index 000000000000..8982338512f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/sifive-serial.txt
@@ -0,0 +1,21 @@
+SiFive asynchronous serial interface (UART)
+
+Required properties:
+
+- compatible: should be "sifive,fu540-c000-uart0" or "sifive,uart0"

As I mentioned for the
intc and now the pwm block bindings, if you are going to do version
numbers please document the versioning scheme.


Will add that to the binding document.
I don't seem to be making my point clear. I don't want any of this
added to a binding doc for particular IP blocks. Write a common doc
that explains the scheme and addresses the questions I asked. Then
just reference that doc here.

Maybe this is documented somewhere already? Otherwise, if one is
creating a new IP block, how do they know what the versioning scheme
is or what goes in the DT ROM?


Seems like there might be some confusion between IP blocks as integrated on
an SoC vs. IP blocks in isolation.  It's not necessarily the SoC integrator
that sets an IP block version number; this can come from the IP block vendor
itself.  So each IP block may have its own version numbering practices for
the IP block alone.


For SiFive IP blocks, we at SiFive could probably align on a common version
numbering structure for what's in the sifive-blocks repository.

I thought you had that from what Palmer said and what I've seen so far.
You have at least 3 bindings so far it seems.

Yep.

But other IP blocks from other vendors may not align to that, or may not
have version numbers exposed at all.  In those cases there's no way for
software folks to find out what they are,  as you pointed out earlier.  This
is the case with most DT compatible strings in the kernel tree.

For example, we've integrated the NVDLA IP block, from NVIDIA, on some
designs.  Any NVIDIA version numbers in that IP block will probably not
follow the SiFive version numbering scheme.  I'd propose the right thing to
do for an IP block compatible string is to follow the vendor's practice, and
then use the SoC integrator's version numbering practice for the
SoC-integrated compatible string.

Experience has shown that using compatible strings only specific to
vendor IP blocks (with or without version numbers) is pretty useless.

For licensed IP, I'd suggest you follow standard practices.

OK

A genericish fallback is generally only used when there's lots of SoCs sharing a block.

In these cases though it needs to be clear what bindings follow some
common versioning scheme and which don't. That's accomplished
by referencing what the version scheme is. Otherwise, I'd expect I'll
see the versioning scheme copied when in fact the source IP in no way
follows it.

In effect, an SoC integration DT compatible string like
"sifive,fu540-c000-uart" implicitly states an IP block version number:
"whatever came out of the fab on the chip"[**].   I'd propose that even in
these cases, there's an advantage to keeping the "0" on the end, since it
uniquely identifies an SoC-independent IP block, rather than just the type
of the IP block.   But if the "0" on the end of the SoC integration DT
compatible string is problematic for you, we can certainly drop that last 0
from the SoC integration DT compatible string, and only suffer a slight lack
of clarity as to what version was integrated on that chip.

Personally I'd leave it off, but I'm fine with either way. It just needs
to be the way you document for SiFive IP blocks.

OK, we'll leave it off.

Really, I'd just like to see folks get better at putting version and
configuration information into registers. We only need DT for what we
can't discover.

Yep, agreed.


- Paul

[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux