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

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

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 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

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

[David Brownell]

On Tuesday 11 March 2008, Anton Vorontsov wrote:
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

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
> 

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

[Anton Vorontsov]

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 <avorontsov@ru.mvista.com>
> 
> 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

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

[David Brownell]

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 <avorontsov@ru.mvista.com>
> > 
> > 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