gpio: sysfs interface

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N
/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low
/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including Hartley Sweeten.

Signed-off-by David Brownell <[email protected]>

---
 arch/avr32/mach-at32ap/pio.c |    2 
 drivers/gpio/Kconfig         |   16 ++
 drivers/gpio/gpiolib.c       |  281 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/mcp23s08.c      |    1 
 drivers/gpio/pca953x.c       |    1 
 drivers/gpio/pcf857x.c       |    1 
 drivers/i2c/chips/tps65010.c |    2 
 drivers/mfd/htc-egpio.c      |    2 
 include/asm-generic/gpio.h   |   28 ++++

}
EXPORT_SYMBOL_GPL(gpio_request);

drivers/gpio/gpiolib.c +6 -0

+
+#ifdef CONFIG_GPIO_SYSFS
+
+/*
+ * /sys/class/gpio/gpio-N/... only for GPIOs that are exported
+ *  - direction

drivers/gpio/gpiolib.c +22 -0

+	return (status < 0) ? status : size;
+}
+
+static const DEVICE_ATTR(direction, 0644, gpio_direction_show, gpio_direction_store);
+
+static ssize_t gpio_value_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	const struct gpio_desc	*desc = dev_get_drvdata(dev);
+	unsigned		gpio = desc - gpio_desc;
+
+	/* handle GPIOs being removed from underneath us... */
+	if (!test_bit(FLAG_EXPORT, &desc->flags))
+		return -EIO;
+
+	return sprintf(buf, "%d\n", gpio_get_value_cansleep(gpio));
+}
+
+static ssize_t gpio_value_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	const struct gpio_desc	*desc = dev_get_drvdata(dev);

drivers/gpio/gpiolib.c +26 -0

+	ret = strict_strtol(buf, 0, &value);
+	if (ret < 0)
+		return ret;
+	gpio_set_value_cansleep(gpio, value != 0);
+	return size;
+}
 
...
 
+/**
+ * gpio_export - export a GPIO through sysfs
+ * @gpio: gpio to make available, already requested
+ *
+ * When drivers want to make a GPIO accessible to userspace after they
+ * have requested it -- perhaps while debugging, or as part of their
+ * public interface -- they may use this routine.
+ *
+ * Returns zero on success, else an error.
+ */
+int gpio_export(unsigned gpio)
+{
+	unsigned long		flags;
+	struct gpio_desc	*desc;
+	int			status = -EINVAL;
+
+	if (!gpio_class)
+		return -ENOSYS;
+
+	if (!gpio_is_valid(gpio))

drivers/gpio/gpiolib.c +29 -0

+	if (status)
+		pr_debug("%s: gpio-%d status %d\n", __func__, gpio, status);
+	else {
+		struct device	*dev;
+
+		dev = device_create(gpio_class, desc->chip->dev, 0,
+				"gpio-%d", gpio);
+		if (dev) {
+			dev_set_drvdata(dev, desc);
+			status = sysfs_create_group(&dev->kobj,
+					&gpio_attr_group);
+		}
+		if (status == 0)
+			set_bit(FLAG_EXPORT, &desc->flags);
+	}
+
+	return status;
+}
+EXPORT_SYMBOL_GPL(gpio_export);
+
+static int match_export(struct device *dev, void *data)
+{
+	return dev_get_drvdata(dev) == data;
+}
+
+/**
+ * gpio_unexport - reverse effect of gpio_export()
+ * @gpio: gpio to make unavailable
+ *

drivers/gpio/gpiolib.c +24 -0

+	spin_lock_irqsave(&gpio_lock, flags);
+	desc = &gpio_desc[gpio];
+	if (test_bit(FLAG_EXPORT, &desc->flags))
+		status = 0;
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	if (status == 0) {
+		struct device	*dev = NULL;
+
+		dev = class_find_device(gpio_class, desc, match_export);
+		if (dev) {
+			clear_bit(FLAG_EXPORT, &desc->flags);
+			put_device(dev);
+			device_unregister(dev);
+		}
+	}
+
+}
+EXPORT_SYMBOL_GPL(gpio_unexport);
+
+/*
+ * /sys/class/gpio/control ... write-only
+ *	export N
+ *	unexport N

drivers/gpio/gpiolib.c +19 -0

+	tmp = strsep(&scratch, " \t\n");
+	if ((tmp && *tmp) || scratch)
+		goto fail;
+
+	if (strcmp(cmd, "export") == 0) {
+		status = gpio_request(gpio, "sysfs");
+		if (status < 0)
+			goto done;
+
+		status = gpio_export(gpio);
+		if (status < 0)
+			gpio_free(gpio);
+		else
+			set_bit(FLAG_SYSFS, &gpio_desc[gpio].flags);
+
+	} else if (strcmp(cmd, "unexport") == 0) {
+		/* reject bogus commands (gpio_unexport ignores them) */
+		if (!gpio_is_valid(gpio))
+			goto fail;

drivers/gpio/gpiolib.c +17 -0

+static CLASS_ATTR(control, 0200, NULL, control_store);
+
+static int __init gpiolib_sysfs_init(void)
+{
+	int	status;
+
+	gpio_class = class_create(THIS_MODULE, "gpio");
+	if (IS_ERR(gpio_class))
+		return PTR_ERR(gpio_class);
+
+	status = class_create_file(gpio_class, &class_attr_control);
+	if (status < 0) {
+		class_destroy(gpio_class);
+		gpio_class = NULL;
+	}
+	return status;
+}

include/asm-generic/gpio.h +12 -0

+/*
+ * 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);
+extern void gpio_unexport(unsigned gpio);
+
+#endif	/* CONFIG_GPIO_SYSFS */
+
+#else	/* !CONFIG_HAVE_GPIO_LIB */
 
 static inline int gpio_is_valid(int number)
 {

@@ -135,4 +150,15 @@ static inline void gpio_set_value_cansle
 
 #endif
 
+#ifndef HAVE_GPIO_SYSFS
+static inline int gpio_export(unsigned gpio)

Quoted 13 lines · On Mon, 28 Apr 2008 12:39:51 -0700 David Brownell <[email protected]> wrote:

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N
/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low
/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

Quoted 5 lines

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

hm, does ths sysfs one-value-per-file rule apply to writes?

Quoted 7 lines

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

		...

Documentation for the interface?

drivers/gpio/gpiolib.c done: · line 306

Quoted 2 lines

drivers/gpio/gpiolib.c +2 -0

+#include <linux/device.h>
+#include <linux/err.h>

Putting includes inside ifdefs tends to increase the risk of compilation errors.

drivers/gpio/gpiolib.c done: · line 319

Quoted 12 lines

drivers/gpio/gpiolib.c +12 -0

+static ssize_t gpio_direction_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	const struct gpio_desc	*desc = dev_get_drvdata(dev);
+
+	/* handle GPIOs being removed from underneath us... */
+	if (!test_bit(FLAG_EXPORT, &desc->flags))
+		return -EIO;
+
+	return sprintf(buf, "%s\n",
+		test_bit(FLAG_IS_OUT, &desc->flags) ? "out" : "in");
+}

