Here is a small series generating a lot of warnings, especially in board
bringup-files. Still, I think it is worthwhile to be strict about checking
return values of gpio-configuration-functions. My suggestion to keep the noise
a bit lower is to put it into linux-next for one cycle and then merge it for
2.6.39? That should give people some time to fix the issues in time. Looking
forward to comments.

Wolfram Sang (2):
  gpiolib: annotate gpio-intialization with __must_check
  gpiolib: add missing functions to generic fallback

 Documentation/gpio.txt     |    2 +-
 include/asm-generic/gpio.h |   10 +++++-----
 include/linux/gpio.h       |   26 +++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 9 deletions(-)

-- 
1.7.2.3

--

[ message continues ]

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Greg KH <gregkh@suse.de>
---
 include/linux/gpio.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 1d5214a..f79d67f 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -13,6 +13,7 @@
 #include <linux/errno.h>
 
 struct device;
+struct gpio;
 struct gpio_chip;
 
 /*
@@ -34,6 +35,17 @@ static inline int __must_check gpio_request(unsigned gpio, const char *label)
 	return -ENOSYS;
 }
 
+static inline int __must_check gpio_request_one(unsigned gpio,
+					unsigned long flags, const char *label)
+{
+	return -ENOSYS;
+}
+
+static inline int __must_check gpio_request_array(struct gpio *array, size_t num)
+{
+	return -ENOSYS;
+}
+
 static inline void gpio_free(unsigned gpio)
 {
 	might_sleep();
@@ -42,6 +54,14 @@ static inline void gpio_free(unsigned gpio)
 	WARN_ON(1);
 }
 
+static inline void gpio_free_array(struct gpio *array, size_t num)
+{
+	might_sleep();
+
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
+
 static inline int __must_check gpio_direction_input(unsigned gpio)
 {
 	return -ENOSYS;
-- 
1.7.2.3

--

[ message continues ]

Because GPIOs can have crucial functions especially in embedded systems,
we are better safe than sorry regarding their configuration. For
gpio_request, the documentation is simply enforced: <quote>"The return
value of gpio_request() must be checked."</quote> For gpio_direction_*
and gpio_request_*, we now act accordingly.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Greg KH <gregkh@suse.de>
---
 Documentation/gpio.txt     |    2 +-
 include/asm-generic/gpio.h |   10 +++++-----
 include/linux/gpio.h       |    6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index 792faa3..a492d92 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -135,7 +135,7 @@ setting up a platform_device using the GPIO, is mark its direction:
 	int gpio_direction_input(unsigned gpio);
 	int gpio_direction_output(unsigned gpio, int value);
 
-The return value is zero for success, else a negative errno.  It should
+The return value is zero for success, else a negative errno.  It must
 be checked, since the get/set calls don't have error returns and since
 misconfiguration is possible.  You should normally issue these calls from
 a task context.  However, for spinlock-safe GPIOs it's OK to use them
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index ff5c660..6098cae 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -147,11 +147,11 @@ extern struct gpio_chip *gpiochip_find(void *data,
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.
  */
-extern int gpio_request(unsigned gpio, const char *label);
+extern int __must_check gpio_request(unsigned gpio, const char *label);
 extern void gpio_free(unsigned gpio);
 
-extern int gpio_direction_input(unsigned gpio);
-extern int gpio_direction_output(unsigned gpio, int value);
+extern int __must_check ...
[ message continues ]

It's ok to add this type of thing, but please, go through and fix the
warnings at the same time.  Otherwise it's a bit rude to force others to
fix their code for something that you did.

Care to also send those patches along?

thanks,

greg k-h
--

[ message continues ]

Yeah, I understand. I was a "victim" of the patch causing all those "key not in
.data" messages back then. So, I actually did start a coccinelle-script fixing
the issues. I examined one sub-directory using a CPU/SoC I know relatively
well. I had to learn that even then, it is pretty hard to determine what
exactly to do if gpio_request() fails. For example, an unavailable GPIO being
the write-protect-pin for SD-cards might be simply ignored, maybe a warning
printed and the card will be rw by default. Another GPIO might be a chip-select
of a device I have never heard of before. It might be crucial and board_init
should fail if it cannot be requested. Or not. Things get worse for
architectures I never used before. This is why I think it is really better to
let people do the fixups who have/understand the hardware in question.
Otherwise the fixups could indeed be more harmful than helpful.

If this is still too rude for your taste, then what about a mechanism similar
to DEBUG_SECTION_MISMATCH?

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
[ message continues ]

Sure, they could be more harmful, but at least try.  Make the patches
up, submit them to the maintainers, and if they are wrong, they will be

No, that's still annoying :)

thanks,

greg k-h
--

[ message continues ]

Well, I could generalize all cases and always issue a WARN() if the request
fails. But this would just move a compile-time warning into a runtime warning.
Also, I have my doubts that even the arch/mach-maintainers know all the boards
and their peculiarities. There are thousands of them.

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
[ message continues ]