[PATCH 0/4] OF GPIO integration for I2C/SPI GPIO chips

[PATCH 0/4] OF GPIO integration for I2C/SPI GPIO chips

[Anton Vorontsov]

Hi all,

Currently it's a burden to make I2C or SPI GPIO chips work with the
OF GPIO infrastructure. I've posted several approaches to solve the
issue before, and others have tried too. Here is another try.

This patch set is used to make things much easier, and completely
seamless for GPIO chips that don't need any platform data (e.g.
mcu_mpc8349emitx) or that have OF bindings already (e.g. pca953x).

Thanks,

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

[PATCH 1/4] gpiolib: Introduce chip addition/removal notifier

[Anton Vorontsov]

Some platforms (e.g. OpenFirmware) want to know when a particular chip
added or removed, so that the platforms could add their specifics for
non-platform devices, like I2C or SPI GPIO chips.

This patch implements the notifier for chip addition and removal events.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/gpio/Kconfig       |    7 +++++++
 drivers/gpio/gpiolib.c     |   28 ++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    8 ++++++++
 3 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1f1d88a..511b29f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -63,6 +63,13 @@ config GPIO_SYSFS
 	  Kernel drivers may also request that a particular GPIO be
 	  exported to userspace; this can be useful when debugging.
 
+config GPIOLIB_NOTIFIER
+	bool
+	help
+	  This symbol is selected by subsystems that need to handle GPIO
+	  chips addition and removal. E.g., this is used for the
+	  OpenFirmware bindings.
+
 # put expanders in the right section, in alphabetical order
 
 comment "Memory mapped GPIO expanders:"
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 350842a..9496b78 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -9,6 +9,7 @@
 #include <linux/seq_file.h>
 #include <linux/gpio.h>
 #include <linux/idr.h>
+#include <linux/notifier.h>
 
 
 /* Optional implementation infrastructure for GPIO interfaces.
@@ -82,6 +83,25 @@ static inline void desc_set_label(struct gpio_desc *d, const char *label)
 #endif
 }
 
+#ifdef CONFIG_GPIOLIB_NOTIFIER
+BLOCKING_NOTIFIER_HEAD(gpio_notifier);
+EXPORT_SYMBOL_GPL(gpio_notifier);
+
+static int gpio_call_chain(struct gpio_chip *chip, enum gpio_notify_msg msg)
+{
+	int ret;
+
+	ret = blocking_notifier_call_chain(&gpio_notifier, msg, chip);
+
+	return notifier_to_errno(ret);
+}
+#else
+static int gpio_call_chain(struct gpio_chip *chip, enum gpio_notify_msg msg)
+{
+	return notifier_to_errno(NOTIFY_OK);
+}
+#endif /* CONFIG_GPIOLIB_NOTIFIER */
+
 /* Warn when drivers omit gpio_request() calls -- legal but ill-advised
  * when setting direction, and otherwise illegal.  Until board setup code
  * and drivers use explicit requests everywhere (which won't happen when
@@ -1103,6 +1123,9 @@ fail:
 		pr_err("gpiochip_add: gpios %d..%d (%s) not registered\n",
 			chip->base, chip->base + chip->ngpio - 1,
 			chip->label ? : "generic");
+	else
+		gpio_call_chain(chip, GPIO_NOTIFY_CHIP_ADDED);
+
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add);
@@ -1119,6 +1142,11 @@ int gpiochip_remove(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 
+	/* Ask external subsystems to release the chip. */
+	status = gpio_call_chain(chip, GPIO_NOTIFY_CHIP_REMOVE);
+	if (status)
+		return status;
+
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	for (id = chip->base; id < chip->base + chip->ngpio; id++) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 485eeb6..84faae4 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -4,6 +4,7 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/notifier.h>
 
 #ifdef CONFIG_GPIOLIB
 
