From mboxrd@z Thu Jan 1 00:00:00 1970 From: greg@kroah.com (Greg KH) Date: Fri, 30 May 2014 16:29:22 -0700 Subject: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void In-Reply-To: <5388CB1B.3090802@metafoo.de> References: <20140530094025.3b78301e@canb.auug.org.au> <1401449454-30895-1-git-send-email-berthe.ab@gmail.com> <1401449454-30895-2-git-send-email-berthe.ab@gmail.com> <5388C0F1.90503@gmail.com> <5388CB1B.3090802@metafoo.de> Message-ID: <20140530232922.GD25854@kroah.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, May 30, 2014 at 08:16:59PM +0200, Lars-Peter Clausen wrote: > On 05/30/2014 07:33 PM, David Daney wrote: > >On 05/30/2014 04:39 AM, Geert Uytterhoeven wrote: > >>On Fri, May 30, 2014 at 1:30 PM, abdoulaye berthe > >>wrote: > >>>--- a/drivers/gpio/gpiolib.c > >>>+++ b/drivers/gpio/gpiolib.c > >>>@@ -1263,10 +1263,9 @@ static void gpiochip_irqchip_remove(struct > >>>gpio_chip *gpiochip); > >>> * > >>> * A gpio_chip with any GPIOs still requested may not be removed. > >>> */ > >>>-int gpiochip_remove(struct gpio_chip *chip) > >>>+void gpiochip_remove(struct gpio_chip *chip) > >>> { > >>> unsigned long flags; > >>>- int status = 0; > >>> unsigned id; > >>> > >>> acpi_gpiochip_remove(chip); > >>>@@ -1278,24 +1277,15 @@ int gpiochip_remove(struct gpio_chip *chip) > >>> of_gpiochip_remove(chip); > >>> > >>> for (id = 0; id < chip->ngpio; id++) { > >>>- if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) { > >>>- status = -EBUSY; > >>>- break; > >>>- } > >>>- } > >>>- if (status == 0) { > >>>- for (id = 0; id < chip->ngpio; id++) > >>>- chip->desc[id].chip = NULL; > >>>- > >>>- list_del(&chip->list); > >>>+ if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) > >>>+ panic("gpio: removing gpiochip with gpios still > >>>requested\n"); > >> > >>panic? > > > >NACK to the patch for this reason. The strongest thing you should do here > >is WARN. > > > >That said, I am not sure why we need this whole patch set in the first place. > > Well, what currently happens when you remove a device that is a provider of > a gpio_chip which is still in use, is that the kernel crashes. Probably with > a rather cryptic error message. So this patch doesn't really change the > behavior, but makes it more explicit what is actually wrong. And even if you > replace the panic() by a WARN() it will again just crash slightly later. > > This is a design flaw in the GPIO subsystem that needs to be fixed. Then fix the GPIO code properly, don't add a new panic() to the kernel. greg k-h