[U-Boot] [PATCH 1/3] serial: uartlite: Move driver to DM

Michal Simek
2015-12-14T16:03:21Z

On 14.12.2015 15:47, Thomas Chou wrote:
> Hi Michal,
>
> On 2015?12?11? 19:54, Michal Simek wrote:
>> Enable SPL DM too.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> arch/microblaze/Kconfig | 1 +
>> configs/microblaze-generic_defconfig | 2 +
>> doc/driver-model/serial-howto.txt | 1 -
>> drivers/serial/serial_xuartlite.c | 176
>> ++++++++++++++---------------------
>> 4 files changed, 71 insertions(+), 109 deletions(-)
>>
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> index 604f6815af5b..30ea484f48aa 100644
>> --- a/arch/microblaze/Kconfig
>> +++ b/arch/microblaze/Kconfig
>> @@ -13,6 +13,7 @@ config TARGET_MICROBLAZE_GENERIC
>> select SUPPORT_SPL
>> select OF_CONTROL
>> select DM
>> + select DM_SERIAL
>>
>> endchoice
>>
>> diff --git a/configs/microblaze-generic_defconfig
>> b/configs/microblaze-generic_defconfig
>> index 54aa3ef3d26f..5df080b6a87c 100644
>> --- a/configs/microblaze-generic_defconfig
>> +++ b/configs/microblaze-generic_defconfig
>> @@ -1,9 +1,11 @@
>> CONFIG_MICROBLAZE=y
>> CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>> +CONFIG_SPL_DM=y
>> CONFIG_TARGET_MICROBLAZE_GENERIC=y
>> CONFIG_DEFAULT_DEVICE_TREE="microblaze-generic"
>> CONFIG_SPL=y
>> CONFIG_SYS_PROMPT="U-Boot-mONStR> "
>> CONFIG_CMD_GPIO=y
>> # CONFIG_CMD_SETEXPR is not set
>> +CONFIG_SPL_OF_CONTROL=y
>> CONFIG_OF_EMBED=y
>> diff --git a/doc/driver-model/serial-howto.txt
>> b/doc/driver-model/serial-howto.txt
>> index 60483a4c49bc..6688abc4d9e3 100644
>> --- a/doc/driver-model/serial-howto.txt
>> +++ b/doc/driver-model/serial-howto.txt
>> @@ -19,7 +19,6 @@ is time for maintainers to start converting over the
>> remaining serial drivers:
>> serial_s3c24x0.c
>> serial_sa1100.c
>> serial_stm32.c
>> - serial_xuartlite.c
>> usbtty.c
>>
>> You should complete this by the end of January 2016.
>> diff --git a/drivers/serial/serial_xuartlite.c
>> b/drivers/serial/serial_xuartlite.c
>> index 988438e75471..10089f5a34b5 100644
>> --- a/drivers/serial/serial_xuartlite.c
>> +++ b/drivers/serial/serial_xuartlite.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * (C) Copyright 2008-2011 Michal Simek <monstr@monstr.eu>
>> + * (C) Copyright 2008 - 2015 Michal Simek <monstr@monstr.eu>
>> * Clean driver and add xilinx constant from header file
>> *
>> * (C) Copyright 2004 Atmark Techno, Inc.
>> @@ -10,11 +10,17 @@
>>
>> #include <config.h>
>> #include <common.h>
>> +#include <debug_uart.h>
>
> Move to debug uart section.
>
>> +#include <dm.h>
>> #include <asm/io.h>
>> #include <linux/compiler.h>
>> #include <serial.h>
>> +#include <watchdog.h>
>
> watchdog.h is not needed.

ok these two should be added when debug_uart is added.
The rest is in the second reply.

