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 <berthe.ab@xxxxxxxxx>
> >>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
