Re: [PATCH] dmaengine: xilinx_dma: Use readl_poll_timeout() instead of atomic variant
[email protected] Mon, 03 Aug 2026 13:41:28 +0000
| Newsgroups | org.kernel.vger.dmaengine |
|---|---|
| Message-ID | <[email protected]> |
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() inst= ead of atomic variant --- commit f549bf1ac3f30db8e280af9c3f9f7f1f4d4ac1b5 Author: Tomi Valkeinen <[email protected]> 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 exampl= e, 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 =3D 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 me= t? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803-xilinx-dma= [email protected]?part=3D1