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

[sashiko-bot@kernel.org]

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