Hi Simon, On Wed, Dec 11, 2019 at 3:35 AM Park, Aiden <[email protected]> wrote: > > Hi Simon, > > > -----Original Message----- > > From: U-Boot <[email protected]> On Behalf Of Simon Glass > > Sent: Monday, December 9, 2019 8:59 AM > > To: U-Boot Mailing List <[email protected]> > > Cc: Stefan Roese <[email protected]>; Angelo Dureghello <[email protected]> > > Subject: [PATCH v2 1/4] serial: ns16550: Support run-time configuration > > > > 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 <[email protected]> > > --- > > > > 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 | 57 ++++++++++++++++++++++++++++++++++---- > > -- > > include/ns16550.h | 13 +++++++++ > > 3 files changed, 83 insertions(+), 8 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..96c4471efd 100644 > > --- a/drivers/serial/ns16550.c > > +++ b/drivers/serial/ns16550.c > > @@ -92,19 +92,57 @@ 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_BE) { > > + if (plat->reg_width == 1) > > + writeb(value, addr + (1 << plat->reg_shift) - 1); > > + else if (plat->flags & NS16550_FLAG_IO) > > + out_be32(addr, value); > > + else > > + writel(value, addr); > > + } else { > > + if (plat->reg_width == 1) > > + writeb(value, addr); > > + else if (plat->flags & NS16550_FLAG_IO) > > + out_le32(addr, value); > > + else > > + writel(value, addr); > > + } > > +} > IO needs to use outb(). It breaks QEMU 0x3f8 IO (reg_width = 1, flags=IO). > I have verified 0x3f8 IO on QEMU and MMIO32 on APL with below code. > if (plat->flags & NS16550_FLAG_IO) { > outb(value, addr); > } else { > if (plat->flags & NS16550_FLAG_BE) { > if (plat->reg_width == 1) > writeb(value, addr + (1 << plat->reg_shift) - > 1); > else > out_be32(addr, value); > } else { > if (plat->reg_width == 1) > writeb(value, addr); > else > out_le32(addr, value); > } > } >
Would you post a v3 that fixes the issue that Aiden pointed out? Regards, Bin
