From bc1fe2a82219772934a7701bf112206fce507869 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Mon, 5 Apr 2021 17:49:20 +0200 Subject: [PATCH 01/10] libplatsupport: trivial comment formatting changes Signed-off-by: Axel Heider --- libplatsupport/include/platsupport/serial.h | 5 +++-- libplatsupport/src/mach/imx/serial/serial.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libplatsupport/include/platsupport/serial.h b/libplatsupport/include/platsupport/serial.h index aaee340cb..2c07498b9 100644 --- a/libplatsupport/include/platsupport/serial.h +++ b/libplatsupport/include/platsupport/serial.h @@ -13,8 +13,9 @@ **** Serial device flags **** *****************************/ -/* Auto-send CR(Carriage Return) after each "\n". - * NOTE: This flag should be set by default. */ +/* Auto-send CR (Carriage Return, "\r") before each "\n". All UART drivers + * should set this flag by default, so the UART can be used as a console. + */ #define SERIAL_AUTO_CR BIT(0) /*****************************/ diff --git a/libplatsupport/src/mach/imx/serial/serial.c b/libplatsupport/src/mach/imx/serial/serial.c index 280196587..3a5921807 100644 --- a/libplatsupport/src/mach/imx/serial/serial.c +++ b/libplatsupport/src/mach/imx/serial/serial.c @@ -261,8 +261,8 @@ int uart_init( #ifdef CONFIG_PLAT_IMX6 #include - /* The UART1 on the IMX6 has the problem that the MUX is not correctly set, - * and the RX PIN is not routed correctly. + /* The UART1 on the IMX6 has the problem that the MUX is not correctly set, and the RX PIN is + * not routed correctly. */ if ((defn->id == IMX_UART1) && mux_sys_valid(&ops->mux_sys)) { if (mux_feature_enable(&ops->mux_sys, MUX_UART1, 0)) { From 084f7f5fc767d013a0f8cd6f4f1277485cf44907 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Fri, 12 Jan 2024 12:21:23 +0100 Subject: [PATCH 02/10] ibplatsupport: remove duplicate space Signed-off-by: Axel Heider --- libplatsupport/src/plat/fvp/serial.c | 2 +- libplatsupport/src/plat/hikey/serial.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libplatsupport/src/plat/fvp/serial.c b/libplatsupport/src/plat/fvp/serial.c index 5b8db1ad3..aef2f5a86 100644 --- a/libplatsupport/src/plat/fvp/serial.c +++ b/libplatsupport/src/plat/fvp/serial.c @@ -60,7 +60,7 @@ int uart_init(const struct dev_defn* defn, return -1; } - /* Set up all the device properties. */ + /* Set up all the device properties. */ dev->id = defn->id; dev->vaddr = (void*)vaddr; dev->read = &uart_read; diff --git a/libplatsupport/src/plat/hikey/serial.c b/libplatsupport/src/plat/hikey/serial.c index e9f30d70b..8dd0118d0 100644 --- a/libplatsupport/src/plat/hikey/serial.c +++ b/libplatsupport/src/plat/hikey/serial.c @@ -57,7 +57,7 @@ int uart_init(const struct dev_defn* defn, return -1; } - /* Set up all the device properties. */ + /* Set up all the device properties. */ dev->id = defn->id; dev->vaddr = (void*)vaddr; dev->read = &uart_read; From 7254d235ac45d5bd2b0f552309b460e93a8b9f31 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Fri, 12 Jan 2024 12:25:53 +0100 Subject: [PATCH 03/10] libplatsupport: improve comments Signed-off-by: Axel Heider --- libplatsupport/src/serial.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libplatsupport/src/serial.c b/libplatsupport/src/serial.c index 2b356ce47..b1529e4ff 100644 --- a/libplatsupport/src/serial.c +++ b/libplatsupport/src/serial.c @@ -18,6 +18,14 @@ ssize_t uart_write( { const unsigned char *data = (const unsigned char *)vdata; for (int i = 0; i < count; i++) { + /* TODO: Some UART drivers implement a busy waiting until there is space + * in the TX FIFO, others return an error in this case. We should + * have a mechanism to allow the user to specify the desired + * behavior, so we don't have to try to guess it. For a standard + * logging console, the behavior is likely to do a blocking wait, + * while a data UART may prefer nog to block and let the caller + * handle the waiting. + */ if (uart_putchar(d, data[i]) < 0) { return i; } From 92456cf1adaf4da05dbbc603e425f372e631efd0 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 8 Dec 2021 19:04:54 +0100 Subject: [PATCH 04/10] libplatsupport: cleanup UART code --- libplatsupport/CMakeLists.txt | 10 ++ .../bcm => drivers/uart_pl011}/pl011_uart.c | 0 .../src/drivers/uart_pl011/uart_pl011.c | 161 ++++++++++++++++++ libplatsupport/src/mach/exynos/serial.c | 84 ++++++--- libplatsupport/src/mach/nvidia/serial.c | 48 +++++- libplatsupport/src/mach/zynq/serial.c | 60 +++++-- libplatsupport/src/plat/fvp/serial.c | 75 -------- libplatsupport/src/plat/hifive/uart.c | 72 +++++--- libplatsupport/src/plat/hikey/serial.c | 72 -------- libplatsupport/src/plat/odroidc2/serial.c | 78 +++++++-- libplatsupport/src/plat/omap3/serial.c | 64 +++++-- libplatsupport/src/plat/pc99/serial.c | 141 ++++++++++----- .../src/plat/qemu-arm-virt/serial.c | 74 -------- libplatsupport/src/plat/rockpro64/serial.c | 87 +++++++--- libplatsupport/src/serial.c | 5 +- 15 files changed, 661 insertions(+), 370 deletions(-) rename libplatsupport/src/{mach/bcm => drivers/uart_pl011}/pl011_uart.c (100%) create mode 100644 libplatsupport/src/drivers/uart_pl011/uart_pl011.c delete mode 100644 libplatsupport/src/plat/fvp/serial.c delete mode 100644 libplatsupport/src/plat/hikey/serial.c delete mode 100644 libplatsupport/src/plat/qemu-arm-virt/serial.c diff --git a/libplatsupport/CMakeLists.txt b/libplatsupport/CMakeLists.txt index 0bae33f2c..126afe774 100644 --- a/libplatsupport/CMakeLists.txt +++ b/libplatsupport/CMakeLists.txt @@ -150,6 +150,16 @@ if(KernelPlatformZynqmp) target_include_directories(platsupport PUBLIC plat_include/zynqmp) endif() +if(KernelPlatformHikey OR KernelPlatformFVP OR KernelPlatformQEMUArmVirt) + target_sources(platsupport PRIVATE "src/drivers/uart_pl011/uart_pl011.c") +endif() + +# ToDo: unify RasPi3 and RasPi4 +# if(KernelPlatformRpi3 OR KernelPlatformRpi4) +# target_sources(platsupport PRIVATE "src/drivers/uart_raspi/uart_raspi.c") +# target_include_directories(platsupport PUBLIC "driver-include/uart_raspi") +# endif() + if(NOT "${LibPlatSupportMach}" STREQUAL "") target_include_directories(platsupport PUBLIC mach_include/${LibPlatSupportMach}) endif() diff --git a/libplatsupport/src/mach/bcm/pl011_uart.c b/libplatsupport/src/drivers/uart_pl011/pl011_uart.c similarity index 100% rename from libplatsupport/src/mach/bcm/pl011_uart.c rename to libplatsupport/src/drivers/uart_pl011/pl011_uart.c diff --git a/libplatsupport/src/drivers/uart_pl011/uart_pl011.c b/libplatsupport/src/drivers/uart_pl011/uart_pl011.c new file mode 100644 index 000000000..4bdf0a1de --- /dev/null +++ b/libplatsupport/src/drivers/uart_pl011/uart_pl011.c @@ -0,0 +1,161 @@ +/* + * ARM PL011 UART driver + * + * Copyright 2017, Data61, CSIRO (ABN 41 687 119 230) + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include "../../chardev.h" + + +#define PL011_UART_RHR_MASK MASK(8) +#define PL011_UART_FR_TXFF BIT(5) +#define PL011_UART_FR_RXFE BIT(4) + +typedef volatile struct { + uint32_t dr; /* 0x00 */ + uint32_t reg_04; /* 0x04 */ + uint32_t reg_08; /* 0x08 */ + uint32_t reg_0C; /* 0x0C */ + uint32_t reg_10; /* 0x10 */ + uint32_t reg_14; /* 0x14 */ + uint32_t fr; /* 0x18 */ + uint32_t reg_1C; /* 0x1C */ + uint32_t reg_20; /* 0x20 */ + uint32_t reg_24; /* 0x24 */ + uint32_t reg_28; /* 0x28 */ + uint32_t reg_2C; /* 0x2C */ + uint32_t reg_30; /* 0x30 */ + uint32_t reg_34; /* 0x34 */ + uint32_t imsc; /* 0x38 */ + uint32_t reg_3C; /* 0x3C */ + uint32_t reg_40; /* 0x40 */ + uint32_t cr; /* 0x44 */ +} uart_pl011_regs_t; + + + +static uart_pl011_regs_t* get_uart_regs(ps_chardevice_t *d) +{ + return (uart_pl011_regs_t *)(d->vaddr); +} + +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static int internal_uart_is_tx_fifo_full(uart_pl011_regs_t *regs) +{ + return regs->fr & PL011_UARTFR_TX_FF; +} + +static void internal_uart_tx_byte(uart_pl011_regs_t *regs, uint8_t byte) +{ + regs->dr = byte; +} + +static uint8_t internal_uart_rx_byte(uart_pl011_regs_t *regs) +{ + return (uint8_t)(regs->dr & PL011_UART_RHR_MASK); +} + +static int internal_uart_is_rx_empty(uart_pl011_regs_t *regs) +{ + return reg->fr & PL011_UART_FR_RXFE; +} + +static void internal_uart_busy_wait_tx_ready(uart_pl011_regs_t *regs) +{ + while (internal_uart_is_tx_fifo_full(regs)) { + /* busy waiting loop */ + } +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + +int uart_getchar(ps_chardevice_t *dev) +{ + uart_pl011_regs_t *regs = get_uart_regs(dev); + + if (internal_uart_is_rx_empty(regs)) { + return -1; + } + + return internal_uart_rx_byte(regs); +} + +int uart_putchar(ps_chardevice_t* dev, int c) +{ + uart_pl011_regs_t *regs = get_uart_regs(dev); + + /* Check if the TX FIFO has space. If not, then fail if SERIAL_NONBLOCKING + * is set and do busy waiting otherwise. + */ + if (internal_uart_is_tx_fifo_full(regs)) + if (d->flags & SERIAL_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(regs); + } + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + internal_uart_busy_wait_tx_ready(regs); + + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. Since we want to make CR/LR atomic, a + * blocking wait will be used to ensure the LF can be sent also. + */ + if (byte == '\n' && (d->flags & SERIAL_AUTO_CR)) { + internal_uart_tx_byte(regs, '\r'); + internal_uart_busy_wait_tx_ready(regs); + } + + internal_uart_tx_byte(regs, byte); + + return byte; +} + +static void +uart_handle_irq(ps_chardevice_t* dev) +{ + uart_pl011_regs_t *regs = get_uart_regs(dev); + + regs->cr = 0x7f0; +} + +int uart_init(const struct dev_defn* defn, + const ps_io_ops_t* ops, + ps_chardevice_t* dev) +{ + memset(dev, 0, sizeof(*dev)); + + uart_pl011_regs_t *regs = (uart_pl011_regs_t *)chardev_map(defn, ops); + if (regs == NULL) { + return -1; + } + + /* Set up all the device properties. */ + dev->id = defn->id; + dev->vaddr = (void *)regs; + dev->read = &uart_read; + dev->write = &uart_write; + dev->handle_irq = &uart_handle_irq; + dev->irqs = defn->irqs; + dev->ioops = *ops; + dev->flags = SERIAL_AUTO_CR; + + regs->imsc = 0x50; + + return 0; +} diff --git a/libplatsupport/src/mach/exynos/serial.c b/libplatsupport/src/mach/exynos/serial.c index 0b9e7b7a9..4576fefe7 100644 --- a/libplatsupport/src/mach/exynos/serial.c +++ b/libplatsupport/src/mach/exynos/serial.c @@ -87,7 +87,9 @@ #define INT_ERR BIT(1) #define INT_RX BIT(0) -#define REG_PTR(base, offset) ((volatile uint32_t *)((char*)(base) + (offset))) +#define REG_PTR(base, offset) ( (volatile uint32_t *)( \ + (uintptr_t)(base) + (offset) ) ) + static clk_t *clk; @@ -134,26 +136,68 @@ static const struct dev_defn dev_defn[] = { UART_DEFN(3), }; +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static int internal_uart_tx_busy(void* reg_base) +{ + return *REG_PTR(reg_base, UFRSTAT) & FRSTAT_TX_FULL; +} + +static int internal_uart_tx(void* reg_base, int c) +{ + *REG_PTR(reg_base, UTXH) = c; +} + +static uint8_t internal_uart_rx_byte(void *reg_base) +{ + return (uint8_t)(*REG_PTR(reg_base, URXH)); +} + +static int internal_uart_is_rx_ready(void *reg_base) +{ + return *REG_PTR(reg_base, UTRSTAT) & TRSTAT_RXBUF_READY; +} + +/* + ******************************************************************************* + * UART access helpers + ******************************************************************************* + */ + static int exynos_uart_putchar(ps_chardevice_t *d, int c) { - if (*REG_PTR(d->vaddr, UFRSTAT) & FRSTAT_TX_FULL) { - /* abort: no room in FIFO */ + void* reg_base = d->vaddr; + + /* if UART is busy return an error */ + if (internal_uart_tx_busy(reg_base)) { return -1; - } else { - /* Write out the next character. */ - *REG_PTR(d->vaddr, UTXH) = c; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - /* In this case, We should have checked that we had two free bytes in - * the FIFO before we submitted the first char, however, the fifo size - * would need to be considered and this differs between UARTs. - * To keep things simple, we recognise that it is rare for a '\n' to - * be sent when there is insufficient FIFO space and accept the - * inefficiencies of spinning, waiting for space. - */ - while (exynos_uart_putchar(d, '\r') < 0); + } + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + /* Write out the next character. */ + internal_uart_tx(reg_base, byte); + + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + /* In this case, we should have checked that we had two free bytes in + * the FIFO before we submitted the first char, however, the FIFO size + * would need to be considered and this differs between UARTs. + * To keep things simple, we recognize that it is rare for a '\n' to + * be sent when there is insufficient FIFO space and accept the + * inefficiencies of spinning, waiting for space. + */ + while (internal_uart_tx_busy(reg_base)) { + /* busy waiting loop */ } - return c; + internal_uart_tx(reg_base, '\r'); } + + return byte; } static int uart_fill_fifo(ps_chardevice_t *d, const char *data, size_t len) @@ -235,11 +279,13 @@ static void uart_handle_tx_irq(ps_chardevice_t *d) static int exynos_uart_getchar(ps_chardevice_t *d) { - if (*REG_PTR(d->vaddr, UTRSTAT) & TRSTAT_RXBUF_READY) { - return *REG_PTR(d->vaddr, URXH); - } else { + void* reg_base = d->vaddr; + + if (!internal_uart_is_rx_ready(reg_base)) { return -1; } + + return internal_uart_rx_byte(reg_base); } static int uart_read_fifo(ps_chardevice_t *d, char *data, size_t len) diff --git a/libplatsupport/src/mach/nvidia/serial.c b/libplatsupport/src/mach/nvidia/serial.c index 825a276bf..bdc263b65 100644 --- a/libplatsupport/src/mach/nvidia/serial.c +++ b/libplatsupport/src/mach/nvidia/serial.c @@ -94,6 +94,12 @@ struct tk1_uart_regs { }; typedef volatile struct tk1_uart_regs tk1_uart_regs_t; +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + static inline tk1_uart_regs_t* tk1_uart_get_priv(ps_chardevice_t *d) { @@ -139,6 +145,24 @@ tk1_uart_set_rbr_irq(tk1_uart_regs_t *regs, bool enable) regs->ier_dlab = ier; } +static int +internal_uart_tx_busy(tk1_uart_regs_t* regs) +{ + return ((regs->lsr & LSR_THRE_EMPTY) != LSR_THRE_EMPTY); +} + +static int +internal_uart_tx(tk1_uart_regs_t* regs, uint8_t c) +{ + regs->thr_dlab = c; +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + int uart_getchar(ps_chardevice_t *d) { tk1_uart_regs_t* regs = tk1_uart_get_priv(d); @@ -155,19 +179,25 @@ int uart_getchar(ps_chardevice_t *d) int uart_putchar(ps_chardevice_t* d, int c) { tk1_uart_regs_t* regs = tk1_uart_get_priv(d); - uint32_t lsr = regs->lsr; - if (((lsr & LSR_THRE_EMPTY) == LSR_THRE_EMPTY)) { - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); - } + /* if UART is busy return an error */ + if (internal_uart_tx_busy(regs)) { + return -1; + } - regs->thr_dlab = (uint8_t) c; + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; - return c; - } else { - return -1; + if (byte == '\n') { + internal_uart_tx(regs, '\r'); + if (internal_uart_tx_busy(regs)) { + return -1; + } } + + internal_uart_tx(regs, byte); + + return byte; } static void diff --git a/libplatsupport/src/mach/zynq/serial.c b/libplatsupport/src/mach/zynq/serial.c index d38adbdfa..e5ed80116 100644 --- a/libplatsupport/src/mach/zynq/serial.c +++ b/libplatsupport/src/mach/zynq/serial.c @@ -213,33 +213,63 @@ int uart_putchar( ps_chardevice_t *d, int c) { - int ret = -1; zynq_uart_regs_t *regs = zynq_uart_get_priv(d); + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + int send_cr = (byte == '\n'); + + /* Usually we send one char only and check UART_SR_TFUL to see if the TX + * FIFO is full. If we have to send CR+LF, then check if there is space for + * 2 bytes in the TX FIFO via UART_SR_TTRIG. This works because the TX + * trigger level is 63 and this bit is set if the FIFO level is greater or + * equal the trigger level. + */ + if (regs->sr & (send_cr ? UART_SR_TTRIG : UART_SR_TFUL)) { + return -1; /* not enough space in the FIFO */ + } + + /* save imr */ uint32_t imr = regs->imr; regs->idr = imr; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - /* check if 2 bytes are free - tx trigger level is 63 and - * this bit is set if the fifo level is >= the trigger level + if (send_cr) + { + /* Send CR+LF (='\r\n'). We have checked above that there is enough + * space in the FIFO to send two chars. */ - if (!(regs->sr & UART_SR_TTRIG)) { + regs->fifo = '\r'; + } - regs->fifo = '\r'; - regs->fifo = '\n'; + regs->fifo = byte; + + /* It is questionable why the initial implementation drains the FIFO now, + * but we keep this behavior. Maybe this is intended to have a guarantee + * that any char accepted by putchar() is really printed before the + * function returns. Thus no log data shows up asynchronously because the + * FIFO drains while the CPU is doing something else. + * + * Potential improvement for the serial subsystem is defining more flags + * besides SERIAL_AUTO_CR that allow controlling the behavior externally + * and thus align all platform implementation: + * + * - SERIAL_DO_NOT_BLOCK could disable putchar() blocking initially if the + * FIFO is full. It would return an error in this case. The flag could + * also be SERIAL_BLOCK and the recommendation is to always set it. + * + * - SERIAL_DRAIN_FIFO could make putchar() always drain the FIFO before + * leaving. For a UART used for logging, this could be useful to give a + * guarantee that logs printed before the code continues. + */ - ret = '\n'; - } - } else if (!(regs->sr & UART_SR_TFUL)) { - regs->fifo = c; - ret = c; + while ((regs->sr & (UART_SR_TEMPTY | UART_SR_TACTIVE)) != UART_SR_TEMPTY) + { + /* busy waiting loop */ } - while ((regs->sr & (UART_SR_TEMPTY | UART_SR_TACTIVE)) != UART_SR_TEMPTY); - regs->ier = imr; - return ret; + return byte; } static void uart_handle_irq( diff --git a/libplatsupport/src/plat/fvp/serial.c b/libplatsupport/src/plat/fvp/serial.c deleted file mode 100644 index aef2f5a86..000000000 --- a/libplatsupport/src/plat/fvp/serial.c +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright 2019, Data61, CSIRO (ABN 41 687 119 230) - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -/* Mostly copy/paste from the HiKey plat. - * Should be moved to a common driver file for PL011 */ - -#include -#include -#include -#include "../../chardev.h" - -#define RHR_MASK MASK(8) -#define UARTDR 0x000 -#define UARTFR 0x018 -#define UARTIMSC 0x038 -#define UARTICR 0x044 -#define PL011_UARTFR_TXFF BIT(5) -#define PL011_UARTFR_RXFE BIT(4) - -#define REG_PTR(base, off) ((volatile uint32_t *)((base) + (off))) - -int uart_getchar(ps_chardevice_t *d) -{ - int ch = EOF; - - if ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_RXFE) == 0) { - ch = *REG_PTR(d->vaddr, UARTDR) & RHR_MASK; - } - return ch; -} - -int uart_putchar(ps_chardevice_t* d, int c) -{ - while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0); - - *REG_PTR(d->vaddr, UARTDR) = c; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); - } - - return c; -} - -static void -uart_handle_irq(ps_chardevice_t* dev) -{ - *REG_PTR(dev->vaddr, UARTICR) = 0x7f0; -} - -int uart_init(const struct dev_defn* defn, - const ps_io_ops_t* ops, - ps_chardevice_t* dev) -{ - memset(dev, 0, sizeof(*dev)); - void* vaddr = chardev_map(defn, ops); - if (vaddr == NULL) { - return -1; - } - - /* Set up all the device properties. */ - dev->id = defn->id; - dev->vaddr = (void*)vaddr; - dev->read = &uart_read; - dev->write = &uart_write; - dev->handle_irq = &uart_handle_irq; - dev->irqs = defn->irqs; - dev->ioops = *ops; - dev->flags = SERIAL_AUTO_CR; - - *REG_PTR(dev->vaddr, UARTIMSC) = 0x50; - return 0; -} diff --git a/libplatsupport/src/plat/hifive/uart.c b/libplatsupport/src/plat/hifive/uart.c index b6abdcb82..4c3719f05 100644 --- a/libplatsupport/src/plat/hifive/uart.c +++ b/libplatsupport/src/plat/hifive/uart.c @@ -36,37 +36,72 @@ struct uart { }; typedef volatile struct uart uart_regs_t; -static inline uart_regs_t* -uart_get_priv(ps_chardevice_t *d) +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static uart_regs_t* uart_get_regs(ps_chardevice_t *d) { return (uart_regs_t*)d->vaddr; } +static int internal_uart_is_tx_fifo_full(uart_regs_t* regs) +{ + return regs->txdata & UART_TX_DATA_FULL; +} + +static int internal_uart_tx_byte(uart_regs_t* regs, uint8_t c) +{ + regs->txdata = c & UART_TX_DATA_MASK; +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + int uart_getchar(ps_chardevice_t *d) { - uart_regs_t* regs = uart_get_priv(d); - uint32_t reg = regs->rxdata; - int c = -1; + uart_regs_t* regs = uart_get_regs(d); - if (!(reg & UART_RX_DATA_EMPTY)) { - c = reg & UART_RX_DATA_MASK; + uint32_t rxdata = regs->rxdata; + if (rxdata & UART_RX_DATA_EMPTY) + { + return -1; } - return c; + + /* return only the lowest 8 bits */ + return (uint8_t)(rxdata & UART_RX_DATA_MASK); } int uart_putchar(ps_chardevice_t* d, int c) { - uart_regs_t* regs = uart_get_priv(d); - if (!(regs->txdata & UART_TX_DATA_FULL)) { - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - regs->txdata = '\r' & UART_TX_DATA_MASK; - while(regs->txdata & UART_TX_DATA_FULL) {} - } - regs->txdata = c & UART_TX_DATA_MASK; - return c; - } else { + uart_regs_t* regs = uart_get_regs(d); + + /* if UART is busy return an error */ + if (internal_uart_is_tx_fifo_full(regs)) { return -1; } + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + if (byte == '\n') { + internal_uart_tx_byte(regs, '\r'); + /* If SERIAL_AUTO_CR is enabled, we assume this UART is used as a + * console, so blocking is fine here. + */ + while (internal_uart_is_tx_fifo_full(regs)) { + /* busy waiting loop */ + } + } + + internal_uart_tx_byte(regs, byte); + + return byte; } static void @@ -80,7 +115,6 @@ int uart_init(const struct dev_defn* defn, const ps_io_ops_t* ops, ps_chardevice_t* dev) { - uart_regs_t* regs; /* Attempt to map the virtual address, assure this works */ void* vaddr = chardev_map(defn, ops); if (vaddr == NULL) { @@ -99,7 +133,7 @@ int uart_init(const struct dev_defn* defn, dev->ioops = *ops; dev->flags = SERIAL_AUTO_CR; - regs = uart_get_priv(dev); + uart_regs_t* regs = uart_get_regs(dev); /* * Enable TX and RX and don't set any watermark levels. diff --git a/libplatsupport/src/plat/hikey/serial.c b/libplatsupport/src/plat/hikey/serial.c deleted file mode 100644 index 8dd0118d0..000000000 --- a/libplatsupport/src/plat/hikey/serial.c +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2017, Data61, CSIRO (ABN 41 687 119 230) - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include -#include -#include "../../chardev.h" - -#define RHR_MASK MASK(8) -#define UARTDR 0x000 -#define UARTFR 0x018 -#define UARTIMSC 0x038 -#define UARTICR 0x044 -#define PL011_UARTFR_TXFF BIT(5) -#define PL011_UARTFR_RXFE BIT(4) - -#define REG_PTR(base, off) ((volatile uint32_t *)((base) + (off))) - -int uart_getchar(ps_chardevice_t *d) -{ - int ch = EOF; - - if ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_RXFE) == 0) { - ch = *REG_PTR(d->vaddr, UARTDR) & RHR_MASK; - } - return ch; -} - -int uart_putchar(ps_chardevice_t* d, int c) -{ - while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0); - - *REG_PTR(d->vaddr, UARTDR) = c; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); - } - - return c; -} - -static void -uart_handle_irq(ps_chardevice_t* dev) -{ - *REG_PTR(dev->vaddr, UARTICR) = 0x7f0; -} - -int uart_init(const struct dev_defn* defn, - const ps_io_ops_t* ops, - ps_chardevice_t* dev) -{ - memset(dev, 0, sizeof(*dev)); - void* vaddr = chardev_map(defn, ops); - if (vaddr == NULL) { - return -1; - } - - /* Set up all the device properties. */ - dev->id = defn->id; - dev->vaddr = (void*)vaddr; - dev->read = &uart_read; - dev->write = &uart_write; - dev->handle_irq = &uart_handle_irq; - dev->irqs = defn->irqs; - dev->ioops = *ops; - dev->flags = SERIAL_AUTO_CR; - - *REG_PTR(dev->vaddr, UARTIMSC) = 0x50; - return 0; -} diff --git a/libplatsupport/src/plat/odroidc2/serial.c b/libplatsupport/src/plat/odroidc2/serial.c index 92cd38865..84e99b96a 100644 --- a/libplatsupport/src/plat/odroidc2/serial.c +++ b/libplatsupport/src/plat/odroidc2/serial.c @@ -16,29 +16,83 @@ #define UART_TX_FULL BIT(21) #define UART_RX_EMPTY BIT(20) -#define REG_PTR(base, off) ((volatile uint32_t *)((base) + (off))) +#define REG_PTR(base, offset) ( (volatile uint32_t *)( \ + (uintptr_t)(base) + (offset) ) ) -int uart_getchar(ps_chardevice_t *d) +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static int internal_uart_is_tx_fifo_full(void *reg_base) +{ + return *REG_PTR(reg_base, UART_STATUS) & UART_TX_FULL; +} + +static void internal_uart_tx_byte(void *reg_base, uint8_t byte) +{ + *REG_PTR(reg_base, UART_WFIFO) = byte; +} + +static int internal_uart_is_rx_empty(void *reg_base) +{ + return *REG_PTR(d->reg_base, UART_STATUS) & UART_RX_EMPTY; +} + +static uint8_t internal_uart_rx_byte(void *reg_base) +{ + return (uint8_t)(*REG_PTR(reg_base, UART_RFIFO)); +} + +static void internal_uart_busy_wait_tx_ready(void *reg_base) +{ + while (internal_uart_is_tx_fifo_full(reg_base)) { + /* busy waiting loop */ + } +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + +int uart_getchar(ps_chardevice_t *dev) { - while ((*REG_PTR(d->vaddr, UART_STATUS) & UART_RX_EMPTY)); - return *REG_PTR(d->vaddr, UART_RFIFO); + void* reg_base = dev->vaddr; + + /* this can potentially block forever if nobody sends anything. */ + while (internal_uart_is_rx_empty(reg_base) { + /* busy waiting loop */ + } + + return internal_uart_rx_byte(reg_base); } -int uart_putchar(ps_chardevice_t *d, int c) +int uart_putchar(ps_chardevice_t *dev, int c) { - while ((*REG_PTR(d->vaddr, UART_STATUS) & UART_TX_FULL)); + void* reg_base = dev->vaddr; + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + internal_uart_busy_wait_tx_ready(reg_base); + + /* Add character to the buffer. TODO: why don't we send the highest bit? */ + internal_uart_putchar(vaddr, byte & 0x7f); - /* Add character to the buffer. */ - *REG_PTR(d->vaddr, UART_WFIFO) = c & 0x7f; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); + /* SERIAL_AUTO_CR: Send '\r' (CR) before every '\n' (LF). */ + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + internal_uart_busy_wait_tx_ready(reg_base); + internal_uart_tx_byte(reg_base, '\r'); } - return c; + return byte; } -static void uart_handle_irq(ps_chardevice_t *dev) +static void uart_handle_irq(UNUSED ps_chardevice_t *dev) { /* nothing to do, interrupts are not used */ } diff --git a/libplatsupport/src/plat/omap3/serial.c b/libplatsupport/src/plat/omap3/serial.c index 25bbdcffd..39b26a932 100644 --- a/libplatsupport/src/plat/omap3/serial.c +++ b/libplatsupport/src/plat/omap3/serial.c @@ -22,25 +22,69 @@ #define IMXUART_LSR_TXSRE (1<<6) #define IMXUART_LSR_RXFIFOSTS (1<<7) -#define REG_PTR(base, offset) ((volatile uint32_t *)((char*)(base) + (offset))) +#define REG_PTR(base, offset) ( (volatile uint32_t *)( \ + (uintptr_t)(base) + (offset) ) ) -int uart_getchar(ps_chardevice_t* d) + +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static int internal_uart_is_tx_idle(void *reg_base) +{ + return *REG_PTR(d->vaddr, MU_LSR) & MU_LSR_TXIDLE; +} + +static void internal_uart_tx_byte(void *reg_base, uint8_t byte) { - if (*REG_PTR(d->vaddr, IMXUART_LSR) & IMXUART_LSR_RXFIFIOE) { - return *REG_PTR(d->vaddr, IMXUART_RHR); - } else { + *REG_PTR(d->vaddr, IMXUART_THR) = byte; +} + +static int internal_uart_is_rx_available(void *reg_base) +{ + return *REG_PTR(d->vaddr, IMXUART_LSR) & IMXUART_LSR_RXFIFIOE; +} + +static uint8_t internal_uart_rx_byte(void *reg_base) +{ + return (uint8_t)(*REG_PTR(d->vaddr, IMXUART_RHR)) +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + +int uart_putchar(ps_chardevice_t* d, int c) +{ + void *reg_base = d->vaddr; + + /* if UART is busy return an error */ + if (!internal_uart_is_tx_idle(reg_base)) { return -1; } + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + internal_uart_tx_byte(reg_base, byte); + + return byte; } -int uart_putchar(ps_chardevice_t* d, int c) +int uart_getchar(ps_chardevice_t* d) { - if (*REG_PTR(d->vaddr, IMXUART_LSR) & IMXUART_LSR_TXFIFOE) { - *REG_PTR(d->vaddr, IMXUART_THR) = c; - return c; - } else { + void *reg_base = d->vaddr; + + /* if UART is does not have data return an error */ + if (!internal_uart_is_rx_available(reg_base)) { return -1; } + + return internal_uart_rx_byte(reg_base); } static void uart_handle_irq(ps_chardevice_t* d) diff --git a/libplatsupport/src/plat/pc99/serial.c b/libplatsupport/src/plat/pc99/serial.c index 22888c803..b7f9a00e9 100644 --- a/libplatsupport/src/plat/pc99/serial.c +++ b/libplatsupport/src/plat/pc99/serial.c @@ -30,65 +30,119 @@ #define SERIAL_LSR 5 /* Line Status Register (R ) */ #define SERIAL_MSR 6 /* Modem Status Register (R ) */ #define SERIAL_SR 7 /* Scratch Register (RW) */ -#define CONSOLE(port, label) ((port) + (SERIAL_##label)) + #define SERIAL_DLAB BIT(7) #define SERIAL_LSR_DATA_READY BIT(0) #define SERIAL_LSR_TRANSMITTER_EMPTY BIT(5) +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static uint32_t get_console_io_port( + ps_chardevice_t *device, + unsigned int offset) +{ + /* Casting points to a specific integer directly is not allowed, must cast + * to uintptr_t and then cast to a specific integer type. + */ + return (uint32_t)((uintptr_t)device->vaddr) + offset; +} + +static void console_io_port_read( + ps_chardevice_t *device, + unsigned int port_offset, + uint32_t *data) +{ + return ps_io_port_in( + &device->ioops.io_port_ops, + get_console_io_port(device, port_offset), + 1, /* io_size */ + data); +} + +static void console_io_port_write( + ps_chardevice_t *device, + unsigned int port_offset, + uint32_t data) +{ + return ps_io_port_out( + &device->ioops.io_port_ops, + get_console_io_port(device, port_offset), + 1, /* io_size */ + data); +} + +static int serial_is_tx_ready(ps_chardevice_t* device) +{ + uint32_t data; + int ret = console_io_port_read(device, SERIAL_LSR, &data); + if (ret != 0) { + return 0; /* claim transmitter is not ready */ + } + return data & SERIAL_LSR_TRANSMITTER_EMPTY; +} + +static int serial_tx_byte(ps_chardevice_t* device, uint8_t byte) +{ + return console_io_port_write(device, SERIAL_THR, byte); +} + +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + int uart_getchar(ps_chardevice_t *device) { - uint32_t res; - uint32_t io_port = (uint32_t) (uintptr_t)device->vaddr; + int ret; + uint32_t data; /* Check if character is available. */ - int error = ps_io_port_in(&device->ioops.io_port_ops, CONSOLE(io_port, LSR), 1, &res); - if (error != 0) { + ret = console_io_port_read(device, SERIAL_LSR, &data); + if (ret != 0) { return -1; } - if (!(res & SERIAL_LSR_DATA_READY)) { + if (!(data & SERIAL_LSR_DATA_READY)) { return -1; } /* retrieve character */ - error = ps_io_port_in(&device->ioops.io_port_ops, CONSOLE(io_port, RBR), 1, &res); - if (error != 0) { + ret = console_io_port_read(device, SERIAL_RBR, &data); + if (ret != 0) { return -1; } - return (int) res; -} - -static int serial_ready(ps_chardevice_t* device) -{ - uint32_t io_port = (uint32_t) (uintptr_t)device->vaddr; - uint32_t res; - int error = ps_io_port_in(&device->ioops.io_port_ops, CONSOLE(io_port, LSR), 1, &res); - if (error != 0) { - return 0; - } - return res & SERIAL_LSR_TRANSMITTER_EMPTY; + return (uint8_t)data; } int uart_putchar(ps_chardevice_t* device, int c) { - uint32_t io_port = (uint32_t) (uintptr_t)device->vaddr; - - /* Check if serial is ready. */ - if (!serial_ready(device)) { + /* Check if serial transmitter is ready. */ + if (!serial_is_tx_ready(device)) { return -1; } - /* Write out the next character. */ - ps_io_port_out(&device->ioops.io_port_ops, CONSOLE(io_port, THR), 1, c); + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + /* Write out the character, ignore return code. */ + (void)serial_tx_byte(device, byte); - if (c == '\n') { - /* If we output immediately then odds are the transmit buffer - * will be full, so we have to wait */ - while (!serial_ready(device)); - uart_putchar(device, '\r'); + if (byte == '\n') { + /* If we output immediately then odds are the transmit buffer will be + * full, so we have to wait. */ + while (!serial_is_tx_ready(device)) { + /* busy waiting loop */ + } + /* Write out the character, ignore return code. */ + (void)serial_tx_byte(device, '\r'); } - return c; + return byte; } static void uart_handle_irq(ps_chardevice_t* device UNUSED) @@ -110,56 +164,55 @@ uart_init(const struct dev_defn* defn, const ps_io_ops_t* ops, ps_chardevice_t* dev->ioops = *ops; /* Initialise the device. */ - uint32_t io_port = (uint32_t) (uintptr_t)dev->vaddr; /* clear DLAB - Divisor Latch Access Bit */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, LCR), 1, 0x00 & ~SERIAL_DLAB) != 0) { + if (console_io_port_write(dev, SERIAL_LCR, 0x00 & ~SERIAL_DLAB) != 0) { return -1; } /* disable generating interrupts */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, IER), 1, 0x00) != 0) { + if (console_io_port_write(dev, SERIAL_IER, 0x00) != 0) { return -1; } /* set DLAB to*/ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, LCR), 1, 0x00 | SERIAL_DLAB) != 0) { + if (console_io_port_write(dev, SERIAL_LCR, 0x00 | SERIAL_DLAB) != 0) { return -1; } /* set low byte of divisor to 0x01 = 115200 baud */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, DLL), 1, 0x01) != 0) { + if (console_io_port_write(dev, SERIAL_DLL, 0x01) != 0) { return -1; } /* set high byte of divisor to 0x00 */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, DLH), 1, 0x00) != 0) { + if (console_io_port_write(dev, SERIAL_DLH, 0x00) != 0) { return -1; } /* line control register: set 8 bit, no parity, 1 stop bit; clear DLAB */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, LCR), 1, 0x03 & ~SERIAL_DLAB) != 0) { + if (console_io_port_write(dev, SERIAL_LCR, 0x03 & ~SERIAL_DLAB) != 0) { return -1; } /* modem control register: set DTR/RTS/OUT2 */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, MCR), 1, 0x0b) != 0) { + if (console_io_port_write(dev, SERIAL_MCR, 0x0b) != 0) { return -1; } uint32_t temp; /* clear receiver port */ - if (ps_io_port_in(&dev->ioops.io_port_ops, CONSOLE(io_port, RBR), 1, &temp) != 0) { + if (console_io_port_write(dev, SERIAL_RBR, &temp) != 0) { return -1; } /* clear line status port */ - if (ps_io_port_in(&dev->ioops.io_port_ops, CONSOLE(io_port, LSR), 1, &temp) != 0) { + if (console_io_port_write(dev, SERIAL_LSR, &temp) != 0) { return -1; } /* clear modem status port */ - if (ps_io_port_in(&dev->ioops.io_port_ops, CONSOLE(io_port, MSR), 1, &temp) != 0) { + if (console_io_port_write(dev, SERIAL_MSR, &temp) != 0) { return -1; } /* Enable the receiver interrupt. */ - if (ps_io_port_out(&dev->ioops.io_port_ops, CONSOLE(io_port, IER), 1, 0x01) != 0) { + if (console_io_port_write(dev, SERIAL_IER, 0x01) != 0) { return -1; } diff --git a/libplatsupport/src/plat/qemu-arm-virt/serial.c b/libplatsupport/src/plat/qemu-arm-virt/serial.c deleted file mode 100644 index 47b665e16..000000000 --- a/libplatsupport/src/plat/qemu-arm-virt/serial.c +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2019, Data61, CSIRO (ABN 41 687 119 230) - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -/* Mostly copy/paste from the HiKey plat. - * Should be moved to a common driver file for PL011 */ - -#include -#include -#include -#include "../../chardev.h" - -#define RHR_MASK MASK(8) -#define UARTDR 0x000 -#define UARTFR 0x018 -#define UARTIMSC 0x038 -#define UARTICR 0x044 -#define PL011_UARTFR_TXFF BIT(5) -#define PL011_UARTFR_RXFE BIT(4) - -#define REG_PTR(base, off) ((volatile uint32_t *)((base) + (off))) - -int uart_getchar(ps_chardevice_t *d) -{ - int ch = EOF; - - if ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_RXFE) == 0) { - ch = *REG_PTR(d->vaddr, UARTDR) & RHR_MASK; - } - return ch; -} - -int uart_putchar(ps_chardevice_t *d, int c) -{ - while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0); - - *REG_PTR(d->vaddr, UARTDR) = c; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); - } - - return c; -} - -static void uart_handle_irq(ps_chardevice_t *dev) -{ - *REG_PTR(dev->vaddr, UARTICR) = 0x7f0; -} - -int uart_init(const struct dev_defn *defn, - const ps_io_ops_t *ops, - ps_chardevice_t *dev) -{ - memset(dev, 0, sizeof(*dev)); - void *vaddr = chardev_map(defn, ops); - if (vaddr == NULL) { - return -1; - } - - /* Set up all the device properties. */ - dev->id = defn->id; - dev->vaddr = (void *)vaddr; - dev->read = &uart_read; - dev->write = &uart_write; - dev->handle_irq = &uart_handle_irq; - dev->irqs = defn->irqs; - dev->ioops = *ops; - dev->flags = SERIAL_AUTO_CR; - - *REG_PTR(dev->vaddr, UARTIMSC) = 0x50; - return 0; -} diff --git a/libplatsupport/src/plat/rockpro64/serial.c b/libplatsupport/src/plat/rockpro64/serial.c index a7a246250..bfd6e1cfd 100644 --- a/libplatsupport/src/plat/rockpro64/serial.c +++ b/libplatsupport/src/plat/rockpro64/serial.c @@ -18,50 +18,98 @@ #define LSR_TXFIFOE BIT(5) #define LSR_RXFIFOE BIT(0) -#define REG_PTR(base, off) ((volatile uint32_t *)((base) + (off))) +#define REG_PTR(base, offset) ( (volatile uint32_t *)( \ + (uintptr_t)(base) + (offset) ) ) -int uart_getchar(ps_chardevice_t *d) +/* + ******************************************************************************* + * UART access primitives + ******************************************************************************* + */ + +static int internal_uart_is_tx_fifo_empty(void *reg_base) +{ + return *REG_PTR(vaddr, LSR) & LSR_TXFIFOE; +} + +static void internal_uart_tx_byte(void *reg_base, uint8_t c) { - int ch = EOF; + *REG_PTR(vaddr, THR) = c; +} - if (*REG_PTR(d->vaddr, LSR) & LSR_RXFIFOE) { - ch = *REG_PTR(d->vaddr, RHR) & RHR_MASK; +static int internal_uart_is_rx_available(void *reg_base) +{ + return *REG_PTR(d->vaddr, LSR) & LSR_RXFIFOE; +} + +static uint8_t internal_uart_rx_byte(void *reg_base) +{ + return (uint8_t)(REG_PTR(d->vaddr, RHR) & RHR_MASK); +} + +static void internal_uart_busy_wait_tx_ready(void *reg_base) +{ + while (!internal_uart_is_tx_fifo_empty(reg_base)) { + /* busy waiting loop */ } - return ch; } -int uart_putchar(ps_chardevice_t* d, int c) +/* + ******************************************************************************* + * UART access API + ******************************************************************************* + */ + +int uart_getchar(ps_chardevice_t *dev) { - while (!(*REG_PTR(d->vaddr, LSR) & LSR_TXFIFOE)) { - continue; + void *reg_base = dev->vaddr; + + if (!internal_uart_is_rx_available(reg_base)) { + return -1; } - *REG_PTR(d->vaddr, THR) = c; - if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); + + return internal_uart_rx_byte(reg_base); +} + +int uart_putchar(ps_chardevice_t *dev, int c) +{ + void *reg_base = dev->vaddr; + + /* Extract the byte to send, drop any flags. */ + uint8_t byte = (uint8_t)c; + + internal_uart_busy_wait_tx_ready(reg_base); + + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + internal_uart_tx_byte(reg_base, '\r'); + internal_uart_busy_wait_tx_ready(reg_base); } - return c; + internal_uart_tx_byte(reg_base, c); + + return byte; } static void -uart_handle_irq(ps_chardevice_t* d UNUSED) +uart_handle_irq(ps_chardevice_t *dev UNUSED) { /* nothing to do */ } -int uart_init(const struct dev_defn* defn, - const ps_io_ops_t* ops, - ps_chardevice_t* dev) +int uart_init(const struct dev_defn *defn, + const ps_io_ops_t *ops, + ps_chardevice_t *dev) { memset(dev, 0, sizeof(*dev)); - void* vaddr = chardev_map(defn, ops); + + void *vaddr = chardev_map(defn, ops); if (vaddr == NULL) { return -1; } /* Set up all the device properties. */ dev->id = defn->id; - dev->vaddr = (void*)vaddr; + dev->vaddr = (void *)vaddr; dev->read = &uart_read; dev->write = &uart_write; dev->handle_irq = &uart_handle_irq; @@ -70,5 +118,6 @@ int uart_init(const struct dev_defn* defn, dev->flags = SERIAL_AUTO_CR; *REG_PTR(dev->vaddr, IER) = IER_RHRIT; + return 0; } diff --git a/libplatsupport/src/serial.c b/libplatsupport/src/serial.c index b1529e4ff..9a924b222 100644 --- a/libplatsupport/src/serial.c +++ b/libplatsupport/src/serial.c @@ -26,7 +26,8 @@ ssize_t uart_write( * while a data UART may prefer nog to block and let the caller * handle the waiting. */ - if (uart_putchar(d, data[i]) < 0) { + int ret = uart_putchar(d, data[i]); + if (ret < 0) { return i; } } @@ -41,7 +42,7 @@ ssize_t uart_read( void *token UNUSED) { char *data = (char *)vdata; - for (int i = 0; i < count; i++) { + for (unsigned int i = 0; i < count; i++) { int ret = uart_getchar(d); if (EOF == ret) { return i; From cbf609c1621de3a37adc3123ed1a84f208afc8cd Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 7 Apr 2021 01:18:00 +0200 Subject: [PATCH 05/10] wip more platforms code cleanup --- libplatsupport/src/plat/am335x/serial.c | 16 +++++++++++----- libplatsupport/src/plat/apq8064/serial.c | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/libplatsupport/src/plat/am335x/serial.c b/libplatsupport/src/plat/am335x/serial.c index 2c215243c..48f18303a 100644 --- a/libplatsupport/src/plat/am335x/serial.c +++ b/libplatsupport/src/plat/am335x/serial.c @@ -30,16 +30,22 @@ int uart_getchar(ps_chardevice_t *d) return ch; } -int uart_putchar(ps_chardevice_t* d, int c) +static void internal_uart_putchar(void* vaddr, int c) { - while (!(*REG_PTR(d->vaddr, LSR) & LSR_TXFIFOE)) { + while (!(*REG_PTR(vaddr, LSR) & LSR_TXFIFOE)) { continue; } - *REG_PTR(d->vaddr, THR) = c; + *REG_PTR(vaddr, THR) = c; +} + +int uart_putchar(ps_chardevice_t* d, int c) +{ + void* vaddr = d->vaddr; + /* SERIAL_AUTO_CR: Send '\r' (CR) before every '\n' (LF). */ if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); + internal_uart_putchar(vaddr, '\r'); } - + internal_uart_putchar(vaddr, c); return c; } diff --git a/libplatsupport/src/plat/apq8064/serial.c b/libplatsupport/src/plat/apq8064/serial.c index 51884047b..2fb8756fd 100644 --- a/libplatsupport/src/plat/apq8064/serial.c +++ b/libplatsupport/src/plat/apq8064/serial.c @@ -28,15 +28,23 @@ uart_handle_irq(ps_chardevice_t* d UNUSED) { } -int uart_putchar(ps_chardevice_t* d, int c) +static void internal_uart_putchar(void* vaddr, int c) { - while (!(*UART_REG(d->vaddr, USR) & USR_TXEMP)); + while (!(*UART_REG(vaddr, USR) & USR_TXEMP)) { + continue; + } + *UART_REG(vaddr, UNTX) = 1; + *UART_REG(vaddr, UTF) = c & 0xff; +} - *UART_REG(d->vaddr, UNTX) = 1; - *UART_REG(d->vaddr, UTF) = c & 0xff; +int uart_putchar(ps_chardevice_t* d, int c) +{ + void* vaddr = d->vaddr; + /* SERIAL_AUTO_CR: Send '\r' (CR) before every '\n' (LF). */ if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) { - uart_putchar(d, '\r'); + internal_uart_putchar(vaddr, '\r'); } + internal_uart_putchar(vaddr, c); return 0; } From 3fa1fde683d063699709c467bd4c0b373b2bd307 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Tue, 6 Apr 2021 22:54:01 +0200 Subject: [PATCH 06/10] libplatsupport: fix CR LF line break printing Fix the bug that LF CR is printed instead of CR LF. Signed-off-by: Axel Heider --- libplatsupport/src/mach/exynos/serial.c | 7 +++---- libplatsupport/src/plat/odroidc2/serial.c | 8 ++++---- libplatsupport/src/plat/pc99/serial.c | 9 ++++----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/libplatsupport/src/mach/exynos/serial.c b/libplatsupport/src/mach/exynos/serial.c index 4576fefe7..9a173b781 100644 --- a/libplatsupport/src/mach/exynos/serial.c +++ b/libplatsupport/src/mach/exynos/serial.c @@ -180,10 +180,8 @@ static int exynos_uart_putchar(ps_chardevice_t *d, int c) /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - /* Write out the next character. */ - internal_uart_tx(reg_base, byte); - if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + internal_uart_tx(vaddr, '\r'); /* In this case, we should have checked that we had two free bytes in * the FIFO before we submitted the first char, however, the FIFO size * would need to be considered and this differs between UARTs. @@ -194,9 +192,10 @@ static int exynos_uart_putchar(ps_chardevice_t *d, int c) while (internal_uart_tx_busy(reg_base)) { /* busy waiting loop */ } - internal_uart_tx(reg_base, '\r'); } + internal_uart_tx(reg_base, byte); + return byte; } diff --git a/libplatsupport/src/plat/odroidc2/serial.c b/libplatsupport/src/plat/odroidc2/serial.c index 84e99b96a..6aea01713 100644 --- a/libplatsupport/src/plat/odroidc2/serial.c +++ b/libplatsupport/src/plat/odroidc2/serial.c @@ -80,15 +80,15 @@ int uart_putchar(ps_chardevice_t *dev, int c) internal_uart_busy_wait_tx_ready(reg_base); - /* Add character to the buffer. TODO: why don't we send the highest bit? */ - internal_uart_putchar(vaddr, byte & 0x7f); - /* SERIAL_AUTO_CR: Send '\r' (CR) before every '\n' (LF). */ if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { - internal_uart_busy_wait_tx_ready(reg_base); internal_uart_tx_byte(reg_base, '\r'); + internal_uart_busy_wait_tx_ready(reg_base); } + /* TODO: why don't we send the highest bit? */ + internal_uart_putchar(vaddr, byte & 0x7f); + return byte; } diff --git a/libplatsupport/src/plat/pc99/serial.c b/libplatsupport/src/plat/pc99/serial.c index b7f9a00e9..cfacbfe12 100644 --- a/libplatsupport/src/plat/pc99/serial.c +++ b/libplatsupport/src/plat/pc99/serial.c @@ -129,19 +129,18 @@ int uart_putchar(ps_chardevice_t* device, int c) /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - /* Write out the character, ignore return code. */ - (void)serial_tx_byte(device, byte); - if (byte == '\n') { /* If we output immediately then odds are the transmit buffer will be * full, so we have to wait. */ + (void)serial_tx_byte(device, '\r'); while (!serial_is_tx_ready(device)) { /* busy waiting loop */ } - /* Write out the character, ignore return code. */ - (void)serial_tx_byte(device, '\r'); } + /* Write out the character, ignore return code. */ + (void)serial_tx_byte(device, byte); + return byte; } From 5eaa1d1db4631b3269607e4a16a491c0c64465ab Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 7 Apr 2021 07:25:26 +0200 Subject: [PATCH 07/10] libplatsupport: clarify UART driver behavior Introduce SERIAL_TX_NONBLOCKING to allow defining UART blocking behavior. Clarify SERIAL_AUTO_CR behavior. Keep behavior of all drivers as it was before, but now the differences become more obvious. Signed-off-by: Axel Heider --- libplatsupport/include/platsupport/serial.h | 13 ++++++- .../src/drivers/uart_pl011/uart_pl011.c | 13 ++++--- libplatsupport/src/mach/exynos/serial.c | 39 ++++++++++++------- libplatsupport/src/mach/nvidia/serial.c | 32 +++++++++++---- libplatsupport/src/mach/zynq/serial.c | 17 ++++++-- libplatsupport/src/plat/hifive/uart.c | 35 ++++++++++++----- libplatsupport/src/plat/odroidc2/serial.c | 20 ++++++++-- libplatsupport/src/plat/omap3/serial.c | 35 +++++++++++++++-- libplatsupport/src/plat/pc99/serial.c | 39 ++++++++++++++----- libplatsupport/src/plat/rockpro64/serial.c | 19 ++++++++- libplatsupport/src/serial.c | 17 ++++---- 11 files changed, 213 insertions(+), 66 deletions(-) diff --git a/libplatsupport/include/platsupport/serial.h b/libplatsupport/include/platsupport/serial.h index 2c07498b9..8e15dcfe7 100644 --- a/libplatsupport/include/platsupport/serial.h +++ b/libplatsupport/include/platsupport/serial.h @@ -16,7 +16,18 @@ /* Auto-send CR (Carriage Return, "\r") before each "\n". All UART drivers * should set this flag by default, so the UART can be used as a console. */ -#define SERIAL_AUTO_CR BIT(0) +#define SERIAL_AUTO_CR BIT(0) + +/* Do not block if the TX FIFO is full, but return an error. When SERIAL_AUTO_CR + * is enabled, CR+LF is considered as an atom, ie either nothing is sent or both + * CR and LF are sent. If the underlying UART implementation can't ensure the TX + * FIFO has space for both chars, it is allowed to block after CR has been sent + * to ensure LF can also be sent. Rational for this is, that SERIAL_AUTO_CR + * usually implies that the UART is used as a console. Blocking in this corner + * case can be neglected considering the issues caused by a missing LF and a CR + * getting sent twice then. + */ +#define SERIAL_TX_NONBLOCKING BIT(1) /*****************************/ diff --git a/libplatsupport/src/drivers/uart_pl011/uart_pl011.c b/libplatsupport/src/drivers/uart_pl011/uart_pl011.c index 4bdf0a1de..b008854e8 100644 --- a/libplatsupport/src/drivers/uart_pl011/uart_pl011.c +++ b/libplatsupport/src/drivers/uart_pl011/uart_pl011.c @@ -97,11 +97,11 @@ int uart_putchar(ps_chardevice_t* dev, int c) { uart_pl011_regs_t *regs = get_uart_regs(dev); - /* Check if the TX FIFO has space. If not, then fail if SERIAL_NONBLOCKING - * is set and do busy waiting otherwise. + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. */ if (internal_uart_is_tx_fifo_full(regs)) - if (d->flags & SERIAL_NONBLOCKING) { + if (d->flags & SERIAL_TX_NONBLOCKING) { return -1; } internal_uart_busy_wait_tx_ready(regs); @@ -113,8 +113,11 @@ int uart_putchar(ps_chardevice_t* dev, int c) internal_uart_busy_wait_tx_ready(regs); /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common - * thing to do for a serial terminal. Since we want to make CR/LR atomic, a - * blocking wait will be used to ensure the LF can be sent also. + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. */ if (byte == '\n' && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx_byte(regs, '\r'); diff --git a/libplatsupport/src/mach/exynos/serial.c b/libplatsupport/src/mach/exynos/serial.c index 9a173b781..2d7729f23 100644 --- a/libplatsupport/src/mach/exynos/serial.c +++ b/libplatsupport/src/mach/exynos/serial.c @@ -162,6 +162,13 @@ static int internal_uart_is_rx_ready(void *reg_base) return *REG_PTR(reg_base, UTRSTAT) & TRSTAT_RXBUF_READY; } +static void internal_uart_busy_wait_tx_ready(void* reg_base) +{ + while (internal_uart_tx_busy(reg_base)) { + /* busy waiting loop */ + } +} + /* ******************************************************************************* * UART access helpers @@ -172,26 +179,29 @@ static int exynos_uart_putchar(ps_chardevice_t *d, int c) { void* reg_base = d->vaddr; - /* if UART is busy return an error */ - if (internal_uart_tx_busy(reg_base)) { - return -1; + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ + if (internal_uart_tx_busy(reg_base)) + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(reg_base); } /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx(vaddr, '\r'); - /* In this case, we should have checked that we had two free bytes in - * the FIFO before we submitted the first char, however, the FIFO size - * would need to be considered and this differs between UARTs. - * To keep things simple, we recognize that it is rare for a '\n' to - * be sent when there is insufficient FIFO space and accept the - * inefficiencies of spinning, waiting for space. - */ - while (internal_uart_tx_busy(reg_base)) { - /* busy waiting loop */ - } + internal_uart_busy_wait_tx_ready(reg_base); } internal_uart_tx(reg_base, byte); @@ -526,7 +536,8 @@ static void chardevice_init(ps_chardevice_t *dev, void *vaddr, const int *irqs) dev->write = &exynos_uart_write; dev->handle_irq = &uart_handle_irq; dev->irqs = irqs; - dev->flags = SERIAL_AUTO_CR; + /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ + dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; /* TODO */ dev->clk = NULL; } diff --git a/libplatsupport/src/mach/nvidia/serial.c b/libplatsupport/src/mach/nvidia/serial.c index bdc263b65..86199807e 100644 --- a/libplatsupport/src/mach/nvidia/serial.c +++ b/libplatsupport/src/mach/nvidia/serial.c @@ -157,6 +157,13 @@ internal_uart_tx(tk1_uart_regs_t* regs, uint8_t c) regs->thr_dlab = c; } +static void internal_uart_busy_wait_tx_ready(tk1_uart_regs_t* regs) +{ + while (internal_uart_tx_busy(regs)) { + /* busy waiting loop */ + } +} + /* ******************************************************************************* * UART access API @@ -180,19 +187,29 @@ int uart_putchar(ps_chardevice_t* d, int c) { tk1_uart_regs_t* regs = tk1_uart_get_priv(d); - /* if UART is busy return an error */ + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ if (internal_uart_tx_busy(regs)) { - return -1; + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(reg_base); } /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - if (byte == '\n') { + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx(regs, '\r'); - if (internal_uart_tx_busy(regs)) { - return -1; - } + internal_uart_busy_wait_tx_ready(regs); } internal_uart_tx(regs, byte); @@ -718,7 +735,8 @@ tk1_uart_init_common(const struct dev_defn *defn, void *const uart_mmio_vaddr, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = ioops_zero; - dev->flags = SERIAL_AUTO_CR; + /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ + dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; /* Zero out the client state. */ dev->write_descriptor = cxd_zero; diff --git a/libplatsupport/src/mach/zynq/serial.c b/libplatsupport/src/mach/zynq/serial.c index e5ed80116..35759f853 100644 --- a/libplatsupport/src/mach/zynq/serial.c +++ b/libplatsupport/src/mach/zynq/serial.c @@ -217,7 +217,7 @@ int uart_putchar( /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - int send_cr = (byte == '\n'); + int send_cr = ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)); /* Usually we send one char only and check UART_SR_TFUL to see if the TX * FIFO is full. If we have to send CR+LF, then check if there is space for @@ -225,8 +225,16 @@ int uart_putchar( * trigger level is 63 and this bit is set if the FIFO level is greater or * equal the trigger level. */ - if (regs->sr & (send_cr ? UART_SR_TTRIG : UART_SR_TFUL)) { - return -1; /* not enough space in the FIFO */ + unsigned int fifo_flag = send_cr ? UART_SR_TTRIG : UART_SR_TFUL; + if (!(regs->sr & fifo_flag)) + { + if (d->flags & SERIAL_TX_NONBLOCKING) + { + return -1; /* not enough space in the FIFO */ + } + while (!(regs->sr & fifo_flag)) { + /* do a busy-waiting loop */ + } } /* save imr */ @@ -466,7 +474,8 @@ static void zynq_uart_dev_init( dev->handle_irq = &uart_handle_irq; dev->ioops = *ops; - dev->flags = SERIAL_AUTO_CR; + /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ + dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; } static int zynq_uart_init( diff --git a/libplatsupport/src/plat/hifive/uart.c b/libplatsupport/src/plat/hifive/uart.c index 4c3719f05..1efbf9f7f 100644 --- a/libplatsupport/src/plat/hifive/uart.c +++ b/libplatsupport/src/plat/hifive/uart.c @@ -57,6 +57,13 @@ static int internal_uart_tx_byte(uart_regs_t* regs, uint8_t c) regs->txdata = c & UART_TX_DATA_MASK; } +static void internal_uart_busy_wait_tx_ready(uart_regs_t* regs) +{ + while (internal_uart_is_tx_fifo_full(regs)) { + /* busy waiting loop */ + } +} + /* ******************************************************************************* * UART access API @@ -81,22 +88,29 @@ int uart_putchar(ps_chardevice_t* d, int c) { uart_regs_t* regs = uart_get_regs(d); - /* if UART is busy return an error */ + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ if (internal_uart_is_tx_fifo_full(regs)) { - return -1; + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(regs); } /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - if (byte == '\n') { + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx_byte(regs, '\r'); - /* If SERIAL_AUTO_CR is enabled, we assume this UART is used as a - * console, so blocking is fine here. - */ - while (internal_uart_is_tx_fifo_full(regs)) { - /* busy waiting loop */ - } + internal_uart_busy_wait_tx_ready(regs); } internal_uart_tx_byte(regs, byte); @@ -131,7 +145,8 @@ int uart_init(const struct dev_defn* defn, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; - dev->flags = SERIAL_AUTO_CR; + /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ + dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; uart_regs_t* regs = uart_get_regs(dev); diff --git a/libplatsupport/src/plat/odroidc2/serial.c b/libplatsupport/src/plat/odroidc2/serial.c index 6aea01713..3d77b5c50 100644 --- a/libplatsupport/src/plat/odroidc2/serial.c +++ b/libplatsupport/src/plat/odroidc2/serial.c @@ -75,12 +75,26 @@ int uart_putchar(ps_chardevice_t *dev, int c) { void* reg_base = dev->vaddr; + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ + if (internal_uart_is_tx_fifo_full(regs)) + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(reg_base); + } + /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - internal_uart_busy_wait_tx_ready(reg_base); - - /* SERIAL_AUTO_CR: Send '\r' (CR) before every '\n' (LF). */ + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx_byte(reg_base, '\r'); internal_uart_busy_wait_tx_ready(reg_base); diff --git a/libplatsupport/src/plat/omap3/serial.c b/libplatsupport/src/plat/omap3/serial.c index 39b26a932..79a3da0fd 100644 --- a/libplatsupport/src/plat/omap3/serial.c +++ b/libplatsupport/src/plat/omap3/serial.c @@ -52,6 +52,13 @@ static uint8_t internal_uart_rx_byte(void *reg_base) return (uint8_t)(*REG_PTR(d->vaddr, IMXUART_RHR)) } +static void internal_uart_busy_wait_tx_ready(void *reg_base) +{ + while (!internal_uart_is_tx_idle(reg_base)) { + /* busy waiting loop */ + } +} + /* ******************************************************************************* * UART access API @@ -62,14 +69,31 @@ int uart_putchar(ps_chardevice_t* d, int c) { void *reg_base = d->vaddr; - /* if UART is busy return an error */ - if (!internal_uart_is_tx_idle(reg_base)) { - return -1; + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ + if (!internal_uart_is_tx_idle(reg_base)) + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_is_tx_idle(reg_base); } /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + internal_uart_tx_byte(reg_base, '\r'); + internal_uart_is_tx_idle(reg_base); + } + internal_uart_tx_byte(reg_base, byte); return byte; @@ -109,6 +133,11 @@ uart_init(const struct dev_defn* defn, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; + /* TODO: + * - SERIAL_AUTO_CR should be enabled by default + * - SERIAL_TX_NONBLOCKING should not be enabled by default + */ + dev->flags = SERIAL_TX_NONBLOCKING; return 0; } diff --git a/libplatsupport/src/plat/pc99/serial.c b/libplatsupport/src/plat/pc99/serial.c index cfacbfe12..1b0a59eef 100644 --- a/libplatsupport/src/plat/pc99/serial.c +++ b/libplatsupport/src/plat/pc99/serial.c @@ -90,6 +90,13 @@ static int serial_tx_byte(ps_chardevice_t* device, uint8_t byte) return console_io_port_write(device, SERIAL_THR, byte); } +static void internal_serial_busy_wait_tx_ready(ps_chardevice_t* device) +{ + while (!serial_is_tx_ready(device)) { + /* busy waiting loop */ + } +} + /* ******************************************************************************* * UART access API @@ -121,21 +128,30 @@ int uart_getchar(ps_chardevice_t *device) int uart_putchar(ps_chardevice_t* device, int c) { - /* Check if serial transmitter is ready. */ - if (!serial_is_tx_ready(device)) { - return -1; + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ + if (!serial_is_tx_ready(device)) + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_serial_busy_wait_tx_ready(device); } /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - if (byte == '\n') { - /* If we output immediately then odds are the transmit buffer will be - * full, so we have to wait. */ + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ + if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { + /* Write CR, ignore the return code. */ (void)serial_tx_byte(device, '\r'); - while (!serial_is_tx_ready(device)) { - /* busy waiting loop */ - } + internal_serial_busy_wait_tx_ready(device); } /* Write out the character, ignore return code. */ @@ -161,6 +177,11 @@ uart_init(const struct dev_defn* defn, const ps_io_ops_t* ops, ps_chardevice_t* dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; + /* TODO: + * - SERIAL_AUTO_CR should be enabled by default + * - SERIAL_TX_NONBLOCKING should not be enabled by default + */ + dev->flags = SERIAL_TX_NONBLOCKING; /* Initialise the device. */ diff --git a/libplatsupport/src/plat/rockpro64/serial.c b/libplatsupport/src/plat/rockpro64/serial.c index bfd6e1cfd..95cf01471 100644 --- a/libplatsupport/src/plat/rockpro64/serial.c +++ b/libplatsupport/src/plat/rockpro64/serial.c @@ -75,11 +75,26 @@ int uart_putchar(ps_chardevice_t *dev, int c) { void *reg_base = dev->vaddr; + /* Check if the TX FIFO has space. If not and SERIAL_TX_NONBLOCKING is set, + * then fail the call, otherwise do busy waiting. + */ + if (!internal_uart_is_tx_fifo_empty(reg_base)) + if (d->flags & SERIAL_TX_NONBLOCKING) { + return -1; + } + internal_uart_busy_wait_tx_ready(reg_base); + } + /* Extract the byte to send, drop any flags. */ uint8_t byte = (uint8_t)c; - internal_uart_busy_wait_tx_ready(reg_base); - + /* SERIAL_AUTO_CR enables sending a CR before any LF, which is the common + * thing to do for a serial terminal. CR/LR are considered an atom, thus a + * blocking wait will be used even if SERIAL_TX_NONBLOCKING is set to ensure + * LF is sent. + * TODO: Check in advance if the TX FIFO has space for two chars if + * SERIAL_TX_NONBLOCKING is set. + */ if ((byte == '\n') && (d->flags & SERIAL_AUTO_CR)) { internal_uart_tx_byte(reg_base, '\r'); internal_uart_busy_wait_tx_ready(reg_base); diff --git a/libplatsupport/src/serial.c b/libplatsupport/src/serial.c index 9a924b222..fbdcbaf4e 100644 --- a/libplatsupport/src/serial.c +++ b/libplatsupport/src/serial.c @@ -17,17 +17,18 @@ ssize_t uart_write( void *token UNUSED) { const unsigned char *data = (const unsigned char *)vdata; - for (int i = 0; i < count; i++) { - /* TODO: Some UART drivers implement a busy waiting until there is space - * in the TX FIFO, others return an error in this case. We should - * have a mechanism to allow the user to specify the desired - * behavior, so we don't have to try to guess it. For a standard - * logging console, the behavior is likely to do a blocking wait, - * while a data UART may prefer nog to block and let the caller - * handle the waiting. + for (unsigned int i = 0; i < count; i++) { + /* Call the UART driver, it is supposed to implement the handling for + * the flags SERIAL_TX_NONBLOCKING and SERIAL_AUTO_CR properly. */ int ret = uart_putchar(d, data[i]); if (ret < 0) { + /* There is nothing we can do, so abort and return how much data we + * could send. Unfortunately, we can return the actual error code, + * so the caller wont know what exactly failed. However, when + * SERIAL_TX_NONBLOCKING is enabled, it's likely that the TX FIFO is + * full so the caller should wait and send the remaining data. + */ return i; } } From 5c6c4dfea9a7107f11b7bc811f25468e1e13b365 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 7 Apr 2021 02:04:42 +0200 Subject: [PATCH 08/10] libplatsupport: align UART driver defaults Enable SERIAL_AUTO_CR and disable SERIAL_NONBLOCKING, so the UART can be used as a console. If application need a data UART, they must explicitly set the defaults. Signed-off-by: Axel Heider --- libplatsupport/src/mach/exynos/serial.c | 3 +-- libplatsupport/src/mach/nvidia/serial.c | 3 +-- libplatsupport/src/mach/zynq/serial.c | 3 +-- libplatsupport/src/plat/hifive/uart.c | 3 +-- libplatsupport/src/plat/omap3/serial.c | 6 +----- libplatsupport/src/plat/pc99/serial.c | 6 +----- 6 files changed, 6 insertions(+), 18 deletions(-) diff --git a/libplatsupport/src/mach/exynos/serial.c b/libplatsupport/src/mach/exynos/serial.c index 2d7729f23..05277da1b 100644 --- a/libplatsupport/src/mach/exynos/serial.c +++ b/libplatsupport/src/mach/exynos/serial.c @@ -536,8 +536,7 @@ static void chardevice_init(ps_chardevice_t *dev, void *vaddr, const int *irqs) dev->write = &exynos_uart_write; dev->handle_irq = &uart_handle_irq; dev->irqs = irqs; - /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ - dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; /* TODO */ dev->clk = NULL; } diff --git a/libplatsupport/src/mach/nvidia/serial.c b/libplatsupport/src/mach/nvidia/serial.c index 86199807e..807c63dd8 100644 --- a/libplatsupport/src/mach/nvidia/serial.c +++ b/libplatsupport/src/mach/nvidia/serial.c @@ -735,8 +735,7 @@ tk1_uart_init_common(const struct dev_defn *defn, void *const uart_mmio_vaddr, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = ioops_zero; - /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ - dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; /* Zero out the client state. */ dev->write_descriptor = cxd_zero; diff --git a/libplatsupport/src/mach/zynq/serial.c b/libplatsupport/src/mach/zynq/serial.c index 35759f853..906b5feee 100644 --- a/libplatsupport/src/mach/zynq/serial.c +++ b/libplatsupport/src/mach/zynq/serial.c @@ -474,8 +474,7 @@ static void zynq_uart_dev_init( dev->handle_irq = &uart_handle_irq; dev->ioops = *ops; - /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ - dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; } static int zynq_uart_init( diff --git a/libplatsupport/src/plat/hifive/uart.c b/libplatsupport/src/plat/hifive/uart.c index 1efbf9f7f..6db3865d7 100644 --- a/libplatsupport/src/plat/hifive/uart.c +++ b/libplatsupport/src/plat/hifive/uart.c @@ -145,8 +145,7 @@ int uart_init(const struct dev_defn* defn, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; - /* TODO: SERIAL_TX_NONBLOCKING should not be enabled by default */ - dev->flags = SERIAL_AUTO_CR | SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; uart_regs_t* regs = uart_get_regs(dev); diff --git a/libplatsupport/src/plat/omap3/serial.c b/libplatsupport/src/plat/omap3/serial.c index 79a3da0fd..827ec55f2 100644 --- a/libplatsupport/src/plat/omap3/serial.c +++ b/libplatsupport/src/plat/omap3/serial.c @@ -133,11 +133,7 @@ uart_init(const struct dev_defn* defn, dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; - /* TODO: - * - SERIAL_AUTO_CR should be enabled by default - * - SERIAL_TX_NONBLOCKING should not be enabled by default - */ - dev->flags = SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; return 0; } diff --git a/libplatsupport/src/plat/pc99/serial.c b/libplatsupport/src/plat/pc99/serial.c index 1b0a59eef..9041b7e13 100644 --- a/libplatsupport/src/plat/pc99/serial.c +++ b/libplatsupport/src/plat/pc99/serial.c @@ -177,11 +177,7 @@ uart_init(const struct dev_defn* defn, const ps_io_ops_t* ops, ps_chardevice_t* dev->handle_irq = &uart_handle_irq; dev->irqs = defn->irqs; dev->ioops = *ops; - /* TODO: - * - SERIAL_AUTO_CR should be enabled by default - * - SERIAL_TX_NONBLOCKING should not be enabled by default - */ - dev->flags = SERIAL_TX_NONBLOCKING; + dev->flags = SERIAL_AUTO_CR; /* Initialise the device. */ From 25432ce8de560cf0e2317547b9fbc099e40c0d4a Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 7 Apr 2021 01:26:56 +0200 Subject: [PATCH 09/10] libplatsupport/odroidc2: send all 8 bits Signed-off-by: Axel Heider --- libplatsupport/src/plat/odroidc2/serial.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libplatsupport/src/plat/odroidc2/serial.c b/libplatsupport/src/plat/odroidc2/serial.c index 3d77b5c50..df2185085 100644 --- a/libplatsupport/src/plat/odroidc2/serial.c +++ b/libplatsupport/src/plat/odroidc2/serial.c @@ -100,8 +100,7 @@ int uart_putchar(ps_chardevice_t *dev, int c) internal_uart_busy_wait_tx_ready(reg_base); } - /* TODO: why don't we send the highest bit? */ - internal_uart_putchar(vaddr, byte & 0x7f); + internal_uart_putchar(vaddr, byte); return byte; } From f9b22ce01cb740218f6b0b30e1530724ae92e9ae Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Wed, 7 Apr 2021 07:46:58 +0200 Subject: [PATCH 10/10] libplatsupport/odroidc2: report rx empty Signed-off-by: Axel Heider --- libplatsupport/src/plat/odroidc2/serial.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libplatsupport/src/plat/odroidc2/serial.c b/libplatsupport/src/plat/odroidc2/serial.c index df2185085..e14315380 100644 --- a/libplatsupport/src/plat/odroidc2/serial.c +++ b/libplatsupport/src/plat/odroidc2/serial.c @@ -63,9 +63,8 @@ int uart_getchar(ps_chardevice_t *dev) { void* reg_base = dev->vaddr; - /* this can potentially block forever if nobody sends anything. */ - while (internal_uart_is_rx_empty(reg_base) { - /* busy waiting loop */ + if (internal_uart_is_rx_empty(reg_base) { + return -1; } return internal_uart_rx_byte(reg_base);