On Wed, Nov 11, 2026 at 03:16:32PM -0700, James Hilliard wrote:
> The generic RX timeout path pauses a transfer, copies its data and
> terminates it asynchronously. Clearing rx_running at that point lets a
> new receive reuse the buffer and cookie while the old callback has not
> yet run. Inspecting the new cookie cannot distinguish that callback from
> the completion of a new transfer.
>
> Reserve the receive buffer until a worker has drained the canceled
> callbacks. Keep FIFO access separate from callback retirement: after a
> successful pause and termination, clear rx_running while rx_terminating
> prevents another descriptor from reusing the buffer. Deliver the paused
> data before allowing the interrupt handler to drain the FIFO through PIO.
> Normal retirement only releases ownership and leaves subsequent receive
> handling to the UART interrupt.
>
> Preserve the normal completion-path LSR read and gate DMA resubmission on
> the receive interrupt enable, so stop_rx() prevents another transfer while
> the status read retains its existing side effects.
>
> Reject new flush work and PIO receive handling during shutdown, including
> late Intel MID DMA timeouts. Stop and drain retirement work before freeing
> the channels and buffer. Once RX has stopped, allow the idle flush used by
> DesignWare LCR recovery during shutdown. Clear the shutdown guard before
> startup configures the UART and tests its interrupts.
>
> Check pause failures before accessing the buffer. A successful pause is
> the quiescence guarantee, even when a provider still reports IN_PROGRESS.
> Also accept DMA_COMPLETE after a failed pause: completion can stop the
> channel before its callback acquires the port lock, leaving nothing for
> the provider to pause. Retire that completed transfer normally.
>
> A failed pause with an incomplete cookie is ambiguous: hardware may have
> finished while its completion interrupt is still pending. Initially retain
> the transfer and retry pause and status in the worker. Mask receive
> interrupts to prevent a timeout storm and keep callbacks from copying or
> resubmitting this worker-owned transfer. Back off retries from 1 ms,
> allowing completion and shutdown to wake the worker immediately.
>
> After one second of unsuccessful pause retries, stop the transfer with
> dmaengine_terminate_sync() outside the port lock. An indefinite wait could
> otherwise leave a request/response peer waiting for data that is stuck in
> a partial DMA buffer. Report that recovery discards the pending payload:
> without a stable residue it cannot safely be copied. Keep rx_running,
> rx_pause_pending and rx_terminating set until termination has stopped DMA
> and drained its callbacks. Then restore receive handling and allow another
> DMA descriptor, without latching a permanent PIO fallback for a pause
> timeout. This trades the stalled transfer's data for continued reception.
>
> If synchronous termination fails, retain ownership and retry with a delay
> that doubles from 1 ms to one second. Report the first failure and ongoing
> failures every five seconds for this recovery. Completion and shutdown
> wake the retry wait; shutdown takes over final channel retirement. A
> permanent termination failure still leaves RX masked and its storage
> reserved: the pause retry budget does not guarantee recovery from a
> provider that cannot stop. Each flush starts with a fresh retry budget.
>
> RX DMA admission already requires cmd_pause. A provider whose hardware
> cannot pause must advertise that correctly. If pause nevertheless returns
> -ENOSYS or -EOPNOTSUPP for an incomplete transfer, enter synchronous
> recovery immediately. Unsupported pause, DMA_ERROR and asynchronous
> termination failure retain the error fallback to PIO until reopen.
>
> Keep the requested enables in the existing IER shadow and filter hardware
> writes during pause retries and recovery, including termios, RS485 and
> overrun-backoff changes. Apply the current enables after retirement so
> stop_rx() and shutdown remain effective. Hold a runtime PM reference
> around worker register accesses.
>
> Abort the DesignWare idle sequence when RX cannot be stopped. Allow the
> empty-FIFO timeout workaround once DMA has released the FIFO, including
> while callbacks retire. Exclude FIFO resets during RX DMA recovery.
...
> +static void serial8250_rx_dma_flush_work(struct work_struct *work)
> +{
> + struct uart_8250_dma *dma = container_of(work, struct uart_8250_dma,
> rx_flush_work);
> + struct uart_8250_port *p = dma->port;
> + unsigned int delay_us = 1 * USEC_PER_MSEC;
> + ktime_t start = ktime_get();
> + bool recover, retry, restore_rx, fallback;
> + bool timed_out = false;
> + int ret = 0;
Once set, it gets never cleared.
> + guard(serial8250_rpm)(p);
> +
> + for (;;) {
> + scoped_guard(uart_port_lock_irqsave, &p->port) {
> + /* Shutdown cancels this work and drains both DMA
> channels. */
> + if (dma->shutting_down)
> + return;
> + /* Serialize reset with completion and shutdown
> notifications. */
> + WRITE_ONCE(dma->rx_pause_wakeup, false);
> + retry = dma->rx_pause_pending && !dma->rx_terminating;
> + if (retry) {
> + ret = serial8250_rx_dma_try_flush(p);
> + retry = dma->rx_pause_pending &&
> !dma->rx_terminating;
> + if (retry &&
> + ktime_ms_delta(ktime_get(), start) >= 1 *
> MSEC_PER_SEC) {
> + dma->rx_terminating = true;
> + timed_out = true;
> + retry = false;
> + }
> + }
> + recover = dma->rx_err || (dma->rx_running &&
> dma->rx_terminating);
> + fallback = dma->rx_err;
> + }
> + if (!retry)
> + break;
> + /* Notifications also cover completion or shutdown before
> sleeping. */
> + wait_event_hrtimeout(dma->rx_pause_wait,
> READ_ONCE(dma->rx_pause_wakeup),
> + ns_to_ktime(delay_us * NSEC_PER_USEC));
> + delay_us = min(delay_us * 2, 1 * USEC_PER_SEC);
> + }
> +
> + if (timed_out)
> + dev_warn(p->port.dev, "RX DMA pause timed out: %d; discarding
> pending data\n", ret);
So here is the potential to get timed_out == true and stale ret. Yes, I see
that code couples them, but the logic of the above is quite complicated.
The scoped_guard() piece better to be in a separate helper (and replaced with
guard()() to make it on indentation level down).
> + if (recover) {
> + if (serial8250_rx_dma_terminate(p))
> + return;
> + } else {
> + dmaengine_synchronize(dma->rxchan);
> + }
> +
> + scoped_guard(uart_port_lock_irqsave, &p->port) {
> + restore_rx = recover || dma->rx_pause_pending;
> + dma->rx_pause_pending = false;
> + dma->rx_terminating = false;
> + dma->rx_running = 0;
> + if (dma->shutting_down)
> + return;
> +
> + if (!restore_rx)
> + return;
> +
> + /* Apply the current enables, including any intervening
> stop_rx(). */
> + serial8250_out_IER(p, p->ier);
> + }
> + if (fallback)
> + dev_warn(p->port.dev, "RX DMA failed, using PIO\n");
> }
...
Overall it makes me think if this is a common DMA issue when we try to stop
ongoing transfers, shouldn't it be solved once for all somewhere in DMAengine?
--
With Best Regards,
Andy Shevchenko
