RE: [PATCH v2] irqchip/renesas-rzg2l: Fix missing put_device

[Fabrizio Castro]

> From: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> Sent: Wednesday, October 9, 2024 11:36 PM
> Subject: [PATCH v2] irqchip/renesas-rzg2l: Fix missing put_device
> 
> rzg2l_irqc_common_init calls of_find_device_by_node, but the
> corresponding put_device call is missing.
> 
> Make sure we call put_device when failing.
> 
> "make coccicheck" will complain about a missing put_device before
> successfully returning from rzv2h_icu_init, however, that's a false

There is a typo in here that deserves a new version.
Instead of rzv2h_icu_init the message should say rzg2l_irqc_common_init.

I'll be sending a new version shortly to fix this issue.

Kind regards,
Fab

> positive.
> 
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> ---
> 
> v1->v2:
> * Drop put_device from the successful path, and add a comment to prevent
>   others from acting upon make coccicheck output.
> 
>  drivers/irqchip/irq-renesas-rzg2l.c | 32 +++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index 693ff285ca2c..040463e3b39c 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -542,33 +542,40 @@ static int rzg2l_irqc_common_init(struct device_node *node, struct device_node *
>  	parent_domain = irq_find_host(parent);
>  	if (!parent_domain) {
>  		dev_err(&pdev->dev, "cannot find parent domain\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto put_dev;
>  	}
> 
>  	rzg2l_irqc_data = devm_kzalloc(&pdev->dev, sizeof(*rzg2l_irqc_data), GFP_KERNEL);
> -	if (!rzg2l_irqc_data)
> -		return -ENOMEM;
> +	if (!rzg2l_irqc_data) {
> +		ret = -ENOMEM;
> +		goto put_dev;
> +	}
> 
>  	rzg2l_irqc_data->irqchip = irq_chip;
> 
>  	rzg2l_irqc_data->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
> -	if (IS_ERR(rzg2l_irqc_data->base))
> -		return PTR_ERR(rzg2l_irqc_data->base);
> +	if (IS_ERR(rzg2l_irqc_data->base)) {
> +		ret = PTR_ERR(rzg2l_irqc_data->base);
> +		goto put_dev;
> +	}
> 
>  	ret = rzg2l_irqc_parse_interrupts(rzg2l_irqc_data, node);
>  	if (ret) {
>  		dev_err(&pdev->dev, "cannot parse interrupts: %d\n", ret);
> -		return ret;
> +		goto put_dev;
>  	}
> 
>  	resetn = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> -	if (IS_ERR(resetn))
> -		return PTR_ERR(resetn);
> +	if (IS_ERR(resetn)) {
> +		ret = PTR_ERR(resetn);
> +		goto put_dev;
> +	}
> 
>  	ret = reset_control_deassert(resetn);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to deassert resetn pin, %d\n", ret);
> -		return ret;
> +		goto put_dev;
>  	}
> 
>  	pm_runtime_enable(&pdev->dev);
> @@ -591,6 +598,10 @@ static int rzg2l_irqc_common_init(struct device_node *node, struct device_node *
> 
>  	register_syscore_ops(&rzg2l_irqc_syscore_ops);
> 
> +	/*
> +	 * coccicheck complains about a missing put_device call before returning, but it's a false
> +	 * positive. We still need &pdev->dev after successfully returning from this function.
> +	 */
>  	return 0;
> 
>  pm_put:
> @@ -598,6 +609,9 @@ static int rzg2l_irqc_common_init(struct device_node *node, struct device_node *
>  pm_disable:
>  	pm_runtime_disable(&pdev->dev);
>  	reset_control_assert(resetn);
> +put_dev:
> +	put_device(&pdev->dev);
> +
>  	return ret;
>  }
> 
> --
> 2.34.1