| Message ID | 20230110124147.1780670-2-loic.poulain@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] serial: mxc: Speed-up character transmission | expand |
Hi Loic, On Tue, Jan 10, 2023 at 9:41 AM Loic Poulain <loic.poulain@linaro.org> 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 proceding to UART reinitialization. s/proceding/proceeding > 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@linaro.org> > --- > v2: Add this commit to the series Should this patch come first in the series? In case someone is bisecting, the current patch 1/2 may cause serial corruption, which is fixed by 2/2. Can we avoid corruption by swapping the order of these patches?
Loic Poulain <loic.poulain@linaro.org> 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 proceding 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@linaro.org> > --- > v2: Add this commit to the series > > 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 f8c49865be..9899155c00 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, > That looks good to me and seems to work well (at least for the CONFIG_DM_SERIAL case). Tested-by: Lothar Waßmann <LW@KARO-electronics.de>
On Tue, 10 Jan 2023 at 13:57, Fabio Estevam <festevam@gmail.com> wrote: > > > 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@linaro.org> > > --- > > v2: Add this commit to the series > > Should this patch come first in the series? > > In case someone is bisecting, the current patch 1/2 may cause serial corruption, > which is fixed by 2/2. > > Can we avoid corruption by swapping the order of these patches? Indeed, sending a v3. Thanks, Loic
Hi, On Tue, 10 Jan 2023 at 05:42, Loic Poulain <loic.poulain@linaro.org> 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 proceding 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@linaro.org> > --- > v2: Add this commit to the series > > drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > Can you just use serial_flush()? It is generic and should work with any device. Regards, Simon
On Wed, 11 Jan 2023 at 01:15, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Tue, 10 Jan 2023 at 05:42, Loic Poulain <loic.poulain@linaro.org> 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 proceding 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@linaro.org> > > --- > > v2: Add this commit to the series > > > > drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > Can you just use serial_flush()? It is generic and should work with any device. Not directly, except that serial_flush is specific to DM and CONSOLE_FLUSH config, it's also not exactly what we're doing here, as we want to check if the UART was running before (re)initialization, and to limit the waiting loop in case the TX/UART is in bad state or stuck due to flow control. Regards, Loic
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index f8c49865be..9899155c00 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,
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 proceding 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@linaro.org> --- v2: Add this commit to the series drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)