linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 2.6.24-rc4-mm 1/6] gpiolib: add gpio_desc[]
       [not found] <200712092022.14062.david-b@pacbell.net>
@ 2007-12-10  4:39 ` David Brownell
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 2/6] gpiolib: more CONFIG_DEBUG_GPIO diagnostics David Brownell
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:39 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list; +Cc: eric miao

From: David Brownell <dbrownell@users.sourceforge.net>

Update gpiolib to use a table of per-GPIO "struct gpio_desc" instead of
a table of "struct gpio_chip".

 - Change "is_out" and "requested" from arrays in "struct gpio_chip" to
   bit fields in "struct gpio_desc", eliminating ARCH_GPIOS_PER_CHIP.

 - Stop overloading "requested" flag with "label" tracked for debugfs.

 - Change gpiochip_is_requested() into a regular function, since it
   now accesses data that's not exported from the gpiolib code.  Also
   change its signature, for the same reason.

 - Reduce default ARCH_NR_GPIOS to 256 to shrink gpio_desc table cost.
   On 32-bit platforms without debugfs, that table size is 2KB.

This makes it easier to work with chips with different numbers of GPIOs,
and to avoid holes in GPIOs number sequences.  Those holes can cost a
lot of unusable irq_desc space for GPIOs that act as IRQs; and needing
the can cause surprises when trying to set up multiple GPIO controllers.

Based on a patch from Eric Miao.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Eric Miao <eric.miao@marvell.com>
---
 arch/avr32/mach-at32ap/pio.c         |    7 -
 include/asm-avr32/arch-at32ap/gpio.h |    2 
 include/asm-generic/gpio.h           |   49 +-------
 lib/gpiolib.c                        |  213 ++++++++++++++++++++---------------
 4 files changed, 139 insertions(+), 132 deletions(-)

--- a/arch/avr32/mach-at32ap/pio.c	2007-12-09 19:50:50.000000000 -0800
+++ b/arch/avr32/mach-at32ap/pio.c	2007-12-09 19:50:59.000000000 -0800
@@ -315,12 +315,15 @@ static void pio_bank_show(struct seq_fil
 	bank = 'A' + pio->pdev->id;
 
 	for (i = 0, mask = 1; i < 32; i++, mask <<= 1) {
-		if (!gpiochip_is_requested(chip, i))
+		const char *label;
+
+		label = gpiochip_is_requested(chip, i);
+		if (!label)
 			continue;
 
 		seq_printf(s, " gpio-%-3d P%c%-2d (%-12s) %s %s %s",
 			chip->base + i, bank, i,
-			chip->requested[i],
+			label,
 			(osr & mask) ? "out" : "in ",
 			(mask & pdsr) ? "hi" : "lo",
 			(mask & pusr) ? "  " : "up");
--- a/include/asm-avr32/arch-at32ap/gpio.h	2007-12-09 19:50:50.000000000 -0800
+++ b/include/asm-avr32/arch-at32ap/gpio.h	2007-12-09 19:50:59.000000000 -0800
@@ -8,7 +8,7 @@
 /* Some GPIO chips can manage IRQs; some can't.  The exact numbers can
  * be changed if needed, but for the moment they're not configurable.
  */
-#define ARCH_NR_GPIOS	(NR_GPIO_IRQS + 2 * ARCH_GPIOS_PER_CHIP)
+#define ARCH_NR_GPIOS	(NR_GPIO_IRQS + 2 * 32)
 
 
 /* Arch-neutral GPIO API, supporting both "native" and external GPIOs. */
--- a/include/asm-generic/gpio.h	2007-12-09 19:50:50.000000000 -0800
+++ b/include/asm-generic/gpio.h	2007-12-09 19:50:59.000000000 -0800
@@ -4,21 +4,16 @@
 #ifdef CONFIG_GPIO_LIB
 
 /* Platforms may implement their GPIO interface with library code,
- * at a small performance cost for non-inlined operations.
+ * at a small performance cost for non-inlined operations and some
+ * extra memory (for code and for per-GPIO table entries).
  *
  * While the GPIO programming interface defines valid GPIO numbers
  * to be in the range 0..MAX_INT, this library restricts them to the
- * smaller range 0..ARCH_NR_GPIOS and allocates them in groups of
- * ARCH_GPIOS_PER_CHIP (which will usually be the word size used for
- * each bank of a SOC processor's integrated GPIO modules).
+ * smaller range 0..ARCH_NR_GPIOS.
  */
 
 #ifndef ARCH_NR_GPIOS
-#define ARCH_NR_GPIOS		512
-#endif
-
-#ifndef ARCH_GPIOS_PER_CHIP
-#define ARCH_GPIOS_PER_CHIP	BITS_PER_LONG
+#define ARCH_NR_GPIOS		256
 #endif
 
 struct seq_file;
@@ -36,21 +31,18 @@ struct seq_file;
  *	state (such as pullup/pulldown configuration).
  * @base: identifies the first GPIO number handled by this chip; or, if
  *	negative during registration, requests dynamic ID allocation.
- * @ngpio: the number of GPIOs handled by this controller; the value must
- *	be at most ARCH_GPIOS_PER_CHIP, so the last GPIO handled is
- *	(base + ngpio - 1).
+ * @ngpio: the number of GPIOs handled by this controller; the last GPIO
+ *	handled is (base + ngpio - 1).
  * @can_sleep: flag must be set iff get()/set() methods sleep, as they
  *	must while accessing GPIO expander chips over I2C or SPI
- * @is_out: bit array where bit N is true iff GPIO with offset N has been
- *	 called successfully to configure this as an output
  *
  * A gpio_chip can help platforms abstract various sources of GPIOs so
  * they can all be accessed through a common programing interface.
  * Example sources would be SOC controllers, FPGAs, multifunction
  * chips, dedicated GPIO expanders, and so on.
  *
- * Each chip controls a number of signals, numbered 0..@ngpio, which are
- * identified in method calls by an "offset" value.  When those signals
+ * Each chip controls a number of signals, identified in method calls
+ * by "offset" values in the range 0..(@ngpio - 1).  When those signals
  * are referenced through calls like gpio_get_value(gpio), the offset
  * is calculated by subtracting @base from the gpio number.
  */
@@ -70,31 +62,10 @@ struct gpio_chip {
 	int			base;
 	u16			ngpio;
 	unsigned		can_sleep:1;
-
-	/* other fields are modified by the gpio library only */
-	DECLARE_BITMAP(is_out, ARCH_GPIOS_PER_CHIP);
-
-#ifdef CONFIG_DEBUG_FS
-	/* fat bits */
-	const char		*requested[ARCH_GPIOS_PER_CHIP];
-#else
-	/* thin bits */
-	DECLARE_BITMAP(requested, ARCH_GPIOS_PER_CHIP);
-#endif
 };
 
-/* returns true iff a given gpio signal has been requested;
- * primarily for code dumping gpio_chip state.
- */
-static inline int
-gpiochip_is_requested(struct gpio_chip *chip, unsigned offset)
-{
-#ifdef CONFIG_DEBUG_FS
-	return chip->requested[offset] != NULL;
-#else
-	return test_bit(offset, chip->requested);
-#endif
-}
+extern const char *gpiochip_is_requested(struct gpio_chip *chip,
+			unsigned offset);
 
 /* add/remove chips */
 extern int gpiochip_add(struct gpio_chip *chip);
--- a/lib/gpiolib.c	2007-12-09 19:50:50.000000000 -0800
+++ b/lib/gpiolib.c	2007-12-09 19:50:59.000000000 -0800
@@ -28,39 +28,43 @@
 #define	extra_checks	0
 #endif
 
-/* gpio_lock protects the table of chips and gpio_chip->requested.
+/* gpio_lock prevents conflicts during gpio_desc[] table updates.
  * While any GPIO is requested, its gpio_chip is not removable;
  * each GPIO's "requested" flag serves as a lock and refcount.
  */
 static DEFINE_SPINLOCK(gpio_lock);
-static struct gpio_chip *chips[DIV_ROUND_UP(ARCH_NR_GPIOS,
-					ARCH_GPIOS_PER_CHIP)];
+
+struct gpio_desc {
+	struct gpio_chip	*chip;
+	unsigned long		flags;
+/* flag symbols are bit numbers */
+#define FLAG_REQUESTED	0
+#define FLAG_IS_OUT	1
+
+#ifdef CONFIG_DEBUG_FS
+	const char		*label;
+#endif
+};
+static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
 
 
 /* Warn when drivers omit gpio_request() calls -- legal but
  * ill-advised when setting direction, and otherwise illegal.
  */
-static void gpio_ensure_requested(struct gpio_chip *chip, unsigned offset)
+static void gpio_ensure_requested(struct gpio_desc *desc)
 {
-	int		requested;
-
+	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
+		pr_warning("GPIO-%d autorequested\n", (int)(desc - gpio_desc));
 #ifdef CONFIG_DEBUG_FS
-	requested = (int) chip->requested[offset];
-	if (!requested)
-		chip->requested[offset] = "[auto]";
-#else
-	requested = test_and_set_bit(offset, chip->requested);
+		desc->label = "[auto]";
 #endif
-
-	if (!requested)
-		printk(KERN_DEBUG "GPIO-%d autorequested\n",
-				chip->base + offset);
+	}
 }
 
 /* caller holds gpio_lock *OR* gpio is marked as requested */
 static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
 {
-	return chips[gpio / ARCH_GPIOS_PER_CHIP];
+	return gpio_desc[gpio].chip;
 }
 
 /**
@@ -78,20 +82,28 @@ int gpiochip_add(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 
-	if (chip->base < 0 || (chip->base % ARCH_GPIOS_PER_CHIP) != 0)
-		return -EINVAL;
-	if ((chip->base + chip->ngpio) >= ARCH_NR_GPIOS)
-		return -EINVAL;
-	if (chip->ngpio > ARCH_GPIOS_PER_CHIP)
+	/* NOTE chip->base negative is reserved to mean a request for
+	 * dynamic allocation.  We don't currently support that.
+	 */
+
+	if (chip->base < 0 || (chip->base  + chip->ngpio) >= ARCH_NR_GPIOS)
 		return -EINVAL;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	id = chip->base / ARCH_GPIOS_PER_CHIP;
-	if (chips[id] == NULL)
-		chips[id] = chip;
-	else
-		status = -EBUSY;
+	/* these GPIO numbers must not be managed by another gpio_chip */
+	for (id = chip->base; id < chip->base + chip->ngpio; id++) {
+		if (gpio_desc[id].chip != NULL) {
+			status = -EBUSY;
+			break;
+		}
+	}
+	if (status == 0) {
+		for (id = chip->base; id < chip->base + chip->ngpio; id++) {
+			gpio_desc[id].chip = chip;
+			gpio_desc[id].flags = 0;
+		}
+	}
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
 	return status;
@@ -108,23 +120,19 @@ int gpiochip_remove(struct gpio_chip *ch
 {
 	unsigned long	flags;
 	int		status = 0;
-	int		offset;
-	unsigned	id = chip->base / ARCH_GPIOS_PER_CHIP;
+	unsigned	id;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	for (offset = 0; offset < chip->ngpio; offset++) {
-		if (gpiochip_is_requested(chip, offset)) {
+	for (id = chip->base; id < chip->base + chip->ngpio; id++) {
+		if (test_bit(FLAG_REQUESTED, &gpio_desc[id].flags)) {
 			status = -EBUSY;
 			break;
 		}
 	}
-
 	if (status == 0) {
-		if (chips[id] == chip)
-			chips[id] = NULL;
-		else
-			status = -EINVAL;
+		for (id = chip->base; id < chip->base + chip->ngpio; id++)
+			gpio_desc[id].chip = NULL;
 	}
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
@@ -139,35 +147,29 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
  */
 int gpio_request(unsigned gpio, const char *label)
 {
-	struct gpio_chip	*chip;
+	struct gpio_desc	*desc;
 	int			status = -EINVAL;
 	unsigned long		flags;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	chip = gpio_to_chip(gpio);
-	if (!chip)
+	if (gpio >= ARCH_NR_GPIOS)
 		goto done;
-	gpio -= chip->base;
-	if (gpio >= chip->ngpio)
+	desc = &gpio_desc[gpio];
+	if (desc->chip == NULL)
 		goto done;
 
 	/* NOTE:  gpio_request() can be called in early boot,
 	 * before IRQs are enabled.
 	 */
 
-	status = 0;
+	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
 #ifdef CONFIG_DEBUG_FS
-	if (!label)
-		label = "?";
-	if (chip->requested[gpio])
-		status = -EBUSY;
-	else
-		chip->requested[gpio] = label;
-#else
-	if (test_and_set_bit(gpio, chip->requested))
-		status = -EBUSY;
+		desc->label = (label == NULL) ? "?" : label;
 #endif
+		status = 0;
+	} else
+		status = -EBUSY;
 
 done:
 	spin_unlock_irqrestore(&gpio_lock, flags);
@@ -178,57 +180,85 @@ EXPORT_SYMBOL_GPL(gpio_request);
 void gpio_free(unsigned gpio)
 {
 	unsigned long		flags;
-	struct gpio_chip	*chip;
+	struct gpio_desc	*desc;
 
-	spin_lock_irqsave(&gpio_lock, flags);
+	if (gpio >= ARCH_NR_GPIOS) {
+		WARN_ON(extra_checks);
+		return;
+	}
 
-	chip = gpio_to_chip(gpio);
-	if (!chip)
-		goto done;
-	gpio -= chip->base;
-	if (gpio >= chip->ngpio)
-		goto done;
+	spin_lock_irqsave(&gpio_lock, flags);
 
+	desc = &gpio_desc[gpio];
+	if (desc->chip && test_and_clear_bit(FLAG_REQUESTED, &desc->flags)) {
 #ifdef CONFIG_DEBUG_FS
-	if (chip->requested[gpio])
-		chip->requested[gpio] = NULL;
-	else
-		chip = NULL;
-#else
-	if (!test_and_clear_bit(gpio, chip->requested))
-		chip = NULL;
+		desc->label = NULL;
 #endif
-	WARN_ON(extra_checks && chip == NULL);
-done:
+	} else
+		WARN_ON(extra_checks);
+
 	spin_unlock_irqrestore(&gpio_lock, flags);
 }
 EXPORT_SYMBOL_GPL(gpio_free);
 
 
+/**
+ * gpiochip_is_requested - return string iff signal was requested
+ * @chip: controller managing the signal
+ * @offset: of signal within controller's 0..(ngpio - 1) range
+ *
+ * Returns NULL if the GPIO is not currently requested, else a string.
+ * If debugfs support is enabled, the string returned is the label passed
+ * to gpio_request(); otherwise it is a meaningless constant.
+ *
+ * This function is for use by GPIO controller drivers.  The label can
+ * help with diagnostics, and knowing that the signal is used as a GPIO
+ * can help avoid accidentally multiplexing it to another controller.
+ */
+const char *gpiochip_is_requested(struct gpio_chip *chip, unsigned offset)
+{
+	unsigned gpio = chip->base + offset;
 
-/* Drivers MUST make configuration calls before get/set calls
+	if (gpio >= ARCH_NR_GPIOS || gpio_desc[gpio].chip != chip)
+		return NULL;
+	if (test_bit(FLAG_REQUESTED, &gpio_desc[gpio].flags) == 0)
+		return NULL;
+#ifdef CONFIG_DEBUG_FS
+	return gpio_desc[gpio].label;
+#else
+	return "?";
+#endif
+}
+EXPORT_SYMBOL_GPL(gpiochip_is_requested);
+
+
+/* Drivers MUST set GPIO direction before making get/set calls.  In
+ * some cases this is done in early boot, before IRQs are enabled.
  *
  * As a rule these aren't called more than once (except for drivers
  * using the open-drain emulation idiom) so these are natural places
- * to accumulate extra debugging checks.  Note that we can't rely on
- * gpio_request() having been called beforehand.
+ * to accumulate extra debugging checks.  Note that we can't (yet)
+ * rely on gpio_request() having been called beforehand.
  */
 
 int gpio_direction_input(unsigned gpio)
 {
 	unsigned long		flags;
 	struct gpio_chip	*chip;
+	struct gpio_desc	*desc = &gpio_desc[gpio];
 	int			status = -EINVAL;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	chip = gpio_to_chip(gpio);
+	if (gpio >= ARCH_NR_GPIOS)
+		goto fail;
+	chip = desc->chip;
 	if (!chip || !chip->get || !chip->direction_input)
 		goto fail;
 	gpio -= chip->base;
 	if (gpio >= chip->ngpio)
 		goto fail;
-	gpio_ensure_requested(chip, gpio);
+	gpio_ensure_requested(desc);
 
 	/* now we know the gpio is valid and chip won't vanish */
 
@@ -238,7 +268,7 @@ int gpio_direction_input(unsigned gpio)
 
 	status = chip->direction_input(chip, gpio);
 	if (status == 0)
-		clear_bit(gpio, chip->is_out);
+		clear_bit(FLAG_IS_OUT, &desc->flags);
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
@@ -250,17 +280,20 @@ int gpio_direction_output(unsigned gpio,
 {
 	unsigned long		flags;
 	struct gpio_chip	*chip;
+	struct gpio_desc	*desc = &gpio_desc[gpio];
 	int			status = -EINVAL;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
-	chip = gpio_to_chip(gpio);
+	if (gpio >= ARCH_NR_GPIOS)
+		goto fail;
+	chip = desc->chip;
 	if (!chip || !chip->set || !chip->direction_output)
 		goto fail;
 	gpio -= chip->base;
 	if (gpio >= chip->ngpio)
 		goto fail;
-	gpio_ensure_requested(chip, gpio);
+	gpio_ensure_requested(desc);
 
 	/* now we know the gpio is valid and chip won't vanish */
 
@@ -270,7 +303,7 @@ int gpio_direction_output(unsigned gpio,
 
 	status = chip->direction_output(chip, gpio, value);
 	if (status == 0)
-		set_bit(gpio, chip->is_out);
+		set_bit(FLAG_IS_OUT, &desc->flags);
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
@@ -366,20 +399,18 @@ EXPORT_SYMBOL_GPL(gpio_set_value_canslee
 
 static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
-	unsigned	i;
-
-	for (i = 0; i < chip->ngpio; i++) {
-		unsigned	gpio;
-		int		is_out;
+	unsigned		i;
+	unsigned		gpio = chip->base;
+	struct gpio_desc	*gdesc = &gpio_desc[gpio];
+	int			is_out;
 
-		if (!chip->requested[i])
+	for (i = 0; i < chip->ngpio; i++, gpio++, gdesc++) {
+		if (!test_bit(FLAG_REQUESTED, &gdesc->flags))
 			continue;
 
-		gpio = chip->base + i;
-		is_out = test_bit(i, chip->is_out);
-
+		is_out = test_bit(FLAG_IS_OUT, &gdesc->flags);
 		seq_printf(s, " gpio-%-3d (%-12s) %s %s",
-			gpio, chip->requested[i],
+			gpio, gdesc->label,
 			is_out ? "out" : "in ",
 			chip->get
 				? (chip->get(chip, i) ? "hi" : "lo")
@@ -435,14 +466,16 @@ static void gpiolib_dbg_show(struct seq_
 
 static int gpiolib_show(struct seq_file *s, void *unused)
 {
-	struct gpio_chip	*chip;
-	unsigned		id;
+	struct gpio_chip	*chip = NULL;
+	unsigned		gpio;
 	int			started = 0;
 
 	/* REVISIT this isn't locked against gpio_chip removal ... */
 
-	for (id = 0; id < ARRAY_SIZE(chips); id++) {
-		chip = chips[id];
+	for (gpio = 0; gpio < ARCH_NR_GPIOS; gpio++) {
+		if (chip == gpio_desc[gpio].chip)
+			continue;
+		chip = gpio_desc[gpio].chip;
 		if (!chip)
 			continue;
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch 2.6.24-rc4-mm 2/6] gpiolib: more CONFIG_DEBUG_GPIO diagnostics
       [not found] <200712092022.14062.david-b@pacbell.net>
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 1/6] gpiolib: add gpio_desc[] David Brownell
@ 2007-12-10  4:39 ` David Brownell
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 3/6] gpiolib: implementor-oriented documentation David Brownell
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:39 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list; +Cc: Tzachi Perelstein

