Re: [PATCH v5 04/23] dmaengine: ste_dma40: Fix failed start cleanup

[Frank Li <Frank.li@oss.nxp.com>]

On Sun, Sep 20, 2026 at 08:59:39PM +0200, Linus Walleij wrote:
> If d40_start() fails after a queued descriptor has been moved to the
> active list, d40_queue_start() currently returns NULL without unwinding
> the transfer state or clearing the channel busy flag.
>
> Fix this pre-existing error path by completing the descriptor through the
> normal tasklet path, clearing the busy flag, balancing the runtime PM
> reference and returning an error pointer to distinguish the failure from
> the no-work case. Do not free the descriptor directly, since it has
> already been submitted and has a DMA cookie.
>
> When starting the next queued transfer from the completion handler, put
> the completed descriptor on the done list first. This preserves FIFO
> completion order if the new transfer fails to start and prevents the
> completed cookie from moving backwards.
>
> Use done-list membership rather than the cyclic flag to identify terminal
> descriptors in the tasklet. A cyclic descriptor that failed to start is
> then completed and removed instead of remaining permanently at the head
> of the done list.
>
> Fixes: 7d83a854a1a4 ("dma40: remove "hardware link with previous jobs" code")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/dmaengine/20260618051539.15E201F000E9@smtp.kernel.org/
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/ste_dma40.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 1f9e8c7249b1..280a0d2f16c5 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1650,8 +1650,15 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
>  		/* Start dma job */
>  		err = d40_start(d40c);
>
> -		if (err)
> -			return NULL;
> +		if (err) {
> +			d40_desc_remove(d40d);
> +			d40_desc_done(d40c, d40d);
> +			d40c->pending_tx++;
> +			d40c->busy = false;
> +			pm_runtime_put_autosuspend(d40c->base->dev);
> +			tasklet_schedule(&d40c->tasklet);
> +			return ERR_PTR(err);
> +		}
>  	}
>
>  	return d40d;
> @@ -1698,14 +1705,14 @@ static void dma_tc_handle(struct d40_chan *d40c)
>  			return;
>  		}
>
> +		d40_desc_remove(d40d);
> +		d40_desc_done(d40c, d40d);
> +
>  		if (d40_queue_start(d40c) == NULL) {
>  			d40c->busy = false;
>
>  			pm_runtime_put_autosuspend(d40c->base->dev);
>  		}
> -
> -		d40_desc_remove(d40d);
> -		d40_desc_done(d40c, d40d);
>  	}
>
>  	d40c->pending_tx += callbacks;
> @@ -1719,20 +1726,22 @@ static void dma_tasklet(struct tasklet_struct *t)
>  	struct d40_desc *d40d;
>  	unsigned long flags;
>  	bool callback_active;
> +	bool from_done;
>  	struct dmaengine_desc_callback cb;
>
>  	spin_lock_irqsave(&d40c->lock, flags);
>
>  	/* Get first entry from the done list */
>  	d40d = d40_first_done(d40c);
> -	if (d40d == NULL) {
> +	from_done = !!d40d;
> +	if (!from_done) {
>  		/* Check if we have reached here for cyclic job */
>  		d40d = d40_first_active_get(d40c);
>  		if (d40d == NULL || !d40d->cyclic)
>  			goto check_pending_tx;
>  	}
>
> -	if (!d40d->cyclic)
> +	if (from_done)
>  		dma_cookie_complete(&d40d->txd);
>
>  	/*
> @@ -1748,7 +1757,7 @@ static void dma_tasklet(struct tasklet_struct *t)
>  	callback_active = !!(d40d->txd.flags & DMA_PREP_INTERRUPT);
>  	dmaengine_desc_get_callback(&d40d->txd, &cb);
>
> -	if (!d40d->cyclic) {
> +	if (from_done) {
>  		if (async_tx_test_ack(&d40d->txd)) {
>  			d40_desc_remove(d40d);
>  			d40_desc_free(d40c, d40d);
>
> --
> 2.55.0
>