Hi Simon,
On 14.07.2017 15:50, Simon Glass wrote:
On 13 July 2017 at 05:33, Stefan Roese sr@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@denx.de Cc: Simon Glass sjg@chromium.org Cc: Bin Meng bmeng.cn@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@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_SERIALdefault nhelpEnable RX interrupt buffer support for the serial driver.This enables pasting longer strings, even when the RX FIFOof the UART is not big enough (e.g. 16 bytes on the normalNS16550).- 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?
All interrupts are disabled before booting into the OS, so strictly it should not be necessary. But yes, I think its a good idea to remove the irq handler upon driver device. I'll change this in v2.
/* 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;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.
Sure. Will add in v2.
Thanks, Stefan