What prevents FLAG_EXPORT from getting cleared just after we tested it?

iow: this looks racy.

drivers/gpio/gpiolib.c done: · line 344

Quoted 12 lines

drivers/gpio/gpiolib.c +12 -0

+	if (buf[len - 1] == '\n')
+		len--;
+
+	if (len == 4 && strncmp(buf, "high", 4) == 0)
+		status = gpio_direction_output(gpio, 1);
+
+	else if (len == 3 && (strncmp(buf, "out", 3) == 0
+			|| strncmp(buf, "low", 3) == 0))
+		status = gpio_direction_output(gpio, 0);
+
+	else if (len == 2 && strncmp(buf, "in", 2) == 0)
+		status = gpio_direction_input(gpio);

urgh.

If we had a strcmp() variant which treats a \n in the first arg as a \0 the above would become

	if (sysfs_streq(buf, "high"))
		status = gpio_direction_output(gpio, 1);
	else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
		status = gpio_direction_output(gpio, 0);
	else if (sysfs_streq(buf, "in"))
		status = gpio_direction_input(gpio);

drivers/gpio/gpiolib.c done: · line 469

Quoted 8 lines

drivers/gpio/gpiolib.c +8 -0

+void gpio_unexport(unsigned gpio)
+{
+	unsigned long		flags;
+	struct gpio_desc	*desc;
+	int			status = -EINVAL;
+
+	if (!gpio_is_valid(gpio))
+		return;

Is that a programming error?

Quoted 9 lines

drivers/gpio/gpiolib.c +9 -0

+/*
+ * /sys/class/gpio/control ... write-only
+ *	export N
+ *	unexport N
+ */
+static ssize_t control_store(struct class *class, const char *buf, size_t len)
+{
+	char *scratch = (char *)buf;
+	char *cmd, *tmp;

drivers/gpio/gpiolib.c done: · line 504

Quoted 5 lines

drivers/gpio/gpiolib.c +5 -0

+	int status;
+	unsigned long gpio;
+
+	/* export/unexport */
+	cmd = strsep(&scratch, " \t\n");

urgh. We cast away the const and then smash up the caller's const string with strsep.

Quoted 6 lines

drivers/gpio/gpiolib.c +6 -0

+	if (!cmd)
+		goto fail;
+
+	/* N */
+	tmp = strsep(&scratch, " \t\n");
+	if (!tmp)

drivers/gpio/gpiolib.c done: · line 516

Quoted 6 lines

drivers/gpio/gpiolib.c +6 -0

+		goto fail;
+	status = strict_strtoul(tmp, 0, &gpio);
+	if (status < 0)
+		goto done;
+
+	/* reject commands with garbage at end */

strict_strtoul() already does that?

Review

Quoted 1 line

(quoted diff) +1 -0

+	...

The string handling in here seems way over-engineered. All we're doing is parting "export 42" or "unexport 42". Surely it can be done better than this.

The more sysfs-friendly way would be to create separate sysfs files for the export and unexport operations, I expect.

include/asm-generic/gpio.h extern void __gpio_set_value(unsigned gp · line 111

Quoted 5 lines

...

include/asm-generic/gpio.h +2 -1

-#else
+#ifdef CONFIG_GPIO_SYSFS
+#define HAVE_GPIO_SYSFS

Can we find a way to make HAVE_GPIO_SYSFS go away? Just use CONFIG_GPIO_SYSFS?

Quoted 4 lines

(quoted diff) +0 -0

@@ -135,4 +150,15 @@ static inline void gpio_set_value_cansle
 
 #endif
 

include/asm-generic/gpio.h static inline void gpio_set_value_cansle · line 164

Quoted 12 lines

include/asm-generic/gpio.h +11 -0

+#ifndef HAVE_GPIO_SYSFS
+static inline int gpio_export(unsigned gpio)
+{
+	return -ENOSYS;
+}
+
+static inline void gpio_unexport(unsigned gpio)
+{
+}
+#endif	/* HAVE_GPIO_SYSFS */
+
 #endif /* _ASM_GENERIC_GPIO_H */

Then this can just be moved into a #else.

Quoted 9 lines

If we had a strcmp() variant which treats a \n in the first arg as a \0 the above would become

	if (sysfs_streq(buf, "high"))
		status = gpio_direction_output(gpio, 1);
	else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
		status = gpio_direction_output(gpio, 0);
	else if (sysfs_streq(buf, "in"))
		status = gpio_direction_input(gpio);

That would indeed be better. Maybe I should whip up a sysfs patch adding that, and have this depend on that patch. (I've CC'd Greg in case he has comments on that...)

Alternatively: strict_streq(), analogy to strict_strto*()?

Quoted 10 lines · On Mon, 28 Apr 2008 16:28:13 -0700 David Brownell <[email protected]> wrote:

Quoted 9 lines

If we had a strcmp() variant which treats a \n in the first arg as a \0 the above would become

	if (sysfs_streq(buf, "high"))
		status = gpio_direction_output(gpio, 1);
	else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
		status = gpio_direction_output(gpio, 0);
	else if (sysfs_streq(buf, "in"))
		status = gpio_direction_input(gpio);

Quoted 3 lines

That would indeed be better. Maybe I should whip up a sysfs patch adding that, and have this depend on that patch. (I've CC'd Greg in case he has comments on that...)

Yes, it would be a standalone patch. The sort which generates oceans of useful feedback ;) The sort which also generates hundreds of use-new-toy-to-clean-up-old-code patches for me to merge :(

Quoted 16 lines · Andrew Morton

Quoted 13 lines · On Mon, 28 Apr 2008 16:28:13 -0700 David Brownell <[email protected]> wrote:

Quoted 9 lines

If we had a strcmp() variant which treats a \n in the first arg as a \0 the above would become

	if (sysfs_streq(buf, "high"))
		status = gpio_direction_output(gpio, 1);
	else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
		status = gpio_direction_output(gpio, 0);
	else if (sysfs_streq(buf, "in"))
		status = gpio_direction_input(gpio);

That would indeed be better. Maybe I should whip up a sysfs patch adding that, and have this depend on that patch. (I've CC'd Greg in case he has comments on that...)

Quoted 3 lines

Yes, it would be a standalone patch. The sort which generates oceans of useful feedback ;) The sort which also generates hundreds of use-new-toy-to-clean-up-old-code patches for me to merge :(

Heh, sounds good to me :)

Quoted 17 lines · Greg KH

Quoted 16 lines · Andrew Morton

Quoted 13 lines · On Mon, 28 Apr 2008 16:28:13 -0700 David Brownell <[email protected]> wrote:

Quoted 9 lines

If we had a strcmp() variant which treats a \n in the first arg as a \0 the above would become

	if (sysfs_streq(buf, "high"))
		status = gpio_direction_output(gpio, 1);
	else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
		status = gpio_direction_output(gpio, 0);
	else if (sysfs_streq(buf, "in"))
		status = gpio_direction_input(gpio);

That would indeed be better. Maybe I should whip up a sysfs patch adding that, and have this depend on that patch. (I've CC'd Greg in case he has comments on that...)

Quoted 5 lines

Quoted 3 lines

Yes, it would be a standalone patch. The sort which generates oceans of useful feedback ;) The sort which also generates hundreds of use-new-toy-to-clean-up-old-code patches for me to merge :(

Heh, sounds good to me :)

Hard to say where should live, but lib/strings.c seemed fair. See the appended patch. I made it not care which string has newline termination, since caring seems very error-prone.

- Dave

========= CUT HERE
Add a new sysfs_streq() string comparison function, which ignores the trailing newlines found in sysfs inputs. By example:

	sysfs_streq("a", "b")	==> false
	sysfs_streq("a", "a")	==> true
	sysfs_streq("a", "a\n")	==> true
	sysfs_streq("a\n", "a")	==> true

This is intended to simplify parsing of sysfs inputs, letting them avoid the need to manually strip off newlines from inputs.

Signed-off-by David Brownell <[email protected]>

---
 include/linux/string.h |    2 ++
 lib/string.c           |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

include/linux/string.h +2 -0

@@ -109,5 +109,7 @@ extern void *kmemdup(const void *src, si
 extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
 
+extern bool sysfs_streq(const char *s1, const char *s2);
+
 #endif
 #endif /* _LINUX_STRING_H_ */

lib/string.c +17 -0

@@ -493,6 +493,33 @@ char *strsep(char **s, const char *ct)
 EXPORT_SYMBOL(strsep);
 #endif
 
+/**
+ * sysfs_streq - return true if strings are equal, modulo trailing newline
+ * @s1: one string
+ * @s2: another string
+ *
+ * This routine returns true iff two strings are equal, treating both
+ * NUL and newline-then-NUL as equivalent string terminations.  It's
+ * geared for use with sysfs input strings, which generally terminate
+ * with newlines but are compared against values without newlines.
+ */
+bool sysfs_streq(const char *s1, const char *s2)
+{
+	while (*s1 && *s1 == *s2) {
+		s1++;
+		s2++;
+	}
+

Quoted 13 lines

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N
/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low
/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

Quoted 5 lines

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

hm, does ths sysfs one-value-per-file rule apply to writes?

Quoted 8 lines

Quoted 6 lines

  echo "export 23" > /sys/class/gpio/control
    ... will gpio_request(23, "sysfs") and gpio_export(23); use /sys/class/gpio/gpio-23/direction to configure it. echo "unexport 23" > /sys/class/gpio/control
    ... will gpio_free(23)

hm, does ths sysfs one-value-per-file rule apply to writes?

Having seen ibm acpi interface... yes, that rule should be aplied for writes, too.

What about mkdir gpio-N to export it?

Quoted 1 line · Pavel Machek

What about mkdir gpio-N to export it?

Ugh. That would create way more complication than I want to see here. I thought about that early on, and decided that was Not The Way To Go.

- Dave

Quoted 11 lines · Pavel Machek

Quoted 8 lines

Quoted 6 lines

  echo "export 23" > /sys/class/gpio/control
    ... will gpio_request(23, "sysfs") and gpio_export(23); use /sys/class/gpio/gpio-23/direction to configure it. echo "unexport 23" > /sys/class/gpio/control
    ... will gpio_free(23)

hm, does ths sysfs one-value-per-file rule apply to writes?

Having seen ibm acpi interface... yes, that rule should be aplied for writes, too.

The latest version of this patch, which I'll post soonish, follows the suggestion to have one file for the "export" operation and another for "unexport".

Quoted 1 line

What about mkdir gpio-N to export it?

Ugh. That would create way more complication than I want to see here. I thought about that early on, and decided that was Not The Way To Go.

Quoted 7 lines

Quoted 3 lines

What about mkdir gpio-N to export it?

Ugh.  That would create way more complication than I want to see here.  I thought about that early on, and decided that was Not The Way To Go.

...but it would be consistent with configfs... and face it... doing echo > file to make a directory is seriously ugly.

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

What about mkdir gpio-N to export it?

Ugh.  That would create way more complication than I want to see here.  I thought about that early on, and decided that was Not The Way To Go.

...but it would be consistent with configfs... and face it... doing echo > file to make a directory is seriously ugly.

This isn't configfs, and I didn't happen to notice any polite way for sysfs users to intercept "mkdir", parse the relevant directory name, reject illegal names, and pre-populate the just-created directory with relevant contents.

Were you implying it should go into configfs? If so, do you maybe have an update to the patch I sent a day or so ago?

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N
/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low
/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including Hartley Sweeten.

Signed-off-by David Brownell <[email protected]>

Quoted 13 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N
/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low
/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

Quoted 5 lines

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

Righteo, so if the kernel explicitly gpio_export()s something, it won't be gpio_request()ed allowing multiple "owners" making driver debugging easier.

Most of the time though we don't want to be able to clobber the kernel's gpios

so the control file wizardry isn't so much to cope with incomplete board support, rather it's the way all regular (ie non-driver-debugging) gpio access is started..?

Or do you class any
situation where userspace needs primary gpio control as a BSP omission as there Should Be a formal in-kernel driver for it all?

Also, gpio number discovery can be done through the debugfs interface already in gpiolib

but once you've got a userspace gpio interface which
relies on gpio numbering like this that information ceases to be simple debugging and becomes pretty mission-critical. IMO it would make more sense to shuffle it in to /sys/class/gpio with all this stuff or at least offer a cut-down chip-to-range mapping in a file here.

Quoted 4 lines

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

Which is good for simplicity but makes async notification kinda tricky.

I would suggest that a lack of pin-change signalling is going to be a problem for people who've become accustomed to having it in their out-of-tree interfaces. Probably not a showstopper here but certainly something which will slow the uptake of this interface.

Quoted 8 lines · Ben Nizette

Quoted 6 lines · David Brownell

echo "export 23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "unexport 23" > /sys/class/gpio/control
	... will gpio_free(23)

Quoted 3 lines

Righteo, so if the kernel explicitly gpio_export()s something, it won't be gpio_request()ed allowing multiple "owners" making driver debugging easier.

The gpio_export() call requires someone (the caller!) to have requested the GPIO already. The "one owner" rule is not being changed.

Quoted 4 lines

Most of the time though we don't want to be able to clobber the kernel's gpios

Right. Not unless we're debugging the driver managing those GPIOs.

Quoted 3 lines

so the control file wizardry isn't so much to cope with incomplete board support, rather it's the way all regular (ie non-driver-debugging) gpio access is started..?

Well, I wouldn't call that "regular"! But then, I don't have this "use GPIOs from userspace" focus. To me, that's the exception not the rule.

Quoted 4 lines

Or do you class any
situation where userspace needs primary gpio control as a BSP omission as there Should Be a formal in-kernel driver for it all?

I suppose I'd prefer to see a formal gpio_export() call from the kernel as part of the BSP, if that's the model for how that particular board stack should be used. But I suspect there will be differing opinions on that point, especially from folk who avoid custom kernels for whatever reasons. (Like, "that binary has been qualified, this one hasn't.")

Quoted 3 lines

Also, gpio number discovery can be done through the debugfs interface already in gpiolib

... intended purely for debugging, not for use with production systems ...

Quoted 5 lines

but once you've got a userspace gpio interface which
relies on gpio numbering like this that information ceases to be simple debugging and becomes pretty mission-critical. IMO it would make more sense to shuffle it in to /sys/class/gpio with all this stuff or at least offer a cut-down chip-to-range mapping in a file here.

I don't follow what you're saying here. GPIO numbering is deeply specific to the hardware; so I'd say the relevant hardware docs are what become mission-critical. The kernel can't know when some field update has rewired a bunch of GPIOs, for example...

Trent pointed out that dynamic range assignment can make trouble, so I can see some help might be needed here. Were you suggesting something like a /sys/class/gpio/chips file with contents like

	0-15	gpio
	16-31	gpio
	32-47	gpio
	48-63	gpio
	192-207	mpuio
	208-213	tps65010

(Matching a stock OMAP 5912 OSK board, for what it's worth.)

Quoted 8 lines

Quoted 6 lines

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

Which is good for simplicity but makes async notification kinda tricky.

Sysfs attributes are supposed to be pollable. I've not done it, but fs/sysfs/file.c::sysfs_notify() looks relevant ...

Quoted 6 lines

I would suggest that a lack of pin-change signalling is going to be a problem for people who've become accustomed to having it in their out-of-tree interfaces. Probably not a showstopper here but certainly something which will slow the uptake of this interface.

We accept patches. Even patches on top of patches. ;)

- Dave

Ah well we're backwards there, though now I think of it I can't think of a great many valid use-cases on my side. Just for funzies I'll post on the avrfreaks AVR32 support forum and see how many I can actually dig up.

Quoted 11 lines

Trent pointed out that dynamic range assignment can make trouble, so I can see some help might be needed here. Were you suggesting something like a /sys/class/gpio/chips file with contents like

	0-15	gpio
	16-31	gpio
	32-47	gpio
	48-63	gpio
	192-207	mpuio
	208-213	tps65010

Quoted 1 line

(Matching a stock OMAP 5912 OSK board, for what it's worth.)

Yeah that's the kind of a thing. Would be well worth having that info especially for dynamically allocated chip bases.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

Which is good for simplicity but makes async notification kinda tricky.

Sysfs attributes are supposed to be pollable. I've not done it, but fs/sysfs/file.c::sysfs_notify() looks relevant ...

Right, that'll work.

Quoted 5 lines · Ben Nizette

Ah well we're backwards there, though now I think of it I can't think of a great many valid use-cases on my side. Just for funzies I'll post on the avrfreaks AVR32 support forum and see how many I can actually dig up.

Use cases would always help clarify things. I've seen just enough to make me understand this is a useful feature, and for more reasons than just "feature equality" letting us obsolete three drivers/i2c/chips/*.c drivers and help vanish half a dozen (at least!) out-of-tree drivers doing that.

The Gumstix user forums and wiki may help too. ISTR they have such a GPIO widget (maybe that's the one I saw which supports polling?) and have shipped it for ages ... so they will surely have some (PXA-specific) examples lurking.

Quoted 15 lines

Quoted 14 lines

Trent pointed out that dynamic range assignment can make trouble, so I can see some help might be needed here. Were you suggesting something like a /sys/class/gpio/chips file with contents like

	0-15	gpio
	16-31	gpio
	32-47	gpio
	48-63	gpio
	192-207	mpuio
	208-213	tps65010

(Matching a stock OMAP 5912 OSK board, for what it's worth.)

Quoted 2 lines

Yeah that's the kind of a thing. Would be well worth having that info especially for dynamically allocated chip bases.

I'd have no problem with that. Some people surely would though; it has more than one value in that file! OMG, it's readable! We can't have any of that!! The Earth will turn in its grave! And Slashdot will be decorated in Pink! Teh End Daze arrive! :)

Quoted 12 lines

Quoted 11 lines

Quoted 8 lines

Quoted 6 lines

The D-space footprint is negligible, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about half of the current size of gpiolib. No /dev node creation involved, and no "udev" support is needed.

Which is good for simplicity but makes async notification kinda tricky.

Sysfs attributes are supposed to be pollable. I've not done it, but fs/sysfs/file.c::sysfs_notify() looks relevant ...

Quoted 1 line

Right, that'll work.

OK. In that case, I think I should plan to rename the "direction" attribute as "configuration" or something a bit broader ... so that writing "irq" (or maybe "rising", "falling", "bothedges", "poll") would eventually configure it as an input with an IRQ handler.

Whenever someone contributes such an async notification scheme, that is. ;)

- Dave

Quoted 10 lines

Quoted 5 lines

Quoted 3 lines

Sysfs attributes are supposed to be pollable.  I've not done it, but fs/sysfs/file.c::sysfs_notify() looks relevant ...

Right, that'll work.

OK.  In that case, I think I should plan to rename the "direction" attribute as "configuration" or something a bit broader ... so that writing "irq" (or maybe "rising", "falling", "bothedges", "poll") would eventually configure it as an input with an IRQ handler.

Good plan, unless you'd prefer to see "direction" and "interrupt" config separate.  I have no real preference but IMO
echo "falling" > interrupt
makes more immediate sense than
echo "falling" > configuration

Quoted 3 lines

Whenever someone contributes such an async notification scheme, that is.  ;)

;)

Quoted 5 lines · Ben Nizette

Ah well we're backwards there, though now I think of it I can't think of a great many valid use-cases on my side. Just for funzies I'll post on the avrfreaks AVR32 support forum and see how many I can actually dig up.

Use cases would always help clarify things. I've seen just enough to make me understand this is a useful feature, and for more reasons than just "feature equality" letting us obsolete three drivers/i2c/chips/*.c drivers and help vanish half a dozen (at least!) out-of-tree drivers doing that.

Quoted 9 lines

Quoted 8 lines

	0-15	gpio
	16-31	gpio
	32-47	gpio
	48-63	gpio
	192-207	mpuio
	208-213	tps65010

(Matching a stock OMAP 5912 OSK board, for what it's worth.)

Quoted 2 lines

Yeah that's the kind of a thing. Would be well worth having that info especially for dynamically allocated chip bases.

I'd have no problem with that. Some people surely would though; it has more than one value in that file! OMG, it's readable! We can't have any of that!! The Earth will turn in its grave! And Slashdot will be decorated in Pink! Teh End Daze arrive! :)

OK. In that case, I think I should plan to rename the "direction" attribute as "configuration" or something a bit broader ... so that writing "irq" (or maybe "rising", "falling", "bothedges", "poll") would eventually configure it as an input with an IRQ handler.

We have a flash chip with a hardware write protected boot block controlled by a gpio.  If we want to flash this block, we need a way to change the gpio. patching the mtd driver to do this automatically would require maintaining an out-of-tree patch, I like to avoid those.  We'd also rather mtd didn't automatically un-write-protect the boot block; kinda defeats the purpose.

The device may have a daughtercard installed in it.  There is a gpio used as a presence detect.  We want to be able, from userspace (any maybe kernel too), to print out "card installed" or "no card installed".  There is also certain stuff that should run if the card is present when the machine boots.

We have some PCA9557 I2C gpio expanders that encode a device version number. We want to print this number out in userspace (e.g.  show it in the web interface, various other application specific interfaces, etc.).  Maybe the kernel will need to know too, we'll see what happens when there is a version two.  The daughter card also has a PCA9557 expander, but of course it might not be connected, the pca9557 driver can probe the bus for this.

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N

/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low

/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including Hartley Sweeten.

drivers/gpio/gpiolib.c +4 -0

+		status = gpio_export(gpio);
+		if (status < 0)
+			gpio_free(gpio);
+		else

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N

I liked it better they way I had it, "label:N".

When you have multiple GPIO sources, it's a lot easier to see where they are comming from if they use the chip label. Especially if support for dynamic allocation of gpio numbers is written.

Quoted 2 lines

/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low

You took away the code for the label field? That was one of the features of my code that Ben Nizette mentioned as an advantage over a char-device interface.

Quoted 6 lines

/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

Why can't all gpios appear read-only in sysfs by default?

Quoted 5 lines

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

I don't see what's wrong with having devices add to gpiolib create a device

for the gpio's to be the children of. You said that some devices can't do this, but I don't see the difficulty.

platform_device_register_simple("my-gpio", 0, NULL, 0);

How hard is that?

Quoted 2 lines

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including Hartley Sweeten.

I don't recall seeing those comments. Where were they posted?

Quoted 10 lines

drivers/gpio/gpiolib.c +10 -0

+		status = gpio_export(gpio);
+		if (status < 0)
+			gpio_free(gpio);
+		else
+			set_bit(FLAG_SYSFS, &gpio_desc[gpio].flags);
+
+	} else if (strcmp(cmd, "unexport") == 0) {
+		/* reject bogus commands (gpio_unexport ignores them) */
+		if (!gpio_is_valid(gpio))
+			goto fail;

drivers/gpio/gpiolib.c done: · line 539

Quoted 4 lines

drivers/gpio/gpiolib.c +4 -0

+		if (!test_and_clear_bit(FLAG_SYSFS, &gpio_desc[gpio].flags))
+			goto fail;
+
+		gpio_free(gpio);

Maybe you could simplify the text parsing by having positive gpio numbers export the gpio and negative numbers un-export the gpio? Then there would not be any need to parse a command with arguments.

Quoted 7 lines · Trent Piepho

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N

I liked it better they way I had it, "label:N".

Those labels may not be available though; or valid in pathnames.

Quoted 6 lines

Quoted 2 lines

/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low

You took away the code for the label field? That was one of the features of my code that Ben Nizette mentioned as an advantage over a char-device interface.

Since it's not always available, I removed it. Note that you're talking about the label passed to gpio_request() here, not the one applied to the gpio_chip. I could restore this as a "gpio_label" attribute, that's not always present ... but I'd rather not have such "optional" stuff.

Quoted 7 lines

Quoted 6 lines

/control ... to request a GPIO be exported or unexported

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be exported by writing to the sysfs control file, helping to cope with incomplete board support:

Quoted 1 line

Why can't all gpios appear read-only in sysfs by default?

Typical SOC based systems have up to a few hundred pins that could be used as GPIOs. The number actually used as GPIOs is rarely more than a dozen or two, with other pins generally muxed to a peripheral

There's no point in having a few hundred useless nodes in sysfs!

Quoted 6 lines

Quoted 5 lines

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

Quoted 1 line

I don't see what's wrong with having devices add to gpiolib create a device

It can't know where in the device tree to put such device nodes, or how to implement operations like suspend() and resume() for such nodes. Creating such nodes, and their drivers, is NOT a role for generic code like gpiolib.

Quoted 6 lines

for the gpio's to be the children of. You said that some devices can't do this, but I don't see the difficulty.

platform_device_register_simple("my-gpio", 0, NULL, 0);

How hard is that?

Most GPIOs come from platform code that doesn't create such a device today. In cases like OMAP, GPIOs are usable even before memory allocations can work, so creating and using device nodes for GPIO controllers would involve updating code running VERY early in boot...

You're free to write patches creating such device nodes, and work with the platform maintainers to convert their GPIO support to use standard driver model devices and drivers, then merge the results.

Quoted 4 lines

Quoted 2 lines

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including Hartley Sweeten.

I don't recall seeing those comments. Where were they posted?

Some were on-list, some were off-list. The comments have been coming off and on for a few years now, so I'm certain you would not have seen all of them. (I wouldn't have, either!)

Quoted 10 lines · David Brownell

Quoted 7 lines · Trent Piepho

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N

I liked it better they way I had it, "label:N".

Those labels may not be available though; or valid in pathnames.

So just fall back to "gpio" if there is no label? The only character that's not valid in a pathname is '/', so that's trivial to check for.

const char *label = chip->label && !strchr(chip->label, '/') ?
chip->label : "gpio"; /* or "generic" or "unknown", or ...*/

This means you don't need a file with number to device assignents. It makes shell scripting a lot easier too. Say I want the first gpio on a pca9557 gpio expander? It's will be something like: /sys/class/gpio/pca9557-0:0

You don't have to worry about dynamic assigments. You don't have to resort to convoluted shell script code to extract the proper range from a mapping file and then construct the name.

Quoted 12 lines · Trent Piepho

Quoted 4 lines · David Brownell

Quoted 1 line · Trent Piepho

I liked it better they way I had it, "label:N".

Those labels may not be available though; or valid in pathnames.

So just fall back to "gpio" if there is no label? The only character that's not valid in a pathname is '/', so that's trivial to check for.

const char *label = chip->label && !strchr(chip->label, '/') ?
chip->label : "gpio"; /* or "generic" or "unknown", or ...*/

Quoted 7 lines

This means you don't need a file with number to device assignents. It makes shell scripting a lot easier too. Say I want the first gpio on a pca9557 gpio expander? It's will be something like: /sys/class/gpio/pca9557-0:0

You don't have to worry about dynamic assigments. You don't have to resort to convoluted shell script code to extract the proper range from a mapping file and then construct the name.

Sorry if I'm being dense; how do you want this bit to work? As I see it, there are a few options:

1) Have the files named as you suggest and all of them always present, albeit read-only until export. Very easy to use, easy to discover which file is which, a decent bit of memory usage having them all listed.

3) Have the files named as you suggest, explicit export/request but better parsing behind the control file so something like echo "export pca9557-0:5" > control
works. Very very nice for the user, big heavy back end.

Quoted 6 lines

Sorry if I'm being dense; how do you want this bit to work?  As I see it, there are a few options:

1) Have the files named as you suggest and all of them always present, albeit read-only until export.  Very easy to use, easy to discover which file is which, a decent bit of memory usage having them all listed.

