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

Simon Glass sjg at chromium.org
Thu Dec 17 16:27:37 CET 2015 

Hi Michal,

On 17 December 2015 at 06:58, Michal Simek <michal.simek at xilinx.com> wrote:
> 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?

Be careful to return 1 if you don't know how many characters are
waiting. It looks like you return 'in_be32(&regs->status) &
SR_RX_FIFO_VALID_DATA' which could be >1.

Regards,
Simon

More information about the U-Boot mailing list