[RFC 0/5] New descriptor-based GPIO interface

[RFC 0/5] New descriptor-based GPIO interface

[Alexandre Courbot]

Here is a first RFC for the new GPIO interface. It's terribly overdue, and in
order to avoid delaying it further I decided to send it as-is instead of being
even more perfectionist. The main issue I have with it is that it highlights
some points that could be simplified in gpiolib, some of which require a
refactoring of gpiolib's clients. I wasn't sure which, of the cleanup or new
interface, should come first and stayed in this state of confusion for several
months ; but in the end I believe it's more appropriate to clearly lay the
interfaces we want to use first, and then refactor the lib accordingly. So
please be mindful that another round of cleanup, both of gpiolib itself and its
consumers, should come once a later version of these patches is validated (and
hopefully merged) and that this series should be considered just another step
towards gpiolib's sanitization (although I also secretly hope the new interface
will be deemed useful by itself).

Another point that definitely needs more attention is the documentation. I am
not sure whether the new interface should be described as a couple of sections
in the existing GPIO documentation (the current approach) or as a new
documentation file of its own.

The new gpiod public interface is just an export of how gpiolib now works
internally. By representing GPIOs as opaque descriptors intead of integers, it
aims at making GPIO usage safer and more coherent with respect to other
subsystems by using a proper get/put mechanism and ensuring all GPIOs in use at
a given time are valid. It also makes GPIO acquisition easier for consumers, by
offering a standard way to bind GPIOs to devices. Finally, it tries to fix some
of the inconsistencies gpiolib acquired over time, like the incoherent handling
of active low GPIOs which was only valid when using the sysfs interface.

The old GPIO interface is still accessible and works the same way as before ; I
do not expect any existing code to break as a result of applying this series
(which is not extensively tested yet BTW).

TODO (preferably once this batch is validated):
- Change gpio_get()'s behavior to allow it to return specifiers other than the
first one for DT-declared GPIOs?
- Update as many users of the legacy interface as possible to use the new
interface
- Simplify gpiolib by removing redundant code and corner-cases (gpio_request
must die)
- Try to implement the GPIO fast-path method with gpiod, maybe using the LSB of
GPIO descriptors as a marker to forge fake descriptors?
- If possible, completely deprecate and remove the old gpio interface

Alexandre Courbot (5):
  gpiolib: factorize gpiod_get/set functions
  gpiolib: export descriptor-based GPIO interface
  gpiolib: port of_ functions to use gpiod
  gpiolib: add gpiod_get() and gpiod_put() functions
  gpiolib: update documentation

 Documentation/gpio.txt        | 119 ++++++++++
 drivers/gpio/gpiolib-of.c     |  28 ++-
 drivers/gpio/gpiolib.c        | 525 ++++++++++++++++++++++++++----------------
 include/asm-generic/gpio.h    | 227 ++++++------------
 include/linux/gpio.h          |  11 +-
 include/linux/gpio/consumer.h | 201 ++++++++++++++++
 include/linux/gpio/driver.h   | 163 +++++++++++++
 include/linux/of_gpio.h       |  29 ++-
 8 files changed, 920 insertions(+), 383 deletions(-)
 create mode 100644 include/linux/gpio/consumer.h
 create mode 100644 include/linux/gpio/driver.h

-- 
1.8.4

[RFC 1/5] gpiolib: factorize gpiod_get/set functions

[Alexandre Courbot]

gpiod_get/set functions share common code between their regular and
cansleep variants. The exporting of the gpiod interface will make
the situation worse. This patch factorizes the common code to avoid code
redundancy.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpio/gpiolib.c | 72 +++++++++++++++++++++++---------------------------
 1 file changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ff0fd65..6608bb8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1826,6 +1826,19 @@ EXPORT_SYMBOL_GPL(gpio_set_debounce);
  * that the GPIO was actually requested.
  */
 
+static int _gpiod_get_value(const struct gpio_desc *desc)
+{
+	struct gpio_chip	*chip;
+	int value;
+	int offset;
+
+	chip = desc->chip;
+	offset = gpio_chip_hwgpio(desc);
+	value = chip->get ? chip->get(chip, offset) : 0;
+	trace_gpio_value(desc_to_gpio(desc), 1, value);
+	return value;
+}
+
 /**
  * __gpio_get_value() - return a gpio's value
  * @gpio: gpio whose value will be returned
@@ -1837,19 +1850,11 @@ EXPORT_SYMBOL_GPL(gpio_set_debounce);
  */
 static int gpiod_get_value(const struct gpio_desc *desc)
 {
-	struct gpio_chip	*chip;
-	int value;
-	int offset;
-
 	if (!desc)
 		return 0;
-	chip = desc->chip;
-	offset = gpio_chip_hwgpio(desc);
 	/* Should be using gpio_get_value_cansleep() */
-	WARN_ON(chip->can_sleep);
-	value = chip->get ? chip->get(chip, offset) : 0;
-	trace_gpio_value(desc_to_gpio(desc), 1, value);
-	return value;
+	WARN_ON(desc->chip->can_sleep);
+	return _gpiod_get_value(desc);
 }
 
 int __gpio_get_value(unsigned gpio)
@@ -1912,6 +1917,20 @@ static void _gpio_set_open_source_value(struct gpio_desc *desc, int value)
 					__func__, desc_to_gpio(desc), err);
 }
 
+static void _gpiod_set_value(struct gpio_desc *desc, int value)
+{
+	struct gpio_chip	*chip;
+
+	chip = desc->chip;
+	trace_gpio_value(desc_to_gpio(desc), 0, value);
+	if (test_bit(FLAG_OPEN_DRAIN, &desc->flags))
+		_gpio_set_open_drain_value(desc, value);
+	else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags))
+		_gpio_set_open_source_value(desc, value);
+	else
+		chip->set(chip, gpio_chip_hwgpio(desc), value);
+}
+
 /**
  * __gpio_set_value() - assign a gpio's value
  * @gpio: gpio whose value will be assigned
@@ -1923,20 +1942,12 @@ static void _gpio_set_open_source_value(struct gpio_desc *desc, int value)
  */
 static void gpiod_set_value(struct gpio_desc *desc, int value)
 {
-	struct gpio_chip	*chip;
 
 	if (!desc)
 		return;
-	chip = desc->chip;
 	/* Should be using gpio_set_value_cansleep() */
-	WARN_ON(chip->can_sleep);
-	trace_gpio_value(desc_to_gpio(desc), 0, value);
-	if (test_bit(FLAG_OPEN_DRAIN, &desc->flags))
-		_gpio_set_open_drain_value(desc, value);
-	else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags))
-		_gpio_set_open_source_value(desc, value);
-	else
-		chip->set(chip, gpio_chip_hwgpio(desc), value);
+	WARN_ON(desc->chip->can_sleep);
+	_gpiod_set_value(desc, value);
 }
 
 void __gpio_set_value(unsigned gpio, int value)
@@ -2001,18 +2012,10 @@ EXPORT_SYMBOL_GPL(__gpio_to_irq);
 
 static int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 {
-	struct gpio_chip	*chip;
-	int value;
-	int offset;
-
 	might_sleep_if(extra_checks);
 	if (!desc)
 		return 0;
-	chip = desc->chip;
-	offset = gpio_chip_hwgpio(desc);
-	value = chip->get ? chip->get(chip, offset) : 0;
-	trace_gpio_value(desc_to_gpio(desc), 1, value);
-	return value;
+	return _gpiod_get_value(desc);
 }
 
 int gpio_get_value_cansleep(unsigned gpio)
@@ -2023,19 +2026,10 @@ EXPORT_SYMBOL_GPL(gpio_get_value_cansleep);
 
 static void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 {
-	struct gpio_chip	*chip;
-
 	might_sleep_if(extra_checks);
 	if (!desc)
 		return;
-	chip = desc->chip;
-	trace_gpio_value(desc_to_gpio(desc), 0, value);
-	if (test_bit(FLAG_OPEN_DRAIN,  &desc->flags))
-		_gpio_set_open_drain_value(desc, value);
-	else if (test_bit(FLAG_OPEN_SOURCE,  &desc->flags))
-		_gpio_set_open_source_value(desc, value);
-	else
-		chip->set(chip, gpio_chip_hwgpio(desc), value);
+	_gpiod_set_value(desc, value);
 }
 
 void gpio_set_value_cansleep(unsigned gpio, int value)
-- 
1.8.4

[RFC 2/5] gpiolib: export descriptor-based GPIO interface

[Alexandre Courbot]

This patch exports the gpiod_* family of API functions, a safer
alternative to the legacy gpio interface. Differences between the gpiod
and gpio APIs are:

- gpio works with integers, whereas gpiod operates on opaque handlers
  which cannot be forged or used before proper acquisition
- gpiod get/set functions are aware of the active low state of a GPIO
- gpio consumers should now include <linux/gpio/consumer.h> to access
  the new interface, whereas chips drivers will use
  <linux/gpio/driver.h>

The legacy gpio API is now built as inline functions on top of gpiod.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpio/gpiolib.c        | 307 ++++++++++++++++++------------------------
 include/asm-generic/gpio.h    | 227 +++++++++----------------------
 include/linux/gpio.h          |  11 +-
 include/linux/gpio/consumer.h | 193 ++++++++++++++++++++++++++
 include/linux/gpio/driver.h   | 125 +++++++++++++++++
 5 files changed, 525 insertions(+), 338 deletions(-)
 create mode 100644 include/linux/gpio/consumer.h
 create mode 100644 include/linux/gpio/driver.h

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6608bb8..e255eee 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -16,16 +16,11 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/gpio.h>
 
-/* Optional implementation infrastructure for GPIO interfaces.
+/* Implementation infrastructure for GPIO interfaces.
  *
- * Platforms may want to use this if they tend to use very many GPIOs
- * that aren't part of a System-On-Chip core; or across I2C/SPI/etc.
- *
- * When kernel footprint or instruction count is an issue, simpler
- * implementations may be preferred.  The GPIO programming interface
- * allows for inlining speed-critical get/set operations for common
- * cases, so that access to SOC-integrated GPIOs can sometimes cost
- * only an instruction or two per bit.
+ * The GPIO programming interface allows for inlining speed-critical
+ * get/set operations for common cases, so that access to SOC-integrated
+ * GPIOs can sometimes cost only an instruction or two per bit.
  */
 
 
