Re: [PATCH v2 3/3] dmaengine: dw-axi-dmac: preserve pending pause requests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dmaengine: dw-axi-dmac: cyclic completion skips suspend
  cleanup on pause
--

commit 2085e97ef4b5e2caaf2c3443bb6eed8c9823633f
Author: James Hilliard <james.hilliard1@gmail.com>

dmaengine: dw-axi-dmac: preserve pending pause requests

This commit tracks pending DMA channel pause requests separately from
hardware suspend acknowledgment, prevents the shared IRQ handler from
clearing DWAXIDMAC_IRQ_SUSPENDED, and defers starting or restarting
transfers while a pause is pending.

> 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 281d7c79246e3..5c1eb120a28b1 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -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 {

[Severity: Medium]
Can setting chan->is_paused = true here leave an unacknowledged hardware
suspend request latched across dma_chan_resume()?

If dma_chan_pause() asserts the hardware suspend bit and times out waiting
for acknowledgment, and the active cyclic block finishes in hardware before
the caller retries pause or calls dma_chan_resume(), hardware clears the
channel enable bit without necessarily setting DWAXIDMAC_IRQ_SUSPENDED.
axi_chan_block_xfer_complete() then sets chan->is_paused = true without
calling axi_chan_disable(chan) or clearing DWAXIDMAC_IRQ_SUSPENDED.

When dma_chan_resume() later calls axi_chan_wait_for_suspend(), it sees
chan->is_paused == true and returns 0 immediately, skipping
axi_chan_is_suspended() (which calls axi_chan_disable(chan) to abort an
unacknowledged suspend on a completed transfer) and skipping
axi_chan_irq_clear(chan, DWAXIDMAC_IRQ_SUSPENDED).  Because hardware
ignores clearing the suspend bit in axi_chan_resume() while an
unacknowledged request is still pending, re-enabling the channel for
chan->cyclic_restart can leave the suspend request active in hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-submit-dw-axi-dma-stop-pause-v1-v2-0-64adc48df793@gmail.com?part=3