[PATCH V3] powerpc/mpc512x: Add gpio driver

[PATCH V3] powerpc/mpc512x: Add gpio driver

[Anatolij Gustschin]

From: Matthias Fuchs <matthias.fuchs@esd.eu>

This patch adds a gpio driver for MPC512X PowerPCs.

It has been tested on our CAN-CBX-CPU5201 module that
uses a MPC5121 CPU. This platform comes with a couple of
LEDs and configuration switches that have been used for testing.

Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Please consider this patch for inclusion in 2.6.36. Thanks!

v3: - rebase to apply on current mainline tree
    - v2 switched to shadow registers but these are
      not pre-initialized (zero). As a result, setting
      pin direction or ODR register will clear other bits
      in direction and ODR registers. Fix this bug by adding
      shadow registers initialization code.

v2: - move driver to arch/powerpc/platforms/512x directory
    - Kconfig changes are now in arch/powerpc/platform/512x/Kconfig
    - put struct mpc512x_gpio_regs in driver's .c file
    - rename GPIO_MASK into MPC512x_GPIO_MASK
    - use shadow registers instead of r/m/w-operations
    - don't use arch_initcall but call mpc512x_add_gpiochips()
      from mpc512x platform setup code.

 arch/powerpc/platforms/512x/Kconfig          |    9 ++
 arch/powerpc/platforms/512x/Makefile         |    1 +
 arch/powerpc/platforms/512x/mpc512x.h        |    3 +
 arch/powerpc/platforms/512x/mpc512x_gpio.c   |  198 ++++++++++++++++++++++++++
 arch/powerpc/platforms/512x/mpc512x_shared.c |    3 +
 5 files changed, 214 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/mpc512x_gpio.c

diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
index 4dac9b0..bd763ee 100644
--- a/arch/powerpc/platforms/512x/Kconfig
+++ b/arch/powerpc/platforms/512x/Kconfig
@@ -30,3 +30,12 @@ config MPC5121_GENERIC
 
 	  Compatible boards include:  Protonic LVT base boards (ZANMCU
 	  and VICVT2).
+
+config MPC512x_GPIO
+	bool "MPC512x GPIO support"
+	depends on PPC_MPC512x
+	select GENERIC_GPIO
+	select ARCH_REQUIRE_GPIOLIB
+	help
+	  Say Y here if you're going to use hardware that connects to the
+	  MPC512x GPIOs.
diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
index 90be2f5..12518e3 100644
--- a/arch/powerpc/platforms/512x/Makefile
+++ b/arch/powerpc/platforms/512x/Makefile
@@ -4,3 +4,4 @@
 obj-y				+= clock.o mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC5121_GENERIC)	+= mpc5121_generic.o
+obj-$(CONFIG_MPC512x_GPIO)	+= mpc512x_gpio.o
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..4a1b094 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,7 @@ extern void __init mpc512x_init(void);
 extern int __init mpc5121_clk_init(void);
 void __init mpc512x_declare_of_platform_devices(void);
 extern void mpc512x_restart(char *cmd);
+#ifdef CONFIG_MPC512x_GPIO
+extern int mpc512x_add_gpiochips(void);
+#endif
 #endif				/* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_gpio.c b/arch/powerpc/platforms/512x/mpc512x_gpio.c
new file mode 100644
index 0000000..fc6ad82
--- /dev/null
+++ b/arch/powerpc/platforms/512x/mpc512x_gpio.c
@@ -0,0 +1,198 @@
+/*
+ * MPC512x gpio driver
+ *
+ * Copyright (c) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
+ *
+ * derived from ppc4xx gpio driver
+ *
+ * Copyright (c) 2008 Harris Corporation
+ * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *
+ * Author: Steve Falco <sfalco@harris.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+
+#define MPC512x_GPIO_MASK(gpio) (0x80000000 >> (gpio))
+
+struct mpc512x_gpio_regs {
+	u32 gpdir;
+	u32 gpodr;
+	u32 gpdat;
+	u32 gpier;
+	u32 gpimr;
+	u32 gpicr1;
+	u32 gpicr2;
+};
+
+struct mpc512x_chip {
+	struct of_mm_gpio_chip mm_gc;
+	spinlock_t lock;
+
+	/* shadow registers */
+	u32 dat;
+	u32 odr;
+	u32 dir;
+};
+
+/*
+ * GPIO LIB API implementation for GPIOs
+ *
+ * There are a maximum of 32 gpios in each gpio controller.
+ */
+static inline struct mpc512x_chip *
+to_mpc512x_gpiochip(struct of_mm_gpio_chip *mm_gc)
+{
+	return container_of(mm_gc, struct mpc512x_chip, mm_gc);
+}
+
+static int mpc512x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+	return in_be32(&regs->gpdat) & MPC512x_GPIO_MASK(gpio);
+}
+
+static inline void
+__mpc512x_gpio_set(struct of_mm_gpio_chip *mm_gc, unsigned int gpio, int val)
+{
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+
+	if (val)
+		chip->dat |= MPC512x_GPIO_MASK(gpio);
+	else
+		chip->dat &= ~MPC512x_GPIO_MASK(gpio);
+
+	out_be32(&regs->gpdat, chip->dat);
+}
+
+static void
+mpc512x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc512x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Float the pin */
+	chip->dir &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int
+mpc512x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* First set initial value */
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Drive the pin */
+	chip->dir |= MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+	return 0;
+}
+
+void __init mpc512x_add_gpiochips(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio") {
+		int ret;
+		struct mpc512x_chip *chip;
+		struct of_mm_gpio_chip *mm_gc;
+		struct of_gpio_chip *of_gc;
+		struct gpio_chip *gc;
+
+		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+		if (!chip) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		spin_lock_init(&chip->lock);
+
+		mm_gc = &chip->mm_gc;
+		of_gc = &mm_gc->of_gc;
+		gc = &of_gc->gc;
+
+		gc->ngpio = 32;
+		gc->direction_input = mpc512x_gpio_dir_in;
+		gc->direction_output = mpc512x_gpio_dir_out;
+		gc->get = mpc512x_gpio_get;
+		gc->set = mpc512x_gpio_set;
+
+		ret = of_mm_gpiochip_add(np, mm_gc);
+		if (ret)
+			goto err;
+		continue;
+err:
+		pr_err("%s: registration failed with status %d\n",
+		       np->full_name, ret);
+		kfree(chip);
+		/* try others anyway */
+	}
+}
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index 707e572..15da1bc 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -178,4 +178,7 @@ void __init mpc512x_init(void)
 	mpc5121_clk_init();
 	mpc512x_restart_init();
 	mpc512x_psc_fifo_init();
+#ifdef CONFIG_MPC512x_GPIO
+	mpc512x_add_gpiochips();
+#endif
 }
-- 
1.7.0.4

[PATCH V4] powerpc/mpc512x: Add gpio driver

[Anatolij Gustschin]

From: Matthias Fuchs <matthias.fuchs@esd.eu>

This patch adds a gpio driver for MPC512X PowerPCs.

It has been tested on our CAN-CBX-CPU5201 module that
uses a MPC5121 CPU. This platform comes with a couple of
LEDs and configuration switches that have been used for testing.

Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Please consider this patch for inclusion in 2.6.36. Thanks!

v4: - actually v3 was rebased but without shadow
      registers init code as stated in the v3 changelog.
      Correct it now.

v3: - rebase to apply on current mainline tree
    - v2 switched to shadow registers but these are
      not pre-initialized (zero). As a result, setting
      pin direction or ODR register will clear other bits
      in direction and ODR registers. Fix this bug by adding
      shadow registers initialization code.

v2: - move driver to arch/powerpc/platforms/512x directory
    - Kconfig changes are now in arch/powerpc/platform/512x/Kconfig
    - put struct mpc512x_gpio_regs in driver's .c file
    - rename GPIO_MASK into MPC512x_GPIO_MASK
    - use shadow registers instead of r/m/w-operations
    - don't use arch_initcall but call mpc512x_add_gpiochips()
      from mpc512x platform setup code.

 arch/powerpc/platforms/512x/Kconfig          |    9 +
 arch/powerpc/platforms/512x/Makefile         |    1 +
 arch/powerpc/platforms/512x/mpc512x.h        |    3 +
 arch/powerpc/platforms/512x/mpc512x_gpio.c   |  204 ++++++++++++++++++++++++++
 arch/powerpc/platforms/512x/mpc512x_shared.c |    3 +
 5 files changed, 220 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/mpc512x_gpio.c

diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
index 4dac9b0..bd763ee 100644
--- a/arch/powerpc/platforms/512x/Kconfig
+++ b/arch/powerpc/platforms/512x/Kconfig
@@ -30,3 +30,12 @@ config MPC5121_GENERIC
 
 	  Compatible boards include:  Protonic LVT base boards (ZANMCU
 	  and VICVT2).
+
+config MPC512x_GPIO
+	bool "MPC512x GPIO support"
+	depends on PPC_MPC512x
+	select GENERIC_GPIO
+	select ARCH_REQUIRE_GPIOLIB
+	help
+	  Say Y here if you're going to use hardware that connects to the
+	  MPC512x GPIOs.
diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
index 90be2f5..12518e3 100644
--- a/arch/powerpc/platforms/512x/Makefile
+++ b/arch/powerpc/platforms/512x/Makefile
@@ -4,3 +4,4 @@
 obj-y				+= clock.o mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC5121_GENERIC)	+= mpc5121_generic.o
+obj-$(CONFIG_MPC512x_GPIO)	+= mpc512x_gpio.o
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..4a1b094 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,7 @@ extern void __init mpc512x_init(void);
 extern int __init mpc5121_clk_init(void);
 void __init mpc512x_declare_of_platform_devices(void);
 extern void mpc512x_restart(char *cmd);
+#ifdef CONFIG_MPC512x_GPIO
+extern int mpc512x_add_gpiochips(void);
+#endif
 #endif				/* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_gpio.c b/arch/powerpc/platforms/512x/mpc512x_gpio.c
new file mode 100644
index 0000000..13b2478
--- /dev/null
+++ b/arch/powerpc/platforms/512x/mpc512x_gpio.c
@@ -0,0 +1,204 @@
+/*
+ * MPC512x gpio driver
+ *
+ * Copyright (c) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
+ *
+ * derived from ppc4xx gpio driver
+ *
+ * Copyright (c) 2008 Harris Corporation
+ * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *
+ * Author: Steve Falco <sfalco@harris.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+
+#define MPC512x_GPIO_MASK(gpio) (0x80000000 >> (gpio))
+
+struct mpc512x_gpio_regs {
+	u32 gpdir;
+	u32 gpodr;
+	u32 gpdat;
+	u32 gpier;
+	u32 gpimr;
+	u32 gpicr1;
+	u32 gpicr2;
+};
+
+struct mpc512x_chip {
+	struct of_mm_gpio_chip mm_gc;
+	spinlock_t lock;
+
+	/* shadow registers */
+	u32 dat;
+	u32 odr;
+	u32 dir;
+};
+
+/*
+ * GPIO LIB API implementation for GPIOs
+ *
+ * There are a maximum of 32 gpios in each gpio controller.
+ */
+static inline struct mpc512x_chip *
+to_mpc512x_gpiochip(struct of_mm_gpio_chip *mm_gc)
+{
+	return container_of(mm_gc, struct mpc512x_chip, mm_gc);
+}
+
+static int mpc512x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+	return in_be32(&regs->gpdat) & MPC512x_GPIO_MASK(gpio);
+}
+
+static inline void
+__mpc512x_gpio_set(struct of_mm_gpio_chip *mm_gc, unsigned int gpio, int val)
+{
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+
+	if (val)
+		chip->dat |= MPC512x_GPIO_MASK(gpio);
+	else
+		chip->dat &= ~MPC512x_GPIO_MASK(gpio);
+
+	out_be32(&regs->gpdat, chip->dat);
+}
+
+static void
+mpc512x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc512x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Float the pin */
+	chip->dir &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int
+mpc512x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* First set initial value */
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Drive the pin */
+	chip->dir |= MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+	return 0;
+}
+
+void __init mpc512x_add_gpiochips(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio") {
+		int ret;
+		struct mpc512x_chip *chip;
+		struct of_mm_gpio_chip *mm_gc;
+		struct of_gpio_chip *of_gc;
+		struct gpio_chip *gc;
+		struct mpc512x_gpio_regs __iomem *regs;
+
+		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+		if (!chip) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		spin_lock_init(&chip->lock);
+
+		mm_gc = &chip->mm_gc;
+		of_gc = &mm_gc->of_gc;
+		gc = &of_gc->gc;
+
+		gc->ngpio = 32;
+		gc->direction_input = mpc512x_gpio_dir_in;
+		gc->direction_output = mpc512x_gpio_dir_out;
+		gc->get = mpc512x_gpio_get;
+		gc->set = mpc512x_gpio_set;
+
+		ret = of_mm_gpiochip_add(np, mm_gc);
+		if (ret)
+			goto err;
+
+		regs = mm_gc->regs;
+		chip->dat = in_be32(&regs->gpdat);
+		chip->dir = in_be32(&regs->gpdir);
+		chip->odr = in_be32(&regs->gpodr);
+		continue;
+err:
+		pr_err("%s: registration failed with status %d\n",
+		       np->full_name, ret);
+		kfree(chip);
+		/* try others anyway */
+	}
+}
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index 707e572..15da1bc 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -178,4 +178,7 @@ void __init mpc512x_init(void)
 	mpc5121_clk_init();
 	mpc512x_restart_init();
 	mpc512x_psc_fifo_init();
