BUG: imx: initial UART receive causes IO errors

Commit Message

Stefan Wahren Nov. 17, 2020, 8:14 a.m. UTC

Hi,

we are using a Modbus application on our customer i.MX6ULL board (RS-485
in hardware, no DMA). While porting the board support package to a
Mainline kernel, we noticed that after boot the initial UART receive
causes IO errors, which breaks our application (tested with Mainline
4.9, 4.19, 5.7, 5.9 but without success). The ancient vendor kernel
(imx-4.9.11) we are still using isn't affect by this issue.

So i bisected the vendor tree and found this huge patch which fixed the
issue [1]. After that i was able to narrow down the fix to the following
patch against Mainline Linux 5.9 (successful tested on i.MX6ULL):

 
@@ -1415,6 +1416,7 @@ static int imx_uart_startup(struct uart_port *port)
     unsigned long flags;
     int dma_is_inited = 0;
     u32 ucr1, ucr2, ucr3, ucr4;
+    unsigned char rx_fifo_trig;
 
     retval = clk_prepare_enable(sport->clk_per);
     if (retval)
@@ -1425,7 +1427,12 @@ static int imx_uart_startup(struct uart_port *port)
         return retval;
     }
 
-    imx_uart_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
+    if (uart_console(&sport->port))
+        rx_fifo_trig = RXTL_DEFAULT;
+    else
+        rx_fifo_trig = RXTL_UART;
+
+    imx_uart_setup_ufcr(sport, TXTL_DEFAULT, rx_fifo_trig);
 
     /* disable the DREN bit (Data Ready interrupt enable) before
      * requesting IRQs
@@ -1674,7 +1681,7 @@ imx_uart_set_termios(struct uart_port *port,
struct ktermios *termios,
      * except those we will or may need to preserve.
      */
     old_ucr2 = imx_uart_readl(sport, UCR2);
-    ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);
+    ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_CTS);
 
     ucr2 |= UCR2_SRST | UCR2_IRTS;
     if ((termios->c_cflag & CSIZE) == CS8)
@@ -1795,6 +1802,9 @@ imx_uart_set_termios(struct uart_port *port,
struct ktermios *termios,
     if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
         imx_uart_enable_ms(&sport->port);
 
+    ucr2 = imx_uart_readl(sport, UCR2);
+    imx_uart_writel(sport, ucr2 | UCR2_ATEN, UCR2);
+
     spin_unlock_irqrestore(&sport->port.lock, flags);
 }

So i someone with a deeper insight can fix this properly.

Thanks Stefan

[1] -
https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/tty/serial/imx.c?h=imx_4.9.11_1.0.0_ga&id=e287334648e0a0f6a76f3d9615eada6cd590cbd9

Comments

Stefan Wahren Dec. 7, 2020, 2:09 p.m. UTC | #1

Hi,

Am 17.11.20 um 09:14 schrieb Stefan Wahren:
> Hi,
>
> we are using a Modbus application on our customer i.MX6ULL board (RS-485
> in hardware, no DMA). While porting the board support package to a
> Mainline kernel, we noticed that after boot the initial UART receive
> causes IO errors, which breaks our application (tested with Mainline
> 4.9, 4.19, 5.7, 5.9 but without success). The ancient vendor kernel
> (imx-4.9.11) we are still using isn't affect by this issue.
>
> So i bisected the vendor tree and found this huge patch which fixed the
> issue [1]. After that i was able to narrow down the fix to the following
> patch against Mainline Linux 5.9 (successful tested on i.MX6ULL):
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 07974f2..3004b5c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1289,6 +1289,7 @@ static void imx_uart_clear_rx_errors(struct
> imx_port *sport)
>  
>  #define TXTL_DEFAULT 2 /* reset default */
>  #define RXTL_DEFAULT 1 /* reset default */
> +#define RXTL_UART 16 /* For UART */
>  #define TXTL_DMA 8 /* DMA burst setting */
>  #define RXTL_DMA 9 /* DMA burst setting */
>  
> @@ -1415,6 +1416,7 @@ static int imx_uart_startup(struct uart_port *port)
>      unsigned long flags;
>      int dma_is_inited = 0;
>      u32 ucr1, ucr2, ucr3, ucr4;
> +    unsigned char rx_fifo_trig;
>  
>      retval = clk_prepare_enable(sport->clk_per);
>      if (retval)
> @@ -1425,7 +1427,12 @@ static int imx_uart_startup(struct uart_port *port)
>          return retval;
>      }
>  
> -    imx_uart_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
> +    if (uart_console(&sport->port))
> +        rx_fifo_trig = RXTL_DEFAULT;
> +    else
> +        rx_fifo_trig = RXTL_UART;
> +
> +    imx_uart_setup_ufcr(sport, TXTL_DEFAULT, rx_fifo_trig);
>  
>      /* disable the DREN bit (Data Ready interrupt enable) before
>       * requesting IRQs
> @@ -1674,7 +1681,7 @@ imx_uart_set_termios(struct uart_port *port,
> struct ktermios *termios,
>       * except those we will or may need to preserve.
>       */
>      old_ucr2 = imx_uart_readl(sport, UCR2);
> -    ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);
> +    ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_CTS);
>  
>      ucr2 |= UCR2_SRST | UCR2_IRTS;
>      if ((termios->c_cflag & CSIZE) == CS8)
> @@ -1795,6 +1802,9 @@ imx_uart_set_termios(struct uart_port *port,
> struct ktermios *termios,
>      if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
>          imx_uart_enable_ms(&sport->port);
>  
> +    ucr2 = imx_uart_readl(sport, UCR2);
> +    imx_uart_writel(sport, ucr2 | UCR2_ATEN, UCR2);
> +
>      spin_unlock_irqrestore(&sport->port.lock, flags);
>  }
>
> So i someone with a deeper insight can fix this properly.
the issue still exists in Linux 5.10 rc6.
>
> Thanks Stefan
>
> [1] -
> https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/tty/serial/imx.c?h=imx_4.9.11_1.0.0_ga&id=e287334648e0a0f6a76f3d9615eada6cd590cbd9
>
>

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 07974f2..3004b5c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1289,6 +1289,7 @@  static void imx_uart_clear_rx_errors(struct
imx_port *sport)
 
 #define TXTL_DEFAULT 2 /* reset default */
 #define RXTL_DEFAULT 1 /* reset default */
+#define RXTL_UART 16 /* For UART */
 #define TXTL_DMA 8 /* DMA burst setting */
 #define RXTL_DMA 9 /* DMA burst setting */