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

[PATCH v4 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.

Tested on a Raspberry Pi CM4 (PL011 on BCM2711) and a CM5 (PL011 on
RP1, behind PCIe on BCM2712).

Deliberately left for follow-ups:
- pl011_rs485_config() sees the old settings, which are only stored
  after it returns. Will be fixed in the RS485 rework.
- A TX flush during DMA leaves the bus driven. Will be fixed by a
  separate patch sent alongside.

---
Changes in v4:
- Patch 1: detect a running transmission from PIO or DMA activity, not
  from queued data (sashiko)
- Link to v3: https://patch.msgid.link/20261019-devel-nbuchwitz-pl011-rs485-reconfig-stall-v3-0-072a9c82ea5e@tipi-net.de

Changes in v3:
- Patch 1: adopt an active break when resuming (sashiko)
- Link to v2: https://patch.msgid.link/20261017-devel-nbuchwitz-pl011-rs485-reconfig-stall-v2-0-6f457a1ee527@tipi-net.de

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 | 64 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 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 v4 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. While PIO or DMA still feeds the FIFO, assert RTS and mark it as
sending, so that the TX path ends it like any other. With only the FIFO
left, drain it from trigger_stop_tx instead. Data that is queued but not
started yet, for example while output is stopped, is not part of the
transmission.

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 # 6.18.x
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/tty/serial/amba-pl011.c | 57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 79776a882ae35..1edbe75611e63 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1285,6 +1285,11 @@ static inline bool pl011_dma_tx_queued(struct uart_amba_port *uap)
 	return uap->dmatx.queued;
 }
 
+static inline bool pl011_dma_tx_active(struct uart_amba_port *uap)
+{
+	return uap->dmacr & UART011_TXDMAE;
+}
+
 #else
 /* Blank functions if the DMA engine is not available */
 static inline void pl011_dma_remove(struct uart_amba_port *uap)
@@ -1341,6 +1346,11 @@ static inline bool pl011_dma_tx_queued(struct uart_amba_port *uap)
 	return false;
 }
 
+static inline bool pl011_dma_tx_active(struct uart_amba_port *uap)
+{
+	return false;
+}
+
 #define pl011_dma_flush_buffer	NULL
 #endif
 
@@ -1734,6 +1744,12 @@ static unsigned int pl011_tx_empty(struct uart_port *port)
 							0 : TIOCSER_TEMT;
 }
 
+/* PIO or DMA still feeds the FIFO */
+static bool pl011_tx_active(struct uart_amba_port *uap)
+{
+	return (uap->im & UART011_TXIM) || pl011_dma_tx_active(uap);
+}
+
 static void pl011_maybe_set_bit(bool cond, unsigned int *ptr, unsigned int mask)
 {
 	if (cond)
@@ -2454,6 +2470,42 @@ 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)
+{
+	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);
+
+	/* pl011_break_ctl() ends the transmission when the break ends */
+	if (pl011_read(uap, REG_LCRH_TX) & UART01x_LCRH_BRK) {
+		uap->rs485_break = true;
+		uap->rs485_tx_state = SEND;
+		return;
+	}
+
+	if (pl011_tx_active(uap)) {
+		uap->rs485_tx_state = SEND;
+		return;
+	}
+
+	/* Only the FIFO is left. The timer drains it with the new settings */
+	uap->rs485_tx_state = WAIT_AFTER_SEND;
+	hrtimer_start(&uap->trigger_stop_tx, uap->rs485_tx_drain_interval,
+		      HRTIMER_MODE_REL);
+}
+
 static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 			      struct serial_rs485 *rs485)
 {
@@ -2483,6 +2535,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_active(uap) || !pl011_tx_empty(port))) {
+		pl011_rs485_tx_resume(uap, rs485);
 	} else if (port->rs485.flags & SER_RS485_ENABLED) {
 		pl011_rs485_tx_stop(uap);
 	}

-- 
2.53.0

Re: [PATCH v4 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. While PIO or DMA still feeds the FIFO, assert RTS and mark it as
> sending, so that the TX path ends it like any other. With only the FIFO
> left, drain it from trigger_stop_tx instead. Data that is queued but not
> started yet, for example while output is stopped, is not part of the
> transmission.
> 
> 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 # 6.18.x
> 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/20261021-devel-nbuchwitz-pl011-rs485-reconfig-stall-v4-0-ed10e219159a@tipi-net.de?part=1

[PATCH v4 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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 1edbe75611e63..e4ac9225cdcba 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2512,12 +2512,13 @@ 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);
 
-	uap->rs485_break = false;
 	if ((port->rs485.flags & SER_RS485_ENABLED) &&
 	    !(rs485->flags & SER_RS485_ENABLED)) {
 		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
@@ -2540,7 +2541,9 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
 		   tty_port_initialized(&port->state->port) &&
 		   (pl011_tx_active(uap) || !pl011_tx_empty(port))) {
 		pl011_rs485_tx_resume(uap, rs485);
-	} 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 v4 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] serial: amba-pl011: stale port->rs485 used in pl011_rs485_config()
--

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

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

