Re: [PATCH 1/1] serial: qcom-geni: fix TX DMA buffer flush

Hi

On 29-07-2026 23:11, jaseg wrote:
> From: Jan Sebastian Götte <[email protected]>
>
> When transmit flushing a qcom-geni UART during an ongoing TX DMA, the
> UART gets stuck infinitely repeating corrupted TX DMA frames.
>
> The DMA-mode uart_ops does not provide a flush_buffer callback, so an
> in-flight transfer can complete after serial core has reset the transmit
> kfifo, underflowing its length and resubmitting page-sized transfers
> indefinitely. Add one that stops the transfer and clears tx_remaining
> and tx_queued.
>
> The stop path was also broken: it unmapped the buffer while the serial
> engine could still read it, and never reset the TX DMA state machine.
> Cancel the main sequencer command first, then reset the state machine
> and wait for it before unmapping. Drop the early return so a pending
> mapping is also cleaned up when the main command is inactive.
>
> The bug can be triggered from userspace with a large write immediately
> followed by TCOFLUSH. A following tcdrain will hang forever. The bug was
> reproduced and this fix was validated on Arduino Uno Q (QRB2210)
> using /dev/ttyHS1.
>
> Assisted-by: Claude:claude-5-opus Codex:gpt-5
> Signed-off-by: Jan Sebastian Götte <[email protected]>
> Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA")
> Cc: [email protected]
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 43 ++++++++++++++-------------
>   1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 7ead87b4eb65..1e39122ec09f 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -158,6 +158,7 @@ static const struct uart_ops qcom_geni_uart_pops;
>   static struct uart_driver qcom_geni_console_driver;
>   static struct uart_driver qcom_geni_uart_driver;
>   
> +static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport);
>   static void __qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>   static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport);
>   static int qcom_geni_serial_port_setup(struct uart_port *uport);
> @@ -636,35 +637,34 @@ static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
>   	return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
>   }
>   
> +static void qcom_geni_serial_flush_buffer_dma(struct uart_port *uport)
> +{
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +
> +	qcom_geni_serial_stop_tx_dma(uport);
> +	port->tx_remaining = 0;
> +	port->tx_queued = 0;
> +}
> +
>   static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
>   {
>   	struct qcom_geni_serial_port *port = to_dev_port(uport);
> -	bool done;
>   
> -	if (!qcom_geni_serial_main_active(uport))
> -		return;
> +	if (qcom_geni_serial_main_active(uport))
> +		__qcom_geni_serial_cancel_tx_cmd(uport);
>   
>   	if (port->tx_dma_addr) {
> +		writel(1, uport->membase + SE_DMA_TX_FSM_RST);
> +		if (!qcom_geni_serial_poll_bit(uport, SE_DMA_TX_IRQ_STAT,
> +					       TX_RESET_DONE, true))
> +			dev_err_ratelimited(uport->dev, "TX DMA reset failed");
> +		writel(TX_RESET_DONE | TX_DMA_DONE,
> +		       uport->membase + SE_DMA_TX_IRQ_CLR);
> +
>   		geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
>   				      port->tx_remaining);
>   		port->tx_dma_addr = 0;
> -		port->tx_remaining = 0;
>   	}
> -
> -	geni_se_cancel_m_cmd(&port->se);
> -
> -	done = qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> -					 M_CMD_CANCEL_EN, true);
> -	if (!done) {
> -		geni_se_abort_m_cmd(&port->se);
> -		done = qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> -						 M_CMD_ABORT_EN, true);
> -		if (!done)
> -			dev_err_ratelimited(uport->dev, "M_CMD_ABORT_EN not set");
> -		writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
> -	}
> -
> -	writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
>   }
>   
>   static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> @@ -1180,7 +1180,7 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport)
>   	uart_port_unlock_irq(uport);
>   }
>   
> -static void qcom_geni_serial_flush_buffer(struct uart_port *uport)
> +static void qcom_geni_serial_flush_buffer_fifo(struct uart_port *uport)
>   {
>   	qcom_geni_serial_cancel_tx_cmd(uport);
>   }
> @@ -1769,7 +1769,7 @@ static const struct uart_ops qcom_geni_console_pops = {
>   	.request_port = qcom_geni_serial_request_port,
>   	.config_port = qcom_geni_serial_config_port,
>   	.shutdown = qcom_geni_serial_shutdown,
> -	.flush_buffer = qcom_geni_serial_flush_buffer,
> +	.flush_buffer = qcom_geni_serial_flush_buffer_fifo,
>   	.type = qcom_geni_serial_get_type,
>   	.set_mctrl = qcom_geni_serial_set_mctrl,
>   	.get_mctrl = qcom_geni_serial_get_mctrl,
> @@ -1792,6 +1792,7 @@ static const struct uart_ops qcom_geni_uart_pops = {
>   	.request_port = qcom_geni_serial_request_port,
>   	.config_port = qcom_geni_serial_config_port,
>   	.shutdown = qcom_geni_serial_shutdown,
> +	.flush_buffer = qcom_geni_serial_flush_buffer_dma,
>   	.type = qcom_geni_serial_get_type,
>   	.set_mctrl = qcom_geni_serial_set_mctrl,
>   	.get_mctrl = qcom_geni_serial_get_mctrl,

Reviewed-by: Praveen Talari <[email protected]>

Thanks,

Praveen Talari