From: David Brownell <dbrownell@users.sourceforge.net>

Update gpiolib behavior with CONFIG_DEBUG_GPIO to include messages
on some fault paths that are common during bringup:  gpiochip_add,
gpio_request, and the two gpio_direction_* calls.

Also morph that CONFIG symbol into compile-time -DDEBUG.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Tzachi Perelstein <tzachi@marvell.com>
---
 lib/Kconfig.debug |   10 ++++++----
 lib/Makefile      |    4 ++++
 lib/gpiolib.c     |   33 ++++++++++++++++++++++++++++-----
 3 files changed, 38 insertions(+), 9 deletions(-)

--- a/lib/Kconfig.debug	2007-12-09 19:50:50.000000000 -0800
+++ b/lib/Kconfig.debug	2007-12-09 19:51:02.000000000 -0800
@@ -160,10 +160,12 @@ config DEBUG_GPIO
 	bool "Debug GPIO calls"
 	depends on DEBUG_KERNEL && GPIO_LIB
 	help
-	  Say Y here to add some extra checks to GPIO calls, ensuring that
-	  GPIOs have been properly initialized before they are used and
-	  that sleeping calls aren't made from nonsleeping contexts.  This
-	  can make bitbanged serial protocols slower.
+	  Say Y here to add some extra checks and diagnostics to GPIO calls.
+	  The checks help ensure that GPIOs have been properly initialized
+	  before they are used and that sleeping calls aren't made from
+	  nonsleeping contexts.  They can make bitbanged serial protocols
+	  slower.  The diagnostics help catch the type of setup errors
+	  that are most common when setting up new platforms or boards.
 
 config DEBUG_SLAB_LEAK
 	bool "Memory leak debugging"
