[patch-axel-4] UART driver cleanup

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()

libplatsupport/include/platsupport/serial.h

@@ -13,9 +13,21 @@
  **** Serial device flags ****
  *****************************/
 
-/* Auto-send CR(Carriage Return) after each "\n".
- * NOTE: This flag should be set by default. */
-#define SERIAL_AUTO_CR BIT(0)
+/* 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)
+
+/* 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)
 
 /*****************************/
 

libplatsupport/src/drivers/uart_pl011/uart_pl011.c

@@ -0,0 +1,164 @@
+/*
+ * ARM PL011 UART driver
+ *
+ * Copyright 2017, Data61, CSIRO (ABN 41 687 119 230)
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <stdint.h>
+#include <platsupport/serial.h>
+#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 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(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. 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');
+        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;
+}

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,77 @@ 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;
+}
+
+static void internal_uart_busy_wait_tx_ready(void* reg_base)
+{
+    while (internal_uart_tx_busy(reg_base)) {
+        /* busy waiting loop */
+    }
+}
+
+/*
+ *******************************************************************************
+ * 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 */
-        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);
+    void* reg_base = d->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_tx_busy(reg_base))
+        if (d->flags & SERIAL_TX_NONBLOCKING) {
+            return -1;
         }
-        return c;
+        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');
+        internal_uart_busy_wait_tx_ready(reg_base);
+    }
+
+    internal_uart_tx(reg_base, byte);
+
+    return byte;
 }
 
 static int uart_fill_fifo(ps_chardevice_t *d, const char *data, size_t len)
@@ -235,11 +288,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)

libplatsupport/src/mach/imx/serial/serial.c

@@ -261,8 +261,8 @@ int uart_init(
 
 #ifdef CONFIG_PLAT_IMX6
 #include <platsupport/plat/mux.h>
-    /* 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)) {