Re: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Returning ERR_PTR(err) from d40_queue_start() introduces a PM runtime reference leak and breaks caller logic in dma_tc_handle().
- [High] Freeing a submitted descriptor directly on error bypasses the DMA framework, causing the client to hang.
--

--- Patch [1]: [PATCH 1/4] dmaengine: ste_dma40: Fix failed start cleanup ---
commit 0baedf8f0f8ca2b28e1d6d6a6bce21f2db82302b
Author: Linus Walleij <[email protected]>
Date:   Wed Aug 19 22:35:57 2026 +0000

dmaengine: ste_dma40: Fix failed start cleanup

If d40_start() fails after a queued descriptor has been moved to the active
list, d40_queue_start() currently returns NULL without unwinding the
descriptor state or clearing the channel busy flag. This commit attempts
to fix the error path by freeing the descriptor and returning an error pointer.

Link: https://lore.kernel.org/dmaengine/[email protected]/

> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1531,8 +1531,12 @@ 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_free(d40c, d40d);

[Severity: High]
Does silently freeing the submitted descriptor here cause the DMA client to
hang?

The descriptor has already been submitted to the DMA framework, which means the
client holds a cookie and expects a completion callback. Bypassing the
framework and freeing the memory directly prevents the framework from completing
the cookie, meaning the client will wait indefinitely for a callback that will
never arrive.

> +			d40c->busy = false;
> +			return ERR_PTR(err);

[Severity: High]
Does returning ERR_PTR(err) here introduce a PM runtime reference leak?

Earlier in d40_queue_start(), a PM runtime reference is acquired:

	if (d40d != NULL) {
		if (!d40c->busy) {
			d40c->busy = true;
			pm_runtime_get_sync(d40c->base->dev);
		}

This error path returns ERR_PTR(err) and sets busy to false without dropping
that newly acquired reference.

Additionally, changing the return value breaks the caller logic in
dma_tc_handle(), which explicitly checks for a NULL return to drop the PM
reference:

drivers/dma/ste_dma40.c:dma_tc_handle() {
    ...
		if (d40_queue_start(d40c) == NULL) {
			d40c->busy = false;
			pm_runtime_put_autosuspend(d40c->base->dev);
		}
    ...
}

Because ERR_PTR(err) is not NULL, this condition evaluates to false, skipping
pm_runtime_put_autosuspend() and leaking the reference.

> +		}
>  	}
>  
>  	return d40d;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1