@@ -57,7 +52,7 @@ struct gpio_desc {
 #define FLAG_SYSFS	3	/* exported via /sys/class/gpio/control */
 #define FLAG_TRIG_FALL	4	/* trigger on falling edge */
 #define FLAG_TRIG_RISE	5	/* trigger on rising edge */
-#define FLAG_ACTIVE_LOW	6	/* sysfs value has active low */
+#define FLAG_ACTIVE_LOW	6	/* value has active low */
 #define FLAG_OPEN_DRAIN	7	/* Gpio is open drain type */
 #define FLAG_OPEN_SOURCE 8	/* Gpio is open source type */
 
@@ -80,28 +75,8 @@ static LIST_HEAD(gpio_chips);
 static DEFINE_IDR(dirent_idr);
 #endif
 
-/*
- * Internal gpiod_* API using descriptors instead of the integer namespace.
- * Most of this should eventually go public.
- */
 static int gpiod_request(struct gpio_desc *desc, const char *label);
 static void gpiod_free(struct gpio_desc *desc);
-static int gpiod_direction_input(struct gpio_desc *desc);
-static int gpiod_direction_output(struct gpio_desc *desc, int value);
-static int gpiod_get_direction(const struct gpio_desc *desc);
-static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
-static int gpiod_get_value_cansleep(const struct gpio_desc *desc);
-static void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
-static int gpiod_get_value(const struct gpio_desc *desc);
-static void gpiod_set_value(struct gpio_desc *desc, int value);
-static int gpiod_cansleep(const struct gpio_desc *desc);
-static int gpiod_to_irq(const struct gpio_desc *desc);
-static int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
-static int gpiod_export_link(struct device *dev, const char *name,
-			     struct gpio_desc *desc);
-static int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
-static void gpiod_unexport(struct gpio_desc *desc);
-
 
 static inline void desc_set_label(struct gpio_desc *d, const char *label)
 {
@@ -121,23 +96,25 @@ static int gpio_chip_hwgpio(const struct gpio_desc *desc)
 /**
  * Convert a GPIO number to its descriptor
  */
-static struct gpio_desc *gpio_to_desc(unsigned gpio)
+struct gpio_desc *gpio_to_desc(unsigned gpio)
 {
 	if (WARN(!gpio_is_valid(gpio), "invalid GPIO %d\n", gpio))
 		return NULL;
 	else
 		return &gpio_desc[gpio];
 }
+EXPORT_SYMBOL_GPL(gpio_to_desc);
 
 /**
  * Convert a GPIO descriptor to the integer namespace.
  * This should disappear in the future but is needed since we still
  * use GPIO numbers for error messages and sysfs nodes
  */
-static int desc_to_gpio(const struct gpio_desc *desc)
+int desc_to_gpio(const struct gpio_desc *desc)
 {
 	return desc->chip->base + gpio_chip_hwgpio(desc);
 }
+EXPORT_SYMBOL_GPL(desc_to_gpio);
 
 
 /* Warn when drivers omit gpio_request() calls -- legal but ill-advised
@@ -172,16 +149,11 @@ static int gpio_ensure_requested(struct gpio_desc *desc)
 	return 0;
 }
 
-static struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
+struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 {
 	return desc ? desc->chip : NULL;
 }
-
-/* caller holds gpio_lock *OR* gpio is marked as requested */
-struct gpio_chip *gpio_to_chip(unsigned gpio)
-{
-	return gpiod_to_chip(gpio_to_desc(gpio));
-}
+EXPORT_SYMBOL_GPL(gpiod_to_chip);
 
 /* dynamic allocation of GPIOs, e.g. on a hotplugged device */
 static int gpiochip_find_base(int ngpio)
@@ -208,7 +180,7 @@ static int gpiochip_find_base(int ngpio)
 }
 
 /* caller ensures gpio is valid and requested, chip->get_direction may sleep  */
-static int gpiod_get_direction(const struct gpio_desc *desc)
+int gpiod_get_direction(const struct gpio_desc *desc)
 {
 	struct gpio_chip	*chip;
 	unsigned		offset;
@@ -234,6 +206,7 @@ static int gpiod_get_direction(const struct gpio_desc *desc)
 	}
 	return status;
 }
+EXPORT_SYMBOL_GPL(gpiod_get_direction);
 
 #ifdef CONFIG_GPIO_SYSFS
 
@@ -318,17 +291,10 @@ static ssize_t gpio_value_show(struct device *dev,
 
 	mutex_lock(&sysfs_lock);
 
-	if (!test_bit(FLAG_EXPORT, &desc->flags)) {
+	if (!test_bit(FLAG_EXPORT, &desc->flags))
 		status = -EIO;
-	} else {
-		int value;
-
-		value = !!gpiod_get_value_cansleep(desc);
-		if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
-			value = !value;
-
-		status = sprintf(buf, "%d\n", value);
-	}
+	else
+		status = sprintf(buf, "%d\n", gpiod_get_value_cansleep());
 
 	mutex_unlock(&sysfs_lock);
 	return status;
@@ -351,9 +317,7 @@ static ssize_t gpio_value_store(struct device *dev,
 
 		status = strict_strtol(buf, 0, &value);
 		if (status == 0) {
-			if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
-				value = !value;
-			gpiod_set_value_cansleep(desc, value != 0);
+			gpiod_set_value_cansleep(desc, value);
 			status = size;
 		}
 	}
@@ -736,7 +700,7 @@ static struct class gpio_class = {
 
 
 /**
- * gpio_export - export a GPIO through sysfs
+ * gpiod_export - export a GPIO through sysfs
  * @gpio: gpio to make available, already requested
  * @direction_may_change: true if userspace may change gpio direction
  * Context: arch_initcall or later
@@ -750,7 +714,7 @@ static struct class gpio_class = {
  *
  * Returns zero on success, else an error.
  */
-static int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
+int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 {
 	unsigned long		flags;
 	int			status;
@@ -828,12 +792,7 @@ fail_unlock:
 		 status);
 	return status;
 }
-
-int gpio_export(unsigned gpio, bool direction_may_change)
-{
-	return gpiod_export(gpio_to_desc(gpio), direction_may_change);
-}
-EXPORT_SYMBOL_GPL(gpio_export);
+EXPORT_SYMBOL_GPL(gpiod_export);
 
 static int match_export(struct device *dev, const void *data)
 {
@@ -841,7 +800,7 @@ static int match_export(struct device *dev, const void *data)
 }
 
 /**
- * gpio_export_link - create a sysfs link to an exported GPIO node
+ * gpiod_export_link - create a sysfs link to an exported GPIO node
  * @dev: device under which to create symlink
  * @name: name of the symlink
  * @gpio: gpio to create symlink to, already exported
@@ -851,8 +810,8 @@ static int match_export(struct device *dev, const void *data)
  *
  * Returns zero on success, else an error.
  */
-static int gpiod_export_link(struct device *dev, const char *name,
-			     struct gpio_desc *desc)
+int gpiod_export_link(struct device *dev, const char *name,
+		      struct gpio_desc *desc)
 {
 	int			status = -EINVAL;
 
@@ -883,15 +842,10 @@ static int gpiod_export_link(struct device *dev, const char *name,
 
 	return status;
 }
-
-int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
-{
-	return gpiod_export_link(dev, name, gpio_to_desc(gpio));
-}
-EXPORT_SYMBOL_GPL(gpio_export_link);
+EXPORT_SYMBOL_GPL(gpiod_export_link);
 
 /**
- * gpio_sysfs_set_active_low - set the polarity of gpio sysfs value
+ * gpiod_sysfs_set_active_low - set the polarity of gpio sysfs value
  * @gpio: gpio to change
  * @value: non-zero to use active low, i.e. inverted values
  *
@@ -902,7 +856,7 @@ EXPORT_SYMBOL_GPL(gpio_export_link);
  *
  * Returns zero on success, else an error.
  */
-static int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value)
+int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value)
 {
 	struct device		*dev = NULL;
 	int			status = -EINVAL;
@@ -933,20 +887,15 @@ unlock:
 
 	return status;
 }
-
-int gpio_sysfs_set_active_low(unsigned gpio, int value)
-{
-	return gpiod_sysfs_set_active_low(gpio_to_desc(gpio), value);
-}
-EXPORT_SYMBOL_GPL(gpio_sysfs_set_active_low);
+EXPORT_SYMBOL_GPL(gpiod_sysfs_set_active_low);
 
 /**
- * gpio_unexport - reverse effect of gpio_export()
+ * gpiod_unexport - reverse effect of gpio_export()
  * @gpio: gpio to make unavailable
  *
  * This is implicit on gpio_free().
  */
-static void gpiod_unexport(struct gpio_desc *desc)
+void gpiod_unexport(struct gpio_desc *desc)
 {
 	int			status = 0;
 	struct device		*dev = NULL;
@@ -979,12 +928,7 @@ static void gpiod_unexport(struct gpio_desc *desc)
 		pr_debug("%s: gpio%d status %d\n", __func__, desc_to_gpio(desc),
 			 status);
 }
-
-void gpio_unexport(unsigned gpio)
-{
-	gpiod_unexport(gpio_to_desc(gpio));
-}
-EXPORT_SYMBOL_GPL(gpio_unexport);
+EXPORT_SYMBOL_GPL(gpiod_unexport);
 
 static int gpiochip_export(struct gpio_chip *chip)
 {
@@ -1091,27 +1035,6 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
 {
 }
 
-static inline int gpiod_export(struct gpio_desc *desc,
-			       bool direction_may_change)
-{
-	return -ENOSYS;
-}
-
-static inline int gpiod_export_link(struct device *dev, const char *name,
-				    struct gpio_desc *desc)
-{
-	return -ENOSYS;
-}
-
-static inline int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value)
-{
-	return -ENOSYS;
-}
-
-static inline void gpiod_unexport(struct gpio_desc *desc)
-{
-}
-
 #endif /* CONFIG_GPIO_SYSFS */
 
 /*
@@ -1623,7 +1546,7 @@ EXPORT_SYMBOL_GPL(gpiochip_is_requested);
  * rely on gpio_request() having been called beforehand.
  */
 
-static int gpiod_direction_input(struct gpio_desc *desc)
+int gpiod_direction_input(struct gpio_desc *desc)
 {
 	unsigned long		flags;
 	struct gpio_chip	*chip;
@@ -1677,14 +1600,9 @@ fail:
 			 desc_to_gpio(desc), status);
 	return status;
 }
+EXPORT_SYMBOL_GPL(gpiod_direction_input);
 
-int gpio_direction_input(unsigned gpio)
-{
-	return gpiod_direction_input(gpio_to_desc(gpio));
-}
-EXPORT_SYMBOL_GPL(gpio_direction_input);
-
-static int gpiod_direction_output(struct gpio_desc *desc, int value)
+int gpiod_direction_output(struct gpio_desc *desc, int value)
 {
 	unsigned long		flags;
 	struct gpio_chip	*chip;
@@ -1746,19 +1664,14 @@ fail:
 			 desc_to_gpio(desc), status);
 	return status;
 }
-
-int gpio_direction_output(unsigned gpio, int value)
-{
-	return gpiod_direction_output(gpio_to_desc(gpio), value);
-}
-EXPORT_SYMBOL_GPL(gpio_direction_output);
+EXPORT_SYMBOL_GPL(gpiod_direction_output);
 
 /**
- * gpio_set_debounce - sets @debounce time for a @gpio
+ * gpiod_set_debounce - sets @debounce time for a @gpio
  * @gpio: the gpio to set debounce time
  * @debounce: debounce time is microseconds
  */
