[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 at gmail.com> wrote:
>>
>> From: Fabio Estevam <festevam at 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 at gateworks.com>
>> Suggested-by: Pali Rohár <pali at kernel.org>
>> Signed-off-by: Fabio Estevam <festevam at 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