>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>>
>> #define SR_TX_FIFO_FULL 0x08 /* transmit FIFO full */
>> +#define SR_TX_FIFO_EMPTY 0x04 /* transmit FIFO empty */
>> #define SR_RX_FIFO_VALID_DATA 0x01 /* data in receive FIFO */
>> #define SR_RX_FIFO_FULL 0x02 /* receive FIFO full */
>>
>> @@ -28,135 +34,89 @@ struct uartlite {
>> unsigned int control;
>> };
>>
>> -static struct uartlite *userial_ports[4] = {
>> -#ifdef XILINX_UARTLITE_BASEADDR
>> - [0] = (struct uartlite *)XILINX_UARTLITE_BASEADDR,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR1
>> - [1] = (struct uartlite *)XILINX_UARTLITE_BASEADDR1,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR2
>> - [2] = (struct uartlite *)XILINX_UARTLITE_BASEADDR2,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR3
>> - [3] = (struct uartlite *)XILINX_UARTLITE_BASEADDR3
>> -#endif
>> +struct uartlite_priv {
>> + struct uartlite *regs;
>> };
>>
>> -static void uartlite_serial_putc(const char c, const int port)
>> +static int uartlite_serial_putc(struct udevice *dev, const char ch)
>> {
>> - struct uartlite *regs = userial_ports[port];
>> + struct uartlite_priv *priv = dev_get_priv(dev);
>> + struct uartlite *regs = priv->regs;
>>
>> - if (c == '\n')
>> - uartlite_serial_putc('\r', port);
>> + if (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>> + return -EAGAIN;
>>
>> - while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>> - ;
>> - out_be32(&regs->tx_fifo, c & 0xff);
>> -}
>> + out_be32(&regs->tx_fifo, ch & 0xff);
>>
>> -static void uartlite_serial_puts(const char *s, const int port)
>> -{
>> - while (*s)
>> - uartlite_serial_putc(*s++, port);
>> + return 0;
>> }
>>
>> -static int uartlite_serial_getc(const int port)
>> +static int uartlite_serial_getc(struct udevice *dev)
>> {
>> - struct uartlite *regs = userial_ports[port];
>> + struct uartlite_priv *priv = dev_get_priv(dev);
>> + struct uartlite *regs = priv->regs;
>> +
>> + if (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
>> + return -EAGAIN;
>>
>> - while (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
>> - ;
>> return in_be32(&regs->rx_fifo) & 0xff;
>> }
>>
>> -static int uartlite_serial_tstc(const int port)
>> +static int uartlite_serial_pending(struct udevice *dev, bool input)
>> {
>> - struct uartlite *regs = userial_ports[port];
>> + struct uartlite_priv *priv = dev_get_priv(dev);
>> + struct uartlite *regs = priv->regs;
>> +
>> + if (input)
>> + return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>>
>> - return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>> + return in_be32(&regs->status) & SR_TX_FIFO_EMPTY;
>> }
>>
>> -static int uartlite_serial_init(const int port)
>> +static int uartlite_serial_probe(struct udevice *dev)
>> {
>> - struct uartlite *regs = userial_ports[port];
>> + struct uartlite_priv *priv = dev_get_priv(dev);
>> + struct uartlite *regs = priv->regs;
>>
>> - if (regs) {
>> - out_be32(&regs->control, 0);
>> - out_be32(&regs->control,
>> - ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
>> - in_be32(&regs->control);
>> - return 0;
>> - }
>> + out_be32(&regs->control, 0);
>> + out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>> ULITE_CONTROL_RST_TX);
>> + in_be32(&regs->control);
>>
>> - return -1;
>> + return 0;
>> }
>>
>> -/* Multi serial device functions */
>> -#define DECLARE_ESERIAL_FUNCTIONS(port) \
>> - static int userial##port##_init(void) \
>> - { return uartlite_serial_init(port); } \
>> - static void userial##port##_setbrg(void) {} \
>> - static int userial##port##_getc(void) \
>> - { return uartlite_serial_getc(port); } \
>> - static int userial##port##_tstc(void) \
>> - { return uartlite_serial_tstc(port); } \
>> - static void userial##port##_putc(const char c) \
>> - { uartlite_serial_putc(c, port); } \
>> - static void userial##port##_puts(const char *s) \
>> - { uartlite_serial_puts(s, port); }
>> -
>> -/* Serial device descriptor */
>> -#define INIT_ESERIAL_STRUCTURE(port, __name) { \
>> - .name = __name, \
>> - .start = userial##port##_init, \
>> - .stop = NULL, \
>> - .setbrg = userial##port##_setbrg, \
>> - .getc = userial##port##_getc, \
>> - .tstc = userial##port##_tstc, \
>> - .putc = userial##port##_putc, \
>> - .puts = userial##port##_puts, \
>> -}
>> -
>> -DECLARE_ESERIAL_FUNCTIONS(0);
>> -struct serial_device uartlite_serial0_device =
>> - INIT_ESERIAL_STRUCTURE(0, "ttyUL0");
>> -DECLARE_ESERIAL_FUNCTIONS(1);
>> -struct serial_device uartlite_serial1_device =
>> - INIT_ESERIAL_STRUCTURE(1, "ttyUL1");
>> -DECLARE_ESERIAL_FUNCTIONS(2);
>> -struct serial_device uartlite_serial2_device =
>> - INIT_ESERIAL_STRUCTURE(2, "ttyUL2");
>> -DECLARE_ESERIAL_FUNCTIONS(3);
>> -struct serial_device uartlite_serial3_device =
>> - INIT_ESERIAL_STRUCTURE(3, "ttyUL3");
>> -
>> -__weak struct serial_device *default_serial_console(void)
>> +static int uartlite_serial_ofdata_to_platdata(struct udevice *dev)
>> {
>> - if (userial_ports[0])
>> - return &uartlite_serial0_device;
>> - if (userial_ports[1])
>> - return &uartlite_serial1_device;
>> - if (userial_ports[2])
>> - return &uartlite_serial2_device;
>> - if (userial_ports[3])
>> - return &uartlite_serial3_device;
>> -
>> - return NULL;
>> -}
>> + struct uartlite_priv *priv = dev_get_priv(dev);
>
> Since the conversion is "ofdatta to platdata", this should be platdata
> rather than priv. If there is not variable part, you may simply use
> platdata in the ops. In some cases, you may add device from platdata
> without ofdata.

look below.

>
>> + fdt_addr_t addr;
>>
>> -void uartlite_serial_initialize(void)
>> -{
>> -#ifdef XILINX_UARTLITE_BASEADDR
>> - serial_register(&uartlite_serial0_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR */
>> -#ifdef XILINX_UARTLITE_BASEADDR1
>> - serial_register(&uartlite_serial1_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR1 */
>> -#ifdef XILINX_UARTLITE_BASEADDR2
>> - serial_register(&uartlite_serial2_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR2 */
>> -#ifdef XILINX_UARTLITE_BASEADDR3
>> - serial_register(&uartlite_serial3_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR3 */
>> + addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>
> Use dev_get_addr() is preferred.

Will fix thanks.

>
>> + if (addr == FDT_ADDR_T_NONE)
>> + return -EINVAL;
>> +
>> + priv->regs = (struct uartlite *)addr;
>> +
>> + return 0;
>> }
>> +
>> +static const struct dm_serial_ops uartlite_serial_ops = {
>> + .putc = uartlite_serial_putc,
>> + .pending = uartlite_serial_pending,
>> + .getc = uartlite_serial_getc,
>> +};
>> +
>> +static const struct udevice_id uartlite_serial_ids[] = {
>> + { .compatible = "xlnx,xps-uartlite-1.00.a" },
>> + { }
>> +};
>> +
>
> Please add binding to doc/device-tree-bindings/serial/ .

ok.

>
>> +U_BOOT_DRIVER(serial_uartlite) = {
>> + .name = "serial_uartlite",
>> + .id = UCLASS_SERIAL,
>> + .of_match = uartlite_serial_ids,
>> + .ofdata_to_platdata = uartlite_serial_ofdata_to_platdata,
>> + .priv_auto_alloc_size = sizeof(struct uartlite_priv),
>
> maybe platdata.

no problem with it.

Thanks,
Michal