@@ -208,4 +209,11 @@ static inline void gpio_unexport(unsigned gpio)
 }
 #endif	/* CONFIG_GPIO_SYSFS */
 
+enum gpio_notify_msg {
+	GPIO_NOTIFY_CHIP_ADDED = 0,
+	GPIO_NOTIFY_CHIP_REMOVE = 1,
+};
+
+extern struct blocking_notifier_head gpio_notifier;
+
 #endif /* _ASM_GENERIC_GPIO_H */
-- 
1.6.5.7

Re: [PATCH 1/4] gpiolib: Introduce chip addition/removal notifier

[David Brownell]

On Monday 25 January 2010, Anton Vorontsov wrote:
> 
> +config GPIOLIB_NOTIFIER
> +       bool
> +       help
> +         This symbol is selected by subsystems that need to handle GPIO
> +         chips addition and removal. E.g., this is used for the
> +         OpenFirmware bindings.
> +

I'm no huge fan of notifiers, but I suppose they have their place.

However ... I don't see a lot of win to making this optional.  Just
inline the little two blocking_notifier_call_chain() calls directly,
making this a *LOT* simpler.

- Dave

Re: [PATCH 1/4] gpiolib: Introduce chip addition/removal notifier

[Anton Vorontsov]

On Mon, Jan 25, 2010 at 10:34:29PM -0800, David Brownell wrote:
> On Monday 25 January 2010, Anton Vorontsov wrote:
> > 
> > +config GPIOLIB_NOTIFIER
> > +       bool
> > +       help
> > +         This symbol is selected by subsystems that need to handle GPIO
> > +         chips addition and removal. E.g., this is used for the
> > +         OpenFirmware bindings.
> > +
> 
> I'm no huge fan of notifiers, but I suppose they have their place.
> 
> However ... I don't see a lot of win to making this optional.

OK, will remove it.

> Just
> inline the little two blocking_notifier_call_chain() calls directly,
> making this a *LOT* simpler.

I'd rather stay with gpio_call_chain() helper, it makes the code
a little bit prettier, IMO. Compare this:

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1103,6 +1107,9 @@ fail:
 		pr_err("gpiochip_add: gpios %d..%d (%s) not registered\n",
 			chip->base, chip->base + chip->ngpio - 1,
 			chip->label ? : "generic");
