diff mbox series

tty: serial: imx: fix rs485 rx after tx

Message ID 20230616104838.2729694-1-martin.fuzzey@flowbird.group
State New
Headers show
Series tty: serial: imx: fix rs485 rx after tx | expand

Commit Message

Martin Fuzzey June 16, 2023, 10:47 a.m. UTC
Since commit 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal
active high") RS485 reception no longer works after a transmission.

The following scenario shows the problem:
	1) Open a port in RS485 mode
	2) Receive data from remote (OK)
	3) Transmit data to remote (OK)
	4) Receive data from remote (Nothing received)

In RS485 mode, imx_uart_start_tx() calls imx_uart_stop_rx() and, when the
transmission is complete, imx_uart_stop_tx() calls imx_uart_start_rx().

Since the above commit imx_uart_stop_rx() now sets the loopback bit but
imx_uart_start_rx() does not clear it causing the hardware to remain in
loopback mode and not receive external data.

Fix this by moving the existing loopback disable code to a helper function
and calling it from imx_uart_start_rx() too.

Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
---
 drivers/tty/serial/imx.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Marek Vasut July 6, 2023, 10:11 p.m. UTC | #1
On 6/16/23 13:16, Marek Vasut wrote:
> On 6/16/23 12:47, Martin Fuzzey wrote:
>> Since commit 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal
>> active high") RS485 reception no longer works after a transmission.
> 
> This RS485 is just a gift that keeps on giving, sigh.
> 
> I'll dig into this in a few days as time permits.

Alas, time did not permit until now, but yes, this makes sense.

Thanks

And sorry for the delayed reply.
Sebastien Laveze Aug. 7, 2023, 9:48 a.m. UTC | #2
> Since commit 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal
> active high") RS485 reception no longer works after a transmission.

I can confirm on a Modbus/RS485 setup.

> Fix this by moving the existing loopback disable code to a helper
> function
> and calling it from imx_uart_start_rx() too.
> 
> Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active
> high")

Unfortunately this doesn't fix the regression on my setup and I had to
fully revert 79d0224f6bf2. 

Since there's a Modbus layer on top, it's always TX to remote then RX.

Note that RS485 communication has never been perfect on my setup. After
TX the DE line is often held active for too long leading to corrupted
RX if too close from last TX. This leads to occasional frame loss in
Modbus but it's not a blocker. Hope to get some time to investigate.

https://bugzilla.kernel.org/show_bug.cgi?id=207751
diff mbox series

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c5e17569c3ad..3fe8ff241522 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -369,6 +369,16 @@  static void imx_uart_soft_reset(struct imx_port *sport)
 	sport->idle_counter = 0;
 }
 
+static void imx_uart_disable_loopback_rs485(struct imx_port *sport)
+{
+	unsigned int uts;
+
+	/* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */
+	uts = imx_uart_readl(sport, imx_uart_uts_reg(sport));
+	uts &= ~UTS_LOOP;
+	imx_uart_writel(sport, uts, imx_uart_uts_reg(sport));
+}
+
 /* called with port.lock taken and irqs off */
 static void imx_uart_start_rx(struct uart_port *port)
 {
@@ -390,6 +400,7 @@  static void imx_uart_start_rx(struct uart_port *port)
 	/* Write UCR2 first as it includes RXEN */
 	imx_uart_writel(sport, ucr2, UCR2);
 	imx_uart_writel(sport, ucr1, UCR1);
+	imx_uart_disable_loopback_rs485(sport);
 }
 
 /* called with port.lock taken and irqs off */
@@ -1422,7 +1433,7 @@  static int imx_uart_startup(struct uart_port *port)
 	int retval;
 	unsigned long flags;
 	int dma_is_inited = 0;
-	u32 ucr1, ucr2, ucr3, ucr4, uts;
+	u32 ucr1, ucr2, ucr3, ucr4;
 
 	retval = clk_prepare_enable(sport->clk_per);
 	if (retval)
@@ -1521,10 +1532,7 @@  static int imx_uart_startup(struct uart_port *port)
 		imx_uart_writel(sport, ucr2, UCR2);
 	}
 
-	/* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */
-	uts = imx_uart_readl(sport, imx_uart_uts_reg(sport));
-	uts &= ~UTS_LOOP;
-	imx_uart_writel(sport, uts, imx_uart_uts_reg(sport));
+	imx_uart_disable_loopback_rs485(sport);
 
 	spin_unlock_irqrestore(&sport->port.lock, flags);