[U-Boot] [PATCH v2 1/3] serial: uartlite: Move driver to DM
Michal Simek
michal.simek at xilinx.com
Fri Dec 18 09:23:32 CET 2015
On 18.12.2015 09:12, Thomas Chou wrote:
> Hi Michal,
>
> On 2015年12月18日 15:52, Michal Simek wrote:
>> On 18.12.2015 00:35, Thomas Chou wrote:
>>> Hi Michal,
>>>
>>> On 2015年12月17日 21:58, Michal Simek 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(®s->status) & SR_TX_FIFO_FULL)
>>>>>> + return -EAGAIN;
>>>>>>
>>>>>> - while (in_be32(®s->status) & SR_TX_FIFO_FULL)
>>>>>> - ;
>>>>>> - out_be32(®s->tx_fifo, c & 0xff);
>>>>>> -}
>>>>>> + out_be32(®s->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(®s->status) & SR_RX_FIFO_VALID_DATA))
>>>>>> + return -EAGAIN;
>>>>>>
>>>>>> - while (!(in_be32(®s->status) & SR_RX_FIFO_VALID_DATA))
>>>>>> - ;
>>>>>> return in_be32(®s->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(®s->status) & SR_RX_FIFO_VALID_DATA;
>>>>>> + if (input)
>>>>>> + return in_be32(®s->status) & SR_RX_FIFO_VALID_DATA;
>>>>>> +
>>>>>> + return in_be32(®s->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?
>>>>
>>>
>>> + return in_be32(®s->status) & SR_TX_FIFO_EMPTY;
>>>
>>> Yes, the logic is wrong. Your code return non 0 if empty, and 0 if not
>>> empty.
>>
>> right.
>>
>> return !in_be32(®s->status) & SR_TX_FIFO_EMPTY;
>
> As suggested by Simon, it might be better to return 1 when we don't know
> the count of pending.
>
> if (input)
> return (in_be32(®s->status) & SR_RX_FIFO_VALID_DATA) ? 1 : 0;
SR_RX_FIFO_VALID_DATA is 1 already that's why I haven't done this
because it is not needed.
>
> return (in_be32(®s->status) & SR_TX_FIFO_EMPTY) ? 0 : 1;
And here FIFO_EMPTY is BIT(2)
It means this should also work.
return !(in_be32(®s->status) & SR_TX_FIFO_EMPTY);
Thanks,
Michal
More information about the U-Boot
mailing list