| Message ID | 20221025213136.873709-1-festevam@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | serial: mxc: Wait until TX FIFO is not full | expand |
Hi On Tue, Oct 25, 2022 at 11:32 PM Fabio Estevam <festevam@gmail.com> wrote: > > From: Fabio Estevam <festevam@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@gateworks.com> > Suggested-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Fabio Estevam <festevam@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 > -- > 2.25.1 >
On 10/26/22 13:08, Michael Nazzareno Trimarchi wrote: > Hi > > On Tue, Oct 25, 2022 at 11:32 PM Fabio Estevam <festevam@gmail.com> wrote: >> >> From: Fabio Estevam <festevam@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@gateworks.com> >> Suggested-by: Pali Rohár <pali@kernel.org> >> Signed-off-by: Fabio Estevam <festevam@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
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);