--- a/lib/Makefile	2007-12-09 19:50:50.000000000 -0800
+++ b/lib/Makefile	2007-12-09 19:51:02.000000000 -0800
@@ -68,6 +68,10 @@ obj-$(CONFIG_FAULT_INJECTION) += fault-i
 
 lib-$(CONFIG_GENERIC_BUG) += bug.o
 
+ifeq ($(CONFIG_DEBUG_GPIO),y)
+CFLAGS_gpiolib.o += -DDEBUG
+endif
+
 lib-$(CONFIG_GPIO_LIB) += gpiolib.o
 
 hostprogs-y	:= gen_crc32table
--- a/lib/gpiolib.c	2007-12-09 19:50:59.000000000 -0800
+++ b/lib/gpiolib.c	2007-12-09 19:51:02.000000000 -0800
@@ -20,9 +20,12 @@
 
 
 /* When debugging, extend minimal trust to callers and platform code.
+ * Also emit diagnostic messages that may help initial bringup, when
+ * board setup or driver bugs are most common.
+ *
  * Otherwise, minimize overhead in what may be bitbanging codepaths.
  */
-#ifdef	CONFIG_DEBUG_GPIO
+#ifdef	DEBUG
 #define	extra_checks	1
 #else
 #define	extra_checks	0
@@ -48,8 +51,11 @@ struct gpio_desc {
 static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
 
 
-/* Warn when drivers omit gpio_request() calls -- legal but
- * ill-advised when setting direction, and otherwise illegal.
+/* 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
+ * those calls have no teeth) we can't avoid autorequesting.  This nag
+ * message should motivate switching to explicit requests...
  */
 static void gpio_ensure_requested(struct gpio_desc *desc)
 {
@@ -86,8 +92,10 @@ int gpiochip_add(struct gpio_chip *chip)
 	 * dynamic allocation.  We don't currently support that.
 	 */
 
-	if (chip->base < 0 || (chip->base  + chip->ngpio) >= ARCH_NR_GPIOS)
-		return -EINVAL;
+	if (chip->base < 0 || (chip->base  + chip->ngpio) >= ARCH_NR_GPIOS) {
+		status = -EINVAL;
+		goto fail;
+	}
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
@@ -106,6 +114,12 @@ int gpiochip_add(struct gpio_chip *chip)
 	}
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
+fail:
+	/* failures here can mean systems won't boot... */
+	if (status)
+		pr_err("gpiochip_add: gpios %d..%d (%s) not registered\n",
+			chip->base, chip->base + chip->ngpio,
+			chip->label ? : "generic");
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add);
@@ -172,6 +186,9 @@ int gpio_request(unsigned gpio, const ch
 		status = -EBUSY;
 
 done:
+	if (status)
+		pr_debug("gpio_request: gpio-%d (%s) status %d\n",
+			gpio, label ? : "?", status);
 	spin_unlock_irqrestore(&gpio_lock, flags);
 	return status;
 }