+	else
+		blocking_notifier_call_chain(&gpio_notifier,
+					     GPIO_NOTIFY_CHIP_ADDED, chip);
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add);
@@ -1119,6 +1126,13 @@ int gpiochip_remove(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 
+	/* Ask external subsystems to release the chip. */
+	status = blocking_notifier_call_chain(&gpio_notifier,
+					      GPIO_NOTIFY_CHIP_REMOVE, chip);
+	status = notifier_to_errno(status);
+	if (status)
+		return status;
+
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	for (id = chip->base; id < chip->base + chip->ngpio; id++) {

--- 

With the call_chain helper:

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1029,6 +1030,16 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
 
 #endif /* CONFIG_GPIO_SYSFS */
 
+static int gpio_call_chain(struct gpio_chip *chip, enum gpio_notify_msg msg)
+{
+	int ret = blocking_notifier_call_chain(&gpio_notifier, msg, chip);
+
+	return notifier_to_errno(ret);
+}
+
 /**
  * gpiochip_add() - register a gpio_chip
  * @chip: the chip to register, with chip->base initialized
@@ -1103,6 +1114,9 @@ fail:
 		pr_err("gpiochip_add: gpios %d..%d (%s) not registered\n",
 			chip->base, chip->base + chip->ngpio - 1,
 			chip->label ? : "generic");
+	else
+		gpio_call_chain(chip, GPIO_NOTIFY_CHIP_ADDED);
+
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add);
@@ -1119,6 +1133,11 @@ int gpiochip_remove(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 
+	/* Ask external subsystems to release the chip. */
+	status = gpio_call_chain(chip, GPIO_NOTIFY_CHIP_REMOVE);
+	if (status)
+		return status;
+
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	for (id = chip->base; id < chip->base + chip->ngpio; id++) {
--- 

Thanks!

Re: [PATCH 1/4] gpiolib: Introduce chip addition/removal notifier

[David Brownell]

On Tuesday 26 January 2010, Anton Vorontsov wrote:
> > Just
> > inline the little two blocking_notifier_call_chain() calls directly,
> > making this a *LOT* simpler.
> 
> I'd rather stay with gpio_call_chain() helper, it makes the code
> a little bit prettier, IMO. Compare this:

The one without the wrapper is IMO more clear, since it doesn't
obfuscate anything.  Fewer lines of code, too.  :)

Pretty is a good attribute ... but is a distant third to clarity.

[PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips

[Anton Vorontsov]

With this patch there are two ways to register OF GPIO controllers:

1. Allocating the of_gpio_chip structure and passing the
   &of_gc->gc pointer to the gpiochip_add. (Can use container_of
   to convert the gpio_chip to the of_gpio_chip.)

2. Allocating and registering the gpio_chip structure separately
   from the of_gpio_chip. (Since two allocations are separate,
   container_of won't work.)

As time goes by we'll kill the first option.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c |    1 +
 drivers/of/gpio.c                              |   23 +++++++++++++++++++++--
 include/linux/of_gpio.h                        |    3 ++-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 82a9bcb..73c7e6b 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -93,6 +93,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
 	gc->base = -1;
 	gc->set = mcu_gpio_set;
 	gc->direction_output = mcu_gpio_dir_out;
+	of_gc->chip = gc;
 	of_gc->gpio_cells = 2;
 	of_gc->xlate = of_gpio_simple_xlate;
 
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 6eea601..12c4af0 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -70,7 +70,7 @@ int of_get_gpio_flags(struct device_node *np, int index,
 	if (ret < 0)
 		goto err1;
 
-	ret += of_gc->gc.base;
+	ret += of_gc->chip->base;
 err1:
 	of_node_put(gc);
 err0:
@@ -140,7 +140,7 @@ int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
 		return -EINVAL;
 	}
 
-	if (*gpio > of_gc->gc.ngpio)
+	if (*gpio > of_gc->chip->ngpio)
 		return -EINVAL;
 
 	if (flags)
@@ -178,6 +178,25 @@ int of_mm_gpiochip_add(struct device_node *np,
 	struct of_gpio_chip *of_gc = &mm_gc->of_gc;
 	struct gpio_chip *gc = &of_gc->gc;
 
+	/*
+	 * Currently there are two ways to register OF GPIO controllers:
+	 *
+	 * 1. Allocating the of_gpio_chip structure and passing the
+	 *    &of_gc->gc pointer to the gpiochip_add. (Can use container_of
+	 *    to convert the gpio_chip to the of_gpio_chip.)
+	 *
+	 * 2. Allocating and registering the gpio_chip structure separately
+	 *    from the of_gpio_chip. (Since two allocations are separate,
+	 *    container_of won't work.)
+	 *
+	 * As time goes by we'll kill the first option. For now just check
+	 * if it's the new-style registration or the old-style.
+	 */
+	if (!of_gc->chip)
+		of_gc->chip = gc;
+	else
+		gc = of_gc->chip;
+
 	gc->label = kstrdup(np->full_name, GFP_KERNEL);
 	if (!gc->label)
 		goto err0;
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index fc2472c..c74cb37 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -36,7 +36,8 @@ enum of_gpio_flags {
  * Generic OF GPIO chip
  */
 struct of_gpio_chip {
-	struct gpio_chip gc;
+	struct gpio_chip gc; /* legacy, don't use for a new code */
+	struct gpio_chip *chip;
 	int gpio_cells;
 	int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
 		     const void *gpio_spec, enum of_gpio_flags *flags);
-- 
1.6.5.7

Re: [PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips

[David Brownell]

On Monday 25 January 2010, Anton Vorontsov wrote:
> With this patch there are two ways to register OF GPIO controllers:
> 
> 1. Allocating the of_gpio_chip structure and passing the
>    &of_gc->gc pointer to the gpiochip_add. (Can use container_of
>    to convert the gpio_chip to the of_gpio_chip.)
> 
> 2. Allocating and registering the gpio_chip structure separately
>    from the of_gpio_chip. (Since two allocations are separate,
>    container_of won't work.)
> 
> As time goes by we'll kill the first option.

Why have two options, instead of just the first/simpler one??

Re: [PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips

[Anton Vorontsov]

On Mon, Jan 25, 2010 at 10:36:15PM -0800, David Brownell wrote:
> On Monday 25 January 2010, Anton Vorontsov wrote:
> > With this patch there are two ways to register OF GPIO controllers:
> > 
> > 1. Allocating the of_gpio_chip structure and passing the
> >    &of_gc->gc pointer to the gpiochip_add. (Can use container_of
> >    to convert the gpio_chip to the of_gpio_chip.)
> > 
> > 2. Allocating and registering the gpio_chip structure separately
> >    from the of_gpio_chip. (Since two allocations are separate,
> >    container_of won't work.)
> > 
> > As time goes by we'll kill the first option.
> 
> Why have two options, instead of just the first/simpler one??

Because I2C/SPI drivers allocate (and register) gpio_chip structures
by themselves, so the first option is a no-go.

You can see the first option in use in
arch/powerpc/sysdev/qe_lib/gpio.c:

struct qe_gpio_chip {
        struct of_mm_gpio_chip mm_gc;
	....
};

Now include/linux/of_gpio.h:

struct of_mm_gpio_chip {
        struct of_gpio_chip of_gc;
	...
};

struct of_gpio_chip {
        struct gpio_chip gc; <- here, I'm going to get rid of it
	...
};

I2C/SPI drivers allocate gpio_chip structure already, so we don't
need to store 'struct gpio_chip gc', instead we need to store just
a pointer, and then attach the already allocated gpio_chip to the
of_gpio_chip stuff.

Having two ways to store gpio_chip isn't good, that's why
I stated that the first option will have to go, i.e. I'm going to
convert arch/powerpc/sysdev/qe_lib/gpio.c and few other of_mm
gpio chips to the new registration scheme soon.

Thanks,

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

Re: [PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips

[David Brownell]

On Tuesday 26 January 2010, Anton Vorontsov wrote:
> 
> > Why have two options, instead of just the first/simpler one??
> 
> Because I2C/SPI drivers allocate (and register) gpio_chip structures
> by themselves, so the first option is a no-go.

You should be mentioning such issues in the patch comments.

[PATCH 3/4] of/gpio: Implement GPIOLIB notifier hooks

[Anton Vorontsov]

This patch implements GPIOLIB notifier hooks, and thus makes device-enabled
GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically
attached to the OpenFirmware subsystem. Which means that now we can handle
I2C and SPI GPIO chips almost* transparently.

* "Almost" because some chips still require platform data, and for these
  chips OF-glue is still needed, though with this support the glue will
  be much smaller.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/of/Kconfig |    1 +
 drivers/of/gpio.c  |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index d2fa27c..de9f987 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -5,6 +5,7 @@ config OF_DEVICE
 config OF_GPIO
 	def_bool y
 	depends on OF && (PPC_OF || MICROBLAZE) && GPIOLIB
+	select GPIOLIB_NOTIFIER
 	help
 	  OpenFirmware GPIO accessors
 
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 12c4af0..9d8df77 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/errno.h>
+#include <linux/notifier.h>
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
@@ -236,3 +237,102 @@ err0:
 	return ret;
 }
 EXPORT_SYMBOL(of_mm_gpiochip_add);
+
+/**
+ * of_gpiochip_register_simple - Register a chip with the OF GPIO subsystem
+ * @chip	pointer to a GPIO chip
+ * @np:		device node to register the GPIO chip with
+ *
+ * This function registers a GPIO chip with the OF infrastructure. It is
+ * assumed that the chip was previsously allocated and added to a generic
+ * GPIOLIB framework (using gpiochip_add() function).
+ *
+ * The `simple' name means that the chip is using simple two-cells scheme for
+ * the gpio-specifier.
+ */
+static int of_gpiochip_register_simple(struct gpio_chip *chip,
+				       struct device_node *np)
+{
+	struct of_gpio_chip *of_gc;
+
+	if (np->data) {
+		WARN_ON(1);
+		return -EBUSY;
+	}
+
+	of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL);
+	if (!of_gc)
+		return -ENOMEM;
+
+	of_gc->gpio_cells = 2;
+	of_gc->xlate = of_gpio_simple_xlate;
+	of_gc->chip = chip;
+	np->data = of_gc;
+	of_node_get(np);
+
+	return 0;
+}
+EXPORT_SYMBOL(of_gpiochip_register_simple);
+
+/**
+ * of_gpiochip_unregister - Unregister a GPIO chip
+ * @chip	pointer to a GPIO chip
+ * @np:		device node for which the GPIO chip was previously registered
+ *
+ * This function unregisters a GPIO chip that was previsously registered
+ * with of_gpiochip_register*().
+ */
+static int of_gpiochip_unregister(struct gpio_chip *chip,
+				  struct device_node *np)
+{
+	struct of_gpio_chip *of_gc = np->data;
+
+	if (!of_gc || of_gc->chip != chip) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	np->data = NULL;
+	kfree(of_gc);
+	of_node_put(np);
+
+	return 0;
+}
+
+static int of_gpio_notify(struct notifier_block *nb, unsigned long msg,
+			  void *chip)
+{
+	struct gpio_chip *gc = chip;
+	struct device_node *np;
+	int ret = 0;
+
+	if (!gc->dev)
+		return NOTIFY_DONE;
+
+	np = dev_archdata_get_node(&gc->dev->archdata);
+	if (!np)
+		return NOTIFY_DONE;
+
+	switch (msg) {
+	case GPIO_NOTIFY_CHIP_ADDED:
+		ret = of_gpiochip_register_simple(gc, np);
+		break;
+	case GPIO_NOTIFY_CHIP_REMOVE:
+		ret = of_gpiochip_unregister(gc, np);
+		break;
+	default:
+		break;
+	}
+
+	return ret ? notifier_from_errno(ret) : NOTIFY_OK;
+}
+
+static struct notifier_block of_gpio_nb = {
+	.notifier_call = of_gpio_notify,
+};
+
+static int __init of_gpio_notifier_init(void)
+{
+	return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb);
+}
+arch_initcall(of_gpio_notifier_init);
-- 
1.6.5.7

[PATCH 4/4] powerpc/mcu_mpc8349emitx: Remove OF GPIO handling stuff

[Anton Vorontsov]

With the new OF GPIO infrastructure it's much easier to handle I2C
GPIO controllers, i.e. now drivers don't have to deal with the
OF-specific bits.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c |   68 +++++-------------------
 1 files changed, 14 insertions(+), 54 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 73c7e6b..5525175 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -18,8 +18,6 @@
 #include <linux/mutex.h>
 #include <linux/i2c.h>
 #include <linux/gpio.h>
-#include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <asm/prom.h>
 #include <asm/machdep.h>
 
@@ -36,7 +34,7 @@ struct mcu {
 	struct mutex lock;
 	struct device_node *np;
 	struct i2c_client *client;
-	struct of_gpio_chip of_gc;
+	struct gpio_chip gc;
 	u8 reg_ctrl;
 };
 
@@ -55,8 +53,7 @@ static void mcu_power_off(void)
 
 static void mcu_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 {
-	struct of_gpio_chip *of_gc = to_of_gpio_chip(gc);
-	struct mcu *mcu = container_of(of_gc, struct mcu, of_gc);
+	struct mcu *mcu = container_of(gc, struct mcu, gc);
 	u8 bit = 1 << (4 + gpio);
 
 	mutex_lock(&mcu->lock);
@@ -75,53 +72,6 @@ static int mcu_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	return 0;
 }
 
-static int mcu_gpiochip_add(struct mcu *mcu)
-{
-	struct device_node *np;
-	struct of_gpio_chip *of_gc = &mcu->of_gc;
-	struct gpio_chip *gc = &of_gc->gc;
-	int ret;
-
-	np = of_find_compatible_node(NULL, NULL, "fsl,mcu-mpc8349emitx");
-	if (!np)
-		return -ENODEV;
-
-	gc->owner = THIS_MODULE;
-	gc->label = np->full_name;
-	gc->can_sleep = 1;
-	gc->ngpio = MCU_NUM_GPIO;
-	gc->base = -1;
-	gc->set = mcu_gpio_set;
-	gc->direction_output = mcu_gpio_dir_out;
-	of_gc->chip = gc;
-	of_gc->gpio_cells = 2;
-	of_gc->xlate = of_gpio_simple_xlate;
-
-	np->data = of_gc;
-	mcu->np = np;
-
-	/*
-	 * We don't want to lose the node, its ->data and ->full_name...
-	 * So, if succeeded, we don't put the node here.
-	 */
-	ret = gpiochip_add(gc);
-	if (ret)
-		of_node_put(np);
-	return ret;
-}
-
-static int mcu_gpiochip_remove(struct mcu *mcu)
-{
-	int ret;
-
-	ret = gpiochip_remove(&mcu->of_gc.gc);
-	if (ret)
-		return ret;
-	of_node_put(mcu->np);
-
-	return 0;
-}
-
 static int __devinit mcu_probe(struct i2c_client *client,
 			       const struct i2c_device_id *id)
 {
@@ -141,7 +91,16 @@ static int __devinit mcu_probe(struct i2c_client *client,
 		goto err;
 	mcu->reg_ctrl = ret;
 
-	ret = mcu_gpiochip_add(mcu);
+	mcu->gc.dev = &client->dev;
+	mcu->gc.owner = THIS_MODULE;
+	mcu->gc.label = dev_name(&client->dev);
+	mcu->gc.can_sleep = 1;
+	mcu->gc.ngpio = MCU_NUM_GPIO;
+	mcu->gc.base = -1;
+	mcu->gc.set = mcu_gpio_set;
+	mcu->gc.direction_output = mcu_gpio_dir_out;
+
+	ret = gpiochip_add(&mcu->gc);
 	if (ret)
 		goto err;
 
@@ -168,9 +127,10 @@ static int __devexit mcu_remove(struct i2c_client *client)
 		glob_mcu = NULL;
 	}
 
-	ret = mcu_gpiochip_remove(mcu);
+	ret = gpiochip_remove(&mcu->gc);
 	if (ret)
 		return ret;
+
 	i2c_set_clientdata(client, NULL);
 	kfree(mcu);
 	return 0;
-- 
1.6.5.7

Re: [PATCH 4/4] powerpc/mcu_mpc8349emitx: Remove OF GPIO handling stuff

[David Brownell]

On Monday 25 January 2010, Anton Vorontsov wrote:
> With the new OF GPIO infrastructure it's much easier to handle I2C
> GPIO controllers, i.e. now drivers don't have to deal with the
> OF-specific bits.

Good, that's how it should have been done in the first place.  :)

Of course, there's still the question of how to get driver-specific
configuration data into the driver...

[PATCH v2 0/4] OF GPIO integration for I2C/SPI GPIO chips

[Anton Vorontsov]

Hi all,

Here comes v2. Changes:

- In 'gpiolib: Introduce chip addition/removal notifier', open-coded
  gpio_call_chain().

- In 'of/gpio: Add support for two-stage registration for the
  of_gpio_chips', improved commit message.


David, I think it would be best if these patches could go via powerpc
or OF trees, so your Ack would be highly appreciated.

Thanks!

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

[PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips

[Anton Vorontsov]

I2C/SPI drivers allocate gpio_chip structure already, so we don't need
'struct gpio_chip gc' in the OF GPIO subsystem, instead we need to store
just a pointer, and then attach the already allocated gpio_chip to the
of_gpio_chip structure.

With this patch there are two ways to register OF GPIO controllers:

1. Allocating the of_gpio_chip structure and passing the
   &of_gc->gc pointer to the gpiochip_add. (Can use container_of
   to convert the gpio_chip to the of_gpio_chip.)

2. Allocating and registering the gpio_chip structure separately
   from the of_gpio_chip. (Since two allocations are separate,
   container_of won't work.)

As time goes by we'll kill the first option.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c |    1 +
 drivers/of/gpio.c                              |   23 +++++++++++++++++++++--
 include/linux/of_gpio.h                        |    3 ++-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 82a9bcb..73c7e6b 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -93,6 +93,7 @@ static int mcu_gpiochip_add(struct mcu *mcu)
 	gc->base = -1;
 	gc->set = mcu_gpio_set;
 	gc->direction_output = mcu_gpio_dir_out;
+	of_gc->chip = gc;
 	of_gc->gpio_cells = 2;
 	of_gc->xlate = of_gpio_simple_xlate;
 
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 6eea601..12c4af0 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -70,7 +70,7 @@ int of_get_gpio_flags(struct device_node *np, int index,
 	if (ret < 0)
 		goto err1;
 
-	ret += of_gc->gc.base;
+	ret += of_gc->chip->base;
 err1:
 	of_node_put(gc);
 err0:
@@ -140,7 +140,7 @@ int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
 		return -EINVAL;
 	}
 
-	if (*gpio > of_gc->gc.ngpio)
+	if (*gpio > of_gc->chip->ngpio)
 		return -EINVAL;
 
 	if (flags)
@@ -178,6 +178,25 @@ int of_mm_gpiochip_add(struct device_node *np,
 	struct of_gpio_chip *of_gc = &mm_gc->of_gc;
 	struct gpio_chip *gc = &of_gc->gc;
 
+	/*
+	 * Currently there are two ways to register OF GPIO controllers:
+	 *
+	 * 1. Allocating the of_gpio_chip structure and passing the
+	 *    &of_gc->gc pointer to the gpiochip_add. (Can use container_of
+	 *    to convert the gpio_chip to the of_gpio_chip.)
+	 *
+	 * 2. Allocating and registering the gpio_chip structure separately
+	 *    from the of_gpio_chip. (Since two allocations are separate,
+	 *    container_of won't work.)
+	 *
+	 * As time goes by we'll kill the first option. For now just check
+	 * if it's the new-style registration or the old-style.
+	 */
+	if (!of_gc->chip)
+		of_gc->chip = gc;
+	else
+		gc = of_gc->chip;
+
 	gc->label = kstrdup(np->full_name, GFP_KERNEL);
 	if (!gc->label)
 		goto err0;
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index fc2472c..c74cb37 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -36,7 +36,8 @@ enum of_gpio_flags {
  * Generic OF GPIO chip
  */
 struct of_gpio_chip {
-	struct gpio_chip gc;
+	struct gpio_chip gc; /* legacy, don't use for a new code */
+	struct gpio_chip *chip;
 	int gpio_cells;
 	int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
 		     const void *gpio_spec, enum of_gpio_flags *flags);
-- 
1.6.5.7