+#ifdef CONFIG_MPC512x_GPIO
+	mpc512x_add_gpiochips();
+#endif
 }
-- 
1.7.0.4

Re: [PATCH V4] powerpc/mpc512x: Add gpio driver

[Grant Likely]

On Thu, Jun 10, 2010 at 10:21 AM, Anatolij Gustschin <agust@denx.de> wrote:
> From: Matthias Fuchs <matthias.fuchs@esd.eu>
>
> This patch adds a gpio driver for MPC512X PowerPCs.
>
> It has been tested on our CAN-CBX-CPU5201 module that
> uses a MPC5121 CPU. This platform comes with a couple of
> LEDs and configuration switches that have been used for testing.
>
> Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Hi Anatolij,

Can you please rework this one on top of my next-devicetree branch.
Change have been made to the of-gpio apis.

g.


> ---
> Please consider this patch for inclusion in 2.6.36. Thanks!
>
> v4: - actually v3 was rebased but without shadow
> =A0 =A0 =A0registers init code as stated in the v3 changelog.
> =A0 =A0 =A0Correct it now.
>
> v3: - rebase to apply on current mainline tree
> =A0 =A0- v2 switched to shadow registers but these are
> =A0 =A0 =A0not pre-initialized (zero). As a result, setting
> =A0 =A0 =A0pin direction or ODR register will clear other bits
> =A0 =A0 =A0in direction and ODR registers. Fix this bug by adding
> =A0 =A0 =A0shadow registers initialization code.
>
> v2: - move driver to arch/powerpc/platforms/512x directory
> =A0 =A0- Kconfig changes are now in arch/powerpc/platform/512x/Kconfig
> =A0 =A0- put struct mpc512x_gpio_regs in driver's .c file
> =A0 =A0- rename GPIO_MASK into MPC512x_GPIO_MASK
> =A0 =A0- use shadow registers instead of r/m/w-operations
> =A0 =A0- don't use arch_initcall but call mpc512x_add_gpiochips()
> =A0 =A0 =A0from mpc512x platform setup code.
>
> =A0arch/powerpc/platforms/512x/Kconfig =A0 =A0 =A0 =A0 =A0| =A0 =A09 +
> =A0arch/powerpc/platforms/512x/Makefile =A0 =A0 =A0 =A0 | =A0 =A01 +
> =A0arch/powerpc/platforms/512x/mpc512x.h =A0 =A0 =A0 =A0| =A0 =A03 +
> =A0arch/powerpc/platforms/512x/mpc512x_gpio.c =A0 | =A0204 ++++++++++++++=
++++++++++++
> =A0arch/powerpc/platforms/512x/mpc512x_shared.c | =A0 =A03 +
> =A05 files changed, 220 insertions(+), 0 deletions(-)
> =A0create mode 100644 arch/powerpc/platforms/512x/mpc512x_gpio.c
>
> diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms=
/512x/Kconfig
> index 4dac9b0..bd763ee 100644
> --- a/arch/powerpc/platforms/512x/Kconfig
> +++ b/arch/powerpc/platforms/512x/Kconfig
> @@ -30,3 +30,12 @@ config MPC5121_GENERIC
>
> =A0 =A0 =A0 =A0 =A0Compatible boards include: =A0Protonic LVT base boards=
 (ZANMCU
> =A0 =A0 =A0 =A0 =A0and VICVT2).
> +
> +config MPC512x_GPIO
> + =A0 =A0 =A0 bool "MPC512x GPIO support"
> + =A0 =A0 =A0 depends on PPC_MPC512x
> + =A0 =A0 =A0 select GENERIC_GPIO
> + =A0 =A0 =A0 select ARCH_REQUIRE_GPIOLIB
> + =A0 =A0 =A0 help
> + =A0 =A0 =A0 =A0 Say Y here if you're going to use hardware that connect=
s to the
> + =A0 =A0 =A0 =A0 MPC512x GPIOs.
> diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platform=
s/512x/Makefile
> index 90be2f5..12518e3 100644
> --- a/arch/powerpc/platforms/512x/Makefile
> +++ b/arch/powerpc/platforms/512x/Makefile
> @@ -4,3 +4,4 @@
> =A0obj-y =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+=3D clock.o =
mpc512x_shared.o
> =A0obj-$(CONFIG_MPC5121_ADS) =A0 =A0 =A0+=3D mpc5121_ads.o mpc5121_ads_cp=
ld.o
> =A0obj-$(CONFIG_MPC5121_GENERIC) =A0+=3D mpc5121_generic.o
> +obj-$(CONFIG_MPC512x_GPIO) =A0 =A0 +=3D mpc512x_gpio.o
> diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platfor=
ms/512x/mpc512x.h
> index b2daca0..4a1b094 100644
> --- a/arch/powerpc/platforms/512x/mpc512x.h
> +++ b/arch/powerpc/platforms/512x/mpc512x.h
> @@ -16,4 +16,7 @@ extern void __init mpc512x_init(void);
> =A0extern int __init mpc5121_clk_init(void);
> =A0void __init mpc512x_declare_of_platform_devices(void);
> =A0extern void mpc512x_restart(char *cmd);
> +#ifdef CONFIG_MPC512x_GPIO
> +extern int mpc512x_add_gpiochips(void);
> +#endif
> =A0#endif =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* __MPC512X_H_=
_ */
> diff --git a/arch/powerpc/platforms/512x/mpc512x_gpio.c b/arch/powerpc/pl=
atforms/512x/mpc512x_gpio.c
> new file mode 100644
> index 0000000..13b2478
> --- /dev/null
> +++ b/arch/powerpc/platforms/512x/mpc512x_gpio.c
> @@ -0,0 +1,204 @@
> +/*
> + * MPC512x gpio driver
> + *
> + * Copyright (c) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
> + *
> + * derived from ppc4xx gpio driver
> + *
> + * Copyright (c) 2008 Harris Corporation
> + * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
> + * Copyright (c) MontaVista Software, Inc. 2008.
> + *
> + * Author: Steve Falco <sfalco@harris.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. =A0See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA =A002111-130=
7 =A0USA
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/spinlock.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
> +#include <linux/types.h>
> +
> +#define MPC512x_GPIO_MASK(gpio) (0x80000000 >> (gpio))
> +
> +struct mpc512x_gpio_regs {
> + =A0 =A0 =A0 u32 gpdir;
> + =A0 =A0 =A0 u32 gpodr;
> + =A0 =A0 =A0 u32 gpdat;
> + =A0 =A0 =A0 u32 gpier;
> + =A0 =A0 =A0 u32 gpimr;
> + =A0 =A0 =A0 u32 gpicr1;
> + =A0 =A0 =A0 u32 gpicr2;
> +};
> +
> +struct mpc512x_chip {
> + =A0 =A0 =A0 struct of_mm_gpio_chip mm_gc;
> + =A0 =A0 =A0 spinlock_t lock;
> +
> + =A0 =A0 =A0 /* shadow registers */
> + =A0 =A0 =A0 u32 dat;
> + =A0 =A0 =A0 u32 odr;
> + =A0 =A0 =A0 u32 dir;
> +};
> +
> +/*
> + * GPIO LIB API implementation for GPIOs
> + *
> + * There are a maximum of 32 gpios in each gpio controller.
> + */
> +static inline struct mpc512x_chip *
> +to_mpc512x_gpiochip(struct of_mm_gpio_chip *mm_gc)
> +{
> + =A0 =A0 =A0 return container_of(mm_gc, struct mpc512x_chip, mm_gc);
> +}
> +
> +static int mpc512x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc);
> + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs;
> +
> + =A0 =A0 =A0 return in_be32(&regs->gpdat) & MPC512x_GPIO_MASK(gpio);
> +}
> +
> +static inline void
> +__mpc512x_gpio_set(struct of_mm_gpio_chip *mm_gc, unsigned int gpio, int=
 val)
> +{
> + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc);
> + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs;
> +
> +
> + =A0 =A0 =A0 if (val)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->dat |=3D MPC512x_GPIO_MASK(gpio);
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->dat &=3D ~MPC512x_GPIO_MASK(gpio);
> +
> + =A0 =A0 =A0 out_be32(&regs->gpdat, chip->dat);
> +}
> +
> +static void
> +mpc512x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc);
> + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc);
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags);
> +
> + =A0 =A0 =A0 __mpc512x_gpio_set(mm_gc, gpio, val);
> +
> + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags);
> +
> + =A0 =A0 =A0 pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +}
> +
> +static int mpc512x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc);
> + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc);
> + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags);
> +
> + =A0 =A0 =A0 /* Disable open-drain function */
> + =A0 =A0 =A0 chip->odr &=3D ~MPC512x_GPIO_MASK(gpio);
> + =A0 =A0 =A0 out_be32(&regs->gpodr, chip->odr);
> +
> + =A0 =A0 =A0 /* Float the pin */
> + =A0 =A0 =A0 chip->dir &=3D ~MPC512x_GPIO_MASK(gpio);
> + =A0 =A0 =A0 out_be32(&regs->gpdir, chip->dir);
> +
> + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags);
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> +static int
> +mpc512x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc);
> + =A0 =A0 =A0 struct mpc512x_chip *chip =3D to_mpc512x_gpiochip(mm_gc);
> + =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs =3D mm_gc->regs;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&chip->lock, flags);
> +
> + =A0 =A0 =A0 /* First set initial value */
> + =A0 =A0 =A0 __mpc512x_gpio_set(mm_gc, gpio, val);
> +
> + =A0 =A0 =A0 /* Disable open-drain function */
> + =A0 =A0 =A0 chip->odr &=3D ~MPC512x_GPIO_MASK(gpio);
> + =A0 =A0 =A0 out_be32(&regs->gpodr, chip->odr);
> +
> + =A0 =A0 =A0 /* Drive the pin */
> + =A0 =A0 =A0 chip->dir |=3D MPC512x_GPIO_MASK(gpio);
> + =A0 =A0 =A0 out_be32(&regs->gpdir, chip->dir);
> +
> + =A0 =A0 =A0 spin_unlock_irqrestore(&chip->lock, flags);
> +
> + =A0 =A0 =A0 pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> +void __init mpc512x_add_gpiochips(void)
> +{
> + =A0 =A0 =A0 struct device_node *np;
> +
> + =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio") {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int ret;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct mpc512x_chip *chip;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct of_gpio_chip *of_gc;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct gpio_chip *gc;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct mpc512x_gpio_regs __iomem *regs;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip =3D kzalloc(sizeof(*chip), GFP_KERNEL)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!chip) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D -ENOMEM;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_init(&chip->lock);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mm_gc =3D &chip->mm_gc;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 of_gc =3D &mm_gc->of_gc;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc =3D &of_gc->gc;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->ngpio =3D 32;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->direction_input =3D mpc512x_gpio_dir_in=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->direction_output =3D mpc512x_gpio_dir_o=
ut;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->get =3D mpc512x_gpio_get;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 gc->set =3D mpc512x_gpio_set;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D of_mm_gpiochip_add(np, mm_gc);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (ret)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 regs =3D mm_gc->regs;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->dat =3D in_be32(&regs->gpdat);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->dir =3D in_be32(&regs->gpdir);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 chip->odr =3D in_be32(&regs->gpodr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue;
> +err:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("%s: registration failed with status=
 %d\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0np->full_name, ret);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(chip);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* try others anyway */
> + =A0 =A0 =A0 }
> +}
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/=
platforms/512x/mpc512x_shared.c
> index 707e572..15da1bc 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -178,4 +178,7 @@ void __init mpc512x_init(void)
> =A0 =A0 =A0 =A0mpc5121_clk_init();
> =A0 =A0 =A0 =A0mpc512x_restart_init();
> =A0 =A0 =A0 =A0mpc512x_psc_fifo_init();
> +#ifdef CONFIG_MPC512x_GPIO
> + =A0 =A0 =A0 mpc512x_add_gpiochips();
> +#endif
> =A0}
> --
> 1.7.0.4
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V4] powerpc/mpc512x: Add gpio driver

