[RFC - patch] add a gpio-sysfs interface - was: Proposal: common kernel-wide GPIO interface

Jim Cromie wrote:

nsc_gpio, pc8736x_gpio , scx200_gpio went thru mm into mainline-rc -they support the legacy gpio-bitaccess via char-device-file. They expose port-wide read/write insidethe kernel,

correction - in-vtable portwide-accessors are in a to-be-applied patchfrom Chris Boot to me.Its in-my-queue for post 18, since we're now rc.


Im attaching a newer version of a patch sent earlier this thread.

It adds a sysfs-gpio interface, for both bits & ports.
- same feature-set for both.
- tried port-synthesis -
   for pin-features that are implemented as per-bit only,
   do set of queries/updates, and bundle them for presentation.
   (having explored this, Im hedging ..)

the set of pin_* and port_*  attributes are supported by 8 sysfs-callbacks,

split on following axes: bit/port, value/config, read/write (always,diff fn-sigs).

*_value callbacks are simple/fast
   both my gpio chips are port-based, bit-wide methods add masking
*_conf callbacks handle many attributes
_bit_conf - natural match - fetches bit, returns bit
_port_conf - synthesize port from bits

The 8 callbacks are mapped into place by 2 macros in nsc_gpio.h,

one each for bits/ports. BUG: the mapping is inelegant. see code formore comments.



WRT port-synthesis from bit-wide properties

- different chips expose different properties as bit-vectors
   pc87366 gives data-in, data-out-rd/wr only,
   sc-1100  also shows interrupt-enable, gpio-event-status as bit-vectors

- port-synthesis lets us hide the difference, if done correctly.
    BUG : Due to the macro design (built for static decl & initialization),
   its cumbersome to do the mapping.

- alternative (and simpler) approach is to :
   selectively create attr-files, thus exposing only the features desired
   let user-space figure it out

this is all restated in patch comments.


Patch does nothing wrt reservations or class_dev design,

Chris, Im hoping you're focussing here, it would be really cool if mycallbacks

were to just drop right into gpio class impl ;-)



The following cut-pastes demonstrate some level of working-ness,
1 - pin reads via char-device-files
2 - same pins, read as sys-attr files,  bit/port (values)
3 - demonstrates bitpos=1 exposing bit-in-port (values)

4 - config-features work - bit-read/write works, effect visible inport-reads

5 - port-wide synthesis mostly works - isnt working yet
6 - currently nsc_gpio.c defines  DEBUG 1, giving output cut-pasted below


pc8736x-gpio port reads:

running: cat /sys/devices/platform/pc8736x_gpio.0/port_0_value/sys/devices/platform/pc8736x_gpio.0/port_1_value/sys/devices/platform/pc8736x_gpio.0/port_2_value/sys/devices/platform/pc8736x_gpio.0/port_3_value

0xff
0xd1
0x9e
0x1d
...

and bit-reads: (bitpos=1 exposes more info)

running: cat /sys/devices/platform/pc8736x_gpio.0/bit_1.0_value/sys/devices/platform/pc8736x_gpio.0/bit_1.1_value/sys/devices/platform/pc8736x_gpio.0/bit_1.2_value/sys/devices/platform/pc8736x_gpio.0/bit_1.3_value/sys/devices/platform/pc8736x_gpio.0/bit_1.4_value/sys/devices/platform/pc8736x_gpio.0/bit_1.5_value/sys/devices/platform/pc8736x_gpio.0/bit_1.6_value/sys/devices/platform/pc8736x_gpio.0/bit_1.7_value

0x1
0x0
0x0
0x0
0x10
0x0
0x40
0x80

test port-synthesis - cmd-letter is borrowed from set understood bychar-dev-write-handler


running: cat /sys/devices/platform/pc8736x_gpio.0/port_0_output_enabled

