On Mon, 2 Nov 2026, James Hilliard wrote:
> Moving RX DMA locking out of __dma_rx_do_complete() left the DMA
> completion callback calling it without rx_dma_lock. Shutdown and runtime
> suspend flush RX under rx_dma_lock but without the port lock, allowing
> these paths to overlap the callback's buffer accounting and flip-buffer
> insertion. RX submission and UART interrupt handling also require the
> port lock.
>
> Take rx_dma_lock inside the port lock in the completion callback. Hold
> the port lock around the shutdown and runtime-suspend flushes, matching
> the existing interrupt-path lock order. Assert both locks in the common
> completion helper and the port lock at the flush entry point.
>
> Fixes: 789898416749 ("serial: 8250: 8250_omap: Move locking out from
> __dma_rx_do_complete()")
> Signed-off-by: James Hilliard <[email protected]>
> ---
> drivers/tty/serial/8250/8250_omap.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_omap.c
> b/drivers/tty/serial/8250/8250_omap.c
> index b6f108d34854..322403cec2de 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -771,8 +771,10 @@ static void omap_8250_shutdown(struct uart_port *port)
> guard(serial8250_rpm)(up);
>
> flush_work(&priv->qos_work);
> - if (up->dma)
> + if (up->dma) {
> + guard(uart_port_lock_irq)(port);
> omap_8250_rx_dma_flush(up);
> + }
>
> serial_out(up, UART_OMAP_WER, 0);
> if (priv->habit & UART_HAS_EFR2)
> @@ -898,7 +900,7 @@ static int omap8250_rs485_config(struct uart_port *port,
> #ifdef CONFIG_SERIAL_8250_DMA
> static int omap_8250_rx_dma(struct uart_8250_port *p);
>
> -/* Must be called while priv->rx_dma_lock is held */
> +/* Must be called with the port lock and priv->rx_dma_lock held. */
Nit, following kerneldoc's style, this would start with Context: (you
don't need to make it full kerneldoc but it is useful to follow the kernel
doc style where possible).
> static void __dma_rx_do_complete(struct uart_8250_port *p)
> {
> struct uart_8250_dma *dma = p->dma;
> @@ -911,6 +913,9 @@ static void __dma_rx_do_complete(struct uart_8250_port *p)
> int ret;
> u32 reg;
>
> + lockdep_assert_held_once(&p->port.lock);
> + lockdep_assert_held_once(&priv->rx_dma_lock);
> +
> if (!dma->rx_running)
> goto out;
>
> @@ -977,7 +982,8 @@ static void __dma_rx_complete(void *param)
> if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) !=
> DMA_COMPLETE)
> return;
>
> - __dma_rx_do_complete(p);
> + scoped_guard(spinlock, &priv->rx_dma_lock)
> + __dma_rx_do_complete(p);
> if (priv->throttled)
> return;
>
> @@ -995,6 +1001,8 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port
> *p)
> unsigned long flags;
> int ret;
>
> + lockdep_assert_held_once(&p->port.lock);
> +
> spin_lock_irqsave(&priv->rx_dma_lock, flags);
>
> if (!dma->rx_running) {
> @@ -1811,8 +1819,10 @@ static int omap8250_runtime_suspend(struct device *dev)
> }
> }
>
> - if (up && up->dma && up->dma->rxchan)
> + if (up && up->dma && up->dma->rxchan) {
> + guard(uart_port_lock_irqsave)(&up->port);
> omap_8250_rx_dma_flush(up);
> + }
>
> priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
> schedule_work(&priv->qos_work);
>
>
--
i.
