After all bytes to be transmitted have been written to the FIFO register, the hardware might still be busy actually sending them.
Thus, wait for the TX FIFO to be empty before starting the timer for the RTS after send delay.
Cc: stable@vger.kernel.org Fixes: fccc9d9233f9 ("tty: serial: uartps: Add rs485 support to uartps driver") Signed-off-by: Martin Kaistra martin.kaistra@linutronix.de --- Changes in v2: - Add cc stable - Add timeout
drivers/tty/serial/xilinx_uartps.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index fe457bf1e15bb..e1dd3843d563c 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -429,7 +429,7 @@ static void cdns_uart_handle_tx(void *dev_id) struct uart_port *port = (struct uart_port *)dev_id; struct cdns_uart *cdns_uart = port->private_data; struct tty_port *tport = &port->state->port; - unsigned int numbytes; + unsigned int numbytes, tmout; unsigned char ch;
if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port)) { @@ -454,6 +454,13 @@ static void cdns_uart_handle_tx(void *dev_id)
if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED && (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port))) { + /* Wait for the tx fifo to be actually empty */ + for (tmout = 1000000; tmout; tmout--) { + if (cdns_uart_tx_empty(port) == TIOCSER_TEMT) + break; + udelay(1); + } + hrtimer_update_function(&cdns_uart->tx_timer, cdns_rs485_rx_callback); hrtimer_start(&cdns_uart->tx_timer, ns_to_ktime(cdns_calc_after_tx_delay(cdns_uart)), HRTIMER_MODE_REL);
Hello Martin,
Why not just start the timer and check TEMT after it has elapsed and restart the timer if not empty? It would prevent busy-loop waiting.
Kind regards, Maarten Brock
________________________________________ From: Martin Kaistra martin.kaistra@linutronix.de Sent: Monday, August 25, 2025 11:22 To: Michal Simek michal.simek@amd.com; Greg Kroah-Hartman gregkh@linuxfoundation.org; linux-serial@vger.kernel.org linux-serial@vger.kernel.org Cc: Manikanta Guntupalli manikanta.guntupalli@amd.com; stable@vger.kernel.org stable@vger.kernel.org Subject: [PATCH v2] serial: uartps: do not deassert RS485 RTS GPIO prematurely
After all bytes to be transmitted have been written to the FIFO register, the hardware might still be busy actually sending them.
Thus, wait for the TX FIFO to be empty before starting the timer for the RTS after send delay.
Cc: stable@vger.kernel.org Fixes: fccc9d9233f9 ("tty: serial: uartps: Add rs485 support to uartps driver") Signed-off-by: Martin Kaistra martin.kaistra@linutronix.de --- Changes in v2: - Add cc stable - Add timeout
drivers/tty/serial/xilinx_uartps.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index fe457bf1e15bb..e1dd3843d563c 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -429,7 +429,7 @@ static void cdns_uart_handle_tx(void *dev_id) struct uart_port *port = (struct uart_port *)dev_id; struct cdns_uart *cdns_uart = port->private_data; struct tty_port *tport = &port->state->port; - unsigned int numbytes; + unsigned int numbytes, tmout; unsigned char ch; if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port)) { @@ -454,6 +454,13 @@ static void cdns_uart_handle_tx(void *dev_id) if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED && (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port))) { + /* Wait for the tx fifo to be actually empty */ + for (tmout = 1000000; tmout; tmout--) { + if (cdns_uart_tx_empty(port) == TIOCSER_TEMT) + break; + udelay(1); + } + hrtimer_update_function(&cdns_uart->tx_timer, cdns_rs485_rx_callback); hrtimer_start(&cdns_uart->tx_timer, ns_to_ktime(cdns_calc_after_tx_delay(cdns_uart)), HRTIMER_MODE_REL);
Am 25.08.25 um 13:58 schrieb Maarten Brock:
Hello Martin,
Hi Maarten,
Why not just start the timer and check TEMT after it has elapsed and restart the timer if not empty? It would prevent busy-loop waiting.
It would, yes, but couldn't this cause the time between last transmitted byte and switching the RTS GPIO to be less than the specified RTS delay?
Martin
Kind regards, Maarten Brock
From: Martin Kaistra martin.kaistra@linutronix.de Sent: Monday, August 25, 2025 11:22 To: Michal Simek michal.simek@amd.com; Greg Kroah-Hartman gregkh@linuxfoundation.org; linux-serial@vger.kernel.org linux-serial@vger.kernel.org Cc: Manikanta Guntupalli manikanta.guntupalli@amd.com; stable@vger.kernel.org stable@vger.kernel.org Subject: [PATCH v2] serial: uartps: do not deassert RS485 RTS GPIO prematurely
After all bytes to be transmitted have been written to the FIFO register, the hardware might still be busy actually sending them.
Thus, wait for the TX FIFO to be empty before starting the timer for the RTS after send delay.
Cc: stable@vger.kernel.org Fixes: fccc9d9233f9 ("tty: serial: uartps: Add rs485 support to uartps driver") Signed-off-by: Martin Kaistra martin.kaistra@linutronix.de
Changes in v2:
- Add cc stable
- Add timeout
drivers/tty/serial/xilinx_uartps.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index fe457bf1e15bb..e1dd3843d563c 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -429,7 +429,7 @@ static void cdns_uart_handle_tx(void *dev_id) struct uart_port *port = (struct uart_port *)dev_id; struct cdns_uart *cdns_uart = port->private_data; struct tty_port *tport = &port->state->port; - unsigned int numbytes; + unsigned int numbytes, tmout; unsigned char ch; if (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port)) { @@ -454,6 +454,13 @@ static void cdns_uart_handle_tx(void *dev_id) if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED && (kfifo_is_empty(&tport->xmit_fifo) || uart_tx_stopped(port))) { + /* Wait for the tx fifo to be actually empty */ + for (tmout = 1000000; tmout; tmout--) { + if (cdns_uart_tx_empty(port) == TIOCSER_TEMT) + break; + udelay(1); + }
hrtimer_update_function(&cdns_uart->tx_timer, cdns_rs485_rx_callback); hrtimer_start(&cdns_uart->tx_timer, ns_to_ktime(cdns_calc_after_tx_delay(cdns_uart)), HRTIMER_MODE_REL);
From: Martin Kaistra martin.kaistra@linutronix.de
Hello Martin,
Am 25.08.25 um 13:58 schrieb Maarten Brock:
Hello Martin,
Hi Maarten,
Why not just start the timer and check TEMT after it has elapsed and restart the timer if not empty? It would prevent busy-loop waiting.
It would, yes, but couldn't this cause the time between last transmitted byte and switching the RTS GPIO to be less than the specified RTS delay?
Maybe you can calculate the nominal duration of the transmission and add that to the delay? But yes, that could still result in a short delay.
You stated that the TEMT interrupt is not useable. Why not?
But it does trigger the question what the RTS delay is used for? - Is it to overcome the transmission of byte(s) still in the UART? - Is it to overcome the transmission of the stop bit(s)? - Can anyone imagine anything else that requires a delay? - Is the given delay a minimum, typical or maximum value?
Maarten
Martin
Kind regards, Maarten Brock
Am 25.08.25 um 15:54 schrieb Maarten Brock:
From: Martin Kaistra martin.kaistra@linutronix.de
Hello Martin,
Am 25.08.25 um 13:58 schrieb Maarten Brock:
Hello Martin,
Hi Maarten,
Why not just start the timer and check TEMT after it has elapsed and restart the timer if not empty? It would prevent busy-loop waiting.
It would, yes, but couldn't this cause the time between last transmitted byte and switching the RTS GPIO to be less than the specified RTS delay?
Maybe you can calculate the nominal duration of the transmission and add that to the delay? But yes, that could still result in a short delay.
You stated that the TEMT interrupt is not useable. Why not?
I see in my traces that it sometimes does not fire (even though it should be enabled and more than enough time has passed since the last byte).
But it does trigger the question what the RTS delay is used for?
- Is it to overcome the transmission of byte(s) still in the UART?
- Is it to overcome the transmission of the stop bit(s)?
- Can anyone imagine anything else that requires a delay?
- Is the given delay a minimum, typical or maximum value?
Maarten
Martin
Kind regards, Maarten Brock
linux-stable-mirror@lists.linaro.org