ARM: ixp4xx gpiolib support
 

This patch adds gpiolib support for the IXP4xx platform

Signed-off-by: Imre Kaloz <kaloz@openwrt.org>
---
arch/arm/Kconfig | 2 +-
arch/arm/mach-ixp4xx/common.c | 39 +++++++++++++++++++++++++
arch/arm/mach-ixp4xx/include/mach/gpio.h | 46 ++++++++++--------------------
3 files changed, 55 insertions(+), 32 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 3269576..e793a75 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -481,7 +481,7 @@ config ARCH_IXP4XX
depends on MMU
select CLKSRC_MMIO
select CPU_XSCALE
- select GENERIC_GPIO
+ select ARCH_REQUIRE_GPIOLIB
select GENERIC_CLOCKEVENTS
select HAVE_SCHED_CLOCK
select MIGHT_HAVE_PCI
diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 0777257..7603456 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -36,6 +36,7 @@
#include <asm/page.h>
#include <asm/irq.h>
#include <asm/sched_clock.h>
+#include <asm/gpio.h>

#include <asm/mach/map.h>
#include <asm/mach/irq.h>
@@ -375,12 +376,50 @@ static struct platform_device *ixp46x_devices[] __initdata = {
unsigned long ixp4xx_exp_bus_size;
EXPORT_SYMBOL(ixp4xx_exp_bus_size);

+static int ixp4xx_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
+{
+ gpio_line_config(gpio, IXP4XX_GPIO_IN);
+ return 0;
+}
+
+static int ixp4xx_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
+{
+ gpio_line_set(gpio, level);
+ gpio_line_config(gpio, IXP4XX_GPIO_OUT);
+ return 0;
+}
+
+static int ixp4xx_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
+{
+ int value;
+
+ gpio_line_get(gpio, &value);
+ return value;
+}
+
+static void ixp4xx_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
+{
+ gpio_line_set(gpio, value);
+}
+
+static struct gpio_chip ixp4xx_gpio_chip = {
+ .label = "IXP4XX_GPIO_CHIP",
+ .direction_input = ixp4xx_gpio_direction_input,
+ .direction_output = ixp4xx_gpio_direction_output,
+ .get = ixp4xx_gpio_get_value,
+ .set = ixp4xx_gpio_set_value,
+ .base = 0,
+ .ngpio = 16,
+};
+
void __init ixp4xx_sys_init(void)
{
ixp4xx_exp_bus_size = SZ_16M;

platform_add_devices(ixp4xx_devices, ARRAY_SIZE(ixp4xx_devices));

+ gpiochip_add(&ixp4xx_gpio_chip);
+
if (cpu_is_ixp46x()) {
int region;

diff --git a/arch/arm/mach-ixp4xx/include/mach/gpio.h b/arch/arm/mach-ixp4xx/include/mach/gpio.h
index a5f87de..86f3596 100644
--- a/arch/arm/mach-ixp4xx/include/mach/gpio.h
+++ b/arch/arm/mach-ixp4xx/include/mach/gpio.h
@@ -27,47 +27,31 @@

#include <linux/kernel.h>
#include <mach/hardware.h>
+#include <asm-generic/gpio.h> /* cansleep wrappers */

-static inline int gpio_request(unsigned gpio, const char *label)
-{
- return 0;
-}
-
-static inline void gpio_free(unsigned gpio)
-{
- might_sleep();
-
- return;
-}
-
-static inline int gpio_direction_input(unsigned gpio)
-{
- gpio_line_config(gpio, IXP4XX_GPIO_IN);
- return 0;
-}
-
-static inline int gpio_direction_output(unsigned gpio, int level)
-{
- gpio_line_set(gpio, level);
- gpio_line_config(gpio, IXP4XX_GPIO_OUT);
- return 0;
-}
+#define NR_BUILTIN_GPIO 16

static inline int gpio_get_value(unsigned gpio)
{
- int value;
-
- gpio_line_get(gpio, &value);
-
- return value;
+ if (gpio < NR_BUILTIN_GPIO)
+ {
+ int value;
+ gpio_line_get(gpio, &value);
+ return value;
+ }
+ else
+ return __gpio_get_value(gpio);
}

static inline void gpio_set_value(unsigned gpio, int value)
{
- gpio_line_set(gpio, value);
+ if (gpio < NR_BUILTIN_GPIO)
+ gpio_line_set(gpio, value);
+ else
+ __gpio_set_value(gpio, value);
}

-#include <asm-generic/gpio.h> /* cansleep wrappers */
+#define gpio_cansleep __gpio_cansleep

extern int gpio_to_irq(int gpio);
extern int irq_to_gpio(unsigned int irq);
--
1.7.1


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 1315226991-3835-1-git-send-email-kaloz@openwrt.org">http://lists.debian.org/1315226991-3835-1-git-send-email-kaloz@openwrt.org

ARM: ixp4xx gpiolib support
 

Imre Kaloz <kaloz@openwrt.org> writes:

Hi,

I can't test it but it looks good. Just small nitpicks. See below.

> This patch adds gpiolib support for the IXP4xx platform
>
> Signed-off-by: Imre Kaloz <kaloz@openwrt.org>
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm/mach-ixp4xx/common.c | 39 +++++++++++++++++++++++++
> arch/arm/mach-ixp4xx/include/mach/gpio.h | 46 ++++++++++--------------------
> 3 files changed, 55 insertions(+), 32 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3269576..e793a75 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -481,7 +481,7 @@ config ARCH_IXP4XX
> depends on MMU
> select CLKSRC_MMIO
> select CPU_XSCALE
> - select GENERIC_GPIO
> + select ARCH_REQUIRE_GPIOLIB
> select GENERIC_CLOCKEVENTS
> select HAVE_SCHED_CLOCK
> select MIGHT_HAVE_PCI
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 0777257..7603456 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -36,6 +36,7 @@
> #include <asm/page.h>
> #include <asm/irq.h>
> #include <asm/sched_clock.h>
> +#include <asm/gpio.h>
>
> #include <asm/mach/map.h>
> #include <asm/mach/irq.h>
> @@ -375,12 +376,50 @@ static struct platform_device *ixp46x_devices[] __initdata = {
> unsigned long ixp4xx_exp_bus_size;
> EXPORT_SYMBOL(ixp4xx_exp_bus_size);
>
> +static int ixp4xx_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
> +{
> + gpio_line_config(gpio, IXP4XX_GPIO_IN);
> + return 0;
> +}
> +
> +static int ixp4xx_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
> +{
> + gpio_line_set(gpio, level);
> + gpio_line_config(gpio, IXP4XX_GPIO_OUT);
> + return 0;
> +}
> +
> +static int ixp4xx_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
> +{
> + int value;
> +
> + gpio_line_get(gpio, &value);
> + return value;
> +}
> +
> +static void ixp4xx_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
> +{
> + gpio_line_set(gpio, value);
> +}
> +
> +static struct gpio_chip ixp4xx_gpio_chip = {
> + .label = "IXP4XX_GPIO_CHIP",
> + .direction_input = ixp4xx_gpio_direction_input,
> + .direction_output = ixp4xx_gpio_direction_output,
> + .get = ixp4xx_gpio_get_value,
> + .set = ixp4xx_gpio_set_value,
> + .base = 0,
> + .ngpio = 16,

Use NR_BUILTIN_GPIO instead of 16 ?

> +};
> +
> void __init ixp4xx_sys_init(void)
> {
> ixp4xx_exp_bus_size = SZ_16M;
>
> platform_add_devices(ixp4xx_devices, ARRAY_SIZE(ixp4xx_devices));
>
> + gpiochip_add(&ixp4xx_gpio_chip);
> +
> if (cpu_is_ixp46x()) {
> int region;
>
> diff --git a/arch/arm/mach-ixp4xx/include/mach/gpio.h b/arch/arm/mach-ixp4xx/include/mach/gpio.h
> index a5f87de..86f3596 100644
> --- a/arch/arm/mach-ixp4xx/include/mach/gpio.h
> +++ b/arch/arm/mach-ixp4xx/include/mach/gpio.h
> @@ -27,47 +27,31 @@
>
> #include <linux/kernel.h>
> #include <mach/hardware.h>
> +#include <asm-generic/gpio.h> /* cansleep wrappers */
>
> -static inline int gpio_request(unsigned gpio, const char *label)
> -{
> - return 0;
> -}
> -
> -static inline void gpio_free(unsigned gpio)
> -{
> - might_sleep();
> -
> - return;
> -}
> -
> -static inline int gpio_direction_input(unsigned gpio)
> -{
> - gpio_line_config(gpio, IXP4XX_GPIO_IN);
> - return 0;
> -}
> -
> -static inline int gpio_direction_output(unsigned gpio, int level)
> -{
> - gpio_line_set(gpio, level);
> - gpio_line_config(gpio, IXP4XX_GPIO_OUT);
> - return 0;
> -}
> +#define NR_BUILTIN_GPIO 16
>
> static inline int gpio_get_value(unsigned gpio)
> {
> - int value;
> -
> - gpio_line_get(gpio, &value);
> -
> - return value;
> + if (gpio < NR_BUILTIN_GPIO)
> + {
> + int value;
> + gpio_line_get(gpio, &value);
> + return value;
> + }
> + else
> + return __gpio_get_value(gpio);

Please use
if () {
} else
I would also put the 'int value' declaration outside the if ().


Thanks,
Arnaud


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 871uvu6u89.fsf@lebrac.rtp-net.org">http://lists.debian.org/871uvu6u89.fsf@lebrac.rtp-net.org

ARM: ixp4xx gpiolib support
 

Hello,

On Mon, Sep 05, 2011 at 02:49:51PM +0200, Imre Kaloz wrote:
> This patch adds gpiolib support for the IXP4xx platform
>
> Signed-off-by: Imre Kaloz <kaloz@openwrt.org>
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm/mach-ixp4xx/common.c | 39 +++++++++++++++++++++++++
> arch/arm/mach-ixp4xx/include/mach/gpio.h | 46 ++++++++++--------------------
> 3 files changed, 55 insertions(+), 32 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3269576..e793a75 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -481,7 +481,7 @@ config ARCH_IXP4XX
> depends on MMU
> select CLKSRC_MMIO
> select CPU_XSCALE
> - select GENERIC_GPIO
> + select ARCH_REQUIRE_GPIOLIB
> select GENERIC_CLOCKEVENTS
> select HAVE_SCHED_CLOCK
> select MIGHT_HAVE_PCI
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 0777257..7603456 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -36,6 +36,7 @@
> #include <asm/page.h>
> #include <asm/irq.h>
> #include <asm/sched_clock.h>
> +#include <asm/gpio.h>
>
> #include <asm/mach/map.h>
> #include <asm/mach/irq.h>
> @@ -375,12 +376,50 @@ static struct platform_device *ixp46x_devices[] __initdata = {
> unsigned long ixp4xx_exp_bus_size;
> EXPORT_SYMBOL(ixp4xx_exp_bus_size);
>
> +static int ixp4xx_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
> +{
> + gpio_line_config(gpio, IXP4XX_GPIO_IN);
> + return 0;
> +}
> +
> +static int ixp4xx_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int level)
> +{
> + gpio_line_set(gpio, level);
> + gpio_line_config(gpio, IXP4XX_GPIO_OUT);
> + return 0;
> +}
> +
> +static int ixp4xx_gpio_get_value(struct gpio_chip *chip, unsigned gpio)
> +{
> + int value;
> +
> + gpio_line_get(gpio, &value);
> + return value;
> +}
> +
> +static void ixp4xx_gpio_set_value(struct gpio_chip *chip, unsigned gpio, int value)
> +{
> + gpio_line_set(gpio, value);
> +}
> +
> +static struct gpio_chip ixp4xx_gpio_chip = {
> + .label = "IXP4XX_GPIO_CHIP",
> + .direction_input = ixp4xx_gpio_direction_input,
> + .direction_output = ixp4xx_gpio_direction_output,
> + .get = ixp4xx_gpio_get_value,
> + .set = ixp4xx_gpio_set_value,
> + .base = 0,
> + .ngpio = 16,
> +};
> +
> void __init ixp4xx_sys_init(void)
> {
> ixp4xx_exp_bus_size = SZ_16M;
>
> platform_add_devices(ixp4xx_devices, ARRAY_SIZE(ixp4xx_devices));
>
> + gpiochip_add(&ixp4xx_gpio_chip);
> +
> if (cpu_is_ixp46x()) {
> int region;
>
> diff --git a/arch/arm/mach-ixp4xx/include/mach/gpio.h b/arch/arm/mach-ixp4xx/include/mach/gpio.h
> index a5f87de..86f3596 100644
> --- a/arch/arm/mach-ixp4xx/include/mach/gpio.h
> +++ b/arch/arm/mach-ixp4xx/include/mach/gpio.h
> @@ -27,47 +27,31 @@
>
> #include <linux/kernel.h>
> #include <mach/hardware.h>
> +#include <asm-generic/gpio.h> /* cansleep wrappers */
>
> -static inline int gpio_request(unsigned gpio, const char *label)
> -{
> - return 0;
> -}
> -
> -static inline void gpio_free(unsigned gpio)
> -{
> - might_sleep();
> -
> - return;
> -}
> -
> -static inline int gpio_direction_input(unsigned gpio)
> -{
> - gpio_line_config(gpio, IXP4XX_GPIO_IN);
> - return 0;
> -}
> -
> -static inline int gpio_direction_output(unsigned gpio, int level)
> -{
> - gpio_line_set(gpio, level);
> - gpio_line_config(gpio, IXP4XX_GPIO_OUT);
> - return 0;
> -}
> +#define NR_BUILTIN_GPIO 16
>
> static inline int gpio_get_value(unsigned gpio)
> {
> - int value;
> -
> - gpio_line_get(gpio, &value);
> -
> - return value;
> + if (gpio < NR_BUILTIN_GPIO)
you might want to test the impact of using

if (__builtin_constant_p(gpio) && gpio < NR_BUILTIN_GPIO)

here. I don't know what to expect from it, but this is the idiom I saw
when I implemented gpio stuff some years ago.

> + {
> + int value;
> + gpio_line_get(gpio, &value);
> + return value;
I wonder about the return value of gpio_line_get. If it's void why not
change it to return the value instead of using an output parameter.

> + }
> + else
> + return __gpio_get_value(gpio);
> }
>
> static inline void gpio_set_value(unsigned gpio, int value)
> {
> - gpio_line_set(gpio, value);
> + if (gpio < NR_BUILTIN_GPIO)
> + gpio_line_set(gpio, value);
> + else
> + __gpio_set_value(gpio, value);
> }
>
> -#include <asm-generic/gpio.h> /* cansleep wrappers */
> +#define gpio_cansleep __gpio_cansleep
>
> extern int gpio_to_irq(int gpio);
> extern int irq_to_gpio(unsigned int irq);

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |


--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20110906073004.GA28816@pengutronix.de">http://lists.debian.org/20110906073004.GA28816@pengutronix.de