[U-Boot] [PATCH] serial: ns16550: Add RX interrupt buffer support

Simon Glass sjg at chromium.org
Fri Jul 14 13:50:57 UTC 2017


On 13 July 2017 at 05:33, Stefan Roese <sr at denx.de> wrote:
> Pasting longer lines into the U-Boot console prompt sometimes leads to
> characters missing. One problem here is the small 16-byte FIFO of the
> legacy NS16550 UART, e.g. on x86 platforms.
>
> This patch now introduces a Kconfig option to enable RX interrupt
> buffer support for NS16550 style UARTs. With this option enabled, I was
> able paste really long lines into the U-Boot console, without any
> characters missing.
>
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> ---
>  drivers/serial/Kconfig   |  10 +++++
>  drivers/serial/ns16550.c | 106 ++++++++++++++++++++++++++++++++++++++++++++---
>  include/ns16550.h        |   6 +++
>  3 files changed, 117 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

nits below

>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index b7dd2ac103..8284febae3 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -64,6 +64,16 @@ config DM_SERIAL
>           implements serial_putc() etc. The uclass interface is
>           defined in include/serial.h.
>
> +config SERIAL_IRQ_BUFFER
> +       bool "Enable RX interrupt buffer for serial input"
> +       depends on DM_SERIAL
> +       default n
> +       help
> +         Enable RX interrupt buffer support for the serial driver.
> +         This enables pasting longer strings, even when the RX FIFO
> +         of the UART is not big enough (e.g. 16 bytes on the normal
> +         NS16550).
> +
>  config SPL_DM_SERIAL
>         bool "Enable Driver Model for serial drivers in SPL"
>         depends on DM_SERIAL
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index e0e70244ce..686c088e1d 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -315,6 +315,80 @@ DEBUG_UART_FUNCS
>  #endif
>
>  #ifdef CONFIG_DM_SERIAL
> +
> +#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
> +
> +#define BUF_COUNT      256
> +
> +static void rx_fifo_to_buf(struct udevice *dev)
> +{
> +       struct NS16550 *const com_port = dev_get_priv(dev);
> +       struct ns16550_platdata *plat = dev->platdata;
> +
> +       /* Read all available chars into buffer */
> +       while ((serial_in(&com_port->lsr) & UART_LSR_DR)) {
> +               plat->buf[plat->wr_ptr++] = serial_in(&com_port->rbr);
> +               plat->wr_ptr %= BUF_COUNT;
> +       }
> +}
> +
> +static int rx_pending(struct udevice *dev)
> +{
> +       struct ns16550_platdata *plat = dev->platdata;
> +
> +       /*
> +        * At startup it may happen, that some already received chars are
> +        * "stuck" in the RX FIFO, even with the interrupt enabled. This
> +        * RX FIFO flushing makes sure, that these chars are read out and
> +        * the RX interrupts works as expected.
> +        */
> +       rx_fifo_to_buf(dev);
> +
> +       return plat->rd_ptr != plat->wr_ptr ? 1 : 0;
> +}
> +
> +static int rx_get(struct udevice *dev)
> +{
> +       struct ns16550_platdata *plat = dev->platdata;
> +       char val;
> +
> +       val = plat->buf[plat->rd_ptr++];
> +       plat->rd_ptr %= BUF_COUNT;
> +
> +       return val;
> +}
> +
> +void ns16550_handle_irq(void *data)
> +{
> +       struct udevice *dev = (struct udevice *)data;
> +       struct NS16550 *const com_port = dev_get_priv(dev);
> +
> +       /* Check if interrupt is pending */
> +       if (serial_in(&com_port->iir) & UART_IIR_NO_INT)
> +               return;
> +
> +       /* Flush all available characters from the RX FIFO into the RX buffer */
> +       rx_fifo_to_buf(dev);
> +}
> +
> +#else /* CONFIG_SERIAL_IRQ_BUFFER */
> +
> +static int rx_pending(struct udevice *dev)
> +{
> +       struct NS16550 *const com_port = dev_get_priv(dev);
> +
> +       return serial_in(&com_port->lsr) & UART_LSR_DR ? 1 : 0;
> +}
> +
> +static int rx_get(struct udevice *dev)
> +{
> +       struct NS16550 *const com_port = dev_get_priv(dev);
> +
> +       return serial_in(&com_port->rbr);
> +}
> +
> +#endif /* CONFIG_SERIAL_IRQ_BUFFER */
> +
>  static int ns16550_serial_putc(struct udevice *dev, const char ch)
>  {
>         struct NS16550 *const com_port = dev_get_priv(dev);
> @@ -340,19 +414,17 @@ static int ns16550_serial_pending(struct udevice *dev, bool input)
>         struct NS16550 *const com_port = dev_get_priv(dev);
>
>         if (input)
> -               return serial_in(&com_port->lsr) & UART_LSR_DR ? 1 : 0;
> +               return rx_pending(dev);
>         else
>                 return serial_in(&com_port->lsr) & UART_LSR_THRE ? 0 : 1;
>  }
>
>  static int ns16550_serial_getc(struct udevice *dev)
>  {
> -       struct NS16550 *const com_port = dev_get_priv(dev);
> -
> -       if (!(serial_in(&com_port->lsr) & UART_LSR_DR))
> +       if (!ns16550_serial_pending(dev, true))
>                 return -EAGAIN;
>
> -       return serial_in(&com_port->rbr);
> +       return rx_get(dev);
>  }
>
>  static int ns16550_serial_setbrg(struct udevice *dev, int baudrate)
> @@ -375,6 +447,21 @@ int ns16550_serial_probe(struct udevice *dev)
>         com_port->plat = dev_get_platdata(dev);
>         NS16550_init(com_port, -1);
>
> +#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
> +       if (gd->flags & GD_FLG_RELOC) {
> +               struct ns16550_platdata *plat = dev->platdata;
> +
> +               /* Allocate the RX buffer */
> +               plat->buf = malloc(BUF_COUNT);
> +
> +               /* Install the interrupt handler */
> +               irq_install_handler(plat->irq, ns16550_handle_irq, dev);

Do we need to remove this in the remove() method?

> +
> +               /* Enable RX interrupts */
> +               serial_out(UART_IER_RDI, &com_port->ier);
> +       }
> +#endif
> +
>         return 0;
>  }
>
> @@ -459,6 +546,15 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
>         if (port_type == PORT_JZ4780)
>                 plat->fcr |= UART_FCR_UME;
>
> +#if CONFIG_IS_ENABLED(SERIAL_IRQ_BUFFER)
> +       plat->irq = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
> +                                  "interrupts", 0);
> +       if (!plat->irq) {
> +               debug("ns16550 interrupt not provided\n");
> +               return -EINVAL;
> +       }
> +#endif
> +
>         return 0;
>  }
>  #endif
> diff --git a/include/ns16550.h b/include/ns16550.h
> index 5fcbcd2e74..70d1ec8e6a 100644
> --- a/include/ns16550.h
> +++ b/include/ns16550.h
> @@ -58,6 +58,12 @@ struct ns16550_platdata {
>         int clock;
>         int reg_offset;
>         u32 fcr;
> +
> +       int irq;
> +
> +       char *buf;
> +       int rd_ptr;
> +       int wr_ptr;

Please add comments to document these members.

>  };
>
>  struct udevice;
> --
> 2.13.3
>

Regards,
Simon


More information about the U-Boot mailing list