@@ -272,6 +289,9 @@ int gpio_direction_input(unsigned gpio)
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
+	if (status)
+		pr_debug("%s: gpio-%d status %d\n",
+			__FUNCTION__, gpio, status);
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpio_direction_input);
@@ -307,6 +327,9 @@ int gpio_direction_output(unsigned gpio,
 	return status;
 fail:
 	spin_unlock_irqrestore(&gpio_lock, flags);
+	if (status)
+		pr_debug("%s: gpio-%d status %d\n",
+			__FUNCTION__, gpio, status);
 	return status;
 }
 EXPORT_SYMBOL_GPL(gpio_direction_output);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch 2.6.24-rc4-mm 3/6] gpiolib: implementor-oriented documentation
       [not found] <200712092022.14062.david-b@pacbell.net>
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 1/6] gpiolib: add gpio_desc[] David Brownell
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 2/6] gpiolib: more CONFIG_DEBUG_GPIO diagnostics David Brownell
@ 2007-12-10  4:39 ` David Brownell
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 4/6] gpiolib: create empty drivers/gpio David Brownell
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:39 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list

From: David Brownell <dbrownell@users.sourceforge.net>

Add some documentation highlighting implementors' views of the new gpiolib
stuff.  Such developers may be supporting new gpio controllers, platforms,
or boards.  Obviously there's often some overlap there, but the concerns
for each task aren't identical.

This also fixes a minor bug, which turned up as a discrepancy against
the documentation:  if a gpio_chip doesn't have a get() method, just
return zero when asked to read its value.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 Documentation/gpio.txt |  103 ++++++++++++++++++++++++++++++++++++++++++++++---
 lib/gpiolib.c          |   29 +++++++++++++
 2 files changed, 126 insertions(+), 6 deletions(-)

--- a/Documentation/gpio.txt	2007-12-09 19:50:50.000000000 -0800
+++ b/Documentation/gpio.txt	2007-12-09 19:51:03.000000000 -0800
@@ -32,7 +32,7 @@ The exact capabilities of GPIOs vary bet
   - Input values are likewise readable (1, 0).  Some chips support readback
     of pins configured as "output", which is very useful in such "wire-OR"
     cases (to support bidirectional signaling).  GPIO controllers may have
-    input de-glitch logic, sometimes with software controls.
+    input de-glitch/debounce logic, sometimes with software controls.
 
   - Inputs can often be used as IRQ signals, often edge triggered but
     sometimes level triggered.  Such IRQs may be configurable as system
@@ -60,12 +60,13 @@ used on a board that's wired differently
 functionality can be very portable.  Other features are platform-specific,
 and that can be critical for glue logic.
 
-Plus, this doesn't define an implementation framework, just an interface.
+Plus, this doesn't require any implementation framework, just an interface.
 One platform might implement it as simple inline functions accessing chip
 registers; another might implement it by delegating through abstractions
 used for several very different kinds of GPIO controller.  (There is some
-library code supporting such an implementation strategy, but drivers acting
-as clients to the GPIO interface should not care how it's implemented.)
+optional code supporting such an implementation strategy, described later
+in this document, but drivers acting as clients to the GPIO interface must
+not care how it's implemented.)
 
 That said, if the convention is supported on their platform, drivers should
 use it when possible.  Platforms should declare GENERIC_GPIO support in
@@ -152,7 +153,7 @@ Use these calls to access such GPIOs:
 The values are boolean, zero for low, nonzero for high.  When reading the
 value of an output pin, the value returned should be what's seen on the
 pin ... that won't always match the specified output value, because of
-issues including wire-OR and output latencies.
+issues including open-drain signaling and output latencies.
 
 The get/set calls have no error returns because "invalid GPIO" should have
 been reported earlier from gpio_direction_*().  However, note that not all
@@ -328,3 +329,95 @@ a side effect of configuring an add-on b
 
 These calls are purely for kernel space, but a userspace API could be built
 on top of them.
+
+
+GPIO implementor's framework (OPTIONAL)
+=======================================
+As noted earlier, there is an optional implementation framework making it
+easier for platforms to support different kinds of GPIO controller using
+the same programming interface.
+
+As a debugging aid, if debugfs is available a /sys/kernel/debug/gpio file
+will be found there.  That will list all the controllers registered through
+this framework, and the state of the GPIOs currently in use.
+
+
+Controller Drivers: gpio_chip
+-----------------------------
+In this framework each GPIO controller is packaged as a "struct gpio_chip"
+with information common to each controller of that type:
+
+ - methods to establish GPIO direction
+ - methods used to access GPIO values
+ - flag saying whether calls to its methods may sleep
+ - optional debugfs dump method (showing extra state like pullup config)
+ - label for diagnostics
+
+There is also per-instance data, which may come from device.platform_data:
+the number of its first GPIO, and how many GPIOs it exposes.
+
+The code implementing a gpio_chip should support multiple instances of the
+controller, possibly using the driver model.  That code will configure each
+gpio_chip and issue gpiochip_add().  Removing a GPIO controller should be
+rare; use gpiochip_remove() when it is unavoidable.
+
+Normally a gpio_chip is part of an instance-specific structure with state
+not exposed by the GPIO interfaces, such as addressing, power management,
+and more.  Chips such as codecs will have complex non-GPIO state,
+
+Any debugfs dump method should normally ignore signals which haven't been
+requested as GPIOs.  They can use gpiochip_is_requested(), which returns
+either NULL or the label associated with that GPIO when it was requested.
+
+
+Platform Support
+----------------
+To support this framework, a platform's Kconfig will "select GPIO_LIB" and
+arrange that its <asm/gpio.h> then include <asm-generic/gpio.h> and define
+three functions: gpio_get_value(), gpio_set_value(), and gpio_cansleep().
+They may also want to provide a custom value for ARCH_NR_GPIOS.
+
+Trivial implementations of those functions can directly use framework
+code, which always dispatches through the gpio_chip:
+
+  #define gpio_get_value	__gpio_get_value
+  #define gpio_set_value	__gpio_set_value
+  #define gpio_cansleep		__gpio_cansleep
+
+Fancier implementations could instead define those as inline functions with
+logic optimizing access to specific SOC-based GPIOs.  For example, if the
+referenced GPIO is the constant "12", getting or setting its value could
+cost as little as two or three instructions, never sleeping.  When such an
+optimization is not possible those calls must delegate to the framework
+code, costing probably a few dozen instructions.  For bitbanged I/O, such
+instruction savings can be significant.
+
+For SOCs, platform-specific code defines and registers gpio_chip instances
+for each bank of on-chip GPIOs.  Those GPIOs should be numbered/labeled to
+match chip vendor documentation, and directly match board schematics.  They
+probably start at zero and go up to a platform-specific limit.  Such GPIOs
+are normally integrated into platform initialization so these GPIOs are
+always available, from arch_initcall() or earlier, and can serve as IRQs.
+
+
+Board Support
+-------------
+For external GPIO controllers -- such as I2C or SPI expanders, ASICs, multi
+function devices, FPGAs or CPLDs -- board-specific code is responsible for
+registering controller devices and ensuring their drivers know what GPIO
+numbers to use with gpiochip_add().  Their numbers usually start right after
+platform-specific GPIOs.
+
+For example, their I2C or SPI board_info structures would pass platform_data
+identifying the range of GPIOs that chip will expose.  The board setup code
+uses those structs to assign each GPIO expander chip a set of GPIOs; then the
+chip's driver passes that data to gpiochip_add() from probe().
+
+Initialization order can be important.  For example, when a device relies on
+an I2C-based GPIO, its probe() routine should only be called after that GPIO
+becomes available.  That probably means the device should not be registered
+until calls for that GPIO can work.  One way to address such dependencies is
+for such gpio_chip controllers to provide setup() and teardown() callbacks to
+board specific code; those board specific callbacks would register devices
+once all the necessary resources are available.
+
--- a/lib/gpiolib.c	2007-12-09 19:51:02.000000000 -0800
+++ b/lib/gpiolib.c	2007-12-09 19:51:03.000000000 -0800
@@ -356,16 +356,35 @@ EXPORT_SYMBOL_GPL(gpio_direction_output)
  * REVISIT when debugging, consider adding some instrumentation to ensure
  * that the GPIO was actually requested.
  */
+
+/**
+ * __gpio_get_value() - return a gpio's value
+ * @gpio: gpio whose value will be returned
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_get_value().
+ * It returns the zero or nonzero value provided by the associated
+ * gpio_chip.get() method; or zero if no such method is provided.
+ */
 int __gpio_get_value(unsigned gpio)
 {
 	struct gpio_chip	*chip;
 
 	chip = gpio_to_chip(gpio);
 	WARN_ON(extra_checks && chip->can_sleep);
-	return chip->get(chip, gpio - chip->base);
+	return chip->get ? chip->get(chip, gpio - chip->base) : 0;
 }
 EXPORT_SYMBOL_GPL(__gpio_get_value);
 
+/**
+ * __gpio_set_value() - assign a gpio's value
+ * @gpio: gpio whose value will be assigned
+ * @value: value to assign
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_set_value().
+ * It invokes the associated gpio_chip.set() method.
+ */
 void __gpio_set_value(unsigned gpio, int value)
 {
 	struct gpio_chip	*chip;
@@ -376,6 +395,14 @@ void __gpio_set_value(unsigned gpio, int
 }
 EXPORT_SYMBOL_GPL(__gpio_set_value);
 
+/**
+ * __gpio_cansleep() - report whether gpio value access will sleep
+ * @gpio: gpio in question
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_cansleep().  It
+ * returns nonzero if access reading or writing the GPIO value can sleep.
+ */
 int __gpio_cansleep(unsigned gpio)
 {
 	struct gpio_chip	*chip;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch 2.6.24-rc4-mm 4/6] gpiolib:  create empty drivers/gpio
       [not found] <200712092022.14062.david-b@pacbell.net>
                   ` (2 preceding siblings ...)
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 3/6] gpiolib: implementor-oriented documentation David Brownell
@ 2007-12-10  4:39 ` David Brownell
  2007-12-10 16:16   ` Jean Delvare
  2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 5/6] gpiolib: pcf857x i2c expander driver David Brownell
  2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 6/6] gpiolib: replacement mcp23s08 driver David Brownell
  5 siblings, 1 reply; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:39 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list; +Cc: eric miao, Jean Delvare

From: David Brownell <dbrownell@users.sourceforge.net>

Add an empty drivers/gpio directory for gpiolib based GPIO expanders.
We already have three of them (two I2C, one SPI), and there are dozens
of similar chips that only exist for GPIO expansion.

This won't be the only place to hold such gpio_chip code.  Many external
chips add a few GPIOs as secondary functionality, and platform code
frequently needs to closely integrate GPIO and IRQ support.

