| Message ID | 20241010164155.808931-1-fabrizio.castro.jz@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v3] irqchip/renesas-rzg2l: Fix missing put_device | expand |
> 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 rzg2l_irqc_common_init, however, that's > a false positive. > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") Might the application of scope-based resource management become more interesting accordingly? Regards, Markus
Hi Markus, On Thu, Oct 10, 2024 at 7:53 PM Markus Elfring <Markus.Elfring@web.de> wrote: > > 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 rzg2l_irqc_common_init, however, that's > > a false positive. > > > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") > > Might the application of scope-based resource management become more > interesting accordingly? No, as explained in the comments: > > + /* > > + * 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. > > + */ Gr{oetje,eeting}s, Geert
>>> 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 rzg2l_irqc_common_init, however, that's >>> a false positive. >>> >>> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") >> >> Might the application of scope-based resource management become more >> interesting accordingly? > > No, as explained in the comments: > >>> + /* >>> + * 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. >>> + */ Please reconsider the applicability of mentioned programming interfaces once more (as an evolving software design alternative for goto chains). https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/device.h#L1239 Regards, Markus
Hi Markus, thanks for your feedback. > From: Markus Elfring <Markus.Elfring@web.de> > Sent: Thursday, October 10, 2024 7:18 PM > Subject: Re: [PATCH v3] 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 rzg2l_irqc_common_init, however, that's > >>> a false positive. > >>> > >>> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") > >> > >> Might the application of scope-based resource management become more > >> interesting accordingly? > > > > No, as explained in the comments: > > > >>> + /* > >>> + * 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. > >>> + */ > > Please reconsider the applicability of mentioned programming interfaces once more > (as an evolving software design alternative for goto chains). > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/device.h#L1239 I can give it a shot. I'll send a new version shortly. Kind regards, Fab > > Regards, > Markus
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; }
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 rzg2l_irqc_common_init, however, that's a false positive. Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com> --- v2->v3: * Fixed typo in commit log (rzv2h_icu_init replaced with rzg2l_irqc_common_init). v1->v2: * Dropped put_device from the successful path, and added 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(-)