[PATCH] gpiolib: allow exported GPIO nodes to be named using sysfs links

[PATCH] gpiolib: allow exported GPIO nodes to be named using sysfs links

[Jani Nikula]

Commit 926b663ce8215ba448960e1ff6e58b67a2c3b99b (gpiolib: allow GPIOs to
be named) already provides naming on the chip level. This patch provides
more flexibility by allowing multiple names where ever in sysfs on a per
GPIO basis.

Adapted from David Brownell's comments on a similar concept:
http://lkml.org/lkml/2009/4/20/203.

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
---
 Documentation/gpio.txt     |   10 +++++++++
 drivers/gpio/gpiolib.c     |   48 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    8 +++++++
 include/linux/gpio.h       |    9 ++++++++
 4 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index 145c25a..decd2ef 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -555,6 +555,11 @@ requested using gpio_request():
 	/* reverse gpio_export() */
 	void gpio_unexport();
 
+	/* create a sysfs link to an exported GPIO node */
+	int gpio_export_link(struct device *dev, const char *name,
+		unsigned gpio)
+
+
 After a kernel driver requests a GPIO, it may only be made available in
 the sysfs interface by gpio_export().  The driver can control whether the
 signal direction may change.  This helps drivers prevent userspace code
@@ -563,3 +568,8 @@ from accidentally clobbering important system state.
 This explicit exporting can help with debugging (by making some kinds
 of experiments easier), or can provide an always-there interface that's
 suitable for documenting as part of a board support package.
+
+After the GPIO has been exported, gpio_export_link() allows creating
+symlinks from elsewhere in sysfs to the GPIO sysfs node.  Drivers can
+use this to provide the interface under their own device in sysfs with
+a descriptive name.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 51a8d41..860fb95 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -505,6 +505,54 @@ static int match_export(struct device *dev, void *data)
 }
 
 /**
+ * gpio_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
+ *
+ * Set up a symlink from /sys/.../dev/name to /sys/class/gpio/gpioN
+ * node. Caller is responsible for unlinking.
+ *
+ * Returns zero on success, else an error.
+ */
+int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
+{
+	struct gpio_desc	*desc;
+	int			status = -EINVAL;
+
+	BUG_ON(dev == NULL);
+	BUG_ON(name == NULL);
+
+	if (!gpio_is_valid(gpio))
+		goto done;
+
+	mutex_lock(&sysfs_lock);
+
+	desc = &gpio_desc[gpio];
+
+	if (test_bit(FLAG_EXPORT, &desc->flags)) {
+		struct device *tdev;
+
+		tdev = class_find_device(&gpio_class, NULL, desc, match_export);
+		if (tdev != NULL) {
+			status = sysfs_create_link(&dev->kobj, &tdev->kobj,
+						name);
+		} else {
+			status = -ENODEV;
+		}
+	}
+
+	mutex_unlock(&sysfs_lock);
+
+done:
+	if (status)
+		pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
+
+	return status;
+}
+EXPORT_SYMBOL_GPL(gpio_export_link);
+
+/**
  * gpio_unexport - reverse effect of gpio_export()
  * @gpio: gpio to make unavailable
  *
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6c379d..9cca378 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -141,6 +141,8 @@ extern int __gpio_to_irq(unsigned gpio);
  * 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 void gpio_unexport(unsigned gpio);
 
 #endif	/* CONFIG_GPIO_SYSFS */
@@ -185,6 +187,12 @@ 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 void gpio_unexport(unsigned gpio)
 {
 }
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index e10c49a..ff374ce 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -89,6 +89,15 @@ static inline int gpio_export(unsigned gpio, bool direction_may_change)
 	return -EINVAL;
 }
 
