On Mon, Nov 02, 2026 at 04:14:34PM -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. Canceling that
> descriptor can discard the entire completed receive buffer and switch
> permanently to PIO. Retain the transfer and retry pause and status in the
> worker, with receive interrupts masked to prevent a timeout interrupt
> storm. The worker copies only after a successful pause or reported
> completion, then terminates and drains the old callback before reuse.
> The pending-pause guard keeps callbacks from copying or resubmitting the
> transfer concurrently.
>
> Start pause retries at 1 ms and double the delay up to one second to limit
> CPU use during persistent failure. Sleep outside the port lock on a timed
> waitqueue, with completion and shutdown publishing a wakeup under the
> same lock as the worker's retry check. This also catches notifications
> arriving just before the worker sleeps, avoiding a full backoff delay
> after completion or during shutdown. Reset the delay for each new flush.
> Report a stall lasting at least one second once per flush, rate-limited
> across repeated stalls. A provider which never pauses or completes retains
> RX ownership until an error or shutdown; a timeout alone cannot establish
> that discarding the buffer is safe.
>
> On DMA_ERROR or termination failure, let the worker stop DMA before
> handing the FIFO to PIO. 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 and keep RX
> on PIO until reopen after a confirmed error.
>
> 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. If the
> provider still cannot terminate the channel in process context, retain
> its buffer and leave receive interrupts masked.
...
> +static inline bool serial8250_rx_dma_running(struct uart_8250_port *up)
> +{
> + return up->dma && up->dma->rx_running;
> +}
> +
> +static inline bool serial8250_rx_dma_recovering(struct uart_8250_port *up)
> +{
> + return up->dma && (up->dma->rx_pause_pending ||
> + (up->dma->rx_err && up->dma->rx_terminating));
Slightly better formatting is to put some line breaks.
return up->dma &&
(up->dma->rx_pause_pending || (up->dma->rx_err &&
up->dma->rx_terminating));
Alternatively
if (!up->dma)
return false;
return up->dma->rx_pause_pending || (up->dma->rx_err &&
up->dma->rx_terminating);
> +}
> +/* Keep the desired enables in up->ier while RX DMA recovery masks hardware.
> */
> +static inline void serial8250_out_IER(struct uart_8250_port *up, u8 ier)
> +{
> + lockdep_assert_held_once(&up->port.lock);
> +
> + if (up->dma && up->dma->shutting_down)
> + ier = 0;
> + else if (serial8250_rx_dma_recovering(up))
> + ier &= ~(UART_IER_RDI | UART_IER_RLSI | UART_IER_RTOIE);
> + serial_out(up, UART_IER, ier);
> +}
...
> struct tty_port *tty_port = &p->port.state->port;
> - struct dma_tx_state state;
> - enum dma_status dma_status;
> - int count;
> -
> - /*
> - * New DMA Rx can be started during the completion handler before it
> - * could acquire port's lock and it might still be ongoing. Don't to
> - * anything in such case.
> - */
> - dma_status = dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
> - if (dma_status == DMA_IN_PROGRESS)
> - return;
> -
> - count = dma->rx_size - state.residue;
> + size_t count = dma->rx_size - state->residue;
No need to put too many spaces between type and variable name.
(Yes, the first one will be left untouched.)
...
> +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 = USEC_PER_MSEC;
> + ktime_t start = ktime_get();
> + bool recover, retry, restore_rx;
> + bool warned = false;
> + int ret;
> +
> + 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;
> + }
> + recover = dma->rx_err;
> + }
> + if (!retry)
> + break;
> + if (!warned && ktime_ms_delta(ktime_get(), start) >=
> MSEC_PER_SEC) {
> + dev_warn_ratelimited(p->port.dev,
> + "RX DMA pause stalled: %d;
> retaining transfer and retrying\n",
> + ret);
_ratelimited() is a slop of AI without understanding the code. It makes nonsense
in combination with warned.
> + warned = true;
Altogether NIH _once().
> + }
> + /* 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, USEC_PER_SEC);
> + }
> +
> + 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;
> + }
> + } 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 (recover)
> + dev_warn(p->port.dev, "RX DMA failed, using PIO\n");
> }
...
> -void serial8250_rx_dma_flush(struct uart_8250_port *p)
> +int serial8250_rx_dma_flush(struct uart_8250_port *p)
> {
> struct uart_8250_dma *dma = p->dma;
> + int ret;
>
> - if (!dma || !dma->rxchan || !dma->rx_running)
> - return;
> + lockdep_assert_held_once(&p->port.lock);
>
> - dmaengine_pause(dma->rxchan);
> - __dma_rx_complete(p);
> - dmaengine_terminate_async(dma->rxchan);
> + if (!dma || !dma->rx_running)
You introduced a helper serial8250_rx_dma_running(). Use it.
> + return 0;
> + /* A late interrupt must not hand a still-active receiver to PIO. */
> + if (dma->shutting_down)
> + return -ESHUTDOWN;
> + if (dma->rx_terminating)
> + return -EIO;
> + if (dma->rx_pause_pending)
> + return -EAGAIN;
> +
> + ret = serial8250_rx_dma_try_flush(p);
> + schedule_work(&dma->rx_flush_work);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(serial8250_rx_dma_flush);
...
> /* Manually stop the Rx DMA transfer when acting as flow controller */
> - if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running
> && rx_timeout) {
> + if (quirks & DW_UART_QUIRK_IS_DMA_FC && rx_timeout &&
> + serial8250_rx_dma_running(up) && !up->dma->rx_terminating &&
> + !up->dma->rx_pause_pending) {
Re-wrap it logically (also be consistent with parentheses
when & versus && is used).
if ((quirks & DW_UART_QUIRK_IS_DMA_FC) && rx_timeout &&
serial8250_rx_dma_running(up) &&
!up->dma->rx_terminating && !up->dma->rx_pause_pending) {
> }
--
With Best Regards,
Andy Shevchenko