This commit updates pl011_rs485_config() so that reconfiguring RS485 while it
remains enabled only calls pl011_rs485_tx_stop() when no transmission is in
progress (uap->rs485_tx_state == OFF), and only clears uap->rs485_break when
leaving RS485 mode.

> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 1edbe75611e63..e4ac9225cdcba 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c

[ ... ]

> @@ -2540,7 +2541,9 @@ static int pl011_rs485_config(struct uart_port *port, struct ktermios *termios,
>  		   tty_port_initialized(&port->state->port) &&
>  		   (pl011_tx_active(uap) || !pl011_tx_empty(port))) {
>  		pl011_rs485_tx_resume(uap, rs485);
> -	} 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 a TIOCSRS485 ioctl, uart_set_rs485_config() invokes
port->rs485_config() before assigning the new configuration to port->rs485:

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

In drivers/tty/serial/amba-pl011.c, pl011_rs485_config() calls
pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now(), which reads
port->rs485.flags rather than the new 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;
	...
}

Doesn't this cause three regressions when configuring RS485 via TIOCSRS485?

First, when RS485 is already enabled and idle (uap->rs485_tx_state == OFF)
and userspace changes 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.

Second, because the else-if condition in pl011_rs485_config() checks
port->rs485.flags & SER_RS485_ENABLED instead of
rs485->flags & SER_RS485_ENABLED, newly enabling RS485 on an open, idle port
(where pl011_tx_active(uap) is false and pl011_tx_empty(port) is true) skips
both pl011_rs485_tx_resume() and pl011_rs485_tx_stop(), leaving
UART011_CR_TXE enabled and UART011_CR_RTS at its previous non-RS485 level.

Third, when RS485 is reconfigured while a transmission or break is active
(uap->rs485_tx_state != OFF), all branches in pl011_rs485_config() are
skipped, so changes to SER_RS485_RTS_ON_SEND or SER_RS485_RX_DURING_TX in
rs485->flags are not applied to REG_CR for the ongoing transmission.

Should pl011_rs485_config() check rs485->flags & SER_RS485_ENABLED and apply
the new rs485->flags when setting the idle or active REG_CR state?

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

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

[Nicolai Buchwitz]

On 21.10.2026 14:12, sashiko-bot@kernel.org wrote:

> [...]

> [Severity: Medium]
> During a TIOCSRS485 ioctl, uart_set_rs485_config() invokes
> port->rs485_config() before assigning the new configuration to 
> port->rs485:
> 
> drivers/tty/serial/serial_core.c:uart_set_rs485_config() {
> 	...
> 	scoped_guard(uart_port_lock_irqsave, port) {
> 		ret = port->rs485_config(port, &tty->termios, &rs485);
> 		if (!ret) {
> 			port->rs485 = rs485;
> 	...
> }
> 
> In drivers/tty/serial/amba-pl011.c, pl011_rs485_config() calls
> pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now(), which reads
> port->rs485.flags rather than the new 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;
> 	...
> }
> 
> Doesn't this cause three regressions when configuring RS485 via 
> TIOCSRS485?
> 
> First, when RS485 is already enabled and idle (uap->rs485_tx_state == 
> OFF)
> and userspace changes 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.
> 
> Second, because the else-if condition in pl011_rs485_config() checks
> port->rs485.flags & SER_RS485_ENABLED instead of
> rs485->flags & SER_RS485_ENABLED, newly enabling RS485 on an open, idle 
> port
> (where pl011_tx_active(uap) is false and pl011_tx_empty(port) is true) 
> skips
> both pl011_rs485_tx_resume() and pl011_rs485_tx_stop(), leaving
> UART011_CR_TXE enabled and UART011_CR_RTS at its previous non-RS485 
> level.
> 
> Third, when RS485 is reconfigured while a transmission or break is 
> active
> (uap->rs485_tx_state != OFF), all branches in pl011_rs485_config() are
> skipped, so changes to SER_RS485_RTS_ON_SEND or SER_RS485_RX_DURING_TX 
> in
> rs485->flags are not applied to REG_CR for the ongoing transmission.
> 
> Should pl011_rs485_config() check rs485->flags & SER_RS485_ENABLED and 
> apply
> the new rs485->flags when setting the idle or active REG_CR state?

All three predate this series, the branch read port->rs485 the same way 
before.
The cover letter lists them as left for the RS485 rework, which fixes 
them by
applying the new settings in pl011_rs485_config().

IMHO that's too big for these stable fixes.

Thanks,
Nicolai