[Anatolij Gustschin]

Hi Grant,

On Thu, 10 Jun 2010 15:48:42 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Thu, Jun 10, 2010 at 10:21 AM, Anatolij Gustschin <agust@denx.de> wrote:
> > From: Matthias Fuchs <matthias.fuchs@esd.eu>
> >
> > This patch adds a gpio driver for MPC512X PowerPCs.
> >
> > It has been tested on our CAN-CBX-CPU5201 module that
> > uses a MPC5121 CPU. This platform comes with a couple of
> > LEDs and configuration switches that have been used for testing.
> >
> > Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> 
> Hi Anatolij,
> 
> Can you please rework this one on top of my next-devicetree branch.
> Change have been made to the of-gpio apis.

Ok, I'll look at it.

Anatolij

[PATCH V5] powerpc/mpc512x: Add gpio driver

[Anatolij Gustschin]

From: Matthias Fuchs <matthias.fuchs@esd.eu>

This patch adds a gpio driver for MPC512X PowerPCs.

It has been tested on our CAN-CBX-CPU5201 module that
uses a MPC5121 CPU. This platform comes with a couple of
LEDs and configuration switches that have been used for testing.

After change to the of-gpio api the reworked driver has been
tested on pdm360ng board with some configuration switches.

Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Please consider this patch for inclusion in 2.6.36. Thanks!

v4 -> v5:
 - reworked after change to the of-gpio api,
   tested on top of the next-devicetree branch.

 arch/powerpc/platforms/512x/Kconfig          |    9 +
 arch/powerpc/platforms/512x/Makefile         |    1 +
 arch/powerpc/platforms/512x/mpc512x.h        |    3 +
 arch/powerpc/platforms/512x/mpc512x_gpio.c   |  202 ++++++++++++++++++++++++++
 arch/powerpc/platforms/512x/mpc512x_shared.c |    3 +
 5 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/mpc512x_gpio.c

diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
index 4dac9b0..bd763ee 100644
--- a/arch/powerpc/platforms/512x/Kconfig
+++ b/arch/powerpc/platforms/512x/Kconfig
@@ -30,3 +30,12 @@ config MPC5121_GENERIC
 
 	  Compatible boards include:  Protonic LVT base boards (ZANMCU
 	  and VICVT2).
+
+config MPC512x_GPIO
+	bool "MPC512x GPIO support"
+	depends on PPC_MPC512x
+	select GENERIC_GPIO
+	select ARCH_REQUIRE_GPIOLIB
+	help
+	  Say Y here if you're going to use hardware that connects to the
+	  MPC512x GPIOs.
diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
index 90be2f5..12518e3 100644
--- a/arch/powerpc/platforms/512x/Makefile
+++ b/arch/powerpc/platforms/512x/Makefile
@@ -4,3 +4,4 @@
 obj-y				+= clock.o mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC5121_GENERIC)	+= mpc5121_generic.o
+obj-$(CONFIG_MPC512x_GPIO)	+= mpc512x_gpio.o
diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
index b2daca0..4a1b094 100644
--- a/arch/powerpc/platforms/512x/mpc512x.h
+++ b/arch/powerpc/platforms/512x/mpc512x.h
@@ -16,4 +16,7 @@ extern void __init mpc512x_init(void);
 extern int __init mpc5121_clk_init(void);
 void __init mpc512x_declare_of_platform_devices(void);
 extern void mpc512x_restart(char *cmd);
+#ifdef CONFIG_MPC512x_GPIO
+extern int mpc512x_add_gpiochips(void);
+#endif
 #endif				/* __MPC512X_H__ */
diff --git a/arch/powerpc/platforms/512x/mpc512x_gpio.c b/arch/powerpc/platforms/512x/mpc512x_gpio.c
new file mode 100644
index 0000000..346942e
--- /dev/null
+++ b/arch/powerpc/platforms/512x/mpc512x_gpio.c
@@ -0,0 +1,202 @@
+/*
+ * MPC512x gpio driver
+ *
+ * Copyright (c) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
+ *
+ * derived from ppc4xx gpio driver
+ *
+ * Copyright (c) 2008 Harris Corporation
+ * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *
+ * Author: Steve Falco <sfalco@harris.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+
+#define MPC512x_GPIO_MASK(gpio) (0x80000000 >> (gpio))
+
+struct mpc512x_gpio_regs {
+	u32 gpdir;
+	u32 gpodr;
+	u32 gpdat;
+	u32 gpier;
+	u32 gpimr;
+	u32 gpicr1;
+	u32 gpicr2;
+};
+
+struct mpc512x_chip {
+	struct of_mm_gpio_chip mm_gc;
+	spinlock_t lock;
+
+	/* shadow registers */
+	u32 dat;
+	u32 odr;
+	u32 dir;
+};
+
+/*
+ * GPIO LIB API implementation for GPIOs
+ *
+ * There are a maximum of 32 gpios in each gpio controller.
+ */
+static inline struct mpc512x_chip *
+to_mpc512x_gpiochip(struct of_mm_gpio_chip *mm_gc)
+{
+	return container_of(mm_gc, struct mpc512x_chip, mm_gc);
+}
+
+static int mpc512x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+	return in_be32(&regs->gpdat) & MPC512x_GPIO_MASK(gpio);
+}
+
+static inline void
+__mpc512x_gpio_set(struct of_mm_gpio_chip *mm_gc, unsigned int gpio, int val)
+{
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+
+
+	if (val)
+		chip->dat |= MPC512x_GPIO_MASK(gpio);
+	else
+		chip->dat &= ~MPC512x_GPIO_MASK(gpio);
+
+	out_be32(&regs->gpdat, chip->dat);
+}
+
+static void
+mpc512x_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+}
+
+static int mpc512x_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Float the pin */
+	chip->dir &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int
+mpc512x_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct mpc512x_chip *chip = to_mpc512x_gpiochip(mm_gc);
+	struct mpc512x_gpio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* First set initial value */
+	__mpc512x_gpio_set(mm_gc, gpio, val);
+
+	/* Disable open-drain function */
+	chip->odr &= ~MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpodr, chip->odr);
+
+	/* Drive the pin */
+	chip->dir |= MPC512x_GPIO_MASK(gpio);
+	out_be32(&regs->gpdir, chip->dir);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	pr_debug("%s: gpio: %d val: %d\n", __func__, gpio, val);
+
+	return 0;
+}
+
+void __init mpc512x_add_gpiochips(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio") {
+		int ret;
+		struct mpc512x_chip *chip;
+		struct of_mm_gpio_chip *mm_gc;
+		struct gpio_chip *gc;
+		struct mpc512x_gpio_regs __iomem *regs;
+
+		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+		if (!chip) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		spin_lock_init(&chip->lock);
+
+		mm_gc = &chip->mm_gc;
+		gc = &mm_gc->gc;
+
+		gc->ngpio = 32;
+		gc->direction_input = mpc512x_gpio_dir_in;
+		gc->direction_output = mpc512x_gpio_dir_out;
+		gc->get = mpc512x_gpio_get;
+		gc->set = mpc512x_gpio_set;
+
+		ret = of_mm_gpiochip_add(np, mm_gc);
+		if (ret)
+			goto err;
+
+		regs = mm_gc->regs;
+		chip->dat = in_be32(&regs->gpdat);
+		chip->dir = in_be32(&regs->gpdir);
+		chip->odr = in_be32(&regs->gpodr);
+		continue;
+err:
+		pr_err("%s: registration failed with status %d\n",
+		       np->full_name, ret);
+		kfree(chip);
+		/* try others anyway */
+	}
+}
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index 707e572..15da1bc 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -178,4 +178,7 @@ void __init mpc512x_init(void)
 	mpc5121_clk_init();
 	mpc512x_restart_init();
 	mpc512x_psc_fifo_init();
+#ifdef CONFIG_MPC512x_GPIO
+	mpc512x_add_gpiochips();
+#endif
 }
-- 
1.7.0.4

Re: [PATCH V5] powerpc/mpc512x: Add gpio driver

[Peter Korsgaard]

>>>>> "Anatolij" == Anatolij Gustschin <agust@denx.de> writes:

Hi,

Old mail, I know ..

 Anatolij> From: Matthias Fuchs <matthias.fuchs@esd.eu>
 Anatolij> This patch adds a gpio driver for MPC512X PowerPCs.

 Anatolij> It has been tested on our CAN-CBX-CPU5201 module that
 Anatolij> uses a MPC5121 CPU. This platform comes with a couple of
 Anatolij> LEDs and configuration switches that have been used for testing.

 Anatolij> After change to the of-gpio api the reworked driver has been
 Anatolij> tested on pdm360ng board with some configuration switches.

This looks very similar to the existing
arch/powerpc/sysdev/mpc8xxx_gpio.c - Couldn't we just add 5121 support
there instead?

 Anatolij> +struct mpc512x_gpio_regs {
 Anatolij> +	u32 gpdir;
 Anatolij> +	u32 gpodr;
 Anatolij> +	u32 gpdat;
 Anatolij> +	u32 gpier;
 Anatolij> +	u32 gpimr;
 Anatolij> +	u32 gpicr1;
 Anatolij> +	u32 gpicr2;
 Anatolij> +};

-- 
Bye, Peter Korsgaard

Re: [PATCH V5] powerpc/mpc512x: Add gpio driver

[Grant Likely]

On Wed, Jul 7, 2010 at 5:28 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Anatolij" =3D=3D Anatolij Gustschin <agust@denx.de> writes:
>
> Hi,
>
> Old mail, I know ..
>
> =A0Anatolij> From: Matthias Fuchs <matthias.fuchs@esd.eu>
> =A0Anatolij> This patch adds a gpio driver for MPC512X PowerPCs.
>
> =A0Anatolij> It has been tested on our CAN-CBX-CPU5201 module that
> =A0Anatolij> uses a MPC5121 CPU. This platform comes with a couple of
> =A0Anatolij> LEDs and configuration switches that have been used for test=
ing.
>
> =A0Anatolij> After change to the of-gpio api the reworked driver has been
> =A0Anatolij> tested on pdm360ng board with some configuration switches.
>
> This looks very similar to the existing
> arch/powerpc/sysdev/mpc8xxx_gpio.c - Couldn't we just add 5121 support
> there instead?
>
> =A0Anatolij> +struct mpc512x_gpio_regs {
> =A0Anatolij> + =A0 =A0u32 gpdir;
> =A0Anatolij> + =A0 =A0u32 gpodr;
> =A0Anatolij> + =A0 =A0u32 gpdat;
> =A0Anatolij> + =A0 =A0u32 gpier;
> =A0Anatolij> + =A0 =A0u32 gpimr;
> =A0Anatolij> + =A0 =A0u32 gpicr1;
> =A0Anatolij> + =A0 =A0u32 gpicr2;
> =A0Anatolij> +};

