# Re: \[PATCH v8 6/6\] serial: 8250: retain DMA resources when termination fails

- From: Andy Shevchenko \<andriy.shevchenko@linux.intel.com\>
- Date: 2026-10-29 14:24:04 
- List: linuxppc-dev
- Page: https://ratatoskr.run/linuxppc-dev/2026/10/17827486
- Thread: https://ratatoskr.run/linux-serial/2026/10/17825177/t.md

```text
On Thu, Oct 29, 2026 at 01:37:02AM -0600, James Hilliard wrote:
> serial8250_release_dma() ignores the return from dmaengine_terminate_sync()
> before freeing the coherent RX buffer, unmapping the TX FIFO and releasing
> the channels. If the provider fails to stop a channel, the DMAengine
> wrapper returns the error without synchronizing its callbacks. Hardware
> or a detached callback can then access the released storage and state.
> 
> Checking the error and returning early would not preserve their lifetime:
> serial core frees the TX FIFO after the driver's shutdown callback returns,
> and port removal can then release the callback context as well.
> 
> Require successful synchronous termination of both channels before
> releasing any DMA resources. Retry failed channels after a sleep outside
> the port lock, leaving channels that have already stopped and drained
> alone. Keep the shutdown gate and storage reservation in place throughout
> the retries.
> 
> A permanently failed DMA engine blocks close or removal. This deliberately
> retains the buffers, mappings, channels and callback context until the
> provider can establish that they are no longer in use.

...

> +	while (rx_ret || tx_ret) {
> +		dev_err_ratelimited(p->port.dev,
> +				    "DMA termination failed: RX %d, TX %d; retrying\n",
> +				    rx_ret, tx_ret);

> +		msleep(100);

Huh?!

> +		if (rx_ret)
> +			rx_ret = dmaengine_terminate_sync(dma->rxchan);
> +		if (tx_ret)
> +			tx_ret = dmaengine_terminate_sync(dma->txchan);
> +	}

This is unacceptable. Sleeping is okay, but value should really match the
baudspeed and other parameters of the transfer.

-- 
With Best Regards,
Andy Shevchenko



```