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

Thu Dec 17 14:58:59 CET 2015

On 17.12.2015 14:37, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015年12月17日 20:00, Michal Simek wrote:
>> Enable SPL DM too.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>> Changes in v2:
>> - Remove unneeded headers
>> - Use get_dev_addr instead of fdtdec_get_addr
>> - Use platdata instead of private data
>> - Add opb compatible string to be in sync with Linux
>> - Add binding documentation
>>
>>   arch/microblaze/Kconfig                            |   1 +
>>   configs/microblaze-generic_defconfig               |   2 +
>>   .../serial/xilinx_uartlite.txt                     |  13 ++
>>   doc/driver-model/serial-howto.txt                  |   1 -
>>   drivers/serial/serial_xuartlite.c                  | 170
>> ++++++++-------------
>>   5 files changed, 78 insertions(+), 109 deletions(-)
>>   create mode 100644 doc/device-tree-bindings/serial/xilinx_uartlite.txt
>>
>> 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/device-tree-bindings/serial/xilinx_uartlite.txt
>> b/doc/device-tree-bindings/serial/xilinx_uartlite.txt
>> new file mode 100644
>> index 000000000000..d15753c8c380
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/serial/xilinx_uartlite.txt
>> @@ -0,0 +1,13 @@
>> +Binding for Xilinx Uartlite Controller
>> +
>> +Required properties:
>> +- compatible : should be "xlnx,xps-uartlite-1.00.a", or
>> "xlnx,opb-uartlite-1.00.b"
>> +- reg: Should contain UART controller registers location and length.
>> +- interrupts: Should contain UART controller interrupts.
>> +
>> +Example:
>> +    serial at 40600000 {
>> +        compatible = "xlnx,xps-uartlite-1.00.a";
>> +        interrupts = <1 0>;
>> +        reg = <0x40600000 0x10000>;
>> +    };
>> diff --git a/doc/driver-model/serial-howto.txt
>> b/doc/driver-model/serial-howto.txt
>> index 76ad629ef9cb..381a2a084562 100644
>> --- a/doc/driver-model/serial-howto.txt
>> +++ b/doc/driver-model/serial-howto.txt
>> @@ -18,7 +18,6 @@ is time for maintainers to start converting over the
>> remaining serial drivers:
>>      serial_pxa.c
>>      serial_s3c24x0.c
>>      serial_sa1100.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..8225d9a320a5 100644
>> --- a/drivers/serial/serial_xuartlite.c
>> +++ b/drivers/serial/serial_xuartlite.c
>> @@ -1,5 +1,5 @@
>>   /*
>> - * (C) Copyright 2008-2011 Michal Simek <monstr at monstr.eu>
>> + * (C) Copyright 2008 - 2015 Michal Simek <monstr at monstr.eu>
>>    * Clean driver and add xilinx constant from header file
>>    *
>>    * (C) Copyright 2004 Atmark Techno, Inc.
>> @@ -10,11 +10,15 @@
>>
>>   #include <config.h>
>>   #include <common.h>
>> +#include <dm.h>
>>   #include <asm/io.h>
>>   #include <linux/compiler.h>
>>   #include <serial.h>
>>
>> +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 +32,85 @@ 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_platdata {
>> +    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_platdata *plat = dev_get_platdata(dev);
>> +    struct uartlite *regs = plat->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_platdata *plat = dev_get_platdata(dev);
>> +    struct uartlite *regs = plat->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_platdata *plat = dev_get_platdata(dev);
>> +    struct uartlite *regs = plat->regs;
>>
>> -    return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>> +    if (input)
>> +        return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>> +
>> +    return in_be32(&regs->status) & SR_TX_FIFO_EMPTY;
> 
> Should be inversed.
> 
> Otherwise,
> Reviewed-by: Thomas Chou <thomas at wytron.com.tw>

Can you please tell me more about it?
I was trying to understand logic of this function but
pending function is called just from here. (maybe I missed other
occurrences but
drivers/serial/serial-uclass.c:157:     if (ops->pending)
drivers/serial/serial-uclass.c:158:             return ops->pending(dev,
true);

And not for output.

>From docs
@return number of waiting characters, 0 for none, -ve on error

It means I do read status and mask it if tx fifo is empty or not.
If it is empty I am returning 0, if it is not empty I do return non 0 value.

The similar logix is used for input.

Is this logic wrong?

Thanks,
Michal