RE: [PATCH v3] tty: serial: imx: Fix broken RS485

[Christoph Niedermaier <cniedermaier@dh-electronics.com>]

From: Rickard Andersson <rickaran@axis.com>
Sent: Wednesday, February 21, 2024 12:53 PM
> 
> From: Rickard x Andersson <rickaran@axis.com>
> 
> When about to transmit the function imx_uart_start_tx is called and in
> some RS485 configurations this function will call imx_uart_stop_rx. The
> problem is that imx_uart_stop_rx will enable loopback in order to
> release the RS485 bus, but when loopback is enabled transmitted data
> will just be looped to RX.
> 
> This patch fixes the above problem by not enabling loopback when about
> to transmit.
> 
> This driver now works well when used for RS485 half duplex master
> configurations.
> 
> Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high")
> Signed-off-by: Rickard x Andersson <rickaran@axis.com>
> ---
> V2 -> V3: Added wrapper function
> V1 -> V2: Added fixes tag
>  drivers/tty/serial/imx.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 4aa72d5aeafb..e14813250616 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -462,8 +462,7 @@ static void imx_uart_stop_tx(struct uart_port *port)
>         }
>  }
> 
> -/* called with port.lock taken and irqs off */
> -static void imx_uart_stop_rx(struct uart_port *port)
> +static void imx_uart_stop_rx_with_loopback_ctrl(struct uart_port *port, bool loopback)
>  {
>         struct imx_port *sport = (struct imx_port *)port;
>         u32 ucr1, ucr2, ucr4, uts;
> @@ -485,7 +484,7 @@ static void imx_uart_stop_rx(struct uart_port *port)
>         /* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */
>         if (port->rs485.flags & SER_RS485_ENABLED &&
>             port->rs485.flags & SER_RS485_RTS_ON_SEND &&
> -           sport->have_rtscts && !sport->have_rtsgpio) {
> +           sport->have_rtscts && !sport->have_rtsgpio && loopback) {
>                 uts = imx_uart_readl(sport, imx_uart_uts_reg(sport));
>                 uts |= UTS_LOOP;
>                 imx_uart_writel(sport, uts, imx_uart_uts_reg(sport));
> @@ -497,6 +496,16 @@ static void imx_uart_stop_rx(struct uart_port *port)
>         imx_uart_writel(sport, ucr2, UCR2);
>  }
> 
> +/* called with port.lock taken and irqs off */
> +static void imx_uart_stop_rx(struct uart_port *port)
> +{
> +       /*
> +        * Stop RX and enable loopback in order to make sure RS485 bus
> +        * is not blocked. Se comment in imx_uart_probe().
> +        */
> +       imx_uart_stop_rx_with_loopback_ctrl(port, true);
> +}
> +
>  /* called with port.lock taken and irqs off */
>  static void imx_uart_enable_ms(struct uart_port *port)
>  {
> @@ -682,9 +691,14 @@ static void imx_uart_start_tx(struct uart_port *port)
>                                 imx_uart_rts_inactive(sport, &ucr2);
>                         imx_uart_writel(sport, ucr2, UCR2);
> 
> +                       /*
> +                        * Since we are about to transmit we can not stop RX
> +                        * with loopback enabled because that will make our
> +                        * transmitted data being just looped to RX.
> +                        */
>                         if (!(port->rs485.flags & SER_RS485_RX_DURING_TX) &&
>                             !port->rs485_rx_during_tx_gpio)
> -                               imx_uart_stop_rx(port);
> +                               imx_uart_stop_rx_with_loopback_ctrl(port, false);
> 
>                         sport->tx_state = WAIT_AFTER_RTS;
> 

Tested-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>

Tested on a DHCOM i.MX6ULL SoM.

Regards
Christoph