[PATCH v2 0/2] serial: amba-pl011: fix RS485 state on enable and reconfiguration

[PATCH v2 0/2] serial: amba-pl011: fix RS485 state on enable and reconfiguration

[Nicolai Buchwitz]

More RS485 fixes for pl011, on top of the ones already in tty-next.
Together they are the groundwork for a series I plan to send next
cycle, which refactors the RS485 code for better performance and
reliability.

Both patches fix changing the RS485 settings while data is going out.
Patch 1 keeps the transceiver from driving the bus after the data is
out when RS485 is enabled mid-transmission. Patch 2 keeps a
reconfiguration from stalling the port.

---
Changes in v2:
- Patch 1: only resume on an open port (sashiko)
- Patch 1: handle stopped output and pause DMA first
- Link to v1: https://patch.msgid.link/20261016-devel-nbuchwitz-pl011-rs485-reconfig-stall-v1-0-f07d4da714be@tipi-net.de

To: Russell King <linux@armlinux.org.uk>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Slaby <jirislaby@kernel.org>
To: Nicolai Buchwitz <nb@tipi-net.de>
To: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Cc: linux-kernel@vger.kernel.org
Cc: linux-serial@vger.kernel.org

---
Nicolai Buchwitz (2):
      serial: amba-pl011: finish a transmission still running when RS485 is enabled
      serial: amba-pl011: keep RS485 transmission running on reconfiguration

 drivers/tty/serial/amba-pl011.c | 66 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)
---
base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
change-id: 20261016-devel-nbuchwitz-pl011-rs485-reconfig-stall-f4fe437cccae

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

[PATCH v2 1/2] serial: amba-pl011: finish a transmission still running when RS485 is enabled

[Nicolai Buchwitz]

Disabling RS485 during a transmission keeps it running. If RS485 is
enabled again before it ends, the state is OFF while data is still going
out. The TX path does not end a transmission in state OFF, so nothing
sets the idle RTS level afterwards and the transceiver keeps driving the
bus.

Hand such a transmission back to RS485 when it is enabled on an open
port. Assert RTS and mark it as sending, so that the TX path ends it
like any other. With only the FIFO left or output stopped, the TX path
won't run again, so drain the FIFO from trigger_stop_tx instead. Data
queued while output is stopped is not part of the transmission. Pause
DMA first, as output is marked stopped before the driver pauses it.

Fixes: 97f85204275a ("serial: amba-pl011: keep transmitter enabled when disabling RS485")
Cc: stable@vger.kernel.org # 6.18.x: 1b6d660e038b: serial: amba-pl011: drive the bus during break in RS485 mode
Cc: stable@vger.kernel.org
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/tty/serial/amba-pl011.c | 60 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 79776a882ae35..3b9cce435c650 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1734,6 +1734,18 @@ static unsigned int pl011_tx_empty(struct uart_port *port)
 							0 : TIOCSER_TEMT;
 }
 
+/* While output is stopped, only data already in the FIFO counts */
+static bool pl011_tx_pending(struct uart_amba_port *uap, bool stopped)
+{
+	struct tty_port *tport = &uap->port.state->port;
+
+	if (!pl011_tx_empty(&uap->port))
+		return true;
+
+	return !stopped &&
+	       (!kfifo_is_empty(&tport->xmit_fifo) || pl011_dma_tx_queued(uap));
+}
+
 static void pl011_maybe_set_bit(bool cond, unsigned int *ptr, unsigned int mask)
 {
 	if (cond)
@@ -2454,13 +2466,56 @@ static int pl011_verify_port(struct uart_port *port, struct serial_struct *ser)
 	return ret;
 }
 
