On Wednesday 11 January 2023 08:53:30 Loic Poulain wrote:
> On Wed, 11 Jan 2023 at 00:53, Pali Rohár <p...@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.poul...@linaro.org>
> > > Tested-by: Lothar Waßmann <l...@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.
