[10/19] mach-ep93xx: break out GPIO driver specifics

Message ID	1312978655-18405-1-git-send-email-linus.walleij@stericsson.com (mailing list archive)
State	New, archived
Headers	show Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p7ACWxYt006652 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for <patchwork-linux-arm@patchwork.kernel.org>; Wed, 10 Aug 2011 12:33:20 GMT From: Linus Walleij <linus.walleij@stericsson.com> To: Grant Likely <grant.likely@secretlab.ca>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org> Subject: [PATCH 10/19] mach-ep93xx: break out GPIO driver specifics Date: Wed, 10 Aug 2011 14:17:35 +0200 Message-ID: <1312978655-18405-1-git-send-email-linus.walleij@stericsson.com> MIME-Version: 1.0 summary: Content analysis details: (-2.3 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [207.126.144.149 listed in list.dnswl.org] Cc: Hartley Sweeten <hsweeten@visionengravers.com>, Linus Walleij <linus.walleij@linaro.org>, Lee Jones <lee.jones@linaro.org>, Ryan Mallon <rmallon@gmail.com> Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Message ID

1312978655-18405-1-git-send-email-linus.walleij@stericsson.com (mailing list archive)

State

New, archived

Headers

From: Linus Walleij <linus.walleij@stericsson.com>
To: Grant Likely <grant.likely@secretlab.ca>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: [PATCH 10/19] mach-ep93xx: break out GPIO driver specifics
Date: Wed, 10 Aug 2011 14:17:35 +0200
Message-ID: <1312978655-18405-1-git-send-email-linus.walleij@stericsson.com>
MIME-Version: 1.0
Cc: Hartley Sweeten <hsweeten@visionengravers.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Lee Jones <lee.jones@linaro.org>, Ryan Mallon <rmallon@gmail.com>
Precedence: list
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: linux-arm-kernel-bounces@lists.infradead.org
Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Commit Message

Linus Walleij Aug. 10, 2011, 12:17 p.m. UTC

From: Linus Walleij <linus.walleij@linaro.org>

The <mach/gpio.h> file is included from upper directories
and deal with generic GPIO and gpiolib stuff. Break out the
platform and driver specific defines and functions into its own
header file.

Cc: Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ryan Mallon <rmallon@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-ep93xx/core.c                     |    1 +
 arch/arm/mach-ep93xx/edb93xx.c                  |    1 +
 arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h |  100 +++++++++++++++++++++++
 arch/arm/mach-ep93xx/include/mach/gpio.h        |   97 ++---------------------
 arch/arm/mach-ep93xx/simone.c                   |    2 +-
 arch/arm/mach-ep93xx/snappercl15.c              |    2 +-
 6 files changed, 110 insertions(+), 93 deletions(-)
 create mode 100644 arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h

Comments

Hartley Sweeten Aug. 10, 2011, 5:23 p.m. UTC | #1

On Wednesday, August 10, 2011 5:18 AM, Linus Walleij wrote:
>
> The <mach/gpio.h> file is included from upper directories
> and deal with generic GPIO and gpiolib stuff. Break out the
> platform and driver specific defines and functions into its own
> header file.
>
> Cc: Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <rmallon@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/mach-ep93xx/core.c                     |    1 +
>  arch/arm/mach-ep93xx/edb93xx.c                  |    1 +
>  arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h |  100 +++++++++++++++++++++++
>  arch/arm/mach-ep93xx/include/mach/gpio.h        |   97 ++---------------------
>  arch/arm/mach-ep93xx/simone.c                   |    2 +-
>  arch/arm/mach-ep93xx/snappercl15.c              |    2 +-
>  6 files changed, 110 insertions(+), 93 deletions(-)
>  create mode 100644 arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h

Linus,

I'm a bit confused by the intentions of this patch.  Please see below.

> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
> index c60f081..94c78bc 100644
> --- a/arch/arm/mach-ep93xx/core.c
> +++ b/arch/arm/mach-ep93xx/core.c
> @@ -38,6 +38,7 @@
>  #include <mach/fb.h>
>  #include <mach/ep93xx_keypad.h>
>  #include <mach/ep93xx_spi.h>
> +#include <mach/gpio-ep93xx.h>

Why is this additional include needed?  With your change to the ep93xx
<mach/gpio.h> below, this header is already included by <linux/gpio.h>.

Since this file actually uses the gpiolib calls, the include of
<linux/gpio.h> is needed and appropriate.

Additional comment on the change to the ep93xx <mach/gpio.h> below.

>  #include <asm/mach/map.h>
>  #include <asm/mach/time.h>
> diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c
> index 9969bb1..3f320c6 100644
> --- a/arch/arm/mach-ep93xx/edb93xx.c
> +++ b/arch/arm/mach-ep93xx/edb93xx.c
> @@ -37,6 +37,7 @@
>  #include <mach/hardware.h>
>  #include <mach/fb.h>
>  #include <mach/ep93xx_spi.h>
> +#include <mach/gpio-ep93xx.h>

Same comment as above.

>  #include <asm/mach-types.h>
>  #include <asm/mach/arch.h>
> diff --git a/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h b/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h
> new file mode 100644
> index 0000000..8aff2ea
> --- /dev/null
> +++ b/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h
> @@ -0,0 +1,100 @@
> +/* Include file for the EP93XX GPIO controller machine specifics */
> +
> +#ifndef __GPIO_EP93XX_H
> +#define __GPIO_EP93XX_H
> +
> +/* GPIO port A.  */
> +#define EP93XX_GPIO_LINE_A(x)		((x) + 0)
> +#define EP93XX_GPIO_LINE_EGPIO0		EP93XX_GPIO_LINE_A(0)
> +#define EP93XX_GPIO_LINE_EGPIO1		EP93XX_GPIO_LINE_A(1)
> +#define EP93XX_GPIO_LINE_EGPIO2		EP93XX_GPIO_LINE_A(2)
> +#define EP93XX_GPIO_LINE_EGPIO3		EP93XX_GPIO_LINE_A(3)
> +#define EP93XX_GPIO_LINE_EGPIO4		EP93XX_GPIO_LINE_A(4)
> +#define EP93XX_GPIO_LINE_EGPIO5		EP93XX_GPIO_LINE_A(5)
> +#define EP93XX_GPIO_LINE_EGPIO6		EP93XX_GPIO_LINE_A(6)
> +#define EP93XX_GPIO_LINE_EGPIO7		EP93XX_GPIO_LINE_A(7)
> +
> +/* GPIO port B.  */
> +#define EP93XX_GPIO_LINE_B(x)		((x) + 8)
> +#define EP93XX_GPIO_LINE_EGPIO8		EP93XX_GPIO_LINE_B(0)
> +#define EP93XX_GPIO_LINE_EGPIO9		EP93XX_GPIO_LINE_B(1)
> +#define EP93XX_GPIO_LINE_EGPIO10	EP93XX_GPIO_LINE_B(2)
> +#define EP93XX_GPIO_LINE_EGPIO11	EP93XX_GPIO_LINE_B(3)
> +#define EP93XX_GPIO_LINE_EGPIO12	EP93XX_GPIO_LINE_B(4)
> +#define EP93XX_GPIO_LINE_EGPIO13	EP93XX_GPIO_LINE_B(5)
> +#define EP93XX_GPIO_LINE_EGPIO14	EP93XX_GPIO_LINE_B(6)
> +#define EP93XX_GPIO_LINE_EGPIO15	EP93XX_GPIO_LINE_B(7)
> +
> +/* GPIO port C.  */
> +#define EP93XX_GPIO_LINE_C(x)		((x) + 40)
> +#define EP93XX_GPIO_LINE_ROW0		EP93XX_GPIO_LINE_C(0)
> +#define EP93XX_GPIO_LINE_ROW1		EP93XX_GPIO_LINE_C(1)
> +#define EP93XX_GPIO_LINE_ROW2		EP93XX_GPIO_LINE_C(2)
> +#define EP93XX_GPIO_LINE_ROW3		EP93XX_GPIO_LINE_C(3)
> +#define EP93XX_GPIO_LINE_ROW4		EP93XX_GPIO_LINE_C(4)
> +#define EP93XX_GPIO_LINE_ROW5		EP93XX_GPIO_LINE_C(5)
> +#define EP93XX_GPIO_LINE_ROW6		EP93XX_GPIO_LINE_C(6)
> +#define EP93XX_GPIO_LINE_ROW7		EP93XX_GPIO_LINE_C(7)
> +
> +/* GPIO port D.  */
> +#define EP93XX_GPIO_LINE_D(x)		((x) + 24)
> +#define EP93XX_GPIO_LINE_COL0		EP93XX_GPIO_LINE_D(0)
> +#define EP93XX_GPIO_LINE_COL1		EP93XX_GPIO_LINE_D(1)
> +#define EP93XX_GPIO_LINE_COL2		EP93XX_GPIO_LINE_D(2)
> +#define EP93XX_GPIO_LINE_COL3		EP93XX_GPIO_LINE_D(3)
> +#define EP93XX_GPIO_LINE_COL4		EP93XX_GPIO_LINE_D(4)
> +#define EP93XX_GPIO_LINE_COL5		EP93XX_GPIO_LINE_D(5)
> +#define EP93XX_GPIO_LINE_COL6		EP93XX_GPIO_LINE_D(6)
> +#define EP93XX_GPIO_LINE_COL7		EP93XX_GPIO_LINE_D(7)
> +
> +/* GPIO port E.  */
> +#define EP93XX_GPIO_LINE_E(x)		((x) + 32)
> +#define EP93XX_GPIO_LINE_GRLED		EP93XX_GPIO_LINE_E(0)
> +#define EP93XX_GPIO_LINE_RDLED		EP93XX_GPIO_LINE_E(1)
> +#define EP93XX_GPIO_LINE_DIORn		EP93XX_GPIO_LINE_E(2)
> +#define EP93XX_GPIO_LINE_IDECS1n	EP93XX_GPIO_LINE_E(3)
> +#define EP93XX_GPIO_LINE_IDECS2n	EP93XX_GPIO_LINE_E(4)
> +#define EP93XX_GPIO_LINE_IDEDA0		EP93XX_GPIO_LINE_E(5)
> +#define EP93XX_GPIO_LINE_IDEDA1		EP93XX_GPIO_LINE_E(6)
> +#define EP93XX_GPIO_LINE_IDEDA2		EP93XX_GPIO_LINE_E(7)
> +
> +/* GPIO port F.  */
> +#define EP93XX_GPIO_LINE_F(x)		((x) + 16)
> +#define EP93XX_GPIO_LINE_WP		EP93XX_GPIO_LINE_F(0)
> +#define EP93XX_GPIO_LINE_MCCD1		EP93XX_GPIO_LINE_F(1)
> +#define EP93XX_GPIO_LINE_MCCD2		EP93XX_GPIO_LINE_F(2)
> +#define EP93XX_GPIO_LINE_MCBVD1		EP93XX_GPIO_LINE_F(3)
> +#define EP93XX_GPIO_LINE_MCBVD2		EP93XX_GPIO_LINE_F(4)
> +#define EP93XX_GPIO_LINE_VS1		EP93XX_GPIO_LINE_F(5)
> +#define EP93XX_GPIO_LINE_READY		EP93XX_GPIO_LINE_F(6)
> +#define EP93XX_GPIO_LINE_VS2		EP93XX_GPIO_LINE_F(7)
> +
> +/* GPIO port G.  */
> +#define EP93XX_GPIO_LINE_G(x)		((x) + 48)
> +#define EP93XX_GPIO_LINE_EECLK		EP93XX_GPIO_LINE_G(0)
> +#define EP93XX_GPIO_LINE_EEDAT		EP93XX_GPIO_LINE_G(1)
> +#define EP93XX_GPIO_LINE_SLA0		EP93XX_GPIO_LINE_G(2)
> +#define EP93XX_GPIO_LINE_SLA1		EP93XX_GPIO_LINE_G(3)
> +#define EP93XX_GPIO_LINE_DD12		EP93XX_GPIO_LINE_G(4)
> +#define EP93XX_GPIO_LINE_DD13		EP93XX_GPIO_LINE_G(5)
> +#define EP93XX_GPIO_LINE_DD14		EP93XX_GPIO_LINE_G(6)
> +#define EP93XX_GPIO_LINE_DD15		EP93XX_GPIO_LINE_G(7)
> +
> +/* GPIO port H.  */
> +#define EP93XX_GPIO_LINE_H(x)		((x) + 56)
> +#define EP93XX_GPIO_LINE_DD0		EP93XX_GPIO_LINE_H(0)
> +#define EP93XX_GPIO_LINE_DD1		EP93XX_GPIO_LINE_H(1)
> +#define EP93XX_GPIO_LINE_DD2		EP93XX_GPIO_LINE_H(2)
> +#define EP93XX_GPIO_LINE_DD3		EP93XX_GPIO_LINE_H(3)
> +#define EP93XX_GPIO_LINE_DD4		EP93XX_GPIO_LINE_H(4)
> +#define EP93XX_GPIO_LINE_DD5		EP93XX_GPIO_LINE_H(5)
> +#define EP93XX_GPIO_LINE_DD6		EP93XX_GPIO_LINE_H(6)
> +#define EP93XX_GPIO_LINE_DD7		EP93XX_GPIO_LINE_H(7)
> +
> +/* maximum value for gpio line identifiers */
> +#define EP93XX_GPIO_LINE_MAX		EP93XX_GPIO_LINE_H(7)
> +
> +/* maximum value for irq capable line identifiers */
> +#define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
> +
> +#endif /* __GPIO_EP93XX_H */

If the end goal of this is to get rid of the <mach/gpio.h> files, I have no
problem with this move.

Question... Shouldn't the gpio_to_irq and irq_to_gpio defines also be moved
here?

> diff --git a/arch/arm/mach-ep93xx/include/mach/gpio.h b/arch/arm/mach-ep93xx/include/mach/gpio.h
> index 071f676..acfc113 100644
> --- a/arch/arm/mach-ep93xx/include/mach/gpio.h
> +++ b/arch/arm/mach-ep93xx/include/mach/gpio.h
> @@ -5,99 +5,14 @@
>  #ifndef __ASM_ARCH_GPIO_H
>  #define __ASM_ARCH_GPIO_H
>  
> -/* GPIO port A.  */
> -#define EP93XX_GPIO_LINE_A(x)		((x) + 0)
> -#define EP93XX_GPIO_LINE_EGPIO0		EP93XX_GPIO_LINE_A(0)
> -#define EP93XX_GPIO_LINE_EGPIO1		EP93XX_GPIO_LINE_A(1)
> -#define EP93XX_GPIO_LINE_EGPIO2		EP93XX_GPIO_LINE_A(2)
> -#define EP93XX_GPIO_LINE_EGPIO3		EP93XX_GPIO_LINE_A(3)
> -#define EP93XX_GPIO_LINE_EGPIO4		EP93XX_GPIO_LINE_A(4)
> -#define EP93XX_GPIO_LINE_EGPIO5		EP93XX_GPIO_LINE_A(5)
> -#define EP93XX_GPIO_LINE_EGPIO6		EP93XX_GPIO_LINE_A(6)
> -#define EP93XX_GPIO_LINE_EGPIO7		EP93XX_GPIO_LINE_A(7)
> +/* new generic GPIO API - see Documentation/gpio.txt */

Russell's patch removed this comment.  I don't see any reason to
put it back.

>  
> -/* GPIO port B.  */
> -#define EP93XX_GPIO_LINE_B(x)		((x) + 8)
> -#define EP93XX_GPIO_LINE_EGPIO8		EP93XX_GPIO_LINE_B(0)
> -#define EP93XX_GPIO_LINE_EGPIO9		EP93XX_GPIO_LINE_B(1)
> -#define EP93XX_GPIO_LINE_EGPIO10	EP93XX_GPIO_LINE_B(2)
> -#define EP93XX_GPIO_LINE_EGPIO11	EP93XX_GPIO_LINE_B(3)
> -#define EP93XX_GPIO_LINE_EGPIO12	EP93XX_GPIO_LINE_B(4)
> -#define EP93XX_GPIO_LINE_EGPIO13	EP93XX_GPIO_LINE_B(5)
> -#define EP93XX_GPIO_LINE_EGPIO14	EP93XX_GPIO_LINE_B(6)
> -#define EP93XX_GPIO_LINE_EGPIO15	EP93XX_GPIO_LINE_B(7)
> +#include <asm-generic/gpio.h>

I believe Russell's patch moves this include to arm's <asm/gpio.h>.
Having it included here is a bit redundant.

> +#include "gpio-ep93xx.h"

Why this form?  Isn't <mach/gpio-ep93xx.h> the preferred form?
 
> -/* GPIO port C.  */
> -#define EP93XX_GPIO_LINE_C(x)		((x) + 40)
> -#define EP93XX_GPIO_LINE_ROW0		EP93XX_GPIO_LINE_C(0)
> -#define EP93XX_GPIO_LINE_ROW1		EP93XX_GPIO_LINE_C(1)
> -#define EP93XX_GPIO_LINE_ROW2		EP93XX_GPIO_LINE_C(2)
> -#define EP93XX_GPIO_LINE_ROW3		EP93XX_GPIO_LINE_C(3)
> -#define EP93XX_GPIO_LINE_ROW4		EP93XX_GPIO_LINE_C(4)
> -#define EP93XX_GPIO_LINE_ROW5		EP93XX_GPIO_LINE_C(5)
> -#define EP93XX_GPIO_LINE_ROW6		EP93XX_GPIO_LINE_C(6)
> -#define EP93XX_GPIO_LINE_ROW7		EP93XX_GPIO_LINE_C(7)
> -
> -/* GPIO port D.  */
> -#define EP93XX_GPIO_LINE_D(x)		((x) + 24)
> -#define EP93XX_GPIO_LINE_COL0		EP93XX_GPIO_LINE_D(0)
> -#define EP93XX_GPIO_LINE_COL1		EP93XX_GPIO_LINE_D(1)
> -#define EP93XX_GPIO_LINE_COL2		EP93XX_GPIO_LINE_D(2)
> -#define EP93XX_GPIO_LINE_COL3		EP93XX_GPIO_LINE_D(3)
> -#define EP93XX_GPIO_LINE_COL4		EP93XX_GPIO_LINE_D(4)
> -#define EP93XX_GPIO_LINE_COL5		EP93XX_GPIO_LINE_D(5)
> -#define EP93XX_GPIO_LINE_COL6		EP93XX_GPIO_LINE_D(6)
> -#define EP93XX_GPIO_LINE_COL7		EP93XX_GPIO_LINE_D(7)
> -
> -/* GPIO port E.  */
> -#define EP93XX_GPIO_LINE_E(x)		((x) + 32)
> -#define EP93XX_GPIO_LINE_GRLED		EP93XX_GPIO_LINE_E(0)
> -#define EP93XX_GPIO_LINE_RDLED		EP93XX_GPIO_LINE_E(1)
> -#define EP93XX_GPIO_LINE_DIORn		EP93XX_GPIO_LINE_E(2)
> -#define EP93XX_GPIO_LINE_IDECS1n	EP93XX_GPIO_LINE_E(3)
> -#define EP93XX_GPIO_LINE_IDECS2n	EP93XX_GPIO_LINE_E(4)
> -#define EP93XX_GPIO_LINE_IDEDA0		EP93XX_GPIO_LINE_E(5)
> -#define EP93XX_GPIO_LINE_IDEDA1		EP93XX_GPIO_LINE_E(6)
> -#define EP93XX_GPIO_LINE_IDEDA2		EP93XX_GPIO_LINE_E(7)
> -
> -/* GPIO port F.  */
> -#define EP93XX_GPIO_LINE_F(x)		((x) + 16)
> -#define EP93XX_GPIO_LINE_WP		EP93XX_GPIO_LINE_F(0)
> -#define EP93XX_GPIO_LINE_MCCD1		EP93XX_GPIO_LINE_F(1)
> -#define EP93XX_GPIO_LINE_MCCD2		EP93XX_GPIO_LINE_F(2)
> -#define EP93XX_GPIO_LINE_MCBVD1		EP93XX_GPIO_LINE_F(3)
> -#define EP93XX_GPIO_LINE_MCBVD2		EP93XX_GPIO_LINE_F(4)
> -#define EP93XX_GPIO_LINE_VS1		EP93XX_GPIO_LINE_F(5)
> -#define EP93XX_GPIO_LINE_READY		EP93XX_GPIO_LINE_F(6)
> -#define EP93XX_GPIO_LINE_VS2		EP93XX_GPIO_LINE_F(7)
> -
> -/* GPIO port G.  */
> -#define EP93XX_GPIO_LINE_G(x)		((x) + 48)
> -#define EP93XX_GPIO_LINE_EECLK		EP93XX_GPIO_LINE_G(0)
> -#define EP93XX_GPIO_LINE_EEDAT		EP93XX_GPIO_LINE_G(1)
> -#define EP93XX_GPIO_LINE_SLA0		EP93XX_GPIO_LINE_G(2)
> -#define EP93XX_GPIO_LINE_SLA1		EP93XX_GPIO_LINE_G(3)
> -#define EP93XX_GPIO_LINE_DD12		EP93XX_GPIO_LINE_G(4)
> -#define EP93XX_GPIO_LINE_DD13		EP93XX_GPIO_LINE_G(5)
> -#define EP93XX_GPIO_LINE_DD14		EP93XX_GPIO_LINE_G(6)
> -#define EP93XX_GPIO_LINE_DD15		EP93XX_GPIO_LINE_G(7)
> -
> -/* GPIO port H.  */
> -#define EP93XX_GPIO_LINE_H(x)		((x) + 56)
> -#define EP93XX_GPIO_LINE_DD0		EP93XX_GPIO_LINE_H(0)
> -#define EP93XX_GPIO_LINE_DD1		EP93XX_GPIO_LINE_H(1)
> -#define EP93XX_GPIO_LINE_DD2		EP93XX_GPIO_LINE_H(2)
> -#define EP93XX_GPIO_LINE_DD3		EP93XX_GPIO_LINE_H(3)
> -#define EP93XX_GPIO_LINE_DD4		EP93XX_GPIO_LINE_H(4)
> -#define EP93XX_GPIO_LINE_DD5		EP93XX_GPIO_LINE_H(5)
> -#define EP93XX_GPIO_LINE_DD6		EP93XX_GPIO_LINE_H(6)
> -#define EP93XX_GPIO_LINE_DD7		EP93XX_GPIO_LINE_H(7)
> -
> -/* maximum value for gpio line identifiers */
> -#define EP93XX_GPIO_LINE_MAX		EP93XX_GPIO_LINE_H(7)
> -
> -/* maximum value for irq capable line identifiers */
> -#define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
> +#define gpio_get_value	__gpio_get_value
> +#define gpio_set_value	__gpio_set_value
> +#define gpio_cansleep	__gpio_cansleep
>  
>  /*
>   * Map GPIO A0..A7  (0..7)  to irq 64..71,
> diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
> index 8392e95..1a472ff 100644
> --- a/arch/arm/mach-ep93xx/simone.c
> +++ b/arch/arm/mach-ep93xx/simone.c
> @@ -18,12 +18,12 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
> -#include <linux/gpio.h>
>  #include <linux/i2c.h>
>  #include <linux/i2c-gpio.h>
>  
>  #include <mach/hardware.h>
>  #include <mach/fb.h>
> +#include <mach/gpio-ep93xx.h>

Here I can kind of agree on the removal of <linux/gpio.h> and adding <mach/gpio-ep93xx.h>.

This file does not use any of the gpiolib calls.  It just needs to pick up the defines
for the two gpio pins used for the I2C bus.

Still, it seems a bit awkward....

>  #include <asm/mach-types.h>
>  #include <asm/mach/arch.h>
> diff --git a/arch/arm/mach-ep93xx/snappercl15.c b/arch/arm/mach-ep93xx/snappercl15.c
> index 2e9c614..4f4b0b2 100644
> --- a/arch/arm/mach-ep93xx/snappercl15.c
> +++ b/arch/arm/mach-ep93xx/snappercl15.c
> @@ -20,7 +20,6 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> -#include <linux/gpio.h>
>  #include <linux/i2c.h>
>  #include <linux/i2c-gpio.h>
>  #include <linux/fb.h>
> @@ -30,6 +29,7 @@
>  
>  #include <mach/hardware.h>
>  #include <mach/fb.h>
> +#include <mach/gpio-ep93xx.h>
>  

Same comment here. It's technically correct but it's seems awkward.

>  #include <asm/mach-types.h>
>  #include <asm/mach/arch.h>

Regards,
Hartley

Linus Walleij Aug. 11, 2011, 12:29 p.m. UTC | #2

On Wed, Aug 10, 2011 at 7:23 PM, H Hartley Sweeten
<hartleys@visionengravers.com> wrote:
> On Wednesday, August 10, 2011 5:18 AM, Linus Walleij wrote:

> I'm a bit confused by the intentions of this patch.  Please see below.

The intentions are in path 0, in this case specifically to
let gpio.h be only about generic GPIO and gpiolib and
gpio-<foo>.h be platform and driver specifics.

>> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
>> index c60f081..94c78bc 100644
>> --- a/arch/arm/mach-ep93xx/core.c
>> +++ b/arch/arm/mach-ep93xx/core.c
>> @@ -38,6 +38,7 @@
>>  #include <mach/fb.h>
>>  #include <mach/ep93xx_keypad.h>
>>  #include <mach/ep93xx_spi.h>
>> +#include <mach/gpio-ep93xx.h>
>
> Why is this additional include needed?  With your change to the ep93xx
> <mach/gpio.h> below, this header is already included by <linux/gpio.h>.

Yes, but it is included in <mach/gpio.h> because the quick
generic gpio macros in there use them, not because it
wants to expose symbols from <mach/gpio-ep93xx.h> to
the entire system, that is just a side effect due to
low encapsulation.

> Since this file actually uses the gpiolib calls, the include of
> <linux/gpio.h> is needed and appropriate.

Yes that one *also*.

Because you do both generic and driver-specific
operations.

The intention is not to minimize the number of #include<>
statements (if we want that we could always rely on
implicit includes from other files, which is an abomonation),
the intention is separation of concerns.

> Question... Shouldn't the gpio_to_irq and irq_to_gpio defines also be moved
> here?

These are currently part of the generic GPIO and gpiolib
since they are defined in <linux/gpio.h> :-/

We will get rid of them eventually but I cannot refactor
the entire universe at once.

>> -#define EP93XX_GPIO_LINE_EGPIO7              EP93XX_GPIO_LINE_A(7)
>> +/* new generic GPIO API - see Documentation/gpio.txt */
>
> Russell's patch removed this comment.  I don't see any reason to
> put it back.

My bad, fixing it up.

>> -/* GPIO port B.  */
>> -#define EP93XX_GPIO_LINE_B(x)                ((x) + 8)
>> -#define EP93XX_GPIO_LINE_EGPIO8              EP93XX_GPIO_LINE_B(0)
>> -#define EP93XX_GPIO_LINE_EGPIO9              EP93XX_GPIO_LINE_B(1)
>> -#define EP93XX_GPIO_LINE_EGPIO10     EP93XX_GPIO_LINE_B(2)
>> -#define EP93XX_GPIO_LINE_EGPIO11     EP93XX_GPIO_LINE_B(3)
>> -#define EP93XX_GPIO_LINE_EGPIO12     EP93XX_GPIO_LINE_B(4)
>> -#define EP93XX_GPIO_LINE_EGPIO13     EP93XX_GPIO_LINE_B(5)
>> -#define EP93XX_GPIO_LINE_EGPIO14     EP93XX_GPIO_LINE_B(6)
>> -#define EP93XX_GPIO_LINE_EGPIO15     EP93XX_GPIO_LINE_B(7)
>> +#include <asm-generic/gpio.h>
>
> I believe Russell's patch moves this include to arm's <asm/gpio.h>.
> Having it included here is a bit redundant.

Fixing it. My error.

>> +#include "gpio-ep93xx.h"
>
> Why this form?  Isn't <mach/gpio-ep93xx.h> the preferred form?

This is to make the inclusion very local. The only reason it is
included at all is to get EP93XX_GPIO_LINE_MAX_IRQ,
nothing else.

>> -/* maximum value for irq capable line identifiers */
>> -#define EP93XX_GPIO_LINE_MAX_IRQ     EP93XX_GPIO_LINE_F(7)
>> +#define gpio_get_value       __gpio_get_value
>> +#define gpio_set_value       __gpio_set_value
>> +#define gpio_cansleep        __gpio_cansleep

You didn't comment on this but it's yet another bug due to
bad rebasing. Fixing it. Thse should not be reintroduced.

>> diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
>> index 8392e95..1a472ff 100644
>> --- a/arch/arm/mach-ep93xx/simone.c
>> +++ b/arch/arm/mach-ep93xx/simone.c
>> @@ -18,12 +18,12 @@
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>>  #include <linux/platform_device.h>
>> -#include <linux/gpio.h>
>>  #include <linux/i2c.h>
>>  #include <linux/i2c-gpio.h>
>>
>>  #include <mach/hardware.h>
>>  #include <mach/fb.h>
>> +#include <mach/gpio-ep93xx.h>
>
> Here I can kind of agree on the removal of <linux/gpio.h> and adding <mach/gpio-ep93xx.h>.
>
> This file does not use any of the gpiolib calls.  It just needs to pick up the defines
> for the two gpio pins used for the I2C bus.

Yes that's the idea.

> Still, it seems a bit awkward....

In what way?

> Same comment here. It's technically correct but it's seems awkward.

Define awkward.

I know you can get the same *implicitly* from <linux/gpio.h>
but that is the awkward problem we're trying to get rid of.

Eventually <linux/gpio.h> will *not* bring in *any* platform-
or driver-specific crap. Not today, but as soon as we get
somewhere with the single image concept.

Similarly, platforms with GPIO not using generic GPIO
or gpiolib at all have simply had their headers renamed
<mach/gpio-foo.h>.

Atleast that's my idea, I guess someone may smack my
fingers.

Thanks,
Linus Walleij

Hartley Sweeten Aug. 11, 2011, 5:16 p.m. UTC | #3

On Thursday, August 11, 2011 5:29 AM, Linus Walleij wrote:
> On Wed, Aug 10, 2011 at 7:23 PM, H Hartley Sweeten wrote:
>> On Wednesday, August 10, 2011 5:18 AM, Linus Walleij wrote:
>
>> I'm a bit confused by the intentions of this patch.  Please see below.
>
> The intentions are in path 0, in this case specifically to
> let gpio.h be only about generic GPIO and gpiolib and
> gpio-<foo>.h be platform and driver specifics.

OK.  I overlooked that part.  Thanks.

>>> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
>>> index c60f081..94c78bc 100644
>>> --- a/arch/arm/mach-ep93xx/core.c
>>> +++ b/arch/arm/mach-ep93xx/core.c
>>> @@ -38,6 +38,7 @@
>>>  #include <mach/fb.h>
>>>  #include <mach/ep93xx_keypad.h>
>>>  #include <mach/ep93xx_spi.h>
>>> +#include <mach/gpio-ep93xx.h>
>>
>> Why is this additional include needed?  With your change to the ep93xx
>> <mach/gpio.h> below, this header is already included by <linux/gpio.h>.
>
> Yes, but it is included in <mach/gpio.h> because the quick
> generic gpio macros in there use them, not because it
> wants to expose symbols from <mach/gpio-ep93xx.h> to
> the entire system, that is just a side effect due to
> low encapsulation.
>
>> Since this file actually uses the gpiolib calls, the include of
>> <linux/gpio.h> is needed and appropriate.
>
> Yes that one *also*.
>
> Because you do both generic and driver-specific
> operations.

Hmmm.. Ok, I guess I see your point.

> The intention is not to minimize the number of #include<>
> statements (if we want that we could always rely on
> implicit includes from other files, which is an abomonation),
> the intention is separation of concerns.

Of course.  Any header that is "needed" by the source _should_ be included.
Relying on them being included by other headers is just asking for trouble.

>> Question... Shouldn't the gpio_to_irq and irq_to_gpio defines also be moved
>> here?
>
> These are currently part of the generic GPIO and gpiolib
> since they are defined in <linux/gpio.h> :-/

I see your point.

> We will get rid of them eventually but I cannot refactor
> the entire universe at once.
>
>>> -#define EP93XX_GPIO_LINE_EGPIO7              EP93XX_GPIO_LINE_A(7)
>>> +/* new generic GPIO API - see Documentation/gpio.txt */
>>
>> Russell's patch removed this comment.  I don't see any reason to
>> put it back.
>
> My bad, fixing it up.

BTW, it looks like Russell's patches hit linux-next this morning.

>>> -/* GPIO port B.  */
>>> -#define EP93XX_GPIO_LINE_B(x)                ((x) + 8)
>>> -#define EP93XX_GPIO_LINE_EGPIO8              EP93XX_GPIO_LINE_B(0)
>> -#define EP93XX_GPIO_LINE_EGPIO9              EP93XX_GPIO_LINE_B(1)
>>> -#define EP93XX_GPIO_LINE_EGPIO10     EP93XX_GPIO_LINE_B(2)
>>> -#define EP93XX_GPIO_LINE_EGPIO11     EP93XX_GPIO_LINE_B(3)
>>> -#define EP93XX_GPIO_LINE_EGPIO12     EP93XX_GPIO_LINE_B(4)
>>> -#define EP93XX_GPIO_LINE_EGPIO13     EP93XX_GPIO_LINE_B(5)
>>> -#define EP93XX_GPIO_LINE_EGPIO14     EP93XX_GPIO_LINE_B(6)
>>> -#define EP93XX_GPIO_LINE_EGPIO15     EP93XX_GPIO_LINE_B(7)
>>> +#include <asm-generic/gpio.h>
>>
>> I believe Russell's patch moves this include to arm's <asm/gpio.h>.
>> Having it included here is a bit redundant.
>
> Fixing it. My error.

;-)