[ 2039.060925] platform pc8736x_gpio.0: get-portconf bit:0.0 cfg:44cmd'O' res:0[ 2039.068312] platform pc8736x_gpio.0: get-portconf bit:0.1 cfg:44cmd'O' res:0[ 2039.075703] platform pc8736x_gpio.0: get-portconf bit:0.2 cfg:44cmd'O' res:0[ 2039.083053] platform pc8736x_gpio.0: get-portconf bit:0.3 cfg:44cmd'O' res:0[ 2039.090434] platform pc8736x_gpio.0: get-portconf bit:0.4 cfg:44cmd'O' res:0[ 2039.097789] platform pc8736x_gpio.0: get-portconf bit:0.5 cfg:44cmd'O' res:0[ 2039.105140] platform pc8736x_gpio.0: get-portconf bit:0.6 cfg:44cmd'O' res:0[ 2039.112487] platform pc8736x_gpio.0: get-portconf bit:0.7 cfg:44cmd'O' res:0

[ 2039.119875] platform pc8736x_gpio.0: get-portconf() idx=0 cmd='O' ret:0
0x0

running: cat /sys/devices/platform/pc8736x_gpio.0/port_0_pullup_enabled

[ 2039.185437] platform pc8736x_gpio.0: get-portconf bit:0.0 cfg:44cmd'P' res:0[ 2039.192829] platform pc8736x_gpio.0: get-portconf bit:0.1 cfg:44cmd'P' res:0[ 2039.200181] platform pc8736x_gpio.0: get-portconf bit:0.2 cfg:44cmd'P' res:0[ 2039.207531] platform pc8736x_gpio.0: get-portconf bit:0.3 cfg:44cmd'P' res:0[ 2039.214887] platform pc8736x_gpio.0: get-portconf bit:0.4 cfg:44cmd'P' res:0[ 2039.222244] platform pc8736x_gpio.0: get-portconf bit:0.5 cfg:44cmd'P' res:0[ 2039.229633] platform pc8736x_gpio.0: get-portconf bit:0.6 cfg:44cmd'P' res:0[ 2039.236995] platform pc8736x_gpio.0: get-portconf bit:0.7 cfg:44cmd'P' res:0

[ 2039.244300] platform pc8736x_gpio.0: get-portconf() idx=0 cmd='P' ret:0
0x0

soekris:/sys/devices/platform# echo 1 >pc8736x_gpio.0/bit_0.1_pullup_enabled

[ 2760.304169] platform pc8736x_gpio.0: GPIO1 pullup enabled=1
[ 2760.310014] platform pc8736x_gpio.0: set-bitconf() idx:1 cmd:'P' buf:'1
[ 2760.310059] ' set:1
soekris:/sys/devices/platform#
soekris:/sys/devices/platform# cat pc8736x_gpio.0/port_0_pullup_enabled

[ 2780.140281] platform pc8736x_gpio.0: get-portconf bit:0.0 cfg:44cmd'P' res:0[ 2780.147761] platform pc8736x_gpio.0: get-portconf bit:0.1 cfg:46cmd'P' res:2[ 2780.155139] platform pc8736x_gpio.0: get-portconf bit:0.2 cfg:44cmd'P' res:0[ 2780.162585] platform pc8736x_gpio.0: get-portconf bit:0.3 cfg:44cmd'P' res:0[ 2780.169960] platform pc8736x_gpio.0: get-portconf bit:0.4 cfg:44cmd'P' res:0[ 2780.177379] platform pc8736x_gpio.0: get-portconf bit:0.5 cfg:44cmd'P' res:0[ 2780.184749] platform pc8736x_gpio.0: get-portconf bit:0.6 cfg:44cmd'P' res:0[ 2780.192158] platform pc8736x_gpio.0: get-portconf bit:0.7 cfg:44cmd'P' res:0

[ 2780.199484] platform pc8736x_gpio.0: get-portconf() idx=0 cmd='P' ret:2
0x2




TIA for any comments, feedback.


