[PATCH v3 1/2] serial: mxc: Wait for TX completion before reset

[Loic Poulain]

On Wed, 11 Jan 2023 at 00:53, Pali Rohár <pali at kernel.org> wrote:
>
> On Tuesday 10 January 2023 20:24:06 Loic Poulain wrote:
> > The u-boot console may show some corrupted characters when
> > printing in board_init() due to reset of the UART (probe)
> > before the TX FIFO has been completely drained.
> >
> > To fix this issue, and in case UART is still running, we now
> > try to flush the FIFO before proceeding to UART reinitialization.
> > For this we're waiting for Transmitter Complete bit, indicating
> > that the FIFO and the shift register are empty.
> >
> > flushing has a 4ms timeout guard, which is normally more than
> > enough to consume the FIFO @ low baudrate (9600bps).
> >
> > Signed-off-by: Loic Poulain <loic.poulain at linaro.org>
> > Tested-by: Lothar Waßmann <LW at KARO-electronics.de>
>
> Hello! Last time when I looked at this driver I was in impression that
> also _mxc_serial_setbrg() function requires calling some flush function
> at the beginning. Could you please check if it is needed or not? I'm
> really not sure.

_mxc_serial_setbrg is usually called after init, which now includes that flush.

>
> > ---
> >  v2: Add this commit to the series
> >  v3: Fix typo & reordering commits for good bisectability
> >
> >  drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> > index 82c0d84628..3c89781f2c 100644
> > --- a/drivers/serial/serial_mxc.c
> > +++ b/drivers/serial/serial_mxc.c
> > @@ -13,6 +13,7 @@
> >  #include <dm/platform_data/serial_mxc.h>
> >  #include <serial.h>
> >  #include <linux/compiler.h>
> > +#include <linux/delay.h>
> >
> >  /* UART Control Register Bit Fields.*/
> >  #define URXD_CHARRDY (1<<15)
> > @@ -144,8 +145,22 @@ struct mxc_uart {
> >       u32 ts;
> >  };
> >
> > +static void _mxc_serial_flush(struct mxc_uart *base)
> > +{
> > +     unsigned int timeout = 4000;
> > +
> > +     if (!(readl(&base->cr1) & UCR1_UARTEN) ||
> > +         !(readl(&base->cr2) & UCR2_TXEN))
> > +             return;
> > +
> > +     while (!(readl(&base->sr2) & USR2_TXDC) && --timeout)
> > +             udelay(1);
> > +}
> > +
> >  static void _mxc_serial_init(struct mxc_uart *base, int use_dte)
> >  {
> > +     _mxc_serial_flush(base);
> > +
> >       writel(0, &base->cr1);
> >       writel(0, &base->cr2);
> >
> > @@ -252,10 +267,17 @@ static int mxc_serial_init(void)
> >       return 0;
> >  }
> >
> > +static int mxc_serial_stop(void)
> > +{
> > +     _mxc_serial_flush(mxc_base);
> > +
> > +     return 0;
> > +}
> > +
> >  static struct serial_device mxc_serial_drv = {
> >       .name   = "mxc_serial",
> >       .start  = mxc_serial_init,
> > -     .stop   = NULL,
> > +     .stop   = mxc_serial_stop,
> >       .setbrg = mxc_serial_setbrg,
> >       .putc   = mxc_serial_putc,
> >       .puts   = default_serial_puts,
>
> Anyway, this code touches _only_ non-DM version of the driver. So same
> fix should be implemented also for DM version because non-DM is now
> legacy and will be removed in the future from U-Boot. Please look at the
> DM version too.

This code impacts both DM and non-DM, as _mxc_serial_init is a common version,
and was the main issue (several init() leading to corrupted char).

Regards,
Loic