-static int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
+int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
 {
 	unsigned long		flags;
 	struct gpio_chip	*chip;
@@ -1797,12 +1710,19 @@ fail:
 
 	return status;
 }
+EXPORT_SYMBOL_GPL(gpiod_set_debounce);
 
-int gpio_set_debounce(unsigned gpio, unsigned debounce)
+/**
+ * gpiod_is_active_low - test whether a GPIO is active-low or not
+ * @desc: the gpio descriptor to test
+ *
+ * Returns 1 if the GPIO is active-low, 0 otherwise.
+ */
+int gpiod_is_active_low(const struct gpio_desc *desc)
 {
-	return gpiod_set_debounce(gpio_to_desc(gpio), debounce);
+	return test_bit(FLAG_ACTIVE_LOW, &desc->flags);
 }
-EXPORT_SYMBOL_GPL(gpio_set_debounce);
+EXPORT_SYMBOL_GPL(gpiod_is_active_low);
 
 /* I/O calls are only valid after configuration completed; the relevant
  * "is this a valid GPIO" error checks should already have been done.
@@ -1826,7 +1746,7 @@ EXPORT_SYMBOL_GPL(gpio_set_debounce);
  * that the GPIO was actually requested.
  */
 
-static int _gpiod_get_value(const struct gpio_desc *desc)
+static int _gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	struct gpio_chip	*chip;
 	int value;
@@ -1840,7 +1760,7 @@ static int _gpiod_get_value(const struct gpio_desc *desc)
 }
 
 /**
- * __gpio_get_value() - return a gpio's value
+ * gpiod_get_raw_value() - return a gpio's raw value
  * @gpio: gpio whose value will be returned
  * Context: any
  *
@@ -1848,25 +1768,44 @@ static int _gpiod_get_value(const struct gpio_desc *desc)
  * It returns the zero or nonzero value provided by the associated
  * gpio_chip.get() method; or zero if no such method is provided.
  */
-static int gpiod_get_value(const struct gpio_desc *desc)
+int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	if (!desc)
 		return 0;
 	/* Should be using gpio_get_value_cansleep() */
 	WARN_ON(desc->chip->can_sleep);
-	return _gpiod_get_value(desc);
+	return _gpiod_get_raw_value(desc);
 }
+EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
 
-int __gpio_get_value(unsigned gpio)
+/**
+ * gpiod_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 gpiod_get_value(const struct gpio_desc *desc)
 {
-	return gpiod_get_value(gpio_to_desc(gpio));
+	int value;
+	if (!desc)
+		return 0;
+	/* Should be using gpio_get_value_cansleep() */
+	WARN_ON(desc->chip->can_sleep);
+
+	value = _gpiod_get_raw_value(desc);
+	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+		value = !value;
+
+	return value;
 }
-EXPORT_SYMBOL_GPL(__gpio_get_value);
+EXPORT_SYMBOL_GPL(gpiod_get_value);
 
 /*
  *  _gpio_set_open_drain_value() - Set the open drain gpio's value.
- * @gpio: Gpio whose state need to be set.
- * @chip: Gpio chip.
+ * @desc: gpio descriptor whose state need to be set.
  * @value: Non-zero for setting it HIGH otherise it will set to LOW.
  */
 static void _gpio_set_open_drain_value(struct gpio_desc *desc, int value)
@@ -1891,9 +1830,8 @@ static void _gpio_set_open_drain_value(struct gpio_desc *desc, int value)
 }
 
 /*
- *  _gpio_set_open_source() - Set the open source gpio's value.
- * @gpio: Gpio whose state need to be set.
- * @chip: Gpio chip.
+ *  _gpio_set_open_source_value() - Set the open source gpio's value.
+ * @desc: gpio descriptor whose state need to be set.
  * @value: Non-zero for setting it HIGH otherise it will set to LOW.
  */
 static void _gpio_set_open_source_value(struct gpio_desc *desc, int value)
@@ -1917,7 +1855,7 @@ static void _gpio_set_open_source_value(struct gpio_desc *desc, int value)
 					__func__, desc_to_gpio(desc), err);
 }
 
-static void _gpiod_set_value(struct gpio_desc *desc, int value)
+static void _gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
 	struct gpio_chip	*chip;
 
@@ -1932,7 +1870,7 @@ static void _gpiod_set_value(struct gpio_desc *desc, int value)
 }
 
 /**
- * __gpio_set_value() - assign a gpio's value
+ * gpiod_set_raw_value() - assign a gpio's raw value
  * @gpio: gpio whose value will be assigned
  * @value: value to assign
  * Context: any
@@ -1940,46 +1878,55 @@ static void _gpiod_set_value(struct gpio_desc *desc, int value)
  * This is used directly or indirectly to implement gpio_set_value().
  * It invokes the associated gpio_chip.set() method.
  */
-static void gpiod_set_value(struct gpio_desc *desc, int value)
+void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
-
 	if (!desc)
 		return;
 	/* Should be using gpio_set_value_cansleep() */
 	WARN_ON(desc->chip->can_sleep);
-	_gpiod_set_value(desc, value);
+	_gpiod_set_raw_value(desc, value);
 }
-
-void __gpio_set_value(unsigned gpio, int value)
+EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
+/**
+ * gpiod_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 gpiod_set_value(struct gpio_desc *desc, int value)
 {
-	return gpiod_set_value(gpio_to_desc(gpio), value);
+	if (!desc)
+		return;
+	/* Should be using gpio_set_value_cansleep() */
+	WARN_ON(desc->chip->can_sleep);
+	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+		value = !value;
+	_gpiod_set_raw_value(desc, value);
 }
-EXPORT_SYMBOL_GPL(__gpio_set_value);
+EXPORT_SYMBOL_GPL(gpiod_set_value);
 
 /**
- * __gpio_cansleep() - report whether gpio value access will sleep
+ * gpiod_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.
  */
-static int gpiod_cansleep(const struct gpio_desc *desc)
+int gpiod_cansleep(const struct gpio_desc *desc)
 {
 	if (!desc)
 		return 0;
 	/* only call this on GPIOs that are valid! */
 	return desc->chip->can_sleep;
 }
-
-int __gpio_cansleep(unsigned gpio)
-{
-	return gpiod_cansleep(gpio_to_desc(gpio));
-}
-EXPORT_SYMBOL_GPL(__gpio_cansleep);
+EXPORT_SYMBOL_GPL(gpiod_cansleep);
 
 /**
- * __gpio_to_irq() - return the IRQ corresponding to a GPIO
+ * gpiod_to_irq() - return the IRQ corresponding to a GPIO
  * @gpio: gpio whose IRQ will be returned (already requested)
  * Context: any
  *
@@ -1987,7 +1934,7 @@ EXPORT_SYMBOL_GPL(__gpio_cansleep);
  * It returns the number of the IRQ signaled by this (input) GPIO,
  * or a negative errno.
  */
-static int gpiod_to_irq(const struct gpio_desc *desc)
+int gpiod_to_irq(const struct gpio_desc *desc)
 {
 	struct gpio_chip	*chip;
 	int			offset;
@@ -1998,45 +1945,57 @@ static int gpiod_to_irq(const struct gpio_desc *desc)
 	offset = gpio_chip_hwgpio(desc);
 	return chip->to_irq ? chip->to_irq(chip, offset) : -ENXIO;
 }
-
-int __gpio_to_irq(unsigned gpio)
-{
-	return gpiod_to_irq(gpio_to_desc(gpio));
-}
-EXPORT_SYMBOL_GPL(__gpio_to_irq);
-
+EXPORT_SYMBOL_GPL(gpiod_to_irq);
 
 /* There's no value in making it easy to inline GPIO calls that may sleep.
  * Common examples include ones connected to I2C or SPI chips.
  */
 
-static int gpiod_get_value_cansleep(const struct gpio_desc *desc)
+int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
 {
 	might_sleep_if(extra_checks);
 	if (!desc)
 		return 0;
-	return _gpiod_get_value(desc);
+	return _gpiod_get_raw_value(desc);
 }
+EXPORT_SYMBOL_GPL(gpiod_get_raw_value_cansleep);
 
-int gpio_get_value_cansleep(unsigned gpio)
+int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 {
-	return gpiod_get_value_cansleep(gpio_to_desc(gpio));
+	int value;
+
+	might_sleep_if(extra_checks);
+	if (!desc)
+		return 0;
+
+	value = _gpiod_get_raw_value(desc);
+	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+		value = !value;
+
+	return value;
 }
-EXPORT_SYMBOL_GPL(gpio_get_value_cansleep);
+EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
 
-static void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
+void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value)
 {
 	might_sleep_if(extra_checks);
 	if (!desc)
 		return;
-	_gpiod_set_value(desc, value);
+	_gpiod_set_raw_value(desc, value);
 }
+EXPORT_SYMBOL_GPL(gpiod_set_raw_value_cansleep);
 
-void gpio_set_value_cansleep(unsigned gpio, int value)
+void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 {
-	return gpiod_set_value_cansleep(gpio_to_desc(gpio), value);
+	might_sleep_if(extra_checks);
+	if (!desc)
+		return;
+
+	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+		value = !value;
+	_gpiod_set_raw_value(desc, value);
 }