This is placed *early* in the build/link sequence since it's common for
other drivers to depend on GPIOs to do their work, so they need to be
initialized early in the device_initcall() sequence.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Acked-by: Jean Delvare <khali@linux-fr.org>
Cc: Eric Miao <eric.miao@marvell.com>
---
 arch/arm/Kconfig      |    2 ++
 drivers/Kconfig       |    2 ++
 drivers/Makefile      |    1 +
 drivers/gpio/Kconfig  |   14 ++++++++++++++
 drivers/gpio/Makefile |    1 +
 5 files changed, 20 insertions(+)

--- a/arch/arm/Kconfig	2007-12-09 19:50:39.000000000 -0800
+++ b/arch/arm/Kconfig	2007-12-09 19:51:04.000000000 -0800
@@ -1034,6 +1034,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/gpio/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/power/Kconfig"
--- a/drivers/Kconfig	2007-12-09 19:50:39.000000000 -0800
+++ b/drivers/Kconfig	2007-12-09 19:51:04.000000000 -0800
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/gpio/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/power/Kconfig"
--- a/drivers/Makefile	2007-12-09 19:50:39.000000000 -0800
+++ b/drivers/Makefile	2007-12-09 19:51:04.000000000 -0800
@@ -5,6 +5,7 @@
 # Rewritten to use lists instead of if-statements.
 #
 
+obj-$(CONFIG_GPIO_LIB)		+= gpio/
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/gpio/Kconfig	2007-12-09 19:51:04.000000000 -0800
@@ -0,0 +1,14 @@
+#
+# platform-neutral GPIO support
+#
+
+menu "GPIO Expanders"
+	depends on GPIO_LIB
+
+# put expanders in the right section, in alphabetical order
+
+comment "I2C GPIO expanders:"
+
+comment "SPI GPIO expanders:"
+
+endmenu
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/gpio/Makefile	2007-12-09 19:51:04.000000000 -0800
@@ -0,0 +1 @@
+# gpio support: dedicated expander chips, etc

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch 2.6.24-rc4-mm 5/6] gpiolib: pcf857x i2c expander driver
       [not found] <200712092022.14062.david-b@pacbell.net>
                   ` (3 preceding siblings ...)
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 4/6] gpiolib: create empty drivers/gpio David Brownell
@ 2007-12-10  4:40 ` David Brownell
  2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 6/6] gpiolib: replacement mcp23s08 driver David Brownell
  5 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:40 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list; +Cc: eric miao, Jean Delvare

From: David Brownell <dbrownell@users.sourceforge.net>

This is a new-style I2C driver for most common 8 and 16 bit I2C based
"quasi-bidirectional" GPIO expanders:  pcf8574 or pcf8575, and several
compatible models (mostly faster, supporting I2C at up to 1 MHz).

The driver exposes the GPIO signals using the platform-neutral GPIO
programming interface, so they are easily accessed by other kernel code.
The lack of such a flexible kernel API has been a big factor in the
proliferation of board-specific drivers for these chips... stuff that
rarely makes it upstream since it's so ugly.  This driver will let such
boards use standard calls.

Since it's a new-style driver, these devices must be configured as
part of board-specific init.  That eliminates the need for error-prone
manual configuration of module parameters, and makes compatibility with
legacy drivers (pcf8574.c, pc8575.c) for these chips easier (there's
a clear either/or disjunction).

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Acked-by: Jean Delvare <khali@linux-fr.org>
---
 drivers/gpio/Kconfig        |   23 +++
 drivers/gpio/Makefile       |    2 
 drivers/gpio/pcf857x.c      |  330 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/pcf857x.h |   45 ++++++
 4 files changed, 400 insertions(+)

--- a/drivers/gpio/Kconfig	2007-12-09 19:51:04.000000000 -0800
+++ b/drivers/gpio/Kconfig	2007-12-09 19:51:05.000000000 -0800
@@ -9,6 +9,29 @@ menu "GPIO Expanders"
 
 comment "I2C GPIO expanders:"
 
+config GPIO_PCF857X
+	tristate "PCF857x, PCA857x, and PCA967x I2C GPIO expanders"
+	depends on I2C
+	help
+	  Say yes here to provide access to most "quasi-bidirectional" I2C
+	  GPIO expanders used for additional digital outputs or inputs.
+	  Most of these parts are from NXP, though TI is a second source for
+	  some of them.  Compatible models include:
+
+	  8 bits:   pcf8574, pcf8574a, pca8574, pca8574a,
+	            pca9670, pca9672, pca9674, pca9674a
+
+	  16 bits:  pcf8575, pcf8575c, pca8575,
+	            pca9671, pca9673, pca9675
+
+	  Your board setup code will need to declare the expanders in
+	  use, and assign numbers to the GPIOs they expose.  Those GPIOs
+	  can then be used from drivers and other kernel code, just like
+	  other GPIOs, but only accessible from task contexts.
+
+	  This driver provides an in-kernel interface to those GPIOs using
+	  platform-neutral GPIO calls.
+
 comment "SPI GPIO expanders:"
 
 endmenu
--- a/drivers/gpio/Makefile	2007-12-09 19:51:04.000000000 -0800
+++ b/drivers/gpio/Makefile	2007-12-09 19:51:05.000000000 -0800
@@ -1 +1,3 @@
 # gpio support: dedicated expander chips, etc
