linux-mips
[Top] [All Lists]

Re: [PATCH][MIPS][1/7] AR7: core support

To: technoboy85@gmail.com
Subject: Re: [PATCH][MIPS][1/7] AR7: core support
From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Date: Sun, 23 Sep 2007 01:18:01 +0900 (JST)
Cc: linux-mips@linux-mips.org, florian@openwrt.org, nbd@openwrt.org, ejka@imfi.kspu.ru, nico@openwrt.org, ralf@linux-mips.org, akpm@linux-foundation.org
In-reply-to: <200709201743.48164.technoboy85@gmail.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <200709201728.10866.technoboy85@gmail.com> <200709201743.48164.technoboy85@gmail.com>
Sender: linux-mips-bounce@linux-mips.org
On Thu, 20 Sep 2007 17:43:46 +0200, Matteo Croce <technoboy85@gmail.com> 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

<Prev in Thread] Current Thread [Next in Thread>