-EXPORT_SYMBOL_GPL(gpio_set_value_cansleep);
+EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
 
 #ifdef CONFIG_DEBUG_FS
 
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index bde6469..4c18bae 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -10,6 +10,8 @@
 #ifdef CONFIG_GPIOLIB
 
 #include <linux/compiler.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/consumer.h>
 
 /* Platforms may implement their GPIO interface with library code,
  * at a small performance cost for non-inlined operations and some
@@ -49,122 +51,11 @@ struct module;
 struct device_node;
 struct gpio_desc;
 
-/**
- * struct gpio_chip - abstract a GPIO controller
- * @label: for diagnostics
- * @dev: optional device providing the GPIOs
- * @owner: helps prevent removal of modules exporting active GPIOs
- * @list: links gpio_chips together for traversal
- * @request: optional hook for chip-specific activation, such as
- *	enabling module power and clock; may sleep
- * @free: optional hook for chip-specific deactivation, such as
- *	disabling module power and clock; may sleep
- * @get_direction: returns direction for signal "offset", 0=out, 1=in,
- *	(same as GPIOF_DIR_XXX), or negative error
- * @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
- * @direction_output: configures signal "offset" as output, or returns error
- * @set_debounce: optional hook for setting debounce time for specified gpio in
- *      interrupt triggered gpio chips
- * @set: assigns output value for signal "offset"
- * @to_irq: optional hook supporting non-static gpio_to_irq() mappings;
- *	implementation may not sleep
- * @dbg_show: optional routine to show contents in debugfs; default code
- *	will be used when this is omitted, but custom code can show extra
- *	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 last GPIO
- *	handled is (base + ngpio - 1).
- * @desc: array of ngpio descriptors. Private.
- * @can_sleep: flag must be set iff get()/set() methods sleep, as they
- *	must while accessing GPIO expander chips over I2C or SPI
- * @names: if set, must be an array of strings to use as alternative
- *      names for the GPIOs in this chip. Any entry in the array
- *      may be NULL if there is no alias for the GPIO, however the
- *      array must be @ngpio entries long.  A name can include a single printk
- *      format specifier for an unsigned int.  It is substituted by the actual
- *      number of the gpio.
- *
- * 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, 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.
- */
-struct gpio_chip {
-	const char		*label;
-	struct device		*dev;
-	struct module		*owner;
-	struct list_head        list;
-
-	int			(*request)(struct gpio_chip *chip,
-						unsigned offset);
-	void			(*free)(struct gpio_chip *chip,
-						unsigned offset);
-	int			(*get_direction)(struct gpio_chip *chip,
-						unsigned offset);
-	int			(*direction_input)(struct gpio_chip *chip,
-						unsigned offset);
-	int			(*get)(struct gpio_chip *chip,
-						unsigned offset);
-	int			(*direction_output)(struct gpio_chip *chip,
-						unsigned offset, int value);
-	int			(*set_debounce)(struct gpio_chip *chip,
-						unsigned offset, unsigned debounce);
-
-	void			(*set)(struct gpio_chip *chip,
-						unsigned offset, int value);
-
-	int			(*to_irq)(struct gpio_chip *chip,
-						unsigned offset);
-
-	void			(*dbg_show)(struct seq_file *s,
-						struct gpio_chip *chip);
-	int			base;
-	u16			ngpio;
-	struct gpio_desc	*desc;
-	const char		*const *names;
-	unsigned		can_sleep:1;
-	unsigned		exported:1;
-
-#if defined(CONFIG_OF_GPIO)
-	/*
-	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
-	 * device tree automatically may have an OF translation
-	 */
-	struct device_node *of_node;
-	int of_gpio_n_cells;
-	int (*of_xlate)(struct gpio_chip *gc,
-		        const struct of_phandle_args *gpiospec, u32 *flags);
-#endif
-#ifdef CONFIG_PINCTRL
-	/*
-	 * If CONFIG_PINCTRL is enabled, then gpio controllers can optionally
-	 * describe the actual pin range which they serve in an SoC. This
-	 * information would be used by pinctrl subsystem to configure
-	 * corresponding pins for gpio usage.
-	 */
-	struct list_head pin_ranges;
-#endif
-};
-
-extern const char *gpiochip_is_requested(struct gpio_chip *chip,
-			unsigned offset);
-extern struct gpio_chip *gpio_to_chip(unsigned gpio);
-
-/* add/remove chips */
-extern int gpiochip_add(struct gpio_chip *chip);
-extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
-					int (*match)(struct gpio_chip *chip,
-						     void *data));
-
+/* caller holds gpio_lock *OR* gpio is marked as requested */
+static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
+{
+	return gpiod_to_chip(gpio_to_desc(gpio));
+}
 
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.
@@ -172,43 +63,86 @@ extern struct gpio_chip *gpiochip_find(void *data,
 extern int gpio_request(unsigned gpio, const char *label);
 extern void gpio_free(unsigned gpio);
 
-extern int gpio_direction_input(unsigned gpio);
-extern int gpio_direction_output(unsigned gpio, int value);
+static inline int gpio_direction_input(unsigned gpio)
+{
+	return gpiod_direction_input(gpio_to_desc(gpio));
+}
+static inline int gpio_direction_output(unsigned gpio, int value)
+{
+	return gpiod_direction_output(gpio_to_desc(gpio), value);
+}
 
-extern int gpio_set_debounce(unsigned gpio, unsigned debounce);
+static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
+{
+	return gpiod_set_debounce(gpio_to_desc(gpio), debounce);
+}
 
-extern int gpio_get_value_cansleep(unsigned gpio);
-extern void gpio_set_value_cansleep(unsigned gpio, int value);
+static inline int gpio_get_value_cansleep(unsigned gpio)
+{
+	extern int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
+	return gpiod_get_raw_value_cansleep(gpio_to_desc(gpio));
+}
+static inline void gpio_set_value_cansleep(unsigned gpio, int value)
+{
+	extern void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
+						 int value);
+	return gpiod_set_raw_value_cansleep(gpio_to_desc(gpio), value);
+}
 
 
 /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
  * the GPIO is constant and refers to some always-present controller,
  * giving direct access to chip registers and tight bitbanging loops.
  */
-extern int __gpio_get_value(unsigned gpio);
-extern void __gpio_set_value(unsigned gpio, int value);
+static inline int __gpio_get_value(unsigned gpio)
+{
+	extern int gpiod_get_raw_value(const struct gpio_desc *desc);
+	return gpiod_get_raw_value(gpio_to_desc(gpio));
+}
+static inline void __gpio_set_value(unsigned gpio, int value)
+{
+	extern void gpiod_set_raw_value(struct gpio_desc *desc, int value);
+	return gpiod_set_raw_value(gpio_to_desc(gpio), value);
+}
 
-extern int __gpio_cansleep(unsigned gpio);
+static inline int __gpio_cansleep(unsigned gpio)
+{
+	return gpiod_cansleep(gpio_to_desc(gpio));
+}
 
-extern int __gpio_to_irq(unsigned gpio);
+static inline int __gpio_to_irq(unsigned gpio)
+{
+	return gpiod_to_irq(gpio_to_desc(gpio));
+}
 
 extern int gpio_request_one(unsigned gpio, unsigned long flags, const char *label);
 extern int gpio_request_array(const struct gpio *array, size_t num);
 extern void gpio_free_array(const struct gpio *array, size_t num);
 
-#ifdef CONFIG_GPIO_SYSFS
-
 /*
  * A sysfs interface can be exported by individual drivers if they want,
  * but more typically is configured entirely from userspace.
  */
-extern int gpio_export(unsigned gpio, bool direction_may_change);
-extern int gpio_export_link(struct device *dev, const char *name,
-			unsigned gpio);
-extern int gpio_sysfs_set_active_low(unsigned gpio, int value);
-extern void gpio_unexport(unsigned gpio);
+static inline int gpio_export(unsigned gpio, bool direction_may_change)
+{
+	return gpiod_export(gpio_to_desc(gpio), direction_may_change);
+}
 
-#endif	/* CONFIG_GPIO_SYSFS */
+static inline int gpio_export_link(struct device *dev, const char *name,
+				   unsigned gpio)
+{
+	return gpiod_export_link(dev, name, gpio_to_desc(gpio));
+}
+
+static inline int gpio_sysfs_set_active_low(unsigned gpio, int value)
+{
+	return gpiod_sysfs_set_active_low(gpio_to_desc(gpio), value);
+}
+
+static inline void gpio_unexport(unsigned gpio)
+{
+	gpiod_unexport(gpio_to_desc(gpio));
+}
 
 #ifdef CONFIG_PINCTRL
 
@@ -278,31 +212,4 @@ static inline void gpio_set_value_cansleep(unsigned gpio, int value)
 
 #endif /* !CONFIG_GPIOLIB */
 
-#ifndef CONFIG_GPIO_SYSFS
-
-struct device;
-
-/* sysfs support is only available with gpiolib, where it's optional */
-
-static inline int gpio_export(unsigned gpio, bool direction_may_change)
-{
-	return -ENOSYS;
-}
-
-static inline int gpio_export_link(struct device *dev, const char *name,
-				unsigned gpio)
-{
-	return -ENOSYS;
-}
-
-static inline int gpio_sysfs_set_active_low(unsigned gpio, int value)
-{
-	return -ENOSYS;
-}
-
-static inline void gpio_unexport(unsigned gpio)
-{
-}
-#endif	/* CONFIG_GPIO_SYSFS */
-
 #endif /* _ASM_GENERIC_GPIO_H */
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 552e3f4..95717df 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -16,14 +16,17 @@
 #define GPIOF_OUT_INIT_LOW	(GPIOF_DIR_OUT | GPIOF_INIT_LOW)
 #define GPIOF_OUT_INIT_HIGH	(GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
 
+/* Gpio pin is active-low */
+#define GPIOF_ACTIVE_LOW        (1 << 2)
+
 /* Gpio pin is open drain */
-#define GPIOF_OPEN_DRAIN	(1 << 2)
+#define GPIOF_OPEN_DRAIN	(1 << 3)
 
 /* Gpio pin is open source */
-#define GPIOF_OPEN_SOURCE	(1 << 3)
+#define GPIOF_OPEN_SOURCE	(1 << 4)
 
