[PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

Add a new get_direction() function to the gpio_chip structure.  This is
useful so that the direction of a pin can be determined when its
exported.  Previously, the direction defaulted to 'in' regardless of the
actual configuration of the GPIO pin which resulted in the 'direction'
sysfs file often being incorrect.

If a GPIO driver implements get_direction(), it is called in
gpio_request() to set the initial direction of the pin accurately.

Cc: Alek Du <alek.du@intel.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 drivers/gpio/gpiolib.c     |   15 ++++++++-------
 include/asm-generic/gpio.h |    4 ++++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 649550e..0a360d8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1062,13 +1062,6 @@ int gpiochip_add(struct gpio_chip *chip)
 	if (status == 0) {
 		for (id = base; id < base + chip->ngpio; id++) {
 			gpio_desc[id].chip = chip;
-
-			/* REVISIT:  most hardware initializes GPIOs as
-			 * inputs (often with pullups enabled) so power
-			 * usage is minimized.  Linux code should set the
-			 * gpio direction first thing; but until it does,
-			 * we may expose the wrong direction in sysfs.
-			 */
 			gpio_desc[id].flags = !chip->direction_input
 				? (1 << FLAG_IS_OUT)
 				: 0;
@@ -1215,6 +1208,14 @@ int gpio_request(unsigned gpio, const char *label)
 		}
 	}
 
+	if (chip->get_direction) {
+		/* chip->get_direction may sleep */
+		spin_unlock_irqrestore(&gpio_lock, flags);
+		if (chip->get_direction(chip, gpio - chip->base) > 0)
+			set_bit(FLAG_IS_OUT, &desc->flags);
+		spin_lock_irqsave(&gpio_lock, flags);
+	}
+
 done:
 	if (status)
 		pr_debug("gpio_request: gpio-%d (%s) status %d\n",
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index ff5c660..7d55cf7 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -57,6 +57,8 @@ struct device_node;
  * @direction_input: configures signal "offset" as input, or returns error
  * @get: returns value for signal "offset"; for output signals this
  *	returns either the value actually sensed, or zero
+ * @get_direction: optional hook to determine if a GPIO signal is configured
+ *	as an input (0) or output (1).
  * @direction_output: configures signal "offset" as output, or returns error
  * @set: assigns output value for signal "offset"
  * @to_irq: optional hook supporting non-static gpio_to_irq() mappings;
@@ -101,6 +103,8 @@ struct gpio_chip {
 						unsigned offset);
 	int			(*get)(struct gpio_chip *chip,
 						unsigned offset);
+	int			(*get_direction)(struct gpio_chip *chip,
+						unsigned offset);
 	int			(*direction_output)(struct gpio_chip *chip,
 						unsigned offset, int value);
 	int			(*set_debounce)(struct gpio_chip *chip,
-- 
1.7.0.4

[PATCH 2/3] gpio: pca953x: Implement get_direction() hook

[Peter Tyser]

Adding the get_direction() function allows the accurate GPIO pin
direction data to be shown in sysfs.  Previously, the direction was
was always initialized to 'input', even if the pin was configured as an
output.

Cc: Alek Du <alek.du@intel.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 drivers/gpio/pca953x.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index 5018666..d291028 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -159,6 +159,16 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 	return 0;
 }
 
+static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
+{
+	struct pca953x_chip *chip;
+
+	chip = container_of(gc, struct pca953x_chip, gpio_chip);
+
+	/* return 0 if IO pin is input, 1 if its an output */
+	return chip->reg_direction & (1u << off) ? 0 : 1;
+}
+
 static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 {
 	struct pca953x_chip *chip;
@@ -207,6 +217,7 @@ static void pca953x_setup_gpio(struct pca953x_chip *chip, int gpios)
 
 	gc->direction_input  = pca953x_gpio_direction_input;
 	gc->direction_output = pca953x_gpio_direction_output;
+	gc->get_direction = pca953x_gpio_get_direction;
 	gc->get = pca953x_gpio_get_value;
 	gc->set = pca953x_gpio_set_value;
 	gc->can_sleep = 1;
-- 
1.7.0.4

[PATCH 3/3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO

[Peter Tyser]

This driver works on many Intel chipsets, including the ICH6, ICH7,
ICH8, ICH9, ICH10, 3100, Series 5/3400 (Ibex Peak), Series 6/C200
(Cougar Point).

Additional Intel chipsets should be easily supported if needed, eg the
ICH1-5, EP80579, etc.

Tested on a QM57 (Ibex Peak) and 3100.

Cc: Alek Du <alek.du@intel.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: David Brownell <dbrownell@users.sourceforge.net>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
 MAINTAINERS              |    5 +
 drivers/gpio/Kconfig     |   11 +
 drivers/gpio/Makefile    |    1 +
 drivers/gpio/ichx_gpio.c |  551 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 568 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/ichx_gpio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 71e40f9..95df3b9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2946,6 +2946,11 @@ W:	http://www.developer.ibm.com/welcome/netfinity/serveraid.html
 S:	Supported
 F:	drivers/scsi/ips.*
 
+ICHX GPIO DRIVER
+M:	Peter Tyser <ptyser@xes-inc.com>
+S:	Maintained
+F:	drivers/gpio/ichx_gpio.c
+
 IDE SUBSYSTEM
 M:	"David S. Miller" <davem@davemloft.net>
 L:	linux-ide@vger.kernel.org
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3143ac7..aa5f540 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -319,6 +319,17 @@ config GPIO_BT8XX
 
 	  If unsure, say N.
 
+config GPIO_ICHX
+	tristate "Intel ICHx GPIO"
+	depends on PCI && GPIOLIB && X86
+	help
+	  Say yes here to support the GPIO functionality of a number of Intel
+	  ICH-based chipsets.  Currently supported devices: ICH6, ICH7, ICH8
+	  ICH9, ICH10, Series 5/3400 (ie Ibex Peak), Series 6/C200 (eg
+	  Cougar Point), and 3100.
+
+	  If unsure, say N.
+
 config GPIO_LANGWELL
 	bool "Intel Langwell/Penwell GPIO support"
 	depends on PCI
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index bdf3dde..d02cf70 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
 obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
+obj-$(CONFIG_GPIO_ICHX)		+= ichx_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/ichx_gpio.c b/drivers/gpio/ichx_gpio.c
new file mode 100644
index 0000000..63d27b5
--- /dev/null
+++ b/drivers/gpio/ichx_gpio.c
@@ -0,0 +1,551 @@
+/*
+ * Intel ICH6-10, Series 5 and 6 GPIO driver
+ *
+ * Copyright (C) 2010 Extreme Engineering Solutions.
+ *
+ * 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/module.h>
+#include <linux/pci.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "ichx_gpio"
+
+/* PCI config register offsets into LPC I/F - D31:F0 */
+#define PCI_ICHX_ACPI_BAR	0x40
+#define PCI_ICHX_GPIO_BAR	0x48
+#define PCI_ICHX_GPIO_CTRL	0x4C
+
+/*
+ * GPIO register offsets in GPIO I/O space.
+ * Each chunk of 32 GPIOs is manipulated via its own USE_SELx, IO_SELx, and
+ * LVLx registers.  Logic in the read/write functions takes a register and
+ * an absolute bit number and determines the proper register offset and bit
+ * number in that register.  For example, to read the value of GPIO bit 50
+ * the code would access offset ichx_regs[2(=GPIO_LVL)][1(=50/32)],
+ * bit 18 (50%32).
+ */
+enum GPIO_REG {
+	GPIO_USE_SEL = 0,
+	GPIO_IO_SEL,
+	GPIO_LVL,
+};
+static const u8 ichx_regs[3][3] = {
+	{0x00, 0x30, 0x40},	/* USE_SEL[1-3] offsets */
+	{0x04, 0x34, 0x44},	/* IO_SEL[1-3] offsets */
+	{0x0c, 0x38, 0x48},	/* LVL[1-3] offsets */
+};
+
+#define ICHX_GPIO_WRITE(val, reg)	outl(val, (reg) + ichx_priv.gpio_base)
+#define ICHX_GPIO_READ(reg)		inl((reg) + ichx_priv.gpio_base)
+#define ICHX_PM_WRITE(val, reg)		outl(val, (reg) + ichx_priv.pm_base)
+#define ICHX_PM_READ(reg)		inl((reg) + ichx_priv.pm_base)
+
+/* Convenient wrapper to make our PCI ID table */
+#define ICHX_GPIO_PCI_DEVICE(dev, data) \
+	.vendor = PCI_VENDOR_ID_INTEL,	\
+	.device = dev,			\
+	.subvendor = PCI_ANY_ID,	\
+	.subdevice = PCI_ANY_ID,	\
+	.class = 0,			\
+	.class_mask = 0,		\
+	.driver_data = (ulong)data
+
+struct ichx_desc {
+	/* Max GPIO pins the chipset can have */
+	uint ngpio;
+
+	/* The offset of GPE0_STS in the PM IO region, 0 if unneeded */
+	uint gpe0_sts_ofs;
+
+	/* USE_SEL is bogus on some chipsets, eg 3100 */
+	u32 use_sel_ignore[3];
+
+	/* Some chipsets have quirks, let these use their own request/get */
+	int (*request)(struct gpio_chip *chip, unsigned offset);
+	int (*get)(struct gpio_chip *chip, unsigned offset);
+};
+
+static struct {
+	spinlock_t lock;
+	struct platform_device *dev;
+	struct pci_dev *pdev;
+	struct gpio_chip chip;
+	unsigned long gpio_base;/* GPIO IO base */
+	unsigned long pm_base;	/* Power Mangagment IO base */
+	struct ichx_desc *desc;	/* Pointer to chipset-specific description */
+	u32 orig_gpio_ctrl;	/* Orig CTRL value, used to restore on exit */
+} ichx_priv;
+
+static struct platform_device *ichx_gpio_platform_device;
+
+static int modparam_gpiobase = -1 /* dynamic */;
+module_param_named(gpiobase, modparam_gpiobase, int, 0444);
+MODULE_PARM_DESC(gpiobase, "The GPIO number base. -1 means dynamic, "
+			   "which is the default.");
+
+static int ichx_write_bit(int reg, unsigned nr, int val, int verify)
+{
+	unsigned long flags;
+	u32 data;
+	int reg_nr = nr / 32;
+	int bit = nr & 0x1f;
+	int ret = 0;
+
+	spin_lock_irqsave(&ichx_priv.lock, flags);
+
+	data = ICHX_GPIO_READ(ichx_regs[reg][reg_nr]);
+	data = (data & ~(1 << bit)) | (val << bit);
+	ICHX_GPIO_WRITE(data, ichx_regs[reg][reg_nr]);
+	if (verify && (data != ICHX_GPIO_READ(ichx_regs[reg][reg_nr])))
+		ret = -EPERM;
+
+	spin_unlock_irqrestore(&ichx_priv.lock, flags);
+
+	return ret;
+}
+
+static int ichx_read_bit(int reg, unsigned nr)
+{
+	unsigned long flags;
+	u32 data;
+	int reg_nr = nr / 32;
+	int bit = nr & 0x1f;
+
+	spin_lock_irqsave(&ichx_priv.lock, flags);
+
+	data = ICHX_GPIO_READ(ichx_regs[reg][reg_nr]);
+
+	spin_unlock_irqrestore(&ichx_priv.lock, flags);
+
+	return data & (1 << bit) ? 1 : 0;
+}
+
+static int ichx_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
+{
+	/*
+	 * Try setting pin as an input and verify it worked since many pins
+	 * are output-only.
+	 */
+	if (ichx_write_bit(GPIO_IO_SEL, nr, 1, 1))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ichx_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
+					int val)
+{
+	/* Set GPIO output value. */
+	ichx_write_bit(GPIO_LVL, nr, val, 0);
+
+	/*
+	 * Try setting pin as an output and verify it worked since many pins
+	 * are input-only.
+	 */
+	if (ichx_write_bit(GPIO_IO_SEL, nr, 0, 1))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ichx_gpio_get_direction(struct gpio_chip *chip, unsigned nr)
+{
+	/* Return 0 if IO pin is input, 1 if its an output */
+	return !ichx_read_bit(GPIO_IO_SEL, nr);
+}
+
+static int ichx_gpio_get(struct gpio_chip *chip, unsigned nr)
+{
+	return ichx_read_bit(GPIO_LVL, nr);
+}
+
+static int ich6_gpio_get(struct gpio_chip *chip, unsigned nr)
+{
+	unsigned long flags;
+	u32 data;
+
+	/*
+	 * GPI 0 - 15 need to be read from the power management registers on
+	 * a ICH6/3100 bridge.
+	 */
+	if (nr < 16) {
+		spin_lock_irqsave(&ichx_priv.lock, flags);
+
+		/* GPI 0 - 15 are latched, write 1 to clear*/
+		ICHX_PM_WRITE(1 << (16 + nr), ichx_priv.desc->gpe0_sts_ofs);
+
+		data = ICHX_PM_READ(ichx_priv.desc->gpe0_sts_ofs);
+
+		spin_unlock_irqrestore(&ichx_priv.lock, flags);
+
+		return (data >> 16) & (1 << nr) ? 1 : 0;
+	} else {
+		return ichx_gpio_get(chip, nr);
+	}
+}
+
+static int ichx_gpio_request(struct gpio_chip *chip, unsigned nr)
+{
+	/*
+	 * Note we assume the BIOS properly set a bridge's USE value.  Some
+	 * chips (eg Intel 3100) have bogus USE values though, so first see if
+	 * the chipset's USE value can be trusted for this specific bit.
+	 * If it can't be trusted, assume that the pin can be used as a GPIO.
+	 */
+	if (ichx_priv.desc->use_sel_ignore[nr / 32] & (1 << (nr & 0x1f)))
+		return 1;
+
+	return ichx_read_bit(GPIO_USE_SEL, nr) ? 0 : -ENODEV;
+}
+
+static int ich6_gpio_request(struct gpio_chip *chip, unsigned nr)
+{
+	/*
+	 * Fixups for bits 16 and 17 are necessary on the Intel ICH6/3100
+	 * brdige as they are controlled by USE register bits 0 and 1.  See
+	 * "Table 704 GPIO_USE_SEL1 register" in the i3100 datasheet for
+	 * additional info.
+	 */
+	if ((nr == 16) || (nr == 17))
+		nr -= 16;
+
+	return ichx_gpio_request(chip, nr);
+}
+
+static void ichx_gpio_set(struct gpio_chip *chip, unsigned nr, int val)
+{
+	ichx_write_bit(GPIO_LVL, nr, val, 0);
+}
+
+static void __devinit ichx_gpiolib_setup(struct gpio_chip *chip)
+{
+	chip->owner = THIS_MODULE;
+	chip->label = DRV_NAME;
+
+	/* Allow chip-specific overrides of request()/get() */
+	chip->request = ichx_priv.desc->request ?
+		ichx_priv.desc->request : ichx_gpio_request;
+	chip->get = ichx_priv.desc->get ?
+		ichx_priv.desc->get : ichx_gpio_get;
+
+	chip->set = ichx_gpio_set;
+	chip->get_direction = ichx_gpio_get_direction;
+	chip->direction_input = ichx_gpio_direction_input;
+	chip->direction_output = ichx_gpio_direction_output;
+	chip->base = modparam_gpiobase;
+	chip->ngpio = ichx_priv.desc->ngpio;
+	chip->can_sleep = 0;
+	chip->dbg_show = NULL;
+}
+
+static int __devinit ichx_gpio_init(struct pci_dev *pdev,
+				const struct pci_device_id *ent,
+				struct platform_device *dev)
+{
+	int err;
+
+	ichx_priv.pdev = pdev;
+	ichx_priv.dev = dev;
+	ichx_priv.desc = (struct ichx_desc *)ent->driver_data;
+
+	/* Determine I/O address of GPIO registers */
+	pci_read_config_dword(pdev, PCI_ICHX_GPIO_BAR,
+			      (u32 *)&ichx_priv.gpio_base);
+	ichx_priv.gpio_base &= PCI_BASE_ADDRESS_IO_MASK;
+	if (!ichx_priv.gpio_base) {
+		printk(KERN_ERR "%s: GPIO BAR not enumerated\n", DRV_NAME);
+		return -ENODEV;
+	}
+
+	/*
+	 * If necessary, determine the I/O address of ACPI/power management
+	 * registers which are needed to read the the GPE0 register for GPI pins
+	 * 0 - 15 on some chipsets.
+	 */
+	if (ichx_priv.desc->gpe0_sts_ofs) {
+		pci_read_config_dword(pdev, PCI_ICHX_ACPI_BAR,
+				      (u32 *)&ichx_priv.pm_base);
+		ichx_priv.pm_base &= PCI_BASE_ADDRESS_IO_MASK;
+		if (!ichx_priv.pm_base)
+			printk(KERN_WARNING "%s: ACPI BAR not enumerated, "
+			       "GPI 0 - 15 unusable\n", DRV_NAME);
+	}
+
+	/* Enable GPIO */
+	pci_read_config_dword(pdev, PCI_ICHX_GPIO_CTRL,
+			&ichx_priv.orig_gpio_ctrl);
+	pci_write_config_dword(pdev, PCI_ICHX_GPIO_CTRL,
+			ichx_priv.orig_gpio_ctrl | 0x10);
+
+	/*
+	 * Reserve GPIO I/O region. The power management region is used by other
+	 * drivers thus shouldn't be reserved.  The GPIO I/O region is 64
+	 * bytes for older chipsets with <= 64 GPIO, 128 bytes for newer
+	 * chipsets with > 64 GPIO.
+	 */
+	if (!devm_request_region(&dev->dev, ichx_priv.gpio_base,
+			ichx_priv.desc->ngpio > 64 ? 128 : 64,
+			"ichxgpio")) {
+		printk(KERN_ERR "%s: Can't request iomem (0x%lx)\n",
+		       DRV_NAME, ichx_priv.gpio_base);
+		return -EBUSY;
+	}
+
+	ichx_gpiolib_setup(&ichx_priv.chip);
+
+	err = gpiochip_add(&ichx_priv.chip);
+	if (err) {
+		printk(KERN_ERR "%s: Failed to register GPIOs\n", DRV_NAME);
+		return err;
+	}
+
+	printk(KERN_INFO "%s: GPIO from %d to %d on %s\n", DRV_NAME,
+	       ichx_priv.chip.base,
+	       ichx_priv.chip.base + ichx_priv.chip.ngpio - 1,
+	       dev_name(&pdev->dev));
+
+	return 0;
+}
+
+/* ICH6-based, 631xesb-based */
+static struct ichx_desc ich6_desc = {
+	/* Bridges using the ICH6 controller need fixups for GPIO 0 - 17 */
+	.request = ich6_gpio_request,
+	.get = ich6_gpio_get,
+
+	/* GPIO 0-15 are read in the GPE0_STS PM register */
+	.gpe0_sts_ofs = 0x28,
+
+	.ngpio = 50,
+};
+
+/* Intel 3100 */
+static struct ichx_desc i3100_desc = {
+	/*
+	 * Bits 16,17, 20 of USE_SEL and bit 16 of USE_SEL2 always read 0 on
+	 * the Intel 3100.  See "Table 712. GPIO Summary Table" of 3100
+	 * Datasheet for more info.
+	 */
+	.use_sel_ignore = {0x00130000, 0x00010000, 0x0},
+
+	/* The 3100 needs fixups for GPIO 0 - 17 */
+	.request = ich6_gpio_request,
+	.get = ich6_gpio_get,
+
+	/* GPIO 0-15 are read in the GPE0_STS PM register */
+	.gpe0_sts_ofs = 0x28,
+
+	.ngpio = 50,
+};
+
+/* ICH7 and ICH8-based */
+static struct ichx_desc ich7_desc = {
+	.ngpio = 50,
+};
+
+/* ICH9-based */
+static struct ichx_desc ich9_desc = {
+	.ngpio = 61,
+};
+
+/* ICH10-based - Consumer/corporate versions have different amount of GPIO */
+static struct ichx_desc ich10_cons_desc = {
+	.ngpio = 51,
+};
+static struct ichx_desc ich10_corp_desc = {
+	.ngpio = 72,
+};
+
+/* Intel 5 series, 6 series, 3400 series, and C200 series */
+static struct ichx_desc intel5_desc = {
+	.ngpio = 76,
+};
+
+/*
+ * Note that we don't register a PCI driver since the PCI device has additional
+ * functionality that is used by other drivers, eg iTCO_wdt.  We use the table
+ * to probe for matching devices, then use a platform driver.
+ *
+ * Also note that some additional, old chipsets should in theory be supported
+ * by this driver (eg 82801XX chips), but they are left out due to the fact
+ * that they complicate the driver and there likely isn't much of a demand
+ * since a driver doesn't already exist for the 10+ years the chipsets have
+ * existed for.
+ */
+static DEFINE_PCI_DEVICE_TABLE(ichx_pci_tbl) = {
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH6_0, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH6_1, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH6_2, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH7_0, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH7_1, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH7_30, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH7_31, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH8_0, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH8_1, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH8_2, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH8_3, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH8_4, &ich7_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_1, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_2, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_4, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_5, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_7, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH9_8, &ich9_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH10_0, &ich10_corp_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH10_3, &ich10_corp_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH10_1, &ich10_cons_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ICH10_2, &ich10_cons_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(PCI_DEVICE_ID_INTEL_ESB2_0, &i3100_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2671, &ich6_desc) }, /* 631Xesb series */
+	{ ICHX_GPIO_PCI_DEVICE(0x2672, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2673, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2674, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2675, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2676, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2677, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2678, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x2679, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267a, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267b, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267c, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267d, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267e, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x267f, &ich6_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b00, &intel5_desc) }, /* 5 series */
+	{ ICHX_GPIO_PCI_DEVICE(0x3b01, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b02, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b03, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b04, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b05, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b06, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b07, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b08, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b09, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0a, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0b, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0c, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0d, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0e, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b0f, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c41, &intel5_desc) }, /* 6 Series */
+	{ ICHX_GPIO_PCI_DEVICE(0x1c42, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c43, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c44, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c45, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c46, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c47, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c48, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c49, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4a, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4b, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4c, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4d, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4e, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c4f, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c50, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c51, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c52, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c53, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c54, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c55, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c56, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c57, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c58, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c59, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5a, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5b, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5c, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5d, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5e, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x1c5f, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b12, &intel5_desc) }, /* 3400 series */
+	{ ICHX_GPIO_PCI_DEVICE(0x3b14, &intel5_desc) },
+	{ ICHX_GPIO_PCI_DEVICE(0x3b16, &intel5_desc) },
+	{ 0, },
+};
+MODULE_DEVICE_TABLE(pci, ichx_pci_tbl);
+
+static int __devinit ichx_gpio_probe(struct platform_device *dev)
+{
+	struct pci_dev *pdev = NULL;
+	const struct pci_device_id *ent;
+
+	spin_lock_init(&ichx_priv.lock);
+
+	for_each_pci_dev(pdev) {
+		ent = pci_match_id(ichx_pci_tbl, pdev);
+		if (ent)
+			return ichx_gpio_init(pdev, ent, dev);
+	}
+
+	return -ENODEV;
+}
+
+static int __devexit ichx_gpio_remove(struct platform_device *dev)
+{
+	int ret = gpiochip_remove(&ichx_priv.chip);
+
+	/* Restore original GPIO control register value */
+	pci_write_config_dword(ichx_priv.pdev, PCI_ICHX_GPIO_CTRL,
+			ichx_priv.orig_gpio_ctrl);
+
+	return ret;
+}
+
+static struct platform_driver ichx_gpio_driver = {
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= DRV_NAME,
+	},
+	.probe		= ichx_gpio_probe,
+	.remove		= __devexit_p(ichx_gpio_remove),
+};
+
+static int __devinit ichx_gpio_init_module(void)
+{
+	int err = platform_driver_register(&ichx_gpio_driver);
+	if (err)
+		return err;
+
+	ichx_gpio_platform_device =
+		platform_device_register_simple(DRV_NAME, -1, NULL, 0);
+
+	if (IS_ERR(ichx_gpio_platform_device)) {
+		err = PTR_ERR(ichx_gpio_platform_device);
+		goto unreg_platform_driver;
+	}
+
+	return 0;
+
+unreg_platform_driver:
+	platform_driver_unregister(&ichx_gpio_driver);
+	return err;
+}
+
+static void __devexit ichx_gpio_exit_module(void)
+{
+	platform_device_unregister(ichx_gpio_platform_device);
+	platform_driver_unregister(&ichx_gpio_driver);
+}
+
+module_init(ichx_gpio_init_module);
+module_exit(ichx_gpio_exit_module);
+
+MODULE_AUTHOR("Peter Tyser <ptyser@xes-inc.com>");
+MODULE_DESCRIPTION("Intel ICHx series bridge GPIO driver");
+MODULE_LICENSE("GPL");
-- 
1.7.0.4

Re: [PATCH 3/3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO

[David Brownell]

Interesting.  I had an ICH8 (plus) driver at
some point, but held back doing anything with it
because of the ACPI interactions needed on most
relevant hardware, for wakeups and other issues.
Regardless, I'm glad to see someone pick up on
the fact that these are just GPIOs, and that Linux
should be able to work with them

- Dave

Re: [PATCH 2/3] gpio: pca953x: Implement get_direction() hook

[David Brownell]

--- On Thu, 1/6/11, Peter Tyser <ptyser@xes-inc.com> wrote:

> Subject: [PATCH 2/3] gpio: pca953x: Implement get_direction() hook


Ack   This can affect initialization too ISTR.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

On Thu, 2011-01-06 at 13:54 -0600, Peter Tyser wrote:
> Add a new get_direction() function to the gpio_chip structure.  This is
> useful so that the direction of a pin can be determined when its
> exported.  Previously, the direction defaulted to 'in' regardless of the
> actual configuration of the GPIO pin which resulted in the 'direction'
> sysfs file often being incorrect.
> 
> If a GPIO driver implements get_direction(), it is called in
> gpio_request() to set the initial direction of the pin accurately.

I see Grant was just added as a GPIO maintainer, so added him on CC.

Anything gating getting these 3 patches being picked up?

Thanks,
Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Mon, Feb 14, 2011 at 8:48 AM, Peter Tyser <ptyser@xes-inc.com> wrote:
> On Thu, 2011-01-06 at 13:54 -0600, Peter Tyser wrote:
>> Add a new get_direction() function to the gpio_chip structure.  This is
>> useful so that the direction of a pin can be determined when its
>> exported.  Previously, the direction defaulted to 'in' regardless of the
>> actual configuration of the GPIO pin which resulted in the 'direction'
>> sysfs file often being incorrect.
>>
>> If a GPIO driver implements get_direction(), it is called in
>> gpio_request() to set the initial direction of the pin accurately.
>
> I see Grant was just added as a GPIO maintainer, so added him on CC.
>
> Anything gating getting these 3 patches being picked up?

I'll take a look at them later today.

g.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Alan Cox]

> > If a GPIO driver implements get_direction(), it is called in
> > gpio_request() to set the initial direction of the pin accurately.
> 
> I see Grant was just added as a GPIO maintainer, so added him on CC.
> 
> Anything gating getting these 3 patches being picked up?

We need four states for a gpio pin direction though. A pin can be

- input
- output
- unknown (hardware lacks get functionality and it has not been set by
  software yet)
- alt_func (pin is in use for some other purpose)

(and being able to set them alt_func was proposed a while ago and I think
wants revisiting judging by the number of platforms which use gpio, and
in their own arch code are privately handling alt_func stuff)

Alan

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Mon, Feb 14, 2011 at 10:08 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> > If a GPIO driver implements get_direction(), it is called in
>> > gpio_request() to set the initial direction of the pin accurately.
>>
>> I see Grant was just added as a GPIO maintainer, so added him on CC.
>>
>> Anything gating getting these 3 patches being picked up?
>
> We need four states for a gpio pin direction though. A pin can be
>
> - input
> - output

There are actually multiple output modes that a specific gpio
controller could implement, but the gpio api only has a boolean
understanding of output.  I don't know if it is really worthwhile to
try and encode all the possible configurations in this API.

> - unknown (hardware lacks get functionality and it has not been set by
>  software yet)
> - alt_func (pin is in use for some other purpose)

What is the use-case for alt_func?  From the point of view of a GPIO
driver, I don't think it cares if the pin has been dedicated to
something else.  It can twiddle all it wants, but if the pin is routed
to something else then it won't have any external effects (pin mux is
often a separate logic block from the gpio controller).  Also with
GPIOs, the engineers fiddling with them *really* needs to know what
the gpios are routed to.  It is highly unlikely to have any kind of
automatic configuration of gpios; ie. if it isn't wired as a gpio,
then don't go twiddling it.

>
> (and being able to set them alt_func was proposed a while ago and I think
> wants revisiting judging by the number of platforms which use gpio, and
> in their own arch code are privately handling alt_func stuff)

Fair enough; convince me on alt_func.  What is the use case that I'm missing?

g.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Mark Brown]

On Mon, Feb 14, 2011 at 10:26:18AM -0700, Grant Likely wrote:
> On Mon, Feb 14, 2011 at 10:08 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > - input
> > - output

> There are actually multiple output modes that a specific gpio
> controller could implement, but the gpio api only has a boolean
> understanding of output.  I don't know if it is really worthwhile to
> try and encode all the possible configurations in this API.

Ditto for inputs, and of course there's all the pad configuration stuff.

> > - alt_func (pin is in use for some other purpose)

> What is the use-case for alt_func?  From the point of view of a GPIO
> driver, I don't think it cares if the pin has been dedicated to
> something else.  It can twiddle all it wants, but if the pin is routed
> to something else then it won't have any external effects (pin mux is
> often a separate logic block from the gpio controller).  Also with