>>> +#include "gpio-ep93xx.h"
>>
>> Why this form?  Isn't <mach/gpio-ep93xx.h> the preferred form?
>
> This is to make the inclusion very local. The only reason it is
> included at all is to get EP93XX_GPIO_LINE_MAX_IRQ,
> nothing else.

Hmm..  It is needed for gpio_to_irq()...

The only other user of that define is drivers/gpio/gpio-ep93xx.c.

To follow the intentions of this patch it would be nice to remove this 
include from <mach/gpio.h> and add <mach/gpio-ep93xx.h> to the gpio driver.
But, any user of gpio_to_irq() whould also have to include <mach/gpio-ep93xx.h>
also.

The other solution is to hook up the gpiolib to_irq callback in
gpio-ep93xx and do:

#define gpio_to_irq	__gpio_to_irq

Maybe this is a better option?

>>> -/* maximum value for irq capable line identifiers */
>>> -#define EP93XX_GPIO_LINE_MAX_IRQ     EP93XX_GPIO_LINE_F(7)
>>> +#define gpio_get_value       __gpio_get_value
>>> +#define gpio_set_value       __gpio_set_value
>>> +#define gpio_cansleep        __gpio_cansleep
>
> You didn't comment on this but it's yet another bug due to
> bad rebasing. Fixing it. Thse should not be reintroduced.