-#define GPIOF_EXPORT		(1 << 4)
-#define GPIOF_EXPORT_CHANGEABLE	(1 << 5)
+#define GPIOF_EXPORT		(1 << 5)
+#define GPIOF_EXPORT_CHANGEABLE	(1 << 6)
 #define GPIOF_EXPORT_DIR_FIXED	(GPIOF_EXPORT)
 #define GPIOF_EXPORT_DIR_CHANGEABLE (GPIOF_EXPORT | GPIOF_EXPORT_CHANGEABLE)
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
new file mode 100644
index 0000000..ba39e09
--- /dev/null
+++ b/include/linux/gpio/consumer.h
@@ -0,0 +1,193 @@
+#ifndef __LINUX_GPIO_CONSUMER_H
+#define __LINUX_GPIO_CONSUMER_H
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+
+#ifdef CONFIG_GPIOLIB
+
+struct device;
+struct gpio_chip;
+
+/**
+ * Opaque descriptor for a GPIO. These are obtained using gpiod_get() and are
+ * preferable to the old integer-based handles.
+ *
+ * Contrary to integers, a pointer to a gpio_desc is guaranteed to be valid
+ * until the GPIO is released.
+ */
+struct gpio_desc;
+
+struct gpio_desc *gpio_to_desc(unsigned gpio);
+int desc_to_gpio(const struct gpio_desc *desc);
+struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
+
+int gpiod_get_direction(const struct gpio_desc *desc);
+int gpiod_direction_input(struct gpio_desc *desc);
+int gpiod_direction_output(struct gpio_desc *desc, int value);
+
+int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
+
+int gpiod_is_active_low(const struct gpio_desc *desc);
+int gpiod_get_value(const struct gpio_desc *desc);
+void gpiod_set_value(struct gpio_desc *desc, int value);
+
+int gpiod_cansleep(const struct gpio_desc *desc);
+int gpiod_get_value_cansleep(const struct gpio_desc *desc);
+void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
+
+int gpiod_to_irq(const struct gpio_desc *desc);
+
+#else /* CONFIG_GPIOLIB */
+
+static inline struct gpio_desc *__must_check gpiod_get(struct device *dev,
+						       const char *con_id)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline void gpiod_put(struct gpio_desc *desc)
+{
+	might_sleep();
+
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
+
+static inline struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
+							    const char *con_id)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
+{
+	might_sleep();
+
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
+
+static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
+{
+	return ERR_PTR(-EINVAL);
+}
+static inline int desc_to_gpio(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -EINVAL;
+}
+
+static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return ERR_PTR(-ENODEV);
+}
+
+
+static inline int gpiod_get_direction(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -ENOSYS;
+}
+static inline int gpiod_direction_input(struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -ENOSYS;
+}
+static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -ENOSYS;
+}
+
+
+static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -ENOSYS;
+}
+
+static inline int gpiod_is_active_low(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return 0;
+}
+static inline int gpiod_get_value(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return 0;
+}
+static inline void gpiod_set_value(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
+
+static inline int gpiod_cansleep(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return 0;
+}
+static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return 0;
+}
+static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
+
+static inline int gpiod_to_irq(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+	return -EINVAL;
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+#if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
+
+int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
+int gpiod_export_link(struct device *dev, const char *name,
+		      struct gpio_desc *desc);
+int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value);
+void gpiod_unexport(struct gpio_desc *desc);
+
+#else  /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */
+
+static inline int gpiod_export(struct gpio_desc *desc,
+			       bool direction_may_change)
+{
+	return -ENOSYS;
+}
+
+static inline int gpiod_export_link(struct device *dev, const char *name,
+				    struct gpio_desc *desc)
+{
+	return -ENOSYS;
+}
+
+static inline int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value)
+{
+	return -ENOSYS;
+}
+
+static inline void gpiod_unexport(struct gpio_desc *desc)
+{
+}
+
+#endif /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */
+
+#endif
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
new file mode 100644
index 0000000..b84ca26
--- /dev/null
+++ b/include/linux/gpio/driver.h
@@ -0,0 +1,125 @@
+#ifndef __LINUX_GPIO_DRIVER_H
+#define __LINUX_GPIO_DRIVER_H
+
+#include <linux/types.h>
+
+struct device;
+struct gpio_desc;
+
+/**
+ * struct gpio_chip - abstract a GPIO controller
+ * @label: for diagnostics
+ * @dev: optional device providing the GPIOs
+ * @owner: helps prevent removal of modules exporting active GPIOs
+ * @list: links gpio_chips together for traversal
+ * @request: optional hook for chip-specific activation, such as
+ *	enabling module power and clock; may sleep
+ * @free: optional hook for chip-specific deactivation, such as
+ *	disabling module power and clock; may sleep
+ * @get_direction: returns direction for signal "offset", 0=out, 1=in,
+ *	(same as GPIOF_DIR_XXX), or negative error
+ * @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
+ * @direction_output: configures signal "offset" as output, or returns error
+ * @set_debounce: optional hook for setting debounce time for specified gpio in
+ *      interrupt triggered gpio chips
+ * @set: assigns output value for signal "offset"
+ * @to_irq: optional hook supporting non-static gpio_to_irq() mappings;
+ *	implementation may not sleep
+ * @dbg_show: optional routine to show contents in debugfs; default code
+ *	will be used when this is omitted, but custom code can show extra
+ *	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 last GPIO
+ *	handled is (base + ngpio - 1).
+ * @desc: array of ngpio descriptors. Private.
+ * @can_sleep: flag must be set iff get()/set() methods sleep, as they
+ *	must while accessing GPIO expander chips over I2C or SPI
+ * @names: if set, must be an array of strings to use as alternative
+ *      names for the GPIOs in this chip. Any entry in the array
+ *      may be NULL if there is no alias for the GPIO, however the
+ *      array must be @ngpio entries long.  A name can include a single printk
+ *      format specifier for an unsigned int.  It is substituted by the actual
+ *      number of the gpio.
+ *
+ * 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, 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.
+ */
+struct gpio_chip {
+	const char		*label;
+	struct device		*dev;
+	struct module		*owner;
+	struct list_head        list;
+
+	int			(*request)(struct gpio_chip *chip,
+						unsigned offset);
+	void			(*free)(struct gpio_chip *chip,
+						unsigned offset);
+	int			(*get_direction)(struct gpio_chip *chip,
+						unsigned offset);
+	int			(*direction_input)(struct gpio_chip *chip,
+						unsigned offset);
+	int			(*get)(struct gpio_chip *chip,
+						unsigned offset);
+	int			(*direction_output)(struct gpio_chip *chip,
+						unsigned offset, int value);
+	int			(*set_debounce)(struct gpio_chip *chip,
+						unsigned offset,
+						unsigned debounce);
+
+	void			(*set)(struct gpio_chip *chip,
+						unsigned offset, int value);
+
+	int			(*to_irq)(struct gpio_chip *chip,
+						unsigned offset);
+
+	void			(*dbg_show)(struct seq_file *s,
+						struct gpio_chip *chip);
+	int			base;
+	u16			ngpio;
+	struct gpio_desc	*desc;
+	const char		*const *names;
+	unsigned		can_sleep:1;
+	unsigned		exported:1;
+
+#if defined(CONFIG_OF_GPIO)
+	/*
+	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
+	 * device tree automatically may have an OF translation
+	 */
+	struct device_node *of_node;
+	int of_gpio_n_cells;
+	int (*of_xlate)(struct gpio_chip *gc,
+			const struct of_phandle_args *gpiospec, u32 *flags);
+#endif
+#ifdef CONFIG_PINCTRL
+	/*
+	 * If CONFIG_PINCTRL is enabled, then gpio controllers can optionally
+	 * describe the actual pin range which they serve in an SoC. This
+	 * information would be used by pinctrl subsystem to configure
+	 * corresponding pins for gpio usage.
+	 */
+	struct list_head pin_ranges;
+#endif
+};
+
+extern const char *gpiochip_is_requested(struct gpio_chip *chip,
+			unsigned offset);
+
+/* add/remove chips */
+extern int gpiochip_add(struct gpio_chip *chip);
+extern int __must_check gpiochip_remove(struct gpio_chip *chip);
+extern struct gpio_chip *gpiochip_find(void *data,
+			      int (*match)(struct gpio_chip *chip, void *data));
+
+
+#endif
-- 
1.8.4

[RFC 3/5] gpiolib: port of_ functions to use gpiod

[Alexandre Courbot]

Refactor the of_ functions of gpiolib to use the now public gpiod
interface, and export of_get_named_gpiod() and of_get_gpiod() functions.

Flags are handled internally in the gpiod interface, so these functions
do not need a flags output parameter contrary to their legacy interface
counterparts.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpio/gpiolib-of.c | 28 +++++++++++++++++-----------
 include/linux/of_gpio.h   | 29 ++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 665f953..fd60bbc 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -15,19 +15,21 @@
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/io.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/slab.h>
 
+struct gpio_desc;
+
 /* Private data structure for of_gpiochip_find_and_xlate */
 struct gg_data {
 	enum of_gpio_flags *flags;
 	struct of_phandle_args gpiospec;
 
-	int out_gpio;
+	struct gpio_desc *out_gpio;
 };
 
 /* Private function for resolving node pointer to gpio_chip */
@@ -45,28 +47,31 @@ static int of_gpiochip_find_and_xlate(struct gpio_chip *gc, void *data)
 	if (ret < 0)
 		return false;
 
-	gg_data->out_gpio = ret + gc->base;
+	gg_data->out_gpio = gpio_to_desc(ret + gc->base);
 	return true;
 }
 
 /**
- * of_get_named_gpio_flags() - Get a GPIO number and flags to use with GPIO API
+ * of_get_named_gpiod_flags() - Get a GPIO descriptor and flags for GPIO API
  * @np:		device node to get GPIO from
  * @propname:	property name containing gpio specifier(s)
  * @index:	index of the GPIO
  * @flags:	a flags pointer to fill in
  *
- * Returns GPIO number to use with Linux generic GPIO API, or one of the errno
+ * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno
  * value on the error condition. If @flags is not NULL the function also fills
  * in flags for the GPIO.
  */
-int of_get_named_gpio_flags(struct device_node *np, const char *propname,
-			   int index, enum of_gpio_flags *flags)
+struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
+		     const char *propname, int index, enum of_gpio_flags *flags)
 {
 	/* Return -EPROBE_DEFER to support probe() functions to be called
 	 * later when the GPIO actually becomes available
 	 */
-	struct gg_data gg_data = { .flags = flags, .out_gpio = -EPROBE_DEFER };
+	struct gg_data gg_data = {
+		.flags = flags,
+		.out_gpio = ERR_PTR(-EPROBE_DEFER)
+	};
 	int ret;
 
 	/* .of_xlate might decide to not fill in the flags, so clear it. */
@@ -77,16 +82,17 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 					 &gg_data.gpiospec);
 	if (ret) {
 		pr_debug("%s: can't parse gpios property\n", __func__);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	gpiochip_find(&gg_data, of_gpiochip_find_and_xlate);
 
 	of_node_put(gg_data.gpiospec.np);
-	pr_debug("%s exited with status %d\n", __func__, gg_data.out_gpio);
+	pr_debug("%s exited with status %d\n", __func__,
+		 PTR_RET(gg_data.out_gpio));
 	return gg_data.out_gpio;
 }
-EXPORT_SYMBOL(of_get_named_gpio_flags);
+EXPORT_SYMBOL(of_get_named_gpiod_flags);
 
 /**
  * of_gpio_simple_xlate - translate gpio_spec to the GPIO number and flags
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index a83dc6f..d71f2cc 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -21,6 +21,7 @@
 #include <linux/of.h>
 
 struct device_node;
+struct gpio_desc;
 
 /*
  * This is Linux-specific flags. By default controllers' and Linux' mapping
@@ -47,7 +48,7 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc)
 	return container_of(gc, struct of_mm_gpio_chip, gc);
 }
 
-extern int of_get_named_gpio_flags(struct device_node *np,
+extern struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 		const char *list_name, int index, enum of_gpio_flags *flags);
 
 extern int of_mm_gpiochip_add(struct device_node *np,
@@ -62,10 +63,10 @@ extern int of_gpio_simple_xlate(struct gpio_chip *gc,
 #else /* CONFIG_OF_GPIO */
 
 /* Drivers may not strictly depend on the GPIO support, so let them link. */
-static inline int of_get_named_gpio_flags(struct device_node *np,
+static inline struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 		const char *list_name, int index, enum of_gpio_flags *flags)
 {
-	return -ENOSYS;
+	return ERR_PTR(-ENOSYS);
 }
 
 static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
@@ -80,6 +81,18 @@ static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
 
 #endif /* CONFIG_OF_GPIO */
 
