[U-Boot] [PATCH v2 1/3] serial: uartlite: Move driver to DM
Michal Simek
michal.simek at xilinx.com
Thu Dec 17 19:52:39 CET 2015
On 17.12.2015 16:27, Simon Glass wrote:
> 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(®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?
>
> Be careful to return 1 if you don't know how many characters are
> waiting. It looks like you return 'in_be32(®s->status) &
> SR_RX_FIFO_VALID_DATA' which could be >1.
IP doesn't provide information how many chars is in FIFO. It can be 1 to
16.
No problem to return just 1 for more than 1 char in FIFO.
Is core working with that information?
Thanks,
Michal
More information about the U-Boot
mailing list