Re: [PATCH] dmaengine: xilinx_dma: Use readl_poll_timeout() instead of atomic variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch replaces `readl_poll_timeout_atomic()` with `readl_poll_timeout()` in an atomic context, violating the `iopoll.h` API contract and introducing a risk of infinite loops and severe latency.
--

--- Patch [1]: [PATCH] dmaengine: xilinx_dma: Use readl_poll_timeout() instead of atomic variant ---
commit f549bf1ac3f30db8e280af9c3f9f7f1f4d4ac1b5
Author: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

dmaengine: xilinx_dma: Use readl_poll_timeout() instead of atomic variant

This commit switches xilinx_dma_poll_timeout() from the atomic to the
regular readl_poll_timeout() macro. The commit message states that this
is safe because zero delay means it never sleeps.

> Calling xilinx_dma_poll_timeout() from atomic context is still safe even
> after this change, as readl_poll_timeout() with zero delay never sleeps.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -530,7 +530,7 @@ struct xilinx_dma_device {
>  #define to_dma_tx_descriptor(tx) \
>  	container_of(tx, struct xilinx_dma_tx_descriptor, async_tx)
>  #define xilinx_dma_poll_timeout(chan, reg, val, cond, delay_us, timeout_us) \
> -	readl_poll_timeout_atomic(chan->xdev->regs + chan->ctrl_offset + reg, \
> +	readl_poll_timeout(chan->xdev->regs + chan->ctrl_offset + reg, \
>  				  val, cond, delay_us, timeout_us)

[Severity: High]
Is the assertion in the commit message accurate for all hardware
configurations, or could this regression lead to a hard lockup during DMA
transfers or error resets?

While it is true that readl_poll_timeout() does not sleep with a zero delay,
it still relies on ktime_get() for timekeeping. The API documentation in
iopoll.h explicitly forbids calling readl_poll_timeout() from atomic context
if timeout_us is used.

This macro is called with interrupts disabled in multiple paths. For example,
xilinx_dma_issue_pending() calls start_transfer() under spin_lock_irqsave(),
and the irq handler also calls it under a spinlock:

drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_irq_handler() {
        ...
        spin_lock(&chan->lock);
        xilinx_dma_complete_descriptor(chan);
        chan->idle = true;
        chan->start_transfer(chan);
        ...
}

If ktime_get() relies on timer interrupts to advance the clock on certain
hardware configurations, and those interrupts are disabled, would this cause
the polling loop to hang indefinitely if the hardware condition is never met?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-xilinx-dma-poll-fix-v1-1-892302c4847d@ideasonboard.com?part=1