I noticed this also but figured you would see it when you rebased
on top of Russell's patches.

>>> diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
>>> index 8392e95..1a472ff 100644
>>> --- a/arch/arm/mach-ep93xx/simone.c
>>> +++ b/arch/arm/mach-ep93xx/simone.c
>>> @@ -18,12 +18,12 @@
>>>  #include <linux/kernel.h>
>>>  #include <linux/init.h>
>>>  #include <linux/platform_device.h>
>>> -#include <linux/gpio.h>
>>>  #include <linux/i2c.h>
>>>  #include <linux/i2c-gpio.h>
>>>
>>>  #include <mach/hardware.h>
>>>  #include <mach/fb.h>
>>> +#include <mach/gpio-ep93xx.h>
>>
>> Here I can kind of agree on the removal of <linux/gpio.h> and adding <mach/gpio-ep93xx.h>.
>>
>> This file does not use any of the gpiolib calls.  It just needs to pick up the defines
>> for the two gpio pins used for the I2C bus.
>
> Yes that's the idea.

OK.  I get it know.

>> Still, it seems a bit awkward....
>
> In what way?

It seemed awkward only in the fact that including <linux/gpio.h> was implying that this
file was asking for gpio support for the ep93xx.  Including the mach specific file instead
just reads a bit strange.  I'll get over it.