Hi Anatolij,

Peter's right, the register map looks the same, except for the
additional gpicr1 & 2 registers in the 512x version.  Can the 512x
gpios be supported by the 8xxx gpio driver?

g.

Re: [PATCH V5] powerpc/mpc512x: Add gpio driver

[Anatolij Gustschin]

On Thu, 29 Jul 2010 01:19:23 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Wed, Jul 7, 2010 at 5:28 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>>> "Anatolij" =3D=3D Anatolij Gustschin <agust@denx.de> writes:
> >
> > Hi,
> >
> > Old mail, I know ..
> >
> > =A0Anatolij> From: Matthias Fuchs <matthias.fuchs@esd.eu>
> > =A0Anatolij> This patch adds a gpio driver for MPC512X PowerPCs.
> >
> > =A0Anatolij> It has been tested on our CAN-CBX-CPU5201 module that
> > =A0Anatolij> uses a MPC5121 CPU. This platform comes with a couple of
> > =A0Anatolij> LEDs and configuration switches that have been used for te=
sting.
> >
> > =A0Anatolij> After change to the of-gpio api the reworked driver has be=
en
> > =A0Anatolij> tested on pdm360ng board with some configuration switches.
> >
> > This looks very similar to the existing
> > arch/powerpc/sysdev/mpc8xxx_gpio.c - Couldn't we just add 5121 support
> > there instead?
> >
> > =A0Anatolij> +struct mpc512x_gpio_regs {
> > =A0Anatolij> + =A0 =A0u32 gpdir;
> > =A0Anatolij> + =A0 =A0u32 gpodr;
> > =A0Anatolij> + =A0 =A0u32 gpdat;
> > =A0Anatolij> + =A0 =A0u32 gpier;
> > =A0Anatolij> + =A0 =A0u32 gpimr;
> > =A0Anatolij> + =A0 =A0u32 gpicr1;
> > =A0Anatolij> + =A0 =A0u32 gpicr2;
> > =A0Anatolij> +};
>=20
> Hi Anatolij,
>=20
> Peter's right, the register map looks the same, except for the
> additional gpicr1 & 2 registers in the 512x version.  Can the 512x
> gpios be supported by the 8xxx gpio driver?

Hi Grant,

I wanted to extend/test this driver but didn't have time so far. I'll
look at 8xxx gpio driver this weekend to see if it can be used for
512x gpios.

Anatolij

[PATCH] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anatolij Gustschin]

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 arch/powerpc/platforms/Kconfig     |    7 ++--
 arch/powerpc/sysdev/mpc8xxx_gpio.c |   54 +++++++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index d1663db..471115a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -304,13 +304,14 @@ config OF_RTC
 source "arch/powerpc/sysdev/bestcomm/Kconfig"
 
 config MPC8xxx_GPIO
-	bool "MPC8xxx GPIO support"
-	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
+	bool "MPC512x/MPC8xxx GPIO support"
+	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
+		   FSL_SOC_BOOKE || PPC_86xx
 	select GENERIC_GPIO
 	select ARCH_REQUIRE_GPIOLIB
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
 
 config SIMPLE_GPIO
 	bool "Support for simple, memory-mapped GPIO controllers"
diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
index 2b69084..f5b4959 100644
--- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
+++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
@@ -1,5 +1,5 @@
 /*
- * GPIOs on MPC8349/8572/8610 and compatible
+ * GPIOs on MPC512x/8349/8572/8610 and compatible
  *
  * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
  *
@@ -26,6 +26,7 @@
 #define GPIO_IER		0x0c
 #define GPIO_IMR		0x10
 #define GPIO_ICR		0x14
+#define GPIO_ICR2		0x18
 
 struct mpc8xxx_gpio_chip {
 	struct of_mm_gpio_chip mm_gc;
@@ -215,6 +216,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
 	return 0;
 }
 
+static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
+{
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
+	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
+	unsigned long gpio = virq_to_hw(virq);
+	void __iomem *reg;
+	unsigned int shift;
+	unsigned long flags;
+
+	if (gpio < 16) {
+		reg = mm->regs + GPIO_ICR;
+		shift = (15 - gpio) * 2;
+	} else {
+		reg = mm->regs + GPIO_ICR2;
+		shift = (15 - (gpio % 16)) * 2;
+	}
+
+	switch (flow_type) {
+	case IRQ_TYPE_EDGE_FALLING:
+	case IRQ_TYPE_LEVEL_LOW:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 2 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 1 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrbits32(reg, 3 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct irq_chip mpc8xxx_irq_chip = {
 	.name		= "mpc8xxx-gpio",
 	.unmask		= mpc8xxx_irq_unmask,
@@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip = {
 static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
 				irq_hw_number_t hw)
 {
+	if (of_device_is_compatible(h->of_node, "fsl,mpc5121-gpio"))
+		mpc8xxx_irq_chip.set_type = mpc512x_irq_set_type;
+
 	set_irq_chip_data(virq, h->host_data);
 	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
 	set_irq_type(virq, IRQ_TYPE_NONE);
@@ -330,6 +379,9 @@ static int __init mpc8xxx_add_gpiochips(void)
 	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
 		mpc8xxx_add_controller(np);
 
+	for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio")
+		mpc8xxx_add_controller(np);
+
 	return 0;
 }
 arch_initcall(mpc8xxx_add_gpiochips);
-- 
1.7.0.4

Re: [PATCH] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

Hi Anatolij,

Looks pretty good, but some comments below...

On Sat, Aug 7, 2010 at 7:28 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

You haven't written a patch description.  Give some details about how
the 512x gpio controller is different from the 8xxx one.

> ---
> =A0arch/powerpc/platforms/Kconfig =A0 =A0 | =A0 =A07 ++--
> =A0arch/powerpc/sysdev/mpc8xxx_gpio.c | =A0 54 ++++++++++++++++++++++++++=
+++++++++-
> =A02 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kcon=
fig
> index d1663db..471115a 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -304,13 +304,14 @@ config OF_RTC
> =A0source "arch/powerpc/sysdev/bestcomm/Kconfig"
>
> =A0config MPC8xxx_GPIO
> - =A0 =A0 =A0 bool "MPC8xxx GPIO support"
> - =A0 =A0 =A0 depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL=
_SOC_BOOKE || PPC_86xx
> + =A0 =A0 =A0 bool "MPC512x/MPC8xxx GPIO support"
> + =A0 =A0 =A0 depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC=
_MPC837x || \
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0FSL_SOC_BOOKE || PPC_86xx
> =A0 =A0 =A0 =A0select GENERIC_GPIO
> =A0 =A0 =A0 =A0select ARCH_REQUIRE_GPIOLIB
> =A0 =A0 =A0 =A0help
> =A0 =A0 =A0 =A0 =A0Say Y here if you're going to use hardware that connec=
ts to the
> - =A0 =A0 =A0 =A0 MPC831x/834x/837x/8572/8610 GPIOs.
> + =A0 =A0 =A0 =A0 MPC512x/831x/834x/837x/8572/8610 GPIOs.
>
> =A0config SIMPLE_GPIO
> =A0 =A0 =A0 =A0bool "Support for simple, memory-mapped GPIO controllers"
> diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc=
8xxx_gpio.c
> index 2b69084..f5b4959 100644
> --- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
> +++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
> @@ -1,5 +1,5 @@
> =A0/*
> - * GPIOs on MPC8349/8572/8610 and compatible
> + * GPIOs on MPC512x/8349/8572/8610 and compatible
> =A0*
> =A0* Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
> =A0*
> @@ -26,6 +26,7 @@
> =A0#define GPIO_IER =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x0c
> =A0#define GPIO_IMR =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x10
> =A0#define GPIO_ICR =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x14
> +#define GPIO_ICR2 =A0 =A0 =A0 =A0 =A0 =A0 =A00x18
>
> =A0struct mpc8xxx_gpio_chip {
> =A0 =A0 =A0 =A0struct of_mm_gpio_chip mm_gc;
> @@ -215,6 +216,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, u=
nsigned int flow_type)
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> +static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_typ=
e)
> +{
> + =A0 =A0 =A0 struct mpc8xxx_gpio_chip *mpc8xxx_gc =3D get_irq_chip_data(=
virq);
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm =3D &mpc8xxx_gc->mm_gc;
> + =A0 =A0 =A0 unsigned long gpio =3D virq_to_hw(virq);
> + =A0 =A0 =A0 void __iomem *reg;
> + =A0 =A0 =A0 unsigned int shift;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 if (gpio < 16) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D mm->regs + GPIO_ICR;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 shift =3D (15 - gpio) * 2;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D mm->regs + GPIO_ICR2;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 shift =3D (15 - (gpio % 16)) * 2;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 switch (flow_type) {
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_FALLING:
> + =A0 =A0 =A0 case IRQ_TYPE_LEVEL_LOW:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrsetbits_be32(reg, 3 << shift, 2 << shift=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_RISING:
> + =A0 =A0 =A0 case IRQ_TYPE_LEVEL_HIGH:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrsetbits_be32(reg, 3 << shift, 1 << shift=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_BOTH:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrbits32(reg, 3 << shift);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 default:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> =A0static struct irq_chip mpc8xxx_irq_chip =3D {
> =A0 =A0 =A0 =A0.name =A0 =A0 =A0 =A0 =A0 =3D "mpc8xxx-gpio",
> =A0 =A0 =A0 =A0.unmask =A0 =A0 =A0 =A0 =3D mpc8xxx_irq_unmask,
> @@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip =3D {
> =A0static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0irq_hw_num=
ber_t hw)
> =A0{
> + =A0 =A0 =A0 if (of_device_is_compatible(h->of_node, "fsl,mpc5121-gpio")=
)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_irq_chip.set_type =3D mpc512x_irq_s=
et_type;
> +

You can put the set type hook into the of_match_table data which you
will need for of_find_matching_node() (see below).

> =A0 =A0 =A0 =A0set_irq_chip_data(virq, h->host_data);
> =A0 =A0 =A0 =A0set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_l=
evel_irq);
> =A0 =A0 =A0 =A0set_irq_type(virq, IRQ_TYPE_NONE);
> @@ -330,6 +379,9 @@ static int __init mpc8xxx_add_gpiochips(void)
> =A0 =A0 =A0 =A0for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc8xxx_add_controller(np);
>
> + =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio")
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_add_controller(np);
> +

This list is getting too long.  Refactor this function to use
for_each_matching_node().  Also doing it this way is dangerous because
if say a 5121 gpio node claims compatibility with a 8610 gpio node,
then the gpio controller will get registered twice.

> =A0 =A0 =A0 =A0return 0;
> =A0}
> =A0arch_initcall(mpc8xxx_add_gpiochips);
> --
> 1.7.0.4
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anatolij Gustschin]

Hi Grant,

On Sat, 7 Aug 2010 09:12:50 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> Hi Anatolij,
>=20
> Looks pretty good, but some comments below...
>=20
> On Sat, Aug 7, 2010 at 7:28 AM, Anatolij Gustschin <agust@denx.de> wrote:
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
>=20
> You haven't written a patch description.  Give some details about how
> the 512x gpio controller is different from the 8xxx one.

Ok, will fix.

...
> > @@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip =3D {
> > =A0static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int vir=
q,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0irq_hw_n=
umber_t hw)
> > =A0{
> > + =A0 =A0 =A0 if (of_device_is_compatible(h->of_node, "fsl,mpc5121-gpio=
"))
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_irq_chip.set_type =3D mpc512x_irq=
_set_type;
> > +
>=20
> You can put the set type hook into the of_match_table data which you
> will need for of_find_matching_node() (see below).

How can I get this match table data reference in mpc8xxx_gpio_irq_map() ?
Is it okay to set data field of struct device_node to the set type
hook? I could do it in mpc8xxx_add_gpiochips() but I'm not sure whether
the data field will be used for other purposes somewhere else.

...
> > @@ -330,6 +379,9 @@ static int __init mpc8xxx_add_gpiochips(void)
> > =A0 =A0 =A0 =A0for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc8xxx_add_controller(np);
> >
> > + =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc5121-gpio")
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_add_controller(np);
> > +
>=20
> This list is getting too long.  Refactor this function to use
> for_each_matching_node().  Also doing it this way is dangerous because
> if say a 5121 gpio node claims compatibility with a 8610 gpio node,
> then the gpio controller will get registered twice.

Fixed.

Thanks,
Anatolij

Re: [PATCH] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

On Sat, Aug 7, 2010 at 10:39 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Grant Likely <grant.likely@secretlab.ca> wrote:
>> > @@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip =3D {
>> > =A0static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int vi=
rq,
>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0irq_hw_=
number_t hw)
>> > =A0{
>> > + =A0 =A0 =A0 if (of_device_is_compatible(h->of_node, "fsl,mpc5121-gpi=
o"))
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_irq_chip.set_type =3D mpc512x_ir=
q_set_type;
>> > +
>>
>> You can put the set type hook into the of_match_table data which you
>> will need for of_find_matching_node() (see below).
>
> How can I get this match table data reference in mpc8xxx_gpio_irq_map() ?

of_match_node() will return the matching entry in the table.

> Is it okay to set data field of struct device_node to the set type
> hook? I could do it in mpc8xxx_add_gpiochips() but I'm not sure whether
> the data field will be used for other purposes somewhere else.

You are safe to use the .data field.

g.

[PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anatolij Gustschin]

The GPIO controller of MPC512x is slightly different from
8xxx GPIO controllers. The register interface is the same
except the external interrupt control register. The MPC512x
GPIO controller differentiates between four interrupt event
types and therefore provides two interrupt control registers,
GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.

This patch adds MPC512x speciffic set_type() callback and
updates config file and comments. Additionally the gpio chip
registration function is changed to use for_each_matching_node()
preventing multiple registration if a node claimes compatibility
with another gpio controller type.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - add patch description
 - use match table data to set irq set_type hook as
   recommended
 - refactor to use for_each_matching_node() in
   mpc8xxx_add_gpiochips() as suggested by Grant

 arch/powerpc/platforms/Kconfig     |    7 ++-
 arch/powerpc/sysdev/mpc8xxx_gpio.c |   72 ++++++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index d1663db..471115a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -304,13 +304,14 @@ config OF_RTC
 source "arch/powerpc/sysdev/bestcomm/Kconfig"
 
 config MPC8xxx_GPIO
-	bool "MPC8xxx GPIO support"
-	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
+	bool "MPC512x/MPC8xxx GPIO support"
+	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
+		   FSL_SOC_BOOKE || PPC_86xx
 	select GENERIC_GPIO
 	select ARCH_REQUIRE_GPIOLIB
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
 
 config SIMPLE_GPIO
 	bool "Support for simple, memory-mapped GPIO controllers"
diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
index 2b69084..87ad655 100644
--- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
+++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
@@ -1,5 +1,5 @@
 /*
- * GPIOs on MPC8349/8572/8610 and compatible
+ * GPIOs on MPC512x/8349/8572/8610 and compatible
  *
  * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
  *
@@ -26,6 +26,7 @@
 #define GPIO_IER		0x0c
 #define GPIO_IMR		0x10
 #define GPIO_ICR		0x14
+#define GPIO_ICR2		0x18
 
 struct mpc8xxx_gpio_chip {
 	struct of_mm_gpio_chip mm_gc;
@@ -215,6 +216,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
 	return 0;
 }
 
+static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
+{
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
+	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
+	unsigned long gpio = virq_to_hw(virq);
+	void __iomem *reg;
+	unsigned int shift;
+	unsigned long flags;
+
+	if (gpio < 16) {
+		reg = mm->regs + GPIO_ICR;
+		shift = (15 - gpio) * 2;
+	} else {
+		reg = mm->regs + GPIO_ICR2;
+		shift = (15 - (gpio % 16)) * 2;
+	}
+
+	switch (flow_type) {
+	case IRQ_TYPE_EDGE_FALLING:
+	case IRQ_TYPE_LEVEL_LOW:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 2 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 1 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrbits32(reg, 3 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct irq_chip mpc8xxx_irq_chip = {
 	.name		= "mpc8xxx-gpio",
 	.unmask		= mpc8xxx_irq_unmask,
@@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip = {
 static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
 				irq_hw_number_t hw)
 {
+	if (h->of_node && h->of_node->data)
+		mpc8xxx_irq_chip.set_type = h->of_node->data;
+
 	set_irq_chip_data(virq, h->host_data);
 	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
 	set_irq_type(virq, IRQ_TYPE_NONE);
@@ -317,18 +366,25 @@ err:
 	return;
 }
 
+static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
+	{ .compatible = "fsl,mpc8349-gpio", },
+	{ .compatible = "fsl,mpc8572-gpio", },
+	{ .compatible = "fsl,mpc8610-gpio", },
+	{ .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
+	{}
+};
+
 static int __init mpc8xxx_add_gpiochips(void)
 {
+	const struct of_device_id *id;
 	struct device_node *np;
 
-	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
-		mpc8xxx_add_controller(np);
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
-		mpc8xxx_add_controller(np);
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
+	for_each_matching_node(np, mpc8xxx_gpio_ids) {
+		id = of_match_node(mpc8xxx_gpio_ids, np);
+		if (id)
+			np->data = id->data;
 		mpc8xxx_add_controller(np);
+	}
 
 	return 0;
 }
-- 
1.7.0.4

Re: [PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

"Anatolij Gustschin" <agust@denx.de> wrote:

>The GPIO controller of MPC512x is slightly different from
>8xxx GPIO controllers. The register interface is the same
>except the external interrupt control register. The MPC512x
>GPIO controller differentiates between four interrupt event
>types and therefore provides two interrupt control registers,
>GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
>configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.
>
>This patch adds MPC512x speciffic set_type() callback and
>updates config file and comments. Additionally the gpio chip
>registration function is changed to use for_each_matching_node()
>preventing multiple registration if a node claimes compatibility
>with another gpio controller type.
>
>Signed-off-by: Anatolij Gustschin <agust@denx.de>
>---
>v2:
> - add patch description
> - use match table data to set irq set_type hook as
>   recommended
> - refactor to use for_each_matching_node() in
>   mpc8xxx_add_gpiochips() as suggested by Grant
>
> arch/powerpc/platforms/Kconfig     |    7 ++-
> arch/powerpc/sysdev/mpc8xxx_gpio.c |   72 ++++++++++++++++++++++++++++++++----
> 2 files changed, 68 insertions(+), 11 deletions(-)
>
>diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
>index d1663db..471115a 100644
>--- a/arch/powerpc/platforms/Kconfig
>+++ b/arch/powerpc/platforms/Kconfig
>@@ -304,13 +304,14 @@ config OF_RTC
> source "arch/powerpc/sysdev/bestcomm/Kconfig"
>
> config MPC8xxx_GPIO
>-	bool "MPC8xxx GPIO support"
>-	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
>+	bool "MPC512x/MPC8xxx GPIO support"
>+	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
>+		   FSL_SOC_BOOKE || PPC_86xx
> 	select GENERIC_GPIO
> 	select ARCH_REQUIRE_GPIOLIB
> 	help
> 	  Say Y here if you're going to use hardware that connects to the
>-	  MPC831x/834x/837x/8572/8610 GPIOs.
>+	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
>
> config SIMPLE_GPIO
> 	bool "Support for simple, memory-mapped GPIO controllers"
>diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
>index 2b69084..87ad655 100644
>--- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
>+++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
>@@ -1,5 +1,5 @@
> /*
>- * GPIOs on MPC8349/8572/8610 and compatible
>+ * GPIOs on MPC512x/8349/8572/8610 and compatible
>  *
>  * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
>  *
>@@ -26,6 +26,7 @@
> #define GPIO_IER		0x0c
> #define GPIO_IMR		0x10
> #define GPIO_ICR		0x14
>+#define GPIO_ICR2		0x18
>
> struct mpc8xxx_gpio_chip {
> 	struct of_mm_gpio_chip mm_gc;
>@@ -215,6 +216,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
> 	return 0;
> }
>
>+static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
>+{
>+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
>+	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
>+	unsigned long gpio = virq_to_hw(virq);
>+	void __iomem *reg;
>+	unsigned int shift;
>+	unsigned long flags;
>+
>+	if (gpio < 16) {
>+		reg = mm->regs + GPIO_ICR;
>+		shift = (15 - gpio) * 2;
>+	} else {
>+		reg = mm->regs + GPIO_ICR2;
>+		shift = (15 - (gpio % 16)) * 2;
>+	}
>+
>+	switch (flow_type) {
>+	case IRQ_TYPE_EDGE_FALLING:
>+	case IRQ_TYPE_LEVEL_LOW:
>+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
>+		clrsetbits_be32(reg, 3 << shift, 2 << shift);
>+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
>+		break;
>+
>+	case IRQ_TYPE_EDGE_RISING:
>+	case IRQ_TYPE_LEVEL_HIGH:
>+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
>+		clrsetbits_be32(reg, 3 << shift, 1 << shift);
>+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
>+		break;
>+
>+	case IRQ_TYPE_EDGE_BOTH:
>+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
>+		clrbits32(reg, 3 << shift);
>+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
>+		break;
>+
>+	default:
>+		return -EINVAL;
>+	}
>+
>+	return 0;
>+}
>+
> static struct irq_chip mpc8xxx_irq_chip = {
> 	.name		= "mpc8xxx-gpio",
> 	.unmask		= mpc8xxx_irq_unmask,
>@@ -226,6 +272,9 @@ static struct irq_chip mpc8xxx_irq_chip = {
> static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
> 				irq_hw_number_t hw)
> {
>+	if (h->of_node && h->of_node->data)
>+		mpc8xxx_irq_chip.set_type = h->of_node->data;
>+
> 	set_irq_chip_data(virq, h->host_data);
> 	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
> 	set_irq_type(virq, IRQ_TYPE_NONE);
>@@ -317,18 +366,25 @@ err:
> 	return;
> }
>
>+static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
>+	{ .compatible = "fsl,mpc8349-gpio", },
>+	{ .compatible = "fsl,mpc8572-gpio", },
>+	{ .compatible = "fsl,mpc8610-gpio", },
>+	{ .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
>+	{}
>+};
>+
> static int __init mpc8xxx_add_gpiochips(void)
> {
>+	const struct of_device_id *id;
> 	struct device_node *np;
>
>-	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
>-		mpc8xxx_add_controller(np);
>-
>-	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
>-		mpc8xxx_add_controller(np);
>-
>-	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
>+	for_each_matching_node(np, mpc8xxx_gpio_ids) {
>+		id = of_match_node(mpc8xxx_gpio_ids, np);
>+		if (id)
>+			np->data = id->data;
> 		mpc8xxx_add_controller(np);
>+	}

Sorry, I miss led you.  id->data is fine, but don't use np->data.
Call of_match_node() inside mpc8xxx_add_controller() instead, or
change the function signature to pass it in explicitly...

Actually, there is absolutely no reason to keep mpc8xxx_add_gpiochip()
as a separate function with the simplification of
mpc8xxx_add_gpiochips().  I'd simplify the whole thing by merging the
two functions together.

g.

Re: [PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anton Vorontsov]

On Sat, Aug 07, 2010 at 06:40:22PM -0600, Grant Likely wrote:
[...]
> > static int __init mpc8xxx_add_gpiochips(void)
> > {
> >+	const struct of_device_id *id;
> > 	struct device_node *np;
> >
> >-	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
> >-		mpc8xxx_add_controller(np);
> >-
> >-	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
> >-		mpc8xxx_add_controller(np);
> >-
> >-	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
> >+	for_each_matching_node(np, mpc8xxx_gpio_ids) {
> >+		id = of_match_node(mpc8xxx_gpio_ids, np);
> >+		if (id)
> >+			np->data = id->data;
> > 		mpc8xxx_add_controller(np);
> >+	}
[...]
> Actually, there is absolutely no reason to keep mpc8xxx_add_gpiochip()
> as a separate function with the simplification of
> mpc8xxx_add_gpiochips().  I'd simplify the whole thing by merging the
> two functions together.

You mean mpc8xxx_add_controller()? Putting 65-line function
on a second indentation level, inside the for loop... sounds
like a bad idea.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH v3] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anatolij Gustschin]

The GPIO controller of MPC512x is slightly different from
8xxx GPIO controllers. The register interface is the same
except the external interrupt control register. The MPC512x
GPIO controller differentiates between four interrupt event
types and therefore provides two interrupt control registers,
GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.

This patch adds MPC512x speciffic set_type() callback and
updates config file and comments. Additionally the gpio chip
registration function is changed to use for_each_matching_node()
preventing multiple registration if a node claimes compatibility
with another gpio controller type. Also merge two chip
registration functions into one.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v3:
  - merge mpc8xxx_add_controller() into mpc8xxx_add_gpiochips()
  - do not use of_node's data field for set type hook,
    use added void data pointer in the gpio chip struct
    instead.

v2:
 - add patch description
 - use match table data to set irq set_type hook as
   recommended
 - refactor to use for_each_matching_node() in
   mpc8xxx_add_gpiochips() as suggested by Grant

 arch/powerpc/platforms/Kconfig     |    7 +-
 arch/powerpc/sysdev/mpc8xxx_gpio.c |  196 +++++++++++++++++++++++-------------
 2 files changed, 129 insertions(+), 74 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index d1663db..471115a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -304,13 +304,14 @@ config OF_RTC
 source "arch/powerpc/sysdev/bestcomm/Kconfig"
 
 config MPC8xxx_GPIO
-	bool "MPC8xxx GPIO support"
-	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
+	bool "MPC512x/MPC8xxx GPIO support"
+	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
+		   FSL_SOC_BOOKE || PPC_86xx
 	select GENERIC_GPIO
 	select ARCH_REQUIRE_GPIOLIB
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
 
 config SIMPLE_GPIO
 	bool "Support for simple, memory-mapped GPIO controllers"
diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
index 2b69084..072f490 100644
--- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
+++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
@@ -1,5 +1,5 @@
 /*
- * GPIOs on MPC8349/8572/8610 and compatible
+ * GPIOs on MPC512x/8349/8572/8610 and compatible
  *
  * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
  *
@@ -26,6 +26,7 @@
 #define GPIO_IER		0x0c
 #define GPIO_IMR		0x10
 #define GPIO_ICR		0x14
+#define GPIO_ICR2		0x18
 
 struct mpc8xxx_gpio_chip {
 	struct of_mm_gpio_chip mm_gc;
@@ -37,6 +38,7 @@ struct mpc8xxx_gpio_chip {
 	 */
 	u32 data;
 	struct irq_host *irq;
