[U-Boot] [PATCH 4/9] ns16550: Support I/O accessors selected by DT
Simon Glass
sjg at chromium.org
Fri Jan 29 15:56:19 CET 2016
Hi Paul,
On 29 January 2016 at 06:54, Paul Burton <paul.burton at imgtec.com> wrote:
> Support making use of I/O accessors for ports when a device tree
> specified an I/O resource. This allows for systems where we have a mix
> of ns16550 ports, some of which should be accessed via I/O ports & some
> of which should be accessed via readb/writeb.
>
> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
> ---
>
> drivers/serial/ns16550.c | 46 ++++++++++++++++++++++++++++++++++++++--------
> include/ns16550.h | 31 ++++++++++++++++++-------------
> 2 files changed, 56 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 93dad33..b1d5319 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -11,6 +11,7 @@
> #include <ns16550.h>
> #include <serial.h>
> #include <watchdog.h>
> +#include <linux/ioport.h>
> #include <linux/types.h>
> #include <asm/io.h>
>
> @@ -102,7 +103,7 @@ static void ns16550_writeb(NS16550_t port, int offset, int value)
> offset *= 1 << plat->reg_shift;
> addr = map_physmem(plat->base, 0, MAP_NOCACHE) + offset;
> /*
> - * As far as we know it doesn't make sense to support selection of
> + * For many platforms 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_shift, value);
> @@ -119,13 +120,33 @@ static int ns16550_readb(NS16550_t port, int offset)
> return serial_in_shift(addr, plat->reg_shift);
> }
>
> +static void ns16550_outb(NS16550_t port, int offset, int value)
> +{
> + struct ns16550_platdata *plat = port->plat;
> +
> + offset *= 1 << plat->reg_shift;
> + outb(value, plat->base + offset);
> +}
> +
> +static int ns16550_inb(NS16550_t port, int offset)
> +{
> + struct ns16550_platdata *plat = port->plat;
> +
> + offset *= 1 << plat->reg_shift;
> + return inb(plat->base + offset);
> +}
> +
> /* We can clean these up once everything is moved to driver model */
> -#define serial_out(value, addr) \
> - ns16550_writeb(com_port, \
> - (unsigned char *)addr - (unsigned char *)com_port, value)
> -#define serial_in(addr) \
> - ns16550_readb(com_port, \
> - (unsigned char *)addr - (unsigned char *)com_port)
> +#define serial_out(value, addr) do { \
> + int offset = (unsigned char *)addr - (unsigned char *)com_port; \
> + com_port->plat->out(com_port, offset, value); \
I really don't want to introduce function pointers here. This driver
is a mess but my hope was that when we remove the non-driver-model
code we can clean it up.
I suggest storing the access method in com_port->plat and then using
if() or switch() to select the right one. We can always add #ifdefs to
keep the code size down if it becomes a problem.
> +} while (0)
> +
> +#define serial_in(addr) ({ \
> + int offset = (unsigned char *)addr - (unsigned char *)com_port; \
> + int value = com_port->plat->in(com_port, offset); \
> + value; \
> +})
> #endif
>
> static inline int calc_divisor(NS16550_t port, int clock, int baudrate)
> @@ -365,9 +386,10 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> {
> struct ns16550_platdata *plat = dev->platdata;
> fdt_addr_t addr;
> + unsigned int flags;
>
> /* try Processor Local Bus device first */
> - addr = dev_get_addr(dev);
> + addr = dev_get_addr_flags(dev, &flags);
> #if defined(CONFIG_PCI) && defined(CONFIG_DM_PCI)
> if (addr == FDT_ADDR_T_NONE) {
> /* then try pci device */
> @@ -400,6 +422,14 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> if (addr == FDT_ADDR_T_NONE)
> return -EINVAL;
>
> + if (flags & IORESOURCE_IO) {
> + plat->in = ns16550_inb;
> + plat->out = ns16550_outb;
> + } else {
> + plat->in = ns16550_readb;
> + plat->out = ns16550_writeb;
> + }
> +
> plat->base = addr;
> plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> "reg-shift", 0);
> diff --git a/include/ns16550.h b/include/ns16550.h
> index 4e62067..097f64b 100644
> --- a/include/ns16550.h
> +++ b/include/ns16550.h
> @@ -45,19 +45,6 @@
> unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
> #endif
>
> -/**
> - * struct ns16550_platdata - information about a NS16550 port
> - *
> - * @base: Base register address
> - * @reg_shift: Shift size of registers (0=byte, 1=16bit, 2=32bit...)
> - * @clock: UART base clock speed in Hz
> - */
> -struct ns16550_platdata {
> - unsigned long base;
> - int reg_shift;
> - int clock;
> -};
> -
> struct udevice;
>
> struct NS16550 {
> @@ -100,6 +87,24 @@ struct NS16550 {
>
> typedef struct NS16550 *NS16550_t;
>
> +/**
> + * struct ns16550_platdata - information about a NS16550 port
> + *
> + * @base: Base register address
> + * @reg_shift: Shift size of registers (0=byte, 1=16bit, 2=32bit...)
> + * @clock: UART base clock speed in Hz
> + * @is_io: Use I/O, not memory, accessors
> + */
> +struct ns16550_platdata {
> + unsigned long base;
> + int reg_shift;
> + int clock;
> +#ifdef CONFIG_DM_SERIAL
> + int (*in)(NS16550_t port, int offset);
> + void (*out)(NS16550_t port, int offset, int value);
> +#endif
> +};
Does the above need to move? It would reduce the diff if you left it
in the same place.
> +
> /*
> * These are the definitions for the FIFO Control Register
> */
> --
> 2.7.0
>
Regards,
Simon
More information about the U-Boot
mailing list