Messages in this thread | | | Date | Thu, 21 Dec 2006 12:25:06 -0500 (EST) | From | Nicolas Pitre <> | Subject | Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers |
| |
On Thu, 21 Dec 2006, pHilipp Zabel wrote:
> David suggested to have both inline and non-inline functions depending > on whether gpio is constant. How is this patch?
More comments below.
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ linux-2.6/include/asm-arm/arch-pxa/gpio.h 2006-12-21 > 07:57:12.000000000 +0100 > @@ -0,0 +1,86 @@ [...] > +static inline int gpio_direction_input(unsigned gpio) > +{ > + if (gpio > PXA_LAST_GPIO) > + return -EINVAL; > + pxa_gpio_mode(gpio | GPIO_IN); > +} > + > +static inline int gpio_direction_output(unsigned gpio) > +{ > + if (gpio > PXA_LAST_GPIO) > + return -EINVAL; > + pxa_gpio_mode(gpio | GPIO_OUT); > +}
Please push this test against PXA_LAST_GPIO inside pxa_gpio_mode(). It has no advantage to be inline if you're about to call a function anyway. That would make pxa_gpio_mode() more reliable for those not calling it through the generic API wrt that kind of error as well.
> --- linux-2.6.orig/arch/arm/mach-pxa/generic.c 2006-12-16 > +++ linux-2.6/arch/arm/mach-pxa/generic.c 2006-12-16 16:47:45.000000000 > @@ -129,6 +129,29 @@ > EXPORT_SYMBOL(pxa_gpio_mode); > > /* > + * Return GPIO level, nonzero means high, zero is low > + */ > +int pxa_gpio_get_value(unsigned gpio) > +{ > + return GPLR(gpio) & GPIO_bit(gpio); > +} > + > +EXPORT_SYMBOL(pxa_gpio_get_value); > + > +/* > + * Set output GPIO level > + */ > +void pxa_gpio_set_value(unsigned gpio, int value) > +{ > + if (value) > + GPSR(gpio) = GPIO_bit(gpio); > + else > + GPCR(gpio) = GPIO_bit(gpio); > +} > + > +EXPORT_SYMBOL(pxa_gpio_set_value);
Instead of duplicating code here, you probably should just reuse __gpio_set_value() and __gpio_get_value() inside those functions.
Nicolas - 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/
| |