| Message ID | 20220621064036.147797-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | c1c2a15c2b5379ea8e44dcdcc298e3de42076ba0 |
| Headers | show |
| Series | [v2] gpio: grgpio: Fix device removing | expand |
On Tue, Jun 21, 2022 at 8:40 AM 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 > 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 are unmapped. > > This is a preparation for making platform remove callbacks return void. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Changes since (implicit) v1 > (Message-Id: 20220620122933.106035-1-u.kleine-koenig@pengutronix.de): > > - Fix capitalisation of GPIO and IRQ in commit log > - Add another "are" in the third paragraph. > - Drop note about a potential bug in GPIO framework > I'm not sure there is actually a bug. > > Thanks to Andy Shevchenko for his feedback. > > Best regards > Uwe > > 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 > Applied, thanks! Bart
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[] = {
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 are unmapped. This is a preparation for making platform remove callbacks return void. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Changes since (implicit) v1 (Message-Id: 20220620122933.106035-1-u.kleine-koenig@pengutronix.de): - Fix capitalisation of GPIO and IRQ in commit log - Add another "are" in the third paragraph. - Drop note about a potential bug in GPIO framework I'm not sure there is actually a bug. Thanks to Andy Shevchenko for his feedback. Best regards Uwe drivers/gpio/gpio-grgpio.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56