+
+obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/gpio/pcf857x.c	2007-12-09 19:51:05.000000000 -0800
@@ -0,0 +1,330 @@
+/*
+ * pcf857x - driver for pcf857x, pca857x, and pca967x I2C GPIO expanders
+ *
+ * Copyright (C) 2007 David Brownell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c/pcf857x.h>
+
+#include <asm/gpio.h>
+
+
+/*
+ * The pcf857x, pca857x, and pca967x chips only expose one read and one
+ * write register.  Writing a "one" bit (to match the reset state) lets
+ * that pin be used as an input; it's not an open-drain model, but acts
+ * a bit like one.  This is described as "quasi-bidirectional"; read the
+ * chip documentation for details.
+ *
+ * Many other I2C GPIO expander chips (like the pca953x models) have
+ * more complex register models and more conventional circuitry using
+ * push/pull drivers.  They often use the same 0x20..0x27 addresses as
+ * pcf857x parts, making the "legacy" I2C driver model problematic.
+ */
+struct pcf857x {
+	struct gpio_chip	chip;
+	struct i2c_client	*client;
+	unsigned		out;		/* software latch */
+};
+
+/*-------------------------------------------------------------------------*/
+
+/* Talk to 8-bit I/O expander */
+
+static int pcf857x_input8(struct gpio_chip *chip, unsigned offset)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+
+	gpio->out |= (1 << offset);
+	return i2c_smbus_write_byte(gpio->client, gpio->out);
+}
+
+static int pcf857x_get8(struct gpio_chip *chip, unsigned offset)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+	s32		value;
+
+	value = i2c_smbus_read_byte(gpio->client);
+	return (value < 0) ? 0 : (value & (1 << offset));
+}
+
+static int pcf857x_output8(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+	unsigned	bit = 1 << offset;
+
+	if (value)
+		gpio->out |= bit;
+	else
+		gpio->out &= ~bit;
+	return i2c_smbus_write_byte(gpio->client, gpio->out);
+}
+
+static void pcf857x_set8(struct gpio_chip *chip, unsigned offset, int value)
+{
+	pcf857x_output8(chip, offset, value);
+}
+
+/*-------------------------------------------------------------------------*/
+
+/* Talk to 16-bit I/O expander */
+
+static int i2c_write_le16(struct i2c_client *client, u16 word)
+{
+	u8 buf[2] = { word & 0xff, word >> 8, };
+	int status;
+
+	status = i2c_master_send(client, buf, 2);
+	return (status < 0) ? status : 0;
+}
+
+static int i2c_read_le16(struct i2c_client *client)
+{
+	u8 buf[2];
+	int status;
+
+	status = i2c_master_recv(client, buf, 2);
+	if (status < 0)
+		return status;
+	return (buf[1] << 8) | buf[0];
+}
+
+static int pcf857x_input16(struct gpio_chip *chip, unsigned offset)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+
+	gpio->out |= (1 << offset);
+	return i2c_write_le16(gpio->client, gpio->out);
+}
+
+static int pcf857x_get16(struct gpio_chip *chip, unsigned offset)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+	int		value;
+
+	value = i2c_read_le16(gpio->client);
+	return (value < 0) ? 0 : (value & (1 << offset));
+}
+
+static int pcf857x_output16(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct pcf857x	*gpio = container_of(chip, struct pcf857x, chip);
+	unsigned	bit = 1 << offset;
+
+	if (value)
+		gpio->out |= bit;
+	else
+		gpio->out &= ~bit;
+	return i2c_write_le16(gpio->client, gpio->out);
+}
+
+static void pcf857x_set16(struct gpio_chip *chip, unsigned offset, int value)
+{
+	pcf857x_output16(chip, offset, value);
+}
+
+/*-------------------------------------------------------------------------*/
+
+static int pcf857x_probe(struct i2c_client *client)
+{
+	struct pcf857x_platform_data	*pdata;
+	struct pcf857x			*gpio;
+	int				status;
+
+	pdata = client->dev.platform_data;
+	if (!pdata)
+		return -ENODEV;
+
+	/* Allocate, initialize, and register this gpio_chip. */
+	gpio = kzalloc(sizeof *gpio, GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->chip.base = pdata->gpio_base;
+	gpio->chip.can_sleep = 1;
+
+	/* NOTE:  the OnSemi jlc1562b is also largely compatible with
+	 * these parts, notably for output.  It has a low-resolution
+	 * DAC instead of pin change IRQs; and its inputs can be the
+	 * result of comparators.
+	 */
+
+	/* 8574 addresses are 0x20..0x27; 8574a uses 0x38..0x3f;
+	 * 9670, 9672, 9764, and 9764a use quite a variety.
+	 *
+	 * NOTE: we don't distinguish here between *4 and *4a parts.
+	 */
+	if (strcmp(client->name, "pcf8574") == 0
+			|| strcmp(client->name, "pca8574") == 0
+			|| strcmp(client->name, "pca9670") == 0
+			|| strcmp(client->name, "pca9672") == 0
+			|| strcmp(client->name, "pca9674") == 0
+			) {
+		gpio->chip.ngpio = 8;
+		gpio->chip.direction_input = pcf857x_input8;
+		gpio->chip.get = pcf857x_get8;
+		gpio->chip.direction_output = pcf857x_output8;
+		gpio->chip.set = pcf857x_set8;
+
+		if (!i2c_check_functionality(client->adapter,
+				I2C_FUNC_SMBUS_BYTE))
+			status = -EIO;
+
+		/* fail if there's no chip present */
+		else
+			status = i2c_smbus_read_byte(client);
+
+	/* '75/'75c addresses are 0x20..0x27, just like the '74;
+	 * the '75c doesn't have a current source pulling high.
+	 * 9671, 9673, and 9765 use quite a variety of addresses.
+	 *
+	 * NOTE: we don't distinguish here between '75 and '75c parts.
+	 */
+	} else if (strcmp(client->name, "pcf8575") == 0
+			|| strcmp(client->name, "pca8575") == 0
+			|| strcmp(client->name, "pca9671") == 0
+			|| strcmp(client->name, "pca9673") == 0
+			|| strcmp(client->name, "pca9675") == 0
+			) {
+		gpio->chip.ngpio = 16;
+		gpio->chip.direction_input = pcf857x_input16;
+		gpio->chip.get = pcf857x_get16;
+		gpio->chip.direction_output = pcf857x_output16;
+		gpio->chip.set = pcf857x_set16;
+
+		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+			status = -EIO;
+
+		/* fail if there's no chip present */
+		else
+			status = i2c_read_le16(client);
+
+	} else
+		status = -ENODEV;
+
+	if (status < 0)
+		goto fail;
+
+	gpio->chip.label = client->name;
+
+	gpio->client = client;
+	i2c_set_clientdata(client, gpio);
+
+	/* NOTE:  these chips have strange "quasi-bidirectional" I/O pins.
+	 * We can't actually know whether a pin is configured (a) as output
+	 * and driving the signal low, or (b) as input and reporting a low
+	 * value ... without knowing the last value written since the chip
+	 * came out of reset (if any).  We can't read the latched output.
+	 *
+	 * In short, the only reliable solution for setting up pin direction
+	 * is to do it explicitly.  The setup() method can do that, but it
+	 * may cause transient glitching since it can't know the last value
+	 * written (some pins may need to be driven low).
+	 *
+	 * Using pdata->n_latch avoids that trouble.  When left initialized
+	 * to zero, our software copy of the "latch" then matches the chip's
+	 * all-ones reset state.  Otherwise it flags pins to be driven low.
+	 */
+	gpio->out = ~pdata->n_latch;
+
+	status = gpiochip_add(&gpio->chip);
+	if (status < 0)
+		goto fail;
+
+	/* NOTE: these chips can issue "some pin-changed" IRQs, which we
+	 * don't yet even try to use.  Among other issues, the relevant
+	 * genirq state isn't available to modular drivers; and most irq
+	 * methods can't be called from sleeping contexts.
+	 */
+
+	dev_info(&client->dev, "gpios %d..%d on a %s%s\n",
+			gpio->chip.base,
+			gpio->chip.base + gpio->chip.ngpio - 1,
+			client->name,
+			client->irq ? " (irq ignored)" : "");
+
+	/* Let platform code set up the GPIOs and their users.
+	 * Now is the first time anyone could use them.
+	 */
+	if (pdata->setup) {
+		status = pdata->setup(client,
+				gpio->chip.base, gpio->chip.ngpio,
+				pdata->context);
+		if (status < 0)
+			dev_warn(&client->dev, "setup --> %d\n", status);
+	}
+
+	return 0;
+
+fail:
+	dev_dbg(&client->dev, "probe error %d for '%s'\n",
+			status, client->name);
+	kfree(gpio);
+	return status;
+}
+
+static int pcf857x_remove(struct i2c_client *client)
+{
+	struct pcf857x_platform_data	*pdata = client->dev.platform_data;
+	struct pcf857x			*gpio = i2c_get_clientdata(client);
+	int				status = 0;
+
+	if (pdata->teardown) {
+		status = pdata->teardown(client,
+				gpio->chip.base, gpio->chip.ngpio,
+				pdata->context);
+		if (status < 0) {
+			dev_err(&client->dev, "%s --> %d\n",
+					"teardown", status);
+			return status;
+		}
+	}
+
+	status = gpiochip_remove(&gpio->chip);
+	if (status == 0)
+		kfree(gpio);
+	else
+		dev_err(&client->dev, "%s --> %d\n", "remove", status);
+	return status;
+}
+
+static struct i2c_driver pcf857x_driver = {
+	.driver = {
+		.name	= "pcf857x",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= pcf857x_probe,
+	.remove	= pcf857x_remove,
+};
+
+static int __init pcf857x_init(void)
+{
+	return i2c_add_driver(&pcf857x_driver);
+}
+module_init(pcf857x_init);
+
+static void __exit pcf857x_exit(void)
+{
+	i2c_del_driver(&pcf857x_driver);
+}
+module_exit(pcf857x_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Brownell");
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/include/linux/i2c/pcf857x.h	2007-12-09 19:51:05.000000000 -0800
@@ -0,0 +1,45 @@
+#ifndef __LINUX_PCF857X_H
+#define __LINUX_PCF857X_H
+
+/**
+ * struct pcf857x_platform_data - data to set up pcf857x driver
+ * @gpio_base: number of the chip's first GPIO
+ * @n_latch: optional bit-inverse of initial register value; if
+ *	you leave this initialized to zero the driver will act
+ *	like the chip was just reset
+ * @setup: optional callback issued once the GPIOs are valid
+ * @teardown: optional callback issued before the GPIOs are invalidated
+ * @context: optional parameter passed to setup() and teardown()
+ *
+ * In addition to the I2C_BOARD_INFO() state appropriate to each chip,
+ * the i2c_board_info used with the pcf875x driver must provide the
+ * chip "type" ("pcf8574", "pcf8574a", "pcf8575", "pcf8575c") and its
+ * platform_data (pointer to one of these structures) with at least
+ * the gpio_base value initialized.
+ *
+ * The @setup callback may be used with the kind of board-specific glue
+ * which hands the (now-valid) GPIOs to other drivers, or which puts
+ * devices in their initial states using these GPIOs.
+ *
+ * These GPIO chips are only "quasi-bidirectional"; read the chip specs
+ * to understand the behavior.  They don't have separate registers to
+ * record which pins are used for input or output, record which output
+ * values are driven, or provide access to input values.  That must be
+ * inferred by reading the chip's value and knowing the last value written
+ * to it.  If you leave n_latch initialized to zero, that last written
+ * value is presumed to be all ones (as if the chip were just reset).
+ */
+struct pcf857x_platform_data {
+	unsigned	gpio_base;
+	unsigned	n_latch;
+
+	int		(*setup)(struct i2c_client *client,
+					int gpio, unsigned ngpio,
+					void *context);
+	int		(*teardown)(struct i2c_client *client,
+					int gpio, unsigned ngpio,
+					void *context);
+	void		*context;
+};
+
+#endif /* __LINUX_PCF857X_H */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch 2.6.24-rc4-mm 6/6] gpiolib: replacement mcp23s08 driver
       [not found] <200712092022.14062.david-b@pacbell.net>
                   ` (4 preceding siblings ...)
  2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 5/6] gpiolib: pcf857x i2c expander driver David Brownell
@ 2007-12-10  4:40 ` David Brownell
  5 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10  4:40 UTC (permalink / raw)
  To: Andrew Morton, Linux Kernel list

From: David Brownell <dbrownell@users.sourceforge.net>

Basic driver for 8-bit SPI based MCP23S08 GPIO expander, without support for
IRQs or the shared chipselect mechanism.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
Other than the new directory and Kconfig symbol, this is identical to the
code currently in MM except for:  (a) gpio_desc[] related changes, and
(b) now using module_init(), since this directory is initialzed earlier.

 drivers/gpio/Kconfig         |    7 
 drivers/gpio/Makefile        |    1 
 drivers/gpio/mcp23s08.c      |  357 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/mcp23s08.h |   24 ++
 4 files changed, 389 insertions(+)

--- a/drivers/gpio/Kconfig	2007-12-09 19:51:05.000000000 -0800
+++ b/drivers/gpio/Kconfig	2007-12-09 19:51:06.000000000 -0800
@@ -34,4 +34,11 @@ config GPIO_PCF857X
 
 comment "SPI GPIO expanders:"
 
+config GPIO_MCP23S08
+	tristate "Microchip MCP23S08 I/O expander"
+	depends on SPI_MASTER
+	help
+	  SPI driver for Microchip MCP23S08 I/O expander.  This provides
+	  a GPIO interface supporting inputs and outputs.
+
 endmenu
--- a/drivers/gpio/Makefile	2007-12-09 19:51:05.000000000 -0800
+++ b/drivers/gpio/Makefile	2007-12-09 19:51:06.000000000 -0800
@@ -1,3 +1,4 @@
 # gpio support: dedicated expander chips, etc
 
+obj-$(CONFIG_GPIO_MCP23S08)	+= mcp23s08.o
 obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/drivers/gpio/mcp23s08.c	2007-12-09 19:51:06.000000000 -0800
@@ -0,0 +1,357 @@
+/*
+ * mcp23s08.c - SPI gpio expander driver
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+
+#include <linux/spi/spi.h>
+#include <linux/spi/mcp23s08.h>
+
+#include <asm/gpio.h>
+
+
+/* Registers are all 8 bits wide.
+ *
+ * The mcp23s17 has twice as many bits, and can be configured to work
+ * with either 16 bit registers or with two adjacent 8 bit banks.
+ *
+ * Also, there are I2C versions of both chips.
+ */
+#define MCP_IODIR	0x00		/* init/reset:  all ones */
+#define MCP_IPOL	0x01
+#define MCP_GPINTEN	0x02
+#define MCP_DEFVAL	0x03
+#define MCP_INTCON	0x04
+#define MCP_IOCON	0x05
+#	define IOCON_SEQOP	(1 << 5)
+#	define IOCON_HAEN	(1 << 3)
+#	define IOCON_ODR	(1 << 2)
+#	define IOCON_INTPOL	(1 << 1)
+#define MCP_GPPU	0x06
+#define MCP_INTF	0x07
+#define MCP_INTCAP	0x08
+#define MCP_GPIO	0x09
+#define MCP_OLAT	0x0a
+
+struct mcp23s08 {
+	struct spi_device	*spi;
+	u8			addr;
+
+	/* lock protects the cached values */
+	struct mutex		lock;
+	u8			cache[11];
+
+	struct gpio_chip	chip;
+
+	struct work_struct	work;
+};
+
+static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
+{
+	u8	tx[2], rx[1];
+	int	status;
+
+	tx[0] = mcp->addr | 0x01;
+	tx[1] = reg;
+	status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, sizeof rx);
+	return (status < 0) ? status : rx[0];
+}
+
+static int mcp23s08_write(struct mcp23s08 *mcp, unsigned reg, u8 val)
+{
+	u8	tx[3];
+
+	tx[0] = mcp->addr;
+	tx[1] = reg;
+	tx[2] = val;
+	return spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
+}
+
+static int
+mcp23s08_read_regs(struct mcp23s08 *mcp, unsigned reg, u8 *vals, unsigned n)
+{
+	u8	tx[2];
+
+	if ((n + reg) > sizeof mcp->cache)
+		return -EINVAL;
+	tx[0] = mcp->addr | 0x01;
+	tx[1] = reg;
+	return spi_write_then_read(mcp->spi, tx, sizeof tx, vals, n);
+}
+
+/*----------------------------------------------------------------------*/
+
+static int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct mcp23s08	*mcp = container_of(chip, struct mcp23s08, chip);
+	int status;
+
+	mutex_lock(&mcp->lock);
+	mcp->cache[MCP_IODIR] |= (1 << offset);
+	status = mcp23s08_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+	mutex_unlock(&mcp->lock);
+	return status;
+}
+
+static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct mcp23s08	*mcp = container_of(chip, struct mcp23s08, chip);
+	int status;
+
+	mutex_lock(&mcp->lock);
+
+	/* REVISIT reading this clears any IRQ ... */
+	status = mcp23s08_read(mcp, MCP_GPIO);
+	if (status < 0)
+		status = 0;
+	else {
+		mcp->cache[MCP_GPIO] = status;
+		status = !!(status & (1 << offset));
+	}
+	mutex_unlock(&mcp->lock);
+	return status;
+}
+
+static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, int value)
+{
+	u8 olat = mcp->cache[MCP_OLAT];
+
+	if (value)
+		olat |= mask;
+	else
+		olat &= ~mask;
+	mcp->cache[MCP_OLAT] = olat;
+	return mcp23s08_write(mcp, MCP_OLAT, olat);
+}
+
+static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct mcp23s08	*mcp = container_of(chip, struct mcp23s08, chip);
+	u8 mask = 1 << offset;
+
+	mutex_lock(&mcp->lock);
+	__mcp23s08_set(mcp, mask, value);
+	mutex_unlock(&mcp->lock);
+}
+
+static int
+mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct mcp23s08	*mcp = container_of(chip, struct mcp23s08, chip);
+	u8 mask = 1 << offset;
+	int status;
+
+	mutex_lock(&mcp->lock);
+	status = __mcp23s08_set(mcp, mask, value);
+	if (status == 0) {
+		mcp->cache[MCP_IODIR] &= ~mask;
+		status = mcp23s08_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+	}
+	mutex_unlock(&mcp->lock);
+	return status;
+}
+
+/*----------------------------------------------------------------------*/
+
+#ifdef CONFIG_DEBUG_FS
+
+#include <linux/seq_file.h>
+
+/*
+ * This shows more info than the generic gpio dump code:
+ * pullups, deglitching, open drain drive.
+ */
+static void mcp23s08_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	struct mcp23s08	*mcp;
+	char		bank;
+	unsigned	t;
+	unsigned	mask;
+
+	mcp = container_of(chip, struct mcp23s08, chip);
+
+	/* NOTE: we only handle one bank for now ... */
+	bank = '0' + ((mcp->addr >> 1) & 0x3);
+
+	mutex_lock(&mcp->lock);
+	t = mcp23s08_read_regs(mcp, 0, mcp->cache, sizeof mcp->cache);
+	if (t < 0) {
+		seq_printf(s, " I/O ERROR %d\n", t);
+		goto done;
+	}
+
+	for (t = 0, mask = 1; t < 8; t++, mask <<= 1) {
+		const char	*label;
+
+		label = gpiochip_is_requested(chip, t);
+		if (!label)
+			continue;
+
+		seq_printf(s, " gpio-%-3d P%c.%d (%-12s) %s %s %s",
+			chip->base + t, bank, t, label,
+			(mcp->cache[MCP_IODIR] & mask) ? "in " : "out",
+			(mcp->cache[MCP_GPIO] & mask) ? "hi" : "lo",
+			(mcp->cache[MCP_GPPU] & mask) ? "  " : "up");
+		/* NOTE:  ignoring the irq-related registers */
+		seq_printf(s, "\n");
+	}
+done:
+	mutex_unlock(&mcp->lock);
+}
+
+#else
+#define mcp23s08_dbg_show	NULL
+#endif
+
+/*----------------------------------------------------------------------*/
+
+static int mcp23s08_probe(struct spi_device *spi)
+{
+	struct mcp23s08			*mcp;
+	struct mcp23s08_platform_data	*pdata;
+	int				status;
+	int				do_update = 0;
+
+	pdata = spi->dev.platform_data;
+	if (!pdata || pdata->slave > 3 || !pdata->base)
+		return -ENODEV;
+
+	mcp = kzalloc(sizeof *mcp, GFP_KERNEL);
+	if (!mcp)
+		return -ENOMEM;
+
+	mutex_init(&mcp->lock);
+
+	mcp->spi = spi;
+	mcp->addr = 0x40 | (pdata->slave << 1);
+
+	mcp->chip.label = "mcp23s08",
+
+	mcp->chip.direction_input = mcp23s08_direction_input;
+	mcp->chip.get = mcp23s08_get;
+	mcp->chip.direction_output = mcp23s08_direction_output;
+	mcp->chip.set = mcp23s08_set;
+	mcp->chip.dbg_show = mcp23s08_dbg_show;
+
+	mcp->chip.base = pdata->base;
+	mcp->chip.ngpio = 8;
+	mcp->chip.can_sleep = 1;
+
+	spi_set_drvdata(spi, mcp);
+
+	/* verify MCP_IOCON.SEQOP = 0, so sequential reads work */
+	status = mcp23s08_read(mcp, MCP_IOCON);
+	if (status < 0)
+		goto fail;
+	if (status & IOCON_SEQOP) {
+		status &= ~IOCON_SEQOP;
+		status = mcp23s08_write(mcp, MCP_IOCON, (u8) status);
+		if (status < 0)
+			goto fail;
+	}
+
+	/* configure ~100K pullups */
+	status = mcp23s08_write(mcp, MCP_GPPU, pdata->pullups);
+	if (status < 0)
+		goto fail;
+
+	status = mcp23s08_read_regs(mcp, 0, mcp->cache, sizeof mcp->cache);
+	if (status < 0)
+		goto fail;
+
+	/* disable inverter on input */
+	if (mcp->cache[MCP_IPOL] != 0) {
+		mcp->cache[MCP_IPOL] = 0;
+		do_update = 1;
+	}
+
+	/* disable irqs */
+	if (mcp->cache[MCP_GPINTEN] != 0) {
+		mcp->cache[MCP_GPINTEN] = 0;
+		do_update = 1;
+	}
+
+	if (do_update) {
+		u8 tx[4];
+
+		tx[0] = mcp->addr;
+		tx[1] = MCP_IPOL;
+		memcpy(&tx[2], &mcp->cache[MCP_IPOL], sizeof(tx) - 2);
+		status = spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
+
+		/* FIXME check status... */
+	}
+
+	status = gpiochip_add(&mcp->chip);
+
+	/* NOTE:  these chips have a relatively sane IRQ framework, with
+	 * per-signal masking and level/edge triggering.  It's not yet
+	 * handled here...
+	 */
+
+	if (pdata->setup) {
+		status = pdata->setup(spi, mcp->chip.base,
+				mcp->chip.ngpio, pdata->context);
+		if (status < 0)
+			dev_dbg(&spi->dev, "setup --> %d\n", status);
+	}
+
+	return 0;
+
+fail:
+	kfree(mcp);
+	return status;
+}
+
+static int mcp23s08_remove(struct spi_device *spi)
+{
+	struct mcp23s08			*mcp = spi_get_drvdata(spi);
+	struct mcp23s08_platform_data	*pdata = spi->dev.platform_data;
+	int				status = 0;
+
+	if (pdata->teardown) {
+		status = pdata->teardown(spi,
+				mcp->chip.base, mcp->chip.ngpio,
+				pdata->context);
+		if (status < 0) {
+			dev_err(&spi->dev, "%s --> %d\n", "teardown", status);
+			return status;
+		}
+	}
+
+	status = gpiochip_remove(&mcp->chip);
+	if (status == 0)
+		kfree(mcp);
+	else
+		dev_err(&spi->dev, "%s --> %d\n", "remove", status);
+	return status;
+}
+
+static struct spi_driver mcp23s08_driver = {
+	.probe		= mcp23s08_probe,
+	.remove		= mcp23s08_remove,
+	.driver = {
+		.name	= "mcp23s08",
+		.owner	= THIS_MODULE,
+	},
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __init mcp23s08_init(void)
+{
+	return spi_register_driver(&mcp23s08_driver);
+}
+module_init(mcp23s08_init);
+
+static void __exit mcp23s08_exit(void)
+{
+	spi_unregister_driver(&mcp23s08_driver);
+}
+module_exit(mcp23s08_exit);
+
+MODULE_LICENSE("GPL");
+
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ b/include/linux/spi/mcp23s08.h	2007-12-09 19:51:06.000000000 -0800
@@ -0,0 +1,24 @@
+
+/* FIXME driver should be able to handle all four slaves that
+ * can be hooked up to each chipselect, as well as IRQs...
+ */
+
+struct mcp23s08_platform_data {
+	/* four slaves can share one SPI chipselect */
+	u8		slave;
+
+	/* number assigned to the first GPIO */
+	unsigned	base;
+
+	/* pins with pullups */
+	u8		pullups;
+
+	void		*context;	/* param to setup/teardown */
+
+	int		(*setup)(struct spi_device *spi,
+					int gpio, unsigned ngpio,
+					void *context);
+	int		(*teardown)(struct spi_device *spi,
+					int gpio, unsigned ngpio,
+					void *context);
+};

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 2.6.24-rc4-mm 4/6] gpiolib:  create empty drivers/gpio
  2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 4/6] gpiolib: create empty drivers/gpio David Brownell
