Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
From: Paul Walmsley <paul.walmsley@sifive.com>
To: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Paul Walmsley <paul@pwsan.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Palmer Dabbelt <palmer@sifive.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/2] dt-bindings: serial: add documentation for the SiFive UART driver
Date: Fri, 16 Nov 2018 15:10:35 -0800 (PST)
Message-ID: <alpine.DEB.2.21.9999.1811161505240.25712@viisi.sifive.com> (raw)
Message-ID: <20181116231035.CYT-EKSUVZQBK3fsjAsdDdM5Z5niVKak-9h4jSvTeQc@z> (raw)
In-Reply-To: <20181024173232.GB5652@bogus>

[-- Attachment #1: Type: text/plain, Size: 5470 bytes --]

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@sifive.com> wrote:
>>>> On 10/19/18 1:45 PM, Rob Herring wrote:
>>>>> On Fri, Oct 19, 2018 at 1:48 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>>>>>> Add DT binding documentation for the Linux driver for the SiFive
>>>>>> asynchronous serial IP block.  Nothing too exotic.
>>>>>>
>>>>>> Cc: linux-serial@vger.kernel.org
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> Cc: linux-riscv@lists.infradead.org
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>>> Cc: Palmer Dabbelt <palmer@sifive.com>
>>>>>> Reviewed-by: Palmer Dabbelt <palmer@sifive.com>
>>>>>> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
>>>>>> Signed-off-by: Paul Walmsley <paul@pwsan.com>
>>>>>> ---
>>>>>>    .../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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  parent reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19 18:48 [PATCH v2 0/2] tty: serial: add DT bindings and serial driver for the SiFive FU540 UART paul.walmsley
2018-10-19 18:48 ` Paul Walmsley
2018-10-19 18:48 ` [PATCH v2 1/2] dt-bindings: serial: add documentation for the SiFive UART driver paul.walmsley
2018-10-19 18:48   ` Paul Walmsley
2018-10-19 20:45   ` robh+dt
2018-10-19 20:45     ` Rob Herring
2018-10-19 22:05     ` paul.walmsley
2018-10-19 22:05       ` Paul Walmsley
2018-10-20 14:21       ` robh+dt
2018-10-20 14:21         ` Rob Herring
2018-10-23 17:05         ` paul.walmsley
2018-10-23 17:05           ` Paul Walmsley
2018-10-24 17:32           ` robh
2018-10-24 17:32             ` Rob Herring
2018-11-16 23:10             ` paul.walmsley [this message]
2018-11-16 23:10               ` Paul Walmsley
2018-10-22 16:41     ` palmer
2018-10-22 16:41       ` Palmer Dabbelt
2018-10-24 16:53       ` robh
2018-10-24 16:53         ` Rob Herring
2018-10-19 18:48 ` [PATCH v2 2/2] tty: serial: add driver for the SiFive UART paul.walmsley
2018-10-19 18:48   ` Paul Walmsley
2018-10-20  2:47   ` lkp
2018-10-20  2:47     ` kbuild test robot

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.9999.1811161505240.25712@viisi.sifive.com \
    --to=paul.walmsley@sifive.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=palmer@sifive.com \
    --cc=paul@pwsan.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox