Re: [PATCH] dmaengine: mediatek: uart: make channel termination atomic

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Oct 22, 2026 at 03:18:26PM -0600, James Hilliard wrote:
> The 8250 transmit flush path calls dmaengine_terminate_async() with the
> UART port spinlock held and interrupts disabled. UART APDMA termination
> uses sleeping register polls and synchronize_irq(), which can also wait for
> a threaded handler or for an interrupt pending on the calling CPU.
>
> Serialize the stop sequence with the interrupt handler using the virtual
> channel lock and use atomic register polling. A handler that was waiting
> for that lock must check interrupt status before accessing a descriptor:
> termination may already have masked and acknowledged its interrupt. This
> allows termination to retire descriptors without waiting for the handler.
>
> If the channel does not stop, return the timeout and retain its descriptors
> instead of freeing state still owned by the transfer. Continue attempting
> STOP after a flush timeout, since stopping the channel is sufficient for
> termination even when the flush could not finish.
>
> Fixes: 9135408c3ace ("dmaengine: mediatek: Add MediaTek UART APDMA support")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/mediatek/mtk-uart-apdma.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/mediatek/mtk-uart-apdma.c b/drivers/dma/mediatek/mtk-uart-apdma.c
> index f74e9a328588..73d4b701ac85 100644
> --- a/drivers/dma/mediatek/mtk-uart-apdma.c
> +++ b/drivers/dma/mediatek/mtk-uart-apdma.c
> @@ -256,6 +256,12 @@ static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
>  	unsigned long flags;
>
>  	spin_lock_irqsave(&c->vc.lock, flags);
> +	/* Termination may have acknowledged an interrupt before we got the lock. */
> +	if (!mtk_uart_apdma_read(c, VFF_INT_FLAG)) {
> +		spin_unlock_irqrestore(&c->vc.lock, flags);
> +		return IRQ_NONE;
> +	}
> +
>  	if (c->dir == DMA_DEV_TO_MEM)
>  		mtk_uart_apdma_rx_handler(c);
>  	else if (c->dir == DMA_MEM_TO_DEV)
> @@ -400,10 +406,12 @@ static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
>  	LIST_HEAD(head);
>  	int ret;
>
> +	spin_lock_irqsave(&c->vc.lock, flags);
> +
>  	mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
>
> -	ret = readx_poll_timeout(readl, c->base + VFF_FLUSH,
> -			  status, status != VFF_FLUSH_B, 10, 100);
> +	ret = readx_poll_timeout_atomic(readl, c->base + VFF_FLUSH,
> +					status, status != VFF_FLUSH_B, 10, 100);
>  	if (ret)
>  		dev_err(c->vc.chan.device->dev, "flush: fail, status=0x%x\n",
>  			mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> @@ -415,11 +423,14 @@ static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
>  	 * 3. set stop as 0
>  	 *
>  	mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_B);
> -	ret = readx_poll_timeout(readl, c->base + VFF_EN,
> -			  status, !status, 10, 100);
> -	if (ret)
> +	ret = readx_poll_timeout_atomic(readl, c->base + VFF_EN,
> +					status, !status, 10, 100);
> +	if (ret) {
>  		dev_err(c->vc.chan.device->dev, "stop: fail, status=0x%x\n",
>  			mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> +		spin_unlock_irqrestore(&c->vc.lock, flags);
> +		return ret;
> +	}
>
>  	mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_CLR_B);
>  	mtk_uart_apdma_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> @@ -429,9 +440,6 @@ static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
>  	else if (c->dir == DMA_MEM_TO_DEV)
>  		mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_CLR_B);
>
> -	synchronize_irq(c->irq);
> -
> -	spin_lock_irqsave(&c->vc.lock, flags);
>  	vchan_get_all_descriptors(&c->vc, &head);
>  	spin_unlock_irqrestore(&c->vc.lock, flags);
>
>
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> change-id: 20261022-submit-mtk-uart-dma-terminate-atomic-v1-19693366ae4c
>
> Best regards,
> --
> James Hilliard <james.hilliard1@gmail.com>
>