dma_chan_pause() clears SUSPENDED even when polling times out. Hardware can set that status after the last poll, and the unconditional clear then discards an acknowledgment the caller never observed. Another channel's interrupt can also consume the status because the shared IRQ handler clears every channel's status before taking its lock. Leave SUSPENDED to the pause and resume paths. Track the suspend request separately from its acknowledgment so a timeout can be followed by either a pause retry or a resume. Wait for acknowledgment before clearing the hardware request, since hardware ignores an earlier clear. If that wait also times out, return -EAGAIN with the request still pending. A completed transfer is also quiescent, but its completion handler may still need to advance a cyclic descriptor. Defer that restart while a pause is pending, and resume it without reprogramming the first LLI. Track the active descriptor until its completion handler runs so that resume and issue_pending cannot restart a completed cyclic segment or reissue a completed noncyclic descriptor while its IRQ is pending. Start queued work after the active descriptor is retired, respecting the pause request. Clear stale suspension status at the start of a new descriptor rather than at each cyclic restart. Reset the software state after successful termination and when allocating a channel for a new client. Store the lock-protected pause flags as bitfields in existing padding, keeping cyclic in a separate byte because it is set outside vc.lock. Fixes: 1fe20f1b8454 ("dmaengine: Introduce DW AXI DMAC driver") Cc: stable@vger.kernel.org Signed-off-by: James Hilliard --- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 98 ++++++++++++++++++++++---- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 9 ++- 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index bb305ab94d02..6f15fcb790a8 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -481,6 +481,8 @@ static void axi_chan_block_xfer_start(struct axi_dma_chan *chan, irq_mask |= DWAXIDMAC_IRQ_SUSPENDED; axi_chan_irq_set(chan, irq_mask); + axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED); + chan->desc = first; axi_chan_enable(chan); } @@ -489,6 +491,9 @@ static void axi_chan_start_first_queued(struct axi_dma_chan *chan) struct axi_dma_desc *desc; struct virt_dma_desc *vd; + if (chan->desc || chan->pause_requested) + return; + vd = vchan_next_desc(&chan->vc); if (!vd) return; @@ -543,6 +548,10 @@ static int dma_chan_alloc_chan_resources(struct dma_chan *dchan) pm_runtime_put(chan->chip->dev); return -ENOMEM; } + chan->desc = NULL; + chan->pause_requested = false; + chan->is_paused = false; + chan->cyclic_restart = false; dev_vdbg(dmaengine_chan_dev(dchan), "%s: allocating\n", axi_chan_name(chan)); return 0; @@ -1087,6 +1096,8 @@ static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status) vchan_cookie_complete(vd); /* Try to restart the controller */ + chan->desc = NULL; + chan->cyclic_restart = false; axi_chan_start_first_queued(chan); out: @@ -1135,12 +1146,19 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan) } } - axi_chan_enable(chan); + if (chan->pause_requested) { + chan->is_paused = true; + chan->cyclic_restart = true; + } else { + axi_chan_enable(chan); + } } } else { /* Remove the completed descriptor from issued list before completing */ list_del(&vd->node); vchan_cookie_complete(vd); + chan->desc = NULL; + axi_chan_start_first_queued(chan); } out: @@ -1162,6 +1180,8 @@ static irqreturn_t dw_axi_dma_interrupt(int irq, void *dev_id) for (i = 0; i < dw->hdata->nr_channels; i++) { chan = &dw->chan[i]; status = axi_chan_irq_read(chan); + /* Leave suspension status for the pause and resume paths. */ + status &= ~DWAXIDMAC_IRQ_SUSPENDED; axi_chan_irq_clear(chan, status); dev_vdbg(chip->dev, "%s %u IRQ status: 0x%08x\n", @@ -1208,6 +1228,11 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) vchan_get_all_descriptors(&chan->vc, &head); + axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED); + chan->desc = NULL; + chan->pause_requested = false; + chan->is_paused = false; + chan->cyclic_restart = false; chan->cyclic = false; spin_unlock_irqrestore(&chan->vc.lock, flags); @@ -1218,16 +1243,50 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) return 0; } +static bool axi_chan_is_suspended(struct axi_dma_chan *chan) +{ + if (!axi_chan_is_hw_enable(chan)) { + /* A completed transfer cannot acknowledge a new suspend request. */ + axi_chan_disable(chan); + return true; + } + + return axi_chan_irq_read(chan) & DWAXIDMAC_IRQ_SUSPENDED; +} + +/* A timed-out request can still be acknowledged. */ +static int axi_chan_wait_for_suspend(struct axi_dma_chan *chan) + __must_hold(&chan->vc.lock) +{ + bool suspended; + int ret; + + if (chan->is_paused) + return 0; + + ret = readx_poll_timeout_atomic(axi_chan_is_suspended, chan, + suspended, suspended, 2, 40); + if (ret) + return -EAGAIN; + + axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED); + chan->is_paused = true; + return 0; +} + static int dma_chan_pause(struct dma_chan *dchan) { struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan); unsigned long flags; - u32 status; int ret; u64 val; spin_lock_irqsave(&chan->vc.lock, flags); + chan->pause_requested = true; + if (chan->is_paused || !axi_chan_is_hw_enable(chan)) + goto wait; + if (chan->chip->dw->hdata->nr_channels >= DMAC_CHAN_16) { val = axi_dma_ioread64(chan->chip, DMAC_CHSUSPREG); if (chan->id >= DMAC_CHAN_16) { @@ -1254,17 +1313,11 @@ static int dma_chan_pause(struct dma_chan *dchan) } } - ret = readx_poll_timeout_atomic(axi_chan_irq_read, chan, - status, status & DWAXIDMAC_IRQ_SUSPENDED, - 2, 40); - - axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED); - - chan->is_paused = true; - +wait: + ret = axi_chan_wait_for_suspend(chan); spin_unlock_irqrestore(&chan->vc.lock, flags); - return ret ? -EAGAIN : 0; + return ret; } /* Called in chan locked context */ @@ -1298,6 +1351,8 @@ static inline void axi_chan_resume(struct axi_dma_chan *chan) } } + axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED); + chan->pause_requested = false; chan->is_paused = false; } @@ -1305,15 +1360,30 @@ static int dma_chan_resume(struct dma_chan *dchan) { struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan); unsigned long flags; + int ret = 0; spin_lock_irqsave(&chan->vc.lock, flags); - if (chan->is_paused) - axi_chan_resume(chan); + if (!chan->pause_requested) + goto out; + + /* Hardware ignores resume until the suspend request is acknowledged. */ + ret = axi_chan_wait_for_suspend(chan); + if (ret) + goto out; + axi_chan_resume(chan); + if (chan->cyclic_restart) { + chan->cyclic_restart = false; + axi_chan_enable(chan); + } else { + axi_chan_start_first_queued(chan); + } + +out: spin_unlock_irqrestore(&chan->vc.lock, flags); - return 0; + return ret; } static int axi_dma_suspend(struct axi_dma_chip *chip) diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h index d8dd07efe60f..4322f9f89fd7 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -42,6 +42,12 @@ struct axi_dma_chan { void __iomem *chan_regs; u8 id; u8 hw_handshake_num; + bool cyclic; + /* Protected by vc.lock. */ + bool pause_requested:1; + bool is_paused:1; + bool cyclic_restart:1; + atomic_t descs_allocated; struct dma_pool *desc_pool; @@ -50,9 +56,6 @@ struct axi_dma_chan { struct axi_dma_desc *desc; struct dma_slave_config config; enum dma_transfer_direction direction; - bool cyclic; - /* these other elements are all protected by vc.lock */ - bool is_paused; }; struct dw_axi_dma { -- 2.53.0