Not always entirely true, some of the controllers I've worked with have
input and output both as alternate functions among the others there so
selecting input or output mode will tend to force the mode but...

> GPIOs, the engineers fiddling with them *really* needs to know what
> the gpios are routed to.  It is highly unlikely to have any kind of
> automatic configuration of gpios; ie. if it isn't wired as a gpio,
> then don't go twiddling it.

...it doesn't make much difference for the reasons you mention.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

> > We need four states for a gpio pin direction though. A pin can be
> >
> > - input
> > - output
> 
> There are actually multiple output modes that a specific gpio
> controller could implement, but the gpio api only has a boolean
> understanding of output.  I don't know if it is really worthwhile to
> try and encode all the possible configurations in this API.
> 
> > - unknown (hardware lacks get functionality and it has not been set by
> >  software yet)

I'm not sure how we could handle unknown directions in a better way.  We
really should know the direction by this point for most (all?) GPIO
chips.  I'd think the proper fix would be to make sure we can detect a
direction for all chips - either by reading hardware bits or by having
the platform code let us know (eg pdata->n_latch in pcf857x.c).  If you
have a suggestion about how unknown pins should be used, I can look into
it and submit a follow up patch.

> > - alt_func (pin is in use for some other purpose)
> 
> What is the use-case for alt_func?  From the point of view of a GPIO
> driver, I don't think it cares if the pin has been dedicated to
> something else.  It can twiddle all it wants, but if the pin is routed
> to something else then it won't have any external effects (pin mux is
> often a separate logic block from the gpio controller).  Also with
> GPIOs, the engineers fiddling with them *really* needs to know what
> the gpios are routed to.  It is highly unlikely to have any kind of
> automatic configuration of gpios; ie. if it isn't wired as a gpio,
> then don't go twiddling it.

