Re: [PATCH v6 11/23] dmaengine: ste_dma40: Fix DMA registration unwind

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

On Thu, Sep 24, 2026 at 10:35:23AM +0200, Linus Walleij wrote:
> d40_dmaengine_init() registers DMAengine devices using devres-managed
> unregister actions. If probe fails after one of those registrations, the
> DMAengine devices stay visible until devres unwinds after d40_probe()
> returns.
>
> The channel tasklets are also initialized before each DMAengine device is
> registered. An interrupt can therefore have queued a tasklet by the time a
> later registration step fails, but unregistering the DMAengine devices
> does not drain that tasklet before probe-owned storage is released.
>
> Add tasklet cleanup actions to the DMAengine registration devres group.
> On failure, free the IRQ before releasing the group so no new tasklets can
> be scheduled, then unregister the DMAengine devices and drain their
> tasklets before freeing the remaining probe resources. Keep the managed
> registrations and cleanup actions in place on successful probe by removing
> only the temporary group markers.
>
> Fixes: 42ae6f1695be ("dmaengine: ste_dma40: Remove platform data")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/dmaengine/20260819225215.5EA2F1F000E9@smtp.kernel.org/
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/dma/ste_dma40.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 36f4cbd4da36..87b5f1d97ac2 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -3042,6 +3042,18 @@ static void __init d40_chan_init(struct d40_base *base, struct dma_device *dma,
>  	}
>  }
>
> +static void d40_kill_tasklets(void *data)
> +{
> +	struct dma_device *dma = data;
> +	struct d40_chan *d40c;
> +	struct dma_chan *chan;
> +
> +	list_for_each_entry(chan, &dma->channels, device_node) {
> +		d40c = container_of(chan, struct d40_chan, chan);
> +		tasklet_kill(&d40c->tasklet);
> +	}
> +}
> +
>  static void d40_ops_init(struct d40_base *base, struct dma_device *dev)
>  {
>  	if (dma_has_cap(DMA_SLAVE, dev->cap_mask)) {
> @@ -3088,6 +3100,11 @@ static int __init d40_dmaengine_init(struct d40_base *base,
>
>  	d40_ops_init(base, &base->dma_slave);
>
> +	err = devm_add_action_or_reset(base->dev, d40_kill_tasklets,
> +				       &base->dma_slave);
> +	if (err)
> +		goto exit;
> +
>  	err = dmaenginem_async_device_register(&base->dma_slave);
>
>  	if (err) {
> @@ -3103,6 +3120,11 @@ static int __init d40_dmaengine_init(struct d40_base *base,
>
>  	d40_ops_init(base, &base->dma_memcpy);
>
> +	err = devm_add_action_or_reset(base->dev, d40_kill_tasklets,
> +				       &base->dma_memcpy);
> +	if (err)
> +		goto exit;
> +
>  	err = dmaenginem_async_device_register(&base->dma_memcpy);
>
>  	if (err) {
> @@ -3120,6 +3142,12 @@ static int __init d40_dmaengine_init(struct d40_base *base,
>  	dma_cap_set(DMA_CYCLIC, base->dma_both.cap_mask);
>
>  	d40_ops_init(base, &base->dma_both);
> +
> +	err = devm_add_action_or_reset(base->dev, d40_kill_tasklets,
> +				       &base->dma_both);
> +	if (err)
> +		goto exit;
> +
>  	err = dmaenginem_async_device_register(&base->dma_both);
>
>  	if (err) {
> @@ -3717,6 +3745,7 @@ static int __init d40_probe(struct platform_device *pdev)
>  	struct d40_base *base;
>  	struct resource *res;
>  	struct resource res_lcpa;
> +	void *dmaenginem_reg_group;
>  	int num_reserved_chans;
>  	bool irq_requested = false;
>  	u32 val;
> @@ -3852,20 +3881,33 @@ static int __init d40_probe(struct platform_device *pdev)
>  	d40_hw_init(base);
>  	enable_irq(base->irq);
>
> +	dmaenginem_reg_group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!dmaenginem_reg_group) {
> +		ret = -ENOMEM;
> +		goto destroy_cache;
> +	}
> +
>  	ret = d40_dmaengine_init(base, num_reserved_chans);
>  	if (ret)
> -		goto destroy_cache;
> +		goto release_dmaenginem;
>
>  	ret = of_dma_controller_register(np, d40_xlate, NULL);

if use devm_of_dma_controller_register(), does it avoid use devres group?

Frank

>  	if (ret) {
>  		dev_err(dev,
>  			"could not register of_dma_controller\n");
> -		goto destroy_cache;
> +		goto release_dmaenginem;
>  	}
> +	devres_remove_group(dev, dmaenginem_reg_group);
>
>  	dev_info(base->dev, "initialized\n");
>  	return 0;
>
> + release_dmaenginem:
> +	if (irq_requested) {
> +		free_irq(base->irq, base);
> +		irq_requested = false;
> +	}
> +	devres_release_group(dev, dmaenginem_reg_group);
>   destroy_cache:
>  	if (base->lcla_pool.dma_addr)
>  		dma_unmap_single(base->dev, base->lcla_pool.dma_addr,
>
> --
> 2.55.0
>