[PATCH] pinctrl: pinctrl-pic32: Fix resource leaks

[PATCH] pinctrl: pinctrl-pic32: Fix resource leaks

[Ethan Tidmore]

Add three missing clk_disable_unprepare() in early return error paths.

Detected by Smatch:
drivers/pinctrl/pinctrl-pic32.c:2211 pic32_pinctrl_probe() warn:
'pctl->clk' from clk_prepare_enable() not released on lines: 2208.

drivers/pinctrl/pinctrl-pic32.c:2274 pic32_gpio_probe() warn:
'bank->clk' from clk_prepare_enable() not released on lines: 2264,2272.

Fixes: 2ba384e6c3810 ("pinctrl: pinctrl-pic32: Add PIC32 pin control driver")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/pinctrl/pinctrl-pic32.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-pic32.c b/drivers/pinctrl/pinctrl-pic32.c
index eb438c9d9667..cf098c183b14 100644
--- a/drivers/pinctrl/pinctrl-pic32.c
+++ b/drivers/pinctrl/pinctrl-pic32.c
@@ -2204,6 +2204,7 @@ static int pic32_pinctrl_probe(struct platform_device *pdev)
 	pctl->pctldev = devm_pinctrl_register(&pdev->dev, &pic32_pinctrl_desc,
 					      pctl);
 	if (IS_ERR(pctl->pctldev)) {
+		clk_disable_unprepare(pctl->clk);
 		dev_err(&pdev->dev, "Failed to register pinctrl device\n");
 		return PTR_ERR(pctl->pctldev);
 	}
@@ -2260,8 +2261,10 @@ static int pic32_gpio_probe(struct platform_device *pdev)
 	girq->num_parents = 1;
 	girq->parents = devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents),
 				     GFP_KERNEL);
-	if (!girq->parents)
-		return -ENOMEM;
+	if (!girq->parents) {
+		ret = -ENOMEM;
+		goto err_clk_cleanup;
+	}
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_level_irq;
 	girq->parents[0] = irq;
@@ -2269,9 +2272,13 @@ static int pic32_gpio_probe(struct platform_device *pdev)
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to add GPIO chip %u: %d\n",
 			id, ret);
-		return ret;
+		goto err_clk_cleanup;
 	}
 	return 0;
+
+err_clk_cleanup:
+	clk_disable_unprepare(bank->clk);
+	return ret;
 }
 
 static const struct of_device_id pic32_pinctrl_of_match[] = {
-- 
2.53.0

Re: [PATCH] pinctrl: pinctrl-pic32: Fix resource leaks

[Linus Walleij]

Hi Ethan,

thanks for your patch!

On Tue, Feb 24, 2026 at 9:29 PM Ethan Tidmore <ethantidmore06@gmail.com> wrote:

> Add three missing clk_disable_unprepare() in early return error paths.
>
> Detected by Smatch:
> drivers/pinctrl/pinctrl-pic32.c:2211 pic32_pinctrl_probe() warn:
> 'pctl->clk' from clk_prepare_enable() not released on lines: 2208.
>
> drivers/pinctrl/pinctrl-pic32.c:2274 pic32_gpio_probe() warn:
> 'bank->clk' from clk_prepare_enable() not released on lines: 2264,2272.
>
> Fixes: 2ba384e6c3810 ("pinctrl: pinctrl-pic32: Add PIC32 pin control driver")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
(...)
>         if (IS_ERR(pctl->pctldev)) {
> +               clk_disable_unprepare(pctl->clk);
(...)
> +               goto err_clk_cleanup;
(...)
> +               goto err_clk_cleanup;
> +err_clk_cleanup:
> +       clk_disable_unprepare(bank->clk);
> +       return ret;

Can't you just use devm_clk_get_enabled() and let devres do this?

Yours,
Linus Walleij

Re: [PATCH] pinctrl: pinctrl-pic32: Fix resource leaks

[Ethan Tidmore]

On Thu Feb 26, 2026 at 4:37 PM CST, Linus Walleij wrote:

...

> Can't you just use devm_clk_get_enabled() and let devres do this?

I thought about that but wasn't sure because I saw:

	ret = gpiochip_add_data(&bank->gpio_chip, bank);

Later in the function and knew that you're not really suppose to mix
manual resource allocation and devres.

Thanks,

ET

Re: [PATCH] pinctrl: pinctrl-pic32: Fix resource leaks

[Linus Walleij]

On Fri, Feb 27, 2026 at 10:01 PM Ethan Tidmore <ethantidmore06@gmail.com> wrote:
> On Thu Feb 26, 2026 at 4:37 PM CST, Linus Walleij wrote:
>
> > Can't you just use devm_clk_get_enabled() and let devres do this?
>
> I thought about that but wasn't sure because I saw:
>
>         ret = gpiochip_add_data(&bank->gpio_chip, bank);
>
> Later in the function and knew that you're not really suppose to mix
> manual resource allocation and devres.

And there is a bunch of other devm_* stuff before it so it's
confusing isn't it? A mix however you put it.

In this case that is just because the gpiochip_add_data() happens
last in probe().

If you study the driver you see it does not have a .remove() function
and you can bet your life no-one is manually testing to remove it,
so all the devm_* stuff is just there for exititing a failed or deferred
probe.

When the probe reaches gpiochip_add_data() that is the last
thing that can fail, and if it fails there are just all the other
resources that need to be free:ed.

I'm all for changing that to devm_gpiochip_add_data()
for completion but semanically it  won't matter unless someone
goes in and unbinds the driver in sysfs.

So use devm_clk_get_enabled() and optionally convert
to devm_gpiochip_add_data() as well.

Yours,
Linus Walleij