Additionally, for this case I thought the low level GPIO driver should
implement a request() function to prevent a non-GPIO pin from being used
in the first place.  Eg like chip_gpio_request() in cs5535-gpio.c, or
ichx_gpio_request() in patch 3 of this series.

> > (and being able to set them alt_func was proposed a while ago and I think
> > wants revisiting judging by the number of platforms which use gpio, and
> > in their own arch code are privately handling alt_func stuff)
> 
> Fair enough; convince me on alt_func.  What is the use case that I'm missing?

Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Mon, Feb 14, 2011 at 10:45 AM, Peter Tyser <ptyser@xes-inc.com> wrote:
>> > We need four states for a gpio pin direction though. A pin can be
>> >
>> > - input
>> > - output
>>
>> There are actually multiple output modes that a specific gpio
>> controller could implement, but the gpio api only has a boolean
>> understanding of output.  I don't know if it is really worthwhile to
>> try and encode all the possible configurations in this API.
>>
>> > - unknown (hardware lacks get functionality and it has not been set by
>> >  software yet)
>
> I'm not sure how we could handle unknown directions in a better way.  We
> really should know the direction by this point for most (all?) GPIO
> chips.  I'd think the proper fix would be to make sure we can detect a
> direction for all chips - either by reading hardware bits or by having
> the platform code let us know (eg pdata->n_latch in pcf857x.c).  If you
> have a suggestion about how unknown pins should be used, I can look into
> it and submit a follow up patch.

