[PATCH v2 0/2] tty: serial: add DT bindings and serial driver for the SiFive FU540 UART

[PATCH v2 0/2] tty: serial: add DT bindings and serial driver for the SiFive FU540 UART

[Paul Walmsley]

This series adds a serial driver, with console support, for the
UART IP block present on the SiFive FU540 SoC.  The programming
model is straightforward, but unique.

Boot-tested on a SiFive FU540 HiFive-U board (with appropriate patches
to the DT data).

The patches in this series can also be found at:

https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v4.19-rc7

This second version fixes a few problems identified by the 0-day
build system, mostly focused around remnants relating to
CONFIG_CONSOLE_POLL, and a missing spin_unlock() in the ISR.

Paul Walmsley (2):
  dt-bindings: serial: add documentation for the SiFive UART driver
  tty: serial: add driver for the SiFive UART

 .../bindings/serial/sifive-serial.txt         |   21 +
 drivers/tty/serial/Kconfig                    |   24 +
 drivers/tty/serial/Makefile                   |    1 +
 drivers/tty/serial/sifive.c                   | 1067 +++++++++++++++++
 include/uapi/linux/serial_core.h              |    3 +
 5 files changed, 1116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/sifive-serial.txt
 create mode 100644 drivers/tty/serial/sifive.c


Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Wesley Terpstra <wesley@sifive.com>
Cc: linux-serial at vger.kernel.org
Cc: linux-riscv at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
Cc: devicetree at vger.kernel.org

-- 
2.19.1

[PATCH v2 0/2] tty: serial: add DT bindings and serial driver for the SiFive FU540 UART

[Paul Walmsley]

This series adds a serial driver, with console support, for the
UART IP block present on the SiFive FU540 SoC.  The programming
model is straightforward, but unique.

Boot-tested on a SiFive FU540 HiFive-U board (with appropriate patches
to the DT data).

The patches in this series can also be found at:

https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v4.19-rc7

This second version fixes a few problems identified by the 0-day
build system, mostly focused around remnants relating to
CONFIG_CONSOLE_POLL, and a missing spin_unlock() in the ISR.

Paul Walmsley (2):
  dt-bindings: serial: add documentation for the SiFive UART driver
  tty: serial: add driver for the SiFive UART

 .../bindings/serial/sifive-serial.txt         |   21 +
 drivers/tty/serial/Kconfig                    |   24 +
 drivers/tty/serial/Makefile                   |    1 +
 drivers/tty/serial/sifive.c                   | 1067 +++++++++++++++++
 include/uapi/linux/serial_core.h              |    3 +
 5 files changed, 1116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/sifive-serial.txt
 create mode 100644 drivers/tty/serial/sifive.c


Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Wesley Terpstra <wesley@sifive.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org

-- 
2.19.1


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

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

[Paul Walmsley]

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"
+- 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
+- interrupts: Should contain the UART interrupt identifier
+- clocks: Should contain a clock identifier for the UART's parent clock
+
+
+Example:
+
+uart0: serial at 10010000 {
+	compatible = "sifive,uart0";
+	interrupt-parent = <&plic0>;
+	interrupts = <80>;
+	reg = <0x0 0x10010000 0x0 0x1000>;
+	clocks = <&prci PRCI_CLK_TLCLK>;
+};
-- 
2.19.1

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

[Paul Walmsley]

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"
+- 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
+- interrupts: Should contain the UART interrupt identifier
+- clocks: Should contain a clock identifier for the UART's parent clock
+
+
+Example:
+
+uart0: serial@10010000 {
+	compatible = "sifive,uart0";
+	interrupt-parent = <&plic0>;
+	interrupts = <80>;
+	reg = <0x0 0x10010000 0x0 0x1000>;
+	clocks = <&prci PRCI_CLK_TLCLK>;
+};
-- 
2.19.1


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

[PATCH v2 2/2] tty: serial: add driver for the SiFive UART

[Paul Walmsley]

Add a serial driver for the SiFive UART, found on SiFive FU540 devices
(among others).

The underlying serial IP block is relatively basic, and currently does
not support serial break detection.  Further information on the IP
block can be found in the documentation and Chisel sources:

https://static.dev.sifive.com/FU540-C000-v1.0.pdf

https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/uart

This driver was written in collaboration with Wesley Terpstra
<wesley@sifive.com>.

Boot-tested on a SiFive HiFive Unleashed A00 board.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Wesley Terpstra <wesley@sifive.com>
Cc: linux-serial at vger.kernel.org
Cc: linux-riscv at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

v2: remove leftover junk related to CONFIG_CONSOLE_POLL; add missing
    spin_unlock() in ISR found by 0-day and Julia Lawall

 drivers/tty/serial/Kconfig       |   24 +
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/sifive.c      | 1067 ++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |    3 +
 4 files changed, 1095 insertions(+)
 create mode 100644 drivers/tty/serial/sifive.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index df8bd0c7b97d..fdd624df75c8 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1060,6 +1060,30 @@ config SERIAL_OMAP_CONSOLE
 	  your boot loader about how to pass options to the kernel at
 	  boot time.)
 
+config SERIAL_SIFIVE
+	tristate "SiFive UART support"
+	depends on OF
+	select SERIAL_CORE
+	help
+	  Select this option if you are building a kernel for a device that
+	  contains a SiFive UART IP block.  This type of UART is present on
+	  SiFive FU540 SoCs, among others.
+
+config SERIAL_SIFIVE_CONSOLE
+	bool "Console on SiFive UART"
+	depends on SERIAL_SIFIVE=y
+	select SERIAL_CORE_CONSOLE
+	help
+	  Select this option if you would like to use a SiFive UART as the
+	  system console.
+
+	  Even if you say Y here, the currently visible virtual console
+	  (/dev/tty0) will still be used as the system console by default, but
+	  you can alter that using a kernel command line option such as
+	  "console=ttySIFx". (Try "man bootparam" or see the documentation of
+	  your boot loader about how to pass options to the kernel at
+	  boot time.)
+
 config SERIAL_LANTIQ
 	bool "Lantiq serial driver"
 	depends on LANTIQ
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675612df..7e906d3c0455 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_MVEBU_UART)	+= mvebu-uart.o
 obj-$(CONFIG_SERIAL_PIC32)	+= pic32_uart.o
 obj-$(CONFIG_SERIAL_MPS2_UART)	+= mps2-uart.o
 obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
+obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
new file mode 100644
index 000000000000..fd0220626918
--- /dev/null
+++ b/drivers/tty/serial/sifive.c
@@ -0,0 +1,1067 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SiFive UART driver
+ * Copyright (C) 2018 Paul Walmsley <paul@pwsan.com>
+ * Copyright (C) 2018 SiFive
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based partially on:
+ * - drivers/tty/serial/pxa.c
+ * - drivers/tty/serial/amba-pl011.c
+ * - drivers/tty/serial/uartlite.c
+ * - drivers/tty/serial/omap-serial.c
+ * - drivers/pwm/pwm-sifive.c
+ *
+ * See the following sources for further documentation:
+ * - Chapter 19 "Universal Asynchronous Receiver/Transmitter (UART)" of
+ *   SiFive FE310-G000 v2p3
+ * - The tree/master/src/main/scala/devices/uart directory of
+ *   https://github.com/sifive/sifive-blocks/
+ *
+ * The SiFive UART design is not 8250-compatible.  The following common
+ * features are not supported:
+ * - Word lengths other than 8 bits
+ * - Break handling
+ * - Parity
+ * - Flow control
+ * - Modem signals (DSR, RI, etc.)
+ * On the other hand, the design is free from the baggage of the 8250
+ * programming model.
+ */
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+/*
+ * Register offsets
+ */
+
+/* TXDATA */
+#define SIFIVE_SERIAL_TXDATA_OFFS		0x0
+#define SIFIVE_SERIAL_TXDATA_FULL_SHIFT		31
+#define SIFIVE_SERIAL_TXDATA_FULL_MASK		(1 << SIFIVE_SERIAL_TXDATA_FULL_SHIFT)
+#define SIFIVE_SERIAL_TXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_TXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_TXDATA_DATA_SHIFT)
+
+/* RXDATA */
+#define SIFIVE_SERIAL_RXDATA_OFFS		0x4
+#define SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT	31
+#define SIFIVE_SERIAL_RXDATA_EMPTY_MASK		(1 << SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT)
+#define SIFIVE_SERIAL_RXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_RXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_RXDATA_DATA_SHIFT)
+
+/* TXCTRL */
+#define SIFIVE_SERIAL_TXCTRL_OFFS		0x8
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT	16
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT	1
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_MASK		(1 << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT		0
+#define SIFIVE_SERIAL_TXCTRL_TXEN_MASK		(1 << SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT)
+
+/* RXCTRL */
+#define SIFIVE_SERIAL_RXCTRL_OFFS		0xC
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT	16
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT		0
+#define SIFIVE_SERIAL_RXCTRL_RXEN_MASK		(1 << SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT)
+
+/* IE */
+#define SIFIVE_SERIAL_IE_OFFS			0x10
+#define SIFIVE_SERIAL_IE_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IE_RXWM_MASK		(1 << SIFIVE_SERIAL_IE_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IE_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IE_TXWM_MASK		(1 << SIFIVE_SERIAL_IE_TXWM_SHIFT)
+
+/* IP */
+#define SIFIVE_SERIAL_IP_OFFS			0x14
+#define SIFIVE_SERIAL_IP_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IP_RXWM_MASK		(1 << SIFIVE_SERIAL_IP_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IP_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IP_TXWM_MASK		(1 << SIFIVE_SERIAL_IP_TXWM_SHIFT)
+
+/* DIV */
+#define SIFIVE_SERIAL_DIV_OFFS			0x18
+#define SIFIVE_SERIAL_DIV_DIV_SHIFT		0
+#define SIFIVE_SERIAL_DIV_DIV_MASK		(0xffff << SIFIVE_SERIAL_IP_DIV_SHIFT)
+
+/*
+ * Config macros
+ */
+
+/*
+ * SIFIVE_SERIAL_MAX_PORTS: maximum number of UARTs on a device that can
+ *                          host a serial console
+ */
+#define SIFIVE_SERIAL_MAX_PORTS			8
+
+/*
+ * SIFIVE_DEFAULT_BAUD_RATE: default baud rate that the driver should
+ *                           configure itself to use
+ */
+#define SIFIVE_DEFAULT_BAUD_RATE		115200
+
+/* SIFIVE_SERIAL_NAME: our driver's name that we pass to the operating system */
+#define SIFIVE_SERIAL_NAME			"sifive-serial"
+
+/* SIFIVE_TTY_PREFIX: tty name prefix for SiFive serial ports */
+#define SIFIVE_TTY_PREFIX			"ttySIF"
+
+/* SIFIVE_TX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_TX_FIFO_DEPTH			8
+
+/* SIFIVE_RX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_RX_FIFO_DEPTH			8
+
+#if (SIFIVE_TX_FIFO_DEPTH != SIFIVE_RX_FIFO_DEPTH)
+#error Driver does not support configurations with different TX, RX FIFO sizes
+#endif
+
+/*
+ *
+ */
+
+/**
+ * sifive_serial_port - driver-specific data extension to struct uart_port
+ * @port: struct uart_port embedded in this struct
+ * @dev: struct device *
+ * @ier: shadowed copy of the interrupt enable register
+ * @clkin_rate: input clock to the UART IP block.
+ * @baud_rate: UART serial line rate (e.g., 115200 baud)
+ * @clk_notifier: clock rate change notifier for upstream clock changes
+ *
+ * Configuration data specific to this SiFive UART.
+ */
+struct sifive_serial_port {
+	struct uart_port	port;
+	struct device		*dev;
+	unsigned char		ier;
+	unsigned long		clkin_rate;
+	unsigned long		baud_rate;
+	struct clk		*clk;
+	struct notifier_block	clk_notifier;
+};
+
+/*
+ * Structure container-of macros
+ */
+
+#define port_to_sifive_serial_port(p) (container_of((p), \
+						    struct sifive_serial_port, \
+						    port))
+
+#define notifier_to_sifive_serial_port(nb) (container_of((nb), \
+							 struct sifive_serial_port, \
+							 clk_notifier))
+
+/*
+ * Forward declarations
+ */
+static void sifive_serial_stop_tx(struct uart_port *port);
+
+/*
+ * Internal functions
+ */
+
+/**
+ * __ssp_early_writel() - write to a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ * @v: value to write to the register
+ *
+ * Given a pointer @port to a struct uart_port record, write the value
+ * @v to the IP block register address offset @offs.  This function is
+ * intended for early console use.
+ *
+ * Context: Intended to be used only by the earlyconsole code.
+ */
+static void __ssp_early_writel(u32 v, u16 offs, struct uart_port *port)
+{
+	writel_relaxed(v, port->membase + offs);
+}
+
+/**
+ * __ssp_early_readl() - read from a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Given a pointer @port to a struct uart_port record, read the
+ * contents of the IP block register located at offset @offs from the
+ * IP block base and return it.  This function is intended for early
+ * console use.
+ *
+ * Context: Intended to be called only by the earlyconsole code or by
+ *          __ssp_readl() or __ssp_writel() (in this driver)
+ *
+ * Returns: the register value read from the UART.
+ */
+static u32 __ssp_early_readl(struct uart_port *port, u16 offs)
+{
+	return readl_relaxed(port->membase + offs);
+}
+
+/**
+ * __ssp_writel() - write to a SiFive serial port register
+ * @v: value to write to the register
+ * @offs: register address offset from the IP block base address
+ * @ssp: pointer to a struct sifive_serial_port record
+ *
+ * Write the value @v to the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ */
+static void __ssp_writel(u32 v, u16 offs, struct sifive_serial_port *ssp)
+{
+	__ssp_early_writel(v, offs, &ssp->port);
+}
+
+/**
+ * __ssp_readl() - read from a SiFive serial port register
+ * @ssp: pointer to a struct sifive_serial_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Read the contents of the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ *
+ * Returns: the value of the UART register
+ */
+static u32 __ssp_readl(struct sifive_serial_port *ssp, u16 offs)
+{
+	return __ssp_early_readl(&ssp->port, offs);
+}
+
+/**
+ * sifive_serial_is_txfifo_full() - is the TXFIFO full?
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Read the transmit FIFO "full" bit, returning a non-zero value if the
+ * TX FIFO is full, or zero if space remains.  Intended to be used to prevent
+ * writes to the TX FIFO when it's full.
+ *
+ * Returns: SIFIVE_SERIAL_TXDATA_FULL_MASK (non-zero) if the transmit FIFO
+ * is full, or 0 if space remains.
+ */
+static int sifive_serial_is_txfifo_full(struct sifive_serial_port *ssp)
+{
+	return __ssp_readl(ssp, SIFIVE_SERIAL_TXDATA_OFFS) &
+		SIFIVE_SERIAL_TXDATA_FULL_MASK;
+}
+
+/**
+ * __ssp_transmit_char() - enqueue a byte to transmit onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ * @ch: character to transmit
+ *
+ * Enqueue a byte @ch onto the transmit FIFO, given a pointer @ssp to the
+ * struct sifive_serial_port * to transmit on.  Caller should first check to
+ * ensure that the TXFIFO has space; see sifive_serial_is_txfifo_full().
+ *
+ * Context: Any context.
+ */
+static void __ssp_transmit_char(struct sifive_serial_port *ssp, int ch)
+{
+	__ssp_writel(ch, SIFIVE_SERIAL_TXDATA_OFFS, ssp);
+}
+
+/**
+ * __ssp_transmit_chars() - enqueue multiple bytes onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Transfer up to a TX FIFO size's worth of characters from the Linux serial
+ * transmit buffer to the SiFive UART TX FIFO.
+ *
+ * Context: Any context.  Expects @ssp->port.lock to be held by caller.
+ */
+static void __ssp_transmit_chars(struct sifive_serial_port *ssp)
+{
+	struct circ_buf *xmit = &ssp->port.state->xmit;
+	int count;
+
+	if (ssp->port.x_char) {
+		__ssp_transmit_char(ssp, ssp->port.x_char);
+		ssp->port.icount.tx++;
+		ssp->port.x_char = 0;
+		return;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(&ssp->port)) {
+		sifive_serial_stop_tx(&ssp->port);
+		return;
+	}
+	count = SIFIVE_TX_FIFO_DEPTH;
+	do {
+		__ssp_transmit_char(ssp, xmit->buf[xmit->tail]);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		ssp->port.icount.tx++;
+		if (uart_circ_empty(xmit))
+			break;
+	} while (--count > 0);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&ssp->port);
+
+	if (uart_circ_empty(xmit))
+		sifive_serial_stop_tx(&ssp->port);
+}
+
+/**
+ * __ssp_enable_txwm() - enable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the transmit FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_txwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_enable_rxwm() - enable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the receive FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_txwm() - disable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the transmit FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_txwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_rxwm() - disable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the receive FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_receive_char() - receive a byte from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ * @is_empty: char pointer to return whether the RX FIFO is empty
+ *
+ * Try to read a byte from the SiFive UART RX FIFO, referenced by
+ * @ssp, and to return it.  Also returns the RX FIFO empty bit in
+ * the char pointed to by @ch.  The caller must pass the byte back to the
+ * Linux serial layer if needed.
+ *
+ * Returns: the byte read from the UART RX FIFO.
+ */
+static char __ssp_receive_char(struct sifive_serial_port *ssp, char *is_empty)
+{
+	u32 v;
+	u8 ch;
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXDATA_OFFS);
+
+	if (!is_empty)
+		WARN_ON(1);
+	else
+		*is_empty = (v & SIFIVE_SERIAL_RXDATA_EMPTY_MASK) >>
+			SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT;
+
+	ch = (v & SIFIVE_SERIAL_RXDATA_DATA_MASK) >>
+		SIFIVE_SERIAL_RXDATA_DATA_SHIFT;
+
+	return ch;
+}
+
+/**
+ * __ssp_receive_chars() - receive multiple bytes from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Receive up to an RX FIFO's worth of bytes from the SiFive UART referred
+ * to by @ssp and pass them up to the Linux serial layer.
+ *
+ * Context: Expects ssp->port.lock to be held by caller.
+ */
+static void __ssp_receive_chars(struct sifive_serial_port *ssp)
+{
+	unsigned char ch;
+	char is_empty;
+	int c;
+
+	for (c = SIFIVE_RX_FIFO_DEPTH; c > 0; --c) {
+		ch = __ssp_receive_char(ssp, &is_empty);
+		if (is_empty)
+			break;
+
+		ssp->port.icount.rx++;
+		uart_insert_char(&ssp->port, 0, 0, ch, TTY_NORMAL);
+	}
+
+	spin_unlock(&ssp->port.lock);
+	tty_flip_buffer_push(&ssp->port.state->port);
+	spin_lock(&ssp->port.lock);
+}
+
+/**
+ * __ssp_update_div() - calculate the divisor setting by the line rate
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Calculate the appropriate value of the clock divisor for the UART
+ * and target line rate referred to by @ssp and write it into the
+ * hardware.
+ */
+static void __ssp_update_div(struct sifive_serial_port *ssp)
+{
+	u16 div;
+
+	div = DIV_ROUND_UP(ssp->clkin_rate, ssp->baud_rate) - 1;
+
+	__ssp_writel(div, SIFIVE_SERIAL_DIV_OFFS, ssp);
+}
+
+/**
+ * __ssp_update_baud_rate() - set the UART "baud rate"
+ * @ssp: pointer to a struct sifive_serial_port
+ * @rate: new target bit rate
+ *
+ * Calculate the UART divisor value for the target bit rate @rate for the
+ * SiFive UART described by @ssp and program it into the UART.  There may
+ * be some error between the target bit rate and the actual bit rate implemented
+ * by the UART due to clock ratio granularity.
+ */
+static void __ssp_update_baud_rate(struct sifive_serial_port *ssp,
+				   unsigned int rate)
+{
+	if (ssp->baud_rate == rate)
+		return;
+
+	ssp->baud_rate = rate;
+	__ssp_update_div(ssp);
+}
+
+/**
+ * __ssp_set_stop_bits() - set the number of stop bits
+ * @ssp: pointer to a struct sifive_serial_port
+ * @nstop: 1 or 2 (stop bits)
+ *
+ * Program the SiFive UART referred to by @ssp to use @nstop stop bits.
+ */
+static void __ssp_set_stop_bits(struct sifive_serial_port *ssp, char nstop)
+{
+	u32 v;
+
+	if (nstop < 1 || nstop > 2) {
+		WARN_ON(1);
+		return;
+	}
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_TXCTRL_OFFS);
+	v &= ~SIFIVE_SERIAL_TXCTRL_NSTOP_MASK;
+	v |= (nstop - 1) << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT;
+	__ssp_writel(v, SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+}
+
+/**
+ * __ssp_wait_for_xmitr() - wait for an empty slot on the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Delay while the UART TX FIFO referred to by @ssp is marked as full.
+ *
+ * Context: Any context.
+ */
+static void __maybe_unused __ssp_wait_for_xmitr(struct sifive_serial_port *ssp)
+{
+	while (sifive_serial_is_txfifo_full(ssp))
+		udelay(1); /* XXX Could probably be more intelligent here */
+}
+
+/*
+ * Linux serial API functions
+ */
+
+static void sifive_serial_stop_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_txwm(ssp);
+}
+
+static void sifive_serial_stop_rx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+}
+
+static void sifive_serial_start_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_txwm(ssp);
+}
+
+static irqreturn_t sifive_serial_irq(int irq, void *dev_id)
+{
+	struct sifive_serial_port *ssp = dev_id;
+	u32 ip;
+
+	spin_lock(&ssp->port.lock);
+
+	ip = __ssp_readl(ssp, SIFIVE_SERIAL_IP_OFFS);
+	if (!ip) {
+		spin_unlock(&ssp->port.lock);
+		return IRQ_NONE;
+	}
+
+	if (ip & SIFIVE_SERIAL_IP_RXWM_MASK)
+		__ssp_receive_chars(ssp);
+	if (ip & SIFIVE_SERIAL_IP_TXWM_MASK)
+		__ssp_transmit_chars(ssp);
+
+	spin_unlock(&ssp->port.lock);
+
+	return IRQ_HANDLED;
+}
+
+static unsigned int sifive_serial_tx_empty(struct uart_port *port)
+{
+	return TIOCSER_TEMT;
+}
+
+static unsigned int sifive_serial_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
+}
+
+static void sifive_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* IP block does not support these signals */
+}
+
+static void sifive_serial_break_ctl(struct uart_port *port, int break_state)
+{
+	/* IP block does not support sending a break */
+}
+
+static int sifive_serial_startup(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_rxwm(ssp);
+
+	return 0;
+}
+
+static void sifive_serial_shutdown(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+	__ssp_disable_txwm(ssp);
+}
+
+/**
+ * sifive_serial_clk_notifier() - clock post-rate-change notifier
+ * @nb: pointer to the struct notifier_block, from the notifier code
+ * @event: event mask from the notifier code
+ * @data: pointer to the struct clk_notifier_data from the notifier code
+ *
+ * On the V0 SoC, the UART IP block is derived from the CPU clock source
+ * after a synchronous divide-by-two divider, so any CPU clock rate change
+ * requires the UART baud rate to be updated.  This presumably could corrupt any
+ * serial word currently being transmitted or received.  It would probably
+ * be better to stop receives and transmits, then complete the baud rate
+ * change, then re-enable them.
+ */
+static int sifive_serial_clk_notifier(struct notifier_block *nb,
+				      unsigned long event, void *data)
+{
+	struct clk_notifier_data *cnd = data;
+	struct sifive_serial_port *ssp = notifier_to_sifive_serial_port(nb);
+
+	if (event == POST_RATE_CHANGE && ssp->clkin_rate != cnd->new_rate) {
+		ssp->clkin_rate = cnd->new_rate;
+		__ssp_update_div(ssp);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void sifive_serial_set_termios(struct uart_port *port,
+				      struct ktermios *termios,
+				      struct ktermios *old)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+	unsigned long flags;
+	u32 v, old_v;
+	int rate;
+	char nstop;
+
+	if ((termios->c_cflag & CSIZE) != CS8) {
+		dev_err(ssp->port.dev, "only 8-bit words supported\n");
+		return;
+	}
+
+	/* Set number of stop bits */
+	nstop = (termios->c_cflag & CSTOPB) ? 2 : 1;
+	__ssp_set_stop_bits(ssp, nstop);
+
+	/* Set line rate */
+	rate = uart_get_baud_rate(port, termios, old, 0, ssp->clkin_rate / 16);
+	__ssp_update_baud_rate(ssp, rate);
+
+	spin_lock_irqsave(&ssp->port.lock, flags);
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, rate);
+
+	ssp->port.read_status_mask = 0;
+	if (termios->c_iflag & INPCK) {
+		dev_err(ssp->port.dev, "INPCK flag not supported\n");
+		goto ssst_out;
+	}
+	if (termios->c_iflag & (BRKINT | PARMRK)) {
+		dev_err(ssp->port.dev, "BRKINT/PARMRK flag not supported\n");
+		goto ssst_out;
+	}
+
+	/*
+	 * ignore all characters if CREAD is not set
+	 */
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXCTRL_OFFS);
+	old_v = v;
+	if ((termios->c_cflag & CREAD) == 0)
+		v &= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	else
+		v |= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	if (v != old_v)
+		__ssp_writel(v, SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+ssst_out:
+	spin_unlock_irqrestore(&ssp->port.lock, flags);
+}
+
+static void sifive_serial_release_port(struct uart_port *port)
+{
+}
+
+static int sifive_serial_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+static void sifive_serial_config_port(struct uart_port *port, int flags)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	ssp->port.type = PORT_SIFIVE_V0;
+}
+
+static int sifive_serial_verify_port(struct uart_port *port,
+				     struct serial_struct *ser)
+{
+	return -EINVAL;
+}
+
+static const char *sifive_serial_type(struct uart_port *port)
+{
+	return port->type == PORT_SIFIVE_V0 ? "SiFive UART v0" : NULL;
+}
+
+/*
+ * Early console support
+ */
+
+#ifdef CONFIG_SERIAL_EARLYCON
+static void early_sifive_serial_putc(struct uart_port *port, int c)
+{
+	while (__ssp_early_readl(port, SIFIVE_SERIAL_TXDATA_OFFS) &
+	       SIFIVE_SERIAL_TXDATA_FULL_MASK)
+		cpu_relax();
+
+	__ssp_early_writel(c, SIFIVE_SERIAL_TXDATA_OFFS, port);
+}
+
+static void early_sifive_serial_write(struct console *con, const char *s,
+				      unsigned int n)
+{
+	struct earlycon_device *dev = con->data;
+	struct uart_port *port = &dev->port;
+
+	uart_console_write(port, s, n, early_sifive_serial_putc);
+}
+
+static int __init early_sifive_serial_setup(struct earlycon_device *dev,
+					    const char *options)
+{
+	struct uart_port *port = &dev->port;
+
+	if (!port->membase)
+		return -ENODEV;
+
+	dev->con->write = early_sifive_serial_write;
+
+	return 0;
+}
+
+OF_EARLYCON_DECLARE(sifive, "sifive,uart0", early_sifive_serial_setup);
+OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
+		    early_sifive_serial_setup);
+#endif /* CONFIG_SERIAL_EARLYCON */
+
+/*
+ * Linux console interface
+ */
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+
+static struct sifive_serial_port *sifive_serial_console_ports[SIFIVE_SERIAL_MAX_PORTS];
+
+static void sifive_serial_console_putchar(struct uart_port *port, int ch)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_wait_for_xmitr(ssp);
+	__ssp_transmit_char(ssp, ch);
+}
+
+static void sifive_serial_console_write(struct console *co, const char *s,
+					unsigned int count)
+{
+	struct sifive_serial_port *ssp = sifive_serial_console_ports[co->index];
+	unsigned long flags;
+	unsigned int ier;
+	int locked = 1;
+
+	if (!ssp)
+		return;
+
+	local_irq_save(flags);
+	if (ssp->port.sysrq)
+		locked = 0;
+	else if (oops_in_progress)
+		locked = spin_trylock(&ssp->port.lock);
+	else
+		spin_lock(&ssp->port.lock);
+
+	ier = __ssp_readl(ssp, SIFIVE_SERIAL_IE_OFFS);
+	__ssp_writel(0, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	uart_console_write(&ssp->port, s, count, sifive_serial_console_putchar);
+
+	__ssp_writel(ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	if (locked)
+		spin_unlock(&ssp->port.lock);
+	local_irq_restore(flags);
+}
+
+static int __init sifive_serial_console_setup(struct console *co, char *options)
+{
+	struct sifive_serial_port *ssp;
+	int baud = SIFIVE_DEFAULT_BAUD_RATE;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (co->index < 0 || co->index >= SIFIVE_SERIAL_MAX_PORTS)
+		return -ENODEV;
+
+	ssp = sifive_serial_console_ports[co->index];
+	if (!ssp)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(&ssp->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sifive_serial_uart_driver;
+
+static struct console sifive_serial_console = {
+	.name		= SIFIVE_TTY_PREFIX,
+	.write		= sifive_serial_console_write,
+	.device		= uart_console_device,
+	.setup		= sifive_serial_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &sifive_serial_uart_driver,
+};
+
+static int __init sifive_console_init(void)
+{
+	register_console(&sifive_serial_console);
+	return 0;
+}
+
+console_initcall(sifive_console_init);
+
+static void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = ssp;
+}
+
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = 0;
+}
+
+#define SIFIVE_SERIAL_CONSOLE	(&sifive_serial_console)
+
+#else
+
+#define SIFIVE_SERIAL_CONSOLE	NULL
+
+static inline void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{}
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{}
+
+#endif
+
+static const struct uart_ops sifive_serial_uops = {
+	.tx_empty	= sifive_serial_tx_empty,
+	.set_mctrl	= sifive_serial_set_mctrl,
+	.get_mctrl	= sifive_serial_get_mctrl,
+	.stop_tx	= sifive_serial_stop_tx,
+	.start_tx	= sifive_serial_start_tx,
+	.stop_rx	= sifive_serial_stop_rx,
+	.break_ctl	= sifive_serial_break_ctl,
+	.startup	= sifive_serial_startup,
+	.shutdown	= sifive_serial_shutdown,
+	.set_termios	= sifive_serial_set_termios,
+	.type		= sifive_serial_type,
+	.release_port	= sifive_serial_release_port,
+	.request_port	= sifive_serial_request_port,
+	.config_port	= sifive_serial_config_port,
+	.verify_port	= sifive_serial_verify_port,
+};
+
+static struct uart_driver sifive_serial_uart_driver = {
+	.owner		= THIS_MODULE,
+	.driver_name	= SIFIVE_SERIAL_NAME,
+	.dev_name	= SIFIVE_TTY_PREFIX,
+	.nr		= SIFIVE_SERIAL_MAX_PORTS,
+	.cons		= SIFIVE_SERIAL_CONSOLE,
+};
+
+static int sifive_serial_probe(struct platform_device *pdev)
+{
+	struct sifive_serial_port *ssp;
+	struct resource *mem;
+	struct clk *clk;
+	void __iomem *base;
+	int irq, id, r;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "could not acquire interrupt\n");
+		return -EPROBE_DEFER;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(base)) {
+		dev_err(&pdev->dev, "could not acquire device memory\n");
+		return PTR_ERR(base);
+	}
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "unable to find controller clock\n");
+		return PTR_ERR(clk);
+	}
+
+	id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (id < 0) {
+		dev_err(&pdev->dev, "missing aliases entry\n");
+		return id;
+	}
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+	if (id > SIFIVE_SERIAL_MAX_PORTS) {
+		dev_err(&pdev->dev, "too many UARTs (%d)\n", id);
+		return -EINVAL;
+	}
+#endif
+
+	ssp = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
+	if (!ssp)
+		return -ENOMEM;
+
+	ssp->port.dev = &pdev->dev;
+	ssp->port.type = PORT_SIFIVE_V0;
+	ssp->port.iotype = UPIO_MEM;
+	ssp->port.irq = irq;
+	ssp->port.fifosize = SIFIVE_TX_FIFO_DEPTH;
+	ssp->port.ops = &sifive_serial_uops;
+	ssp->port.line = id;
+	ssp->port.mapbase = mem->start;
+	ssp->port.membase = base;
+	ssp->dev = &pdev->dev;
+	ssp->clk = clk;
+	ssp->clk_notifier.notifier_call = sifive_serial_clk_notifier;
+
+	r = clk_notifier_register(ssp->clk, &ssp->clk_notifier);
+	if (r) {
+		dev_err(&pdev->dev, "could not register clock notifier: %d\n",
+			r);
+		goto probe_out1;
+	}
+
+	/* Set up clock divider */
+	ssp->clkin_rate = clk_get_rate(ssp->clk);
+	ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE;
+	__ssp_update_div(ssp);
+
+	platform_set_drvdata(pdev, ssp);
+
+	/* Enable transmits and set the watermark level to 1 */
+	__ssp_writel((1 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT) |
+		     SIFIVE_SERIAL_TXCTRL_TXEN_MASK,
+		     SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+
+	/* Enable receives and set the watermark level to 0 */
+	__ssp_writel((0 << SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT) |
+		     SIFIVE_SERIAL_RXCTRL_RXEN_MASK,
+		     SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+	r = request_irq(ssp->port.irq, sifive_serial_irq, ssp->port.irqflags,
+			dev_name(&pdev->dev), ssp);
+	if (r) {
+		dev_err(&pdev->dev, "could not attach interrupt: %d\n", r);
+		goto probe_out2;
+	}
+
+	__ssp_add_console_port(ssp);
+
+	r = uart_add_one_port(&sifive_serial_uart_driver, &ssp->port);
+	if (r != 0) {
+		dev_err(&pdev->dev, "could not add uart: %d\n", r);
+		goto probe_out3;
+	}
+
+	return 0;
+
+probe_out3:
+	__ssp_remove_console_port(ssp);
+	free_irq(ssp->port.irq, ssp);
+probe_out2:
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+probe_out1:
+	return r;
+}
+
+static int sifive_serial_remove(struct platform_device *dev)
+{
+	struct sifive_serial_port *ssp = platform_get_drvdata(dev);
+
+	__ssp_remove_console_port(ssp);
+	uart_remove_one_port(&sifive_serial_uart_driver, &ssp->port);
+	free_irq(ssp->port.irq, ssp);
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+
+	return 0;
+}
+
+static const struct of_device_id sifive_serial_of_match[] = {
+	{ .compatible = "sifive,fu540-c000-uart0" },
+	{ .compatible = "sifive,uart0" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sifive_serial_match);
+
+static struct platform_driver sifive_serial_platform_driver = {
+	.probe		= sifive_serial_probe,
+	.remove		= sifive_serial_remove,
+	.driver		= {
+		.name	= SIFIVE_SERIAL_NAME,
+		.of_match_table = of_match_ptr(sifive_serial_of_match),
+	},
+};
+
+static int __init sifive_serial_init(void)
+{
+	int r;
+
+	r = uart_register_driver(&sifive_serial_uart_driver);
+	if (r)
+		goto init_out1;
+
+	r = platform_driver_register(&sifive_serial_platform_driver);
+	if (r)
+		goto init_out2;
+
+	return 0;
+
+init_out2:
+	uart_unregister_driver(&sifive_serial_uart_driver);
+init_out1:
+	return r;
+}
+
+static void __exit sifive_serial_exit(void)
+{
+	platform_driver_unregister(&sifive_serial_platform_driver);
+	uart_unregister_driver(&sifive_serial_uart_driver);
+}
+
+module_init(sifive_serial_init);
+module_exit(sifive_serial_exit);
+
+MODULE_DESCRIPTION("SiFive UART serial driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Paul Walmsley <paul@pwsan.com>");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index dce5f9dae121..569d5172d23c 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -281,4 +281,7 @@
 /* MediaTek BTIF */
 #define PORT_MTK_BTIF	117
 
+/* SiFive UART */
+#define PORT_SIFIVE_V0	118
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
2.19.1

[PATCH v2 2/2] tty: serial: add driver for the SiFive UART

[Paul Walmsley]

Add a serial driver for the SiFive UART, found on SiFive FU540 devices
(among others).

The underlying serial IP block is relatively basic, and currently does
not support serial break detection.  Further information on the IP
block can be found in the documentation and Chisel sources:

https://static.dev.sifive.com/FU540-C000-v1.0.pdf

https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/uart

This driver was written in collaboration with Wesley Terpstra
<wesley@sifive.com>.

Boot-tested on a SiFive HiFive Unleashed A00 board.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Wesley Terpstra <wesley@sifive.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---

v2: remove leftover junk related to CONFIG_CONSOLE_POLL; add missing
    spin_unlock() in ISR found by 0-day and Julia Lawall

 drivers/tty/serial/Kconfig       |   24 +
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/sifive.c      | 1067 ++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |    3 +
 4 files changed, 1095 insertions(+)
 create mode 100644 drivers/tty/serial/sifive.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index df8bd0c7b97d..fdd624df75c8 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1060,6 +1060,30 @@ config SERIAL_OMAP_CONSOLE
 	  your boot loader about how to pass options to the kernel at
 	  boot time.)
 
+config SERIAL_SIFIVE
+	tristate "SiFive UART support"
+	depends on OF
+	select SERIAL_CORE
+	help
+	  Select this option if you are building a kernel for a device that
+	  contains a SiFive UART IP block.  This type of UART is present on
+	  SiFive FU540 SoCs, among others.
+
+config SERIAL_SIFIVE_CONSOLE
+	bool "Console on SiFive UART"
+	depends on SERIAL_SIFIVE=y
+	select SERIAL_CORE_CONSOLE
+	help
+	  Select this option if you would like to use a SiFive UART as the
+	  system console.
+
+	  Even if you say Y here, the currently visible virtual console
+	  (/dev/tty0) will still be used as the system console by default, but
+	  you can alter that using a kernel command line option such as
+	  "console=ttySIFx". (Try "man bootparam" or see the documentation of
+	  your boot loader about how to pass options to the kernel at
+	  boot time.)
+
 config SERIAL_LANTIQ
 	bool "Lantiq serial driver"
 	depends on LANTIQ
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675612df..7e906d3c0455 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_MVEBU_UART)	+= mvebu-uart.o
 obj-$(CONFIG_SERIAL_PIC32)	+= pic32_uart.o
 obj-$(CONFIG_SERIAL_MPS2_UART)	+= mps2-uart.o
 obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
+obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
new file mode 100644
index 000000000000..fd0220626918
--- /dev/null
+++ b/drivers/tty/serial/sifive.c
@@ -0,0 +1,1067 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SiFive UART driver
+ * Copyright (C) 2018 Paul Walmsley <paul@pwsan.com>
+ * Copyright (C) 2018 SiFive
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based partially on:
+ * - drivers/tty/serial/pxa.c
+ * - drivers/tty/serial/amba-pl011.c
+ * - drivers/tty/serial/uartlite.c
+ * - drivers/tty/serial/omap-serial.c
+ * - drivers/pwm/pwm-sifive.c
+ *
+ * See the following sources for further documentation:
+ * - Chapter 19 "Universal Asynchronous Receiver/Transmitter (UART)" of
+ *   SiFive FE310-G000 v2p3
+ * - The tree/master/src/main/scala/devices/uart directory of
+ *   https://github.com/sifive/sifive-blocks/
+ *
+ * The SiFive UART design is not 8250-compatible.  The following common
+ * features are not supported:
+ * - Word lengths other than 8 bits
+ * - Break handling
+ * - Parity
+ * - Flow control
+ * - Modem signals (DSR, RI, etc.)
+ * On the other hand, the design is free from the baggage of the 8250
+ * programming model.
+ */
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+/*
+ * Register offsets
+ */
+
+/* TXDATA */
+#define SIFIVE_SERIAL_TXDATA_OFFS		0x0
+#define SIFIVE_SERIAL_TXDATA_FULL_SHIFT		31
+#define SIFIVE_SERIAL_TXDATA_FULL_MASK		(1 << SIFIVE_SERIAL_TXDATA_FULL_SHIFT)
+#define SIFIVE_SERIAL_TXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_TXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_TXDATA_DATA_SHIFT)
+
+/* RXDATA */
+#define SIFIVE_SERIAL_RXDATA_OFFS		0x4
+#define SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT	31
+#define SIFIVE_SERIAL_RXDATA_EMPTY_MASK		(1 << SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT)
+#define SIFIVE_SERIAL_RXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_RXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_RXDATA_DATA_SHIFT)
+
+/* TXCTRL */
+#define SIFIVE_SERIAL_TXCTRL_OFFS		0x8
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT	16
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT	1
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_MASK		(1 << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT		0
+#define SIFIVE_SERIAL_TXCTRL_TXEN_MASK		(1 << SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT)
+
+/* RXCTRL */
+#define SIFIVE_SERIAL_RXCTRL_OFFS		0xC
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT	16
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT		0
+#define SIFIVE_SERIAL_RXCTRL_RXEN_MASK		(1 << SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT)
+
+/* IE */
+#define SIFIVE_SERIAL_IE_OFFS			0x10
+#define SIFIVE_SERIAL_IE_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IE_RXWM_MASK		(1 << SIFIVE_SERIAL_IE_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IE_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IE_TXWM_MASK		(1 << SIFIVE_SERIAL_IE_TXWM_SHIFT)
+
+/* IP */
+#define SIFIVE_SERIAL_IP_OFFS			0x14
+#define SIFIVE_SERIAL_IP_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IP_RXWM_MASK		(1 << SIFIVE_SERIAL_IP_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IP_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IP_TXWM_MASK		(1 << SIFIVE_SERIAL_IP_TXWM_SHIFT)
+
+/* DIV */
+#define SIFIVE_SERIAL_DIV_OFFS			0x18
+#define SIFIVE_SERIAL_DIV_DIV_SHIFT		0
+#define SIFIVE_SERIAL_DIV_DIV_MASK		(0xffff << SIFIVE_SERIAL_IP_DIV_SHIFT)
+
+/*
+ * Config macros
+ */
+
+/*
+ * SIFIVE_SERIAL_MAX_PORTS: maximum number of UARTs on a device that can
+ *                          host a serial console
+ */
+#define SIFIVE_SERIAL_MAX_PORTS			8
+
+/*
+ * SIFIVE_DEFAULT_BAUD_RATE: default baud rate that the driver should
+ *                           configure itself to use
+ */
+#define SIFIVE_DEFAULT_BAUD_RATE		115200
+
+/* SIFIVE_SERIAL_NAME: our driver's name that we pass to the operating system */
+#define SIFIVE_SERIAL_NAME			"sifive-serial"
+
+/* SIFIVE_TTY_PREFIX: tty name prefix for SiFive serial ports */
+#define SIFIVE_TTY_PREFIX			"ttySIF"
+
+/* SIFIVE_TX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_TX_FIFO_DEPTH			8
+
+/* SIFIVE_RX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_RX_FIFO_DEPTH			8
+
+#if (SIFIVE_TX_FIFO_DEPTH != SIFIVE_RX_FIFO_DEPTH)
+#error Driver does not support configurations with different TX, RX FIFO sizes
+#endif
+
+/*
+ *
+ */
+
+/**
+ * sifive_serial_port - driver-specific data extension to struct uart_port
+ * @port: struct uart_port embedded in this struct
+ * @dev: struct device *
+ * @ier: shadowed copy of the interrupt enable register
+ * @clkin_rate: input clock to the UART IP block.
+ * @baud_rate: UART serial line rate (e.g., 115200 baud)
+ * @clk_notifier: clock rate change notifier for upstream clock changes
+ *
+ * Configuration data specific to this SiFive UART.
+ */
+struct sifive_serial_port {
+	struct uart_port	port;
+	struct device		*dev;
+	unsigned char		ier;
+	unsigned long		clkin_rate;
+	unsigned long		baud_rate;
+	struct clk		*clk;
+	struct notifier_block	clk_notifier;
+};
+
+/*
+ * Structure container-of macros
+ */
+
+#define port_to_sifive_serial_port(p) (container_of((p), \
+						    struct sifive_serial_port, \
+						    port))
+
+#define notifier_to_sifive_serial_port(nb) (container_of((nb), \
+							 struct sifive_serial_port, \
+							 clk_notifier))
+
+/*
+ * Forward declarations
+ */
+static void sifive_serial_stop_tx(struct uart_port *port);
+
+/*
+ * Internal functions
+ */
+
+/**
+ * __ssp_early_writel() - write to a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ * @v: value to write to the register
+ *
+ * Given a pointer @port to a struct uart_port record, write the value
+ * @v to the IP block register address offset @offs.  This function is
+ * intended for early console use.
+ *
+ * Context: Intended to be used only by the earlyconsole code.
+ */
+static void __ssp_early_writel(u32 v, u16 offs, struct uart_port *port)
+{
+	writel_relaxed(v, port->membase + offs);
+}
+
+/**
+ * __ssp_early_readl() - read from a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Given a pointer @port to a struct uart_port record, read the
+ * contents of the IP block register located at offset @offs from the
+ * IP block base and return it.  This function is intended for early
+ * console use.
+ *
+ * Context: Intended to be called only by the earlyconsole code or by
+ *          __ssp_readl() or __ssp_writel() (in this driver)
+ *
+ * Returns: the register value read from the UART.
+ */
+static u32 __ssp_early_readl(struct uart_port *port, u16 offs)
+{
+	return readl_relaxed(port->membase + offs);
+}
+
+/**
+ * __ssp_writel() - write to a SiFive serial port register
+ * @v: value to write to the register
+ * @offs: register address offset from the IP block base address
+ * @ssp: pointer to a struct sifive_serial_port record
+ *
+ * Write the value @v to the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ */
+static void __ssp_writel(u32 v, u16 offs, struct sifive_serial_port *ssp)
+{
+	__ssp_early_writel(v, offs, &ssp->port);
+}
+
+/**
+ * __ssp_readl() - read from a SiFive serial port register
+ * @ssp: pointer to a struct sifive_serial_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Read the contents of the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ *
+ * Returns: the value of the UART register
+ */
+static u32 __ssp_readl(struct sifive_serial_port *ssp, u16 offs)
+{
+	return __ssp_early_readl(&ssp->port, offs);
+}
+
+/**
+ * sifive_serial_is_txfifo_full() - is the TXFIFO full?
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Read the transmit FIFO "full" bit, returning a non-zero value if the
+ * TX FIFO is full, or zero if space remains.  Intended to be used to prevent
+ * writes to the TX FIFO when it's full.
+ *
+ * Returns: SIFIVE_SERIAL_TXDATA_FULL_MASK (non-zero) if the transmit FIFO
+ * is full, or 0 if space remains.
+ */
+static int sifive_serial_is_txfifo_full(struct sifive_serial_port *ssp)
+{
+	return __ssp_readl(ssp, SIFIVE_SERIAL_TXDATA_OFFS) &
+		SIFIVE_SERIAL_TXDATA_FULL_MASK;
+}
+
+/**
+ * __ssp_transmit_char() - enqueue a byte to transmit onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ * @ch: character to transmit
+ *
+ * Enqueue a byte @ch onto the transmit FIFO, given a pointer @ssp to the
+ * struct sifive_serial_port * to transmit on.  Caller should first check to
+ * ensure that the TXFIFO has space; see sifive_serial_is_txfifo_full().
+ *
+ * Context: Any context.
+ */
+static void __ssp_transmit_char(struct sifive_serial_port *ssp, int ch)
+{
+	__ssp_writel(ch, SIFIVE_SERIAL_TXDATA_OFFS, ssp);
+}
+
+/**
+ * __ssp_transmit_chars() - enqueue multiple bytes onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Transfer up to a TX FIFO size's worth of characters from the Linux serial
+ * transmit buffer to the SiFive UART TX FIFO.
+ *
+ * Context: Any context.  Expects @ssp->port.lock to be held by caller.
+ */
+static void __ssp_transmit_chars(struct sifive_serial_port *ssp)
+{
+	struct circ_buf *xmit = &ssp->port.state->xmit;
+	int count;
+
+	if (ssp->port.x_char) {
+		__ssp_transmit_char(ssp, ssp->port.x_char);
+		ssp->port.icount.tx++;
+		ssp->port.x_char = 0;
+		return;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(&ssp->port)) {
+		sifive_serial_stop_tx(&ssp->port);
+		return;
+	}
+	count = SIFIVE_TX_FIFO_DEPTH;
+	do {
+		__ssp_transmit_char(ssp, xmit->buf[xmit->tail]);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		ssp->port.icount.tx++;
+		if (uart_circ_empty(xmit))
+			break;
+	} while (--count > 0);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&ssp->port);
+
+	if (uart_circ_empty(xmit))
+		sifive_serial_stop_tx(&ssp->port);
+}
+
+/**
+ * __ssp_enable_txwm() - enable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the transmit FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_txwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_enable_rxwm() - enable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the receive FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_txwm() - disable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the transmit FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_txwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_rxwm() - disable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the receive FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_receive_char() - receive a byte from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ * @is_empty: char pointer to return whether the RX FIFO is empty
+ *
+ * Try to read a byte from the SiFive UART RX FIFO, referenced by
+ * @ssp, and to return it.  Also returns the RX FIFO empty bit in
+ * the char pointed to by @ch.  The caller must pass the byte back to the
+ * Linux serial layer if needed.
+ *
+ * Returns: the byte read from the UART RX FIFO.
+ */
+static char __ssp_receive_char(struct sifive_serial_port *ssp, char *is_empty)
+{
+	u32 v;
+	u8 ch;
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXDATA_OFFS);
+
+	if (!is_empty)
+		WARN_ON(1);
+	else
+		*is_empty = (v & SIFIVE_SERIAL_RXDATA_EMPTY_MASK) >>
+			SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT;
+
+	ch = (v & SIFIVE_SERIAL_RXDATA_DATA_MASK) >>
+		SIFIVE_SERIAL_RXDATA_DATA_SHIFT;
+
+	return ch;
+}
+
+/**
+ * __ssp_receive_chars() - receive multiple bytes from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Receive up to an RX FIFO's worth of bytes from the SiFive UART referred
+ * to by @ssp and pass them up to the Linux serial layer.
+ *
+ * Context: Expects ssp->port.lock to be held by caller.
+ */
+static void __ssp_receive_chars(struct sifive_serial_port *ssp)
+{
+	unsigned char ch;
+	char is_empty;
+	int c;
+
+	for (c = SIFIVE_RX_FIFO_DEPTH; c > 0; --c) {
+		ch = __ssp_receive_char(ssp, &is_empty);
+		if (is_empty)
+			break;
+
+		ssp->port.icount.rx++;
+		uart_insert_char(&ssp->port, 0, 0, ch, TTY_NORMAL);
+	}
+
+	spin_unlock(&ssp->port.lock);
+	tty_flip_buffer_push(&ssp->port.state->port);
+	spin_lock(&ssp->port.lock);
+}
+
+/**
+ * __ssp_update_div() - calculate the divisor setting by the line rate
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Calculate the appropriate value of the clock divisor for the UART
+ * and target line rate referred to by @ssp and write it into the
+ * hardware.
+ */
+static void __ssp_update_div(struct sifive_serial_port *ssp)
+{
+	u16 div;
+
+	div = DIV_ROUND_UP(ssp->clkin_rate, ssp->baud_rate) - 1;
+
+	__ssp_writel(div, SIFIVE_SERIAL_DIV_OFFS, ssp);
+}
+
+/**
+ * __ssp_update_baud_rate() - set the UART "baud rate"
+ * @ssp: pointer to a struct sifive_serial_port
+ * @rate: new target bit rate
+ *
+ * Calculate the UART divisor value for the target bit rate @rate for the
+ * SiFive UART described by @ssp and program it into the UART.  There may
+ * be some error between the target bit rate and the actual bit rate implemented
+ * by the UART due to clock ratio granularity.
+ */
+static void __ssp_update_baud_rate(struct sifive_serial_port *ssp,
+				   unsigned int rate)
+{
+	if (ssp->baud_rate == rate)
+		return;
+
+	ssp->baud_rate = rate;
+	__ssp_update_div(ssp);
+}
+
+/**
+ * __ssp_set_stop_bits() - set the number of stop bits
+ * @ssp: pointer to a struct sifive_serial_port
+ * @nstop: 1 or 2 (stop bits)
+ *
+ * Program the SiFive UART referred to by @ssp to use @nstop stop bits.
+ */
+static void __ssp_set_stop_bits(struct sifive_serial_port *ssp, char nstop)
+{
+	u32 v;
+
+	if (nstop < 1 || nstop > 2) {
+		WARN_ON(1);
+		return;
+	}
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_TXCTRL_OFFS);
+	v &= ~SIFIVE_SERIAL_TXCTRL_NSTOP_MASK;
+	v |= (nstop - 1) << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT;
+	__ssp_writel(v, SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+}
+
+/**
+ * __ssp_wait_for_xmitr() - wait for an empty slot on the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Delay while the UART TX FIFO referred to by @ssp is marked as full.
+ *
+ * Context: Any context.
+ */
+static void __maybe_unused __ssp_wait_for_xmitr(struct sifive_serial_port *ssp)
+{
+	while (sifive_serial_is_txfifo_full(ssp))
+		udelay(1); /* XXX Could probably be more intelligent here */
+}
+
+/*
+ * Linux serial API functions
+ */
+
+static void sifive_serial_stop_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_txwm(ssp);
+}
+
+static void sifive_serial_stop_rx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+}
+
+static void sifive_serial_start_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_txwm(ssp);
+}
+
+static irqreturn_t sifive_serial_irq(int irq, void *dev_id)
+{
+	struct sifive_serial_port *ssp = dev_id;
+	u32 ip;
+
+	spin_lock(&ssp->port.lock);
+
+	ip = __ssp_readl(ssp, SIFIVE_SERIAL_IP_OFFS);
+	if (!ip) {
+		spin_unlock(&ssp->port.lock);
+		return IRQ_NONE;
+	}
+
+	if (ip & SIFIVE_SERIAL_IP_RXWM_MASK)
+		__ssp_receive_chars(ssp);
+	if (ip & SIFIVE_SERIAL_IP_TXWM_MASK)
+		__ssp_transmit_chars(ssp);
+
+	spin_unlock(&ssp->port.lock);
+
+	return IRQ_HANDLED;
+}
+
+static unsigned int sifive_serial_tx_empty(struct uart_port *port)
+{
+	return TIOCSER_TEMT;
+}
+
+static unsigned int sifive_serial_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
+}
+
+static void sifive_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* IP block does not support these signals */
+}
+
+static void sifive_serial_break_ctl(struct uart_port *port, int break_state)
+{
+	/* IP block does not support sending a break */
+}
+
+static int sifive_serial_startup(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_rxwm(ssp);
+
+	return 0;
+}
+
+static void sifive_serial_shutdown(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+	__ssp_disable_txwm(ssp);
+}
+
+/**
+ * sifive_serial_clk_notifier() - clock post-rate-change notifier
+ * @nb: pointer to the struct notifier_block, from the notifier code
+ * @event: event mask from the notifier code
+ * @data: pointer to the struct clk_notifier_data from the notifier code
+ *
+ * On the V0 SoC, the UART IP block is derived from the CPU clock source
+ * after a synchronous divide-by-two divider, so any CPU clock rate change
+ * requires the UART baud rate to be updated.  This presumably could corrupt any
+ * serial word currently being transmitted or received.  It would probably
+ * be better to stop receives and transmits, then complete the baud rate
+ * change, then re-enable them.
+ */
+static int sifive_serial_clk_notifier(struct notifier_block *nb,
+				      unsigned long event, void *data)
+{
+	struct clk_notifier_data *cnd = data;
+	struct sifive_serial_port *ssp = notifier_to_sifive_serial_port(nb);
+
+	if (event == POST_RATE_CHANGE && ssp->clkin_rate != cnd->new_rate) {
+		ssp->clkin_rate = cnd->new_rate;
+		__ssp_update_div(ssp);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void sifive_serial_set_termios(struct uart_port *port,
+				      struct ktermios *termios,
+				      struct ktermios *old)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+	unsigned long flags;
+	u32 v, old_v;
+	int rate;
+	char nstop;
+
+	if ((termios->c_cflag & CSIZE) != CS8) {
+		dev_err(ssp->port.dev, "only 8-bit words supported\n");
+		return;
+	}
+
+	/* Set number of stop bits */
+	nstop = (termios->c_cflag & CSTOPB) ? 2 : 1;
+	__ssp_set_stop_bits(ssp, nstop);
+
+	/* Set line rate */
+	rate = uart_get_baud_rate(port, termios, old, 0, ssp->clkin_rate / 16);
+	__ssp_update_baud_rate(ssp, rate);
+
+	spin_lock_irqsave(&ssp->port.lock, flags);
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, rate);
+
+	ssp->port.read_status_mask = 0;
+	if (termios->c_iflag & INPCK) {
+		dev_err(ssp->port.dev, "INPCK flag not supported\n");
+		goto ssst_out;
+	}
+	if (termios->c_iflag & (BRKINT | PARMRK)) {
+		dev_err(ssp->port.dev, "BRKINT/PARMRK flag not supported\n");
+		goto ssst_out;
+	}
+
+	/*
+	 * ignore all characters if CREAD is not set
+	 */
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXCTRL_OFFS);
+	old_v = v;
+	if ((termios->c_cflag & CREAD) == 0)
+		v &= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	else
+		v |= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	if (v != old_v)
+		__ssp_writel(v, SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+ssst_out:
+	spin_unlock_irqrestore(&ssp->port.lock, flags);
+}
+
+static void sifive_serial_release_port(struct uart_port *port)
+{
+}
+
+static int sifive_serial_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+static void sifive_serial_config_port(struct uart_port *port, int flags)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	ssp->port.type = PORT_SIFIVE_V0;
+}
+
+static int sifive_serial_verify_port(struct uart_port *port,
+				     struct serial_struct *ser)
+{
+	return -EINVAL;
+}
+
+static const char *sifive_serial_type(struct uart_port *port)
+{
+	return port->type == PORT_SIFIVE_V0 ? "SiFive UART v0" : NULL;
+}
+
+/*
+ * Early console support
+ */
+
+#ifdef CONFIG_SERIAL_EARLYCON
+static void early_sifive_serial_putc(struct uart_port *port, int c)
+{
+	while (__ssp_early_readl(port, SIFIVE_SERIAL_TXDATA_OFFS) &
+	       SIFIVE_SERIAL_TXDATA_FULL_MASK)
+		cpu_relax();
+
+	__ssp_early_writel(c, SIFIVE_SERIAL_TXDATA_OFFS, port);
+}
+
+static void early_sifive_serial_write(struct console *con, const char *s,
+				      unsigned int n)
+{
+	struct earlycon_device *dev = con->data;
+	struct uart_port *port = &dev->port;
+
+	uart_console_write(port, s, n, early_sifive_serial_putc);
+}
+
+static int __init early_sifive_serial_setup(struct earlycon_device *dev,
+					    const char *options)
+{
+	struct uart_port *port = &dev->port;
+
+	if (!port->membase)
+		return -ENODEV;
+
+	dev->con->write = early_sifive_serial_write;
+
+	return 0;
+}
+
+OF_EARLYCON_DECLARE(sifive, "sifive,uart0", early_sifive_serial_setup);
+OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
+		    early_sifive_serial_setup);
+#endif /* CONFIG_SERIAL_EARLYCON */
+
+/*
+ * Linux console interface
+ */
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+
+static struct sifive_serial_port *sifive_serial_console_ports[SIFIVE_SERIAL_MAX_PORTS];
+
+static void sifive_serial_console_putchar(struct uart_port *port, int ch)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_wait_for_xmitr(ssp);
+	__ssp_transmit_char(ssp, ch);
+}
+
+static void sifive_serial_console_write(struct console *co, const char *s,
+					unsigned int count)
+{
+	struct sifive_serial_port *ssp = sifive_serial_console_ports[co->index];
+	unsigned long flags;
+	unsigned int ier;
+	int locked = 1;
+
+	if (!ssp)
+		return;
+
+	local_irq_save(flags);
+	if (ssp->port.sysrq)
+		locked = 0;
+	else if (oops_in_progress)
+		locked = spin_trylock(&ssp->port.lock);
+	else
+		spin_lock(&ssp->port.lock);
+
+	ier = __ssp_readl(ssp, SIFIVE_SERIAL_IE_OFFS);
+	__ssp_writel(0, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	uart_console_write(&ssp->port, s, count, sifive_serial_console_putchar);
+
+	__ssp_writel(ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	if (locked)
+		spin_unlock(&ssp->port.lock);
+	local_irq_restore(flags);
+}
+
+static int __init sifive_serial_console_setup(struct console *co, char *options)
+{
+	struct sifive_serial_port *ssp;
+	int baud = SIFIVE_DEFAULT_BAUD_RATE;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (co->index < 0 || co->index >= SIFIVE_SERIAL_MAX_PORTS)
+		return -ENODEV;
+
+	ssp = sifive_serial_console_ports[co->index];
+	if (!ssp)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	return uart_set_options(&ssp->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sifive_serial_uart_driver;
+
+static struct console sifive_serial_console = {
+	.name		= SIFIVE_TTY_PREFIX,
+	.write		= sifive_serial_console_write,
+	.device		= uart_console_device,
+	.setup		= sifive_serial_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &sifive_serial_uart_driver,
+};
+
+static int __init sifive_console_init(void)
+{
+	register_console(&sifive_serial_console);
+	return 0;
+}
+
+console_initcall(sifive_console_init);
+
+static void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = ssp;
+}
+
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = 0;
+}
+
+#define SIFIVE_SERIAL_CONSOLE	(&sifive_serial_console)
+
+#else
+
+#define SIFIVE_SERIAL_CONSOLE	NULL
+
+static inline void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{}
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{}
+
+#endif
+
+static const struct uart_ops sifive_serial_uops = {
+	.tx_empty	= sifive_serial_tx_empty,
+	.set_mctrl	= sifive_serial_set_mctrl,
+	.get_mctrl	= sifive_serial_get_mctrl,
+	.stop_tx	= sifive_serial_stop_tx,
+	.start_tx	= sifive_serial_start_tx,
+	.stop_rx	= sifive_serial_stop_rx,
+	.break_ctl	= sifive_serial_break_ctl,
+	.startup	= sifive_serial_startup,
+	.shutdown	= sifive_serial_shutdown,
+	.set_termios	= sifive_serial_set_termios,
+	.type		= sifive_serial_type,
+	.release_port	= sifive_serial_release_port,
+	.request_port	= sifive_serial_request_port,
+	.config_port	= sifive_serial_config_port,
+	.verify_port	= sifive_serial_verify_port,
+};
+
+static struct uart_driver sifive_serial_uart_driver = {
+	.owner		= THIS_MODULE,
+	.driver_name	= SIFIVE_SERIAL_NAME,
+	.dev_name	= SIFIVE_TTY_PREFIX,
+	.nr		= SIFIVE_SERIAL_MAX_PORTS,
+	.cons		= SIFIVE_SERIAL_CONSOLE,
+};
+
+static int sifive_serial_probe(struct platform_device *pdev)
+{
+	struct sifive_serial_port *ssp;
+	struct resource *mem;
+	struct clk *clk;
+	void __iomem *base;
+	int irq, id, r;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "could not acquire interrupt\n");
+		return -EPROBE_DEFER;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(base)) {
+		dev_err(&pdev->dev, "could not acquire device memory\n");
+		return PTR_ERR(base);
+	}
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "unable to find controller clock\n");
+		return PTR_ERR(clk);
+	}
+
+	id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (id < 0) {
+		dev_err(&pdev->dev, "missing aliases entry\n");
+		return id;
+	}
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+	if (id > SIFIVE_SERIAL_MAX_PORTS) {
+		dev_err(&pdev->dev, "too many UARTs (%d)\n", id);
+		return -EINVAL;
+	}
+#endif
+
+	ssp = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
+	if (!ssp)
+		return -ENOMEM;
+
+	ssp->port.dev = &pdev->dev;
+	ssp->port.type = PORT_SIFIVE_V0;
+	ssp->port.iotype = UPIO_MEM;
+	ssp->port.irq = irq;
+	ssp->port.fifosize = SIFIVE_TX_FIFO_DEPTH;
+	ssp->port.ops = &sifive_serial_uops;
+	ssp->port.line = id;
+	ssp->port.mapbase = mem->start;
+	ssp->port.membase = base;
+	ssp->dev = &pdev->dev;
+	ssp->clk = clk;
+	ssp->clk_notifier.notifier_call = sifive_serial_clk_notifier;
+
+	r = clk_notifier_register(ssp->clk, &ssp->clk_notifier);
+	if (r) {
+		dev_err(&pdev->dev, "could not register clock notifier: %d\n",
+			r);
+		goto probe_out1;
+	}
+
+	/* Set up clock divider */
+	ssp->clkin_rate = clk_get_rate(ssp->clk);
+	ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE;
+	__ssp_update_div(ssp);
+
+	platform_set_drvdata(pdev, ssp);
+
+	/* Enable transmits and set the watermark level to 1 */
+	__ssp_writel((1 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT) |
+		     SIFIVE_SERIAL_TXCTRL_TXEN_MASK,
+		     SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+
+	/* Enable receives and set the watermark level to 0 */
+	__ssp_writel((0 << SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT) |
+		     SIFIVE_SERIAL_RXCTRL_RXEN_MASK,
+		     SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+	r = request_irq(ssp->port.irq, sifive_serial_irq, ssp->port.irqflags,
+			dev_name(&pdev->dev), ssp);
+	if (r) {
+		dev_err(&pdev->dev, "could not attach interrupt: %d\n", r);
+		goto probe_out2;
+	}
+
+	__ssp_add_console_port(ssp);
+
+	r = uart_add_one_port(&sifive_serial_uart_driver, &ssp->port);
+	if (r != 0) {
+		dev_err(&pdev->dev, "could not add uart: %d\n", r);
+		goto probe_out3;
+	}
+
+	return 0;
+
+probe_out3:
+	__ssp_remove_console_port(ssp);
+	free_irq(ssp->port.irq, ssp);
+probe_out2:
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+probe_out1:
+	return r;
+}
+
+static int sifive_serial_remove(struct platform_device *dev)
+{
+	struct sifive_serial_port *ssp = platform_get_drvdata(dev);
+
+	__ssp_remove_console_port(ssp);
+	uart_remove_one_port(&sifive_serial_uart_driver, &ssp->port);
+	free_irq(ssp->port.irq, ssp);
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+
+	return 0;
+}
+
+static const struct of_device_id sifive_serial_of_match[] = {
+	{ .compatible = "sifive,fu540-c000-uart0" },
+	{ .compatible = "sifive,uart0" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sifive_serial_match);
+
+static struct platform_driver sifive_serial_platform_driver = {
+	.probe		= sifive_serial_probe,
+	.remove		= sifive_serial_remove,
+	.driver		= {
+		.name	= SIFIVE_SERIAL_NAME,
+		.of_match_table = of_match_ptr(sifive_serial_of_match),
+	},
+};
+
+static int __init sifive_serial_init(void)
+{
+	int r;
+
+	r = uart_register_driver(&sifive_serial_uart_driver);
+	if (r)
+		goto init_out1;
+
+	r = platform_driver_register(&sifive_serial_platform_driver);
+	if (r)
+		goto init_out2;
+
+	return 0;
+
+init_out2:
+	uart_unregister_driver(&sifive_serial_uart_driver);
+init_out1:
+	return r;
+}
+
+static void __exit sifive_serial_exit(void)
+{
+	platform_driver_unregister(&sifive_serial_platform_driver);
+	uart_unregister_driver(&sifive_serial_uart_driver);
+}
+
+module_init(sifive_serial_init);
+module_exit(sifive_serial_exit);
+
+MODULE_DESCRIPTION("SiFive UART serial driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Paul Walmsley <paul@pwsan.com>");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index dce5f9dae121..569d5172d23c 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -281,4 +281,7 @@
 /* MediaTek BTIF */
 #define PORT_MTK_BTIF	117
 
+/* SiFive UART */
+#define PORT_SIFIVE_V0	118
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
2.19.1


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

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

[Rob Herring]

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? 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. Palmer mentioned the
compatible string is part of the IP block repository? Where does the
number come from? What's the next version? Major vs. minor versions?
ECO fixes? Is the version s/w readable? How do you ensure it gets
updated? All that should be addressed.

Otherwise, don't do version numbers because we have no visibility to
what they mean.

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

> +- interrupts: Should contain the UART interrupt identifier
> +- clocks: Should contain a clock identifier for the UART's parent clock
> +
> +
> +Example:
> +
> +uart0: serial at 10010000 {
> +       compatible = "sifive,uart0";
> +       interrupt-parent = <&plic0>;
> +       interrupts = <80>;
> +       reg = <0x0 0x10010000 0x0 0x1000>;
> +       clocks = <&prci PRCI_CLK_TLCLK>;
> +};
> --
> 2.19.1
>

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

[Rob Herring]

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? 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. Palmer mentioned the
compatible string is part of the IP block repository? Where does the
number come from? What's the next version? Major vs. minor versions?
ECO fixes? Is the version s/w readable? How do you ensure it gets
updated? All that should be addressed.

Otherwise, don't do version numbers because we have no visibility to
what they mean.

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

> +- interrupts: Should contain the UART interrupt identifier
> +- clocks: Should contain a clock identifier for the UART's parent clock
> +
> +
> +Example:
> +
> +uart0: serial@10010000 {
> +       compatible = "sifive,uart0";
> +       interrupt-parent = <&plic0>;
> +       interrupts = <80>;
> +       reg = <0x0 0x10010000 0x0 0x1000>;
> +       clocks = <&prci PRCI_CLK_TLCLK>;
> +};
> --
> 2.19.1
>

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

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

[Paul Walmsley]

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

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

[Paul Walmsley]

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

[PATCH v2 2/2] tty: serial: add driver for the SiFive UART

[kbuild test robot]

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.19-rc8 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Walmsley/tty-serial-add-DT-bindings-and-serial-driver-for-the-SiFive-FU540-UART/20181020-071123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   In file included from drivers/tty/serial/sifive.c:47:0:
>> drivers/tty/serial/sifive.c:1025:25: error: 'sifive_serial_match' undeclared here (not in a function); did you mean 'sifive_serial_of_match'?
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
                            ^
   include/linux/module.h:213:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                  ^~~~
>> include/linux/module.h:213:21: error: '__mod_of__sifive_serial_match_device_table' aliased to undefined symbol 'sifive_serial_match'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                        ^
   drivers/tty/serial/sifive.c:1025:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
    ^~~~~~~~~~~~~~~~~~~
--
   In file included from drivers/tty//serial/sifive.c:47:0:
   drivers/tty//serial/sifive.c:1025:25: error: 'sifive_serial_match' undeclared here (not in a function); did you mean 'sifive_serial_of_match'?
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
                            ^
   include/linux/module.h:213:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                  ^~~~
>> include/linux/module.h:213:21: error: '__mod_of__sifive_serial_match_device_table' aliased to undefined symbol 'sifive_serial_match'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                        ^
   drivers/tty//serial/sifive.c:1025:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
    ^~~~~~~~~~~~~~~~~~~

vim +1025 drivers/tty/serial/sifive.c

  1019	
  1020	static const struct of_device_id sifive_serial_of_match[] = {
  1021		{ .compatible = "sifive,fu540-c000-uart0" },
  1022		{ .compatible = "sifive,uart0" },
  1023		{},
  1024	};
> 1025	MODULE_DEVICE_TABLE(of, sifive_serial_match);
  1026	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 57462 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20181020/e8fb1852/attachment.gz>

Re: [PATCH v2 2/2] tty: serial: add driver for the SiFive UART

[kbuild test robot]

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

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.19-rc8 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Walmsley/tty-serial-add-DT-bindings-and-serial-driver-for-the-SiFive-FU540-UART/20181020-071123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   In file included from drivers/tty/serial/sifive.c:47:0:
>> drivers/tty/serial/sifive.c:1025:25: error: 'sifive_serial_match' undeclared here (not in a function); did you mean 'sifive_serial_of_match'?
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
                            ^
   include/linux/module.h:213:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                  ^~~~
>> include/linux/module.h:213:21: error: '__mod_of__sifive_serial_match_device_table' aliased to undefined symbol 'sifive_serial_match'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                        ^
   drivers/tty/serial/sifive.c:1025:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
    ^~~~~~~~~~~~~~~~~~~
--
   In file included from drivers/tty//serial/sifive.c:47:0:
   drivers/tty//serial/sifive.c:1025:25: error: 'sifive_serial_match' undeclared here (not in a function); did you mean 'sifive_serial_of_match'?
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
                            ^
   include/linux/module.h:213:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                  ^~~~
>> include/linux/module.h:213:21: error: '__mod_of__sifive_serial_match_device_table' aliased to undefined symbol 'sifive_serial_match'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                        ^
   drivers/tty//serial/sifive.c:1025:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
    ^~~~~~~~~~~~~~~~~~~

vim +1025 drivers/tty/serial/sifive.c

  1019	
  1020	static const struct of_device_id sifive_serial_of_match[] = {
  1021		{ .compatible = "sifive,fu540-c000-uart0" },
  1022		{ .compatible = "sifive,uart0" },
  1023		{},
  1024	};
> 1025	MODULE_DEVICE_TABLE(of, sifive_serial_match);
  1026	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57462 bytes --]

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

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

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

[Rob Herring]

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

Optional? Well, that's pointless to have then.

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

Then you should probably just follow normal conventions.

I don't think DT in the h/w is the best strategy anyways. Ideally,
what the h/w should have are version and capabilities (assuming there
are configuration options) registers which aren't optional and can't
be forgotten to be updated. The version should probably have a vendor
too.

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

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

I'm not going to go read your RTL, sorry.

Rob

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

[Rob Herring]

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

Optional? Well, that's pointless to have then.

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

Then you should probably just follow normal conventions.

I don't think DT in the h/w is the best strategy anyways. Ideally,
what the h/w should have are version and capabilities (assuming there
are configuration options) registers which aren't optional and can't
be forgotten to be updated. The version should probably have a vendor
too.

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

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

I'm not going to go read your RTL, sorry.

Rob

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

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

[Palmer Dabbelt]

On Fri, 19 Oct 2018 13:45:57 PDT (-0700), robh+dt at kernel.org 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? 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. Palmer mentioned the
> compatible string is part of the IP block repository? Where does the
> number come from? What's the next version? Major vs. minor versions?
> ECO fixes? Is the version s/w readable? How do you ensure it gets
> updated? All that should be addressed.

The RISC-V ecosystem is a bit different than that of ARM, MIPS, or Intel in 
that the ISA is an royalty-free open standard that anyone can implement (ie, 
without even signing a license agreement), with only the "RISC-V" trademark 
being held behind a pay+conformance wall.  As a result, we don't actually have 
any control over who builds a RISC-V chip so all we at SiFive can really to is 
try to demonstrate good practices in software land and go from there.

As far as SiFive's codebase is concerned, the version number is embedded in the 
RTL generator, and a device tree is generated along with the RTL.  This device 
tree is then embedded into a mask ROM on the chip, which allows the earliest 
stage of boot to proceed.  As I'm sure you know, boot is a very complicated 
process and as a result the device tree passed to Linux doesn't necessarily 
look like what's in the ROM, but the intent is to keep iterating until we can 
get these as similar as possible -- that's why we're submitting every 
devicetree binding to the standard.

Specifically as far as the UART is concerned, the compat string that's not 
chip-specific lives here (the "sifive,fu540-c000-uart" string lives in an 
internal chip repo that I can't point to):

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

The version numbering scheme right now is pretty simple: I try to pay as much 
attention as possible to how the hardware changes (both by looking and with 
some automation), and I go yell at anyone who does something stupid.  I know 
it's not the most scalable of schemes, but it's the best we have.  The UART is 
actually an interesting case right now because we have an outstanding pull 
request that adds a bit to the UART and then adds "sifive,uart1" to the compat 
string

    https://github.com/sifive/sifive-blocks/pull/90

My intent is to ensure that the device tree's compat string uniquely identifies 
the software interface to a block.  Thus, whenever a device's implementation 
changes in a software-visible way (bug fix or feature addition) we change the 
compat string -- either adding one (as is the case of the UART, where the 
compat string will be both "sifive,uart1" and "sifive,uart0" since the new 
feature is backwards compatible with the old software) or changing one (if the 
interface change is not compatible with old software).

Like I said above, this is all a manual process right now and this only applies 
to SiFive's implementations.  I'm confident that I can at least ensure that, 
for any given SiFive implementation, a block's compat string will uniquely 
identify the software interface to it.  For the rest of the RISC-V world all we 
can do is set a good example and review the software.

> Otherwise, don't do version numbers because we have no visibility to
> what they mean.
>
>> +- 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.
>
>> +- interrupts: Should contain the UART interrupt identifier
>> +- clocks: Should contain a clock identifier for the UART's parent clock
>> +
>> +
>> +Example:
>> +
>> +uart0: serial at 10010000 {
>> +       compatible = "sifive,uart0";
>> +       interrupt-parent = <&plic0>;
>> +       interrupts = <80>;
>> +       reg = <0x0 0x10010000 0x0 0x1000>;
>> +       clocks = <&prci PRCI_CLK_TLCLK>;
>> +};
>> --
>> 2.19.1
>>

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

[Palmer Dabbelt]

On Fri, 19 Oct 2018 13:45:57 PDT (-0700), robh+dt@kernel.org 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? 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. Palmer mentioned the
> compatible string is part of the IP block repository? Where does the
> number come from? What's the next version? Major vs. minor versions?
> ECO fixes? Is the version s/w readable? How do you ensure it gets
> updated? All that should be addressed.

The RISC-V ecosystem is a bit different than that of ARM, MIPS, or Intel in 
that the ISA is an royalty-free open standard that anyone can implement (ie, 
without even signing a license agreement), with only the "RISC-V" trademark 
being held behind a pay+conformance wall.  As a result, we don't actually have 
any control over who builds a RISC-V chip so all we at SiFive can really to is 
try to demonstrate good practices in software land and go from there.

As far as SiFive's codebase is concerned, the version number is embedded in the 
RTL generator, and a device tree is generated along with the RTL.  This device 
tree is then embedded into a mask ROM on the chip, which allows the earliest 
stage of boot to proceed.  As I'm sure you know, boot is a very complicated 
process and as a result the device tree passed to Linux doesn't necessarily 
look like what's in the ROM, but the intent is to keep iterating until we can 
get these as similar as possible -- that's why we're submitting every 
devicetree binding to the standard.

Specifically as far as the UART is concerned, the compat string that's not 
chip-specific lives here (the "sifive,fu540-c000-uart" string lives in an 
internal chip repo that I can't point to):

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

The version numbering scheme right now is pretty simple: I try to pay as much 
attention as possible to how the hardware changes (both by looking and with 
some automation), and I go yell at anyone who does something stupid.  I know 
it's not the most scalable of schemes, but it's the best we have.  The UART is 
actually an interesting case right now because we have an outstanding pull 
request that adds a bit to the UART and then adds "sifive,uart1" to the compat 
string

    https://github.com/sifive/sifive-blocks/pull/90

My intent is to ensure that the device tree's compat string uniquely identifies 
the software interface to a block.  Thus, whenever a device's implementation 
changes in a software-visible way (bug fix or feature addition) we change the 
compat string -- either adding one (as is the case of the UART, where the 
compat string will be both "sifive,uart1" and "sifive,uart0" since the new 
feature is backwards compatible with the old software) or changing one (if the 
interface change is not compatible with old software).

Like I said above, this is all a manual process right now and this only applies 
to SiFive's implementations.  I'm confident that I can at least ensure that, 
for any given SiFive implementation, a block's compat string will uniquely 
identify the software interface to it.  For the rest of the RISC-V world all we 
can do is set a good example and review the software.

> Otherwise, don't do version numbers because we have no visibility to
> what they mean.
>
>> +- 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.
>
>> +- interrupts: Should contain the UART interrupt identifier
>> +- clocks: Should contain a clock identifier for the UART's parent clock
>> +
>> +
>> +Example:
>> +
>> +uart0: serial@10010000 {
>> +       compatible = "sifive,uart0";
>> +       interrupt-parent = <&plic0>;
>> +       interrupts = <80>;
>> +       reg = <0x0 0x10010000 0x0 0x1000>;
>> +       clocks = <&prci PRCI_CLK_TLCLK>;
>> +};
>> --
>> 2.19.1
>>

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

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

[Paul Walmsley]

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

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.


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.


But for IP block-specific version strings like "sifive,uart0", I think 
we can address your concern, at least for these public IP blocks. Since 
the SiFive UART and some other peripheral IP blocks are open-source, the 
public can have a pretty good idea of what DT version number corresponds 
to the source RTL, since the RTL is public. ? The version number 
identifies a specific programming model, without tying that programming 
model to any SoC-specific workarounds, etc.? So for these cases, I think 
there's a pretty good case for having IP block-specific version numbers 
in DT compatible strings, and I hope you'll agree.


The advantage for all of us is that there's then no need to embed 
chip-specific DT match strings in these drivers, for the most part.? We 
just match on "sifive,uart0" and that's it, assuming no chip-specific 
workarounds are needed.


>>>    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
> I'm not going to go read your RTL, sorry.


There's no need, but you did ask where it came from.? Sorry you didn't 
like the answer.


Please let us know what you want us to do.


Thanks for your review


- Paul


** The caveat is that even with SoC identifiers in the Linux DT 
compatible strings, there's not enough information in many of the 
existing kernel DT compatibility strings to uniquely identify chip 
versions.? Taking OMAP and Tegra as examples, there are several 
different chip versions for a given SoC generation that came out of the 
fab.? ? OMAP chip version strings usually began with "ES"; Tegra version 
numbers, as I recall, were a letter and two numbers.? For the most part, 
those versions were never specifically identified in the upstream kernel 
DT strings or in DT file names. (There are some exceptions with OMAP 
where we did identify specific chip version numbers, because sizable 
numbers of folks had boards with early silicon, and we were committed to 
supporting them at the time.)??? Sadly even adding these additional chip 
version identifiers to the DT strings wouldn't be perfect: I've seen at 
least one large vendor implementing metal-only ECOs without incrementing 
public chip version numbers. The point here is that we're already not 
uniquely identifying IP blocks with our current Linux DT compatibility 
string scheme.

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

[Paul Walmsley]

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.

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.


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.


But for IP block-specific version strings like "sifive,uart0", I think 
we can address your concern, at least for these public IP blocks. Since 
the SiFive UART and some other peripheral IP blocks are open-source, the 
public can have a pretty good idea of what DT version number corresponds 
to the source RTL, since the RTL is public.   The version number 
identifies a specific programming model, without tying that programming 
model to any SoC-specific workarounds, etc.  So for these cases, I think 
there's a pretty good case for having IP block-specific version numbers 
in DT compatible strings, and I hope you'll agree.


The advantage for all of us is that there's then no need to embed 
chip-specific DT match strings in these drivers, for the most part.  We 
just match on "sifive,uart0" and that's it, assuming no chip-specific 
workarounds are needed.


>>>    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
> I'm not going to go read your RTL, sorry.


There's no need, but you did ask where it came from.  Sorry you didn't 
like the answer.


Please let us know what you want us to do.


Thanks for your review


- Paul


** The caveat is that even with SoC identifiers in the Linux DT 
compatible strings, there's not enough information in many of the 
existing kernel DT compatibility strings to uniquely identify chip 
versions.  Taking OMAP and Tegra as examples, there are several 
different chip versions for a given SoC generation that came out of the 
fab.    OMAP chip version strings usually began with "ES"; Tegra version 
numbers, as I recall, were a letter and two numbers.  For the most part, 
those versions were never specifically identified in the upstream kernel 
DT strings or in DT file names. (There are some exceptions with OMAP 
where we did identify specific chip version numbers, because sizable 
numbers of folks had boards with early silicon, and we were committed to 
supporting them at the time.)    Sadly even adding these additional chip 
version identifiers to the DT strings wouldn't be perfect: I've seen at 
least one large vendor implementing metal-only ECOs without incrementing 
public chip version numbers. The point here is that we're already not 
uniquely identifying IP blocks with our current Linux DT compatibility 
string scheme.


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

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

[Rob Herring]

On Mon, Oct 22, 2018 at 09:41:51AM -0700, Palmer Dabbelt wrote:
> On Fri, 19 Oct 2018 13:45:57 PDT (-0700), robh+dt at kernel.org 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? 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. Palmer mentioned the
> > compatible string is part of the IP block repository? Where does the
> > number come from? What's the next version? Major vs. minor versions?
> > ECO fixes? Is the version s/w readable? How do you ensure it gets
> > updated? All that should be addressed.
> 
> The RISC-V ecosystem is a bit different than that of ARM, MIPS, or Intel in
> that the ISA is an royalty-free open standard that anyone can implement (ie,
> without even signing a license agreement), with only the "RISC-V" trademark
> being held behind a pay+conformance wall.  As a result, we don't actually
> have any control over who builds a RISC-V chip so all we at SiFive can
> really to is try to demonstrate good practices in software land and go from
> there.

Rights to the ISA and cores may be different, but how chips are built 
is not really all that different (or doesn't have to be).

> As far as SiFive's codebase is concerned, the version number is embedded in
> the RTL generator, and a device tree is generated along with the RTL.  This
> device tree is then embedded into a mask ROM on the chip, which allows the
> earliest stage of boot to proceed.  As I'm sure you know, boot is a very
> complicated process and as a result the device tree passed to Linux doesn't
> necessarily look like what's in the ROM, but the intent is to keep iterating
> until we can get these as similar as possible -- that's why we're submitting
> every devicetree binding to the standard.

So all this discussion is purely SiFive specific and really has nothing 
to do with RISC-V ecosystem.

Putting the DT into the ROM isn't something I'd do. It's simply not 
going to work timeline wise IMO.

> Specifically as far as the UART is concerned, the compat string that's not
> chip-specific lives here (the "sifive,fu540-c000-uart" string lives in an
> internal chip repo that I can't point to):
> 
>    https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43
> 
> The version numbering scheme right now is pretty simple: I try to pay as
> much attention as possible to how the hardware changes (both by looking and
> with some automation), and I go yell at anyone who does something stupid.  I
> know it's not the most scalable of schemes, but it's the best we have.  The
> UART is actually an interesting case right now because we have an
> outstanding pull request that adds a bit to the UART and then adds
> "sifive,uart1" to the compat string
> 
>    https://github.com/sifive/sifive-blocks/pull/90

Relying on people to catch whether changes are important or not is bound 
to fail. It's really got to be built into the design flow.

Even just updating a version register I've experienced the h/w designers 
forgetting to update it.

> My intent is to ensure that the device tree's compat string uniquely
> identifies the software interface to a block.  Thus, whenever a device's
> implementation changes in a software-visible way (bug fix or feature
> addition) we change the compat string -- either adding one (as is the case
> of the UART, where the compat string will be both "sifive,uart1" and
> "sifive,uart0" since the new feature is backwards compatible with the old
> software) or changing one (if the interface change is not compatible with
> old software).

What about config options? Say the UART has a configurable FIFO size.

What about major vs. minor version changes? Respins of chips would need 
to make minor changes if picking up major changes are deemed too risky.

> Like I said above, this is all a manual process right now and this only
> applies to SiFive's implementations.  I'm confident that I can at least
> ensure that, for any given SiFive implementation, a block's compat string
> will uniquely identify the software interface to it.  For the rest of the
> RISC-V world all we can do is set a good example and review the software.

This is all good information and is essentially what I'm looking for. I 
just don't want it lost in a reply to an email, but something you can 
reference. Look at bindings/arm/primecell.txt for example. That 
describes a family of IP blocks and not any specific device.

Whether the versioning is sufficient or not, I don't really care as long 
as you docuemnt what it is so it is consistent. Since you have a common 
schema across IP blocks, that means you should have a common document.

Rob

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

[Rob Herring]

On Mon, Oct 22, 2018 at 09:41:51AM -0700, Palmer Dabbelt wrote:
> On Fri, 19 Oct 2018 13:45:57 PDT (-0700), robh+dt@kernel.org 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? 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. Palmer mentioned the
> > compatible string is part of the IP block repository? Where does the
> > number come from? What's the next version? Major vs. minor versions?
> > ECO fixes? Is the version s/w readable? How do you ensure it gets
> > updated? All that should be addressed.
> 
> The RISC-V ecosystem is a bit different than that of ARM, MIPS, or Intel in
> that the ISA is an royalty-free open standard that anyone can implement (ie,
> without even signing a license agreement), with only the "RISC-V" trademark
> being held behind a pay+conformance wall.  As a result, we don't actually
> have any control over who builds a RISC-V chip so all we at SiFive can
> really to is try to demonstrate good practices in software land and go from
> there.

Rights to the ISA and cores may be different, but how chips are built 
is not really all that different (or doesn't have to be).

> As far as SiFive's codebase is concerned, the version number is embedded in
> the RTL generator, and a device tree is generated along with the RTL.  This
> device tree is then embedded into a mask ROM on the chip, which allows the
> earliest stage of boot to proceed.  As I'm sure you know, boot is a very
> complicated process and as a result the device tree passed to Linux doesn't
> necessarily look like what's in the ROM, but the intent is to keep iterating
> until we can get these as similar as possible -- that's why we're submitting
> every devicetree binding to the standard.

So all this discussion is purely SiFive specific and really has nothing 
to do with RISC-V ecosystem.

Putting the DT into the ROM isn't something I'd do. It's simply not 
going to work timeline wise IMO.

> Specifically as far as the UART is concerned, the compat string that's not
> chip-specific lives here (the "sifive,fu540-c000-uart" string lives in an
> internal chip repo that I can't point to):
> 
>    https://github.com/sifive/sifive-blocks/blob/master/src/main/scala/devices/uart/UART.scala#L43
> 
> The version numbering scheme right now is pretty simple: I try to pay as
> much attention as possible to how the hardware changes (both by looking and
> with some automation), and I go yell at anyone who does something stupid.  I
> know it's not the most scalable of schemes, but it's the best we have.  The
> UART is actually an interesting case right now because we have an
> outstanding pull request that adds a bit to the UART and then adds
> "sifive,uart1" to the compat string
> 
>    https://github.com/sifive/sifive-blocks/pull/90

Relying on people to catch whether changes are important or not is bound 
to fail. It's really got to be built into the design flow.

Even just updating a version register I've experienced the h/w designers 
forgetting to update it.

> My intent is to ensure that the device tree's compat string uniquely
> identifies the software interface to a block.  Thus, whenever a device's
> implementation changes in a software-visible way (bug fix or feature
> addition) we change the compat string -- either adding one (as is the case
> of the UART, where the compat string will be both "sifive,uart1" and
> "sifive,uart0" since the new feature is backwards compatible with the old
> software) or changing one (if the interface change is not compatible with
> old software).

What about config options? Say the UART has a configurable FIFO size.

What about major vs. minor version changes? Respins of chips would need 
to make minor changes if picking up major changes are deemed too risky.

> Like I said above, this is all a manual process right now and this only
> applies to SiFive's implementations.  I'm confident that I can at least
> ensure that, for any given SiFive implementation, a block's compat string
> will uniquely identify the software interface to it.  For the rest of the
> RISC-V world all we can do is set a good example and review the software.

This is all good information and is essentially what I'm looking for. I 
just don't want it lost in a reply to an email, but something you can 
reference. Look at bindings/arm/primecell.txt for example. That 
describes a family of IP blocks and not any specific device.

Whether the versioning is sufficient or not, I don't really care as long 
as you docuemnt what it is so it is consistent. Since you have a common 
schema across IP blocks, that means you should have a common document.

Rob

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

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

[Rob Herring]

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

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

> But for IP block-specific version strings like "sifive,uart0", I think we
> can address your concern, at least for these public IP blocks. Since the
> SiFive UART and some other peripheral IP blocks are open-source, the public
> can have a pretty good idea of what DT version number corresponds to the
> source RTL, since the RTL is public. ? The version number identifies a
> specific programming model, without tying that programming model to any
> SoC-specific workarounds, etc.? So for these cases, I think there's a pretty
> good case for having IP block-specific version numbers in DT compatible
> strings, and I hope you'll agree.
> 
> 
> The advantage for all of us is that there's then no need to embed
> chip-specific DT match strings in these drivers, for the most part.? We just
> match on "sifive,uart0" and that's it, assuming no chip-specific workarounds
> are needed.
> 
> 
> > > >    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
> > I'm not going to go read your RTL, sorry.
> 
> 
> There's no need, but you did ask where it came from.? Sorry you didn't like
> the answer.

I only meant that in context of reviewing the IP block. My questions 
were meant to be what questions should a common document answer.

> Please let us know what you want us to do.
> 
> 
> Thanks for your review
> 
> 
> - Paul
> 
> 
> ** The caveat is that even with SoC identifiers in the Linux DT compatible
> strings, there's not enough information in many of the existing kernel DT
> compatibility strings to uniquely identify chip versions.? Taking OMAP and
> Tegra as examples, there are several different chip versions for a given SoC
> generation that came out of the fab.? ? OMAP chip version strings usually
> began with "ES"; Tegra version numbers, as I recall, were a letter and two
> numbers.? For the most part, those versions were never specifically
> identified in the upstream kernel DT strings or in DT file names. (There are
> some exceptions with OMAP where we did identify specific chip version
> numbers, because sizable numbers of folks had boards with early silicon, and
> we were committed to supporting them at the time.)??? Sadly even adding
> these additional chip version identifiers to the DT strings wouldn't be
> perfect: I've seen at least one large vendor implementing metal-only ECOs
> without incrementing public chip version numbers. The point here is that
> we're already not uniquely identifying IP blocks with our current Linux DT
> compatibility string scheme.

Yes, I'm certainly aware of this aspect. We have to draw the line 
somewhere between enough information to distinguish differences and 
having a sane number of compatible strings. I mainly expect that the 1st 
versions of SoCs are short lived and ECO changes don't affect 
compatibility. That's obviously not always the case, but hopefully is 
sufficient in most cases.

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.

Rob

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

[Rob Herring]

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.

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

> But for IP block-specific version strings like "sifive,uart0", I think we
> can address your concern, at least for these public IP blocks. Since the
> SiFive UART and some other peripheral IP blocks are open-source, the public
> can have a pretty good idea of what DT version number corresponds to the
> source RTL, since the RTL is public.   The version number identifies a
> specific programming model, without tying that programming model to any
> SoC-specific workarounds, etc.  So for these cases, I think there's a pretty
> good case for having IP block-specific version numbers in DT compatible
> strings, and I hope you'll agree.
> 
> 
> The advantage for all of us is that there's then no need to embed
> chip-specific DT match strings in these drivers, for the most part.  We just
> match on "sifive,uart0" and that's it, assuming no chip-specific workarounds
> are needed.
> 
> 
> > > >    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
> > I'm not going to go read your RTL, sorry.
> 
> 
> There's no need, but you did ask where it came from.  Sorry you didn't like
> the answer.

I only meant that in context of reviewing the IP block. My questions 
were meant to be what questions should a common document answer.

> Please let us know what you want us to do.
> 
> 
> Thanks for your review
> 
> 
> - Paul
> 
> 
> ** The caveat is that even with SoC identifiers in the Linux DT compatible
> strings, there's not enough information in many of the existing kernel DT
> compatibility strings to uniquely identify chip versions.  Taking OMAP and
> Tegra as examples, there are several different chip versions for a given SoC
> generation that came out of the fab.    OMAP chip version strings usually
> began with "ES"; Tegra version numbers, as I recall, were a letter and two
> numbers.  For the most part, those versions were never specifically
> identified in the upstream kernel DT strings or in DT file names. (There are
> some exceptions with OMAP where we did identify specific chip version
> numbers, because sizable numbers of folks had boards with early silicon, and
> we were committed to supporting them at the time.)    Sadly even adding
> these additional chip version identifiers to the DT strings wouldn't be
> perfect: I've seen at least one large vendor implementing metal-only ECOs
> without incrementing public chip version numbers. The point here is that
> we're already not uniquely identifying IP blocks with our current Linux DT
> compatibility string scheme.

Yes, I'm certainly aware of this aspect. We have to draw the line 
somewhere between enough information to distinguish differences and 
having a sane number of compatible strings. I mainly expect that the 1st 
versions of SoCs are short lived and ECO changes don't affect 
compatibility. That's obviously not always the case, but hopefully is 
sufficient in most cases.

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.

Rob

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

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

[Paul Walmsley]

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

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

[Paul Walmsley]

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