+static inline int of_get_named_gpio_flags(struct device_node *np,
+		const char *list_name, int index, enum of_gpio_flags *flags)
+{
+	struct gpio_desc *desc;
+	desc = of_get_named_gpiod_flags(np, list_name, index, flags);
+
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+	else
+		return desc_to_gpio(desc);
+}
+
 /**
  * of_gpio_named_count() - Count GPIOs for a device
  * @np:		device node to count GPIOs for
@@ -117,15 +130,21 @@ static inline int of_gpio_count(struct device_node *np)
 }
 
 /**
- * of_get_gpio_flags() - Get a GPIO number and flags to use with GPIO API
+ * of_get_gpiod_flags() - Get a GPIO descriptor and flags to use with GPIO API
  * @np:		device node to get GPIO from
  * @index:	index of the GPIO
  * @flags:	a flags pointer to fill in
  *
- * Returns GPIO number to use with Linux generic GPIO API, or one of the errno
+ * Returns GPIO descriptor to use with Linux generic GPIO API, or a errno
  * value on the error condition. If @flags is not NULL the function also fills
  * in flags for the GPIO.
  */
+static inline struct gpio_desc *of_get_gpiod_flags(struct device_node *np,
+					int index, enum of_gpio_flags *flags)
+{
+	return of_get_named_gpiod_flags(np, "gpios", index, flags);
+}
+
 static inline int of_get_gpio_flags(struct device_node *np, int index,
 		      enum of_gpio_flags *flags)
 {
-- 
1.8.4

[RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Alexandre Courbot]

Add gpiod_get() and gpiod_put() functions that provide safer handling of
GPIOs.

These functions put the GPIO framework in line with the conventions of
other frameworks in the kernel, and help ensure every GPIO is declared
properly and valid while it is used.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpio/gpiolib.c        | 166 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |   8 ++
 include/linux/gpio/driver.h   |  38 ++++++++++
 3 files changed, 212 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e255eee..4a91e03 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -69,6 +69,8 @@ static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
 
 #define GPIO_OFFSET_VALID(chip, offset) (offset >= 0 && offset < chip->ngpio)
 
+static DEFINE_MUTEX(gpio_lookup_lock);
+static LIST_HEAD(gpio_lookup_list);
 static LIST_HEAD(gpio_chips);
 
 #ifdef CONFIG_GPIO_SYSFS
@@ -1997,6 +1999,170 @@ void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 }
 EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
 
+/**
+ * gpiod_add_table() - register GPIO device consumers
+ * @table: array of consumers to register
+ * @num: number of consumers in table
+ */
+void gpiod_add_table(struct gpiod_lookup *table, size_t size)
+{
+	mutex_lock(&gpio_lookup_lock);
+
+	while (size--) {
+		list_add_tail(&table->list, &gpio_lookup_list);
+		table++;
+	}
+
+	mutex_unlock(&gpio_lookup_lock);
+}
+
+/*
+ * Caller must have a acquired gpio_lookup_lock
+ */
+static struct gpio_chip *find_chip_by_name(const char *name)
+{
+	struct gpio_chip *chip = NULL;
+
+	list_for_each_entry(chip, &gpio_lookup_list, list) {
+		if (chip->label == NULL)
+			continue;
+		if (!strcmp(chip->label, name))
+			break;
+	}
+
+	return chip;
+}
+
+#ifdef CONFIG_OF
+static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
+				      unsigned long *flags)
+{
+	char prop_name[32]; /* 32 is max size of property name */
+	enum of_gpio_flags of_flags;
+	struct gpio_desc *desc;
+
+	if (con_id)
+		snprintf(prop_name, 32, "%s-gpios", con_id);
+	else
+		snprintf(prop_name, 32, "gpios");
+
+	desc = of_get_named_gpiod_flags(dev->of_node, prop_name, 0, &of_flags);
+
+	if (IS_ERR(desc))
+		return desc;
+
+	if (of_flags & OF_GPIO_ACTIVE_LOW)
+		*flags |= GPIOF_ACTIVE_LOW;
+
+	return desc;
+}
+#else
+static struct device_node *of_find_gpio(struct device *dev, const char *id
+					unsigned long flags)
+{
+	return NULL;
+}
+#endif
+
+static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
+				    unsigned long *flags)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	struct gpio_desc *desc = ERR_PTR(-ENODEV);
+	unsigned int match, best = 0;
+	struct gpiod_lookup *p;
+
+	mutex_lock(&gpio_lookup_lock);
+
+	list_for_each_entry(p, &gpio_lookup_list, list) {
+		match = 0;
+
+		if (p->dev_id) {
+			if (!dev_id || strcmp(p->dev_id, dev_id))
+				continue;
+
+			match += 2;
+		}
+
+		if (p->con_id) {
+			if (!con_id || strcmp(p->con_id, con_id))
+				continue;
+
+			match += 1;
+		}
+
+		if (match > best) {
+			struct gpio_chip *chip;
+
+			chip = find_chip_by_name(p->chip_label);
+
+			if (!chip) {
+				dev_warn(dev, "cannot find GPIO chip %s\n",
+					 p->chip_label);
+				continue;
+			}
+
+			if (chip->ngpio >= p->chip_hwnum) {
+				dev_warn(dev, "GPIO chip %s has %d GPIOs\n",
+					 chip->label, chip->ngpio);
+				continue;
+			}
+
+			desc = gpio_to_desc(chip->base + p->chip_hwnum);
+			*flags = p->flags;
+
+			if (match != 3)
+				best = match;
+			else
+				break;
+		}
+	}
+
+	mutex_unlock(&gpio_lookup_lock);
+
+	return desc;
+}
+
+struct gpio_desc *__must_check gpiod_get(struct device *dev, const char *con_id)
+{
+	struct gpio_desc *desc;
+	int status;
+	unsigned long flags = 0;
+
+	dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
+
+	/* Using device tree? */
+	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node) {
+		dev_dbg(dev, "using device tree for GPIO lookup\n");
+		desc = of_find_gpio(dev, con_id, &flags);
+	} else {
+		dev_dbg(dev, "using lookup tables for GPIO lookup");
+		desc = gpiod_find(dev, con_id, &flags);
+	}
+
+	if (IS_ERR(desc)) {
+		dev_warn(dev, "lookup for GPIO %s failed\n", con_id);
+		return desc;
+	}
+
+	status = gpiod_request(desc, con_id);
+
+	if (status < 0)
+		return ERR_PTR(status);
+
+	if (flags & GPIOF_ACTIVE_LOW)
+		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
+	return desc;
+}
+EXPORT_SYMBOL_GPL(gpiod_get);
+
+void gpiod_put(struct gpio_desc *desc)
+{
+	gpiod_free(desc);
+}
+EXPORT_SYMBOL_GPL(gpiod_put);
+
 #ifdef CONFIG_DEBUG_FS
 
 static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index ba39e09..da9cdb2 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -18,6 +18,14 @@ struct gpio_chip;
  */
 struct gpio_desc;
 
+struct gpio_desc *__must_check gpiod_get(struct device *dev,
+					 const char *con_id);
+void gpiod_put(struct gpio_desc *desc);
+
+struct gpio_desc *__must_check devm_gpiod_get(struct device *dev,
+					      const char *con_id);
+void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
+
 struct gpio_desc *gpio_to_desc(unsigned gpio);
 int desc_to_gpio(const struct gpio_desc *desc);
 struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index b84ca26..5aa26b5 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -121,5 +121,43 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
 extern struct gpio_chip *gpiochip_find(void *data,
 			      int (*match)(struct gpio_chip *chip, void *data));
 
+/**
+ * Lookup table for associating GPIOs to specific devices and functions using
+ * platform data.
+ */
+struct gpiod_lookup {
+	struct list_head list;
+	/*
+	 * name of the chip the GPIO belongs to
+	 */
+	const char *chip_label;
+	/*
+	 * hardware number (i.e. relative to the chip) of the GPIO
+	 */
+	u16 chip_hwnum;
+	/*
+	 * name of device that can claim this GPIO
+	 */
+	const char *dev_id;
+	/*
+	 * name of the GPIO from the device's point of view
+	 */
+	const char *con_id;
+	/*
+	 * mask of GPIOF_* values
+	 */
+	unsigned long flags;
+};
+
+#define GPIO_LOOKUP(_chip_label, _chip_hwnum, _dev_id, _con_id, flags) \
+{                                                                      \
+	.chip_label = _chip_label,                                     \
+	.chip_hwnum = _chip_hwnum,                                     \
+	.dev_id = _dev_id,                                             \
+	.con_id = _con_id,                                             \
+	.flags = flags,                                                \
+}
+
+void gpiod_add_table(struct gpiod_lookup *table, size_t size);
 
 #endif
-- 
1.8.4

[RFC 5/5] gpiolib: update documentation

[Alexandre Courbot]

Add sections about the new gpiod interface in the documentation,
explaining the motivations behind it and the differences with respect to
the legacy API.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 Documentation/gpio.txt | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index 6f83fa9..e254a38 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -87,6 +87,38 @@ Note that these operations include I/O barriers on platforms which need to
 use them; drivers don't need to add them explicitly.
 
 
+GPIO interfaces
+---------------
+The GPIO framework has quite a bit of history behind it. Currently there exist
+two different (although very similar) ways of using GPIOs:
+
+  - The legacy integer-based interface represents GPIOs as integers. This is
+    the "historic" way of accessing GPIOs and it was done so because it makes
+    GPIOs easy to represent and also allows for the compiler to statically know
+    the GPIO number and use fast-paths on GPIOs for which performance matters.
+    However, GPIOs can easily be forged this way, and the maximum number of
+    GPIOs in the system must be known in advance. Functions of this interface
+    are prefixed with "gpio_".
+
+  - The new descriptor-based interface represents GPIOs as an opaque pointer.
+    This ensures GPIOs are properly acquired before usage, and also does not
+    presume anything about their underlying implementation. This interface
+    provides get/put functions to acquire GPIOs according to their function for
+    a particular device, similarly to e.g. the regulator framework. For these
+    reasons, it is the preferred way to access GPIOs. Its functions are prefixed
+    with "gpiod_".
+
+Both interfaces rely on gpiolib. Actually, the older integer-based interface is
+built as inline functions on top of the new descriptor interface. It is possible
+and easy to convert a GPIO descriptor to an integer and vice-versa, although it
+is recommended that drivers stick to using a single interface, preferably the
+new descriptor-based one.
+
+The remainder of this document will describes the old integer-based interface,
+up to the last section which details the differences of the descriptor-based
+interface.
+
+
 Identifying GPIOs
 -----------------
 GPIOs are identified by unsigned integers in the range 0..MAX_INT.  That
@@ -773,3 +805,90 @@ differences between boards from user space.  This only affects the
 sysfs interface.  Polarity change can be done both before and after
 gpio_export(), and previously enabled poll(2) support for either
 rising or falling edge will be reconfigured to follow this setting.
