Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
Hi abdoulaye, On Fri, 30 May 2014 00:32:02 +0200 abdoulaye berthe wrote: > > + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) > + panic("gpio: removing gpiochip with gpios still > requested\n"); Someone else already commented that panic is probably overkill ... is there no way to log this and continue? -- Cheers, Stephen Rothwells...@canb.auug.org.au signature.asc Description: PGP signature
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..022543f 100644 --- 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; unsignedid; 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, >desc[id].flags)) { - status = -EBUSY; - break; - } - } - if (status == 0) { - for (id = 0; id < chip->ngpio; id++) - chip->desc[id].chip = NULL; - - list_del(>list); + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) + panic("gpio: removing gpiochip with gpios still requested\n"); } + for (id = 0; id < chip->ngpio; id++) + chip->desc[id].chip = NULL; + list_del(>list); spin_unlock_irqrestore(_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..022543f 100644 --- 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; unsignedid; 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); } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
Hi abdoulaye, On Fri, 30 May 2014 00:32:02 +0200 abdoulaye berthe berthe...@gmail.com wrote: + if (test_bit(FLAG_REQUESTED, chip-desc[id].flags)) + panic(gpio: removing gpiochip with gpios still requested\n); Someone else already commented that panic is probably overkill ... is there no way to log this and continue? -- Cheers, Stephen Rothwells...@canb.auug.org.au signature.asc Description: PGP signature
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Fri, May 23, 2014 at 7:03 PM, abdoulaye berthe wrote: > This avoids handling gpiochip remove error in device > remove handler. > > Signed-off-by: abdoulaye berthe In general this is the right thing to do. > for (id = 0; id < chip->ngpio; id++) { > - if (test_bit(FLAG_REQUESTED, >desc[id].flags)) { > - status = -EBUSY; > - break; > - } > - } > - if (status == 0) { > - for (id = 0; id < chip->ngpio; id++) > - chip->desc[id].chip = NULL; > - > - list_del(>list); > + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) > + panic("gpiolib.c: gpiochip is still requested\n"); panic("gpio: removing gpiochip with gpios still requested\"); is more helpful. But I want a patch that also removes *ALL* users of the return value from this function in the *same* patch to avoid bisectability issues. $ git grep gpiochip_remove Then fix all of them or use coccinelle. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Fri, May 23, 2014 at 7:03 PM, abdoulaye berthe berthe...@gmail.com wrote: This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe berthe...@gmail.com In general this is the right thing to do. 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(gpiolib.c: gpiochip is still requested\n); panic(gpio: removing gpiochip with gpios still requested\); is more helpful. But I want a patch that also removes *ALL* users of the return value from this function in the *same* patch to avoid bisectability issues. $ git grep gpiochip_remove Then fix all of them or use coccinelle. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Mon, May 26, 2014 at 1:40 AM, abdoulaye berthe wrote: > Well, ignoring the return value as it is done in gpio-bt8xx makes the > compiler complain and display a warning message. The problem with > false warning is that it might distract you. Isn't the warning due to the __must_check in the function's declaration? If so just removing it might do the trick... > I think that the patch > will makes things consistent once completed Yeah fundamentally speaking I am not against this patch - I just wonder if it is worth going through all the call sites and changing them. Also we cannot exclude that a few users actually make something meaningful with the return value (don't know what that would be though). > Thanks a lot for the review. > > On Sun, May 25, 2014 at 9:46 AM, Alexandre Courbot wrote: >> On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe >> wrote: >>> This avoids handling gpiochip remove error in device >>> remove handler. >> >> Be aware that at the moment many callers of gpiochip_remove() read its >> return value. So applying your patch as-is would break compilation. >> >> This patch should therefore be the last of a series that first >> modifies all callers of gpiochip_remove() to ignore its return value, >> then neutralizes the function itself. >> >> I am not sure whether the world would really be a better place after >> this though. Callers that don't need the return value of >> gpiochip_remove() can simply ignore it... >> >>> >>> Signed-off-by: abdoulaye berthe >>> --- >>> drivers/gpio/gpiolib.c | 24 +++- >>> include/linux/gpio/driver.h | 2 +- >>> 2 files changed, 8 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c >>> index f48817d..4878980 100644 >>> --- 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; >>> unsignedid; >>> >>> 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, >desc[id].flags)) { >>> - status = -EBUSY; >>> - break; >>> - } >>> - } >>> - if (status == 0) { >>> - for (id = 0; id < chip->ngpio; id++) >>> - chip->desc[id].chip = NULL; >>> - >>> - list_del(>list); >>> + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) >>> + panic("gpiolib.c: gpiochip is still requested\n"); >> >> panic() sounds a little harsh here. Maybe a dev_err() would be enough? >> >>> } >>> + for (id = 0; id < chip->ngpio; id++) >>> + chip->desc[id].chip = NULL; >>> >>> + list_del(>list); >>> spin_unlock_irqrestore(_lock, flags); >>> - >>> - if (status == 0) >>> - gpiochip_unexport(chip); >>> - >>> - return status; >>> + gpiochip_unexport(chip); >>> } >>> EXPORT_SYMBOL_GPL(gpiochip_remove); >>> >>> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h >>> index 1827b43..72ed256 100644 >>> --- a/include/linux/gpio/driver.h >>> +++ b/include/linux/gpio/driver.h >>> @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct >>> gpio_chip *chip, >>> >>> /* add/remove chips */ >>> extern int gpiochip_add(struct gpio_chip *chip); >>> -extern int __must_check gpiochip_remove(struct gpio_chip *chip); >>> +void gpiochip_remove(struct gpio_chip *chip); >> >> "extern" should be preserved here for style consistency. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
Well, ignoring the return value as it is done in gpio-bt8xx makes the compiler complain and display a warning message. The problem with false warning is that it might distract you. I think that the patch will makes things consistent once completed Thanks a lot for the review. On Sun, May 25, 2014 at 9:46 AM, Alexandre Courbot wrote: > On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe wrote: >> This avoids handling gpiochip remove error in device >> remove handler. > > Be aware that at the moment many callers of gpiochip_remove() read its > return value. So applying your patch as-is would break compilation. > > This patch should therefore be the last of a series that first > modifies all callers of gpiochip_remove() to ignore its return value, > then neutralizes the function itself. > > I am not sure whether the world would really be a better place after > this though. Callers that don't need the return value of > gpiochip_remove() can simply ignore it... > >> >> Signed-off-by: abdoulaye berthe >> --- >> drivers/gpio/gpiolib.c | 24 +++- >> include/linux/gpio/driver.h | 2 +- >> 2 files changed, 8 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c >> index f48817d..4878980 100644 >> --- 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; >> unsignedid; >> >> 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, >desc[id].flags)) { >> - status = -EBUSY; >> - break; >> - } >> - } >> - if (status == 0) { >> - for (id = 0; id < chip->ngpio; id++) >> - chip->desc[id].chip = NULL; >> - >> - list_del(>list); >> + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) >> + panic("gpiolib.c: gpiochip is still requested\n"); > > panic() sounds a little harsh here. Maybe a dev_err() would be enough? > >> } >> + for (id = 0; id < chip->ngpio; id++) >> + chip->desc[id].chip = NULL; >> >> + list_del(>list); >> spin_unlock_irqrestore(_lock, flags); >> - >> - if (status == 0) >> - gpiochip_unexport(chip); >> - >> - return status; >> + gpiochip_unexport(chip); >> } >> EXPORT_SYMBOL_GPL(gpiochip_remove); >> >> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h >> index 1827b43..72ed256 100644 >> --- a/include/linux/gpio/driver.h >> +++ b/include/linux/gpio/driver.h >> @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct >> gpio_chip *chip, >> >> /* add/remove chips */ >> extern int gpiochip_add(struct gpio_chip *chip); >> -extern int __must_check gpiochip_remove(struct gpio_chip *chip); >> +void gpiochip_remove(struct gpio_chip *chip); > > "extern" should be preserved here for style consistency. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe wrote: > This avoids handling gpiochip remove error in device > remove handler. Be aware that at the moment many callers of gpiochip_remove() read its return value. So applying your patch as-is would break compilation. This patch should therefore be the last of a series that first modifies all callers of gpiochip_remove() to ignore its return value, then neutralizes the function itself. I am not sure whether the world would really be a better place after this though. Callers that don't need the return value of gpiochip_remove() can simply ignore it... > > Signed-off-by: abdoulaye berthe > --- > drivers/gpio/gpiolib.c | 24 +++- > include/linux/gpio/driver.h | 2 +- > 2 files changed, 8 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index f48817d..4878980 100644 > --- 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; > unsignedid; > > 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, >desc[id].flags)) { > - status = -EBUSY; > - break; > - } > - } > - if (status == 0) { > - for (id = 0; id < chip->ngpio; id++) > - chip->desc[id].chip = NULL; > - > - list_del(>list); > + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) > + panic("gpiolib.c: gpiochip is still requested\n"); panic() sounds a little harsh here. Maybe a dev_err() would be enough? > } > + for (id = 0; id < chip->ngpio; id++) > + chip->desc[id].chip = NULL; > > + list_del(>list); > spin_unlock_irqrestore(_lock, flags); > - > - if (status == 0) > - gpiochip_unexport(chip); > - > - return status; > + gpiochip_unexport(chip); > } > EXPORT_SYMBOL_GPL(gpiochip_remove); > > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index 1827b43..72ed256 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip > *chip, > > /* add/remove chips */ > extern int gpiochip_add(struct gpio_chip *chip); > -extern int __must_check gpiochip_remove(struct gpio_chip *chip); > +void gpiochip_remove(struct gpio_chip *chip); "extern" should be preserved here for style consistency. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe berthe...@gmail.com wrote: This avoids handling gpiochip remove error in device remove handler. Be aware that at the moment many callers of gpiochip_remove() read its return value. So applying your patch as-is would break compilation. This patch should therefore be the last of a series that first modifies all callers of gpiochip_remove() to ignore its return value, then neutralizes the function itself. I am not sure whether the world would really be a better place after this though. Callers that don't need the return value of gpiochip_remove() can simply ignore it... Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); panic() sounds a little harsh here. Maybe a dev_err() would be enough? } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern should be preserved here for style consistency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
Well, ignoring the return value as it is done in gpio-bt8xx makes the compiler complain and display a warning message. The problem with false warning is that it might distract you. I think that the patch will makes things consistent once completed Thanks a lot for the review. On Sun, May 25, 2014 at 9:46 AM, Alexandre Courbot gnu...@gmail.com wrote: On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe berthe...@gmail.com wrote: This avoids handling gpiochip remove error in device remove handler. Be aware that at the moment many callers of gpiochip_remove() read its return value. So applying your patch as-is would break compilation. This patch should therefore be the last of a series that first modifies all callers of gpiochip_remove() to ignore its return value, then neutralizes the function itself. I am not sure whether the world would really be a better place after this though. Callers that don't need the return value of gpiochip_remove() can simply ignore it... Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); panic() sounds a little harsh here. Maybe a dev_err() would be enough? } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern should be preserved here for style consistency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
On Mon, May 26, 2014 at 1:40 AM, abdoulaye berthe berthe...@gmail.com wrote: Well, ignoring the return value as it is done in gpio-bt8xx makes the compiler complain and display a warning message. The problem with false warning is that it might distract you. Isn't the warning due to the __must_check in the function's declaration? If so just removing it might do the trick... I think that the patch will makes things consistent once completed Yeah fundamentally speaking I am not against this patch - I just wonder if it is worth going through all the call sites and changing them. Also we cannot exclude that a few users actually make something meaningful with the return value (don't know what that would be though). Thanks a lot for the review. On Sun, May 25, 2014 at 9:46 AM, Alexandre Courbot gnu...@gmail.com wrote: On Sat, May 24, 2014 at 2:12 AM, abdoulaye berthe berthe...@gmail.com wrote: This avoids handling gpiochip remove error in device remove handler. Be aware that at the moment many callers of gpiochip_remove() read its return value. So applying your patch as-is would break compilation. This patch should therefore be the last of a series that first modifies all callers of gpiochip_remove() to ignore its return value, then neutralizes the function itself. I am not sure whether the world would really be a better place after this though. Callers that don't need the return value of gpiochip_remove() can simply ignore it... Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); panic() sounds a little harsh here. Maybe a dev_err() would be enough? } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern should be preserved here for style consistency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
If this step is ok I will continue by updating the codes that use it. On Fri, May 23, 2014 at 7:12 PM, abdoulaye berthe wrote: > This avoids handling gpiochip remove error in device > remove handler. > > Signed-off-by: abdoulaye berthe > --- > drivers/gpio/gpiolib.c | 24 +++- > include/linux/gpio/driver.h | 2 +- > 2 files changed, 8 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index f48817d..4878980 100644 > --- 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; > unsignedid; > > 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, >desc[id].flags)) { > - status = -EBUSY; > - break; > - } > - } > - if (status == 0) { > - for (id = 0; id < chip->ngpio; id++) > - chip->desc[id].chip = NULL; > - > - list_del(>list); > + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) > + panic("gpiolib.c: gpiochip is still requested\n"); > } > + for (id = 0; id < chip->ngpio; id++) > + chip->desc[id].chip = NULL; > > + list_del(>list); > spin_unlock_irqrestore(_lock, flags); > - > - if (status == 0) > - gpiochip_unexport(chip); > - > - return status; > + gpiochip_unexport(chip); > } > EXPORT_SYMBOL_GPL(gpiochip_remove); > > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index 1827b43..72ed256 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip > *chip, > > /* add/remove chips */ > extern int gpiochip_add(struct gpio_chip *chip); > -extern int __must_check gpiochip_remove(struct gpio_chip *chip); > +void gpiochip_remove(struct gpio_chip *chip); > extern struct gpio_chip *gpiochip_find(void *data, > int (*match)(struct gpio_chip *chip, void > *data)); > > -- > 1.8.3.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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, >desc[id].flags)) { - status = -EBUSY; - break; - } - } - if (status == 0) { - for (id = 0; id < chip->ngpio; id++) - chip->desc[id].chip = NULL; - - list_del(>list); + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) + panic("gpiolib.c: gpiochip is still requested\n"); } + for (id = 0; id < chip->ngpio; id++) + chip->desc[id].chip = NULL; + list_del(>list); spin_unlock_irqrestore(_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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, >desc[id].flags)) { - status = -EBUSY; - break; - } - } - if (status == 0) { - for (id = 0; id < chip->ngpio; id++) - chip->desc[id].chip = NULL; - - list_del(>list); + if (test_bit(FLAG_REQUESTED, >desc[id].flags)) + panic("gpiolib.c: gpiochip is still requested\n"); } + for (id = 0; id < chip->ngpio; id++) + chip->desc[id].chip = NULL; + list_del(>list); spin_unlock_irqrestore(_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: gpiolib: set gpiochip_remove retval to void
This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: gpiolib: set gpiochip_remove retval to void
If this step is ok I will continue by updating the codes that use it. On Fri, May 23, 2014 at 7:12 PM, abdoulaye berthe berthe...@gmail.com wrote: This avoids handling gpiochip remove error in device remove handler. Signed-off-by: abdoulaye berthe berthe...@gmail.com --- drivers/gpio/gpiolib.c | 24 +++- include/linux/gpio/driver.h | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index f48817d..4878980 100644 --- 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; unsignedid; 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(gpiolib.c: gpiochip is still requested\n); } + for (id = 0; id chip-ngpio; id++) + chip-desc[id].chip = NULL; + list_del(chip-list); spin_unlock_irqrestore(gpio_lock, flags); - - if (status == 0) - gpiochip_unexport(chip); - - return status; + gpiochip_unexport(chip); } EXPORT_SYMBOL_GPL(gpiochip_remove); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..72ed256 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -138,7 +138,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip, /* add/remove chips */ extern int gpiochip_add(struct gpio_chip *chip); -extern int __must_check gpiochip_remove(struct gpio_chip *chip); +void gpiochip_remove(struct gpio_chip *chip); extern struct gpio_chip *gpiochip_find(void *data, int (*match)(struct gpio_chip *chip, void *data)); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/