On Thu, Oct 29, 2026 at 6:32 PM James Hilliard [off-list ref] wrote:
With a persistent pause failure and an incomplete transfer, the
worker keeps retrying. RX remains masked and the runtime PM reference
is held until the condition resolves or shutdown takes over. There
is no receive-progress guarantee if the peer remains idle and pause
continues to fail.
...
A failed pause with an incomplete cookie does not distinguish a
persistent hardware fault from completion that has not yet been
published. A fixed retry limit does not resolve that ambiguity.
Terminating after the limit may discard received data, including a
completed buffer whose completion is still unpublished. Successful
termination would make subsequent buffer reuse safe, but would not
recover those bytes. This would therefore be a potentially lossy
error-recovery policy rather than a lossless timeout flush.
Checking the in-tree dmaengine providers shows why bounding the retry loop
and escalating to recovery is necessary in practice:
1. `serial8250_request_dma()` (`8250_dma.c:428`) checks `caps.cmd_pause`,
which `dma_get_slave_caps()` (`drivers/dma/dmaengine.c:614`) sets purely
from `!!device->device_pause`. However, `drivers/dma/tegra186-gpc-dma.c`
(lines 422-423, 1351, 1521) unconditionally assigns
`tdma->dma_dev.device_pause = tegra_dma_device_pause` (so
`caps.cmd_pause == true`), and then returns `-ENOSYS` at runtime on
Tegra186 where `tdc->tdma->chip_data->hw_support_pause` is false. And on
Tegra194/234/264 (`tegra186-gpc-dma.c:399-413`) and Tegra210
(`tegra210-adma.c:574-579`), `.device_pause()` polls a hardware busy bit
with an atomic timeout and returns `-ETIMEDOUT` or `-EBUSY` if an active
burst does not pause in time.
2. In the in-tree providers where `.device_pause()` returns an error because
a transfer already completed—such as `drivers/dma/ti/edma.c:870, 1468-1469`
(returning `-EINVAL` when `!echan->edesc`) and
`drivers/dma/stm32/stm32-dma.c:751-753, 836, 1432` (returning `-EPERM`
when `chan->status != DMA_IN_PROGRESS`)—the DMA hardirq handler calls
`vchan_cookie_complete()` *before* clearing the active descriptor, so
`dmaengine_tx_status()` already reports `DMA_COMPLETE` on the very first
check in `serial8250_rx_dma_try_flush()`.
Even on a controller where hardware rejects `pause` the instant the 4096th
byte finishes while its completion hardirq is still masked by `port->lock`,
the only thing delaying `completed_cookie` publication is local IRQ masking
under `port->lock`. Once `serial8250_rx_dma_flush_work()` drops `port->lock`
(enabling local IRQs) and sleeps `usleep_range(1000, 2000)` in process
context, any pending DMA completion hardirq runs in microseconds. Across
5 retries (5-10 ms with `port->lock` released on each sleep), a completed
4 KB transfer will have published `DMA_COMPLETE`.
By contrast, leaving the `for (;;)` loop unbounded turns any persistent
`dmaengine_pause()` error (`-ENOSYS`, `-EINVAL`, `-EBUSY`, `-ETIMEDOUT`) on
a partial transfer into a guaranteed 100% loss of all current and future RX
data until `close()`, a 500-1000 Hz `system_percpu_wq` spin, a permanent
runtime-PM leak, and silently aborted `dw8250_set_divisor()` /
`dw8250_check_lcr()` calls (`-EAGAIN`). Escalating to recovery after a
bounded retry budget is strictly better than wedging the receiver forever.
That requires termination to succeed and callbacks to be drained.
If termination fails, the channel may still access the FIFO, so PIO
must remain excluded.
The recovery requirements depend on why the provider cannot pause.
Agreed—and `serial8250_rx_dma_flush_work()` in Patch 5/6 already enforces
that when `serial8250_rx_dma_recover(p)` is called (`8250_dma.c:210-216`):
if (recover) {
ret = dmaengine_terminate_sync(dma->rxchan);
if (ret) {
/* Retain ownership and keep RX masked if DMA cannot be stopped. */
dev_err(p->port.dev, "failed to stop RX DMA: %d\n", ret);
return;
}
}
When `serial8250_rx_dma_recover(p)` sets `dma->rx_err = true` and
`dma->rx_terminating = true`, the worker exits the retry loop and calls
`dmaengine_terminate_sync(dma->rxchan)`. If `dmaengine_terminate_sync()`
returns non-zero, the worker returns early at line 215 with
`dma->rx_terminating == true` and `dma->rx_running == 1`, keeping RX
interrupts masked in `IER` and keeping PIO excluded (`serial8250_rx_dma_flush()`
continues returning `-EIO`). It only clears `dma->rx_running = 0` and unmasks
`IER` for PIO fallback after `dmaengine_terminate_sync(dma->rxchan)` succeeds
(`ret == 0`).
Thanks,
Raul