Does it really matter?  Sure it's *nice* to have the current status
information if the driver can easily get it, but it probably isn't
critical.  Any user of the pin should be putting the pin in the mode
it needs regardless of the initial state.

>
>> > - alt_func (pin is in use for some other purpose)
>>
>> What is the use-case for alt_func?  From the point of view of a GPIO
>> driver, I don't think it cares if the pin has been dedicated to
>> something else.  It can twiddle all it wants, but if the pin is routed
>> to something else then it won't have any external effects (pin mux is
>> often a separate logic block from the gpio controller).  Also with
>> GPIOs, the engineers fiddling with them *really* needs to know what
>> the gpios are routed to.  It is highly unlikely to have any kind of
>> automatic configuration of gpios; ie. if it isn't wired as a gpio,
>> then don't go twiddling it.
>
> Additionally, for this case I thought the low level GPIO driver should
> implement a request() function to prevent a non-GPIO pin from being used
> in the first place.  Eg like chip_gpio_request() in cs5535-gpio.c, or
> ichx_gpio_request() in patch 3 of this series.

Yes, *if* a driver has the knowledge that a pin is unusable, then it
should not allow the pin to be requested.

g.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

On Mon, 2011-02-14 at 11:04 -0700, Grant Likely wrote:
> On Mon, Feb 14, 2011 at 10:45 AM, Peter Tyser <ptyser@xes-inc.com> wrote:
> >> > We need four states for a gpio pin direction though. A pin can be
> >> >
> >> > - input
> >> > - output
> >>
> >> There are actually multiple output modes that a specific gpio
> >> controller could implement, but the gpio api only has a boolean
> >> understanding of output.  I don't know if it is really worthwhile to
> >> try and encode all the possible configurations in this API.
> >>
> >> > - unknown (hardware lacks get functionality and it has not been set by
> >> >  software yet)
> >
> > I'm not sure how we could handle unknown directions in a better way.  We
> > really should know the direction by this point for most (all?) GPIO
> > chips.  I'd think the proper fix would be to make sure we can detect a
> > direction for all chips - either by reading hardware bits or by having
> > the platform code let us know (eg pdata->n_latch in pcf857x.c).  If you
> > have a suggestion about how unknown pins should be used, I can look into
> > it and submit a follow up patch.
> 
> Does it really matter?  Sure it's *nice* to have the current status
> information if the driver can easily get it, but it probably isn't
> critical.  Any user of the pin should be putting the pin in the mode
> it needs regardless of the initial state.