>> Same comment here. It's technically correct but it's seems awkward.
>
> Define awkward.
> 
> I know you can get the same *implicitly* from <linux/gpio.h>
> but that is the awkward problem we're trying to get rid of.
> 
> Eventually <linux/gpio.h> will *not* bring in *any* platform-
> or driver-specific crap. Not today, but as soon as we get
> somewhere with the single image concept.
> 
> Similarly, platforms with GPIO not using generic GPIO
> or gpiolib at all have simply had their headers renamed
> <mach/gpio-foo.h>.
> 
> Atleast that's my idea, I guess someone may smack my
> fingers.

Thanks for the clarifications.  Please rebase on top of Russell's patches and
I'll take a look at them again.

Regards,
Hartley

diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index c60f081..94c78bc 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -38,6 +38,7 @@ 
 #include <mach/fb.h>
 #include <mach/ep93xx_keypad.h>
 #include <mach/ep93xx_spi.h>
+#include <mach/gpio-ep93xx.h>
 
 #include <asm/mach/map.h>
 #include <asm/mach/time.h>
diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c
index 9969bb1..3f320c6 100644
--- a/arch/arm/mach-ep93xx/edb93xx.c
+++ b/arch/arm/mach-ep93xx/edb93xx.c
@@ -37,6 +37,7 @@ 
 #include <mach/hardware.h>
 #include <mach/fb.h>
 #include <mach/ep93xx_spi.h>