+static inline int gpio_export_link(struct device *dev, const char *name,
+				unsigned gpio)
+{
+	/* GPIO can never have been exported */
+	WARN_ON(1);
+	return -EINVAL;
+}
+
+
 static inline void gpio_unexport(unsigned gpio)
 {
 	/* GPIO can never have been exported */
-- 
1.6.0.4

Re: [PATCH] gpiolib: allow exported GPIO nodes to be named using sysfs links

[Andrew Morton]

On Thu, 11 Jun 2009 12:19:03 +0300
Jani Nikula <ext-jani.1.nikula@nokia.com> wrote:

> +int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
> +{
> +	struct gpio_desc	*desc;
> +	int			status = -EINVAL;
> +
> +	BUG_ON(dev == NULL);
> +	BUG_ON(name == NULL);

We usually don't bother with these assertions.  If the kernel will
reliably oops if one of these is NULL, then that provides the same
information anyway.

Re: [PATCH] gpiolib: allow exported GPIO nodes to be named using sysfs links

[David Brownell]

On Monday 22 June 2009, Andrew Morton wrote:
> On Thu, 11 Jun 2009 12:19:03 +0300
> Jani Nikula <ext-jani.1.nikula@nokia.com> wrote:
> 
> > +int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
> > +{
> > +	struct gpio_desc	*desc;
> > +	int			status = -EINVAL;
> > +
> > +	BUG_ON(dev == NULL);
> > +	BUG_ON(name == NULL);
> 
> We usually don't bother with these assertions.  If the kernel will
> reliably oops if one of these is NULL, then that provides the same
> information anyway.

Yeah, it'd be better to remove those BUG_ON() things before this goes upstream...

[PATCH v2] gpiolib: allow exported GPIO nodes to be named using sysfs links

[Jani Nikula]

Commit 926b663ce8215ba448960e1ff6e58b67a2c3b99b (gpiolib: allow GPIOs to
be named) already provides naming on the chip level. This patch provides
more flexibility by allowing multiple names where ever in sysfs on a per
GPIO basis.

Adapted from David Brownell's comments on a similar concept:
http://lkml.org/lkml/2009/4/20/203.

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>

---

Changes in v2:
 - remove BUG_ON()
---
 Documentation/gpio.txt     |   10 +++++++++
 drivers/gpio/gpiolib.c     |   45 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    8 +++++++
 include/linux/gpio.h       |    9 ++++++++
 4 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index e4b6985..566edaa 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -555,6 +555,11 @@ requested using gpio_request():
 	/* reverse gpio_export() */
 	void gpio_unexport();
 
+	/* create a sysfs link to an exported GPIO node */
+	int gpio_export_link(struct device *dev, const char *name,
+		unsigned gpio)
+
+
 After a kernel driver requests a GPIO, it may only be made available in
 the sysfs interface by gpio_export().  The driver can control whether the
 signal direction may change.  This helps drivers prevent userspace code
@@ -563,3 +568,8 @@ from accidentally clobbering important system state.
 This explicit exporting can help with debugging (by making some kinds
 of experiments easier), or can provide an always-there interface that's
 suitable for documenting as part of a board support package.
+
+After the GPIO has been exported, gpio_export_link() allows creating
+symlinks from elsewhere in sysfs to the GPIO sysfs node.  Drivers can
+use this to provide the interface under their own device in sysfs with
+a descriptive name.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 51a8d41..aef6b3d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -505,6 +505,51 @@ static int match_export(struct device *dev, void *data)
 }
 
 /**
+ * gpio_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
+ *
+ * Set up a symlink from /sys/.../dev/name to /sys/class/gpio/gpioN
+ * node. Caller is responsible for unlinking.
+ *
+ * Returns zero on success, else an error.
+ */
+int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
+{
+	struct gpio_desc	*desc;
+	int			status = -EINVAL;
+
+	if (!gpio_is_valid(gpio))
+		goto done;
+
+	mutex_lock(&sysfs_lock);
+
+	desc = &gpio_desc[gpio];
+
+	if (test_bit(FLAG_EXPORT, &desc->flags)) {
+		struct device *tdev;
+
+		tdev = class_find_device(&gpio_class, NULL, desc, match_export);
+		if (tdev != NULL) {
+			status = sysfs_create_link(&dev->kobj, &tdev->kobj,
+						name);
+		} else {
+			status = -ENODEV;
+		}
+	}
+
+	mutex_unlock(&sysfs_lock);
+
+done:
+	if (status)
+		pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
+
+	return status;
+}
+EXPORT_SYMBOL_GPL(gpio_export_link);
+
+/**
  * gpio_unexport - reverse effect of gpio_export()
  * @gpio: gpio to make unavailable
  *
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6c379d..9cca378 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -141,6 +141,8 @@ extern int __gpio_to_irq(unsigned gpio);
  * 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 void gpio_unexport(unsigned gpio);
 
 #endif	/* CONFIG_GPIO_SYSFS */
@@ -185,6 +187,12 @@ 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 void gpio_unexport(unsigned gpio)
 {
 }
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index e10c49a..ff374ce 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -89,6 +89,15 @@ static inline int gpio_export(unsigned gpio, bool direction_may_change)
 	return -EINVAL;
 }
 
+static inline int gpio_export_link(struct device *dev, const char *name,
+				unsigned gpio)
+{
+	/* GPIO can never have been exported */
+	WARN_ON(1);
+	return -EINVAL;
+}
+
+
 static inline void gpio_unexport(unsigned gpio)
 {
 	/* GPIO can never have been exported */
-- 
1.6.3.2

Re: [PATCH v2] gpiolib: allow exported GPIO nodes to be named using sysfs links

[David Brownell]

On Wednesday 01 July 2009, Jani Nikula wrote:
> Commit 926b663ce8215ba448960e1ff6e58b67a2c3b99b (gpiolib: allow GPIOs to
> be named) already provides naming on the chip level. This patch provides
> more flexibility by allowing multiple names where ever in sysfs on a per
> GPIO basis.
> 
> Adapted from David Brownell's comments on a similar concept:
> http://lkml.org/lkml/2009/4/20/203.
> 
> Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>

Acked-by: David Brownell <dbrownell@users.sourceforge.net>

Andrew, this replaces a patch now in your MM tree...

> 
> ---
> 
> Changes in v2:
>  - remove BUG_ON()
> ---
>  Documentation/gpio.txt     |   10 +++++++++
>  drivers/gpio/gpiolib.c     |   45 ++++++++++++++++++++++++++++++++++++++++++++
>  include/asm-generic/gpio.h |    8 +++++++
>  include/linux/gpio.h       |    9 ++++++++
>  4 files changed, 72 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
> index e4b6985..566edaa 100644
> --- a/Documentation/gpio.txt
> +++ b/Documentation/gpio.txt
> @@ -555,6 +555,11 @@ requested using gpio_request():
>  	/* reverse gpio_export() */
>  	void gpio_unexport();
>  
> +	/* create a sysfs link to an exported GPIO node */
> +	int gpio_export_link(struct device *dev, const char *name,
> +		unsigned gpio)
> +
> +
>  After a kernel driver requests a GPIO, it may only be made available in
>  the sysfs interface by gpio_export().  The driver can control whether the
>  signal direction may change.  This helps drivers prevent userspace code
> @@ -563,3 +568,8 @@ from accidentally clobbering important system state.
>  This explicit exporting can help with debugging (by making some kinds
>  of experiments easier), or can provide an always-there interface that's
>  suitable for documenting as part of a board support package.
> +
> +After the GPIO has been exported, gpio_export_link() allows creating
> +symlinks from elsewhere in sysfs to the GPIO sysfs node.  Drivers can
> +use this to provide the interface under their own device in sysfs with
> +a descriptive name.
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 51a8d41..aef6b3d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -505,6 +505,51 @@ static int match_export(struct device *dev, void *data)
>  }
>  
>  /**
> + * gpio_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
> + *
> + * Set up a symlink from /sys/.../dev/name to /sys/class/gpio/gpioN
> + * node. Caller is responsible for unlinking.
> + *
> + * Returns zero on success, else an error.
> + */
> +int gpio_export_link(struct device *dev, const char *name, unsigned gpio)
> +{
> +	struct gpio_desc	*desc;
> +	int			status = -EINVAL;
> +
> +	if (!gpio_is_valid(gpio))
> +		goto done;
> +
> +	mutex_lock(&sysfs_lock);
> +
> +	desc = &gpio_desc[gpio];
> +
> +	if (test_bit(FLAG_EXPORT, &desc->flags)) {
> +		struct device *tdev;
> +
> +		tdev = class_find_device(&gpio_class, NULL, desc, match_export);
> +		if (tdev != NULL) {
> +			status = sysfs_create_link(&dev->kobj, &tdev->kobj,
> +						name);
> +		} else {
> +			status = -ENODEV;
> +		}
> +	}
> +
> +	mutex_unlock(&sysfs_lock);
> +
> +done:
> +	if (status)
> +		pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
> +
> +	return status;
> +}
> +EXPORT_SYMBOL_GPL(gpio_export_link);
> +
> +/**
>   * gpio_unexport - reverse effect of gpio_export()
>   * @gpio: gpio to make unavailable
>   *
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index d6c379d..9cca378 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -141,6 +141,8 @@ extern int __gpio_to_irq(unsigned gpio);
>   * 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 void gpio_unexport(unsigned gpio);
>  
>  #endif	/* CONFIG_GPIO_SYSFS */
> @@ -185,6 +187,12 @@ 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 void gpio_unexport(unsigned gpio)
>  {
>  }
> diff --git a/include/linux/gpio.h b/include/linux/gpio.h
> index e10c49a..ff374ce 100644
> --- a/include/linux/gpio.h
> +++ b/include/linux/gpio.h
> @@ -89,6 +89,15 @@ static inline int gpio_export(unsigned gpio, bool direction_may_change)
>  	return -EINVAL;
>  }
>  
> +static inline int gpio_export_link(struct device *dev, const char *name,
> +				unsigned gpio)
> +{
> +	/* GPIO can never have been exported */
> +	WARN_ON(1);
> +	return -EINVAL;
> +}
> +
> +
>  static inline void gpio_unexport(unsigned gpio)
>  {
>  	/* GPIO can never have been exported */
> -- 
> 1.6.3.2
> 
>