I don't think it matters too much since I haven't encountered a driver
where you can't determine a pin direction yet.  But if it were
impossible to determine a direction, it would throw people off.  They'd
have to know that the pin direction couldn't be trusted initially, which
is a big leap (this is the bug I'm trying to fix with this patch).  Eg
if someone sees a pin direction as "input" via sysfs, what are the odds
that they'll run "echo input > direction" just to make sure...

Anyway, I don't think its a critical issue either since its a corner
case with an easy workaround.  If we did want to add support for
allowing a direction of "unknown", it could be in a follow up patch
since its technically a bigger issue than this patch.

Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Mon, Feb 14, 2011 at 09:02:02AM -0700, Grant Likely wrote:
> On Mon, Feb 14, 2011 at 8:48 AM, Peter Tyser <ptyser@xes-inc.com> wrote:
> > On Thu, 2011-01-06 at 13:54 -0600, Peter Tyser wrote:
> >> Add a new get_direction() function to the gpio_chip structure.  This is
> >> useful so that the direction of a pin can be determined when its
> >> exported.  Previously, the direction defaulted to 'in' regardless of the
> >> actual configuration of the GPIO pin which resulted in the 'direction'
> >> sysfs file often being incorrect.
> >>
> >> If a GPIO driver implements get_direction(), it is called in
> >> gpio_request() to set the initial direction of the pin accurately.
> >
> > I see Grant was just added as a GPIO maintainer, so added him on CC.
> >
> > Anything gating getting these 3 patches being picked up?
> 
> I'll take a look at them later today.

What are the other two patches?  I only see this one.  Can you repost?

g.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Alan Cox]

> What is the use-case for alt_func?  From the point of view of a GPIO
> driver, I don't think it cares if the pin has been dedicated to

Currently it doesn't.

However the moment it starts setting input and output itself on requests
then it does because it may kick the pin out of alt_func mode when you
merely want to request it so you know which pin to stick into alt_func
mode, or to find a mapping. The current situation is an "ignorance is
bliss" one, but making it smarter backfires. We have the same problem
with unknown state - if I have a set of pins some of whose state is known
at boot time then I can't now provide a get_direction interface because
of the lack of states. At the very least we need input/output/godknows
where the latter means the gpio_request code keeps its nose out.

	reconfigure_resource();
	see_if_we_own_it()