+#include <mach/gpio-ep93xx.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
diff --git a/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h b/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h
new file mode 100644
index 0000000..8aff2ea
--- /dev/null
+++ b/arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h
@@ -0,0 +1,100 @@ 
+/* Include file for the EP93XX GPIO controller machine specifics */
+
+#ifndef __GPIO_EP93XX_H
+#define __GPIO_EP93XX_H
+
+/* GPIO port A.  */
+#define EP93XX_GPIO_LINE_A(x)		((x) + 0)
+#define EP93XX_GPIO_LINE_EGPIO0		EP93XX_GPIO_LINE_A(0)
+#define EP93XX_GPIO_LINE_EGPIO1		EP93XX_GPIO_LINE_A(1)
+#define EP93XX_GPIO_LINE_EGPIO2		EP93XX_GPIO_LINE_A(2)
+#define EP93XX_GPIO_LINE_EGPIO3		EP93XX_GPIO_LINE_A(3)
+#define EP93XX_GPIO_LINE_EGPIO4		EP93XX_GPIO_LINE_A(4)
+#define EP93XX_GPIO_LINE_EGPIO5		EP93XX_GPIO_LINE_A(5)
+#define EP93XX_GPIO_LINE_EGPIO6		EP93XX_GPIO_LINE_A(6)
+#define EP93XX_GPIO_LINE_EGPIO7		EP93XX_GPIO_LINE_A(7)
+
+/* GPIO port B.  */
+#define EP93XX_GPIO_LINE_B(x)		((x) + 8)
+#define EP93XX_GPIO_LINE_EGPIO8		EP93XX_GPIO_LINE_B(0)
+#define EP93XX_GPIO_LINE_EGPIO9		EP93XX_GPIO_LINE_B(1)
+#define EP93XX_GPIO_LINE_EGPIO10	EP93XX_GPIO_LINE_B(2)
+#define EP93XX_GPIO_LINE_EGPIO11	EP93XX_GPIO_LINE_B(3)
+#define EP93XX_GPIO_LINE_EGPIO12	EP93XX_GPIO_LINE_B(4)
+#define EP93XX_GPIO_LINE_EGPIO13	EP93XX_GPIO_LINE_B(5)
+#define EP93XX_GPIO_LINE_EGPIO14	EP93XX_GPIO_LINE_B(6)
+#define EP93XX_GPIO_LINE_EGPIO15	EP93XX_GPIO_LINE_B(7)
+
+/* GPIO port C.  */
+#define EP93XX_GPIO_LINE_C(x)		((x) + 40)
+#define EP93XX_GPIO_LINE_ROW0		EP93XX_GPIO_LINE_C(0)
+#define EP93XX_GPIO_LINE_ROW1		EP93XX_GPIO_LINE_C(1)
+#define EP93XX_GPIO_LINE_ROW2		EP93XX_GPIO_LINE_C(2)
+#define EP93XX_GPIO_LINE_ROW3		EP93XX_GPIO_LINE_C(3)
+#define EP93XX_GPIO_LINE_ROW4		EP93XX_GPIO_LINE_C(4)
+#define EP93XX_GPIO_LINE_ROW5		EP93XX_GPIO_LINE_C(5)
+#define EP93XX_GPIO_LINE_ROW6		EP93XX_GPIO_LINE_C(6)
+#define EP93XX_GPIO_LINE_ROW7		EP93XX_GPIO_LINE_C(7)
+
+/* GPIO port D.  */
+#define EP93XX_GPIO_LINE_D(x)		((x) + 24)
+#define EP93XX_GPIO_LINE_COL0		EP93XX_GPIO_LINE_D(0)
+#define EP93XX_GPIO_LINE_COL1		EP93XX_GPIO_LINE_D(1)
+#define EP93XX_GPIO_LINE_COL2		EP93XX_GPIO_LINE_D(2)
+#define EP93XX_GPIO_LINE_COL3		EP93XX_GPIO_LINE_D(3)
+#define EP93XX_GPIO_LINE_COL4		EP93XX_GPIO_LINE_D(4)
+#define EP93XX_GPIO_LINE_COL5		EP93XX_GPIO_LINE_D(5)
+#define EP93XX_GPIO_LINE_COL6		EP93XX_GPIO_LINE_D(6)
+#define EP93XX_GPIO_LINE_COL7		EP93XX_GPIO_LINE_D(7)
+
+/* GPIO port E.  */
+#define EP93XX_GPIO_LINE_E(x)		((x) + 32)
+#define EP93XX_GPIO_LINE_GRLED		EP93XX_GPIO_LINE_E(0)
+#define EP93XX_GPIO_LINE_RDLED		EP93XX_GPIO_LINE_E(1)
+#define EP93XX_GPIO_LINE_DIORn		EP93XX_GPIO_LINE_E(2)
+#define EP93XX_GPIO_LINE_IDECS1n	EP93XX_GPIO_LINE_E(3)
+#define EP93XX_GPIO_LINE_IDECS2n	EP93XX_GPIO_LINE_E(4)
+#define EP93XX_GPIO_LINE_IDEDA0		EP93XX_GPIO_LINE_E(5)
+#define EP93XX_GPIO_LINE_IDEDA1		EP93XX_GPIO_LINE_E(6)
+#define EP93XX_GPIO_LINE_IDEDA2		EP93XX_GPIO_LINE_E(7)
+
+/* GPIO port F.  */
+#define EP93XX_GPIO_LINE_F(x)		((x) + 16)
+#define EP93XX_GPIO_LINE_WP		EP93XX_GPIO_LINE_F(0)
+#define EP93XX_GPIO_LINE_MCCD1		EP93XX_GPIO_LINE_F(1)
+#define EP93XX_GPIO_LINE_MCCD2		EP93XX_GPIO_LINE_F(2)
+#define EP93XX_GPIO_LINE_MCBVD1		EP93XX_GPIO_LINE_F(3)
+#define EP93XX_GPIO_LINE_MCBVD2		EP93XX_GPIO_LINE_F(4)
+#define EP93XX_GPIO_LINE_VS1		EP93XX_GPIO_LINE_F(5)
+#define EP93XX_GPIO_LINE_READY		EP93XX_GPIO_LINE_F(6)
+#define EP93XX_GPIO_LINE_VS2		EP93XX_GPIO_LINE_F(7)
+
+/* GPIO port G.  */
+#define EP93XX_GPIO_LINE_G(x)		((x) + 48)
+#define EP93XX_GPIO_LINE_EECLK		EP93XX_GPIO_LINE_G(0)
+#define EP93XX_GPIO_LINE_EEDAT		EP93XX_GPIO_LINE_G(1)
+#define EP93XX_GPIO_LINE_SLA0		EP93XX_GPIO_LINE_G(2)
+#define EP93XX_GPIO_LINE_SLA1		EP93XX_GPIO_LINE_G(3)
+#define EP93XX_GPIO_LINE_DD12		EP93XX_GPIO_LINE_G(4)
+#define EP93XX_GPIO_LINE_DD13		EP93XX_GPIO_LINE_G(5)
+#define EP93XX_GPIO_LINE_DD14		EP93XX_GPIO_LINE_G(6)
+#define EP93XX_GPIO_LINE_DD15		EP93XX_GPIO_LINE_G(7)
+
+/* GPIO port H.  */
+#define EP93XX_GPIO_LINE_H(x)		((x) + 56)
+#define EP93XX_GPIO_LINE_DD0		EP93XX_GPIO_LINE_H(0)
+#define EP93XX_GPIO_LINE_DD1		EP93XX_GPIO_LINE_H(1)
+#define EP93XX_GPIO_LINE_DD2		EP93XX_GPIO_LINE_H(2)
+#define EP93XX_GPIO_LINE_DD3		EP93XX_GPIO_LINE_H(3)
+#define EP93XX_GPIO_LINE_DD4		EP93XX_GPIO_LINE_H(4)
+#define EP93XX_GPIO_LINE_DD5		EP93XX_GPIO_LINE_H(5)
+#define EP93XX_GPIO_LINE_DD6		EP93XX_GPIO_LINE_H(6)
+#define EP93XX_GPIO_LINE_DD7		EP93XX_GPIO_LINE_H(7)
+
+/* maximum value for gpio line identifiers */
+#define EP93XX_GPIO_LINE_MAX		EP93XX_GPIO_LINE_H(7)
+
+/* maximum value for irq capable line identifiers */
+#define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
+
+#endif /* __GPIO_EP93XX_H */
diff --git a/arch/arm/mach-ep93xx/include/mach/gpio.h b/arch/arm/mach-ep93xx/include/mach/gpio.h
index 071f676..acfc113 100644
--- a/arch/arm/mach-ep93xx/include/mach/gpio.h
+++ b/arch/arm/mach-ep93xx/include/mach/gpio.h
@@ -5,99 +5,14 @@ 
 #ifndef __ASM_ARCH_GPIO_H
 #define __ASM_ARCH_GPIO_H
 