+/*
+ * Data from before RS485 was enabled is still going out. Finish it as an
+ * RS485 transmission, so that its end sets the idle level.
+ */
+static void pl011_rs485_tx_resume(struct uart_amba_port *uap,
+				  const struct serial_rs485 *rs485, bool stopped)
+{
+	struct tty_port *tport = &uap->port.state->port;
+	u32 cr = pl011_read(uap, REG_CR);
+
+	cr |= UART011_CR_TXE;
+	if (!(rs485->flags & SER_RS485_RX_DURING_TX))
+		cr &= ~UART011_CR_RXE;
+	if (rs485->flags & SER_RS485_RTS_ON_SEND)
+		cr &= ~UART011_CR_RTS;
+	else
+		cr |= UART011_CR_RTS;
+	pl011_write(cr, uap, REG_CR);
+
+	/*
+	 * With only the FIFO left or output stopped, the TX path won't end it.
+	 * The timer does, once the new settings are stored.
+	 */
+	if (stopped ||
+	    (kfifo_is_empty(&tport->xmit_fifo) && !pl011_dma_tx_queued(uap))) {
+		uap->rs485_tx_state = WAIT_AFTER_SEND;
+		hrtimer_start(&uap->trigger_stop_tx, uap->rs485_tx_drain_interval,
+			      HRTIMER_MODE_REL);
+	} else {
+		uap->rs485_tx_state = SEND;
+	}
+}
+
 static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 			      struct serial_rs485 *rs485)
 {
 	struct uart_amba_port *uap =
 		container_of(port, struct uart_amba_port, port);
+	bool tx_stopped = uart_tx_stopped(port);
 
 	uap->rs485_break = false;
+
+	/*
+	 * Output is marked stopped before stop_tx pauses DMA. Pause it now, so
+	 * that clearing TXE below cannot strand DMA data in the FIFO. Output
+	 * stopped after this check is ended by stop_tx.
+	 */
+	if ((rs485->flags & SER_RS485_ENABLED) && tx_stopped)
+		pl011_dma_tx_stop(uap);
+
 	if ((port->rs485.flags & SER_RS485_ENABLED) &&
 	    !(rs485->flags & SER_RS485_ENABLED)) {
 		bool tx_pending = uap->rs485_tx_state == WAIT_AFTER_RTS;
@@ -2483,6 +2538,11 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 		if (tx_pending && !uart_tx_stopped(port) &&
 		    !pl011_dma_tx_start(uap))
 			pl011_start_tx_pio(uap);
+	} else if ((rs485->flags & SER_RS485_ENABLED) &&
+		   uap->rs485_tx_state == OFF &&
+		   tty_port_initialized(&port->state->port) &&
+		   pl011_tx_pending(uap, tx_stopped)) {
+		pl011_rs485_tx_resume(uap, rs485, tx_stopped);
 	} else if (port->rs485.flags & SER_RS485_ENABLED) {
 		pl011_rs485_tx_stop(uap);
 	}

-- 
2.53.0

Re: [PATCH v2 1/2] serial: amba-pl011: finish a transmission still running when RS485 is enabled

[sashiko-bot]

> Disabling RS485 during a transmission keeps it running. If RS485 is
> enabled again before it ends, the state is OFF while data is still going
> out. The TX path does not end a transmission in state OFF, so nothing
> sets the idle RTS level afterwards and the transceiver keeps driving the
> bus.
> 
> Hand such a transmission back to RS485 when it is enabled on an open
> port. Assert RTS and mark it as sending, so that the TX path ends it
> like any other. With only the FIFO left or output stopped, the TX path
> won't run again, so drain the FIFO from trigger_stop_tx instead. Data
> queued while output is stopped is not part of the transmission. Pause
> DMA first, as output is marked stopped before the driver pauses it.
> 
> Fixes: 97f85204275a ("serial: amba-pl011: keep transmitter enabled when disabling RS485")
> Cc: stable@vger.kernel.org # 6.18.x: 1b6d660e038b: serial: amba-pl011: drive the bus during break in RS485 mode
> Cc: stable@vger.kernel.org
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-devel-nbuchwitz-pl011-rs485-reconfig-stall-v2-0-6f457a1ee527@tipi-net.de?part=1

[PATCH v2 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration

[Nicolai Buchwitz]

Changing the RS485 settings while RS485 stays enabled calls
pl011_rs485_tx_stop(), also while a transmission is in progress. Unlike
pl011_stop_tx() it leaves the TX interrupt enabled, so the remaining
data is still pushed into the FIFO after the drain turned off the
transmitter. The FIFO never empties, the port stalls, and closing it
hangs in pl011_dma_shutdown() waiting for the transmitter to go idle.
A start still waiting for delay_rts_before_send is dropped as well.

Only stop the transmitter when no transmission is in progress. The
call is still needed there, it sets the idle RTS level when the port
is configured or resumed. A transmission in progress sets that level
when it ends.

An active break counts as a transmission too. Forget it only when
leaving RS485 mode, so that pl011_break_ctl() still ends the
transmission when the break ends.

Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support")
Cc: stable@vger.kernel.org # 6.18.x
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/tty/serial/amba-pl011.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3b9cce435c650..036a7245f1f29 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2506,8 +2506,6 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 		container_of(port, struct uart_amba_port, port);
 	bool tx_stopped = uart_tx_stopped(port);
 
-	uap->rs485_break = false;
-
 	/*
 	 * Output is marked stopped before stop_tx pauses DMA. Pause it now, so
 	 * that clearing TXE below cannot strand DMA data in the FIFO. Output
@@ -2521,6 +2519,8 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 		bool tx_pending = uap->rs485_tx_state == WAIT_AFTER_RTS;
 		u32 cr = pl011_read(uap, REG_CR);
 
+		uap->rs485_break = false;
+
 		/*
 		 * Leaving RS485 mode, which only enables the transmitter while
 		 * sending. Don't stop it, as that stalls a DMA transfer still
@@ -2543,7 +2543,9 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 		   tty_port_initialized(&port->state->port) &&
 		   pl011_tx_pending(uap, tx_stopped)) {
 		pl011_rs485_tx_resume(uap, rs485, tx_stopped);
-	} else if (port->rs485.flags & SER_RS485_ENABLED) {
+	} else if ((port->rs485.flags & SER_RS485_ENABLED) &&
+		   uap->rs485_tx_state == OFF) {
+		/* Set the idle level now, a transmission sets it when done */
 		pl011_rs485_tx_stop(uap);
 	}
 

-- 
2.53.0

Re: [PATCH v2 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] serial: amba-pl011: use stale port->rs485 in pl011_rs485_config() when idle
- [Medium] serial: amba-pl011: missing break state adoption in pl011_rs485_tx_resume()
--

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

serial: amba-pl011: keep RS485 transmission running on reconfiguration

This commit prevents pl011_rs485_config() from stopping an ongoing RS485
transmission when RS485 settings are updated, and preserves uap->rs485_break
unless RS485 mode is being disabled.

> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 3b9cce435c650..036a7245f1f29 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2506,8 +2506,6 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
>  		container_of(port, struct uart_amba_port, port);
>  	bool tx_stopped = uart_tx_stopped(port);
>  
> -	uap->rs485_break = false;
> -
>  	/*
>  	 * Output is marked stopped before stop_tx pauses DMA. Pause it now, so
>  	 * that clearing TXE below cannot strand DMA data in the FIFO. Output
> @@ -2521,6 +2519,8 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
>  		bool tx_pending = uap->rs485_tx_state == WAIT_AFTER_RTS;
>  		u32 cr = pl011_read(uap, REG_CR);
>  
> +		uap->rs485_break = false;
> +
>  		/*
>  		 * Leaving RS485 mode, which only enables the transmitter while
>  		 * sending. Don't stop it, as that stalls a DMA transfer still
> @@ -2543,7 +2543,9 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
>  		   tty_port_initialized(&port->state->port) &&
>  		   pl011_tx_pending(uap, tx_stopped)) {
>  		pl011_rs485_tx_resume(uap, rs485, tx_stopped);

[Severity: Medium]
What happens if RS485 is enabled via TIOCSRS485 while a break is active on
the port (or if RS485 is disabled during a break, clearing uap->rs485_break
above while UART01x_LCRH_BRK remains set in REG_LCRH_TX, and then re-enabled)?

While UART01x_LCRH_BRK is set, the PL011 keeps UART01x_FR_BUSY asserted in
REG_FR, so pl011_tx_pending() returns true via !pl011_tx_empty() and enters
pl011_rs485_tx_resume():

drivers/tty/serial/amba-pl011.c:pl011_rs485_tx_resume() {
    ...
	if (stopped ||
	    (kfifo_is_empty(&tport->xmit_fifo) && !pl011_dma_tx_queued(uap))) {
		uap->rs485_tx_state = WAIT_AFTER_SEND;
		hrtimer_start(&uap->trigger_stop_tx, uap->rs485_tx_drain_interval,
			      HRTIMER_MODE_REL);
	} else {
		uap->rs485_tx_state = SEND;
	}
}

Because pl011_rs485_tx_resume() does not check REG_LCRH_TX for
UART01x_LCRH_BRK and set uap->rs485_break = true, uap->rs485_break stays
false. Could this cause pl011_trigger_stop_tx() -> pl011_rs485_tx_stop() to
bypass the uap->rs485_break check and continuously re-arm trigger_stop_tx
every character interval in hardirq context for the entire duration of the
break?

> -	} else if (port->rs485.flags & SER_RS485_ENABLED) {
> +	} else if ((port->rs485.flags & SER_RS485_ENABLED) &&
> +		   uap->rs485_tx_state == OFF) {
> +		/* Set the idle level now, a transmission sets it when done */
>  		pl011_rs485_tx_stop(uap);
>  	}