@ 2007-12-10 16:16   ` Jean Delvare
  2007-12-10 16:52     ` David Brownell
  0 siblings, 1 reply; 8+ messages in thread
From: Jean Delvare @ 2007-12-10 16:16 UTC (permalink / raw)
  To: David Brownell; +Cc: Andrew Morton, Linux Kernel list, eric miao

Hi David,

On Sun, 9 Dec 2007 20:39:57 -0800, David Brownell wrote:
> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> Add an empty drivers/gpio directory for gpiolib based GPIO expanders.
> We already have three of them (two I2C, one SPI), and there are dozens
> of similar chips that only exist for GPIO expansion.
> 
> This won't be the only place to hold such gpio_chip code.  Many external
> chips add a few GPIOs as secondary functionality, and platform code
> frequently needs to closely integrate GPIO and IRQ support.
> 
> This is placed *early* in the build/link sequence since it's common for
> other drivers to depend on GPIOs to do their work, so they need to be
> initialized early in the device_initcall() sequence.

Note that I foresee potential problems with this. These GPIO drivers
need I2C or SPI to be initialized before they can load. However on some
systems, I2C or SPI buses are hanging off GPIOs, meaning that these
GPIOS need to be initialized before I2C or SPI, respectively. I can
imagine weird setups where for example an SPI bus would be driven by an
I2C-based GPIO chip, or vice-versa. I sure hope that nobody does this,
but you never know... And I'm not sure how we would possibly support
this.

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 2.6.24-rc4-mm 4/6] gpiolib:  create empty drivers/gpio
  2007-12-10 16:16   ` Jean Delvare
@ 2007-12-10 16:52     ` David Brownell
  0 siblings, 0 replies; 8+ messages in thread
