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;
>>> 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("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/