+
+
+GPIO descriptor interface
+=========================
+A more secure, alternative GPIO interface using descriptors is also available.
+Instead of relying on integers (which can easily be forged and used without
+being properly requested) to reference GPIOs, it uses a system of opaque
+descriptors that must be properly obtained and disposed through the common
+get/put set of functions. This ensures that all GPIO descriptors are valid at
+any time and makes it unnecessary to check the validity of a GPIO. Apart from
+this difference, the interface is similar to the integer-based one, excepted
+that the gpio_ prefix is changed to gpiod_. There is only one difference in
+behavior: for GPIOs acquired with a GPIOF_ACTIVE_LOW flag, or for which the
+active low property has been set through the sysfs interface, the get/set_value
+functions will take the active low property into account when setting the value.
+The old interface ignores this property, and its get/set_value work with the
+actual physical level of the GPIO.
+
+Using GPIOs
+-----------
+GPIO consumers should include <linux/gpio/consumer.h> which declares the
+consumer-side GPIO functions. GPIOs are obtained through gpiod_get:
+
+ struct gpio_desc *gpiod_get(struct device *dev, const char *con_id);
+
+This will return the GPIO descriptor corresponding to the con_id function of
+dev, or an error code in case of error. A devm variant is also available:
+
+ struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id);
+
+GPIO descriptors are disposed using the corresponding put functions:
+
+ void gpiod_put(struct gpio_desc *desc);
+ void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
+
+A valid descriptor can then be used with one of the gpiod_ functions. Their
+interface is identical to the integer-based API, excepted that they take a GPIO
+descriptor instead of an integer:
+
+ int gpiod_direction_input(struct gpio_desc *desc);
+ int gpiod_direction_output(struct gpio_desc *desc, int value);
+ int gpiod_get_value(struct gpio_desc *desc);
+ void gpiod_set_value(struct gpio_desc *desc, int value);
+ ...
+
+If you need to convert a descriptor to an integer or vice-versa, you can use
+gpio_to_desc or desc_to_gpio:
+
+ struct gpio_desc *gpio_to_desc(unsigned gpio);
+ int desc_to_gpio(const struct gpio_desc *desc);
+
+The same GPIO can be used by both interfaces as long as it has properly been
+acquired by one of them (i.e. using either gpio_request() or gpiod_get()).
+
+Declaring GPIOs
+---------------
+GPIOs can be made available for devices either through board-specific lookup
+tables, or using the device tree.
+
+Board-specific lookup tables match a device name and consumer ID to a GPIO chip
+and GPIO number relative to that chip. They are declared as follows:
+
+ static struct gpio_lookup board_gpio_lookup[] = {
+     GPIO_LOOKUP("tegra-gpio", 28, "backlight.1", "power", GPIOF_ACTIVE_LOW),
+ };
+
+ static void __init board_init(void)
+ {
+     ...
+     gpiod_add_table(board_gpio_lookup,
+     ARRAY_SIZE(board_gpio_lookup));
+     ...
+ }
+
+In the driver side, the following code:
+
+ gpiod_get(dev, "power");
+
+will return the descriptor for GPIO 28 of the "tegra-gpio" chip provided
+strcmp(dev_name(dev), "backlight.1") == 0. This GPIO will be active low.
+
+If the device tree is used, then the same "power" GPIO can be declared into the
+device's node as follows:
+
+ power-gpios = <&gpio 28 GPIO_ACTIVE_LOW>;
+
+Note that gpiod_get() only allows to access the first GPIO specifier.
\ No newline at end of file
-- 
1.8.4

Re: [RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Stephen Warren]

On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
> Add gpiod_get() and gpiod_put() functions that provide safer handling of
> GPIOs.
> 
> These functions put the GPIO framework in line with the conventions of
> other frameworks in the kernel, and help ensure every GPIO is declared
> properly and valid while it is used.

> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h

> +struct gpio_desc *__must_check gpiod_get(struct device *dev,
> +					 const char *con_id);
> +void gpiod_put(struct gpio_desc *desc);

It might be nice to add an "int index" parameter to this function. For
example, a bit-banged parallel bus protocol driver might have 1
chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
"bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
gpiod_get_index(dev, con_id, index)?

In DT terms, this would map to:

cs-gpios = <&gpio 3 0>;
clock-gpios = <&gpio 5 0>;
bus-gpios = <&gpio 10 0 ... &gpio 17 0>;

... and with the mapping table registration mechanism, we could
presumably add "int index" to struct gpiod_lookup.

Re: [RFC 2/5] gpiolib: export descriptor-based GPIO interface

[Stephen Warren]

On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
> This patch exports the gpiod_* family of API functions, a safer
> alternative to the legacy gpio interface. Differences between the gpiod
> and gpio APIs are:
> 
> - gpio works with integers, whereas gpiod operates on opaque handlers
>   which cannot be forged or used before proper acquisition
> - gpiod get/set functions are aware of the active low state of a GPIO
> - gpio consumers should now include <linux/gpio/consumer.h> to access
>   the new interface, whereas chips drivers will use
>   <linux/gpio/driver.h>
> 
> The legacy gpio API is now built as inline functions on top of gpiod.
> 

> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h

> +struct gpio_chip {

> +	int			(*get_direction)(struct gpio_chip *chip,
> +						unsigned offset);
> +	int			(*direction_input)(struct gpio_chip *chip,
> +						unsigned offset);
> +	int			(*get)(struct gpio_chip *chip,
> +						unsigned offset);
> +	int			(*direction_output)(struct gpio_chip *chip,
> +						unsigned offset, int value);
> +	int			(*set_debounce)(struct gpio_chip *chip,
> +						unsigned offset,
> +						unsigned debounce);
> +
> +	void			(*set)(struct gpio_chip *chip,
> +						unsigned offset, int value);
> +

Minor nit: It might be nice to sort/group these entries more cohesively,
i.e. get_direction, direction_input, direction_output, get, set,
set_debounce?

Re: [RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Alexandre Courbot]

On Thu, Sep 5, 2013 at 4:56 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
>> Add gpiod_get() and gpiod_put() functions that provide safer handling of
>> GPIOs.
>>
>> These functions put the GPIO framework in line with the conventions of
>> other frameworks in the kernel, and help ensure every GPIO is declared
>> properly and valid while it is used.
>
>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>
>> +struct gpio_desc *__must_check gpiod_get(struct device *dev,
>> +                                      const char *con_id);
>> +void gpiod_put(struct gpio_desc *desc);
>
> It might be nice to add an "int index" parameter to this function. For
> example, a bit-banged parallel bus protocol driver might have 1
> chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
> 0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
> "bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
> implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
> gpiod_get_index(dev, con_id, index)?
>
> In DT terms, this would map to:
>
> cs-gpios = <&gpio 3 0>;
> clock-gpios = <&gpio 5 0>;
> bus-gpios = <&gpio 10 0 ... &gpio 17 0>;
>
> ... and with the mapping table registration mechanism, we could
> presumably add "int index" to struct gpiod_lookup.

Having the index argument of of_get_named_gpiod_flags() propagated
into gpiod_get() is something I also thought about (see the cover
letter), but I really can't make up my mind about it.

On the one hand, having several GPIO per DT property is already
allowed, and presumably used in bindings already. It might also make
sense to have several lines under the same name, e.g. for bitbanging.

On the other hand, I'm afraid people will abuse this, and group all
the GPIOs for a device under a single property instead of using proper
names.

I guess that if we want gpiod to cover all that gpio allows already
(as the ultimate goal is to superseed the latter) we have no choice
but support this though. A (possibly cleaner) alternative would be to
have a get_gpios(device, name, gpio_desc **descs, int nb_desc)
function that returns all the GPIOs defined under a single property,
and fails if the number of descriptors does not match nb_desc. If that
works it would be more tailored towards grouping GPIOs that serve the
same logical purpose, and discourage the behavior I worried about
above.

Alex.

Re: [RFC 2/5] gpiolib: export descriptor-based GPIO interface

[Alexandre Courbot]

On Thu, Sep 5, 2013 at 4:58 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
>> This patch exports the gpiod_* family of API functions, a safer
>> alternative to the legacy gpio interface. Differences between the gpiod
>> and gpio APIs are:
>>
>> - gpio works with integers, whereas gpiod operates on opaque handlers
>>   which cannot be forged or used before proper acquisition
>> - gpiod get/set functions are aware of the active low state of a GPIO
>> - gpio consumers should now include <linux/gpio/consumer.h> to access
>>   the new interface, whereas chips drivers will use
>>   <linux/gpio/driver.h>
>>
>> The legacy gpio API is now built as inline functions on top of gpiod.
>>
>
>> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
>
>> +struct gpio_chip {
>
>> +     int                     (*get_direction)(struct gpio_chip *chip,
>> +                                             unsigned offset);
>> +     int                     (*direction_input)(struct gpio_chip *chip,
>> +                                             unsigned offset);
>> +     int                     (*get)(struct gpio_chip *chip,
>> +                                             unsigned offset);
>> +     int                     (*direction_output)(struct gpio_chip *chip,
>> +                                             unsigned offset, int value);
>> +     int                     (*set_debounce)(struct gpio_chip *chip,
>> +                                             unsigned offset,
>> +                                             unsigned debounce);
>> +
>> +     void                    (*set)(struct gpio_chip *chip,
>> +                                             unsigned offset, int value);
>> +
>
> Minor nit: It might be nice to sort/group these entries more cohesively,
> i.e. get_direction, direction_input, direction_output, get, set,
> set_debounce?

Sure, since we are moving this around anyway... :)

Alex.

Re: [RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2920 bytes --]

On Thu, Sep 05, 2013 at 12:44:34PM +0900, Alexandre Courbot wrote:
> On Thu, Sep 5, 2013 at 4:56 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
> >> Add gpiod_get() and gpiod_put() functions that provide safer handling of
> >> GPIOs.
> >>
> >> These functions put the GPIO framework in line with the conventions of
> >> other frameworks in the kernel, and help ensure every GPIO is declared
> >> properly and valid while it is used.
> >
> >> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> >
> >> +struct gpio_desc *__must_check gpiod_get(struct device *dev,
> >> +                                      const char *con_id);
> >> +void gpiod_put(struct gpio_desc *desc);
> >
> > It might be nice to add an "int index" parameter to this function. For
> > example, a bit-banged parallel bus protocol driver might have 1
> > chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
> > 0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
> > "bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
> > implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
> > gpiod_get_index(dev, con_id, index)?
> >
> > In DT terms, this would map to:
> >
> > cs-gpios = <&gpio 3 0>;
> > clock-gpios = <&gpio 5 0>;
> > bus-gpios = <&gpio 10 0 ... &gpio 17 0>;
> >
> > ... and with the mapping table registration mechanism, we could
> > presumably add "int index" to struct gpiod_lookup.
> 
> Having the index argument of of_get_named_gpiod_flags() propagated
> into gpiod_get() is something I also thought about (see the cover
> letter), but I really can't make up my mind about it.
> 
> On the one hand, having several GPIO per DT property is already
> allowed, and presumably used in bindings already. It might also make
> sense to have several lines under the same name, e.g. for bitbanging.
> 
> On the other hand, I'm afraid people will abuse this, and group all
> the GPIOs for a device under a single property instead of using proper
> names.

I wouldn't worry about this too much. Preventing abuse is a large part
of why we have DT bindings reviews and I think that regardless of what
the implementation allows us to, that shouldn't influence the way how
bindings are defined.

After all the bindings would ideally be usable on any other OS as well,
so it should be sound irrespective of the specific implementation.

I think having a gpiod_get_index(dev, con_id, index) with an inline
wrapper gpiod_get(dev, con_id) will give us the same functionality that
we have using the current set of helpers from include/linux/of_gpio.h
and allows non-DT to specify them in an analogous way.

In my opinion that's enough for now. We can always refactor common
patterns (such as busses with 0..n GPIOs) when they start to emerge.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 0/5] New descriptor-based GPIO interface

