This is a patch series for UART driver for Suplus SP7021 SoC.
Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
many peripherals (ex: UART. I2C, SPI, SDIO, eMMC, USB, SD card and
etc.) into a single chip. It is designed for industrial control.
Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html
Hammer Hsieh (2):
dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver
serial:sunplus-uart:Add Sunplus SoC UART Driver
.../bindings/serial/sunplus,sp7021-uart.yaml | 56 ++
MAINTAINERS | 6 +
drivers/tty/serial/Kconfig | 25 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/sunplus-uart.c | 782 +++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
6 files changed, 873 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
create mode 100644 drivers/tty/serial/sunplus-uart.c
--
2.7.4
Add Sunplus SoC UART Driver
Signed-off-by: Hammer Hsieh <[email protected]>
---
Changes in v5:
- Addressed all comments from Andy Shevchenko.
MAINTAINERS | 1 +
drivers/tty/serial/Kconfig | 25 ++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/sunplus-uart.c | 782 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
5 files changed, 812 insertions(+)
create mode 100644 drivers/tty/serial/sunplus-uart.c
diff --git a/MAINTAINERS b/MAINTAINERS
index f2ee40c..65681ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17949,6 +17949,7 @@ SUNPLUS UART DRIVER
M: Hammer Hsieh <[email protected]>
S: Maintained
F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
+F: drivers/tty/serial/sunplus-uart.c
SUPERH
M: Yoshinori Sato <[email protected]>
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 131a6a5..0865da3 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1561,6 +1561,31 @@ config SERIAL_LITEUART_CONSOLE
and warnings and which allows logins in single user mode).
Otherwise, say 'N'.
+config SERIAL_SUNPLUS
+ tristate "Sunplus UART support"
+ depends on OF || COMPILE_TEST
+ select SERIAL_CORE
+ help
+ Select this option if you would like to use Sunplus serial port on
+ Sunplus SoC SP7021.
+ If you enable this option, Sunplus serial ports in the system will
+ be registered as ttySUPx.
+ This driver can also be built as a module. If so, the module will be
+ called sunplus-uart.
+
+config SERIAL_SUNPLUS_CONSOLE
+ bool "Console on Sunplus UART"
+ depends on SERIAL_SUNPLUS
+ select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
+ help
+ Select this option if you would like to use a Sunplus 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=ttySUPx".
+
endmenu
config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 7da0856..61cc8de 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -87,6 +87,7 @@ obj-$(CONFIG_SERIAL_RDA) += rda-uart.o
obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o
obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
+obj-$(CONFIG_SERIAL_SUNPLUS) += sunplus-uart.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sunplus-uart.c b/drivers/tty/serial/sunplus-uart.c
new file mode 100644
index 0000000..a8146db
--- /dev/null
+++ b/drivers/tty/serial/sunplus-uart.c
@@ -0,0 +1,782 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sunplus SoC UART driver
+ *
+ * Author: Hammer Hsieh <[email protected]>
+ */
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/serial_core.h>
+#include <linux/sysrq.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <asm/irq.h>
+
+/* Register offsets */
+#define SUP_UART_DATA 0x00
+#define SUP_UART_LSR 0x04
+#define SUP_UART_MSR 0x08
+#define SUP_UART_LCR 0x0C
+#define SUP_UART_MCR 0x10
+#define SUP_UART_DIV_L 0x14
+#define SUP_UART_DIV_H 0x18
+#define SUP_UART_ISC 0x1C
+#define SUP_UART_TX_RESIDUE 0x20
+#define SUP_UART_RX_RESIDUE 0x24
+
+/* Line Status Register bits */
+#define SUP_UART_LSR_TXE BIT(6) /* tx empty */
+#define SUP_UART_LSR_BC BIT(5) /* break condition status */
+#define SUP_UART_LSR_FE BIT(4) /* frame error status */
+#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
+#define SUP_UART_LSR_PE BIT(2) /* parity error status */
+#define SUP_UART_LSR_RX BIT(1) /* 1: receive fifo not empty */
+#define SUP_UART_LSR_TX BIT(0) /* 1: transmit fifo is not full */
+#define SUP_UART_LSR_TX_NOT_FULL 1
+#define SUP_UART_LSR_BRK_ERROR_BITS GENMASK(5, 2)
+
+/* Line Control Register bits */
+#define SUP_UART_LCR_BC BIT(5) /* break condition select */
+#define SUP_UART_LCR_PR BIT(4) /* parity bit polarity select */
+#define SUP_UART_LCR_PE BIT(3) /* parity bit enable */
+#define SUP_UART_LCR_ST BIT(2) /* stop bits select */
+#define SUP_UART_LCR_WL5 0x00 /* word length 5 */
+#define SUP_UART_LCR_WL6 0x01 /* word length 6 */
+#define SUP_UART_LCR_WL7 0x02 /* word length 7 */
+#define SUP_UART_LCR_WL8 0x03 /* word length 8 (default) */
+
+/* Modem Control Register bits */
+#define SUP_UART_MCR_LB BIT(4) /* Loopback mode */
+#define SUP_UART_MCR_RI BIT(3) /* ring indicator */
+#define SUP_UART_MCR_DCD BIT(2) /* data carrier detect */
+#define SUP_UART_MCR_RTS BIT(1) /* request to send */
+#define SUP_UART_MCR_DTS BIT(0) /* data terminal ready */
+
+/* Interrupt Status/Control Register bits */
+#define SUP_UART_ISC_RXM BIT(5) /* RX interrupt enable */
+#define SUP_UART_ISC_TXM BIT(4) /* TX interrupt enable */
+#define SUP_UART_ISC_RX BIT(1) /* RX interrupt status */
+#define SUP_UART_ISC_TX BIT(0) /* TX interrupt status */
+
+#define SUP_DUMMY_READ BIT(16) /* drop bytes received on a !CREAD port */
+#define SUP_UART_NR 5
+
+struct sunplus_uart_port {
+ struct uart_port port;
+ struct clk *clk;
+ struct reset_control *rstc;
+};
+
+static void sp_uart_put_char(struct uart_port *port, unsigned int ch)
+{
+ writel(ch, port->membase + SUP_UART_DATA);
+}
+
+static u32 sunplus_tx_buf_not_full(struct uart_port *port)
+{
+ unsigned int lsr = readl(port->membase + SUP_UART_LSR);
+
+ return (lsr & SUP_UART_LSR_TX) ? SUP_UART_LSR_TX_NOT_FULL : 0;
+}
+
+static unsigned int sunplus_tx_empty(struct uart_port *port)
+{
+ unsigned int lsr = readl(port->membase + SUP_UART_LSR);
+
+ return (lsr & SUP_UART_LSR_TXE) ? TIOCSER_TEMT : 0;
+}
+
+static void sunplus_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ unsigned int mcr = readl(port->membase + SUP_UART_MCR);
+
+ if (mctrl & TIOCM_DTR)
+ mcr |= SUP_UART_MCR_DTS;
+ else
+ mcr &= ~SUP_UART_MCR_DTS;
+
+ if (mctrl & TIOCM_RTS)
+ mcr |= SUP_UART_MCR_RTS;
+ else
+ mcr &= ~SUP_UART_MCR_RTS;
+
+ if (mctrl & TIOCM_CAR)
+ mcr |= SUP_UART_MCR_DCD;
+ else
+ mcr &= ~SUP_UART_MCR_DCD;
+
+ if (mctrl & TIOCM_RI)
+ mcr |= SUP_UART_MCR_RI;
+ else
+ mcr &= ~SUP_UART_MCR_RI;
+
+ if (mctrl & TIOCM_LOOP)
+ mcr |= SUP_UART_MCR_LB;
+ else
+ mcr &= ~SUP_UART_MCR_LB;
+
+ writel(mcr, port->membase + SUP_UART_MCR);
+}
+
+static unsigned int sunplus_get_mctrl(struct uart_port *port)
+{
+ unsigned int ret, mcr;
+
+ mcr = readl(port->membase + SUP_UART_MCR);
+
+ if (mcr & SUP_UART_MCR_DTS)
+ ret |= TIOCM_DTR;
+
+ if (mcr & SUP_UART_MCR_RTS)
+ ret |= TIOCM_RTS;
+
+ if (mcr & SUP_UART_MCR_DCD)
+ ret |= TIOCM_CAR;
+
+ if (mcr & SUP_UART_MCR_RI)
+ ret |= TIOCM_RI;
+
+ if (mcr & SUP_UART_MCR_LB)
+ ret |= TIOCM_LOOP;
+
+ return ret;
+}
+
+static void sunplus_stop_tx(struct uart_port *port)
+{
+ unsigned int isc;
+
+ isc = readl(port->membase + SUP_UART_ISC);
+ isc &= ~SUP_UART_ISC_TXM;
+ writel(isc, port->membase + SUP_UART_ISC);
+}
+
+static void sunplus_start_tx(struct uart_port *port)
+{
+ unsigned int isc;
+
+ isc = readl(port->membase + SUP_UART_ISC);
+ isc |= SUP_UART_ISC_TXM;
+ writel(isc, port->membase + SUP_UART_ISC);
+}
+
+static void sunplus_stop_rx(struct uart_port *port)
+{
+ unsigned int isc;
+
+ isc = readl(port->membase + SUP_UART_ISC);
+ isc &= ~SUP_UART_ISC_RXM;
+ writel(isc, port->membase + SUP_UART_ISC);
+}
+
+static void sunplus_break_ctl(struct uart_port *port, int ctl)
+{
+ unsigned long flags;
+ unsigned int lcr;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ lcr = readl(port->membase + SUP_UART_LCR);
+
+ if (ctl)
+ lcr |= SUP_UART_LCR_BC; /* start break */
+ else
+ lcr &= ~SUP_UART_LCR_BC; /* stop break */
+
+ writel(lcr, port->membase + SUP_UART_LCR);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void transmit_chars(struct uart_port *port)
+{
+ struct circ_buf *xmit = &port->state->xmit;
+
+ if (port->x_char) {
+ sp_uart_put_char(port, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ return;
+ }
+
+ if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+ sunplus_stop_tx(port);
+ return;
+ }
+
+ do {
+ sp_uart_put_char(port, xmit->buf[xmit->tail]);
+ xmit->tail = (xmit->tail + 1) % UART_XMIT_SIZE;
+ port->icount.tx++;
+
+ if (uart_circ_empty(xmit))
+ break;
+ } while (sunplus_tx_buf_not_full(port));
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit))
+ sunplus_stop_tx(port);
+}
+
+static void receive_chars(struct uart_port *port)
+{
+ unsigned int lsr = readl(port->membase + SUP_UART_LSR);
+ unsigned int ch, flag;
+
+ do {
+ ch = readl(port->membase + SUP_UART_DATA);
+ flag = TTY_NORMAL;
+ port->icount.rx++;
+
+ if (unlikely(lsr & SUP_UART_LSR_BRK_ERROR_BITS)) {
+ if (lsr & SUP_UART_LSR_BC) {
+ lsr &= ~(SUP_UART_LSR_FE | SUP_UART_LSR_PE);
+ port->icount.brk++;
+ if (uart_handle_break(port))
+ goto ignore_char;
+ } else if (lsr & SUP_UART_LSR_PE) {
+ port->icount.parity++;
+ } else if (lsr & SUP_UART_LSR_FE) {
+ port->icount.frame++;
+ }
+
+ if (lsr & SUP_UART_LSR_OE)
+ port->icount.overrun++;
+
+ if (lsr & SUP_UART_LSR_BC)
+ flag = TTY_BREAK;
+ else if (lsr & SUP_UART_LSR_PE)
+ flag = TTY_PARITY;
+ else if (lsr & SUP_UART_LSR_FE)
+ flag = TTY_FRAME;
+ }
+
+ if (port->ignore_status_mask & SUP_DUMMY_READ)
+ goto ignore_char;
+
+ if (uart_handle_sysrq_char(port, ch))
+ goto ignore_char;
+
+ uart_insert_char(port, lsr, SUP_UART_LSR_OE, ch, flag);
+
+ignore_char:
+ lsr = readl(port->membase + SUP_UART_LSR);
+ } while (lsr & SUP_UART_LSR_RX);
+
+ tty_flip_buffer_push(&port->state->port);
+}
+
+static irqreturn_t sunplus_uart_irq(int irq, void *args)
+{
+ struct uart_port *port = (struct uart_port *)args;
+ unsigned int isc = readl(port->membase + SUP_UART_ISC);
+
+ spin_lock(&port->lock);
+
+ if (isc & SUP_UART_ISC_RX)
+ receive_chars(port);
+
+ if (isc & SUP_UART_ISC_TX)
+ transmit_chars(port);
+
+ spin_unlock(&port->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int sunplus_startup(struct uart_port *port)
+{
+ unsigned long flags;
+ unsigned int isc;
+ int ret;
+
+ ret = request_irq(port->irq, sunplus_uart_irq, 0, "sunplus_uart", port);
+ if (ret)
+ return ret;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ isc |= SUP_UART_ISC_RXM;
+ writel(isc, port->membase + SUP_UART_ISC);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ return 0;
+}
+
+static void sunplus_shutdown(struct uart_port *port)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ writel(0, port->membase + SUP_UART_ISC);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ free_irq(port->irq, port);
+}
+
+static void sunplus_set_termios(struct uart_port *port,
+ struct ktermios *termios,
+ struct ktermios *oldtermios)
+{
+ u32 ext, div, div_l, div_h, baud, lcr;
+ u32 clk = port->uartclk;
+ unsigned long flags;
+
+ baud = uart_get_baud_rate(port, termios, oldtermios, 0, port->uartclk / 16);
+
+ /* baud rate = uartclk / ((16 * divisor + 1) + divisor_ext) */
+ clk += baud >> 1;
+ div = clk / baud;
+ ext = div & 0x0F;
+ div = (div >> 4) - 1;
+ div_l = (div & 0xFF) | (ext << 12);
+ div_h = div >> 8;
+
+ switch (termios->c_cflag & CSIZE) {
+ case CS5:
+ lcr = SUP_UART_LCR_WL5;
+ break;
+ case CS6:
+ lcr = SUP_UART_LCR_WL6;
+ break;
+ case CS7:
+ lcr = SUP_UART_LCR_WL7;
+ break;
+ default: /* CS8 */
+ lcr = SUP_UART_LCR_WL8;
+ break;
+ }
+
+ if (termios->c_cflag & CSTOPB)
+ lcr |= SUP_UART_LCR_ST;
+
+ if (termios->c_cflag & PARENB) {
+ lcr |= SUP_UART_LCR_PE;
+
+ if (!(termios->c_cflag & PARODD))
+ lcr |= SUP_UART_LCR_PR;
+ }
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ port->read_status_mask = 0;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= SUP_UART_LSR_PE | SUP_UART_LSR_FE;
+
+ if (termios->c_iflag & (BRKINT | PARMRK))
+ port->read_status_mask |= SUP_UART_LSR_BC;
+
+ /* Characters to ignore */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SUP_UART_LSR_FE | SUP_UART_LSR_PE;
+
+ if (termios->c_iflag & IGNBRK) {
+ port->ignore_status_mask |= SUP_UART_LSR_BC;
+
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SUP_UART_LSR_OE;
+ }
+
+ /* Ignore all characters if CREAD is not set */
+ if ((termios->c_cflag & CREAD) == 0) {
+ port->ignore_status_mask |= SUP_DUMMY_READ;
+ /* flush rx data FIFO */
+ writel(0, port->membase + SUP_UART_RX_RESIDUE);
+ }
+
+ /* Settings for baud rate divisor and lcr */
+ writel(div_h, port->membase + SUP_UART_DIV_H);
+ writel(div_l, port->membase + SUP_UART_DIV_L);
+ writel(lcr, port->membase + SUP_UART_LCR);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void sunplus_set_ldisc(struct uart_port *port, struct ktermios *termios)
+{
+ int new = termios->c_line;
+
+ if (new == N_PPS)
+ port->flags |= UPF_HARDPPS_CD;
+ else
+ port->flags &= ~UPF_HARDPPS_CD;
+}
+
+static const char *sunplus_type(struct uart_port *port)
+{
+ return port->type == PORT_SUNPLUS ? "sunplus_uart" : NULL;
+}
+
+static void sunplus_release_port(struct uart_port *port)
+{
+}
+
+static int sunplus_request_port(struct uart_port *port)
+{
+ return 0;
+}
+
+static void sunplus_config_port(struct uart_port *port, int type)
+{
+ if (type & UART_CONFIG_TYPE) {
+ port->type = PORT_SUNPLUS;
+ sunplus_request_port(port);
+ }
+}
+
+static int sunplus_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+ if (ser->type != PORT_UNKNOWN && ser->type != PORT_SUNPLUS)
+ return -EINVAL;
+
+ return 0;
+}
+
+#ifdef CONFIG_CONSOLE_POLL
+static void sunplus_poll_put_char(struct uart_port *port, unsigned char data)
+{
+ wait_for_xmitr(port);
+ sp_uart_put_char(port, data);
+}
+
+static int sunplus_poll_get_char(struct uart_port *port)
+{
+ unsigned int lsr = readl(port->membase + SUP_UART_LSR);
+
+ if (!(lsr & SUP_UART_LSR_RX))
+ return NO_POLL_CHAR;
+
+ return readl(port->membase + SUP_UART_DATA);
+}
+#endif
+
+static const struct uart_ops sunplus_uart_ops = {
+ .tx_empty = sunplus_tx_empty,
+ .set_mctrl = sunplus_set_mctrl,
+ .get_mctrl = sunplus_get_mctrl,
+ .stop_tx = sunplus_stop_tx,
+ .start_tx = sunplus_start_tx,
+ .stop_rx = sunplus_stop_rx,
+ .break_ctl = sunplus_break_ctl,
+ .startup = sunplus_startup,
+ .shutdown = sunplus_shutdown,
+ .set_termios = sunplus_set_termios,
+ .set_ldisc = sunplus_set_ldisc,
+ .type = sunplus_type,
+ .release_port = sunplus_release_port,
+ .request_port = sunplus_request_port,
+ .config_port = sunplus_config_port,
+ .verify_port = sunplus_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_put_char = sunplus_poll_put_char,
+ .poll_get_char = sunplus_poll_get_char,
+#endif
+};
+
+#ifdef CONFIG_SERIAL_SUNPLUS_CONSOLE
+struct sunplus_uart_port *sunplus_console_ports[SUP_UART_NR];
+
+static void wait_for_xmitr(struct uart_port *port)
+{
+ unsigned int val;
+ int ret;
+
+ /* Wait while FIFO is full or timeout */
+ ret = readl_poll_timeout_atomic(port->membase + SUP_UART_LSR, val,
+ (val & SUP_UART_LSR_TX), 1, 10000);
+
+ if (ret == -ETIMEDOUT) {
+ dev_err(port->dev, "Timeout waiting while UART TX FULL\n");
+ return;
+ }
+}
+
+static void sunplus_uart_console_putchar(struct uart_port *port, int ch)
+{
+ wait_for_xmitr(port);
+ sp_uart_put_char(port, ch);
+}
+
+static void sunplus_console_write(struct console *co,
+ const char *s,
+ unsigned int count)
+{
+ unsigned long flags;
+ int locked = 1;
+
+ local_irq_save(flags);
+
+ if (sunplus_console_ports[co->index]->port.sysrq)
+ locked = 0;
+ else if (oops_in_progress)
+ locked = spin_trylock(&sunplus_console_ports[co->index]->port.lock);
+ else
+ spin_lock(&sunplus_console_ports[co->index]->port.lock);
+
+ uart_console_write(&sunplus_console_ports[co->index]->port, s, count,
+ sunplus_uart_console_putchar);
+
+ if (locked)
+ spin_unlock(&sunplus_console_ports[co->index]->port.lock);
+
+ local_irq_restore(flags);
+}
+
+static int __init sunplus_console_setup(struct console *co, char *options)
+{
+ struct sunplus_uart_port *sup;
+ int baud = 115200;
+ int bits = 8;
+ int parity = 'n';
+ int flow = 'n';
+
+ if (co->index < 0 || co->index >= SUP_UART_NR)
+ return -EINVAL;
+
+ sup = sunplus_console_ports[co->index];
+ if (!sup)
+ return -ENODEV;
+
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ return uart_set_options(&sup->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sunplus_uart_driver;
+static struct console sunplus_uart_console = {
+ .name = "ttySUP",
+ .write = sunplus_console_write,
+ .device = uart_console_device,
+ .setup = sunplus_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &sunplus_uart_driver
+};
+
+static int __init sunplus_console_init(void)
+{
+ register_console(&sunplus_uart_console);
+ return 0;
+}
+console_initcall(sunplus_console_init);
+#endif
+
+static struct uart_driver sunplus_uart_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "sunplus_uart",
+ .dev_name = "ttySUP",
+ .major = TTY_MAJOR,
+ .minor = 64,
+ .nr = SUP_UART_NR,
+ .cons = NULL,
+};
+
+static int sunplus_uart_probe(struct platform_device *pdev)
+{
+ struct sunplus_uart_port *sup;
+ struct uart_port *port;
+ struct resource *res;
+ int ret, irq;
+
+ pdev->id = of_alias_get_id(pdev->dev.of_node, "serial");
+
+ if (pdev->id < 0 || pdev->id >= SUP_UART_NR)
+ return -EINVAL;
+
+ sup = devm_kzalloc(&pdev->dev, sizeof(*sup), GFP_KERNEL);
+ if (!sup)
+ return -ENOMEM;
+
+ sup->clk = devm_clk_get_optional(&pdev->dev, NULL);
+ if (IS_ERR(sup->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(sup->clk), "clk not found\n");
+
+ ret = clk_prepare_enable(sup->clk);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))clk_disable_unprepare,
+ sup->clk);
+ if (ret)
+ return ret;
+
+ sup->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(sup->rstc))
+ return dev_err_probe(&pdev->dev, PTR_ERR(sup->rstc), "rstc not found\n");
+
+ port = &sup->port;
+
+ port->membase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(port->membase))
+ return dev_err_probe(&pdev->dev, PTR_ERR(port->membase), "membase not found\n");
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ port->mapbase = res->start;
+ port->uartclk = clk_get_rate(sup->clk);
+ port->line = pdev->id;
+ port->irq = irq;
+ port->dev = &pdev->dev;
+ port->iotype = UPIO_MEM;
+ port->ops = &sunplus_uart_ops;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->fifosize = 128;
+
+ ret = reset_control_deassert(sup->rstc);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(&pdev->dev,
+ (void(*)(void *))reset_control_assert,
+ sup->rstc);
+ if (ret)
+ return ret;
+
+#ifdef CONFIG_SERIAL_SUNPLUS_CONSOLE
+ sunplus_console_ports[sup->port.line] = sup;
+#endif
+
+ platform_set_drvdata(pdev, &sup->port);
+
+ ret = uart_add_one_port(&sunplus_uart_driver, &sup->port);
+#ifdef CONFIG_SERIAL_SUNPLUS_CONSOLE
+ if (ret)
+ sunplus_console_ports[sup->port.line] = NULL;
+#endif
+
+ return ret;
+}
+
+static int sunplus_uart_remove(struct platform_device *pdev)
+{
+ struct sunplus_uart_port *sup = platform_get_drvdata(pdev);
+
+ uart_remove_one_port(&sunplus_uart_driver, &sup->port);
+
+ return 0;
+}
+
+static int sunplus_uart_suspend(struct device *dev)
+{
+ struct sunplus_uart_port *sup = dev_get_drvdata(dev);
+
+ if (!uart_console(&sup->port))
+ uart_suspend_port(&sunplus_uart_driver, &sup->port);
+
+ return 0;
+}
+
+static int sunplus_uart_resume(struct device *dev)
+{
+ struct sunplus_uart_port *sup = dev_get_drvdata(dev);
+
+ if (!uart_console(&sup->port))
+ uart_resume_port(&sunplus_uart_driver, &sup->port);
+
+ return 0;
+}
+
+static const struct dev_pm_ops sunplus_uart_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(sunplus_uart_suspend, sunplus_uart_resume)
+};
+
+static const struct of_device_id sp_uart_of_match[] = {
+ { .compatible = "sunplus,sp7021-uart" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, sp_uart_of_match);
+
+static struct platform_driver sunplus_uart_platform_driver = {
+ .probe = sunplus_uart_probe,
+ .remove = sunplus_uart_remove,
+ .driver = {
+ .name = "sunplus_uart",
+ .of_match_table = sp_uart_of_match,
+ .pm = &sunplus_uart_pm_ops,
+ }
+};
+
+static int __init sunplus_uart_init(void)
+{
+ int ret;
+
+#ifdef CONFIG_SERIAL_SUNPLUS_CONSOLE
+ sunplus_uart_driver.cons = &sunplus_uart_console;
+#endif
+
+ ret = uart_register_driver(&sunplus_uart_driver);
+ if (ret)
+ return ret;
+
+ ret = platform_driver_register(&sunplus_uart_platform_driver);
+ if (ret)
+ uart_unregister_driver(&sunplus_uart_driver);
+
+ return ret;
+}
+module_init(sunplus_uart_init);
+
+static void __exit sunplus_uart_exit(void)
+{
+ platform_driver_unregister(&sunplus_uart_platform_driver);
+ uart_unregister_driver(&sunplus_uart_driver);
+}
+module_exit(sunplus_uart_exit);
+
+#ifdef CONFIG_SERIAL_EARLYCON
+static void sunplus_uart_putc(struct uart_port *port, int c)
+{
+ unsigned int val;
+ int ret;
+
+ ret = readl_poll_timeout_atomic(port->membase + SUP_UART_LSR, val,
+ (val & SUP_UART_LSR_TXE), 1, 10000);
+ if (ret)
+ return;
+
+ writel(c, port->membase + SUP_UART_DATA);
+}
+
+static void sunplus_uart_early_write(struct console *con, const char *s, unsigned int n)
+{
+ struct earlycon_device *dev = con->data;
+
+ uart_console_write(&dev->port, s, n, sunplus_uart_putc);
+}
+
+static int __init
+sunplus_uart_early_setup(struct earlycon_device *dev, const char *opt)
+{
+ if (!(dev->port.membase || dev->port.iobase))
+ return -ENODEV;
+
+ dev->con->write = sunplus_uart_early_write;
+
+ return 0;
+}
+OF_EARLYCON_DECLARE(sunplus_uart, "sunplus,sp7021-uart", sunplus_uart_early_setup);
+#endif
+
+MODULE_DESCRIPTION("Sunplus UART driver");
+MODULE_AUTHOR("Hammer Hsieh <[email protected]>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index c4042dc..2dfe443 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -274,4 +274,7 @@
/* Freescale LINFlexD UART */
#define PORT_LINFLEXUART 122
+/* Sunplus UART */
+#define PORT_SUNPLUS 123
+
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
2.7.4
Add bindings doc for Sunplus SoC UART Driver
Signed-off-by: Hammer Hsieh <[email protected]>
---
Changes in v4:
- no change.
- Reviewed-by : Rob Herring <[email protected]> in v3.
Changes in v5:
- remove another two author's name, the whole driver already quite different compared
with patch v1. The other two authors request me to remove it.
.../bindings/serial/sunplus,sp7021-uart.yaml | 56 ++++++++++++++++++++++
MAINTAINERS | 5 ++
2 files changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
diff --git a/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml b/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
new file mode 100644
index 0000000..ab66a0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/serial/sunplus,sp7021-uart.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Sunplus SoC SP7021 UART Controller Device Tree Bindings
+
+maintainers:
+ - Hammer Hsieh <[email protected]>
+
+allOf:
+ - $ref: serial.yaml#
+
+properties:
+ compatible:
+ const: sunplus,sp7021-uart
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ aliases {
+ serial0 = &uart0;
+ };
+
+ uart0: serial@9c000900 {
+ compatible = "sunplus,sp7021-uart";
+ reg = <0x9c000900 0x80>;
+ interrupt-parent = <&intc>;
+ interrupts = <53 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clkc 0x28>;
+ resets = <&rstc 0x18>;
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 3b79fd4..f2ee40c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17945,6 +17945,11 @@ L: [email protected]
S: Maintained
F: drivers/net/ethernet/dlink/sundance.c
+SUNPLUS UART DRIVER
+M: Hammer Hsieh <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
+
SUPERH
M: Yoshinori Sato <[email protected]>
M: Rich Felker <[email protected]>
--
2.7.4
On 13. 12. 21, 8:10, Hammer Hsieh wrote:
> Add Sunplus SoC UART Driver
>
> Signed-off-by: Hammer Hsieh <[email protected]>
...
> --- /dev/null
> +++ b/drivers/tty/serial/sunplus-uart.c
...
> +static void receive_chars(struct uart_port *port)
> +{
> + unsigned int lsr = readl(port->membase + SUP_UART_LSR);
> + unsigned int ch, flag;
> +
> + do {
> + ch = readl(port->membase + SUP_UART_DATA);
> + flag = TTY_NORMAL;
> + port->icount.rx++;
> +
> + if (unlikely(lsr & SUP_UART_LSR_BRK_ERROR_BITS)) {
> + if (lsr & SUP_UART_LSR_BC) {
> + lsr &= ~(SUP_UART_LSR_FE | SUP_UART_LSR_PE);
> + port->icount.brk++;
> + if (uart_handle_break(port))
> + goto ignore_char;
> + } else if (lsr & SUP_UART_LSR_PE) {
> + port->icount.parity++;
> + } else if (lsr & SUP_UART_LSR_FE) {
> + port->icount.frame++;
> + }
> +
> + if (lsr & SUP_UART_LSR_OE)
> + port->icount.overrun++;
> +
> + if (lsr & SUP_UART_LSR_BC)
> + flag = TTY_BREAK;
> + else if (lsr & SUP_UART_LSR_PE)
> + flag = TTY_PARITY;
> + else if (lsr & SUP_UART_LSR_FE)
> + flag = TTY_FRAME;
Why do you handle these separately and not above?
> + }
> +
> + if (port->ignore_status_mask & SUP_DUMMY_READ)
> + goto ignore_char;
> +
> + if (uart_handle_sysrq_char(port, ch))
> + goto ignore_char;
> +
> + uart_insert_char(port, lsr, SUP_UART_LSR_OE, ch, flag);
> +
> +ignore_char:
> + lsr = readl(port->membase + SUP_UART_LSR);
> + } while (lsr & SUP_UART_LSR_RX);
> +
> + tty_flip_buffer_push(&port->state->port);
> +}
> +
> +static irqreturn_t sunplus_uart_irq(int irq, void *args)
> +{
> + struct uart_port *port = (struct uart_port *)args;
No need to cast here.
> + unsigned int isc = readl(port->membase + SUP_UART_ISC);
Shouldn't this be under the spinlock?
And "if (!isc) return IRQ_NONE"?
> + spin_lock(&port->lock);
> +
> + if (isc & SUP_UART_ISC_RX)
> + receive_chars(port);
> +
> + if (isc & SUP_UART_ISC_TX)
> + transmit_chars(port);
> +
> + spin_unlock(&port->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sunplus_startup(struct uart_port *port)
> +{
> + unsigned long flags;
> + unsigned int isc;
> + int ret;
> +
> + ret = request_irq(port->irq, sunplus_uart_irq, 0, "sunplus_uart", port);
Cannot the interrupt be shared?
> + if (ret)
> + return ret;
> +
> + spin_lock_irqsave(&port->lock, flags);
> +
> + isc |= SUP_UART_ISC_RXM;
> + writel(isc, port->membase + SUP_UART_ISC);
> +
> + spin_unlock_irqrestore(&port->lock, flags);
> +
> + return 0;
> +}
> +
> +static void sunplus_shutdown(struct uart_port *port)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&port->lock, flags);
> + writel(0, port->membase + SUP_UART_ISC);
What bus is this -- posting?
> + spin_unlock_irqrestore(&port->lock, flags);
> +
> + free_irq(port->irq, port);
> +}
...
> +static void sunplus_release_port(struct uart_port *port)
> +{
> +}
> +
> +static int sunplus_request_port(struct uart_port *port)
> +{
> + return 0;
> +}
These two are optional -- no need to provide them.
regards,
--
js
suse labs
Hi, Jiri Slaby:
Thanks for your review.
My response is below in mail.
Best Regards,
Hammer Hsieh
> -----Original Message-----
> From: Jiri Slaby <[email protected]>
> Sent: Monday, December 13, 2021 3:47 PM
> To: Hammer Hsieh <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Wells Lu 呂芳騰 <[email protected]>; Hammer Hsieh 謝宏孟
> <[email protected]>
> Subject: Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver
>
> On 13. 12. 21, 8:10, Hammer Hsieh wrote:
> > Add Sunplus SoC UART Driver
> >
> > Signed-off-by: Hammer Hsieh <[email protected]>
>
> ...
>
> > --- /dev/null
> > +++ b/drivers/tty/serial/sunplus-uart.c
>
> ...
>
> > +static void receive_chars(struct uart_port *port) {
> > + unsigned int lsr = readl(port->membase + SUP_UART_LSR);
> > + unsigned int ch, flag;
> > +
> > + do {
> > + ch = readl(port->membase + SUP_UART_DATA);
> > + flag = TTY_NORMAL;
> > + port->icount.rx++;
> > +
> > + if (unlikely(lsr & SUP_UART_LSR_BRK_ERROR_BITS)) {
> > + if (lsr & SUP_UART_LSR_BC) {
> > + lsr &= ~(SUP_UART_LSR_FE | SUP_UART_LSR_PE);
> > + port->icount.brk++;
> > + if (uart_handle_break(port))
> > + goto ignore_char;
> > + } else if (lsr & SUP_UART_LSR_PE) {
> > + port->icount.parity++;
> > + } else if (lsr & SUP_UART_LSR_FE) {
> > + port->icount.frame++;
> > + }
> > +
> > + if (lsr & SUP_UART_LSR_OE)
> > + port->icount.overrun++;
> > +
> > + if (lsr & SUP_UART_LSR_BC)
> > + flag = TTY_BREAK;
> > + else if (lsr & SUP_UART_LSR_PE)
> > + flag = TTY_PARITY;
> > + else if (lsr & SUP_UART_LSR_FE)
> > + flag = TTY_FRAME;
>
> Why do you handle these separately and not above?
>
Indeed, I will modify as below.
if (lsr & SUP_UART_LSR_BC) {
lsr &= ~(SUP_UART_LSR_FE | SUP_UART_LSR_PE);
port->icount.brk++;
flag = TTY_BREAK;
if (uart_handle_break(port))
goto ignore_char;
} else if (lsr & SUP_UART_LSR_PE) {
port->icount.parity++;
flag = TTY_PARITY;
} else if (lsr & SUP_UART_LSR_FE) {
port->icount.frame++;
flag = TTY_FRAME;
}
> > + }
> > +
> > + if (port->ignore_status_mask & SUP_DUMMY_READ)
> > + goto ignore_char;
> > +
> > + if (uart_handle_sysrq_char(port, ch))
> > + goto ignore_char;
> > +
> > + uart_insert_char(port, lsr, SUP_UART_LSR_OE, ch, flag);
> > +
> > +ignore_char:
> > + lsr = readl(port->membase + SUP_UART_LSR);
> > + } while (lsr & SUP_UART_LSR_RX);
> > +
> > + tty_flip_buffer_push(&port->state->port);
> > +}
> > +
> > +static irqreturn_t sunplus_uart_irq(int irq, void *args) {
> > + struct uart_port *port = (struct uart_port *)args;
>
> No need to cast here.
>
Ok, will modify it.
> > + unsigned int isc = readl(port->membase + SUP_UART_ISC);
>
> Shouldn't this be under the spinlock?
>
> And "if (!isc) return IRQ_NONE"?
>
Will modify it as below.
Spin_lock()
isc = readl(port->membase + SUP_UART_ISC);
if (!isc) return IRQ_NONE;
if(isc&RX) receive_chars();
if(isc&TX) transmit_chars();
Spin_unlock()
> > + spin_lock(&port->lock);
> > +
> > + if (isc & SUP_UART_ISC_RX)
> > + receive_chars(port);
> > +
> > + if (isc & SUP_UART_ISC_TX)
> > + transmit_chars(port);
> > +
> > + spin_unlock(&port->lock);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int sunplus_startup(struct uart_port *port) {
> > + unsigned long flags;
> > + unsigned int isc;
> > + int ret;
> > +
> > + ret = request_irq(port->irq, sunplus_uart_irq, 0, "sunplus_uart",
> > +port);
>
> Cannot the interrupt be shared?
>
Each UART have its own hardware interrupt in Sunplus SP7021 SoC.
No need to set IRQF_SHARED for serial driver.
> > + if (ret)
> > + return ret;
> > +
> > + spin_lock_irqsave(&port->lock, flags);
> > +
> > + isc |= SUP_UART_ISC_RXM;
> > + writel(isc, port->membase + SUP_UART_ISC);
> > +
> > + spin_unlock_irqrestore(&port->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static void sunplus_shutdown(struct uart_port *port) {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&port->lock, flags);
> > + writel(0, port->membase + SUP_UART_ISC);
>
> What bus is this -- posting?
>
Here just clear interrupt.
Not really understand your comment?
> > + spin_unlock_irqrestore(&port->lock, flags);
> > +
> > + free_irq(port->irq, port);
> > +}
>
> ...
>
> > +static void sunplus_release_port(struct uart_port *port) { }
> > +
> > +static int sunplus_request_port(struct uart_port *port) {
> > + return 0;
> > +}
>
> These two are optional -- no need to provide them.
>
Ok, thanks.
I will remove these two functions.
> regards,
> --
> js
> suse labs
On Mon, Dec 13, 2021 at 03:10:06PM +0800, Hammer Hsieh wrote:
> Add bindings doc for Sunplus SoC UART Driver
>
> Signed-off-by: Hammer Hsieh <[email protected]>
> ---
> Changes in v4:
> - no change.
> - Reviewed-by : Rob Herring <[email protected]> in v3.
That goes above your Signed-off-by.
>
> Changes in v5:
> - remove another two author's name, the whole driver already quite different compared
> with patch v1. The other two authors request me to remove it.
>
> .../bindings/serial/sunplus,sp7021-uart.yaml | 56 ++++++++++++++++++++++
> MAINTAINERS | 5 ++
> 2 files changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
>
> diff --git a/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml b/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
> new file mode 100644
> index 0000000..ab66a0f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Co., Ltd. 2021
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/serial/sunplus,sp7021-uart.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Sunplus SoC SP7021 UART Controller Device Tree Bindings
> +
> +maintainers:
> + - Hammer Hsieh <[email protected]>
> +
> +allOf:
> + - $ref: serial.yaml#
> +
> +properties:
> + compatible:
> + const: sunplus,sp7021-uart
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - resets
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + uart0: serial@9c000900 {
> + compatible = "sunplus,sp7021-uart";
> + reg = <0x9c000900 0x80>;
> + interrupt-parent = <&intc>;
> + interrupts = <53 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clkc 0x28>;
> + resets = <&rstc 0x18>;
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3b79fd4..f2ee40c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17945,6 +17945,11 @@ L: [email protected]
> S: Maintained
> F: drivers/net/ethernet/dlink/sundance.c
>
> +SUNPLUS UART DRIVER
> +M: Hammer Hsieh <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml
> +
> SUPERH
> M: Yoshinori Sato <[email protected]>
> M: Rich Felker <[email protected]>
> --
> 2.7.4
>
>
On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> +/* Sunplus UART */
> +#define PORT_SUNPLUS 123
Why do you need a new number? What userspace tool is going to use this?
Why can't you just say you are a normal 8250 device?
thanks,
greg k-h
On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> Add Sunplus SoC UART Driver
Why is this a new driver? It looks like a 8250 variant UART, right? So
why can't you just use that framework instead? What is different about
this hardware that requires a new serial driver at all?
Please explain the hardware here in the changelog text to justify why
this is needed at all.
thanks,
greg k-h
On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> +/* Register offsets */
> +#define SUP_UART_DATA 0x00
> +#define SUP_UART_LSR 0x04
> +#define SUP_UART_MSR 0x08
> +#define SUP_UART_LCR 0x0C
> +#define SUP_UART_MCR 0x10
> +#define SUP_UART_DIV_L 0x14
> +#define SUP_UART_DIV_H 0x18
> +#define SUP_UART_ISC 0x1C
> +#define SUP_UART_TX_RESIDUE 0x20
> +#define SUP_UART_RX_RESIDUE 0x24
> +
> +/* Line Status Register bits */
> +#define SUP_UART_LSR_TXE BIT(6) /* tx empty */
> +#define SUP_UART_LSR_BC BIT(5) /* break condition status */
> +#define SUP_UART_LSR_FE BIT(4) /* frame error status */
> +#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
> +#define SUP_UART_LSR_PE BIT(2) /* parity error status */
> +#define SUP_UART_LSR_RX BIT(1) /* 1: receive fifo not empty */
> +#define SUP_UART_LSR_TX BIT(0) /* 1: transmit fifo is not full */
> +#define SUP_UART_LSR_TX_NOT_FULL 1
> +#define SUP_UART_LSR_BRK_ERROR_BITS GENMASK(5, 2)
> +
> +/* Line Control Register bits */
> +#define SUP_UART_LCR_BC BIT(5) /* break condition select */
> +#define SUP_UART_LCR_PR BIT(4) /* parity bit polarity select */
> +#define SUP_UART_LCR_PE BIT(3) /* parity bit enable */
> +#define SUP_UART_LCR_ST BIT(2) /* stop bits select */
> +#define SUP_UART_LCR_WL5 0x00 /* word length 5 */
> +#define SUP_UART_LCR_WL6 0x01 /* word length 6 */
> +#define SUP_UART_LCR_WL7 0x02 /* word length 7 */
> +#define SUP_UART_LCR_WL8 0x03 /* word length 8 (default) */
> +
> +/* Modem Control Register bits */
> +#define SUP_UART_MCR_LB BIT(4) /* Loopback mode */
> +#define SUP_UART_MCR_RI BIT(3) /* ring indicator */
> +#define SUP_UART_MCR_DCD BIT(2) /* data carrier detect */
> +#define SUP_UART_MCR_RTS BIT(1) /* request to send */
> +#define SUP_UART_MCR_DTS BIT(0) /* data terminal ready */
> +
> +/* Interrupt Status/Control Register bits */
> +#define SUP_UART_ISC_RXM BIT(5) /* RX interrupt enable */
> +#define SUP_UART_ISC_TXM BIT(4) /* TX interrupt enable */
> +#define SUP_UART_ISC_RX BIT(1) /* RX interrupt status */
> +#define SUP_UART_ISC_TX BIT(0) /* TX interrupt status */
> +
> +#define SUP_DUMMY_READ BIT(16) /* drop bytes received on a !CREAD port */
> +#define SUP_UART_NR 5
Aren't most of these defines already in the kernel header files? Why
create them again?
Greg KH <[email protected]> 於 2021年12月20日 週一 下午11:51寫道：
>
> On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> > +/* Register offsets */
> > +#define SUP_UART_DATA 0x00
> > +#define SUP_UART_LSR 0x04
> > +#define SUP_UART_MSR 0x08
> > +#define SUP_UART_LCR 0x0C
> > +#define SUP_UART_MCR 0x10
> > +#define SUP_UART_DIV_L 0x14
> > +#define SUP_UART_DIV_H 0x18
> > +#define SUP_UART_ISC 0x1C
> > +#define SUP_UART_TX_RESIDUE 0x20
> > +#define SUP_UART_RX_RESIDUE 0x24
> > +
> > +/* Line Status Register bits */
> > +#define SUP_UART_LSR_TXE BIT(6) /* tx empty */
> > +#define SUP_UART_LSR_BC BIT(5) /* break condition status */
> > +#define SUP_UART_LSR_FE BIT(4) /* frame error status */
> > +#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
> > +#define SUP_UART_LSR_PE BIT(2) /* parity error status */
> > +#define SUP_UART_LSR_RX BIT(1) /* 1: receive fifo not empty */
> > +#define SUP_UART_LSR_TX BIT(0) /* 1: transmit fifo is not full */
> > +#define SUP_UART_LSR_TX_NOT_FULL 1
> > +#define SUP_UART_LSR_BRK_ERROR_BITS GENMASK(5, 2)
> > +
> > +/* Line Control Register bits */
> > +#define SUP_UART_LCR_BC BIT(5) /* break condition select */
> > +#define SUP_UART_LCR_PR BIT(4) /* parity bit polarity select */
> > +#define SUP_UART_LCR_PE BIT(3) /* parity bit enable */
> > +#define SUP_UART_LCR_ST BIT(2) /* stop bits select */
> > +#define SUP_UART_LCR_WL5 0x00 /* word length 5 */
> > +#define SUP_UART_LCR_WL6 0x01 /* word length 6 */
> > +#define SUP_UART_LCR_WL7 0x02 /* word length 7 */
> > +#define SUP_UART_LCR_WL8 0x03 /* word length 8 (default) */
> > +
> > +/* Modem Control Register bits */
> > +#define SUP_UART_MCR_LB BIT(4) /* Loopback mode */
> > +#define SUP_UART_MCR_RI BIT(3) /* ring indicator */
> > +#define SUP_UART_MCR_DCD BIT(2) /* data carrier detect */
> > +#define SUP_UART_MCR_RTS BIT(1) /* request to send */
> > +#define SUP_UART_MCR_DTS BIT(0) /* data terminal ready */
> > +
> > +/* Interrupt Status/Control Register bits */
> > +#define SUP_UART_ISC_RXM BIT(5) /* RX interrupt enable */
> > +#define SUP_UART_ISC_TXM BIT(4) /* TX interrupt enable */
> > +#define SUP_UART_ISC_RX BIT(1) /* RX interrupt status */
> > +#define SUP_UART_ISC_TX BIT(0) /* TX interrupt status */
> > +
> > +#define SUP_DUMMY_READ BIT(16) /* drop bytes received on a !CREAD port */
> > +#define SUP_UART_NR 5
>
> Aren't most of these defines already in the kernel header files? Why
> create them again?
>
If for reduce code.
I can add #include<linux/serial_reg.h>
And remove some overlap define name.
#define SUP_UART_LCR_PR -> UART_LCR_EPAR
#define SUP_UART_LCR_PE -> UART_LCR_PARITY
#define SUP_UART_LCR_ST -> UART_LCR_STOP
#define SUP_UART_LCR_WL5 -> UART_LCR_WLEN5
#define SUP_UART_LCR_WL6 -> UART_LCR_WLEN6
#define SUP_UART_LCR_WL7 -> UART_LCR_WLEN7
#define SUP_UART_LCR_WL8 -> UART_LCR_WLEN8
#define SUP_UART_MCR_LB -> UART_MCR_LOOP
#define SUP_UART_MCR_RI -> UART_MCR_OUT2 ?
#define SUP_UART_MCR_DCD -> UART_MCR_OUT1 ?
#define SUP_UART_MCR_RTS -> UART_MCR_RTS
#define SUP_UART_MCR_DTS -> UART_MCR_DTR
But the rest define didn't match internal #include<linux/serial_reg.h>
, those define still need to keep.
Some use SUP_xxxx specific define.
Some use internal #include<linux/serial_reg.h>, it is strange.
On Tue, Dec 21, 2021 at 04:14:16PM +0800, hammer hsieh wrote:
> Greg KH <[email protected]> 於 2021年12月20日 週一 下午11:51寫道：
> >
> > On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> > > +/* Register offsets */
> > > +#define SUP_UART_DATA 0x00
> > > +#define SUP_UART_LSR 0x04
> > > +#define SUP_UART_MSR 0x08
> > > +#define SUP_UART_LCR 0x0C
> > > +#define SUP_UART_MCR 0x10
> > > +#define SUP_UART_DIV_L 0x14
> > > +#define SUP_UART_DIV_H 0x18
> > > +#define SUP_UART_ISC 0x1C
> > > +#define SUP_UART_TX_RESIDUE 0x20
> > > +#define SUP_UART_RX_RESIDUE 0x24
> > > +
> > > +/* Line Status Register bits */
> > > +#define SUP_UART_LSR_TXE BIT(6) /* tx empty */
> > > +#define SUP_UART_LSR_BC BIT(5) /* break condition status */
> > > +#define SUP_UART_LSR_FE BIT(4) /* frame error status */
> > > +#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
> > > +#define SUP_UART_LSR_PE BIT(2) /* parity error status */
> > > +#define SUP_UART_LSR_RX BIT(1) /* 1: receive fifo not empty */
> > > +#define SUP_UART_LSR_TX BIT(0) /* 1: transmit fifo is not full */
> > > +#define SUP_UART_LSR_TX_NOT_FULL 1
> > > +#define SUP_UART_LSR_BRK_ERROR_BITS GENMASK(5, 2)
> > > +
> > > +/* Line Control Register bits */
> > > +#define SUP_UART_LCR_BC BIT(5) /* break condition select */
> > > +#define SUP_UART_LCR_PR BIT(4) /* parity bit polarity select */
> > > +#define SUP_UART_LCR_PE BIT(3) /* parity bit enable */
> > > +#define SUP_UART_LCR_ST BIT(2) /* stop bits select */
> > > +#define SUP_UART_LCR_WL5 0x00 /* word length 5 */
> > > +#define SUP_UART_LCR_WL6 0x01 /* word length 6 */
> > > +#define SUP_UART_LCR_WL7 0x02 /* word length 7 */
> > > +#define SUP_UART_LCR_WL8 0x03 /* word length 8 (default) */
> > > +
> > > +/* Modem Control Register bits */
> > > +#define SUP_UART_MCR_LB BIT(4) /* Loopback mode */
> > > +#define SUP_UART_MCR_RI BIT(3) /* ring indicator */
> > > +#define SUP_UART_MCR_DCD BIT(2) /* data carrier detect */
> > > +#define SUP_UART_MCR_RTS BIT(1) /* request to send */
> > > +#define SUP_UART_MCR_DTS BIT(0) /* data terminal ready */
> > > +
> > > +/* Interrupt Status/Control Register bits */
> > > +#define SUP_UART_ISC_RXM BIT(5) /* RX interrupt enable */
> > > +#define SUP_UART_ISC_TXM BIT(4) /* TX interrupt enable */
> > > +#define SUP_UART_ISC_RX BIT(1) /* RX interrupt status */
> > > +#define SUP_UART_ISC_TX BIT(0) /* TX interrupt status */
> > > +
> > > +#define SUP_DUMMY_READ BIT(16) /* drop bytes received on a !CREAD port */
> > > +#define SUP_UART_NR 5
> >
> > Aren't most of these defines already in the kernel header files? Why
> > create them again?
> >
>
> If for reduce code.
> I can add #include<linux/serial_reg.h>
> And remove some overlap define name.
>
> #define SUP_UART_LCR_PR -> UART_LCR_EPAR
> #define SUP_UART_LCR_PE -> UART_LCR_PARITY
> #define SUP_UART_LCR_ST -> UART_LCR_STOP
> #define SUP_UART_LCR_WL5 -> UART_LCR_WLEN5
> #define SUP_UART_LCR_WL6 -> UART_LCR_WLEN6
> #define SUP_UART_LCR_WL7 -> UART_LCR_WLEN7
> #define SUP_UART_LCR_WL8 -> UART_LCR_WLEN8
>
> #define SUP_UART_MCR_LB -> UART_MCR_LOOP
> #define SUP_UART_MCR_RI -> UART_MCR_OUT2 ?
> #define SUP_UART_MCR_DCD -> UART_MCR_OUT1 ?
> #define SUP_UART_MCR_RTS -> UART_MCR_RTS
> #define SUP_UART_MCR_DTS -> UART_MCR_DTR
>
> But the rest define didn't match internal #include<linux/serial_reg.h>
> , those define still need to keep.
> Some use SUP_xxxx specific define.
> Some use internal #include<linux/serial_reg.h>, it is strange.
Do not duplicate defines that we already have for the same hardware
type.
And again, why is this not a normal serial driver for the existing UART
types as this hardware is obviously an 8250 variant?
thanks,
greg k-h
Hi, Greg KH :
In patch v1 coding quite mess, it is almost 2000 LOCs.
For down size code under 1000 LOCs, I decide to drop DMA function code
after patch v3.
I think that's the biggest difference compared with 8250.
Without DMA function, like you said it looks like 8250 variant.
I think I should put DMA function back in next submit.
Another question for why I need PORT_SLUNPLUS ?
I just check many other uart driver, almost all driver define their
own PORT number.
Actually, I didn't know about it.
Maybe some device like bluetooth(use uart port) need autoconfig.
Then it will call ioctl with TIOCSERCONFIG.
I don't have tool for calling type/config/request/release/verify.
Regards,
Hammer Hsieh
Greg KH <[email protected]> 於 2021年12月21日 週二 下午4:21寫道：
>
> On Tue, Dec 21, 2021 at 04:14:16PM +0800, hammer hsieh wrote:
> > Greg KH <[email protected]> 於 2021年12月20日 週一 下午11:51寫道：
> > >
> > > On Mon, Dec 13, 2021 at 03:10:07PM +0800, Hammer Hsieh wrote:
> > > > +/* Register offsets */
> > > > +#define SUP_UART_DATA 0x00
> > > > +#define SUP_UART_LSR 0x04
> > > > +#define SUP_UART_MSR 0x08
> > > > +#define SUP_UART_LCR 0x0C
> > > > +#define SUP_UART_MCR 0x10
> > > > +#define SUP_UART_DIV_L 0x14
> > > > +#define SUP_UART_DIV_H 0x18
> > > > +#define SUP_UART_ISC 0x1C
> > > > +#define SUP_UART_TX_RESIDUE 0x20
> > > > +#define SUP_UART_RX_RESIDUE 0x24
> > > > +
> > > > +/* Line Status Register bits */
> > > > +#define SUP_UART_LSR_TXE BIT(6) /* tx empty */
> > > > +#define SUP_UART_LSR_BC BIT(5) /* break condition status */
> > > > +#define SUP_UART_LSR_FE BIT(4) /* frame error status */
> > > > +#define SUP_UART_LSR_OE BIT(3) /* overrun error status */
> > > > +#define SUP_UART_LSR_PE BIT(2) /* parity error status */
> > > > +#define SUP_UART_LSR_RX BIT(1) /* 1: receive fifo not empty */
> > > > +#define SUP_UART_LSR_TX BIT(0) /* 1: transmit fifo is not full */
> > > > +#define SUP_UART_LSR_TX_NOT_FULL 1
> > > > +#define SUP_UART_LSR_BRK_ERROR_BITS GENMASK(5, 2)
> > > > +
> > > > +/* Line Control Register bits */
> > > > +#define SUP_UART_LCR_BC BIT(5) /* break condition select */
> > > > +#define SUP_UART_LCR_PR BIT(4) /* parity bit polarity select */
> > > > +#define SUP_UART_LCR_PE BIT(3) /* parity bit enable */
> > > > +#define SUP_UART_LCR_ST BIT(2) /* stop bits select */
> > > > +#define SUP_UART_LCR_WL5 0x00 /* word length 5 */
> > > > +#define SUP_UART_LCR_WL6 0x01 /* word length 6 */
> > > > +#define SUP_UART_LCR_WL7 0x02 /* word length 7 */
> > > > +#define SUP_UART_LCR_WL8 0x03 /* word length 8 (default) */
> > > > +
> > > > +/* Modem Control Register bits */
> > > > +#define SUP_UART_MCR_LB BIT(4) /* Loopback mode */
> > > > +#define SUP_UART_MCR_RI BIT(3) /* ring indicator */
> > > > +#define SUP_UART_MCR_DCD BIT(2) /* data carrier detect */
> > > > +#define SUP_UART_MCR_RTS BIT(1) /* request to send */
> > > > +#define SUP_UART_MCR_DTS BIT(0) /* data terminal ready */
> > > > +
> > > > +/* Interrupt Status/Control Register bits */
> > > > +#define SUP_UART_ISC_RXM BIT(5) /* RX interrupt enable */
> > > > +#define SUP_UART_ISC_TXM BIT(4) /* TX interrupt enable */
> > > > +#define SUP_UART_ISC_RX BIT(1) /* RX interrupt status */
> > > > +#define SUP_UART_ISC_TX BIT(0) /* TX interrupt status */
> > > > +
> > > > +#define SUP_DUMMY_READ BIT(16) /* drop bytes received on a !CREAD port */
> > > > +#define SUP_UART_NR 5
> > >
> > > Aren't most of these defines already in the kernel header files? Why
> > > create them again?
> > >
> >
> > If for reduce code.
> > I can add #include<linux/serial_reg.h>
> > And remove some overlap define name.
> >
> > #define SUP_UART_LCR_PR -> UART_LCR_EPAR
> > #define SUP_UART_LCR_PE -> UART_LCR_PARITY
> > #define SUP_UART_LCR_ST -> UART_LCR_STOP
> > #define SUP_UART_LCR_WL5 -> UART_LCR_WLEN5
> > #define SUP_UART_LCR_WL6 -> UART_LCR_WLEN6
> > #define SUP_UART_LCR_WL7 -> UART_LCR_WLEN7
> > #define SUP_UART_LCR_WL8 -> UART_LCR_WLEN8
> >
> > #define SUP_UART_MCR_LB -> UART_MCR_LOOP
> > #define SUP_UART_MCR_RI -> UART_MCR_OUT2 ?
> > #define SUP_UART_MCR_DCD -> UART_MCR_OUT1 ?
> > #define SUP_UART_MCR_RTS -> UART_MCR_RTS
> > #define SUP_UART_MCR_DTS -> UART_MCR_DTR
> >
> > But the rest define didn't match internal #include<linux/serial_reg.h>
> > , those define still need to keep.
> > Some use SUP_xxxx specific define.
> > Some use internal #include<linux/serial_reg.h>, it is strange.
>
> Do not duplicate defines that we already have for the same hardware
> type.
>
> And again, why is this not a normal serial driver for the existing UART
> types as this hardware is obviously an 8250 variant?
>
> thanks,
>
> greg k-h
On Fri, Dec 24, 2021 at 03:16:55PM +0800, hammer hsieh wrote:
> Hi, Greg KH :
>
> In patch v1 coding quite mess, it is almost 2000 LOCs.
> For down size code under 1000 LOCs, I decide to drop DMA function code
> after patch v3.
> I think that's the biggest difference compared with 8250.
> Without DMA function, like you said it looks like 8250 variant.
> I think I should put DMA function back in next submit.
The 8250 driver handles DMA just fine today, why is your chip doing it
differently? Are you sure it is a different chip? Who created a new
uart chip these days?
> Another question for why I need PORT_SLUNPLUS ?
> I just check many other uart driver, almost all driver define their
> own PORT number.
> Actually, I didn't know about it.
> Maybe some device like bluetooth(use uart port) need autoconfig.
> Then it will call ioctl with TIOCSERCONFIG.
> I don't have tool for calling type/config/request/release/verify.
If you do not need it, and you can not test for it, please do not add
it.
thanks,
greg k-h
Hi, Greg KH:
8250 driver means create driver in drivers/tty/8250/ ?
and current I create driver in drivers/tty/serial/ not correct ?
Regards,
Hammer Hsieh
Greg KH <[email protected]> 於 2021年12月24日 週五 下午4:59寫道：
>
> On Fri, Dec 24, 2021 at 03:16:55PM +0800, hammer hsieh wrote:
> > Hi, Greg KH :
> >
> > In patch v1 coding quite mess, it is almost 2000 LOCs.
> > For down size code under 1000 LOCs, I decide to drop DMA function code
> > after patch v3.
> > I think that's the biggest difference compared with 8250.
> > Without DMA function, like you said it looks like 8250 variant.
> > I think I should put DMA function back in next submit.
>
> The 8250 driver handles DMA just fine today, why is your chip doing it
> differently? Are you sure it is a different chip? Who created a new
> uart chip these days?
>
> > Another question for why I need PORT_SLUNPLUS ?
> > I just check many other uart driver, almost all driver define their
> > own PORT number.
> > Actually, I didn't know about it.
> > Maybe some device like bluetooth(use uart port) need autoconfig.
> > Then it will call ioctl with TIOCSERCONFIG.
> > I don't have tool for calling type/config/request/release/verify.
>
> If you do not need it, and you can not test for it, please do not add
> it.
>
> thanks,
>
> greg k-h
Hi, Greg KH:
I am still not really understand why you said the driver looks like 8250.
SP7021 SoC have our own register define.
That's why we submit a new serial driver.
Refer to:
https://sunplus.atlassian.net/wiki/spaces/doc/pages/1873412290/13.+Universal+Asynchronous+Receiver+Transmitter+UART
Regards,
Hammer Hsieh
hammer hsieh <[email protected]> 於 2021年12月24日 週五 下午5:05寫道：
>
> Hi, Greg KH:
>
> 8250 driver means create driver in drivers/tty/8250/ ?
> and current I create driver in drivers/tty/serial/ not correct ?
>
> Regards,
> Hammer Hsieh
>
> Greg KH <[email protected]> 於 2021年12月24日 週五 下午4:59寫道：
> >
> > On Fri, Dec 24, 2021 at 03:16:55PM +0800, hammer hsieh wrote:
> > > Hi, Greg KH :
> > >
> > > In patch v1 coding quite mess, it is almost 2000 LOCs.
> > > For down size code under 1000 LOCs, I decide to drop DMA function code
> > > after patch v3.
> > > I think that's the biggest difference compared with 8250.
> > > Without DMA function, like you said it looks like 8250 variant.
> > > I think I should put DMA function back in next submit.
> >
> > The 8250 driver handles DMA just fine today, why is your chip doing it
> > differently? Are you sure it is a different chip? Who created a new
> > uart chip these days?
> >
> > > Another question for why I need PORT_SLUNPLUS ?
> > > I just check many other uart driver, almost all driver define their
> > > own PORT number.
> > > Actually, I didn't know about it.
> > > Maybe some device like bluetooth(use uart port) need autoconfig.
> > > Then it will call ioctl with TIOCSERCONFIG.
> > > I don't have tool for calling type/config/request/release/verify.
> >
> > If you do not need it, and you can not test for it, please do not add
> > it.
> >
> > thanks,
> >
> > greg k-h
On Fri, Dec 24, 2021 at 05:21:27PM +0800, hammer hsieh wrote:
> Hi, Greg KH:
>
> I am still not really understand why you said the driver looks like 8250.
> SP7021 SoC have our own register define.
> That's why we submit a new serial driver.
>
> Refer to:
> https://sunplus.atlassian.net/wiki/spaces/doc/pages/1873412290/13.+Universal+Asynchronous+Receiver+Transmitter+UART
Odd, ok, I thought this was an 8250-like uart, why did they go and
redesign all of the register values for something as well-known as a
UART?
Anyway, I think you are right, please fix up the other issues and resend
the driver and we will be glad to review it again.
thanks,
greg k-h