is simply the wrong order for a resource.


The second problem is that in many cases you need to call gpio_request to
know you have the pin yourself before you can set the direction. That
means forcing the direction in gpio_request is daft - you force an
undefined value that cannot yet safely be set in all cases.

At the moment the lack of alt_func also has some strange effects and you
end up with code like

foo_firmware_update()
{
	/* Reserve the line for alt_func use for the moment */
	gpio_request(GPIO_FOO, "blah");
	random_gpio_driver_specific_altfunc_foo();
	do stuff();
	random_gpio_driver_specific_altfunc_foo();
	gpio_free(GPIO_FOO);

	/* Now available again for its normal GPIO use */
}


and that means you can't generalise dynamic access to a shared GPIO pin
without extra hardcoded knowledge.

In the case of a fixed pin its easy as you can either a) not register it
or b) just gpio_request it at boot and whack it in arch code, but if you
want to go beyond that it gets interesting.

Alan

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

On Mon, 2011-02-14 at 12:14 -0700, Grant Likely wrote:
> On Mon, Feb 14, 2011 at 09:02:02AM -0700, Grant Likely wrote:
> > On Mon, Feb 14, 2011 at 8:48 AM, Peter Tyser <ptyser@xes-inc.com> wrote:
> > > On Thu, 2011-01-06 at 13:54 -0600, Peter Tyser wrote:
> > >> Add a new get_direction() function to the gpio_chip structure.  This is
> > >> useful so that the direction of a pin can be determined when its
> > >> exported.  Previously, the direction defaulted to 'in' regardless of the
> > >> actual configuration of the GPIO pin which resulted in the 'direction'
> > >> sysfs file often being incorrect.
> > >>
> > >> If a GPIO driver implements get_direction(), it is called in
> > >> gpio_request() to set the initial direction of the pin accurately.
> > >
> > > I see Grant was just added as a GPIO maintainer, so added him on CC.
> > >
> > > Anything gating getting these 3 patches being picked up?
> > 
> > I'll take a look at them later today.
> 
> What are the other two patches?  I only see this one.  Can you repost?

I just resent the 3 patches to you off-list.  They can also be found at:
https://patchwork.kernel.org/patch/460411/
https://patchwork.kernel.org/patch/460321/
https://patchwork.kernel.org/patch/460331/

Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

On Mon, 2011-02-14 at 19:35 +0000, Alan Cox wrote:
> > What is the use-case for alt_func?  From the point of view of a GPIO
> > driver, I don't think it cares if the pin has been dedicated to
> 
> Currently it doesn't.
> 
> However the moment it starts setting input and output itself on requests
> then it does because it may kick the pin out of alt_func mode when you
> merely want to request it so you know which pin to stick into alt_func
> mode, or to find a mapping. The current situation is an "ignorance is
> bliss" one, but making it smarter backfires. We have the same problem
> with unknown state - if I have a set of pins some of whose state is known
> at boot time then I can't now provide a get_direction interface because
> of the lack of states. At the very least we need input/output/godknows
> where the latter means the gpio_request code keeps its nose out.
> 
> 	reconfigure_resource();
> 	see_if_we_own_it()
> 
> is simply the wrong order for a resource.

It looks like most drivers that implement a chip-specific request()
function do check if the gpio pin is available for use prior to
reconfiguring it.  If a pin is reserved for an alt_func, the request
fails and the pin is not touched.  Seems like adding a well coded
chip-specific request() function to drivers where necessary would
resolve your concern about reconfiguring pins before checking ownership?

> The second problem is that in many cases you need to call gpio_request to
> know you have the pin yourself before you can set the direction. That
> means forcing the direction in gpio_request is daft - you force an
> undefined value that cannot yet safely be set in all cases.

My understanding is that gpio_request() doesn't generally force a
direction, it just sets up the pin for use, or checks to make sure the
pin could be used.  Most chip's don't have their own request() function
and thus don't touch hardware at all.  For those that do have their own
request() function, they don't appear to force a value in many cases.

Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
configured the GPIO pins appropriately, which Linux should inherit in
general.  Linux currently inherits GPIO states that were set in firmware
when a GPIO is requested, but it doesn't properly report those values
via sysfs - that's the only bug I'm trying to fix.

> At the moment the lack of alt_func also has some strange effects and you
> end up with code like
> 
> foo_firmware_update()
> {
> 	/* Reserve the line for alt_func use for the moment */
> 	gpio_request(GPIO_FOO, "blah");
> 	random_gpio_driver_specific_altfunc_foo();
> 	do stuff();
> 	random_gpio_driver_specific_altfunc_foo();
> 	gpio_free(GPIO_FOO);
> 
> 	/* Now available again for its normal GPIO use */
> }
> 
> 
> and that means you can't generalise dynamic access to a shared GPIO pin
> without extra hardcoded knowledge.

Are there many cases where people need to swap a pin from GPIO to alt
functionality, and back again in Linux?  I have never seen them used
like that; generally they are either wired up to an alt_func device (I2C
pins, serial pins, etc), or as a GPIO - not both dynamically.  If some
hardware does need to do that, isn't it very chipset/board specific?  I
guess I'm just not really grasping the big advantage of the alt_func
feature, or how it'd be implemented as common code.  It looks like there
was a thread about it back in 2009 that didn't go anywhere:
http://thread.gmane.org/gmane.linux.kernel/851818

The current GPIO implementation has worked well for my usage scenarios,
so you may have to be blunt with me as far as what your looking for:)

Thanks,
Peter 

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Alan Cox]

> fails and the pin is not touched.  Seems like adding a well coded
> chip-specific request() function to drivers where necessary would
> resolve your concern about reconfiguring pins before checking ownership?

That seems sensible and it keeps the knowledge out of gpiolib.

> Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
> configured the GPIO pins appropriately, which Linux should inherit in
> general.  Linux currently inherits GPIO states that were set in firmware
> when a GPIO is requested, but it doesn't properly report those values
> via sysfs - that's the only bug I'm trying to fix.

Yes - however you can't fix it unless you are prepared to admit that the
gpio has multiple states. At minimum you need to be able to report

	input/output/unknown/unavailable

if you want to generalise it. Otherwise you don't solve the problem
because you are asking a question that driver cannot answer correctly.

> Are there many cases where people need to swap a pin from GPIO to alt
> functionality, and back again in Linux?  I have never seen them used
> like that

Well I'm not surprised - you can't currently do it that way. The code
doesn't support it, instead such things are buried in arch code and
driver specific goo. I question whether it should be buried away like
that.

That's really about Grant's why do we need it comment rather than about
your bits. From the point of view of direction reporting (which I'm fully
in favour of) the only problem that matters is the fact a pin has a
minimum of four states to report. From that point of view  "unavailable"
is probably a better way to report it than alt_func as it covers all the
other "unavailable" cases that may exist and haven't yet been thought of.

Alan

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

> > Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
> > configured the GPIO pins appropriately, which Linux should inherit in
> > general.  Linux currently inherits GPIO states that were set in firmware
> > when a GPIO is requested, but it doesn't properly report those values
> > via sysfs - that's the only bug I'm trying to fix.
> 
> Yes - however you can't fix it unless you are prepared to admit that the
> gpio has multiple states. At minimum you need to be able to report
> 
> 	input/output/unknown/unavailable
> 
> if you want to generalise it. Otherwise you don't solve the problem
> because you are asking a question that driver cannot answer correctly.

As far as the "unknown" state, I can update the patch to have the logic:
+       if (chip->get_direction) {
+               /* chip->get_direction may sleep */
+               spin_unlock_irqrestore(&gpio_lock, flags);
+               if (chip->get_direction(chip, gpio - chip->base) > 0)
+                       set_bit(FLAG_IS_OUT, &desc->flags);
+               spin_lock_irqsave(&gpio_lock, flags);
+       } else {
+               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
+       }

