Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
WARNING: multiple messages refer to this Message-ID
From: paul.walmsley@sifive.com (Paul Walmsley)
To: linux-riscv@lists.infradead.org
Subject: [PATCH v2 1/2] dt-bindings: serial: add documentation for the SiFive UART driver
Date: Fri, 19 Oct 2018 15:05:11 -0700
Message-ID: <4317548d-f831-29ba-3be9-35f080587db9@sifive.com> (raw)
In-Reply-To: <CAL_JsqJBSqaxnETdJg1Mdp+H2ay+WKT0JaxLe6D4Fwo_99qUkw@mail.gmail.com>


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 at vger.kernel.org
>> Cc: devicetree at vger.kernel.org
>> Cc: linux-riscv at lists.infradead.org
>> Cc: linux-kernel at 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"
> I assume once again, the last '0' is a version?


Yes.


> Palmer mentioned the
> compatible string is part of the IP block repository?


It is, but there's no guarantee that the compatible string from the RTL 
will make it into a ROM for any given chip.? For example, a customer may 
want the UART, but not want to take the DT ROM to keep area down.


This is one of the reasons why we'll likely switch to the usual 
software-maintained DTS files for Linux, just like the rest of arch/arm, 
arch/powerpc, etc.


> 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.


>   Where does the
> number come from?


It comes from the RTL, which is public:

https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43



> What's the next version?


1 (or something larger)


> Major vs. minor versions?


Not currently for this IP block.


> ECO fixes?


ECOs for a specific chip?? If so, whether an integrator changes the 
version number in a ROM (if present) is up to whomever does the ECO.? 
That may not be SiFive.


Suppose that someone ECOs a SiFive UART in a way that incompatibly 
changes the programming model.? They can choose to submit corresponding 
RTL changes back upstream to the sifive-blocks repository, or not.


If they don't, and they want upstream Linux support, it's up to the 
integrator to define a "foobar,foochip-uart" in their chip DT file, and 
post it upstream to the kernel lists, along with the corresponding 
driver patches.


If however, they do get their changes accepted into the sifive-blocks 
public RTL repository, then the maintainer of sifive-blocks is 
responsible for ensuring that the compatible string in the RTL is 
changed in an appropriate way.


>   Is the version s/w readable?


Not in the UART IP block itself.?? In the specific case of the FU540 
chip, there's a string in a ROM.


> How do you ensure it gets
> updated?


The string in the ROM?? For an IP block like the UART, it's up to the 
engineer patching the UART RTL to update the compatible string when the 
programming model changes, and the sifive-blocks maintainer to enforce it.

For a given chip, it's up to the integrator/end user whether they want 
to include the DT ROM or not, and if it's present, it's up to them what 
it contains.


> All that should be addressed.
>
> Otherwise, don't do version numbers because we have no visibility to
> what they mean.


It's all in the public RTL:

https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43


>> +- reg: address and length of the register space
>> +- interrupt-parent: should contain a phandle pointing to the SoC interrupt
>> +    controller device node that the UART interrupts are connected to
> Don't need to document interrupt-parent here.


OK, will drop it.


- Paul

From: Paul Walmsley <paul.walmsley@sifive.com>
To: Rob Herring <robh+dt@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>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/2] dt-bindings: serial: add documentation for the SiFive UART driver
Date: Fri, 19 Oct 2018 15:05:11 -0700
Message-ID: <4317548d-f831-29ba-3be9-35f080587db9@sifive.com> (raw)
Message-ID: <20181019220511.qM6cgr3ODfZvU3nRSsfPUZg4IV1FWyhF3nq8oxCdTCQ@z> (raw)
In-Reply-To: <CAL_JsqJBSqaxnETdJg1Mdp+H2ay+WKT0JaxLe6D4Fwo_99qUkw@mail.gmail.com>


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"
> I assume once again, the last '0' is a version?


Yes.


> Palmer mentioned the
> compatible string is part of the IP block repository?


It is, but there's no guarantee that the compatible string from the RTL 
will make it into a ROM for any given chip.  For example, a customer may 
want the UART, but not want to take the DT ROM to keep area down.


This is one of the reasons why we'll likely switch to the usual 
software-maintained DTS files for Linux, just like the rest of arch/arm, 
arch/powerpc, etc.


> 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.


>   Where does the
> number come from?


It comes from the RTL, which is public:

https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43



> What's the next version?


1 (or something larger)


> Major vs. minor versions?


Not currently for this IP block.


> ECO fixes?


ECOs for a specific chip?  If so, whether an integrator changes the 
version number in a ROM (if present) is up to whomever does the ECO.  
That may not be SiFive.


Suppose that someone ECOs a SiFive UART in a way that incompatibly 
changes the programming model.  They can choose to submit corresponding 
RTL changes back upstream to the sifive-blocks repository, or not.


If they don't, and they want upstream Linux support, it's up to the 
integrator to define a "foobar,foochip-uart" in their chip DT file, and 
post it upstream to the kernel lists, along with the corresponding 
driver patches.


If however, they do get their changes accepted into the sifive-blocks 
public RTL repository, then the maintainer of sifive-blocks is 
responsible for ensuring that the compatible string in the RTL is 
changed in an appropriate way.


>   Is the version s/w readable?


Not in the UART IP block itself.   In the specific case of the FU540 
chip, there's a string in a ROM.


> How do you ensure it gets
> updated?


The string in the ROM?  For an IP block like the UART, it's up to the 
engineer patching the UART RTL to update the compatible string when the 
programming model changes, and the sifive-blocks maintainer to enforce it.

For a given chip, it's up to the integrator/end user whether they want 
to include the DT ROM or not, and if it's present, it's up to them what 
it contains.


> All that should be addressed.
>
> Otherwise, don't do version numbers because we have no visibility to
> what they mean.


It's all in the public RTL:

https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43


>> +- reg: address and length of the register space
>> +- interrupt-parent: should contain a phandle pointing to the SoC interrupt
>> +    controller device node that the UART interrupts are connected to
> Don't need to document interrupt-parent here.


OK, will drop it.


- Paul


_______________________________________________
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 [this message]
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
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=4317548d-f831-29ba-3be9-35f080587db9@sifive.com \
    --to=paul.walmsley@sifive.com \
    --cc=linux-riscv@lists.infradead.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