* [PATCH] gpio: Convert gpio_is_valid to return bool [not found] ` <BANLkTi=zDJLHUa2ogTeEkmdsbkhvGJCsEw@mail.gmail.com> @ 2011-05-10 23:23 ` Joe Perches 2011-05-10 23:23 ` Joe Perches ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Joe Perches @ 2011-05-10 23:23 UTC (permalink / raw) To: Linus Walleij, Arnd Bergmann, Grant Likely; +Cc: linux-kernel, linux-arch Make the code a bit more readable. Instead of casting an int to an unsigned then comparing to MAX_NR_GPIOS, add a >= 0 test and let the compiler optimizer do the conversion to unsigned. The generated code should be the same. Signed-off-by: Joe Perches <joe@perches.com> --- This came up because of a new pinmux subsystem that used a style copied from gpio and a request from Linus Walleij. On Wed, 2011-05-11 at 00:52 +0200, Linus Walleij wrote: > 2011/5/11 Joe Perches <joe@perches.com>: > > On Wed, 2011-05-11 at 00:18 +0200, Linus Walleij wrote: > >> 2011/5/2 Joe Perches <joe@perches.com>: > >> > On Mon, 2011-05-02 at 21:16 +0200, Linus Walleij wrote: > >> >> From: Linus Walleij <linus.walleij@linaro.org> > >> >> diff --git a/drivers/pinmux/core.c b/drivers/pinmux/core.c > >> > Trivial comments follow > >> >> +static inline int pin_is_valid(int pin) > >> >> +{ > >> >> + return ((unsigned)pin) < MACH_NR_PINS; > >> >> +} > >> > Couldn't pin just be declared unsigned or maybe u32? > >> No, because like in the GPIO subsystem you *may* want to send in invalid > >> pins, and those are identified by negative numbers. > > Then I think this is clearer and the compiler > > should produce the same code. > > static inline bool pin_is_valid(int pin) > > { > > return pin >= 0 && pin < MACH_NR_PINS; > > } > Yes indeed, I'll fix. Can you propose a patch to the same pattern > found in include/asm-generic/gpio.h? It would bring equal > clarity there I believe. include/asm-generic/gpio.h | 6 +++--- include/linux/gpio.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index ce16e70..315ecb7 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -35,9 +35,9 @@ * platform data and other tables. */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { - return ((unsigned)number) < ARCH_NR_GPIOS; + return number >= 0 && number < ARCH_NR_GPIOS; } struct device; @@ -216,7 +216,7 @@ extern void gpio_unexport(unsigned gpio); #else /* !CONFIG_GPIOLIB */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { /* only non-negative numbers are valid */ return number >= 0; diff --git a/include/linux/gpio.h b/include/linux/gpio.h index fa92e50..0af3bca 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -79,9 +79,9 @@ struct gpio_chip; * warning when something is wrongly called. */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { - return 0; + return false; } static inline int gpio_request(unsigned gpio, const char *label) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] gpio: Convert gpio_is_valid to return bool 2011-05-10 23:23 ` [PATCH] gpio: Convert gpio_is_valid to return bool Joe Perches @ 2011-05-10 23:23 ` Joe Perches 2011-05-10 23:42 ` Linus Walleij 2011-05-27 3:02 ` Grant Likely 2 siblings, 0 replies; 4+ messages in thread From: Joe Perches @ 2011-05-10 23:23 UTC (permalink / raw) To: Linus Walleij, Arnd Bergmann, Grant Likely; +Cc: linux-kernel, linux-arch Make the code a bit more readable. Instead of casting an int to an unsigned then comparing to MAX_NR_GPIOS, add a >= 0 test and let the compiler optimizer do the conversion to unsigned. The generated code should be the same. Signed-off-by: Joe Perches <joe@perches.com> --- This came up because of a new pinmux subsystem that used a style copied from gpio and a request from Linus Walleij. On Wed, 2011-05-11 at 00:52 +0200, Linus Walleij wrote: > 2011/5/11 Joe Perches <joe@perches.com>: > > On Wed, 2011-05-11 at 00:18 +0200, Linus Walleij wrote: > >> 2011/5/2 Joe Perches <joe@perches.com>: > >> > On Mon, 2011-05-02 at 21:16 +0200, Linus Walleij wrote: > >> >> From: Linus Walleij <linus.walleij@linaro.org> > >> >> diff --git a/drivers/pinmux/core.c b/drivers/pinmux/core.c > >> > Trivial comments follow > >> >> +static inline int pin_is_valid(int pin) > >> >> +{ > >> >> + return ((unsigned)pin) < MACH_NR_PINS; > >> >> +} > >> > Couldn't pin just be declared unsigned or maybe u32? > >> No, because like in the GPIO subsystem you *may* want to send in invalid > >> pins, and those are identified by negative numbers. > > Then I think this is clearer and the compiler > > should produce the same code. > > static inline bool pin_is_valid(int pin) > > { > > return pin >= 0 && pin < MACH_NR_PINS; > > } > Yes indeed, I'll fix. Can you propose a patch to the same pattern > found in include/asm-generic/gpio.h? It would bring equal > clarity there I believe. include/asm-generic/gpio.h | 6 +++--- include/linux/gpio.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index ce16e70..315ecb7 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -35,9 +35,9 @@ * platform data and other tables. */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { - return ((unsigned)number) < ARCH_NR_GPIOS; + return number >= 0 && number < ARCH_NR_GPIOS; } struct device; @@ -216,7 +216,7 @@ extern void gpio_unexport(unsigned gpio); #else /* !CONFIG_GPIOLIB */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { /* only non-negative numbers are valid */ return number >= 0; diff --git a/include/linux/gpio.h b/include/linux/gpio.h index fa92e50..0af3bca 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -79,9 +79,9 @@ struct gpio_chip; * warning when something is wrongly called. */ -static inline int gpio_is_valid(int number) +static inline bool gpio_is_valid(int number) { - return 0; + return false; } static inline int gpio_request(unsigned gpio, const char *label) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: Convert gpio_is_valid to return bool 2011-05-10 23:23 ` [PATCH] gpio: Convert gpio_is_valid to return bool Joe Perches 2011-05-10 23:23 ` Joe Perches @ 2011-05-10 23:42 ` Linus Walleij 2011-05-27 3:02 ` Grant Likely 2 siblings, 0 replies; 4+ messages in thread From: Linus Walleij @ 2011-05-10 23:42 UTC (permalink / raw) To: Joe Perches; +Cc: Arnd Bergmann, Grant Likely, linux-kernel, linux-arch 2011/5/11 Joe Perches <joe@perches.com>: > Make the code a bit more readable. > > Instead of casting an int to an unsigned then comparing to > MAX_NR_GPIOS, add a >= 0 test and let the compiler optimizer > do the conversion to unsigned. > > The generated code should be the same. > > Signed-off-by: Joe Perches <joe@perches.com> Acked-by: Linus Walleij <linus.walleij@linaro.org> Thanks Joe! Linus Walleij ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: Convert gpio_is_valid to return bool 2011-05-10 23:23 ` [PATCH] gpio: Convert gpio_is_valid to return bool Joe Perches 2011-05-10 23:23 ` Joe Perches 2011-05-10 23:42 ` Linus Walleij @ 2011-05-27 3:02 ` Grant Likely 2 siblings, 0 replies; 4+ messages in thread From: Grant Likely @ 2011-05-27 3:02 UTC (permalink / raw) To: Joe Perches; +Cc: Linus Walleij, Arnd Bergmann, linux-kernel, linux-arch On Tue, May 10, 2011 at 04:23:07PM -0700, Joe Perches wrote: > Make the code a bit more readable. > > Instead of casting an int to an unsigned then comparing to > MAX_NR_GPIOS, add a >= 0 test and let the compiler optimizer > do the conversion to unsigned. > > The generated code should be the same. > > Signed-off-by: Joe Perches <joe@perches.com> Applied, thanks. g. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-05-27 3:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1304363786-30376-1-git-send-email-linus.walleij@stericsson.com>
[not found] ` <1304365077.7792.40.camel@Joe-Laptop>
[not found] ` <BANLkTi=XSevO72NqCYOYuketjE8XjeQU+A@mail.gmail.com>
[not found] ` <1305067020.19586.130.camel@Joe-Laptop>
[not found] ` <BANLkTi=zDJLHUa2ogTeEkmdsbkhvGJCsEw@mail.gmail.com>
2011-05-10 23:23 ` [PATCH] gpio: Convert gpio_is_valid to return bool Joe Perches
2011-05-10 23:23 ` Joe Perches
2011-05-10 23:42 ` Linus Walleij
2011-05-27 3:02 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).