* [PATCH v5 0/2] Add UART driver for Suplus SP7021 SoC @ 2021-12-13 7:10 Hammer Hsieh 2021-12-13 7:10 ` [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver Hammer Hsieh 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh 0 siblings, 2 replies; 16+ messages in thread From: Hammer Hsieh @ 2021-12-13 7:10 UTC (permalink / raw) To: gregkh, robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel Cc: wells.lu, Hammer Hsieh 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver 2021-12-13 7:10 [PATCH v5 0/2] Add UART driver for Suplus SP7021 SoC Hammer Hsieh @ 2021-12-13 7:10 ` Hammer Hsieh 2021-12-13 17:26 ` Rob Herring 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh 1 sibling, 1 reply; 16+ messages in thread From: Hammer Hsieh @ 2021-12-13 7:10 UTC (permalink / raw) To: gregkh, robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel Cc: wells.lu, Hammer Hsieh Add bindings doc for Sunplus SoC UART Driver Signed-off-by: Hammer Hsieh <hammer.hsieh@sunplus.com> --- Changes in v4: - no change. - Reviewed-by : Rob Herring <robh@kernel.org> 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 <hammer.hsieh@sunplus.com> + +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: netdev@vger.kernel.org S: Maintained F: drivers/net/ethernet/dlink/sundance.c +SUNPLUS UART DRIVER +M: Hammer Hsieh <hammer.hsieh@sunplus.com> +S: Maintained +F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml + SUPERH M: Yoshinori Sato <ysato@users.sourceforge.jp> M: Rich Felker <dalias@libc.org> -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver 2021-12-13 7:10 ` [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver Hammer Hsieh @ 2021-12-13 17:26 ` Rob Herring 0 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2021-12-13 17:26 UTC (permalink / raw) To: Hammer Hsieh Cc: gregkh, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 <hammer.hsieh@sunplus.com> > --- > Changes in v4: > - no change. > - Reviewed-by : Rob Herring <robh@kernel.org> 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 <hammer.hsieh@sunplus.com> > + > +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: netdev@vger.kernel.org > S: Maintained > F: drivers/net/ethernet/dlink/sundance.c > > +SUNPLUS UART DRIVER > +M: Hammer Hsieh <hammer.hsieh@sunplus.com> > +S: Maintained > +F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml > + > SUPERH > M: Yoshinori Sato <ysato@users.sourceforge.jp> > M: Rich Felker <dalias@libc.org> > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:10 [PATCH v5 0/2] Add UART driver for Suplus SP7021 SoC Hammer Hsieh 2021-12-13 7:10 ` [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver Hammer Hsieh @ 2021-12-13 7:10 ` Hammer Hsieh 2021-12-13 7:47 ` Jiri Slaby ` (3 more replies) 1 sibling, 4 replies; 16+ messages in thread From: Hammer Hsieh @ 2021-12-13 7:10 UTC (permalink / raw) To: gregkh, robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel Cc: wells.lu, Hammer Hsieh Add Sunplus SoC UART Driver Signed-off-by: Hammer Hsieh <hammer.hsieh@sunplus.com> --- 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 <hammer.hsieh@sunplus.com> S: Maintained F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml +F: drivers/tty/serial/sunplus-uart.c SUPERH M: Yoshinori Sato <ysato@users.sourceforge.jp> 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 <hammer.hsieh@sunplus.com> + */ +#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 <hammer.hsieh@sunplus.com>"); +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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh @ 2021-12-13 7:47 ` Jiri Slaby 2021-12-13 11:37 ` Hammer Hsieh 謝宏孟 2021-12-20 15:47 ` Greg KH ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Jiri Slaby @ 2021-12-13 7:47 UTC (permalink / raw) To: Hammer Hsieh, gregkh, robh+dt, linux-serial, devicetree, linux-kernel, p.zabel Cc: wells.lu, Hammer Hsieh On 13. 12. 21, 8:10, Hammer Hsieh wrote: > Add Sunplus SoC UART Driver > > Signed-off-by: Hammer Hsieh <hammer.hsieh@sunplus.com> ... > --- /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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:47 ` Jiri Slaby @ 2021-12-13 11:37 ` Hammer Hsieh 謝宏孟 0 siblings, 0 replies; 16+ messages in thread From: Hammer Hsieh 謝宏孟 @ 2021-12-13 11:37 UTC (permalink / raw) To: Jiri Slaby, Hammer Hsieh, gregkh, robh+dt, linux-serial, devicetree, linux-kernel, p.zabel Cc: Wells Lu 呂芳騰 Hi, Jiri Slaby: Thanks for your review. My response is below in mail. Best Regards, Hammer Hsieh > -----Original Message----- > From: Jiri Slaby <jirislaby@kernel.org> > Sent: Monday, December 13, 2021 3:47 PM > To: Hammer Hsieh <hammerh0314@gmail.com>; gregkh@linuxfoundation.org; > robh+dt@kernel.org; linux-serial@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; p.zabel@pengutronix.de > Cc: Wells Lu 呂芳騰 <wells.lu@sunplus.com>; Hammer Hsieh 謝宏孟 > <hammer.hsieh@sunplus.com> > 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 <hammer.hsieh@sunplus.com> > > ... > > > --- /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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh 2021-12-13 7:47 ` Jiri Slaby @ 2021-12-20 15:47 ` Greg KH 2021-12-20 15:49 ` Greg KH 2021-12-20 15:51 ` Greg KH 3 siblings, 0 replies; 16+ messages in thread From: Greg KH @ 2021-12-20 15:47 UTC (permalink / raw) To: Hammer Hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh 2021-12-13 7:47 ` Jiri Slaby 2021-12-20 15:47 ` Greg KH @ 2021-12-20 15:49 ` Greg KH 2021-12-20 15:51 ` Greg KH 3 siblings, 0 replies; 16+ messages in thread From: Greg KH @ 2021-12-20 15:49 UTC (permalink / raw) To: Hammer Hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh ` (2 preceding siblings ...) 2021-12-20 15:49 ` Greg KH @ 2021-12-20 15:51 ` Greg KH 2021-12-21 8:14 ` hammer hsieh 3 siblings, 1 reply; 16+ messages in thread From: Greg KH @ 2021-12-20 15:51 UTC (permalink / raw) To: Hammer Hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-20 15:51 ` Greg KH @ 2021-12-21 8:14 ` hammer hsieh 2021-12-21 8:21 ` Greg KH 0 siblings, 1 reply; 16+ messages in thread From: hammer hsieh @ 2021-12-21 8:14 UTC (permalink / raw) To: Greg KH Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh Greg KH <gregkh@linuxfoundation.org> 於 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-21 8:14 ` hammer hsieh @ 2021-12-21 8:21 ` Greg KH 2021-12-24 7:16 ` hammer hsieh 0 siblings, 1 reply; 16+ messages in thread From: Greg KH @ 2021-12-21 8:21 UTC (permalink / raw) To: hammer hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh On Tue, Dec 21, 2021 at 04:14:16PM +0800, hammer hsieh wrote: > Greg KH <gregkh@linuxfoundation.org> 於 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-21 8:21 ` Greg KH @ 2021-12-24 7:16 ` hammer hsieh 2021-12-24 8:59 ` Greg KH 0 siblings, 1 reply; 16+ messages in thread From: hammer hsieh @ 2021-12-24 7:16 UTC (permalink / raw) To: Greg KH Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 <gregkh@linuxfoundation.org> 於 2021年12月21日 週二 下午4:21寫道： > > On Tue, Dec 21, 2021 at 04:14:16PM +0800, hammer hsieh wrote: > > Greg KH <gregkh@linuxfoundation.org> 於 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-24 7:16 ` hammer hsieh @ 2021-12-24 8:59 ` Greg KH 2021-12-24 9:05 ` hammer hsieh 0 siblings, 1 reply; 16+ messages in thread From: Greg KH @ 2021-12-24 8:59 UTC (permalink / raw) To: hammer hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-24 8:59 ` Greg KH @ 2021-12-24 9:05 ` hammer hsieh 2021-12-24 9:21 ` hammer hsieh 0 siblings, 1 reply; 16+ messages in thread From: hammer hsieh @ 2021-12-24 9:05 UTC (permalink / raw) To: Greg KH Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 <gregkh@linuxfoundation.org> 於 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-24 9:05 ` hammer hsieh @ 2021-12-24 9:21 ` hammer hsieh 2021-12-30 12:18 ` Greg KH 0 siblings, 1 reply; 16+ messages in thread From: hammer hsieh @ 2021-12-24 9:21 UTC (permalink / raw) To: Greg KH Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 <hammerh0314@gmail.com> 於 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 <gregkh@linuxfoundation.org> 於 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/2] serial:sunplus-uart:Add Sunplus SoC UART Driver 2021-12-24 9:21 ` hammer hsieh @ 2021-12-30 12:18 ` Greg KH 0 siblings, 0 replies; 16+ messages in thread From: Greg KH @ 2021-12-30 12:18 UTC (permalink / raw) To: hammer hsieh Cc: robh+dt, linux-serial, devicetree, linux-kernel, jirislaby, p.zabel, wells.lu, Hammer Hsieh 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-12-30 12:18 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-13 7:10 [PATCH v5 0/2] Add UART driver for Suplus SP7021 SoC Hammer Hsieh 2021-12-13 7:10 ` [PATCH v5 1/2] dt-bindings:serial:Add bindings doc for Sunplus SoC UART Driver Hammer Hsieh 2021-12-13 17:26 ` Rob Herring 2021-12-13 7:10 ` [PATCH v5 2/2] serial:sunplus-uart:Add " Hammer Hsieh 2021-12-13 7:47 ` Jiri Slaby 2021-12-13 11:37 ` Hammer Hsieh 謝宏孟 2021-12-20 15:47 ` Greg KH 2021-12-20 15:49 ` Greg KH 2021-12-20 15:51 ` Greg KH 2021-12-21 8:14 ` hammer hsieh 2021-12-21 8:21 ` Greg KH 2021-12-24 7:16 ` hammer hsieh 2021-12-24 8:59 ` Greg KH 2021-12-24 9:05 ` hammer hsieh 2021-12-24 9:21 ` hammer hsieh 2021-12-30 12:18 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).