gpio: mvebu: Make use of devm_pwmchip_add

This allows to get rid of a call to pwmchip_remove() in the error path. There
is no .remove function for this driver, so this change fixes a resource leak
when a gpio-mvebu device is unbound.

Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

Note that irq_domain_remove() also isn't called so there is another
resource leak introduced by 812d47889a8e ("gpio/mvebu: Use
irq_domain_add_linear")

 drivers/gpio/gpio-mvebu.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)


base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5

On Mon, Jul 17, 2023 at 04:27:43PM +0200, Uwe Kleine-König wrote:
> This allows to get rid of a call to pwmchip_remove() in the error path. There
> is no .remove function for this driver, so this change fixes a resource leak
> when a gpio-mvebu device is unbound.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

On Mon, Jul 17, 2023 at 04:27:43PM +0200, Uwe Kleine-König wrote:

> Note that irq_domain_remove() also isn't called so there is another
> resource leak introduced by 812d47889a8e ("gpio/mvebu: Use
> irq_domain_add_linear")

I believe it's only theoretical issue as quite likely GPIO will be used and
hence one can't unbound it. Or unbound is unconditional? In that case we
have much bigger issue in the kernel.

On Mon, Jul 17, 2023 at 05:40:11PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 17, 2023 at 04:27:43PM +0200, Uwe Kleine-König wrote:
> 
> > Note that irq_domain_remove() also isn't called so there is another
> > resource leak introduced by 812d47889a8e ("gpio/mvebu: Use
> > irq_domain_add_linear")
> 
> I believe it's only theoretical issue as quite likely GPIO will be used and
> hence one can't unbound it. Or unbound is unconditional? In that case we
> have much bigger issue in the kernel.

You might want to test

	echo f1018100.gpio > /sys/bus/platform/drivers/mvebu-gpio/unbind

(where f1018100.gpio is the name of one of links in
/sys/bus/platform/drivers/mvebu-gpio#) and if that succeeds you should
consider

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index a68f682aec01..620ab247abff 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -1306,6 +1306,7 @@ static struct platform_driver mvebu_gpio_driver = {
 	.driver		= {
 		.name		= "mvebu-gpio",
 		.of_match_table = mvebu_gpio_of_match,
+		.suppress_bind_attrs = true,
 	},
 	.probe		= mvebu_gpio_probe,
 	.suspend        = mvebu_gpio_suspend,

which prevents that path and so the leak (by not providing
/sys/bus/platform/drivers/mvebu-gpio/unbind).

Best regards
Uwe

On Mon, Jul 17, 2023 at 4:27 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> This allows to get rid of a call to pwmchip_remove() in the error path. There
> is no .remove function for this driver, so this change fixes a resource leak
> when a gpio-mvebu device is unbound.
>
> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> Note that irq_domain_remove() also isn't called so there is another
> resource leak introduced by 812d47889a8e ("gpio/mvebu: Use
> irq_domain_add_linear")
>
>  drivers/gpio/gpio-mvebu.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index a68f682aec01..a35958e7adf6 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -874,7 +874,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>
>         spin_lock_init(&mvpwm->lock);
>
> -       return pwmchip_add(&mvpwm->chip);
> +       return devm_pwmchip_add(dev, &mvpwm->chip);
>  }
>
>  #ifdef CONFIG_DEBUG_FS
> @@ -1243,8 +1243,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>         if (!mvchip->domain) {
>                 dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n",
>                         mvchip->chip.label);
> -               err = -ENODEV;
> -               goto err_pwm;
> +               return -ENODEV;
>         }
>
>         err = irq_alloc_domain_generic_chips(
> @@ -1296,9 +1295,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>
>  err_domain:
>         irq_domain_remove(mvchip->domain);
> -err_pwm:
> -       pwmchip_remove(&mvchip->mvpwm->chip);
> -
>         return err;
>  }
>
>
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> --
> 2.39.2
>

Applied, thanks!

Bart