This would have the side effect of having nearly all GPIO drivers
default to an "unknown" direction until they implement the new
get_direction() function, which I think is an improvement over the
current system where they are all unconditionally shown as "input",
often incorrectly.  Are you OK with this Grant?


For the "unavailable" state, I didn't think it would be necessary.  As
is, if someone calls gpio_request() on an invalid or alt_use pin, they
shouldn't get access to the GPIO, which makes the "unavailable value
moot since they couldn't access the GPIO in the first place.

Changing the logic to allow "unavailable" GPIO pins to be
requested/exported would require larger changes to the code, and
wouldn't provide much benefit without additional changes (eg an alt_func
feature).  So I'd vote to not add support for "unavailable" pins in this
patch and rather wait until someone has a specific use for it, and add
it then.

Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Alan Cox]

> +       if (chip->get_direction) {
> +               /* chip->get_direction may sleep */
> +               spin_unlock_irqrestore(&gpio_lock, flags);
> +               if (chip->get_direction(chip, gpio - chip->base) > 0)
> +                       set_bit(FLAG_IS_OUT, &desc->flags);
> +               spin_lock_irqsave(&gpio_lock, flags);
> +       } else {
> +               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
> +       }
> 
> This would have the side effect of having nearly all GPIO drivers
> default to an "unknown" direction until they implement the new
> get_direction() function, which I think is an improvement over the

This doesn't solve anything. If the hardware supports alt_func state then
it now can't implement get_direction, so that's useless.
 
> For the "unavailable" state, I didn't think it would be necessary.  As
> is, if someone calls gpio_request() on an invalid or alt_use pin, they
> shouldn't get access to the GPIO, which makes the "unavailable value
> moot since they couldn't access the GPIO in the first place.

In a word 'sysfs'

We need FLAG_IS_UNKNOWN (or saner would be FLAG_IS_IN to go with
FLAG_IS_OUT) to make the sysfs code report properly (and some other spots
fixing to make it work right)

If you add FLAG_IS_UNKNOWN then the other change you need is in

gpio_direction_show() which needs to also check the UNKNOWN bit and
report appropriately. That would fix that problem and at least allow the
reporting side of GPIO in use for something else to be handled as a
platform thing even though it can't be handled properly.

The combination of RESERVED & EXPORTED is also busted but is actually
sensible - that I gues should report reserved and fail the set operations.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Peter Tyser]

On Tue, 2011-02-15 at 17:19 +0000, Alan Cox wrote:
> > +       if (chip->get_direction) {
> > +               /* chip->get_direction may sleep */
> > +               spin_unlock_irqrestore(&gpio_lock, flags);
> > +               if (chip->get_direction(chip, gpio - chip->base) > 0)
> > +                       set_bit(FLAG_IS_OUT, &desc->flags);
> > +               spin_lock_irqsave(&gpio_lock, flags);
> > +       } else {
> > +               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
> > +       }
> > 
> > This would have the side effect of having nearly all GPIO drivers
> > default to an "unknown" direction until they implement the new
> > get_direction() function, which I think is an improvement over the
> 
> This doesn't solve anything. If the hardware supports alt_func state then
> it now can't implement get_direction, so that's useless.

I don't follow.  If a pin is configured for some alternate function,
then requesting it for GPIO should fail, thus it doesn't matter if it
implements get_direction()?  Since we can't easily toggle back and forth
between GPIO and alt_func, I'd think we shouldn't be able to request
alt_func pins for GPIO - they should be off-limits to the GPIO subsystem
altogether.

My understanding is that currently if some platform wants to toggle pins
back and forth between alt_func and GPIO, it needs to handle that logic
itself.  If platform code is handling that toggling, I'd think the GPIO
code should not touch pins configured as alt_func.  If the platform is
no longer using them as alt_func, then it should poke the appropriate
registers to make them not alt_func so that they can then be used by the
GPIO subsystem.

Maybe we disagree on the above point, which is adding to the confusion?

> > For the "unavailable" state, I didn't think it would be necessary.  As
> > is, if someone calls gpio_request() on an invalid or alt_use pin, they
> > shouldn't get access to the GPIO, which makes the "unavailable value
> > moot since they couldn't access the GPIO in the first place.
> 
> In a word 'sysfs'
> 
> We need FLAG_IS_UNKNOWN (or saner would be FLAG_IS_IN to go with
> FLAG_IS_OUT) to make the sysfs code report properly (and some other spots
> fixing to make it work right)

Agreed.

> If you add FLAG_IS_UNKNOWN then the other change you need is in
> 
> gpio_direction_show() which needs to also check the UNKNOWN bit and
> report appropriately.

Agreed.

> That would fix that problem and at least allow the
> reporting side of GPIO in use for something else to be handled as a
> platform thing even though it can't be handled properly.

I don't follow.  I don't think I'm grasping what you want for alt_func
pins in the short term.  Do you want them to be exported to the GPIO
sysfs filesystem and shown as "unavailable"?  If so, what advantage does
that have over not allowing them to be exported/reserved in the first
place?

Peter

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Alan Cox]

> My understanding is that currently if some platform wants to toggle pins
> back and forth between alt_func and GPIO, it needs to handle that logic
> itself.  If platform code is handling that toggling, I'd think the GPIO

Yes - which is broken as you can't abstract it to the drivers which is
the whole point of GPIO. Anyway put that bit aside - for the get method
it's not actually important since we need an unknown state anyway and
alt_func is unknown or similar.

> > That would fix that problem and at least allow the
> > reporting side of GPIO in use for something else to be handled as a
> > platform thing even though it can't be handled properly.
> 
> I don't follow.  I don't think I'm grasping what you want for alt_func
> pins in the short term.  Do you want them to be exported to the GPIO
> sysfs filesystem and shown as "unavailable"?  If so, what advantage does
> that have over not allowing them to be exported/reserved in the first
> place?

You can see what state they are in. Otherwise you have to clone the GPIO
sysfs to expose private platform specific magic to indicate that.

Anyway first step is to allow 'Unknown' to be reported by a get_direction
method. The direction of a pin in some magic platform owned state is
defacto 'unknown'.

Not having a get_direction method doesn't help as we don't support
changing the methods on the fly (which is horrid for locking and best
kept that way) so we need a way to return input/output/beats me

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Mark Brown]

On Mon, Feb 14, 2011 at 05:35:02PM -0600, Peter Tyser wrote:

> Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
> configured the GPIO pins appropriately, which Linux should inherit in
> general.  Linux currently inherits GPIO states that were set in firmware
> when a GPIO is requested, but it doesn't properly report those values
> via sysfs - that's the only bug I'm trying to fix.

That's one model for these things but it's *far* from universal.
Another model which is at least as common is that the bootloader does
the minimum possible to transfer control to Linux which then does the
actual configuration for the system.

> Are there many cases where people need to swap a pin from GPIO to alt
> functionality, and back again in Linux?  I have never seen them used
> like that; generally they are either wired up to an alt_func device (I2C
> pins, serial pins, etc), or as a GPIO - not both dynamically.  If some
> hardware does need to do that, isn't it very chipset/board specific?  I

No, it's very rare.  One example we have in the kernel is the PXA27x
AC'97 driver - the controller doesn't generate one of the resets
correctly so the driver puts the signals into GPIO mode and manually
generates the reset on the bus for the CODEC when it needs to do so.

> guess I'm just not really grasping the big advantage of the alt_func
> feature, or how it'd be implemented as common code.  It looks like there
> was a thread about it back in 2009 that didn't go anywhere:
> http://thread.gmane.org/gmane.linux.kernel/851818