diff -ruNp -X dontdiff -X exclude-diffs ../linux-2.6.18-rc3-mm2-sk/Documentation/gpio-model.txt roll3-m2/Documentation/gpio-model.txt
--- ../linux-2.6.18-rc3-mm2-sk/Documentation/gpio-model.txt	1969-12-31 17:00:00.000000000 -0700
+++ roll3-m2/Documentation/gpio-model.txt	2006-08-08 16:47:12.000000000 -0600
@@ -0,0 +1,130 @@
+
+Sysfs GPIO Model
+
+GPIO Hardware Design Overview
+
+GPIO (General Purpose IO) Hardware provides digital IO on pins
+(physical connections to external circuitry).  The pins can be read
+and written, and are configurable for adaptability to a variety of
+external circuit designs.  The typical GPIO feature set looks like
+this:
+
+   input:
+	- input (read pin value)
+	- input (read pin driven-value)
+	- input debounce (hw noise conditioning)
+
+   output:
+	- data-out
+	- output-enable !tristate
+
+   config: (used less frequently)
+   	- pullup-resistor
+	- output-enable
+	- open collector (can only pull to zero)
+	- open emmitor (can only pull to high)
+	- interrupt generation & control
+
+
+Hardware Ports Organization
+
+Many GPIO hardware units organize GPIO bits (aka pins) into ports (aka
+banks) with an implementation-dependent width.  The pin values are
+written and read on a port-wide basis, ensuring that all pins in a
+port change *simultaneously*, ie not as result of separate bus cycles.
+This is essential for some applications.  Port-wide access is
+typically just for data.
+
+Config features control the GPIO's electrical properties, and are
+rarely used (data-access dominates).  These features are usually
+implemented per-bit, but with some variations.
+
+
+GPIO-Sysfs Features
+
+All GPIO device-attr-files have 3-part names:
+
+  <prefix>_<id>_<suffix>.
+
+   prefix	'bit_'	or 'port_'
+   id		 1.3	or  2	respectively
+   suffix	feature-name	(value, totem, etc)
+
+for example:
+
+soekris:/sys/devices/platform# ls pc8736x_gpio.0/bit_0.1_*
+pc8736x_gpio.0/bit_0.1_current         pc8736x_gpio.0/bit_0.1_pullup_enabled
+pc8736x_gpio.0/bit_0.1_debounced       pc8736x_gpio.0/bit_0.1_status
+pc8736x_gpio.0/bit_0.1_locked          pc8736x_gpio.0/bit_0.1_totem
+pc8736x_gpio.0/bit_0.1_output_enabled  pc8736x_gpio.0/bit_0.1_value
+
+
+Suffixes:
+
+The Suffix displays the feature-name.  They are named for one of the
+states, rather than the property-name, since a state-name=X is
+self-explanatory.
+
+And, matching my hardware:
+   _output_enabled  vs  _tristate
+   _totem	    vs  _pushpull
+
+The <suffix> should be user replaceable later, with a means to add
+logical inversion selectively.
+
+
+Bit vs Port features.
+
+Depending upon hardware, each GPIO feature is controllable via either
+a bit-wide or port-wide interface.  The model allows driver to
+abstract this, and to synthesize port-wide features where needed.
+
+- driver can choose to expose features only as naturally supported.
+  User space can deal then recognize when port-wide control of
+  output-enable, interrupts, etc are supported by the hardware.
+
+- driver can synthesize port-features by banging the per-bit access.
+  This was done ad-hoc, got reasonable results.
+
+
+Port, Bit Reservation (a few thoughts)
+
+Assuming port-wide GPIOs (seeing a bias?), each hardware port is

+trivially reserved by masking in bits as they're taken.+

+simplifing constraints:
+ - user-ports are always subset of single hw-port
+ - no hw-port spanning
+ - contiguous bit-blocks only (0x5,0x9 disallowed)
+
+
+Various Hardware Notes
+
+General rules
+
+- GPIOs generally power-up into safe states (tristate/input-only),
+  interrupts are off, etc.
+
+- GPIOs can have diminishing feature set across provided resources.  A
+  designer rarely needs homogeneous features, often needs very few
+  pins which must generate interrupts.
+
+- when a pin doesnt support feature, it provides a 'ghost-bit', which
+  reads as disabled, and ignores writes/ enablements.  This is not
+  universal however..
+
+PC87366
+
+- has 4 8-bit ports, with diminishing feature set.
+- pins in all 4 share common *basic-config* register defn
+- ports 2,3 lack interrupts
+--  *and* they lack register too.
+--  they should have given it a ghost register.
+
+SC-1100
+
+This GPIO-port also supports Interrupt-Enable & Status.
+
+Sadly, it doesnt expose port-wide output-enable, which means
+bus-logic (switching all N-lines to hiZ) is prohibitively slow.
+
diff -ruNp -X dontdiff -X exclude-diffs ../linux-2.6.18-rc3-mm2-sk/drivers/char/nsc_gpio.c roll3-m2/drivers/char/nsc_gpio.c
--- ../linux-2.6.18-rc3-mm2-sk/drivers/char/nsc_gpio.c	2006-07-30 11:29:18.000000000 -0600
+++ roll3-m2/drivers/char/nsc_gpio.c	2006-08-08 16:54:01.000000000 -0600
@@ -7,36 +7,112 @@
   Copyright (c) 2005      Jim Cromie <[email protected]>
