Re: [PATCH v2 4/4] drivers/tty/serial: add LiteUART driver

[Rob Herring <robh@kernel.org>]

On Wed, Oct 23, 2019 at 11:47:23AM +0200, Mateusz Holenko wrote:
> From: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> 
> This commit adds driver for the FPGA-based LiteUART serial controller
> from LiteX SoC builder.
> 
> The current implementation supports LiteUART configured
> for 32 bit data width and 8 bit CSR bus width.
> 
> It does not support IRQ.
> 
> Signed-off-by: Filip Kokosinski <fkokosinski@internships.antmicro.com>
> Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
> ---
> Changes in v2:
> - used register access functions from newly introduced litex.h
> - patch number changed from 3 to 4
> 
>  MAINTAINERS                      |   1 +
>  drivers/tty/serial/Kconfig       |  30 +++
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/liteuart.c    | 373 +++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  5 files changed, 408 insertions(+)
>  create mode 100644 drivers/tty/serial/liteuart.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1dc783c9edb7..c24a37833e78 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9499,6 +9499,7 @@ M:	Mateusz Holenko <mholenko@antmicro.com>
>  S:	Maintained
>  F:	include/linux/litex.h
>  F:	Documentation/devicetree/bindings/*/litex,*.yaml
> +F:	drivers/tty/serial/liteuart.c
>  
>  LIVE PATCHING
>  M:	Josh Poimboeuf <jpoimboe@redhat.com>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 4789b5d62f63..b01fe12a1411 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1571,6 +1571,36 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
>  	  receives all kernel messages and warnings and which allows logins in
>  	  single user mode).
>  
> +config SERIAL_LITEUART
> +	tristate "LiteUART serial port support"
> +	depends on HAS_IOMEM
> +	depends on OF
> +	select SERIAL_CORE
> +	help
> +	  This driver is for the FPGA-based LiteUART serial controller from LiteX
> +	  SoC builder.
> +
> +	  Say 'Y' here if you wish to use the LiteUART serial controller.
> +	  Otherwise, say 'N'.
> +
> +config SERIAL_LITEUART_NR_PORTS
> +	int "Number of LiteUART ports"
> +	depends on SERIAL_LITEUART
> +	default "1"
> +	help
> +	  Set this to the number of serial ports you want the driver
> +	  to support.

It's better if the driver supports a dynamic number of ports.

> +
> +config SERIAL_LITEUART_CONSOLE
> +	bool "LiteUART serial port console support"
> +	depends on SERIAL_LITEUART=y
> +	select SERIAL_CORE_CONSOLE
> +	help
> +	  Say 'Y' here if you wish to use the FPGA-based LiteUART serial controller
> +	  from LiteX SoC builder as the system console (the system console is the
> +	  device which receives all kernel messages and warnings and which allows
> +	  logins in single user mode). Otherwise, say 'N'.
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 863f47056539..c8d7e2046284 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
>  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
>  
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
> new file mode 100644
> index 000000000000..e142f78df57a
> --- /dev/null
> +++ b/drivers/tty/serial/liteuart.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LiteUART serial controller (LiteX) Driver
> + *
> + * Copyright (C) 2019 Antmicro Ltd <www.antmicro.com>
> + */
> +
> +#include <linux/console.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/serial.h>
> +#include <linux/serial_core.h>
> +#include <linux/timer.h>
> +#include <linux/tty_flip.h>
> +#include <linux/litex.h>
> +
> +/* module-related defines */
> +#define DRIVER_NAME	"liteuart"
> +#define DRIVER_MAJOR	0
> +#define DRIVER_MINOR	0
> +#define DEV_NAME	"ttyLXU"
> +
> +/* base address offsets */
> +#define OFF_RXTX	0x00
> +#define OFF_TXFULL	0x04
> +#define OFF_RXEMPTY	0x08
> +#define OFF_EV_STATUS	0x0c
> +#define OFF_EV_PENDING	0x10
> +#define OFF_EV_ENABLE	0x14
> +
> +/* events */
> +#define EV_TX	0x1
> +#define EV_RX	0x2
> +
> +struct liteuart_port {
> +	struct uart_port port;
> +	struct timer_list timer;
> +};
> +
> +#define to_liteuart_port(port)	container_of(port, struct liteuart_port, port)
> +
> +static struct liteuart_port liteuart_ports[CONFIG_SERIAL_LITEUART_NR_PORTS];
> +
> +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> +static struct console liteuart_console;
> +#endif
> +
> +static struct uart_driver liteuart_driver = {
> +	.owner = THIS_MODULE,
> +	.driver_name = DRIVER_NAME,
> +	.dev_name = DEV_NAME,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
> +	.nr = CONFIG_SERIAL_LITEUART_NR_PORTS,
> +#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
> +	.cons = &liteuart_console,
> +#endif
> +};
> +
> +static void liteuart_timer(struct timer_list *t)
> +{
> +	struct liteuart_port *uart = from_timer(uart, t, timer);
> +	struct uart_port *port = &uart->port;
> +	unsigned char __iomem *membase = port->membase;
> +	unsigned int flg = TTY_NORMAL;
> +	int ch;
> +	unsigned int status;
> +
> +	while ((status = !LITEX_READ_REG_OFF(membase, OFF_RXEMPTY)) == 1) {
> +		ch = LITEX_READ_REG_OFF(membase, OFF_RXTX);
> +		port->icount.rx++;
> +
> +		/* necessary for RXEMPTY to refresh its value */
> +		LITEX_WRITE_REG_OFF(EV_TX | EV_RX, membase, OFF_EV_PENDING);
> +
> +		/* no overflow bits in status */
> +		if (!(uart_handle_sysrq_char(port, ch)))
> +			uart_insert_char(port, status, 0, ch, flg);
> +
> +		tty_flip_buffer_push(&port->state->port);
> +	}
> +
> +	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> +}
> +
> +static void liteuart_putchar(struct uart_port *port, int ch)
> +{
> +	while (LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> +		cpu_relax();
> +
> +	LITEX_WRITE_REG_OFF(ch, port->membase, OFF_RXTX);
> +}
> +
> +static unsigned int liteuart_tx_empty(struct uart_port *port)
> +{
> +	/* not really tx empty, just checking if tx is not full */
> +	if (!LITEX_READ_REG_OFF(port->membase, OFF_TXFULL))
> +		return TIOCSER_TEMT;
> +
> +	return 0;
> +}
> +
> +static void liteuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +	/* modem control register is not present in LiteUART */
> +}
> +
> +static unsigned int liteuart_get_mctrl(struct uart_port *port)
> +{
> +	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +}
> +
> +static void liteuart_stop_tx(struct uart_port *port)
> +{
> +}
> +
> +static void liteuart_start_tx(struct uart_port *port)
> +{
> +	struct circ_buf *xmit = &port->state->xmit;
> +	unsigned char ch;
> +
> +	if (unlikely(port->x_char)) {
> +		LITEX_WRITE_REG_OFF(port->x_char, port->membase, OFF_RXTX);
> +		port->icount.tx++;
> +		port->x_char = 0;
> +	} else if (!uart_circ_empty(xmit)) {
> +		while (xmit->head != xmit->tail) {
> +			ch = xmit->buf[xmit->tail];
> +			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +			port->icount.tx++;
> +			liteuart_putchar(port, ch);
> +		}
> +	}
> +
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(port);
> +}
> +
> +static void liteuart_stop_rx(struct uart_port *port)
> +{
> +	struct liteuart_port *uart = to_liteuart_port(port);
> +
> +	/* just delete timer */
> +	del_timer(&uart->timer);
> +}
> +
> +static void liteuart_break_ctl(struct uart_port *port, int break_state)
> +{
> +	/* LiteUART doesn't support sending break signal */
> +}
> +
> +static int liteuart_startup(struct uart_port *port)
> +{
> +	struct liteuart_port *uart = to_liteuart_port(port);
> +
> +	/* disable events */
> +	LITEX_WRITE_REG_OFF(0, port->membase, OFF_EV_ENABLE);
> +
> +	/* prepare timer for polling */
> +	timer_setup(&uart->timer, liteuart_timer, 0);
> +	mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
> +
> +	return 0;
> +}
> +
> +static void liteuart_shutdown(struct uart_port *port)
> +{
> +}
> +
> +static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
> +				 struct ktermios *old)
> +{
> +	unsigned int baud;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	/* update baudrate */
> +	baud = uart_get_baud_rate(port, new, old, 0, 460800);
> +	uart_update_timeout(port, new->c_cflag, baud);
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static const char *liteuart_type(struct uart_port *port)
> +{
> +	return (port->type == PORT_LITEUART) ? DRIVER_NAME : NULL;
> +}
> +
> +static void liteuart_release_port(struct uart_port *port)
> +{
> +}
> +
> +static int liteuart_request_port(struct uart_port *port)
> +{
> +	return 0;
> +}
> +
> +static void liteuart_config_port(struct uart_port *port, int flags)
> +{
> +	if (flags & UART_CONFIG_TYPE)
> +		port->type = PORT_LITEUART;
> +}
> +
> +static int liteuart_verify_port(struct uart_port *port,
> +				struct serial_struct *ser)
> +{
> +	if (port->type != PORT_UNKNOWN && ser->type != PORT_LITEUART)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct uart_ops liteuart_ops = {
> +	.tx_empty	= liteuart_tx_empty,
> +	.set_mctrl	= liteuart_set_mctrl,
> +	.get_mctrl	= liteuart_get_mctrl,
> +	.stop_tx	= liteuart_stop_tx,
> +	.start_tx	= liteuart_start_tx,
> +	.stop_rx	= liteuart_stop_rx,
> +	.break_ctl	= liteuart_break_ctl,
> +	.startup	= liteuart_startup,
> +	.shutdown	= liteuart_shutdown,
> +	.set_termios	= liteuart_set_termios,
> +	.type		= liteuart_type,
> +	.release_port	= liteuart_release_port,
> +	.request_port	= liteuart_request_port,
> +	.config_port	= liteuart_config_port,
> +	.verify_port	= liteuart_verify_port,
> +};
> +
> +static int liteuart_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct liteuart_port *uart;
> +	struct uart_port *port;
> +	int dev_id;
> +
> +	/* no device tree */
> +	if (!np)
> +		return -ENODEV;
> +
> +	dev_id = of_alias_get_id(np, "serial");
> +	if (dev_id < 0 || dev_id >= CONFIG_SERIAL_LITEUART_NR_PORTS)
> +		return -EINVAL;

aliases should be optional. There's a helper to get a free index without 
an alias you can use.