On Thu, 20 Sep 2007 17:43:46 +0200, Matteo Croce <technoboy85@xxxxxxxxx> wrote:
> +static inline int gpio_get_value(unsigned gpio)
> +{
> + void __iomem *gpio_in =
> + (void __iomem *)KSEG1ADDR(AR7_REGS_GPIO + AR7_GPIO_INPUT);
> +
> + if (gpio >= AR7_GPIO_MAX)
> + return -EINVAL;
Checking with AR7_GPIO_MAX can be dropped from gpio_set_value() and
gpio_get_value(). The validity checking is done in gpio_direction_*().
--- excerpt from Documentation/gpio.txt ---
The get/set calls have no error returns because "invalid GPIO" should have
been reported earlier from gpio_direction_*(). However, note that not all
platforms can read the value of output pins; those that can't should always
return zero. Also, using these calls for GPIOs that can't safely be accessed
without sleeping (see below) is an error.
Platform-specific implementations are encouraged to optimize the two
calls to access the GPIO value in cases where the GPIO number (and for
output, value) are constant. ...
--- excerpt ---
> +
> + return ((readl(gpio_in) & (1 << gpio)) != 0);
The gpio API defines any non-zero value (not only '1') for "high", so
"!= 0" is not required.
> +}
> +
> +static inline void gpio_set_value(unsigned gpio, int value)
> +{
> + void __iomem *gpio_out =
> + (void __iomem *)KSEG1ADDR(AR7_REGS_GPIO + AR7_GPIO_OUTPUT);
> + volatile unsigned tmp;
This 'volatile' has no sense and can be just dropped.
> +
> + if (gpio >= AR7_GPIO_MAX)
> + return;
Ditto.
> +
> + tmp = readl(gpio_out) & ~(1 << gpio);
> + if (value)
> + tmp |= 1 << gpio;
> + writel(tmp, gpio_out);
> +}
---
Atsushi Nemoto
