[PATCH] serial: mxc: Wait until TX FIFO is not full

[PATCH] serial: mxc: Wait until TX FIFO is not full

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

Tim Harvey reported the console garbage on imx6 since
commit c7878a0483c5 ("serial: mxc: have putc use the TXFIFO").

Do as suggested by Pali Rohár where the the driver should
not return -EAGAIN when the TX FIFO  is full.

It should keep waiting until the TX FIFO is no longer full.

This also aligns with the implementation of the imx serial driver
in the kernel:

static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch)
{
    struct imx_port *sport = (struct imx_port *)port;

    while (imx_uart_readl(sport, imx_uart_uts_reg(sport)) & UTS_TXFULL)
        barrier();

    imx_uart_writel(sport, ch, URTX0);
}

Fixes: c7878a0483c5 ("serial: mxc: have putc use the TXFIFO")
Reported-by: Tim Harvey <tharvey@gateworks.com>
Suggested-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/serial/serial_mxc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index 4cf79c1ca24f..332219fa87fc 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -311,8 +311,8 @@ static int mxc_serial_putc(struct udevice *dev, const char ch)
 	struct mxc_serial_plat *plat = dev_get_plat(dev);
 	struct mxc_uart *const uart = plat->reg;
 
-	if (readl(&uart->ts) & UTS_TXFULL)
-		return -EAGAIN;
+	while (readl(&uart->ts) & UTS_TXFULL)
+		barrier();
 
 	writel(ch, &uart->txd);
 
-- 
2.25.1

Re: [PATCH] serial: mxc: Wait until TX FIFO is not full

[Michael Nazzareno Trimarchi]

Hi

On Tue, Oct 25, 2022 at 11:32 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> From: Fabio Estevam <festevam@denx.de>
>
> Tim Harvey reported the console garbage on imx6 since
> commit c7878a0483c5 ("serial: mxc: have putc use the TXFIFO").
>
> Do as suggested by Pali Rohár where the the driver should
> not return -EAGAIN when the TX FIFO  is full.
>
> It should keep waiting until the TX FIFO is no longer full.
>
> This also aligns with the implementation of the imx serial driver
> in the kernel:
>
> static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch)
> {
>     struct imx_port *sport = (struct imx_port *)port;
>
>     while (imx_uart_readl(sport, imx_uart_uts_reg(sport)) & UTS_TXFULL)
>         barrier();
>
>     imx_uart_writel(sport, ch, URTX0);
> }
>
> Fixes: c7878a0483c5 ("serial: mxc: have putc use the TXFIFO")
> Reported-by: Tim Harvey <tharvey@gateworks.com>
> Suggested-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
>  drivers/serial/serial_mxc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> index 4cf79c1ca24f..332219fa87fc 100644
> --- a/drivers/serial/serial_mxc.c
> +++ b/drivers/serial/serial_mxc.c
> @@ -311,8 +311,8 @@ static int mxc_serial_putc(struct udevice *dev, const char ch)
>         struct mxc_serial_plat *plat = dev_get_plat(dev);
>         struct mxc_uart *const uart = plat->reg;
>
> -       if (readl(&uart->ts) & UTS_TXFULL)
> -               return -EAGAIN;
> +       while (readl(&uart->ts) & UTS_TXFULL)
> +               barrier();
>
>         writel(ch, &uart->txd);
>
_debug_uart_putc

Should not be patched in the same way?

Anyway, does the external loop do the same with -EAGAIN as was
discussed in another thread?

Michael


> --
> 2.25.1
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] serial: mxc: Wait until TX FIFO is not full

[Sean Anderson]

On 10/26/22 13:08, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> On Tue, Oct 25, 2022 at 11:32 PM Fabio Estevam <festevam@gmail.com> wrote:
>>
>> From: Fabio Estevam <festevam@denx.de>
>>
>> Tim Harvey reported the console garbage on imx6 since
>> commit c7878a0483c5 ("serial: mxc: have putc use the TXFIFO").
>>
>> Do as suggested by Pali Rohár where the the driver should
>> not return -EAGAIN when the TX FIFO  is full.
>>
>> It should keep waiting until the TX FIFO is no longer full.
>>
>> This also aligns with the implementation of the imx serial driver
>> in the kernel:
>>
>> static void imx_uart_console_putchar(struct uart_port *port, unsigned char ch)
>> {
>>     struct imx_port *sport = (struct imx_port *)port;
>>
>>     while (imx_uart_readl(sport, imx_uart_uts_reg(sport)) & UTS_TXFULL)
>>         barrier();
>>
>>     imx_uart_writel(sport, ch, URTX0);
>> }
>>
>> Fixes: c7878a0483c5 ("serial: mxc: have putc use the TXFIFO")
>> Reported-by: Tim Harvey <tharvey@gateworks.com>
>> Suggested-by: Pali Rohár <pali@kernel.org>
>> Signed-off-by: Fabio Estevam <festevam@denx.de>
>> ---
>>  drivers/serial/serial_mxc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
>> index 4cf79c1ca24f..332219fa87fc 100644
>> --- a/drivers/serial/serial_mxc.c
>> +++ b/drivers/serial/serial_mxc.c
>> @@ -311,8 +311,8 @@ static int mxc_serial_putc(struct udevice *dev, const char ch)
>>         struct mxc_serial_plat *plat = dev_get_plat(dev);
>>         struct mxc_uart *const uart = plat->reg;
>>
>> -       if (readl(&uart->ts) & UTS_TXFULL)
>> -               return -EAGAIN;
>> +       while (readl(&uart->ts) & UTS_TXFULL)
>> +               barrier();
>>
>>         writel(ch, &uart->txd);
>>
> _debug_uart_putc
> 
> Should not be patched in the same way?
> 
> Anyway, does the external loop do the same with -EAGAIN as was
> discussed in another thread?
> 
> Michael

Yes, see _serial_putc. This is the DM version. The non-DM version
(mxc_serial_putc) does a while loop already.

--Sean

_serial_putc