* [PATCH] gpio: grgpio: Fix device removing
@ 2022-06-20 12:29 Uwe Kleine-König
2022-06-20 16:13 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Uwe Kleine-König @ 2022-06-20 12:29 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski; +Cc: linux-gpio, kernel
If a platform device's remove callback returns non-zero, the device core
emits a warning and still removes the device and calls the devm cleanup
callbacks.
So it's not save to not unregister the gpiochip because on the next request
to a gpio the driver accesses kfree()'d memory. Also if an irq triggers,
the freed memory is accessed.
Instead rely on the gpio framework to ensure that after gpiochip_remove()
all gpios are freed and so the corresponding irqs unmapped. (I'm think the
gpio framework doesn't guarantee that, but that's a bug there and out of
scope for this gpio driver to fix that.)
This is a preparation for making platform remove callbacks return void.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/gpio/gpio-grgpio.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
index df563616f943..bea0e32c195d 100644
--- a/drivers/gpio/gpio-grgpio.c
+++ b/drivers/gpio/gpio-grgpio.c
@@ -434,25 +434,13 @@ static int grgpio_probe(struct platform_device *ofdev)
static int grgpio_remove(struct platform_device *ofdev)
{
struct grgpio_priv *priv = platform_get_drvdata(ofdev);
- int i;
- int ret = 0;
-
- if (priv->domain) {
- for (i = 0; i < GRGPIO_MAX_NGPIO; i++) {
- if (priv->uirqs[i].refcnt != 0) {
- ret = -EBUSY;
- goto out;
- }
- }
- }
gpiochip_remove(&priv->gc);
if (priv->domain)
irq_domain_remove(priv->domain);
-out:
- return ret;
+ return 0;
}
static const struct of_device_id grgpio_match[] = {
base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
--
2.36.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: grgpio: Fix device removing
2022-06-20 12:29 [PATCH] gpio: grgpio: Fix device removing Uwe Kleine-König
@ 2022-06-20 16:13 ` Andy Shevchenko
2022-06-20 17:03 ` Uwe Kleine-König
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2022-06-20 16:13 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
Sascha Hauer
On Mon, Jun 20, 2022 at 2:33 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> If a platform device's remove callback returns non-zero, the device core
> emits a warning and still removes the device and calls the devm cleanup
> callbacks.
>
> So it's not save to not unregister the gpiochip because on the next request
safe
> to a gpio the driver accesses kfree()'d memory. Also if an irq triggers,
GPIO
IRQ
> the freed memory is accessed.
>
> Instead rely on the gpio framework to ensure that after gpiochip_remove()
GPIO
> all gpios are freed and so the corresponding irqs unmapped. (I'm think the
GPIOs
IRQs
are unmapped
I think
> gpio framework doesn't guarantee that, but that's a bug there and out of
GPIO
> scope for this gpio driver to fix that.)
GPIO
> This is a preparation for making platform remove callbacks return void.
...
What a bug are you seeing in the GPIO library? IIRC for IRQ over GPIO
the GPIO holds the module reference count as well as GPIO device
reference count. Am I wrong?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: grgpio: Fix device removing
2022-06-20 16:13 ` Andy Shevchenko
@ 2022-06-20 17:03 ` Uwe Kleine-König
0 siblings, 0 replies; 3+ messages in thread
From: Uwe Kleine-König @ 2022-06-20 17:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Bartosz Golaszewski,
Sascha Hauer
[-- Attachment #1: Type: text/plain, Size: 1734 bytes --]
Hello Andy,
On Mon, Jun 20, 2022 at 06:13:21PM +0200, Andy Shevchenko wrote:
> On Mon, Jun 20, 2022 at 2:33 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > If a platform device's remove callback returns non-zero, the device core
> > emits a warning and still removes the device and calls the devm cleanup
> > callbacks.
> >
> > So it's not save to not unregister the gpiochip because on the next request
>
> safe
>
> > to a gpio the driver accesses kfree()'d memory. Also if an irq triggers,
>
> GPIO
> IRQ
>
> > the freed memory is accessed.
> >
> > Instead rely on the gpio framework to ensure that after gpiochip_remove()
>
> GPIO
>
> > all gpios are freed and so the corresponding irqs unmapped. (I'm think the
>
> GPIOs
> IRQs
> are unmapped
>
> I think
>
> > gpio framework doesn't guarantee that, but that's a bug there and out of
>
> GPIO
>
> > scope for this gpio driver to fix that.)
>
> GPIO
>
> > This is a preparation for making platform remove callbacks return void.
>
>
> ...
>
> What a bug are you seeing in the GPIO library? IIRC for IRQ over GPIO
> the GPIO holds the module reference count as well as GPIO device
> reference count. Am I wrong?
I don't see a bug, I just looked into gpiochip_remove() to check if it
ensures that all gpios go away and at one point is issues
dev_crit(&gdev->dev,
"REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n");
and so I assumed that there is an issue, but maybe this isn't reachable
in practise?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-20 17:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-20 12:29 [PATCH] gpio: grgpio: Fix device removing Uwe Kleine-König
2022-06-20 16:13 ` Andy Shevchenko
2022-06-20 17:03 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.