[PATCH] serial: amba-pl011: keep transmitter enabled when disabling RS485

[PATCH] serial: amba-pl011: keep transmitter enabled when disabling RS485

[Nicolai Buchwitz]

Disabling RS485 on an open port calls pl011_rs485_tx_stop(), which
turns off the transmitter. Only the RS485 TX path turns it back on, so
any later output stays queued and tcdrain() hangs until the port is
closed.

pl011_rs485_tx_stop() also checks only the hardware FIFO. If a DMA
transfer has started but has not filled the FIFO yet, the transmitter
is turned off under the transfer and the DMA channel never recovers.

When leaving RS485 mode, cancel any pending deferred stop and leave the
transmitter enabled.

Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support")
Cc: stable@vger.kernel.org
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
Noticed while implementing a DMX master on a CM5 with its PL011.
Userspace turns RS485 mode off to send the break, and the port then
stopped transmitting.

Tested on that board with an RS485 transceiver at 9600, 115200 and
250000 baud.
---
 drivers/tty/serial/amba-pl011.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index c4824c201e1c..d53dd934cc74 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2397,8 +2397,24 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 	struct uart_amba_port *uap =
 		container_of(port, struct uart_amba_port, port);
 
-	if (port->rs485.flags & SER_RS485_ENABLED)
+	if ((port->rs485.flags & SER_RS485_ENABLED) &&
+	    !(rs485->flags & SER_RS485_ENABLED)) {
+		u32 cr = pl011_read(uap, REG_CR);
+
+		/*
+		 * Leaving RS485 mode, which only enables the transmitter while
+		 * sending. Don't stop it, as that stalls a DMA transfer still
+		 * in flight. Drop a pending deferred stop and keep it enabled.
+		 */
+		hrtimer_try_to_cancel(&uap->trigger_start_tx);
+		hrtimer_try_to_cancel(&uap->trigger_stop_tx);
+		uap->rs485_tx_state = OFF;
+
+		cr |= UART011_CR_TXE | UART011_CR_RXE;
+		pl011_write(cr, uap, REG_CR);
+	} else if (port->rs485.flags & SER_RS485_ENABLED) {
 		pl011_rs485_tx_stop(uap);
+	}
 
 	/* Make sure auto RTS is disabled */
 	if (rs485->flags & SER_RS485_ENABLED) {

---
base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
change-id: 20261011-fix-pl011-rs485-txe-f61e1c969f9a

Best regards,
--  
Nicolai Buchwitz <nb@tipi-net.de>

Re: [PATCH] serial: amba-pl011: keep transmitter enabled when disabling RS485

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tty: serial: amba-pl011: stalled transmission when disabling RS485 during deferred start
--

commit fd97eed6e0ab8bd63d1bb5300339347d34c58cdc
Author: Nicolai Buchwitz <nb@tipi-net.de>

serial: amba-pl011: keep transmitter enabled when disabling RS485

This commit addresses an issue where disabling RS485 mode on an open port
turns off the transmitter, causing pending output and active DMA transfers
to stall. It updates pl011_rs485_config() to leave the transmitter enabled
and cancel pending timers when transitioning out of RS485 mode.

> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2397,8 +2397,24 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
>  	struct uart_amba_port *uap =
>  		container_of(port, struct uart_amba_port, port);
>  
> -	if (port->rs485.flags & SER_RS485_ENABLED)
> +	if ((port->rs485.flags & SER_RS485_ENABLED) &&
> +	    !(rs485->flags & SER_RS485_ENABLED)) {
> +		u32 cr = pl011_read(uap, REG_CR);
> +
> +		/*
> +		 * Leaving RS485 mode, which only enables the transmitter while
> +		 * sending. Don't stop it, as that stalls a DMA transfer still
> +		 * in flight. Drop a pending deferred stop and keep it enabled.
> +		 */
> +		hrtimer_try_to_cancel(&uap->trigger_start_tx);

[Severity: Medium]
Does canceling trigger_start_tx here without manually starting the
transmission risk permanently stranding queued characters?

If a userspace application writes data with a non-zero delay_rts_before_send
in RS485 mode, transmission is deferred via this timer.

If the application then immediately disables RS485 mode via ioctl(TIOCSRS485)
before the timer fires, this code cancels the start timer.

Since there is no fallback to manually kick the stalled transmission (such as
calling pl011_dma_tx_start() or pl011_start_tx_pio()), the TX interrupt
remains masked and the DMA is never started. This could cause subsequent
calls to tcdrain() or close() to hang indefinitely.

> +		hrtimer_try_to_cancel(&uap->trigger_stop_tx);
> +		uap->rs485_tx_state = OFF;
> +
> +		cr |= UART011_CR_TXE | UART011_CR_RXE;
> +		pl011_write(cr, uap, REG_CR);
> +	} else if (port->rs485.flags & SER_RS485_ENABLED) {
>  		pl011_rs485_tx_stop(uap);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-fix-pl011-rs485-txe-v1-1-302b6c034b7f@tipi-net.de?part=1