I tend to agree; I strongly expect that any alternate function code
would just wind up feeding at best device specific if not system
specific constants through and give us no more generality than we have
now.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Uwe Kleine-König]

On Tue, Feb 15, 2011 at 11:49:11AM -0600, Peter Tyser wrote:
> On Tue, 2011-02-15 at 17:19 +0000, Alan Cox wrote:
> > > +       if (chip->get_direction) {
> > > +               /* chip->get_direction may sleep */
> > > +               spin_unlock_irqrestore(&gpio_lock, flags);
> > > +               if (chip->get_direction(chip, gpio - chip->base) > 0)
> > > +                       set_bit(FLAG_IS_OUT, &desc->flags);
> > > +               spin_lock_irqsave(&gpio_lock, flags);
> > > +       } else {
> > > +               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
> > > +       }
> > > 
> > > This would have the side effect of having nearly all GPIO drivers
> > > default to an "unknown" direction until they implement the new
> > > get_direction() function, which I think is an improvement over the
> > 
> > This doesn't solve anything. If the hardware supports alt_func state then
> > it now can't implement get_direction, so that's useless.
> 
> I don't follow.  If a pin is configured for some alternate function,
> then requesting it for GPIO should fail, thus it doesn't matter if it
> implements get_direction()?  Since we can't easily toggle back and forth
> between GPIO and alt_func, I'd think we shouldn't be able to request
> alt_func pins for GPIO - they should be off-limits to the GPIO subsystem
> altogether.
hmm, I'm not sure.  Letting gpio_request fail looks good from the POV of
an uninformed driver.  But for some platform code, it seems more natural
to do:

	gpio_request(mygpio);
	myplatform_iomux_setup(pad_for_altfunc);
	do_something_special();
	/*
	 * the controler is unable to reset some component, so use
	 * bitbanging for that
	 */
	myplatform_iomux_setup(pad_for_gpio);
	gpio_direction_output(mygpio, 0);
	usleep(100);
	myplatform_iomux_setup(pad_for_altfunc);
	...

instead of only being able to gpio_request after
myplatform_iomux_setup(pad_for_gpio). (And so in theory take that risk
that another process grabs the gpio between mux-for-gpio and
gpio_request.) So if you ask me, it's gpio_direction_{in,out}put that
should fail, not gpio_request.  But I'm not that sure about it to
already know now to keep this opinion until after this discussion is
over.

> My understanding is that currently if some platform wants to toggle pins
> back and forth between alt_func and GPIO, it needs to handle that logic
> itself.  If platform code is handling that toggling, I'd think the GPIO
> code should not touch pins configured as alt_func.  If the platform is
> no longer using them as alt_func, then it should poke the appropriate
> registers to make them not alt_func so that they can then be used by the
> GPIO subsystem.
.. or at least make the usage via the gpio subsystem fail using it.
OTOH on arm/plat-mxc (at least the newer chips) there is no easy mapping
between pads and gpios.  So currently we do:  gpio_request and
gpio_direction_{in,out}put yield 0, but it depends on the pin muxing if
the gpio is "visible" anywhere.  I don't like that much, but I agree
that it's not worth to setup a huge table to map gpios to pads and back
just to return -ESOMETHING in the gpio functions.
 
Best regards
Uwe

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

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Mon, Feb 14, 2011 at 07:35:02PM +0000, Alan Cox wrote:
> > What is the use-case for alt_func?  From the point of view of a GPIO
> > driver, I don't think it cares if the pin has been dedicated to
> 
> Currently it doesn't.
> 
> However the moment it starts setting input and output itself on requests
> then it does because it may kick the pin out of alt_func mode when you
> merely want to request it so you know which pin to stick into alt_func
> mode, or to find a mapping. The current situation is an "ignorance is
> bliss" one, but making it smarter backfires. We have the same problem
> with unknown state - if I have a set of pins some of whose state is known
> at boot time then I can't now provide a get_direction interface because
> of the lack of states. At the very least we need input/output/godknows
> where the latter means the gpio_request code keeps its nose out.

Not quite; the gpio api is only about discrete gpios.  If a particular
pin is dedicated to another non-gpio purpose, then from the POV of the
gpio api, the pin is disconnected from the outside world and any
twiddling of it just won't do anything.  If an alt_func has any driver
behaviour impact, then it needs to be handled internal to the driver.

> 
> 	reconfigure_resource();
> 	see_if_we_own_it()
> 
> is simply the wrong order for a resource.

Yes, this is broken.  gpio_request() should not change the state of
the resource.  I don't see anything in gpiolib that currently does
this.

> The second problem is that in many cases you need to call gpio_request to
> know you have the pin yourself before you can set the direction. That
> means forcing the direction in gpio_request is daft - you force an
> undefined value that cannot yet safely be set in all cases.
> 
> At the moment the lack of alt_func also has some strange effects and you
> end up with code like
> 
> foo_firmware_update()
> {
> 	/* Reserve the line for alt_func use for the moment */
> 	gpio_request(GPIO_FOO, "blah");
> 	random_gpio_driver_specific_altfunc_foo();
> 	do stuff();
> 	random_gpio_driver_specific_altfunc_foo();
> 	gpio_free(GPIO_FOO);
> 
> 	/* Now available again for its normal GPIO use */
> }
> 
> 
> and that means you can't generalise dynamic access to a shared GPIO pin
> without extra hardcoded knowledge.

I don't follow the argument.  Of course you have to do weird hardcoded
things when a gpio pin has to be shared between multiple purposes, but
that is also a weird corner case that won't fit any kind of common
pattern.  I don't see the above code snippet as a problem.

g.

Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Grant Likely]

On Tue, Feb 15, 2011 at 11:05:48AM -0600, Peter Tyser wrote:
> > > Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
> > > configured the GPIO pins appropriately, which Linux should inherit in
> > > general.  Linux currently inherits GPIO states that were set in firmware
> > > when a GPIO is requested, but it doesn't properly report those values
> > > via sysfs - that's the only bug I'm trying to fix.
> > 
> > Yes - however you can't fix it unless you are prepared to admit that the
> > gpio has multiple states. At minimum you need to be able to report
> > 
> > 	input/output/unknown/unavailable
> > 
> > if you want to generalise it. Otherwise you don't solve the problem
> > because you are asking a question that driver cannot answer correctly.
> 
> As far as the "unknown" state, I can update the patch to have the logic:
> +       if (chip->get_direction) {
> +               /* chip->get_direction may sleep */
> +               spin_unlock_irqrestore(&gpio_lock, flags);
> +               if (chip->get_direction(chip, gpio - chip->base) > 0)
> +                       set_bit(FLAG_IS_OUT, &desc->flags);
> +               spin_lock_irqsave(&gpio_lock, flags);
> +       } else {
> +               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
> +       }
> 
> This would have the side effect of having nearly all GPIO drivers
> default to an "unknown" direction until they implement the new
> get_direction() function, which I think is an improvement over the
> current system where they are all unconditionally shown as "input",
> often incorrectly.  Are you OK with this Grant?

Not really, no.  Defaulting to "input" may be incorrect, but it is
always safe, it it should only be a minor inconvenience to human users
of the sysfs interface.  Actual usage of a gpio pin must always be to
explicitly set the direction before using a pin.


> Changing the logic to allow "unavailable" GPIO pins to be
> requested/exported would require larger changes to the code, and
> wouldn't provide much benefit without additional changes (eg an alt_func
> feature).  So I'd vote to not add support for "unavailable" pins in this
> patch and rather wait until someone has a specific use for it, and add
> it then.

Agreed.

g.