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

[Pali Rohár]

On Wednesday 11 January 2023 08:53:30 Loic Poulain wrote:
> 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.

I'm looking at it and this function is called also from
mxc_serial_setbrg() callback, which is used by u-boot to change baudrate
after the init.

> >
> > > ---
> > >  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

Yea, now I figured out.