[PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function - Kernel

This is a discussion on [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function - Kernel ; Signed-off-by: Anton Vorontsov --- drivers/gpio/gpiolib.c | 2 +- include/asm-generic/gpio.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 2149e0c..851eb4d 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -161,7 +161,7 @@ static int gpiochip_find_base(int ngpio) * because ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function

  1. [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function


    Signed-off-by: Anton Vorontsov
    ---
    drivers/gpio/gpiolib.c | 2 +-
    include/asm-generic/gpio.h | 2 +-
    2 files changed, 2 insertions(+), 2 deletions(-)

    diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    index 2149e0c..851eb4d 100644
    --- a/drivers/gpio/gpiolib.c
    +++ b/drivers/gpio/gpiolib.c
    @@ -161,7 +161,7 @@ static int gpiochip_find_base(int ngpio)
    * because the chip->base is invalid or already associated with a
    * different chip. Otherwise it returns zero as a success code.
    */
    -int gpiochip_add(struct gpio_chip *chip)
    +int __must_check gpiochip_add(struct gpio_chip *chip)
    {
    unsigned long flags;
    int status = 0;
    diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
    index 20f5d67..51ed230 100644
    --- a/include/asm-generic/gpio.h
    +++ b/include/asm-generic/gpio.h
    @@ -77,7 +77,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip,
    extern int __must_check gpiochip_reserve(int start, int ngpio);

    /* add/remove chips */
    -extern int gpiochip_add(struct gpio_chip *chip);
    +extern int __must_check gpiochip_add(struct gpio_chip *chip);
    extern int __must_check gpiochip_remove(struct gpio_chip *chip);


    --
    1.5.2.2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function

    On Tuesday 11 March 2008, Anton Vorontsov wrote:
    >
    > Signed-off-by: Anton Vorontsov


    I'm curious why you want to change this. When it fails, a
    KERN_ERR message is emitted ... if you're going to insist
    that callers handle this -- e.g. platform setup code might
    use a "WARN_ON(gpiochip_add(...) < 0)" -- why not take out
    that KERN_ERR mechanism too?

    NAK unless it comes with a patch to update all callers to do
    that checking, so that this isn't just "add build warnings".
    But I'm not sure I like these attributes in cases like this.

    - Dave



    > ---
    > drivers/gpio/gpiolib.c | 2 +-
    > include/asm-generic/gpio.h | 2 +-
    > 2 files changed, 2 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    > index 2149e0c..851eb4d 100644
    > --- a/drivers/gpio/gpiolib.c
    > +++ b/drivers/gpio/gpiolib.c
    > @@ -161,7 +161,7 @@ static int gpiochip_find_base(int ngpio)
    > * because the chip->base is invalid or already associated with a
    > * different chip. Otherwise it returns zero as a success code.
    > */
    > -int gpiochip_add(struct gpio_chip *chip)
    > +int __must_check gpiochip_add(struct gpio_chip *chip)
    > {
    > unsigned long flags;
    > int status = 0;
    > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
    > index 20f5d67..51ed230 100644
    > --- a/include/asm-generic/gpio.h
    > +++ b/include/asm-generic/gpio.h
    > @@ -77,7 +77,7 @@ extern const char *gpiochip_is_requested(struct gpio_chip *chip,
    > extern int __must_check gpiochip_reserve(int start, int ngpio);
    >
    > /* add/remove chips */
    > -extern int gpiochip_add(struct gpio_chip *chip);
    > +extern int __must_check gpiochip_add(struct gpio_chip *chip);
    > extern int __must_check gpiochip_remove(struct gpio_chip *chip);
    >
    >
    > --
    > 1.5.2.2
    >



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function

    On Tue, Mar 11, 2008 at 01:02:38PM -0800, David Brownell wrote:
    > On Tuesday 11 March 2008, Anton Vorontsov wrote:
    > >
    > > Signed-off-by: Anton Vorontsov

    >
    > I'm curious why you want to change this. When it fails, a
    > KERN_ERR message is emitted ...


    Ah, I missed that the gpiochip_add() issuing pr_err(), not pr_debug().

    So, please just ignore that patch. Though, printing errors from the
    generic functions is confusing, you never know when your code or the
    function you call should print the error. And usually generic functions
    tend to be silent...

    --
    Anton Vorontsov
    email: cboumailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 3/3] gpiolib: add __must_check attribute to the gpiochip_add function

    On Wednesday 12 March 2008, Anton Vorontsov wrote:
    > On Tue, Mar 11, 2008 at 01:02:38PM -0800, David Brownell wrote:
    > > On Tuesday 11 March 2008, Anton Vorontsov wrote:
    > > >
    > > > Signed-off-by: Anton Vorontsov

    > >
    > > I'm curious why you want to change this. When it fails, a
    > > KERN_ERR message is emitted ...

    >
    > Ah, I missed that the gpiochip_add() issuing pr_err(), not pr_debug().
    >
    > So, please just ignore that patch.


    OK ...

    > Though, printing errors from the
    > generic functions is confusing, you never know when your code or the
    > function you call should print the error. And usually generic functions
    > tend to be silent...


    The logic in this case was that I don't really expect platform
    setup code to check this stuff any more than it checks IRQ setup.
    But, as the comment says, if platform setup code goofs, it can
    cause un-bootable systems. Ergo the pr_err().

    It could be argued either way; that's just what I did, way back
    when I noticed the issue.

    - Dave
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread