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: [email protected]
irc://irc.freenode.net/bd2
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 <[email protected]>
---
drivers/gpio/gpiolib.c | 14 ++++++++++++++
include/asm-generic/gpio.h | 8 ++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 350842a..375c03a 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.
@@ -1029,6 +1030,9 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
#endif /* CONFIG_GPIO_SYSFS */
+BLOCKING_NOTIFIER_HEAD(gpio_notifier);
+EXPORT_SYMBOL_GPL(gpio_notifier);
+
/**
* gpiochip_add() - register a gpio_chip
* @chip: the chip to register, with chip->base initialized
@@ -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++) {
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
drivers/of/gpio.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 100 insertions(+), 0 deletions(-)
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
On Fri, 5 Feb 2010 23:32:36 +0300
Anton Vorontsov <[email protected]> wrote:
> 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.
>
We can clean this up a little with the WARN_ON trick:
diff -puN drivers/of/gpio.c~of-gpio-implement-gpiolib-notifier-hooks-fix drivers/of/gpio.c
--- a/drivers/of/gpio.c~of-gpio-implement-gpiolib-notifier-hooks-fix
+++ a/drivers/of/gpio.c
@@ -255,10 +255,8 @@ static int of_gpiochip_register_simple(s
{
struct of_gpio_chip *of_gc;
- if (np->data) {
- WARN_ON(1);
+ if (WARN_ON(np->data))
return -EBUSY;
- }
of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL);
if (!of_gc)
@@ -287,10 +285,8 @@ static int of_gpiochip_unregister(struct
{
struct of_gpio_chip *of_gc = np->data;
- if (!of_gc || of_gc->chip != chip) {
- WARN_ON(1);
+ if (WARN_ON(!of_gc || of_gc->chip != chip))
return -EINVAL;
- }
np->data = NULL;
kfree(of_gc);
_
On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov
<[email protected]> wrote:
> 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 <[email protected]>
> ---
> ?drivers/of/gpio.c | ?100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> ?1 files changed, 100 insertions(+), 0 deletions(-)
>
> 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;
One concern.
How does an OF-aware GPIO driver override these settings? What is to
be done when a GPIO chip requires a different xlate hook? Or a
different number of gpio_cells?
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov
<[email protected]> wrote:
> 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 <[email protected]>
> ---
> +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);
Another concern; if any gpio chips get registered before this
arch_initcall (not sure if it is possible or not), then those chips
won't get registered with the of gpio infrastructure.
g.
On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov
<[email protected]> wrote:
> 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 <[email protected]>
> ---
> ?drivers/gpio/gpiolib.c ? ? | ? 14 ++++++++++++++
> ?include/asm-generic/gpio.h | ? ?8 ++++++++
> ?2 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 350842a..375c03a 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.
> @@ -1029,6 +1030,9 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
>
> ?#endif /* CONFIG_GPIO_SYSFS */
>
> +BLOCKING_NOTIFIER_HEAD(gpio_notifier);
> +EXPORT_SYMBOL_GPL(gpio_notifier);
> +
> ?/**
> ?* gpiochip_add() - register a gpio_chip
> ?* @chip: the chip to register, with chip->base initialized
> @@ -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);
Rather than doing an else block which will need to be reworked if/when
any additional code is added to the bottom of this routine, please
rework the if() block to bail on failure instead of implicitly falling
through to the return statement.
Otherwise, this patch looks okay to me, so you can go ahead and add my:
Acked-by: Grant Likely <[email protected]>
*however* (and don't kill me for saying this because I know I
suggested the notifier approach in the first place). Looking at the
whole patch series, the notifier call adds a lot of code for very
little gain. If you dropped just the notifier bits (but left the rest
of the series the same), then the the of gpio bits would be
considerably simpler, and the only impact on the core gpiolib would be
the addition of an of_gpiochip_register_simple() and
of_gpiochip_unregister() hooks that will be conditionally compiled.
And to address one of my previous concerns, I've got no problem with
the automatic registration of GPIO devices for OF usage, as long as
of-aware drivers have the option of overriding the simple defaults
when needed.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
On Tue, Feb 09, 2010 at 10:08:00AM -0700, Grant Likely wrote:
> On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov
> <[email protected]> wrote:
> > 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.
[...]
> One concern.
>
> How does an OF-aware GPIO driver override these settings? What is to
> be done when a GPIO chip requires a different xlate hook? Or a
> different number of gpio_cells?
A lot of options...
1. They can hook up onto a notifier chain, ensure that their callback
will be called after OF GPIO subsystem (using notifiers' priority
mechanism), and fixup needed stuff.
2. They can write their own full fledged OF bindings, i.e. they will
need to allocate of_gc struct and save it into np->data.
of_gpiochip_register_simple() won't touch np->data if it's already
used.
If/when needed, we can write some helper function, i.e.
of_gpiochip_register(gpiochip, node, xlate_callback).
3. Or of/gpio.c code will handle this by itself, iff the xlate and
gpio-cells scheme seems generic enough.
4. May be more...
Thanks,
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Tue, Feb 09, 2010 at 10:13:11AM -0700, Grant Likely wrote:
[...]
> > +static int __init of_gpio_notifier_init(void)
> > +{
> > + return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb);
> > +}
> > +arch_initcall(of_gpio_notifier_init);
>
> Another concern; if any gpio chips get registered before this
> arch_initcall (not sure if it is possible or not), then those chips
> won't get registered with the of gpio infrastructure.
Technically, it is possible, but registering usual GPIO controllers
in arch_initcall feels not quite right approach in the first place
(and, btw, it won't work most of the time, because even early drivers
do not register itself earlier than subsys_initcall).
And arch gpio controllers (like QE GPIO) are usually device-less,
and they use of_mm_gpiochip_add(), so we fully control them.
Plus I don't see any reason why we couldn't move
of_gpio_notifier_init() into, say, postcore_initcall, if we ever
need it.
Thanks,
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Fri, Mar 12, 2010 at 02:38:02PM -0700, Grant Likely wrote:
[...]
> How to proceed: I'd like to leave this series out for the 2.6.34
> cycle and I'll pick it into my OF tree before the 2.6.35 merge window,
> but I'll probably modify it to call the OF hooks directly and leave
> out the unnecessary notifier infrastructure.
Ping?