Re: [PATCH V2 3/8] dmaengine: zynqmp_dma: Fix chan probe/remove error handling

[Frank Li]

On Fri, Aug 14, 2026 at 10:26:11AM +0530, Golla Nagendra wrote:
> Keep the real platform_get_irq() error by returning ret directly, stage
> IRQ setup via a local ret variable, and assign chan->irq only after
> devm_request_irq() succeeds.
>
> Initialize chan->irq to -1 and initialize chan->common.device_node
> before it may be touched by teardown. In channel remove, free IRQ only
> for valid IRQ numbers and delete the channel node only when it is linked.
>
> Register the channel node only after successful IRQ setup. On channel
> probe failure in zynqmp_dma_probe(), route cleanup through the existing
> free_chan_resources teardown path.
>
> Fixes: b0cc417c1637 ("dmaengine: Add Xilinx zynqmp dma engine driver support")
> Signed-off-by: Golla Nagendra <nagendra.golla@xxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> changes in v2:
> - Reworked probe failure cleanup to use the existing free_chan_resources teardown path instead of adding direct tasklet kill in the outer probe failure block
> - Kept IRQ/list initialization and channel registration ordering fixes in this patch
> ---
>  drivers/dma/xilinx/zynqmp_dma.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index b7c561280694..22b517c57003 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -205,7 +205,7 @@ struct zynqmp_dma_desc_sw {
>   * @desc_pool_p: Physical allocated descriptor base
>   * @desc_free_cnt: Descriptor available count
>   * @dev: The dma device
> - * @irq: Channel IRQ
> + * @irq: Linux IRQ number, or -1 when not registered
>   * @is_dmacoherent: Tells whether dma operations are coherent or not
>   * @tasklet: Cleanup work after irq
>   * @idle : Channel status;
> @@ -896,10 +896,11 @@ static void zynqmp_dma_chan_remove(struct zynqmp_dma_chan *chan)
>  	if (!chan)
>  		return;
>
> -	if (chan->irq)
> +	if (chan->irq >= 0)
>  		devm_free_irq(chan->zdev->dev, chan->irq, chan);
>  	tasklet_kill(&chan->tasklet);
> -	list_del(&chan->common.device_node);
> +	if (!list_empty(&chan->common.device_node))
> +		list_del(&chan->common.device_node);
>  }
>
>  /**
> @@ -915,13 +916,14 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
>  	struct zynqmp_dma_chan *chan;
>  	struct device_node *node = pdev->dev.of_node;
>  	const struct zynqmp_dma_config *match_data;
> -	int err;
> +	int err, ret;
>
>  	chan = devm_kzalloc(zdev->dev, sizeof(*chan), GFP_KERNEL);
>  	if (!chan)
>  		return -ENOMEM;
>  	chan->dev = zdev->dev;
>  	chan->zdev = zdev;
> +	chan->irq = -1;
>
>  	chan->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(chan->regs))
> @@ -954,22 +956,26 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
>  	INIT_LIST_HEAD(&chan->pending_list);
>  	INIT_LIST_HEAD(&chan->done_list);
>  	INIT_LIST_HEAD(&chan->free_list);
> +	INIT_LIST_HEAD(&chan->common.device_node);
>
>  	dma_cookie_init(&chan->common);
>  	chan->common.device = &zdev->common;
> -	list_add_tail(&chan->common.device_node, &zdev->common.channels);
> -
>  	zynqmp_dma_init(chan);
> -	chan->irq = platform_get_irq(pdev, 0);
> -	if (chan->irq < 0)
> -		return -ENXIO;
> -	err = devm_request_irq(&pdev->dev, chan->irq, zynqmp_dma_irq_handler, 0,
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	err = devm_request_irq(&pdev->dev, ret, zynqmp_dma_irq_handler, 0,
>  			       "zynqmp-dma", chan);
>  	if (err)
>  		return err;
>
> +	chan->irq = ret;
> +
>  	chan->desc_size = sizeof(struct zynqmp_dma_desc_ll);
>  	chan->idle = true;
> +	list_add_tail(&chan->common.device_node, &zdev->common.channels);
> +
>  	return 0;
>  }
>
> @@ -1134,7 +1140,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>  	ret = zynqmp_dma_chan_probe(zdev, pdev);
>  	if (ret) {
>  		dev_err_probe(&pdev->dev, ret, "Probing channel failed\n");
> -		goto err_disable_pm;
> +		goto free_chan_resources;
>  	}
>
>  	p->dst_addr_widths = BIT(zdev->chan->bus_width / 8);
> --
> 2.44.4
>