[Linus Walleij]

On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> Another point that definitely needs more attention is the documentation. I am
> not sure whether the new interface should be described as a couple of sections
> in the existing GPIO documentation (the current approach) or as a new
> documentation file of its own.

The right way to do this would be to list the new way in the gpio.txt document,
then mark the old methods as deprecated.

Yours,
Linus Walleij

Re: [RFC 1/5] gpiolib: factorize gpiod_get/set functions

[Linus Walleij]

On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> gpiod_get/set functions share common code between their regular and
> cansleep variants. The exporting of the gpiod interface will make
> the situation worse. This patch factorizes the common code to avoid code
> redundancy.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

I don't see why this patch should be RFC?

I just rebased and applied it, it's a clean and nice refactoring.

Yours,
Linus Walleij

Re: [RFC 5/5] gpiolib: update documentation

[Linus Walleij]

On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> +The GPIO framework has quite a bit of history behind it. Currently there exist
> +two different (although very similar) ways of using GPIOs:
> +
> +  - The legacy integer-based interface represents GPIOs as integers. This is
> +    the "historic" way of accessing GPIOs and it was done so because it makes
> +    GPIOs easy to represent and also allows for the compiler to statically know
> +    the GPIO number and use fast-paths on GPIOs for which performance matters.
> +    However, GPIOs can easily be forged this way, and the maximum number of
> +    GPIOs in the system must be known in advance. Functions of this interface
> +    are prefixed with "gpio_".
> +
> +  - The new descriptor-based interface represents GPIOs as an opaque pointer.
> +    This ensures GPIOs are properly acquired before usage, and also does not
> +    presume anything about their underlying implementation. This interface
> +    provides get/put functions to acquire GPIOs according to their function for
> +    a particular device, similarly to e.g. the regulator framework. For these
> +    reasons, it is the preferred way to access GPIOs. Its functions are prefixed
> +    with "gpiod_".

I would put all the new style gpiod_* based things on top of the file, and
all the old stuff under a separate heading below DEPRECATED LEGACY INTERFACE
so it's crystal clear that this is going away.

Yours,
Linus Walleij

Re: [RFC 0/5] New descriptor-based GPIO interface

[Linus Walleij]

On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> Here is a first RFC for the new GPIO interface.

I'm quite happy with this, and given that all DT-implementations will start
to use it from day 1 I'll happily merge it when you think it's tested enough.

On my wishlist is to also switch over the ACPI GPIO driver:
drivers/gpio/gpiolib-acpi.c to use the descriptors directly, so
I'd like one of the ACPI folks to have a look at this patch set
and see how it looks from their angle.

Mika, Rafael, Mathias, and either of you guys have a look
at this?

Yours,
Linus Walleij

Re: [RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Linus Walleij]

On Wed, Sep 4, 2013 at 9:56 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
>> Add gpiod_get() and gpiod_put() functions that provide safer handling of
>> GPIOs.
>>
>> These functions put the GPIO framework in line with the conventions of
>> other frameworks in the kernel, and help ensure every GPIO is declared
>> properly and valid while it is used.
>
>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>
>> +struct gpio_desc *__must_check gpiod_get(struct device *dev,
>> +                                      const char *con_id);
>> +void gpiod_put(struct gpio_desc *desc);
>
> It might be nice to add an "int index" parameter to this function. For
> example, a bit-banged parallel bus protocol driver might have 1
> chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
> 0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
> "bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
> implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
> gpiod_get_index(dev, con_id, index)?
>
> In DT terms, this would map to:
>
> cs-gpios = <&gpio 3 0>;
> clock-gpios = <&gpio 5 0>;
> bus-gpios = <&gpio 10 0 ... &gpio 17 0>;
>
> ... and with the mapping table registration mechanism, we could
> presumably add "int index" to struct gpiod_lookup.

This is an interesting usability aspect of the API, so I'd especially
like some input from the ACPI people on this as well.

Paging Mika, Rafael, Mathias.

Yours,
Linus Walleij

Re: [RFC 0/5] New descriptor-based GPIO interface

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

On Fri, Sep 20, 2013 at 08:06:00PM +0200, Linus Walleij wrote:
> On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> > Here is a first RFC for the new GPIO interface.
> 
> I'm quite happy with this, and given that all DT-implementations will start
> to use it from day 1 I'll happily merge it when you think it's tested enough.
> 
> On my wishlist is to also switch over the ACPI GPIO driver:
> drivers/gpio/gpiolib-acpi.c to use the descriptors directly, so
> I'd like one of the ACPI folks to have a look at this patch set
> and see how it looks from their angle.
> 
> Mika, Rafael, Mathias, and either of you guys have a look
> at this?

Given that it seems like there won't be a linux-next from September 28
until close to the next merge window, it would be good to get this into
linux-next before that so that the series can receive broader testing. I
have some patches that could use this, but I'm hesitant to base them on
these patches because I want at least part of them to go into 3.13.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 0/5] New descriptor-based GPIO interface

[Linus Walleij]

On Fri, Sep 20, 2013 at 9:32 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> Given that it seems like there won't be a linux-next from September 28
> until close to the next merge window, it would be good to get this into
> linux-next before that so that the series can receive broader testing.

Good point. Alexandre: ready when you are.

Yours,
Linus Walleij

Re: [RFC 0/5] New descriptor-based GPIO interface

[Alexandre Courbot]

On Sat, Sep 21, 2013 at 6:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Sep 20, 2013 at 9:32 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>
>> Given that it seems like there won't be a linux-next from September 28
>> until close to the next merge window, it would be good to get this into
>> linux-next before that so that the series can receive broader testing.
>
> Good point. Alexandre: ready when you are.

I really need to test this more extensively, but give me a couple of
days and I will send a real submission of it (not a RFC). Then we can
discuss the small issues that remain.

Thanks,
Alex.

Re: [RFC 1/5] gpiolib: factorize gpiod_get/set functions

[Alexandre Courbot]

On Fri, Sep 20, 2013 at 5:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
>> gpiod_get/set functions share common code between their regular and
>> cansleep variants. The exporting of the gpiod interface will make
>> the situation worse. This patch factorizes the common code to avoid code
>> redundancy.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> I don't see why this patch should be RFC?
>
> I just rebased and applied it, it's a clean and nice refactoring.

Oh yeah, I just sent it with the others, but please feel free to apply
it as-is. It should be fine.

Alex.

Re: [RFC 4/5] gpiolib: add gpiod_get() and gpiod_put() functions

[Mika Westerberg]

On Fri, Sep 20, 2013 at 08:40:48PM +0200, Linus Walleij wrote:
> On Wed, Sep 4, 2013 at 9:56 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 09/04/2013 05:29 AM, Alexandre Courbot wrote:
> >> Add gpiod_get() and gpiod_put() functions that provide safer handling of
> >> GPIOs.
> >>
> >> These functions put the GPIO framework in line with the conventions of
> >> other frameworks in the kernel, and help ensure every GPIO is declared
> >> properly and valid while it is used.
> >
> >> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> >
> >> +struct gpio_desc *__must_check gpiod_get(struct device *dev,
> >> +                                      const char *con_id);
> >> +void gpiod_put(struct gpio_desc *desc);
> >
> > It might be nice to add an "int index" parameter to this function. For
> > example, a bit-banged parallel bus protocol driver might have 1
> > chip-select GPIO, 1 clock GPIO, and 8 data GPIOs. gpiod_get(dev, "bus",
> > 0)..gpiod_get(dev, "bus", 7) might be nicer than gpiod_get(dev,
> > "bus0")..gpiod_get(dev, "bus7")? Possibly for client-simplicity,
> > implement both gpiod_get(dev, con_id) (as an inline wrapper for ...) and
> > gpiod_get_index(dev, con_id, index)?
> >
> > In DT terms, this would map to:
> >
> > cs-gpios = <&gpio 3 0>;
> > clock-gpios = <&gpio 5 0>;
> > bus-gpios = <&gpio 10 0 ... &gpio 17 0>;
> >
> > ... and with the mapping table registration mechanism, we could
> > presumably add "int index" to struct gpiod_lookup.
> 
> This is an interesting usability aspect of the API, so I'd especially
> like some input from the ACPI people on this as well.

The index fits well with ACPI as that's the only way we can use to
distinguish different GPIOs.

However, in ACPI world the con_id doesn't have any correspondence. For
example given a DT description:

	power-gpios = <&gpio 28 GPIO_ACTIVE_LOW>;

would be something like this in the ACPI ASL code:

	GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionNone,
        	"\\_SB.PCI0.GPI0", 0x00, ResourceConsumer,,)
        {   
                28
        }

I'm thinking that in order to get ACPI implementation work with this we can
do something like:

	gpiod_get_index(dev, "bus", 0)

	  Here we ignore the con_id and use only the index.

	gpiod_get(dev, "bus")

	  Since we can't map the "bus" to anything, we default to index 0.

We have pretty much similar in the DMA slave helpers where passing "tx"
gives us the first DMA channel and "rx" the second.

Re: [RFC 0/5] New descriptor-based GPIO interface

[Mika Westerberg]

On Fri, Sep 20, 2013 at 08:06:00PM +0200, Linus Walleij wrote:
> On Wed, Sep 4, 2013 at 1:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> > Here is a first RFC for the new GPIO interface.
> 
> I'm quite happy with this, and given that all DT-implementations will start
> to use it from day 1 I'll happily merge it when you think it's tested enough.
> 
> On my wishlist is to also switch over the ACPI GPIO driver:
> drivers/gpio/gpiolib-acpi.c to use the descriptors directly, so
> I'd like one of the ACPI folks to have a look at this patch set
> and see how it looks from their angle.
> 
> Mika, Rafael, Mathias, and either of you guys have a look
> at this?

Apart from the con_id thing I commented, I don't see huge problems
converting the ACPI GPIO helpers to this new interface.

One thing that is special to ACPI is that we have two kind of GPIO
"connections":

	GpioIo - a normal GPIO that the driver can toggle or read values

	GpioInt - this is specifically used as a GPIO interrupt

The drivers should be able to distinguish between the two. Currently we
have an extra parameter in acpi_get_gpio_by_index(..., &info) that can be
used for this. However, with the new gpiod_get() I'm not sure how we are
supposed to do this now?

Anyway, that shouldn't prevent merging this patch set. I'm sure that we can
come up with some working solution :-)