Re: [patch/rfc 2.6.25-git] gpio: sysfs interface

[David Brownell <david-b@pacbell.net>]

On Monday 28 April 2008, Ben Nizette wrote:
> 
> On Mon, 2008-04-28 at 12:39 -0700, David Brownell 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:
> > 
> >   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. 

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


> 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.


> 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.


> 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.")

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

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


> 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.)


> > 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 ...


> 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