*/

+#define DRVNAME "nsc_gpio"
+
+#include <linux/device.h>
#include <linux/config.h>
#include <linux/fs.h>
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/init.h>
-#include <linux/nsc_gpio.h>
#include <linux/platform_device.h>
#include <asm/uaccess.h>
#include <asm/io.h>
+#include <linux/nsc_gpio.h>

-#define NAME "nsc_gpio"
+static int noports = 0;
+module_param(noports, int, 0);
+MODULE_PARM_DESC(noports, "sysgpio 0:both 1:no-status 2:no-sysfs-flags");
+
+static int bitpos = 0;
+module_param(bitpos, int, 0);
+MODULE_PARM_DESC(bitpos, "1 to expose bit position in values");

-void nsc_gpio_dump(struct nsc_gpio_ops *amp, unsigned index)
+static char statbuf[256];
+static ssize_t nsc_gpio_status(struct nsc_gpio_ops *amp, unsigned index)
{
	/* retrieve current config w/o changing it */
	u32 config = amp->gpio_config(index, ~0, 0);

++ return sprintf (

+		statbuf, "i/o%02u:%d/%d  0x%04x %s %s %s %s %s %s %s",
+		index,
+		!!(amp->gpio_get(index)),	/* boolean display */
+		amp->gpio_current(index),	/* someday show [01z] */
+		config,
+		(config & 1) ? "OE" : "TS",      /* output-enabled/tristate */
+		(config & 2) ? "PP" : "OD",      /* push pull / open drain */
+		(config & 4) ? "PUE" : "PUD",    /* pull up enabled/disabled */
+		(config & 8) ? "LOCKED" : "",    /* locked / unlocked */
+		(config & 16) ? "LEVEL" : "EDGE",/* level/edge input */
+		(config & 32) ? "HI" : "LO",     /* trigger on rise/fall edge */
+		(config & 64) ? "DEBOUNCE" : "" /* debounce */
+		);
+}
+
+void nsc_gpio_dump(struct nsc_gpio_ops *amp, unsigned index)
+{
+	nsc_gpio_status(amp,index);
+	dev_info(amp->dev, "%s\n", statbuf);
+}
+
+/* the pin-mode-change 'commands' of the legacy device-file-interface,
+   are reused in the sysfs-interface.
+*/
+static int command_write(struct nsc_gpio_ops *amp, char c, unsigned m)
+{
+	struct device *dev = amp->dev;
+	int err = 0;

-	/* user requested via 'v' command, so its INFO */
-	dev_info(amp->dev, "io%02u: 0x%04x %s %s %s %s %s %s %s\tio:%d/%d\n",
-		 index, config,
-		 (config & 1) ? "OE" : "TS",      /* output-enabled/tristate */
-		 (config & 2) ? "PP" : "OD",      /* push pull / open drain */
-		 (config & 4) ? "PUE" : "PUD",    /* pull up enabled/disabled */
-		 (config & 8) ? "LOCKED" : "",    /* locked / unlocked */
-		 (config & 16) ? "LEVEL" : "EDGE",/* level/edge input */
-		 (config & 32) ? "HI" : "LO",     /* trigger on rise/fall edge */
-		 (config & 64) ? "DEBOUNCE" : "", /* debounce */
+	switch (c) {

-		 amp->gpio_get(index), amp->gpio_current(index));
+	/* cases are a mix of old command letters, and new PF_CMD_*
+	   symbols, for Off & On actions respectively.
+	*/
+	case '0':
+		amp->gpio_set(m, 0);
+		break;
+	case '1':
+		amp->gpio_set(m, 1);
+		break;
+	case PF_CMD_OUT_ENA:
+		dev_dbg(dev, "GPIO%d output enabled\n", m);
+		amp->gpio_config(m, ~1, 1);
+		break;
+	case 'o':
+		dev_dbg(dev, "GPIO%d output disabled\n", m);
+		amp->gpio_config(m, ~1, 0);
+		break;
+	case PF_CMD_TOTEM:
+		dev_dbg(dev, "GPIO%d output is push pull\n", m);
+		amp->gpio_config(m, ~2, 2);
+		break;
+	case 't':
+		dev_dbg(dev, "GPIO%d output is open drain\n", m);
+		amp->gpio_config(m, ~2, 0);
+		break;
+	case PF_CMD_PULLUP:
+		dev_dbg(dev, "GPIO%d pull up enabled\n", m);
+		amp->gpio_config(m, ~4, 4);
+		break;
+	case 'p':
+		dev_dbg(dev, "GPIO%d pull up disabled\n", m);
+		amp->gpio_config(m, ~4, 0);
+		break;
+	case 'v':
+		/* View Current pin settings */
+		amp->gpio_dump(amp, m);
+		break;
+	case '\n':
+		/* end of settings string, do nothing */
+		break;
+	default:
+		dev_err(dev, "io%2d bad setting: chr<0x%2x>\n",
+			m, (int)c);
+		err++;
+	}
+	return err;
}