+	void *of_dev_id_data;
 };
 
 static inline u32 mpc8xxx_gpio2mask(unsigned int gpio)
@@ -215,6 +217,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
 	return 0;
 }
 
+static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
+{
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
+	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
+	unsigned long gpio = virq_to_hw(virq);
+	void __iomem *reg;
+	unsigned int shift;
+	unsigned long flags;
+
+	if (gpio < 16) {
+		reg = mm->regs + GPIO_ICR;
+		shift = (15 - gpio) * 2;
+	} else {
+		reg = mm->regs + GPIO_ICR2;
+		shift = (15 - (gpio % 16)) * 2;
+	}
+
+	switch (flow_type) {
+	case IRQ_TYPE_EDGE_FALLING:
+	case IRQ_TYPE_LEVEL_LOW:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 2 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 1 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrbits32(reg, 3 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct irq_chip mpc8xxx_irq_chip = {
 	.name		= "mpc8xxx-gpio",
 	.unmask		= mpc8xxx_irq_unmask,
@@ -226,6 +273,11 @@ static struct irq_chip mpc8xxx_irq_chip = {
 static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
 				irq_hw_number_t hw)
 {
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = h->host_data;
+
+	if (mpc8xxx_gc->of_dev_id_data)
+		mpc8xxx_irq_chip.set_type = mpc8xxx_gc->of_dev_id_data;
+
 	set_irq_chip_data(virq, h->host_data);
 	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
 	set_irq_type(virq, IRQ_TYPE_NONE);
@@ -253,83 +305,85 @@ static struct irq_host_ops mpc8xxx_gpio_irq_ops = {
 	.xlate	= mpc8xxx_gpio_irq_xlate,
 };
 
-static void __init mpc8xxx_add_controller(struct device_node *np)
+static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
+	{ .compatible = "fsl,mpc8349-gpio", },
+	{ .compatible = "fsl,mpc8572-gpio", },
+	{ .compatible = "fsl,mpc8610-gpio", },
+	{ .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
+	{}
+};
+
+static int __init mpc8xxx_add_gpiochips(void)
 {
 	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
+	struct device_node *np;
+	const struct of_device_id *id;
 	unsigned hwirq;
-	int ret;
-
-	mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
-	if (!mpc8xxx_gc) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	spin_lock_init(&mpc8xxx_gc->lock);
-
-	mm_gc = &mpc8xxx_gc->mm_gc;
-	gc = &mm_gc->gc;
-
-	mm_gc->save_regs = mpc8xxx_gpio_save_regs;
-	gc->ngpio = MPC8XXX_GPIO_PINS;
-	gc->direction_input = mpc8xxx_gpio_dir_in;
-	gc->direction_output = mpc8xxx_gpio_dir_out;
-	if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
-		gc->get = mpc8572_gpio_get;
-	else
-		gc->get = mpc8xxx_gpio_get;
-	gc->set = mpc8xxx_gpio_set;
-	gc->to_irq = mpc8xxx_gpio_to_irq;
-
-	ret = of_mm_gpiochip_add(np, mm_gc);
-	if (ret)
-		goto err;
-
-	hwirq = irq_of_parse_and_map(np, 0);
-	if (hwirq == NO_IRQ)
-		goto skip_irq;
-
-	mpc8xxx_gc->irq =
-		irq_alloc_host(np, IRQ_HOST_MAP_LINEAR, MPC8XXX_GPIO_PINS,
-			       &mpc8xxx_gpio_irq_ops, MPC8XXX_GPIO_PINS);
-	if (!mpc8xxx_gc->irq)
-		goto skip_irq;
-
-	mpc8xxx_gc->irq->host_data = mpc8xxx_gc;
-
-	/* ack and mask all irqs */
-	out_be32(mm_gc->regs + GPIO_IER, 0xffffffff);
-	out_be32(mm_gc->regs + GPIO_IMR, 0);
-
-	set_irq_data(hwirq, mpc8xxx_gc);
-	set_irq_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
-
-skip_irq:
-	return;
-
+	int ret = 0;
+
+	for_each_matching_node(np, mpc8xxx_gpio_ids) {
+		mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
+		if (!mpc8xxx_gc) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		spin_lock_init(&mpc8xxx_gc->lock);
+
+		mm_gc = &mpc8xxx_gc->mm_gc;
+		gc = &mm_gc->gc;
+
+		mm_gc->save_regs = mpc8xxx_gpio_save_regs;
+		gc->ngpio = MPC8XXX_GPIO_PINS;
+		gc->direction_input = mpc8xxx_gpio_dir_in;
+		gc->direction_output = mpc8xxx_gpio_dir_out;
+		if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
+			gc->get = mpc8572_gpio_get;
+		else
+			gc->get = mpc8xxx_gpio_get;
+		gc->set = mpc8xxx_gpio_set;
+		gc->to_irq = mpc8xxx_gpio_to_irq;
+
+		ret = of_mm_gpiochip_add(np, mm_gc);
+		if (ret)
+			goto err;
+
+		hwirq = irq_of_parse_and_map(np, 0);
+		if (hwirq == NO_IRQ)
+			continue;
+
+		mpc8xxx_gc->irq =
+			irq_alloc_host(np, IRQ_HOST_MAP_LINEAR,
+				       MPC8XXX_GPIO_PINS,
+				       &mpc8xxx_gpio_irq_ops,
+				       MPC8XXX_GPIO_PINS);
+		if (!mpc8xxx_gc->irq)
+			continue;
+
+		id = of_match_node(mpc8xxx_gpio_ids, np);
+		if (id)
+			mpc8xxx_gc->of_dev_id_data = id->data;
+
+		mpc8xxx_gc->irq->host_data = mpc8xxx_gc;
+
+		/* ack and mask all irqs */
+		out_be32(mm_gc->regs + GPIO_IER, 0xffffffff);
+		out_be32(mm_gc->regs + GPIO_IMR, 0);
+
+		set_irq_data(hwirq, mpc8xxx_gc);
+		set_irq_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
+
+		continue;
 err:
-	pr_err("%s: registration failed with status %d\n",
-	       np->full_name, ret);
-	kfree(mpc8xxx_gc);
-
-	return;
-}
-
-static int __init mpc8xxx_add_gpiochips(void)
-{
-	struct device_node *np;
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
-		mpc8xxx_add_controller(np);
+		pr_err("%s: registration failed with status %d\n",
+		       np->full_name, ret);
+		kfree(mpc8xxx_gc);
 
-	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
-		mpc8xxx_add_controller(np);
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
-		mpc8xxx_add_controller(np);
+		continue;
+	}
 
-	return 0;
+	return ret;
 }
 arch_initcall(mpc8xxx_add_gpiochips);
-- 
1.7.0.4

Re: [PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

On Sun, Aug 8, 2010 at 1:40 AM, Anton Vorontsov <cbouatmailru@gmail.com> wr=
ote:
> On Sat, Aug 07, 2010 at 06:40:22PM -0600, Grant Likely wrote:
> [...]
>> > static int __init mpc8xxx_add_gpiochips(void)
>> > {
>> >+ =A0 =A0const struct of_device_id *id;
>> > =A0 =A0 struct device_node *np;
>> >
>> >- =A0 =A0for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
>> >- =A0 =A0 =A0 =A0 =A0 =A0mpc8xxx_add_controller(np);
>> >-
>> >- =A0 =A0for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
>> >- =A0 =A0 =A0 =A0 =A0 =A0mpc8xxx_add_controller(np);
>> >-
>> >- =A0 =A0for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
>> >+ =A0 =A0for_each_matching_node(np, mpc8xxx_gpio_ids) {
>> >+ =A0 =A0 =A0 =A0 =A0 =A0id =3D of_match_node(mpc8xxx_gpio_ids, np);
>> >+ =A0 =A0 =A0 =A0 =A0 =A0if (id)
>> >+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0np->data =3D id->data;
>> > =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_add_controller(np);
>> >+ =A0 =A0}
> [...]
>> Actually, there is absolutely no reason to keep mpc8xxx_add_gpiochip()
>> as a separate function with the simplification of
>> mpc8xxx_add_gpiochips(). =A0I'd simplify the whole thing by merging the
>> two functions together.
>
> You mean mpc8xxx_add_controller()? Putting 65-line function
> on a second indentation level, inside the for loop... sounds
> like a bad idea.

That's a good point.  I was thinking about it from the wrong way
around (that the function could bail at the top on a non-match; which
obviously isn't the case).  Anatolij, I was wrong on this point.
Sorry I didn't reply sooner before you respun the patch.  :-(

g.

[PATCH v4] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Anatolij Gustschin]

The GPIO controller of MPC512x is slightly different from
8xxx GPIO controllers. The register interface is the same
except the external interrupt control register. The MPC512x
GPIO controller differentiates between four interrupt event
types and therefore provides two interrupt control registers,
GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.

This patch adds MPC512x speciffic set_type() callback and
updates config file and comments. Additionally the gpio chip
registration function is changed to use for_each_matching_node()
preventing multiple registration if a node claimes compatibility
with another gpio controller type.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v4:
 - undo function merging as it was wrong
 - fix commit message

v3:
  - merge mpc8xxx_add_controller() into mpc8xxx_add_gpiochips()
  - do not use of_node's data field for set type hook,
    use added void data pointer in the gpio chip struct
    instead.

v2:
 - add patch description
 - use match table data to set irq set_type hook as
   recommended
 - refactor to use for_each_matching_node() in
   mpc8xxx_add_gpiochips() as suggested by Grant

 arch/powerpc/platforms/Kconfig     |    7 ++-
 arch/powerpc/sysdev/mpc8xxx_gpio.c |   75 ++++++++++++++++++++++++++++++++----
 2 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index d1663db..471115a 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -304,13 +304,14 @@ config OF_RTC
 source "arch/powerpc/sysdev/bestcomm/Kconfig"
 
 config MPC8xxx_GPIO
-	bool "MPC8xxx GPIO support"
-	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
+	bool "MPC512x/MPC8xxx GPIO support"
+	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
+		   FSL_SOC_BOOKE || PPC_86xx
 	select GENERIC_GPIO
 	select ARCH_REQUIRE_GPIOLIB
 	help
 	  Say Y here if you're going to use hardware that connects to the
-	  MPC831x/834x/837x/8572/8610 GPIOs.
+	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
 
 config SIMPLE_GPIO
 	bool "Support for simple, memory-mapped GPIO controllers"
diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
index 2b69084..3649939 100644
--- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
+++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
@@ -1,5 +1,5 @@
 /*
- * GPIOs on MPC8349/8572/8610 and compatible
+ * GPIOs on MPC512x/8349/8572/8610 and compatible
  *
  * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
  *
@@ -26,6 +26,7 @@
 #define GPIO_IER		0x0c
 #define GPIO_IMR		0x10
 #define GPIO_ICR		0x14
+#define GPIO_ICR2		0x18
 
 struct mpc8xxx_gpio_chip {
 	struct of_mm_gpio_chip mm_gc;
@@ -37,6 +38,7 @@ struct mpc8xxx_gpio_chip {
 	 */
 	u32 data;
 	struct irq_host *irq;
+	void *of_dev_id_data;
 };
 
 static inline u32 mpc8xxx_gpio2mask(unsigned int gpio)
@@ -215,6 +217,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
 	return 0;
 }
 
+static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
+{
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
+	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
+	unsigned long gpio = virq_to_hw(virq);
+	void __iomem *reg;
+	unsigned int shift;
+	unsigned long flags;
+
+	if (gpio < 16) {
+		reg = mm->regs + GPIO_ICR;
+		shift = (15 - gpio) * 2;
+	} else {
+		reg = mm->regs + GPIO_ICR2;
+		shift = (15 - (gpio % 16)) * 2;
+	}
+
+	switch (flow_type) {
+	case IRQ_TYPE_EDGE_FALLING:
+	case IRQ_TYPE_LEVEL_LOW:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 2 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrsetbits_be32(reg, 3 << shift, 1 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
+		clrbits32(reg, 3 << shift);
+		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct irq_chip mpc8xxx_irq_chip = {
 	.name		= "mpc8xxx-gpio",
 	.unmask		= mpc8xxx_irq_unmask,
@@ -226,6 +273,11 @@ static struct irq_chip mpc8xxx_irq_chip = {
 static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
 				irq_hw_number_t hw)
 {
+	struct mpc8xxx_gpio_chip *mpc8xxx_gc = h->host_data;
+
+	if (mpc8xxx_gc->of_dev_id_data)
+		mpc8xxx_irq_chip.set_type = mpc8xxx_gc->of_dev_id_data;
+
 	set_irq_chip_data(virq, h->host_data);
 	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
 	set_irq_type(virq, IRQ_TYPE_NONE);
@@ -253,11 +305,20 @@ static struct irq_host_ops mpc8xxx_gpio_irq_ops = {
 	.xlate	= mpc8xxx_gpio_irq_xlate,
 };
 
+static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
+	{ .compatible = "fsl,mpc8349-gpio", },
+	{ .compatible = "fsl,mpc8572-gpio", },
+	{ .compatible = "fsl,mpc8610-gpio", },
+	{ .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
+	{}
+};
+
 static void __init mpc8xxx_add_controller(struct device_node *np)
 {
 	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
+	const struct of_device_id *id;
 	unsigned hwirq;
 	int ret;
 
@@ -297,6 +358,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
 	if (!mpc8xxx_gc->irq)
 		goto skip_irq;
 
+	id = of_match_node(mpc8xxx_gpio_ids, np);
+	if (id)
+		mpc8xxx_gc->of_dev_id_data = id->data;
+
 	mpc8xxx_gc->irq->host_data = mpc8xxx_gc;
 
 	/* ack and mask all irqs */
@@ -321,13 +386,7 @@ static int __init mpc8xxx_add_gpiochips(void)
 {
 	struct device_node *np;
 
-	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
-		mpc8xxx_add_controller(np);
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
-		mpc8xxx_add_controller(np);
-
-	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
+	for_each_matching_node(np, mpc8xxx_gpio_ids)
 		mpc8xxx_add_controller(np);
 
 	return 0;
-- 
1.7.0.4

Re: [PATCH v4] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

On Sun, Aug 8, 2010 at 11:58 PM, Anatolij Gustschin <agust@denx.de> wrote:
> The GPIO controller of MPC512x is slightly different from
> 8xxx GPIO controllers. The register interface is the same
> except the external interrupt control register. The MPC512x
> GPIO controller differentiates between four interrupt event
> types and therefore provides two interrupt control registers,
> GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
> configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.
>
> This patch adds MPC512x speciffic set_type() callback and
> updates config file and comments. Additionally the gpio chip
> registration function is changed to use for_each_matching_node()
> preventing multiple registration if a node claimes compatibility
> with another gpio controller type.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> v4:
> =A0- undo function merging as it was wrong
> =A0- fix commit message

Looks good, thanks.  I'll pick it up after the merge window closes in
prep for 2.6.27

g.

>
> v3:
> =A0- merge mpc8xxx_add_controller() into mpc8xxx_add_gpiochips()
> =A0- do not use of_node's data field for set type hook,
> =A0 =A0use added void data pointer in the gpio chip struct
> =A0 =A0instead.
>
> v2:
> =A0- add patch description
> =A0- use match table data to set irq set_type hook as
> =A0 recommended
> =A0- refactor to use for_each_matching_node() in
> =A0 mpc8xxx_add_gpiochips() as suggested by Grant
>
> =A0arch/powerpc/platforms/Kconfig =A0 =A0 | =A0 =A07 ++-
> =A0arch/powerpc/sysdev/mpc8xxx_gpio.c | =A0 75 ++++++++++++++++++++++++++=
++++++----
> =A02 files changed, 71 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kcon=
fig
> index d1663db..471115a 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -304,13 +304,14 @@ config OF_RTC
> =A0source "arch/powerpc/sysdev/bestcomm/Kconfig"
>
> =A0config MPC8xxx_GPIO
> - =A0 =A0 =A0 bool "MPC8xxx GPIO support"
> - =A0 =A0 =A0 depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL=
_SOC_BOOKE || PPC_86xx
> + =A0 =A0 =A0 bool "MPC512x/MPC8xxx GPIO support"
> + =A0 =A0 =A0 depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC=
_MPC837x || \
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0FSL_SOC_BOOKE || PPC_86xx
> =A0 =A0 =A0 =A0select GENERIC_GPIO
> =A0 =A0 =A0 =A0select ARCH_REQUIRE_GPIOLIB
> =A0 =A0 =A0 =A0help
> =A0 =A0 =A0 =A0 =A0Say Y here if you're going to use hardware that connec=
ts to the
> - =A0 =A0 =A0 =A0 MPC831x/834x/837x/8572/8610 GPIOs.
> + =A0 =A0 =A0 =A0 MPC512x/831x/834x/837x/8572/8610 GPIOs.
>
> =A0config SIMPLE_GPIO
> =A0 =A0 =A0 =A0bool "Support for simple, memory-mapped GPIO controllers"
> diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc=
8xxx_gpio.c
> index 2b69084..3649939 100644
> --- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
> +++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
> @@ -1,5 +1,5 @@
> =A0/*
> - * GPIOs on MPC8349/8572/8610 and compatible
> + * GPIOs on MPC512x/8349/8572/8610 and compatible
> =A0*
> =A0* Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
> =A0*
> @@ -26,6 +26,7 @@
> =A0#define GPIO_IER =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x0c
> =A0#define GPIO_IMR =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x10
> =A0#define GPIO_ICR =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x14
> +#define GPIO_ICR2 =A0 =A0 =A0 =A0 =A0 =A0 =A00x18
>
> =A0struct mpc8xxx_gpio_chip {
> =A0 =A0 =A0 =A0struct of_mm_gpio_chip mm_gc;
> @@ -37,6 +38,7 @@ struct mpc8xxx_gpio_chip {
> =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0u32 data;
> =A0 =A0 =A0 =A0struct irq_host *irq;
> + =A0 =A0 =A0 void *of_dev_id_data;
> =A0};
>
> =A0static inline u32 mpc8xxx_gpio2mask(unsigned int gpio)
> @@ -215,6 +217,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, u=
nsigned int flow_type)
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> +static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_typ=
e)
> +{
> + =A0 =A0 =A0 struct mpc8xxx_gpio_chip *mpc8xxx_gc =3D get_irq_chip_data(=
virq);
> + =A0 =A0 =A0 struct of_mm_gpio_chip *mm =3D &mpc8xxx_gc->mm_gc;
> + =A0 =A0 =A0 unsigned long gpio =3D virq_to_hw(virq);
> + =A0 =A0 =A0 void __iomem *reg;
> + =A0 =A0 =A0 unsigned int shift;
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 if (gpio < 16) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D mm->regs + GPIO_ICR;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 shift =3D (15 - gpio) * 2;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D mm->regs + GPIO_ICR2;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 shift =3D (15 - (gpio % 16)) * 2;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 switch (flow_type) {
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_FALLING:
> + =A0 =A0 =A0 case IRQ_TYPE_LEVEL_LOW:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrsetbits_be32(reg, 3 << shift, 2 << shift=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_RISING:
> + =A0 =A0 =A0 case IRQ_TYPE_LEVEL_HIGH:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrsetbits_be32(reg, 3 << shift, 1 << shift=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case IRQ_TYPE_EDGE_BOTH:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_irqsave(&mpc8xxx_gc->lock, flags)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clrbits32(reg, 3 << shift);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&mpc8xxx_gc->lock, f=
lags);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 default:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> =A0static struct irq_chip mpc8xxx_irq_chip =3D {
> =A0 =A0 =A0 =A0.name =A0 =A0 =A0 =A0 =A0 =3D "mpc8xxx-gpio",
> =A0 =A0 =A0 =A0.unmask =A0 =A0 =A0 =A0 =3D mpc8xxx_irq_unmask,
> @@ -226,6 +273,11 @@ static struct irq_chip mpc8xxx_irq_chip =3D {
> =A0static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0irq_hw_num=
ber_t hw)
> =A0{
> + =A0 =A0 =A0 struct mpc8xxx_gpio_chip *mpc8xxx_gc =3D h->host_data;
> +
> + =A0 =A0 =A0 if (mpc8xxx_gc->of_dev_id_data)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_irq_chip.set_type =3D mpc8xxx_gc->o=
f_dev_id_data;
> +
> =A0 =A0 =A0 =A0set_irq_chip_data(virq, h->host_data);
> =A0 =A0 =A0 =A0set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_l=
evel_irq);
> =A0 =A0 =A0 =A0set_irq_type(virq, IRQ_TYPE_NONE);
> @@ -253,11 +305,20 @@ static struct irq_host_ops mpc8xxx_gpio_irq_ops =3D=
 {
> =A0 =A0 =A0 =A0.xlate =A0=3D mpc8xxx_gpio_irq_xlate,
> =A0};
>
> +static struct of_device_id mpc8xxx_gpio_ids[] __initdata =3D {
> + =A0 =A0 =A0 { .compatible =3D "fsl,mpc8349-gpio", },
> + =A0 =A0 =A0 { .compatible =3D "fsl,mpc8572-gpio", },
> + =A0 =A0 =A0 { .compatible =3D "fsl,mpc8610-gpio", },
> + =A0 =A0 =A0 { .compatible =3D "fsl,mpc5121-gpio", .data =3D mpc512x_irq=
_set_type, },
> + =A0 =A0 =A0 {}
> +};
> +
> =A0static void __init mpc8xxx_add_controller(struct device_node *np)
> =A0{
> =A0 =A0 =A0 =A0struct mpc8xxx_gpio_chip *mpc8xxx_gc;
> =A0 =A0 =A0 =A0struct of_mm_gpio_chip *mm_gc;
> =A0 =A0 =A0 =A0struct gpio_chip *gc;
> + =A0 =A0 =A0 const struct of_device_id *id;
> =A0 =A0 =A0 =A0unsigned hwirq;
> =A0 =A0 =A0 =A0int ret;
>
> @@ -297,6 +358,10 @@ static void __init mpc8xxx_add_controller(struct dev=
ice_node *np)
> =A0 =A0 =A0 =A0if (!mpc8xxx_gc->irq)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto skip_irq;
>
> + =A0 =A0 =A0 id =3D of_match_node(mpc8xxx_gpio_ids, np);
> + =A0 =A0 =A0 if (id)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_gc->of_dev_id_data =3D id->data;
> +
> =A0 =A0 =A0 =A0mpc8xxx_gc->irq->host_data =3D mpc8xxx_gc;
>
> =A0 =A0 =A0 =A0/* ack and mask all irqs */
> @@ -321,13 +386,7 @@ static int __init mpc8xxx_add_gpiochips(void)
> =A0{
> =A0 =A0 =A0 =A0struct device_node *np;
>
> - =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_add_controller(np);
> -
> - =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc8xxx_add_controller(np);
> -
> - =A0 =A0 =A0 for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
> + =A0 =A0 =A0 for_each_matching_node(np, mpc8xxx_gpio_ids)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc8xxx_add_controller(np);
>
> =A0 =A0 =A0 =A0return 0;
> --
> 1.7.0.4
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH v4] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Peter Korsgaard]

>>>>> "Anatolij" == Anatolij Gustschin <agust@denx.de> writes:

 Anatolij> The GPIO controller of MPC512x is slightly different from
 Anatolij> 8xxx GPIO controllers. The register interface is the same
 Anatolij> except the external interrupt control register. The MPC512x
 Anatolij> GPIO controller differentiates between four interrupt event
 Anatolij> types and therefore provides two interrupt control registers,
 Anatolij> GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
 Anatolij> configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.

 Anatolij> This patch adds MPC512x speciffic set_type() callback and
 Anatolij> updates config file and comments. Additionally the gpio chip
 Anatolij> registration function is changed to use for_each_matching_node()
 Anatolij> preventing multiple registration if a node claimes compatibility
 Anatolij> with another gpio controller type.

 Anatolij> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Sorry, was away on holiday.

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH v4] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

On Mon, Aug 09, 2010 at 07:58:48AM +0200, Anatolij Gustschin wrote:
> The GPIO controller of MPC512x is slightly different from
> 8xxx GPIO controllers. The register interface is the same
> except the external interrupt control register. The MPC512x
> GPIO controller differentiates between four interrupt event
> types and therefore provides two interrupt control registers,
> GPICR1 and GPICR2. GPIO[0:15] interrupt event types are
> configured in GPICR1 register, GPIO[16:31] - in GPICR2 register.
> 
> This patch adds MPC512x speciffic set_type() callback and
> updates config file and comments. Additionally the gpio chip
> registration function is changed to use for_each_matching_node()
> preventing multiple registration if a node claimes compatibility
> with another gpio controller type.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> v4:
>  - undo function merging as it was wrong
>  - fix commit message
> 
> v3:
>   - merge mpc8xxx_add_controller() into mpc8xxx_add_gpiochips()
>   - do not use of_node's data field for set type hook,
>     use added void data pointer in the gpio chip struct
>     instead.
> 
> v2:
>  - add patch description
>  - use match table data to set irq set_type hook as
>    recommended
>  - refactor to use for_each_matching_node() in
>    mpc8xxx_add_gpiochips() as suggested by Grant
> 
>  arch/powerpc/platforms/Kconfig     |    7 ++-
>  arch/powerpc/sysdev/mpc8xxx_gpio.c |   75 ++++++++++++++++++++++++++++++++----
>  2 files changed, 71 insertions(+), 11 deletions(-)

Applied to -next, thanks.

g.

> 
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index d1663db..471115a 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -304,13 +304,14 @@ config OF_RTC
>  source "arch/powerpc/sysdev/bestcomm/Kconfig"
>  
>  config MPC8xxx_GPIO
> -	bool "MPC8xxx GPIO support"
> -	depends on PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
> +	bool "MPC512x/MPC8xxx GPIO support"
> +	depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
> +		   FSL_SOC_BOOKE || PPC_86xx
>  	select GENERIC_GPIO
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
>  	  Say Y here if you're going to use hardware that connects to the
> -	  MPC831x/834x/837x/8572/8610 GPIOs.
> +	  MPC512x/831x/834x/837x/8572/8610 GPIOs.
>  
>  config SIMPLE_GPIO
>  	bool "Support for simple, memory-mapped GPIO controllers"
> diff --git a/arch/powerpc/sysdev/mpc8xxx_gpio.c b/arch/powerpc/sysdev/mpc8xxx_gpio.c
> index 2b69084..3649939 100644
> --- a/arch/powerpc/sysdev/mpc8xxx_gpio.c
> +++ b/arch/powerpc/sysdev/mpc8xxx_gpio.c
> @@ -1,5 +1,5 @@
>  /*
> - * GPIOs on MPC8349/8572/8610 and compatible
> + * GPIOs on MPC512x/8349/8572/8610 and compatible
>   *
>   * Copyright (C) 2008 Peter Korsgaard <jacmet@sunsite.dk>
>   *
> @@ -26,6 +26,7 @@
>  #define GPIO_IER		0x0c
>  #define GPIO_IMR		0x10
>  #define GPIO_ICR		0x14
> +#define GPIO_ICR2		0x18
>  
>  struct mpc8xxx_gpio_chip {
>  	struct of_mm_gpio_chip mm_gc;
> @@ -37,6 +38,7 @@ struct mpc8xxx_gpio_chip {
>  	 */
>  	u32 data;
>  	struct irq_host *irq;
> +	void *of_dev_id_data;
>  };
>  
>  static inline u32 mpc8xxx_gpio2mask(unsigned int gpio)
> @@ -215,6 +217,51 @@ static int mpc8xxx_irq_set_type(unsigned int virq, unsigned int flow_type)
>  	return 0;
>  }
>  
> +static int mpc512x_irq_set_type(unsigned int virq, unsigned int flow_type)
> +{
> +	struct mpc8xxx_gpio_chip *mpc8xxx_gc = get_irq_chip_data(virq);
> +	struct of_mm_gpio_chip *mm = &mpc8xxx_gc->mm_gc;
> +	unsigned long gpio = virq_to_hw(virq);
> +	void __iomem *reg;
> +	unsigned int shift;
> +	unsigned long flags;
> +
> +	if (gpio < 16) {
> +		reg = mm->regs + GPIO_ICR;
> +		shift = (15 - gpio) * 2;
> +	} else {
> +		reg = mm->regs + GPIO_ICR2;
> +		shift = (15 - (gpio % 16)) * 2;
> +	}
> +
> +	switch (flow_type) {
> +	case IRQ_TYPE_EDGE_FALLING:
> +	case IRQ_TYPE_LEVEL_LOW:
> +		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
> +		clrsetbits_be32(reg, 3 << shift, 2 << shift);
> +		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
> +		break;
> +
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
> +		clrsetbits_be32(reg, 3 << shift, 1 << shift);
> +		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
> +		break;
> +
> +	case IRQ_TYPE_EDGE_BOTH:
> +		spin_lock_irqsave(&mpc8xxx_gc->lock, flags);
> +		clrbits32(reg, 3 << shift);
> +		spin_unlock_irqrestore(&mpc8xxx_gc->lock, flags);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct irq_chip mpc8xxx_irq_chip = {
>  	.name		= "mpc8xxx-gpio",
>  	.unmask		= mpc8xxx_irq_unmask,
> @@ -226,6 +273,11 @@ static struct irq_chip mpc8xxx_irq_chip = {
>  static int mpc8xxx_gpio_irq_map(struct irq_host *h, unsigned int virq,
>  				irq_hw_number_t hw)
>  {
> +	struct mpc8xxx_gpio_chip *mpc8xxx_gc = h->host_data;
> +
> +	if (mpc8xxx_gc->of_dev_id_data)
> +		mpc8xxx_irq_chip.set_type = mpc8xxx_gc->of_dev_id_data;
> +
>  	set_irq_chip_data(virq, h->host_data);
>  	set_irq_chip_and_handler(virq, &mpc8xxx_irq_chip, handle_level_irq);
>  	set_irq_type(virq, IRQ_TYPE_NONE);
> @@ -253,11 +305,20 @@ static struct irq_host_ops mpc8xxx_gpio_irq_ops = {
>  	.xlate	= mpc8xxx_gpio_irq_xlate,
>  };
>  
> +static struct of_device_id mpc8xxx_gpio_ids[] __initdata = {
> +	{ .compatible = "fsl,mpc8349-gpio", },
> +	{ .compatible = "fsl,mpc8572-gpio", },
> +	{ .compatible = "fsl,mpc8610-gpio", },
> +	{ .compatible = "fsl,mpc5121-gpio", .data = mpc512x_irq_set_type, },
> +	{}
> +};
> +
>  static void __init mpc8xxx_add_controller(struct device_node *np)
>  {
>  	struct mpc8xxx_gpio_chip *mpc8xxx_gc;
>  	struct of_mm_gpio_chip *mm_gc;
>  	struct gpio_chip *gc;
> +	const struct of_device_id *id;
>  	unsigned hwirq;
>  	int ret;
>  
> @@ -297,6 +358,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
>  	if (!mpc8xxx_gc->irq)
>  		goto skip_irq;
>  
> +	id = of_match_node(mpc8xxx_gpio_ids, np);
> +	if (id)
> +		mpc8xxx_gc->of_dev_id_data = id->data;
> +
>  	mpc8xxx_gc->irq->host_data = mpc8xxx_gc;
>  
>  	/* ack and mask all irqs */
> @@ -321,13 +386,7 @@ static int __init mpc8xxx_add_gpiochips(void)
>  {
>  	struct device_node *np;
>  
> -	for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
> -		mpc8xxx_add_controller(np);
> -
> -	for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
> -		mpc8xxx_add_controller(np);
> -
> -	for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
> +	for_each_matching_node(np, mpc8xxx_gpio_ids)
>  		mpc8xxx_add_controller(np);
>  
>  	return 0;
> -- 
> 1.7.0.4
>