-/* GPIO port A.  */
-#define EP93XX_GPIO_LINE_A(x)		((x) + 0)
-#define EP93XX_GPIO_LINE_EGPIO0		EP93XX_GPIO_LINE_A(0)
-#define EP93XX_GPIO_LINE_EGPIO1		EP93XX_GPIO_LINE_A(1)
-#define EP93XX_GPIO_LINE_EGPIO2		EP93XX_GPIO_LINE_A(2)
-#define EP93XX_GPIO_LINE_EGPIO3		EP93XX_GPIO_LINE_A(3)
-#define EP93XX_GPIO_LINE_EGPIO4		EP93XX_GPIO_LINE_A(4)
-#define EP93XX_GPIO_LINE_EGPIO5		EP93XX_GPIO_LINE_A(5)
-#define EP93XX_GPIO_LINE_EGPIO6		EP93XX_GPIO_LINE_A(6)
-#define EP93XX_GPIO_LINE_EGPIO7		EP93XX_GPIO_LINE_A(7)
+/* new generic GPIO API - see Documentation/gpio.txt */
 
-/* GPIO port B.  */
-#define EP93XX_GPIO_LINE_B(x)		((x) + 8)
-#define EP93XX_GPIO_LINE_EGPIO8		EP93XX_GPIO_LINE_B(0)
-#define EP93XX_GPIO_LINE_EGPIO9		EP93XX_GPIO_LINE_B(1)
-#define EP93XX_GPIO_LINE_EGPIO10	EP93XX_GPIO_LINE_B(2)
-#define EP93XX_GPIO_LINE_EGPIO11	EP93XX_GPIO_LINE_B(3)
-#define EP93XX_GPIO_LINE_EGPIO12	EP93XX_GPIO_LINE_B(4)
-#define EP93XX_GPIO_LINE_EGPIO13	EP93XX_GPIO_LINE_B(5)
-#define EP93XX_GPIO_LINE_EGPIO14	EP93XX_GPIO_LINE_B(6)
-#define EP93XX_GPIO_LINE_EGPIO15	EP93XX_GPIO_LINE_B(7)
+#include <asm-generic/gpio.h>
+#include "gpio-ep93xx.h"
 