From: David Brownell @ 2007-12-10 16:52 UTC (permalink / raw)
  To: khali; +Cc: linux-kernel, eric.y.miao, akpm

> > Add an empty drivers/gpio directory for gpiolib based GPIO expanders.
> > We already have three of them (two I2C, one SPI), and there are dozens
> > of similar chips that only exist for GPIO expansion.
> > 
> > This won't be the only place to hold such gpio_chip code.  Many external
> > chips add a few GPIOs as secondary functionality, and platform code
> > frequently needs to closely integrate GPIO and IRQ support.
> > 
> > This is placed *early* in the build/link sequence since it's common for
> > other drivers to depend on GPIOs to do their work, so they need to be
> > initialized early in the device_initcall() sequence.
>
> Note that I foresee potential problems with this. These GPIO drivers
> need I2C or SPI to be initialized before they can load. However on some
> systems, I2C or SPI buses are hanging off GPIOs, meaning that these
> GPIOS need to be initialized before I2C or SPI, respectively.

In those cases the I2C (or SPI) bus will normally be hanging off
some arch/XX/mach-YY/gpio.c platform code, which will usually be
set up so it's "always there".  (As noted by the docs in patch 3/6
of this series.)  Such code often initializes very early, sometimes
even before IRQs are enabled or kmalloc works.

Bitbanging through an I2C or SPI expander would be possible, but
doesn't seem especially practical.  :)


>	I can
> imagine weird setups where for example an SPI bus would be driven by an
> I2C-based GPIO chip, or vice-versa. I sure hope that nobody does this,
> but you never know... And I'm not sure how we would possibly support
> this.

It's just a routine case of init this before that.  Again, the
dependency issue is noted in the docs in patch 3/6 of this series.
Linking these expanders early reduces such problems, it can never
get rid of them.

It's really no different than needing to hold off initialization
of any other device until the I2C based GPIOs are ready, like the
leds-gpio example [1] I posted some time ago.

- Dave

[1] http://marc.info/?l=linux-kernel&m=119370944606415&w=2
    See evm_led_setup(), setting up the leds-gpio device as a
    child of the GPIO expander; and evm_u18_setup(), setting
    up a switch attribute as a child of a different expander.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-12-10 16:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200712092022.14062.david-b@pacbell.net>
2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 1/6] gpiolib: add gpio_desc[] David Brownell
2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 2/6] gpiolib: more CONFIG_DEBUG_GPIO diagnostics David Brownell
2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 3/6] gpiolib: implementor-oriented documentation David Brownell
2007-12-10  4:39 ` [patch 2.6.24-rc4-mm 4/6] gpiolib: create empty drivers/gpio David Brownell
2007-12-10 16:16   ` Jean Delvare
2007-12-10 16:52     ` David Brownell
2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 5/6] gpiolib: pcf857x i2c expander driver David Brownell
2007-12-10  4:40 ` [patch 2.6.24-rc4-mm 6/6] gpiolib: replacement mcp23s08 driver David Brownell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).