At present this driver uses an assortment of CONFIG options to control
how it accesses the hardware. This is painful for platforms that are
supposed to be controlled by a device tree or a previous-stage bootloader.

Add a new CONFIG option to enable fully dynamic configuration. This
controls register spacing, size, offset and endianness.

Signed-off-by: Simon Glass <s...@chromium.org>
---

Changes in v3:
- Rewrite the conditional logic to a fix a bug and match serial_xx_shift()
- Add a separate flag for whether to use endian-aware out() functions

Changes in v2:
- runtime -> run-time
- Enable run-time config for slimbootloader too
- Improve Kconfig help based on Bin's comments
- Use ns16550 in patch subject

 drivers/serial/Kconfig   | 21 ++++++++++++++
 drivers/serial/ns16550.c | 59 ++++++++++++++++++++++++++++++++++------
 include/ns16550.h        | 16 ++++++++++-
 3 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index ece7d87d4c..472a9f0929 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -598,6 +598,27 @@ config SYS_NS16550
          be used. It can be a constant or a function to get clock, eg,
          get_serial_clock().
 
+config NS16550_DYNAMIC
+       bool "Allow NS16550 to be configured at runtime"
+       default y if SYS_COREBOOT || SYS_SLIMBOOTLOADER
+       help
+         Enable this option to allow device-tree control of the driver.
+
+         Normally this driver is controlled by the following options:
+
+         CONFIG_SYS_NS16550_PORT_MAPPED - indicates that port I/O is used for
+            access. If not enabled, then the UART is memory-mapped.
+         CONFIG_SYS_NS16550_MEM32 - if memory-mapped, indicates that 32-bit
+            access should be used (instead of 8-bit)
+         CONFIG_SYS_NS16550_REG_SIZE - indicates register width and also
+            endianness. If positive, big-endian access is used. If negative,
+            little-endian is used.
+
+         It is not a good practice for a driver to be statically configured,
+         since it prevents the same driver being used for different types of
+         UARTs in a system. This option avoids this problem at the cost of a
+         slightly increased code size.
+
 config INTEL_MID_SERIAL
        bool "Intel MID platform UART support"
        depends on DM_SERIAL && OF_CONTROL
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 754b6e9921..eacca5b703 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -92,19 +92,59 @@ static inline int serial_in_shift(void *addr, int shift)
 #define CONFIG_SYS_NS16550_CLK  0
 #endif
 
+static void serial_out_dynamic(struct ns16550_platdata *plat, u8 *addr,
+                              int value)
+{
+       if (plat->flags & NS16550_FLAG_IO) {
+               outb(value, addr);
+       } else if (plat->reg_width == 4) {
+               if (plat->flags & NS16550_FLAG_ENDIAN) {
+                       if (plat->flags & NS16550_FLAG_BE)
+                               out_be32(addr, value);
+                       else
+                               out_le32(addr, value);
+               } else {
+                       writel(value, addr);
+               }
+       } else if (plat->flags & NS16550_FLAG_BE) {
+               writeb(value, addr + (1 << plat->reg_shift) - 1);
+       } else {
+               writeb(value, addr);
+       }
+}
+
+static int serial_in_dynamic(struct ns16550_platdata *plat, u8 *addr)
+{
+       if (plat->flags & NS16550_FLAG_IO) {
+               return inb(addr);
+       } else if (plat->reg_width == 4) {
+               if (plat->flags & NS16550_FLAG_ENDIAN) {
+                       if (plat->flags & NS16550_FLAG_BE)
+                               return in_be32(addr);
+                       else
+                               return in_le32(addr);
+               } else {
+                       return readl(addr);
+               }
+       } else if (plat->flags & NS16550_FLAG_BE) {
+               return readb(addr + (1 << plat->reg_shift) - 1);
+       } else {
+               return readb(addr);
+       }
+}
+
 static void ns16550_writeb(NS16550_t port, int offset, int value)
 {
        struct ns16550_platdata *plat = port->plat;
        unsigned char *addr;
 
        offset *= 1 << plat->reg_shift;
-       addr = (unsigned char *)plat->base + offset;
+       addr = (unsigned char *)plat->base + offset + plat->reg_offset;
 
-       /*
-        * As far as we know it doesn't make sense to support selection of
-        * these options at run-time, so use the existing CONFIG options.
-        */
-       serial_out_shift(addr + plat->reg_offset, plat->reg_shift, value);
+       if (IS_ENABLED(CONFIG_NS16550_DYNAMIC))
+               serial_out_dynamic(plat, addr, value);
+       else
+               serial_out_shift(addr, plat->reg_shift, value);
 }
 
 static int ns16550_readb(NS16550_t port, int offset)
@@ -113,9 +153,12 @@ static int ns16550_readb(NS16550_t port, int offset)
        unsigned char *addr;
 
        offset *= 1 << plat->reg_shift;
-       addr = (unsigned char *)plat->base + offset;
+       addr = (unsigned char *)plat->base + offset + plat->reg_offset;
 
-       return serial_in_shift(addr + plat->reg_offset, plat->reg_shift);
+       if (IS_ENABLED(CONFIG_NS16550_DYNAMIC))
+               return serial_in_dynamic(plat, addr);
+       else
+               return serial_in_shift(addr, plat->reg_shift);
 }
 
 static u32 ns16550_getfcr(NS16550_t port)
diff --git a/include/ns16550.h b/include/ns16550.h
index 701efeea85..18c9077755 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -31,6 +31,9 @@
 #define CONFIG_SYS_NS16550_REG_SIZE (-1)
 #endif
 
+#ifdef CONFIG_NS16550_DYNAMIC
+#define UART_REG(x)    unsigned char x
+#else
 #if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
 #error "Please define NS16550 registers size."
 #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_DM_SERIAL)
@@ -44,14 +47,24 @@
        unsigned char x;                                                \
        unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
 #endif
+#endif /* CONFIG_NS16550_DYNAMIC */
+
+enum ns16550_flags {
+       NS16550_FLAG_IO         = 1 << 0, /* Use I/O access (else mem-mapped) */
+       NS16550_FLAG_ENDIAN     = 1 << 1, /* Use out_le/be_32() */
+       NS16550_FLAG_BE         = 1 << 2, /* Big-endian access (else little) */
+};
 
 /**
  * struct ns16550_platdata - information about a NS16550 port
  *
  * @base:              Base register address
- * @reg_width:         IO accesses size of registers (in bytes)
+ * @reg_width:         IO accesses size of registers (in bytes, 1 or 4)
  * @reg_shift:         Shift size of registers (0=byte, 1=16bit, 2=32bit...)
+ * @reg_offset:                Offset to start of registers (normally 0)
  * @clock:             UART base clock speed in Hz
+ * @fcr:               Offset of FCR register (normally UART_FCR_DEFVAL)
+ * @flags:             A few flags (enum ns16550_flags)
  * @bdf:               PCI slot/function (pci_dev_t)
  */
 struct ns16550_platdata {
@@ -61,6 +74,7 @@ struct ns16550_platdata {
        int reg_offset;
        int clock;
        u32 fcr;
+       int flags;
 #if defined(CONFIG_PCI) && defined(CONFIG_SPL)
        int bdf;
 #endif
-- 
2.24.1.735.g03f4e72817-goog

Reply via email to