| Message ID | 1371742016.8005.5.camel@phoenix |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
On Thu, Jun 20, 2013 at 11:26:56PM +0800, Axel Lin wrote: > AM33XX has 4 gpio banks, thus the valid gpio range should be 0 ... 127. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Reviewed-by: Tom Rini <trini@ti.com>
On 20.06.2013 17:26, Axel Lin wrote: > AM33XX has 4 gpio banks, thus the valid gpio range should be 0 ... 127. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > drivers/gpio/omap_gpio.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c > index a30d7f0..9aa6d41 100644 > --- a/drivers/gpio/omap_gpio.c > +++ b/drivers/gpio/omap_gpio.c > @@ -55,7 +55,11 @@ static inline int get_gpio_index(int gpio) > > int gpio_is_valid(int gpio) > { > +#if defined(CONFIG_AM33XX) > + return (gpio >= 0) && (gpio < 128); > +#else > return (gpio >= 0) && (gpio < 192); > +#endif > } Those ifdef's in the code really ugly and frowned upon. Better would be to move this into a define in the top of the C file: #if defined(CONFIG_AM33XX) #define CONFIG_OMAP_MAX_GPIO 128 #else #define CONFIG_OMAP_MAX_GPIO 192 #endif And then use this define in the code: return (gpio >= 0) && (gpio < CONFIG_OMAP_MAX_GPIO); Thanks, Stefan
Dear Stefan Roese, > On 20.06.2013 17:26, Axel Lin wrote: > > AM33XX has 4 gpio banks, thus the valid gpio range should be 0 ... 127. > > > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > > --- > > > > drivers/gpio/omap_gpio.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c > > index a30d7f0..9aa6d41 100644 > > --- a/drivers/gpio/omap_gpio.c > > +++ b/drivers/gpio/omap_gpio.c > > @@ -55,7 +55,11 @@ static inline int get_gpio_index(int gpio) > > > > int gpio_is_valid(int gpio) > > { > > > > +#if defined(CONFIG_AM33XX) > > + return (gpio >= 0) && (gpio < 128); > > +#else > > > > return (gpio >= 0) && (gpio < 192); > > > > +#endif > > > > } > > Those ifdef's in the code really ugly and frowned upon. Better would be > to move this into a define in the top of the C file: > > #if defined(CONFIG_AM33XX) > #define CONFIG_OMAP_MAX_GPIO 128 > #else > #define CONFIG_OMAP_MAX_GPIO 192 > #endif > > And then use this define in the code: > > return (gpio >= 0) && (gpio < CONFIG_OMAP_MAX_GPIO); > > Thanks, > Stefan You were faster ;-) and I agree with this method. Best regards, Marek Vasut
diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c index a30d7f0..9aa6d41 100644 --- a/drivers/gpio/omap_gpio.c +++ b/drivers/gpio/omap_gpio.c @@ -55,7 +55,11 @@ static inline int get_gpio_index(int gpio) int gpio_is_valid(int gpio) { +#if defined(CONFIG_AM33XX) + return (gpio >= 0) && (gpio < 128); +#else return (gpio >= 0) && (gpio < 192); +#endif } static int check_gpio(int gpio)
AM33XX has 4 gpio banks, thus the valid gpio range should be 0 ... 127. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- drivers/gpio/omap_gpio.c | 4 ++++ 1 file changed, 4 insertions(+)