[Severity: Medium]
During TIOCSRS485, uart_ioctl() -> uart_set_rs485_config() invokes
port->rs485_config() before storing the new configuration into port->rs485:

drivers/tty/serial/serial_core.c:uart_set_rs485_config() {
    ...
		ret = port->rs485_config(port, &tty->termios, &rs485);
		if (!ret) {
			port->rs485 = rs485;
    ...
}

And pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now() also reads
port->rs485.flags:

drivers/tty/serial/amba-pl011.c:pl011_rs485_tx_stop_now() {
    ...
	if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
		cr &= ~UART011_CR_RTS;
	else
		cr |= UART011_CR_RTS;
    ...
}

Because port->rs485 still holds the old settings here, can this cause two
issues when configuring an idle port (uap->rs485_tx_state == OFF and
!pl011_tx_pending()) via TIOCSRS485:

1. When enabling RS485 for the first time, port->rs485.flags & SER_RS485_ENABLED
   is still false, so pl011_rs485_tx_stop() is skipped, leaving UART011_CR_TXE
   enabled while uap->rs485_tx_state == OFF and leaving UART011_CR_RTS at its
   RS232 level?

2. When RS485 is already enabled and TIOCSRS485 changes the RTS polarity
   (SER_RS485_RTS_ON_SEND vs SER_RS485_RTS_AFTER_SEND),
   pl011_rs485_tx_stop_now() programs UART011_CR_RTS using the old
   port->rs485.flags instead of the new rs485->flags, driving the wrong RTS
   idle level?

Should this branch check rs485->flags & SER_RS485_ENABLED and apply the new
rs485->flags when setting the idle state?

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-devel-nbuchwitz-pl011-rs485-reconfig-stall-v2-0-6f457a1ee527@tipi-net.de?part=2

Re: [PATCH v2 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration

[Nicolai Buchwitz]

On 17.10.2026 08:45, sashiko-bot@kernel.org wrote:

> [...]

> [Severity: Medium]
> What happens if RS485 is enabled via TIOCSRS485 while a break is active 
> on
> the port (or if RS485 is disabled during a break, clearing 
> uap->rs485_break
> above while UART01x_LCRH_BRK remains set in REG_LCRH_TX, and then 
> re-enabled)?
> 
> While UART01x_LCRH_BRK is set, the PL011 keeps UART01x_FR_BUSY asserted 
> in
> REG_FR, so pl011_tx_pending() returns true via !pl011_tx_empty() and 
> enters
> pl011_rs485_tx_resume():
> 
> drivers/tty/serial/amba-pl011.c:pl011_rs485_tx_resume() {
>     ...
> 	if (stopped ||
> 	    (kfifo_is_empty(&tport->xmit_fifo) && !pl011_dma_tx_queued(uap))) 
> {
> 		uap->rs485_tx_state = WAIT_AFTER_SEND;
> 		hrtimer_start(&uap->trigger_stop_tx, uap->rs485_tx_drain_interval,
> 			      HRTIMER_MODE_REL);
> 	} else {
> 		uap->rs485_tx_state = SEND;
> 	}
> }
> 
> Because pl011_rs485_tx_resume() does not check REG_LCRH_TX for
> UART01x_LCRH_BRK and set uap->rs485_break = true, uap->rs485_break 
> stays
> false. Could this cause pl011_trigger_stop_tx() -> 
> pl011_rs485_tx_stop() to
> bypass the uap->rs485_break check and continuously re-arm 
> trigger_stop_tx
> every character interval in hardirq context for the entire duration of 
> the
> break?
> 
>> -	} else if (port->rs485.flags & SER_RS485_ENABLED) {
>> +	} else if ((port->rs485.flags & SER_RS485_ENABLED) &&
>> +		   uap->rs485_tx_state == OFF) {
>> +		/* Set the idle level now, a transmission sets it when done */
>>  		pl011_rs485_tx_stop(uap);
>>  	}
> 

Agree. I verified on my CM4 that BUSY stays set during a break and the 
drain timer
keeps polling until the break ends. Will fix this in v3.


> [Severity: Medium]
> During TIOCSRS485, uart_ioctl() -> uart_set_rs485_config() invokes
> port->rs485_config() before storing the new configuration into 
> port->rs485:
> 
> drivers/tty/serial/serial_core.c:uart_set_rs485_config() {
>     ...
> 		ret = port->rs485_config(port, &tty->termios, &rs485);
> 		if (!ret) {
> 			port->rs485 = rs485;
>     ...
> }
> 
> And pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now() also reads
> port->rs485.flags:
> 
> drivers/tty/serial/amba-pl011.c:pl011_rs485_tx_stop_now() {
>     ...
> 	if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> 		cr &= ~UART011_CR_RTS;
> 	else
> 		cr |= UART011_CR_RTS;
>     ...
> }
> 
> Because port->rs485 still holds the old settings here, can this cause 
> two
> issues when configuring an idle port (uap->rs485_tx_state == OFF and
> !pl011_tx_pending()) via TIOCSRS485:
> 
> 1. When enabling RS485 for the first time, port->rs485.flags & 
> SER_RS485_ENABLED
>    is still false, so pl011_rs485_tx_stop() is skipped, leaving 
> UART011_CR_TXE
>    enabled while uap->rs485_tx_state == OFF and leaving UART011_CR_RTS 
> at its
>    RS232 level?
> 
> 2. When RS485 is already enabled and TIOCSRS485 changes the RTS 
> polarity
>    (SER_RS485_RTS_ON_SEND vs SER_RS485_RTS_AFTER_SEND),
>    pl011_rs485_tx_stop_now() programs UART011_CR_RTS using the old
>    port->rs485.flags instead of the new rs485->flags, driving the wrong 
> RTS
>    idle level?
> 
> Should this branch check rs485->flags & SER_RS485_ENABLED and apply the 
> new
> rs485->flags when setting the idle state?

IMHO this is a pre-existing issue. I even have a fix for this in my 
local RS485
rework series, I plan to send for next cycle. So if there aren't any 
objections,
I will skip this for v3.

Thanks,
Nicolai