Well, is it really that much?  There are 579 files under /sys/class/tty.  But suppose it is too much (why isn't tty too much then?), then we can do 3.

Quoted 4 lines

3) Have the files named as you suggest, explicit export/request but better parsing behind the control file so something like echo "export pca9557-0:5" > control
works.  Very very nice for the user, big heavy back end.

The back end doesn't seem that big to me.  Here's code for it.

If anything,
the parsing code is simpler than what David has.

David's code for parsing the control file plus code for generating a mapping range file would certainly be larger.

Quoted 3 lines

If anything,
the parsing code is simpler than what David has.

Apples vs oranges. Use the same command syntax if you're going to make comparisons; I can save even more with "+export/-unexport" syntax. For comparable syntax, your stuff *IS* bigger.

Quoted 2 lines

David's code for parsing the control file plus code for generating a mapping range file would certainly be larger.

The #3 option presumes some file listing chips and ranges too, since GPIOs are exported only on demand. Ditto #2 and #4...

Quoted 7 lines · David Brownell

Quoted 3 lines

If anything,
the parsing code is simpler than what David has.

Apples vs oranges. Use the same command syntax if you're going to make comparisons; I can save even more with "+export/-unexport" syntax. For comparable syntax, your stuff *IS* bigger.

If the code you wrote it not too complex, then why is the code I wrote, which is not larger, too complex?