ssize_t nsc_gpio_write(struct file *file, const char __user *data,
@@ -44,57 +120,14 @@ ssize_t nsc_gpio_write(struct file *file
{
	unsigned m = iminor(file->f_dentry->d_inode);
	struct nsc_gpio_ops *amp = file->private_data;
-	struct device *dev = amp->dev;
-	size_t i;
-	int err = 0;
+	int i, err = 0;

	for (i = 0; i < len; ++i) {
		char c;
		if (get_user(c, data + i))
			return -EFAULT;
-		switch (c) {
-		case '0':
-			amp->gpio_set(m, 0);
-			break;
-		case '1':
-			amp->gpio_set(m, 1);
-			break;
-		case 'O':
-			dev_dbg(dev, "GPIO%d output enabled\n", m);
-			amp->gpio_config(m, ~1, 1);
-			break;
-		case 'o':
-			dev_dbg(dev, "GPIO%d output disabled\n", m);
-			amp->gpio_config(m, ~1, 0);
-			break;
-		case 'T':
-			dev_dbg(dev, "GPIO%d output is push pull\n", m);
-			amp->gpio_config(m, ~2, 2);
-			break;
-		case 't':
-			dev_dbg(dev, "GPIO%d output is open drain\n", m);
-			amp->gpio_config(m, ~2, 0);
-			break;
-		case 'P':
-			dev_dbg(dev, "GPIO%d pull up enabled\n", m);
-			amp->gpio_config(m, ~4, 4);
-			break;
-		case 'p':
-			dev_dbg(dev, "GPIO%d pull up disabled\n", m);
-			amp->gpio_config(m, ~4, 0);
-			break;
-		case 'v':
-			/* View Current pin settings */
-			amp->gpio_dump(amp, m);
-			break;
-		case '\n':
-			/* end of settings string, do nothing */
-			break;
-		default:
-			dev_err(dev, "io%2d bad setting: chr<0x%2x>\n",
-				m, (int)c);
-			err++;
-		}
+
+		err += command_write(amp, c, m);
	}
	if (err)
		return -EINVAL;	/* full string handled, report error */
@@ -121,15 +154,414 @@ EXPORT_SYMBOL(nsc_gpio_write);
EXPORT_SYMBOL(nsc_gpio_read);
EXPORT_SYMBOL(nsc_gpio_dump);

+#define BITVAL(a) "%u\n", !!(a)		/* force int to boolean */
+#define PORTVAL(a) "0x%x\n", (a)	/* expose bitval pos in port */
+
+/* 8 sysfs-callbacks, partitioned on 3 axes:
+   get/set:	as reqd by 2 different callback fn-signatures
+   value/:	for port-wide
+    /feature:	mostly use-once, slower, synthesized port-access
+   bit/port:	for values, separate for simplicity, speed
+
+   This is arguably over-optimization, may clash with client-module
+   feature to bit/port mappings.
+*/
+ssize_t nsc_gpio_sysfs_get_bitval(struct device *dev,
+				  struct device_attribute *devattr, char *buf)