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

[PATCH v3 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).

---
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 | 73 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 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 v3 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 | 67 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 79776a882ae35..6adfe7a1e0487 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,63 @@ 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);
+
+	/* 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;
+	}
+
+	/*
+	 * 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 +2545,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

[PATCH v3 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 6adfe7a1e0487..14f0e8aa56fd2 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2513,8 +2513,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
@@ -2528,6 +2526,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
@@ -2550,7 +2550,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 v3 2/2] serial: amba-pl011: keep RS485 transmission running on reconfiguration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] serial: amba-pl011: stale port->rs485.flags used in pl011_rs485_config()
- [Medium] serial: amba-pl011: active TX REG_CR not updated in pl011_rs485_config()
- [Medium] serial: amba-pl011: pl011_tx_pending() mistakes queued data for active TX
--

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

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

This commit avoids calling pl011_rs485_tx_stop() in pl011_rs485_config()
when a transmission is already in progress, preventing the transmitter
from being disabled while TX interrupts are still pushing data into the
FIFO. It also 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 6adfe7a1e0487..14f0e8aa56fd2 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c

[ ... ]

> @@ -2550,7 +2550,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);
>  	}

[Severity: High]
Does this branch use stale flags from port->rs485 when called via
uart_set_rs485_config()?

In drivers/tty/serial/serial_core.c, uart_set_rs485_config() invokes
port->rs485_config() before updating port->rs485:

	scoped_guard(uart_port_lock_irqsave, port) {
		ret = port->rs485_config(port, &tty->termios, &rs485);
		if (!ret) {
			port->rs485 = rs485;

Because port->rs485 still holds the old settings inside
pl011_rs485_config(), two regressions can happen when uap->rs485_tx_state
is OFF:

1. When enabling RS485 from RS232 mode via TIOCSRS485 while no
transmission is pending (!pl011_tx_pending(uap, tx_stopped)),
port->rs485.flags does not have SER_RS485_ENABLED set yet. This branch is
skipped, leaving UART011_CR_TXE enabled from RS232 mode (which can also
strand paused DMA data if tx_stopped is true) and leaving UART011_CR_RTS
at its RS232 state rather than the RS485 idle level.

2. When RS485 is already enabled and TIOCSRS485 changes the RTS polarity
(SER_RS485_RTS_AFTER_SEND) while idle, pl011_rs485_tx_stop() calls
pl011_rs485_tx_stop_now(), which reads the old port->rs485.flags instead
of rs485->flags:

	if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
		cr &= ~UART011_CR_RTS;
	else
		cr |= UART011_CR_RTS;

Should this condition check rs485->flags & SER_RS485_ENABLED, and pass the
new rs485 configuration down when setting the idle state?

[Severity: Medium]
What happens to REG_CR if SER_RS485_RTS_ON_SEND or SER_RS485_RX_DURING_TX
is reconfigured via TIOCSRS485 while a transmission is active
(uap->rs485_tx_state != OFF)?

When RS485 remains enabled and uap->rs485_tx_state is WAIT_AFTER_RTS,
SEND, WAIT_AFTER_SEND, or WAIT_AFTER_SEND_DELAY, pl011_rs485_config() now
skips all branches in this if/else-if chain.

However, pl011_rs485_tx_start() has already programmed UART011_CR_RTS and
UART011_CR_RXE in REG_CR using the old flags:
- In WAIT_AFTER_RTS, when trigger_start_tx expires,
  pl011_trigger_start_tx() sets uap->rs485_tx_state = SEND and calls
  pl011_start_tx() without re-running pl011_rs485_tx_start().
- In WAIT_AFTER_SEND or WAIT_AFTER_SEND_DELAY, if new TX data arrives,
  pl011_rs485_tx_start() cancels trigger_stop_tx, sets
  uap->rs485_tx_state = SEND, and returns early without updating REG_CR.

Won't this leave UART011_CR_RTS and UART011_CR_RXE at the old settings in
REG_CR for the rest of the active or pending transmission?

[Severity: Medium]
Can pl011_tx_pending() mistake queued data for an already active
transmission when called from pl011_rs485_config()?

In drivers/tty/serial/amba-pl011.c, pl011_tx_pending() checks:

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));
}

If output was stopped (tty->flow.stopped is true) while uart_write()
queued data into tport->xmit_fifo (or pl011_stop_tx() paused a queued DMA
transfer), uap->rs485_tx_state is OFF and neither PIO (UART011_TXIM) nor
DMA (UART011_TXDMAE) is active in hardware.

When output is restarted in drivers/tty/tty_io.c:__start_tty():

	if (!tty->flow.stopped || tty->flow.tco_stopped)
		return;
	tty->flow.stopped = false;
	if (tty->ops->start)
		tty->ops->start(tty);

tty->flow.stopped is cleared under tty->flow.lock before uart_start()
acquires port->lock. If TIOCSRS485 runs pl011_rs485_config() under
port->lock during this window (or while __uart_start() has deferred
start_tx() to runtime PM resume, or between uart_put_char() and
uart_flush_chars()), tx_stopped is false and pl011_tx_pending() returns
true even though no transmission has started.

pl011_rs485_config() then calls pl011_rs485_tx_resume(), which asserts
RTS, enables UART011_CR_TXE, and sets uap->rs485_tx_state = SEND directly.
When uart_start() subsequently acquires port->lock and calls
pl011_start_tx(), won't it see uap->rs485_tx_state == SEND and transmit
immediately without waiting for delay_rts_before_send?

Should pl011_tx_pending() check whether TX is actually active in the
driver (such as uap->im & UART011_TXIM or uap->dmacr & UART011_TXDMAE)
rather than checking whether xmit_fifo or DMA has queued data while
!stopped?

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] serial: amba-pl011: pl011_tx_pending() treats unstarted TX as active
- [High] serial: amba-pl011: old port->rs485 used in pl011_rs485_config() idle path
--

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

serial: amba-pl011: finish a transmission still running when RS485 is enabled

When RS485 is enabled on an open port while a transmission from before RS485
was enabled is still running, this commit hands the transmission back to the
RS485 state machine so that the idle RTS level is set when it finishes.

> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 79776a882ae35..6adfe7a1e0487 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,63 @@ 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);
> +
> +	/* 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;
> +	}
> +
> +	/*
> +	 * 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;
> +	}
> +}

[Severity: High]
Can pl011_tx_pending() and pl011_rs485_tx_resume() treat an unstarted
transmission as already active in hardware when pl011_tx_empty(&uap->port) is
true?

When !stopped && (!kfifo_is_empty(&tport->xmit_fifo) ||
pl011_dma_tx_queued(uap)), pl011_tx_pending() returns true and
pl011_rs485_tx_resume() asserts RTS, enables UART011_CR_TXE, and sets
uap->rs485_tx_state = SEND without starting PIO/DMA or arming trigger_stop_tx.
This assumes that either PIO (uap->im & UART011_TXIM) or DMA (uap->dmacr &
UART011_TXDMAE) is already active and will later invoke pl011_stop_tx().

However, there appear to be two paths where xmit_fifo is non-empty (or
dmatx.queued is true) and tx_stopped is false while neither UART011_TXIM nor
UART011_TXDMAE is active and the hardware FIFO is empty:

1. Asynchronous runtime PM resume on port->port_dev->dev:
amba_probe() enables runtime PM on port->dev, and serial_port_probe() enables
runtime PM autosuspend on port->port_dev->dev. When port_dev->dev is
runtime-suspended, uart_write() queues data in tport->xmit_fifo and calls
__uart_start(). Because pm_runtime_get(&port_dev->dev) is asynchronous,
__uart_start() returns without calling port->ops->start_tx(port), deferring
pl011_start_tx() to serial_port_runtime_resume().

If ioctl(TIOCSRS485) runs before serial_port_runtime_resume() acquires
port->lock, pl011_rs485_tx_resume() sets uap->rs485_tx_state = SEND and
asserts RTS. When serial_port_runtime_resume() then calls pl011_start_tx(),
it sees uap->rs485_tx_state == SEND and transmits immediately, bypassing
delay_rts_before_send. Furthermore, if tcflush(fd, TCOFLUSH) (via
uart_flush_buffer()) clears xmit_fifo before serial_port_runtime_resume()
runs, serial_port_runtime_resume() sees __serial_port_busy(port) == false and
skips start_tx(), leaving uap->rs485_tx_state stuck in SEND with RTS asserted
and driving the RS485 bus indefinitely.

2. Concurrent __start_tty() (via TCXONC/TCOON or IXON START_CHAR):
n_tty_ioctl_helper() and n_tty_lookahead_flow_ctrl() call __start_tty()
without holding termios_rwsem or port->mutex. __start_tty() clears
tty->flow.stopped under tty->flow.lock before uart_start() acquires
port->lock to call pl011_start_tx(). If data was queued in xmit_fifo while
output was stopped (or DMA was paused with dmatx.queued == true and the
hardware FIFO drained) and pl011_rs485_config() runs in that window,
tx_stopped is false, so pl011_rs485_tx_resume() sets uap->rs485_tx_state =
SEND and uart_start() -> pl011_start_tx() skips delay_rts_before_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 +2545,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);
>  	}

[Severity: High]
When enabling RS485 via TIOCSRS485 while pl011_tx_pending(uap, tx_stopped) is
false, doesn't checking port->rs485.flags in the final else-if branch skip
pl011_rs485_tx_stop(uap) because port->rs485 still holds the old configuration?

In uart_set_rs485_config(), port->rs485_config(port, &tty->termios, &rs485) is
called before port->rs485 = rs485 is assigned:

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

When RS485 was previously disabled, UART011_CR_TXE is set in REG_CR (either
from pl011_startup() or from disabling RS485 in pl011_rs485_config()) and
UART011_CR_RTS reflects the RS232 mctrl state. If RS485 is then enabled when
no transmission is pending (or when tx_stopped is true and the hardware FIFO
is empty, which is the case the new pl011_dma_tx_stop(uap) call above prepares
for so that clearing TXE below cannot strand DMA data), pl011_tx_pending()
returns false.

Execution then reaches:

	} else if (port->rs485.flags & SER_RS485_ENABLED) {
		pl011_rs485_tx_stop(uap);
	}

Because port->rs485.flags does not yet have SER_RS485_ENABLED set,
pl011_rs485_tx_stop() is not called (and if pl011_stop_tx() runs later in the
tx_stopped case, it also skips pl011_rs485_tx_stop() because
uap->rs485_tx_state == OFF). As a result, UART011_CR_TXE is not cleared and
UART011_CR_RTS is not set to the RS485 idle level.

Additionally, even when port->rs485.flags already had SER_RS485_ENABLED set
and TIOCSRS485 reconfigures SER_RS485_RTS_AFTER_SEND while idle,
pl011_rs485_tx_stop() -> pl011_rs485_tx_stop_now() reads
port->rs485.flags & SER_RS485_RTS_AFTER_SEND from the old port->rs485 rather
than the new rs485 struct.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-devel-nbuchwitz-pl011-rs485-reconfig-stall-v3-0-072a9c82ea5e@tipi-net.de?part=1