-/* GPIO port C.  */
-#define EP93XX_GPIO_LINE_C(x)		((x) + 40)
-#define EP93XX_GPIO_LINE_ROW0		EP93XX_GPIO_LINE_C(0)
-#define EP93XX_GPIO_LINE_ROW1		EP93XX_GPIO_LINE_C(1)
-#define EP93XX_GPIO_LINE_ROW2		EP93XX_GPIO_LINE_C(2)
-#define EP93XX_GPIO_LINE_ROW3		EP93XX_GPIO_LINE_C(3)
-#define EP93XX_GPIO_LINE_ROW4		EP93XX_GPIO_LINE_C(4)
-#define EP93XX_GPIO_LINE_ROW5		EP93XX_GPIO_LINE_C(5)
-#define EP93XX_GPIO_LINE_ROW6		EP93XX_GPIO_LINE_C(6)
-#define EP93XX_GPIO_LINE_ROW7		EP93XX_GPIO_LINE_C(7)
-
-/* GPIO port D.  */
-#define EP93XX_GPIO_LINE_D(x)		((x) + 24)
-#define EP93XX_GPIO_LINE_COL0		EP93XX_GPIO_LINE_D(0)
-#define EP93XX_GPIO_LINE_COL1		EP93XX_GPIO_LINE_D(1)
-#define EP93XX_GPIO_LINE_COL2		EP93XX_GPIO_LINE_D(2)
-#define EP93XX_GPIO_LINE_COL3		EP93XX_GPIO_LINE_D(3)
-#define EP93XX_GPIO_LINE_COL4		EP93XX_GPIO_LINE_D(4)
-#define EP93XX_GPIO_LINE_COL5		EP93XX_GPIO_LINE_D(5)
-#define EP93XX_GPIO_LINE_COL6		EP93XX_GPIO_LINE_D(6)
-#define EP93XX_GPIO_LINE_COL7		EP93XX_GPIO_LINE_D(7)
-
-/* GPIO port E.  */
-#define EP93XX_GPIO_LINE_E(x)		((x) + 32)
-#define EP93XX_GPIO_LINE_GRLED		EP93XX_GPIO_LINE_E(0)
-#define EP93XX_GPIO_LINE_RDLED		EP93XX_GPIO_LINE_E(1)
-#define EP93XX_GPIO_LINE_DIORn		EP93XX_GPIO_LINE_E(2)
-#define EP93XX_GPIO_LINE_IDECS1n	EP93XX_GPIO_LINE_E(3)
-#define EP93XX_GPIO_LINE_IDECS2n	EP93XX_GPIO_LINE_E(4)
-#define EP93XX_GPIO_LINE_IDEDA0		EP93XX_GPIO_LINE_E(5)
-#define EP93XX_GPIO_LINE_IDEDA1		EP93XX_GPIO_LINE_E(6)
-#define EP93XX_GPIO_LINE_IDEDA2		EP93XX_GPIO_LINE_E(7)
-
-/* GPIO port F.  */
-#define EP93XX_GPIO_LINE_F(x)		((x) + 16)
-#define EP93XX_GPIO_LINE_WP		EP93XX_GPIO_LINE_F(0)
-#define EP93XX_GPIO_LINE_MCCD1		EP93XX_GPIO_LINE_F(1)
-#define EP93XX_GPIO_LINE_MCCD2		EP93XX_GPIO_LINE_F(2)
-#define EP93XX_GPIO_LINE_MCBVD1		EP93XX_GPIO_LINE_F(3)
-#define EP93XX_GPIO_LINE_MCBVD2		EP93XX_GPIO_LINE_F(4)
-#define EP93XX_GPIO_LINE_VS1		EP93XX_GPIO_LINE_F(5)
-#define EP93XX_GPIO_LINE_READY		EP93XX_GPIO_LINE_F(6)
-#define EP93XX_GPIO_LINE_VS2		EP93XX_GPIO_LINE_F(7)
-
-/* GPIO port G.  */
-#define EP93XX_GPIO_LINE_G(x)		((x) + 48)
-#define EP93XX_GPIO_LINE_EECLK		EP93XX_GPIO_LINE_G(0)
-#define EP93XX_GPIO_LINE_EEDAT		EP93XX_GPIO_LINE_G(1)
-#define EP93XX_GPIO_LINE_SLA0		EP93XX_GPIO_LINE_G(2)
-#define EP93XX_GPIO_LINE_SLA1		EP93XX_GPIO_LINE_G(3)
-#define EP93XX_GPIO_LINE_DD12		EP93XX_GPIO_LINE_G(4)
-#define EP93XX_GPIO_LINE_DD13		EP93XX_GPIO_LINE_G(5)
-#define EP93XX_GPIO_LINE_DD14		EP93XX_GPIO_LINE_G(6)
-#define EP93XX_GPIO_LINE_DD15		EP93XX_GPIO_LINE_G(7)
-
-/* GPIO port H.  */
-#define EP93XX_GPIO_LINE_H(x)		((x) + 56)
-#define EP93XX_GPIO_LINE_DD0		EP93XX_GPIO_LINE_H(0)
-#define EP93XX_GPIO_LINE_DD1		EP93XX_GPIO_LINE_H(1)
-#define EP93XX_GPIO_LINE_DD2		EP93XX_GPIO_LINE_H(2)
-#define EP93XX_GPIO_LINE_DD3		EP93XX_GPIO_LINE_H(3)
-#define EP93XX_GPIO_LINE_DD4		EP93XX_GPIO_LINE_H(4)
-#define EP93XX_GPIO_LINE_DD5		EP93XX_GPIO_LINE_H(5)
-#define EP93XX_GPIO_LINE_DD6		EP93XX_GPIO_LINE_H(6)
-#define EP93XX_GPIO_LINE_DD7		EP93XX_GPIO_LINE_H(7)
-
-/* maximum value for gpio line identifiers */
-#define EP93XX_GPIO_LINE_MAX		EP93XX_GPIO_LINE_H(7)
-
-/* maximum value for irq capable line identifiers */
-#define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
+#define gpio_get_value	__gpio_get_value
+#define gpio_set_value	__gpio_set_value
+#define gpio_cansleep	__gpio_cansleep
 
 /*
  * Map GPIO A0..A7  (0..7)  to irq 64..71,
diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
index 8392e95..1a472ff 100644
--- a/arch/arm/mach-ep93xx/simone.c
+++ b/arch/arm/mach-ep93xx/simone.c
@@ -18,12 +18,12 @@ 
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/i2c-gpio.h>
 
 #include <mach/hardware.h>
 #include <mach/fb.h>
+#include <mach/gpio-ep93xx.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
diff --git a/arch/arm/mach-ep93xx/snappercl15.c b/arch/arm/mach-ep93xx/snappercl15.c
index 2e9c614..4f4b0b2 100644
--- a/arch/arm/mach-ep93xx/snappercl15.c
+++ b/arch/arm/mach-ep93xx/snappercl15.c
@@ -20,7 +20,6 @@ 
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/io.h>
-#include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/i2c-gpio.h>
 #include <linux/fb.h>
@@ -30,6 +29,7 @@ 
 
 #include <mach/hardware.h>
 #include <mach/fb.h>
+#include <mach/gpio-ep93xx.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>

[10/19] mach-ep93xx: break out GPIO driver specifics

Commit Message

Comments

Patch