Quoted 5 lines

Quoted 2 lines

David's code for parsing the control file plus code for generating a mapping range file would certainly be larger.

The #3 option presumes some file listing chips and ranges too, since GPIOs are exported only on demand. Ditto #2 and #4...

You never answered how one was supposed to get the proper device from a script.

4) Status quo.  Easy, efficient, potentially hard to discover which gpio you actually want.

My vote's for 1 or 4.  The first one is heavier but easier.  The last one will need something like the discussed file mapping ranges to gpios.

Quoted 5 lines · Ben Nizette

4) Status quo. Easy, efficient, potentially hard to discover which gpio you actually want.

My vote's for 1 or 4. The first one is heavier but easier. The last one will need something like the discussed file mapping ranges to gpios.

My vote is for #4 with a chip listing file.

I don't like the hacked names ... none of the other /sys/class/*/name files on any of my systems use hacked names. The entire motivation for name hacking seems wrong to me, and by observation it's been rejected for all other class names.

- Dave

I guess one last option (which is made hard by chip label non-uniqueness but I'll throw out anyway) would be

/sys/class/gpio
        /chipa
                /gpio-n
                        /value
                        /direction
                /control
        /chipb
                :
                :

Quoted 4 lines · Ben Nizette

I guess one last option (which is made hard by chip label non-uniqueness but I'll throw out anyway) would be

/sys/class/gpio

Quoted 9 lines

        /chipa
                /gpio-n
                        /value
                        /direction
                /control
        /chipb
                :
                :

Or maybe:

/sys/class/gpio

	/gpiochip-X			<-- range X..(X+ngpio)
		/device			<-- symlink, if it's known
		/ngpio
		/label
		/start			<-- maybe; start == X

with the gpio-N links probably going where you showed. That'd be best in terms of Purity Of Essence.

Quoted 18 lines · Ben Nizette

Quoted 4 lines

I guess one last option (which is made hard by chip label non-uniqueness but I'll throw out anyway) would be

/sys/class/gpio

Quoted 9 lines

        /chipa
                /gpio-n
                        /value
                        /direction
                /control
        /chipb
                :
                :

Or maybe:

/sys/class/gpio

Quoted 8 lines

	/gpiochip-X			<-- range X..(X+ngpio)
		/device			<-- symlink, if it's known
		/ngpio
		/label
		/start			<-- maybe; start == X

with the gpio-N links probably going where you showed. That'd be best in terms of Purity Of Essence.

So you're suggesting that the gpio-N links and control file live inside the gpiochip-X folder along with info about the chip to which they're attached? I don't mind this, sounds good. Certainly feels most sysfsish.

Scripting would be pretty simple assuming there's one control file per chip and the gpio number written to said control file is relative to that chip's base. i.e. finding pcf9557:5 (assuming only one such device) would just be

- find the gpiochip-X folder whose /label == pcf9557
- echo "export 5" > <that_folder>/control
- read/write <that_folder>/gpio-5/{value,direction}

Quoted 6 lines · Ben Nizette

So you're suggesting that the gpio-N links and control file live inside the gpiochip-X folder along with info about the chip to which they're attached? I don't mind this, sounds good. Certainly feels most sysfsish.

Quoted 8 lines

Scripting would be pretty simple assuming there's one control file per chip and the gpio number written to said control file is relative to that chip's base. i.e. finding pcf9557:5 (assuming only one such device) would just be

- find the gpiochip-X folder whose /label == pcf9557
- echo "export 5" > <that_folder>/control
- read/write <that_folder>/gpio-5/{value,direction}

I don't suppose you could actually write the code to do this?

I already wrote the code, and am using it, for the way I have it work.

cat /sys/class/gpio/pcf9557-0:5

But I guess this is too easy. "We can't have any of that!! The Earth will turn in its grave! And Slashdot will be decorated in Pink! Teh End Daze arrive! :)"

Quoted 15 lines · David Brownell

Quoted 13 lines · Ben Nizette

I guess one last option (which is made hard by chip label non-uniqueness but I'll throw out anyway) would be

/sys/class/gpio

        /chipa
                /gpio-n
                        /value
                        /direction
                /control
        /chipb
                :
                :

Quoted 4 lines

Or maybe:

/sys/class/gpio

	/gpiochip-X			<-- range X..(X+ngpio)

Quoted 5 lines

		/device			<-- symlink, if it's known
		/ngpio
		/label
		/start			<-- maybe; start == X

Quoted 2 lines

with the gpio-N links probably going where you showed. That'd be best in terms of Purity Of Essence.

So you're suggesting that the gpio-N links and control file live inside the gpiochip-X folder along with info about the chip to which they're attached? I don't mind this, sounds good. Certainly feels most sysfsish.

Simple sysfs interface for GPIOs.

/sys/class/gpio
/control ... to request a GPIO be imported or returned
/gpioN ... for each exported GPIO #N
/value ... always readable, writes fail for input GPIOs /direction ... r/w as: in, out (default low); write high, low
/gpiochipN ... for each gpiochip; #N is its first GPIO
/base ... (r/o) same as N
/label ... (r/o) descriptive, not necessarily unique /ngpio ... (r/o) number of GPIOs; numbered N .. N+(ngpio - 1)

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they

be imported by writing to the sysfs control file, helping to cope with incomplete board support:

echo "23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use

/sys/class/gpio/gpio-23/direction to configure it.
echo "-23" > /sys/class/gpio/control

	... will gpio_free(23)

Quoted 11 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/control ... to request a GPIO be imported or returned
/gpioN ... for each exported GPIO #N
/value ... always readable, writes fail for input GPIOs /direction ... r/w as: in, out (default low); write high, low
/gpiochipN ... for each gpiochip; #N is its first GPIO
/base ... (r/o) same as N
/label ... (r/o) descriptive, not necessarily unique /ngpio ... (r/o) number of GPIOs; numbered N .. N+(ngpio - 1)

"simple"? What I had was a lot simpler.

So, I want to set gpio 5 on a pcf9557 extender.

cat 1 > /sys/class/gpio/pcf9557-0:0

But now I can't do this anymore, it has to be harder. So how do I do it?

It
doesn't seem very considerate to ignore the real use cases of the person who wrote the code to begin with.

Quoted 2 lines

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they

I have a JTAG interface implemented via GPIOs. With my system, one can see the gpios used in sysfs with the proper labels (TCK, TDO, TDI, etc.). This is very helpful for people connecting something to the interface to know they have the write gpio lines connected. What's the point of allowing one to label gpio lines if it's not going to be easy to see?

It also means that if they have trouble getting what their connecting to work, they can always control the line via sysfs and see with a probe that it changes. They can raise TRST (which they know is the right line from the label) and see the system reset.

Adding gpio_export() calls to the kernel source and recompiling and re-flashing just isn't going to happen for a large portion of users.

Why can't the gpio lines just show up when something requests them?

Quoted 6 lines

be imported by writing to the sysfs control file, helping to cope with incomplete board support:

echo "23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use

So if a driver is already using gpio 23, then there is no way to see it in sysfs, since the gpio_request() will fail?

Quoted 4 lines

/sys/class/gpio/gpio-23/direction to configure it.
echo "-23" > /sys/class/gpio/control

	... will gpio_free(23)

So if a driver was already using gpio 23 and you wanted to look at it from userspace, you'll free it from both sysfs and the driver that was using it when you're done?

Documentation/gpio.txt becomes available. That may mean the de · line 529

Quoted 4 lines

Documentation/gpio.txt +3 -0

+When a kernel driver has requested a GPIO, it may only be made available
+in the sysfs interface by gpio_export().  This prevents userspace code
+from clobbering important state.

You could say the same about 90% of the writable files in sysfs.

Quoted 4 lines

 This is
very helpful for people connecting something to the interface to know they have the write gpio lines connected.  What's the point of allowing one to label gpio lines if it's not going to be easy to see?

That label was always just supposed to be a debugging aid, i.e. something to show up in debugfs.  This is used to reduce D footprint. Maybe if the labels are being stored anyway they can be made available through sysfs as well as debugfs?

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

   /sys/class/gpio
   	...

"simple"? What I had was a lot simpler.

So, I want to set gpio 5 on a pcf9557 extender.

cat 1 > /sys/class/gpio/pcf9557-0:0

But now I can't do this anymore,

it has to be harder. So how do I do it? It doesn't seem very considerate to ignore the real use cases

of the person who wrote the code to begin with.

What's the point of allowing
one to label gpio lines if it's not going to be easy to see?

Quoted 2 lines

echo "23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use

So if a driver is already using gpio 23, then there is no way to see it in sysfs, since the gpio_request() will fail?

Quoted 3 lines

/sys/class/gpio/gpio-23/direction to configure it.
echo "-23" > /sys/class/gpio/control

	... will gpio_free(23)

So if a driver was already using gpio 23 and you wanted to look at it from userspace, you'll free it from both sysfs and the driver that was using it when you're done?

Quoted 6 lines · Trent Piepho

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

   /sys/class/gpio
   	...

Quoted 1 line

"simple"? What I had was a lot simpler.

But it had some "must fix" problems, which I told you when you posted your first patch. You didn't fix them. And since then, your pushback seems to rely very heavily on ignoring issues I pointed out are "must fix" or "can't/doesn't work that way".

Quoted 1 line

So, I want to set gpio 5 on a pcf9557 extender.

Which isn't exactly where most folk start. If it's something

done routinely, they'll likely have a script to run; either the native language type, or something for bash or Python.

Quoted 3 lines

cat 1 > /sys/class/gpio/pcf9557-0:0

But now I can't do this anymore,

"Any more"? You never *could* do that before. The original patch you sent didn't do that, and the last code snippet I saw from you didn't either.

Quoted 2 lines

What's the point of allowing
one to label gpio lines if it's not going to be easy to see?

You can "cat /sys/kernel/debug/gpio" to see them...

Quoted 5 lines

Quoted 2 lines

echo "23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use

So if a driver is already using gpio 23, then there is no way to see it in sysfs, since the gpio_request() will fail?

Right. If that driver wanted to cope with userspace mucking around with its internal state (GPIOs) it would have explicitly enabled it by calling gpio_export(). If it's not prepared to cope with the various flavors of breakage that would facilitate, it doesn't call gpio_export() ... and is safe.

Quoted 9 lines

Quoted 4 lines

Quoted 2 lines

   /sys/class/gpio
   	...

"simple"? What I had was a lot simpler.

But it had some "must fix" problems, which I told you when you posted your first patch. You didn't fix them. And since then, your pushback seems to rely very heavily on ignoring issues I pointed out are "must fix" or "can't/doesn't work that way".

So what are these must fix problems?

You say nothing in sysfs works this way, but I don't agree. Take a look at /sys/class/net, you have names like "tap0", "eth0", "eth1", "lo", etc. This is exactly what I'm saying to do. The first "eth" gpio chip you register is appears as eth0 in sysfs, the second as eth1, and so on.

Quoted 3 lines

Quoted 1 line

So, I want to set gpio 5 on a pcf9557 extender.

Which isn't exactly where most folk start. If it's something

It's where I started.

You say your system works for everything that matters
and just dismiss the problems I'm solving as irrelevant with a wave of your hand. That's very convenient for you, but where does it leave me?

Your worried about the memory usage of some extra sysfs files, but include python on your device? IMHO, if you can't do this simple task in 5 minutes with just busybox, the system is making things too hard.

What is the reason to not have the label with gpio in sysfs?

How does seeing the value, direction, and label of a gpio "muck around with its internal state?"

Of course, one can still change a gpio via /dev/mem or i2c-dev, depending on the source. How is being able to do this via sysfs any different?

Quoted 10 lines · Trent Piepho

Quoted 9 lines

Quoted 4 lines

Quoted 2 lines

   /sys/class/gpio
   	...

"simple"? What I had was a lot simpler.

But it had some "must fix" problems, which I told you when you posted your first patch. You didn't fix them. And since then, your pushback seems to rely very heavily on ignoring issues I pointed out are "must fix" or "can't/doesn't work that way".

Quoted 1 line

So what are these must fix problems?

Not fixed in any update you sent to your original patch, in the past three weeks, that's the key point. In particular,

the number one issue was completely ignored. (Your recent responses haven't come across to me as helpful either..)

Quoted 4 lines

You say nothing in sysfs works this way, but I don't agree. Take a look at /sys/class/net, you have names like "tap0", "eth0", "eth1", "lo", etc. This is exactly what I'm saying to do. The first "eth" gpio chip you register is appears as eth0 in sysfs, the second as eth1, and so on.

So, when I said that sysfs doesn't use names like "gpio-19" but instead uses names like "gpio19" ... exactly what don't you agree with?

Quoted 3 lines

You say your system works for everything that matters
and just dismiss the problems I'm solving as irrelevant with a wave of your hand. That's very convenient for you, but where does it leave me?

Those are *NOT* my words. Again, I don't know what straw man you

Quoted 3 lines

Your worried about the memory usage of some extra sysfs files, but include python on your device? IMHO, if you can't do this simple task in 5 minutes with just busybox, the system is making things too hard.

Right, and since I *can* do stuff like that in busybox with ASH, that quickly, I hope you'll agree your flamage has been blowing things way out of proportion here.

Quoted 3 lines

What is the reason to not have the label with gpio in sysfs?

Elaborated in my reply to Ben; see that.

Quoted 2 lines

How does seeing the value, direction, and label of a gpio "muck around with its internal state?"

Userspace being able to MODIFY the direction and output value is most certainly mucking around!!!

Quoted 2 lines

Of course, one can still change a gpio via /dev/mem or i2c-dev, depending on the source. How is being able to do this via sysfs any different?

It's a question of how easy you make it to break things. You seem to draw the "too easy" line differently than I do. I don't want *anything* else mucking around with state my drivers are responsible for managing.

I've proposed many times that gpios that have been requested by anything be exported READ-ONLY automatically.

Quoted 6 lines

Quoted 2 lines

IMHO, if you can't do this simple task in 5 minutes
with just busybox, the system is making things too hard.

Right, and since I *can* do stuff like that in busybox with ASH, that quickly, I hope you'll agree your flamage has been blowing things way out of proportion here.

It's so easy, yet no one posts the code to do it.

Simple sysfs interface for GPIOs.

/sys/class/gpio
/control ... to request a GPIO be imported or returned /gpioN ... for each exported GPIO #N
/value ... always readable, writes fail for input GPIOs /direction ... r/w as: in, out (default low); write high, low
/gpiochipN ... for each gpiochip; #N is its first GPIO
/base ... (r/o) same as N
/label ... (r/o) descriptive, not necessarily unique /ngpio ... (r/o) number of GPIOs; numbered N .. N+(ngpio - 1)

GPIOs may be exported by kernel code using gpio_export(), which should be most useful for driver debugging. Userspace may also ask that they be imported by writing to the sysfs control file, helping to cope with incomplete board support:

echo "23" > /sys/class/gpio/control

	... will gpio_request(23, "sysfs") and gpio_export(23); use
	/sys/class/gpio/gpio-23/direction to configure it.
  echo "-23" > /sys/class/gpio/control
	... will gpio_free(23)

The extra D-space footprint is a few hundred bytes, except for the sysfs resources associated with each exported GPIO. The additional I-space footprint is about two thirds of the current size of gpiolib (!). Since no /dev node creation is involved, no "udev" support is needed.

This adds a device pointer to "struct gpio_chip". When GPIO providers initialize that, sysfs gpio class devices become children of that device instead of being "virtual" devices. The (few) gpio_chip providers which have such a device node have been updated. (Some also needed to update their module "owner" field ... for which missing kerneldoc was added.)

Based on a patch from Trent Piepho <[email protected]>, and comments from various folk including most recently Hartley Sweeten and Ben Nizette.

Signed-off-by David Brownell <[email protected]>

---
Notable updates from previous version: doc update, mutex protection for sysfs access, export basic gpio_chip info ("what GPIOs exist"), depend on new sysfs_streq() call, simplified control file syntax.

drivers/gpio/gpiolib.c +21 -0

+
+/*
+ * /sys/class/gpio/control ... write-only
+ *	integer N:  non-negative == export; negative == unexport
+ */
+static ssize_t control_store(struct class *class, const char *buf, size_t len)
+{
+	long	gpio;
+	int	status;
+
+	status = strict_strtol(buf, 0, &gpio);
+	if (status < 0)
+		goto done;
+
+	/* No extra locking here; FLAG_SYSFS just signifies that the
+	 * request and export were done by on behalf of userspace, so
+	 * they may be undone on its behalf too.
+	 */
+
+	if (gpio >= 0) {			/* export */
+		status = gpio_request(gpio, "sysfs");

drivers/gpio/gpiolib.c +19 -0

+		gpio = -gpio;
+
+		/* reject bogus commands (gpio_unexport ignores them) */
+		if (!gpio_is_valid(gpio))
+			goto fail;
+		if (!test_and_clear_bit(FLAG_SYSFS, &gpio_desc[gpio].flags))
+			goto fail;
+		status = 0;
+		gpio_free(gpio);
+	}
+done:
+	if (status)
+		pr_debug("%s: status %d\n", __func__, status);
+	return status ? : len;
+fail:
+	pr_debug("%s: fail\n", __func__);
+	return -EINVAL;
+}
+

Quoted 11 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/control ... to request a GPIO be imported or returned /gpioN ... for each exported GPIO #N
/value ... always readable, writes fail for input GPIOs /direction ... r/w as: in, out (default low); write high, low
/gpiochipN ... for each gpiochip; #N is its first GPIO
/base ... (r/o) same as N
/label ... (r/o) descriptive, not necessarily unique /ngpio ... (r/o) number of GPIOs; numbered N .. N+(ngpio - 1)

You dropped the 'device' symlink? Sure it won't always be available but I did quite like the idea of being able to walk back through sysfs and discover, for example, the SPI chip select to which it's attached.

drivers/gpio/gpiolib.c err: · line 333

Quoted 5 lines

drivers/gpio/gpiolib.c +5 -0

+/*
+ * /sys/class/gpio/control ... write-only
+ *	integer N:  non-negative == export; negative == unexport
+ */
+static ssize_t control_store(struct class *class, const char *buf, size_t len)

Unexport gpio 0?

Quoted 4 lines · David Brownell

Simple sysfs interface for GPIOs.

/sys/class/gpio
/gpio-N ... for each exported GPIO #N

I liked it better they way I had it, "label:N".

When you have multiple GPIO sources, it's a lot easier to see where they are comming from if they use the chip label. Especially if support for dynamic allocation of gpio numbers is written.

Quoted 2 lines

/value ... always readable, writes fail except for output GPIOs /direction ... writable as: in, out (default low), high, low

You took away the code for the label field? That was one of the features of my code that Ben Nizette mentioned as an advantage over a char-device interface.