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 <tpiepho@freescale.com>, and comments
from various folk including Hartley Sweeten.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
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 ++++
9 files changed, 333 inser...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
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
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);
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.
--If all gpios are exported read only by default then keeping the label either in the folder naming or as a file is certainly useful to identify what you actually want to talk to. If you're debugging a driver or you've already manually exported a gpio then I'd assume you already know pretty much all you need to know about that pin. --Ben. --
I'd rather see such stuff in a "chip_label" attribute. Easy enough to 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 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 function ... or not even connected. 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 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 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 I had that thought too. Even though I think that solution is kind o fugly. ;) If need be, that can be changed. - Dave --
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
It's nice to be able to see what a driver is using a gpio for. You could also
So make them appear when something in the kernel requests them, explictly
exports them to user-space, or they are requested from user space. The last
two can offer write access, the first only read. I don't see why the explicit
request is necessary for something to show up in sysfs. Nothing else in sysfs
seems to work this way. At least, I see plenty of files in there that I
$ ls /sys/class/tty/ | wc
579 579 4042
I didn't mean for gpiolib to create that device, that's obviously wrong. What
I meant was the platform code, e.g. the code the calls gpiochip_add(), could
just call that one function and then it would have a device for the gpios to
appear under in sysfs. You said that many systems "can't" have a device for
the gpios and I don't see how this is so. Could you give me an example of
something that calls gpiochip_add() and can't provide a dev field in the
If memory allocations don't work, then gpiochip_add() can't possibly do
anything with sysfs, so having a device to parent the sysfs files from is a
I thought you meant comments to my patch.
--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. 2) Have the files named as you suggest and you have to explicitly request them or have the kernel explicitly export them. To request them yourself you're going to need the gpio number so having the created file labelled nicely isn't a win over having it labelled with the gpio number. I 'spose there's a win for kernel exported gpios, they're more human readable, but you're still going to have to have the mappings available somewhere for the manually exported gpios anyway. 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. 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. Do your expectations/ideas fit in cleanly anywhere above? Thanks, --Ben --
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 --
Right, agreed. 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 : : I guess this doesn't gain much over labelling files chipname:N (and has the same pitfalls) but does at least seem less hackish. --
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. - Dave --
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}
If you've got multiple pca9557s then you're always going to have a hard
time distinguishing them but you've been given all the information
available to allow you to discover which is which.
In fact more than enough; if the base is dynamically allocated then you
don't know what to expect /start to be, you know what /ngpio will be and
you never need to find the full gpio number so those 2 files are
redundant yeah?
--Ben.
--What I have working right now is like the original patch, but with "gpiochipN" (and "gpioN") nodes. The "*-N" style doesn't match other sysfs usage. So the "control" and "gpioN" files aren't nesting like that. It could be argued which feels more natural. For external gpiochips it might feel more natural to nest the gpioN nodes. On some SOCs, that also true of the integrated GPIOs (docs refer to "PB29" highlighting port B bit 29). But on many others (OMAP and PXA for starters), they're referenced in docs by a number 0..hundreds, ignoring details like which register bank holds the relevant control bits. That's why I decided not to use nesting. Either choice can seem a bit odd on some platform; this way is easier and takes less kernel code. - Dave --
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 <tpiepho@freescale.com>, and comments
from various folk including most recently Hartley Sweeten and Ben Nizette.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
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, si...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 --
Just didn't describe it in that summary ... like several other standard attributes, like "uevent" and "subsystem". The only thing different about "device" is that when the gpio_chip doesn't say what device it uses, that will (necessarily) be missing. Yeah, gotta fix that. Unfortunately "-0" == "0" ... I'll have to handle any leading "-" by hand. Trivial fix, in all. - Dave --
"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 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. So if a driver is already using gpio 23, then there is no way to see it in 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 You could say the same about 90% of the writable files in sysfs. --
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". 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. If it's a one-off, they probably start from schematics and are fully capable of adding 5 to the base number associated with the particular pcf chip in question (say, the third one on this card stack) ... and doing the same for other pins they need to diddle. Note taking is always a good practice when sorting out such issues, but adding five is in your head "Any more"? You never *could* do that before. The original patch you sent didn't do that, and the last code snippet I I didn't ignore *use cases* at all. You can certainly set the value of such a GPIO with the $SUBJECT interface. Just ... not using that syntax. Use cases are several steps I've seen at least half a dozen different userspace models for how to work with GPIOs. You didn't write all of them. Nor 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, No, that was the one-line summary. It's only freed if it was originally requested from userspace (by "echo 23 > control"). As you can see in the code... - Dave --
So what are these must fix problems? You don't want all the gpios to appear
in sysfs by default in case there are many. So have them only appear when
they are requested by something. Problem solved.
Someone might label the chip with a label using a '/'. First of all, that's
nothing but speculation on a problem that will probably never happen. Do any
existing chip labels use a '/'? Is it really that hard to just not label the
chip that way? But suppose someone just can't avoid it, one line of code,
that I posted, is all it takes to fix this.
Multiple chips with the same label. That's more annoying, but still
easy to fix in a number of ways:
1) If the label isn't unique, don't register that chip with sysfs.
2) If the label isn't unique, appended a suffix to make it so.
3) Always add a sequence number the end of the chip label. This is quite
common in fact.
The sysfs control interface won't be able to handle gpios named this way. Or
doing so will be "too hard". Not true, I wrote and posted code that did it.
It was simpler than the initial code you wrote for the control interface, and
much simpler than the latest with all the gpiochip stuff. BTW, my code also
worked for unexporting gpio 0....
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.
The directories are not just named "netdev0", "netdev1", "netdev2", with some
sort of mapping file telling you ethernet devices are using numbers 2-3.
Look at PCI, you have files like "/sys/bus/pci/devices/0000:01:05.0" and
"/bus/pci/devices/0000:00:0d.2". It's not "pci1", "pci2", "pci3", with a
"bus0" directory telling you that the bus starts at device 3 and has 6
devices.
Look at ACPI, it's /sys/bus/acpi/devices/{LNXSLPBN:00,LNXPWRBN:00,LNXSYSTM:00}
and "device:00", "device:01". It use...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 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? Your examples prove what I *did* say. I don't know who you're trying Which is fine. The $SUBJECT patch provides a simple solution for it. As well as for the other use cases I sketched. With identifiers that Those are *NOT* my words. Again, I don't know what straw man you are arguing with, but please stop painting my face on it, putting your words in its mouth, and using that to claim you're disproving something I've "said" (but haven't). I did say your problem could be solved ... but with different 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 Userspace being able to MODIFY the direction and output value 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. --
Let's be fair about who was waiting for who. You didn't like some things about my original system, I proposed changes here, http://article.gmane.org/gmane.linux.kernel/662862, on April 7th. You didn't respond until April 27th, then decided to take my patch and change it they way Instead of gpio19, it should be the chip label followed by 19. I want "eth0", "tap0". You want "net0", "net1", and then a whole extra system to figure out "However, I think a slightly more common practice in current embedded Linux systems is to build custom kernels that know which daughtercard(s) are available. That's mostly what gets pushed upstream, anyway..." "Which is why this particular side-argument seems like a waste of time to me:" because for a pure userspace interace it can't be helpful unless userspace makes the labels: I just don't see that. I've got a JTAG interface on the gpio pins. The user wants to see what gpio pin is assigned to what function. The information could be right there with the direction and value. They could have a script that searches the gpios for one with a matching label. I've got a board revision that so far the kernel doesn't care about. I could put something like this in my init scripts: for i in 0 1 2 3 do echo "Board Rev [bit $i]" > /sys/class/gpio/pca9557-1:$i/label done Yes, I could only have the information in some docs and force someone to look Now you're the one with the straw-man. I've proposed many times that gpios that have been requested by anything be exported READ-ONLY automatically. I said "*seeing* the value", not "*changing* the value". I've yet to see you Will allowing userspace to request a gpio used by the kernel allow one to break things easier than: cat /dev/zero > /dev/hda I don't think so. Do you think allowing root to use a gpio after root has specifically exported it will allow someone to break their system more easily and to a greater extent than that command? Yet that command...
I think that's because it's a trivial "Shell Programming 101" homework assignment, and we have a general policy against doing people's schoolwork for them ... Here are two clues for you: "for DIR in gpiochip*"; and then "LABEL=$(cat $DIR/label)". I'm quite certain that if you put aside your demonstrably strong desire to flame, you can solve that little problem. Or perhaps the issue here is just bikeshedding. Regardless, the signal-to-noise ratio is way too low.. --
Hmm, maybe that was buried in the volume of other stuff you posted ... this is the first time I recall your mentioning read-only exports. Or limiting exports like that. Certainly no code I've seen from you worked like that. What I recall was your original approach of exporting absolutely everything -- hundreds of (potential) GPIOs, even pins that were configured for other purposes. I don't have any particular objection to exporting things readonly. Maybe you noticed the comment in my patches that suggesting such an export mode might be useful. But I'd rather not export things that don't need exporting, and bias things towards low per-gpio costs. It's easy to add features later, if they turn out to be needed. But removing interfaces is very hard ... which is another reason I'm not very receptive to all those bells'n'whistles you propose. --
We've discussed the down sides of your model. It's got up sides, eg your use case is made very easy, but it ain't all roses. Now, if the kernel has requested that pin and hasn't exported it you can't touch it. This is what we want. Otherwise you will walk the gpiochips, find your chip's base and use it to calculate the gpio number. If the kernel's exported it then the file will be there to use. If the kernel hasn't requested it then you can do so by using the control file. Yes it's longer and harder and more convoluted. It's also much more 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 That's very useful indeed, but if you don't want userspace to be able to do that it's also dangerous. This is why the kernel has to explicitly So you want userspace to be able to clobber any gpio which is used by the kernel? That's what this version of the interface is designed to The way to see it in sysfs is to have the kernel export it. This is the point. The kernel should request it for it's own use then export it if If driver was using it, it will have requested it. If you are looking at it then the kernel must have also exported it. If the kernel's exported it then nothing you can do will unexport it. You can only unexport gpios you've exported yourself manually and the only gpios you --
They could be; I've had code to do it. It never stays in very long, because for a pure userspace interace it can't be helpful unless userspace makes the labels: the labels displayed otherwise just reuse the same constant. For a "kernel cooperates with userspace" model that might be more useful. And I've certainly found the debugfs info hard to interpret without such labels!! But Trent has said he doesn't see anyone modifying enough code to adopt such a "cooperates" model. Which is why this particular side-argument seems like a waste of time to me: even he has not proposed the key change his argument depends on. The original control file syntax I thought about was export NN mode label where it would gpio_request(NN, "label") and then set the direction (and maybe value) according to "mode". But that was just a PITA to cope with ... so it's now been trimmed down to just "NN" (and "-NN"). - Dave --
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! :)" --
I'm sure I could but I'd prefer to have us all come (as close as possible) to an agreed theoretical interface first :-) What you've got working there is nice but it still downsides (unless you can set me straight). First, we still have the problem of either 100s of unused files or an index to use in a request. Also, if you've got many pcf9557s in a system you'll have a bunch of pcf9557-n; how do you know which is which? I think with the above solution we're coming very close to a sexy system. We don't need an index file, we have all the information we need to find which device is which, there's no need to calculate any full gpio number, there's no files which aren't going to be used, it's safe, extensible, scriptable, sysfsish.. Am I missing anything? --Ben --
Well, is it really that much? There are 579 files under /sys/class/tty. But
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. It's certainly not huge.
David's code for parsing the control file plus code for generating a mapping
range file would certainly be larger.
/* Format: -?(chiplabel:)?number
* The optional leading - unexports the gpio, without it the gpio is exported.
* The optional chip label followed by a : gives you the Nth gpio of that
* chip. With no label you get gpio "number".
*/
static ssize_t control_store(struct class *class, const char *buf, size_t len)
{
const char *numstr;
unsigned long num;
int mode = 0;
if (buf[0] == '-') { /* un-export? */
mode = 1;
buf++;
}
numstr = strrchr(buf, ':');
/* Get GPIO number */
if (strict_strtoul(numstr ? numstr + 1 : buf, 0, &num))
return -EINVAL;
/* Match chip label, if one was specified */
if (numstr) {
/* No + 1 in len to not include the ':' at the end */
int i, len = numstr - buf;
const struct gpio_chip *chip = NULL;
for (i = 0; gpio_is_valid(i); i++) {
if (chip == gpio_desc[i].chip)
continue;
chip = gpio_desc[i].chip;
if (!chip)
continue;
if (!strncmp(buf, chip->label, len) &&
chip->label[len] == '\0')
goto found_chip;
}
return -EINVAL;
found_chip:
if (num >= chip->ngpio)
return -EINVAL;
num += chip->base;
}
if (mode) {
/* Unexport */
if (!gpio_is_valid(num))
return -EINVAL;
if (!test_and_clear_bit(FLAG_SYSFS, &gpio_desc[num].flags))
return -EINVAL;
gpio_free(num);
} else {
/* Export */
int status = gpio_request(num, "sysfs");
if (status < 0)
return status;
status = gpio_export(num);
if (status < 0) {
gpio_free(num);
return status;
}
set_bit(FLAG_SYSFS, &gpio_desc[num].flags);
}
...I just ssh'd into three embedded boards I have handy, and they have respectively four, four, and seven entries there. That "seven" case is actually incorrect ... the other three serial ports aren't connected to anything. So: yes, adding a few hundred useless sysfs nodes *IS* a problem in the target environment of embedded boards. Note that "read-only until export" is far from straightforward Apples vs oranges. Use the same command syntax if you're going to make comparisons; I can save even more with "+export/-unexport" The #3 option presumes some file listing chips and ranges too, since GPIOs are exported only on demand. Ditto #2 and #4... - Dave --
If the code you wrote it not too complex, then why is the code I wrote, which You never answered how one was supposed to get the proper device from a script. --
Well, Andrew *did* object to the complexity. But that wasn't the point I was making there: you were comparing apples and oranges ... which makes it particularly easy to reach desired No I didn't. But that's why I liked Ben's suggestion of creating sysfs nodes for each gpio_chip. That's actually a good example of why folk like the one-value-per-attribute model with sysfs, at least for information that would be used with scripting. - Dave --
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 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 [had some comments regarding the code but it seems akpm covered them all already :-)] --
The gpio_export() call requires someone (the caller!) to have requested the GPIO already. The "one owner" rule is Right. Not unless we're debugging the driver managing Well, I wouldn't call that "regular"! But then, I don't have this "use GPIOs from userspace" focus. To me, that's 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 ... intended purely for debugging, not for use with production 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 Sysfs attributes are supposed to be pollable. I've not done it, We accept patches. Even patches on top of patches. ;) - Dave --
Owner's probably the wrong word. I was just confirming that in that case the request/export caller and userspace can both fiddle with the pin. Though of course the caller should expect this and behave 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 Keeping track of which pins aren't used for peripherals then requesting and exporting them is going to be a pain even for smaller SoCs. But as you say, the BSP designer can just opt out if they can't be bothered. Actually, in, eg AVR32 at32ap we'd have to keep track of all this anyway so we can call at32_select_gpio() and make the pins ready for pio access Yeah that's the kind of a thing. Would be well worth having that info I'll wait for a final(ish) version and a spare millisecond and see what might be done :-) --
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 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 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 --
If you want use cases, how about mine? I didn't write the code originally for fun, I wrote it for a real product. 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 Suppose I want line 5 from the mpuio gpios? I'd make it so this was: /sys/class/gpio/mpuio:5 How would you get the sysfs location, with an automatic shell script running Why not have an irq file for that? --
Good examples. Note that the "daughtercard installed" cases generalize somewhat ... it's not uncommon to do like DRAM sticks do with SPD EEPROMS, and have a cheap EEPROM identifying characteristics of that card, since different cards need different initialization/setup. However, I think a slightly more common practice in current embedded Linux systems is to build custom kernels that know which daughtercard(s) are available. That's mostly what gets pushed upstream, anyway... ISTR that Gumstix distro kernels use a more retro scheme to identify what's in the card stack: they poke at various peripheral addresses to see if the device on a given card is there. In the PC space, that's what "ISA Legacy Drivers" do, albeit at the level of individual peripherals, not cards. - Dave --
Oh yeah, nearly every vendor of small not-a-simple-PC Linux boards would have their own solution to this problem. About time they were put to At a glance there's a bunch of how-to but very little why-to. Bugger. In fact their driver looks to be mostly obsoleted by gpio-keys anyway so not only can't a see a specific use-case of their driver, I can't see xD 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 ;) --
OK, I'll leave it be then. Given that not all GPIOs support interrupts, I used the "poll" example intentionally ... being --
Putting includes inside ifdefs tends to increase the risk of compilation What prevents FLAG_EXPORT from getting cleared just after we tested it? 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); urgh. We cast away the const and then smash up the caller's const string 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 Can we find a way to make HAVE_GPIO_SYSFS go away? Just use Then this can just be moved into a #else. --
Having seen ibm acpi interface... yes, that rule should be aplied for writes, too. What about mkdir gpio-N to export it? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
The latest version of this patch, which I'll post soonish, follows the suggestion to have one file for the "export" 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 --
...but it would be consistent with configfs... and face it... doing echo > file to make a directory is seriously ugly. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
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? --
No, sorry. I'm not sure if it should go to configfs, or if sysfs should be improved. But "something" should be done, because we will be stuck with this interface for a long while. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html --
I have a userspace gpio interface which uses /dev and configfs. The configfs side was really nice (and used mkdir, as configfs does) but I wasn't a fan of needing udev.. On balance, I like the all-in-one sysfs --
This adds a simple sysfs interface for GPIOs.
/sys/class/gpio
/export ... asks the kernel to export a GPIO to userspace
/unexport ... to return a GPIO to the kernel
/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 claimed by kernel code may be exported by its owner using a new
gpio_export() call, which should be most useful for driver debugging.
Such exports may optionally be done without a "direction" attribute.
Userspace may ask to take over a GPIO by writing to a sysfs control
file, helping to cope with incomplete board support or other "one-off"
requirements that don't merit full kernel support:
echo 23 > /sys/class/gpio/export
... will gpio_request(23, "sysfs") and gpio_export(23);
use /sys/class/gpio/gpio-23/direction to (re)configure it,
when that GPIO can be used as both input and output.
echo 23 > /sys/class/gpio/unexport
... will gpio_free(23), when it was exported as above
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.
Related changes:
* 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 gpio_chip drivers also needed to update their module
"owner" field ... for which missing kerneldoc was added.
* Some gpio_chips don...ick. So we got caught partway through constification and we need to patch it up with a cast. --
That *is* one value: a single command, to execute atomically! :) ISTR seeing that done elsewhere, and have seen various proposals that rely on that working. In any case, the one-per-file rationale Next version. If the "is this interface OK" part of the RFC flies, Good point, I'll move such stuff out of #ifdefs. I almost Yeah, the isue there is that the attribute files may still be open after the GPIO has been unexported. For this specific method the existing spinlock can solve that problem ... but not in general. Seems like a general fix for that should involve a mutex covering all those sysfs actions: read, write, export, and unexport. The unexport logic would need reworking, but the rest could work pretty much as they do now (but while holding that lock, which would protect that flag). But that wouldn't change the user experience; the sysfs 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. But as with quite a lot of "rmmod" type paths, there's Yeah, kind of ugly. Not particularly happy with that, but I wasn't keen on allocating a temporary copy of that string So it does. That was leftover from a version with a more complex control interface. Easy to remove Or just use negative numbers to mean "unexport"; ugly and hackish, but functional. I don't want to see the components of one command split into separate files, where they could be I'll just make it #ifndef CONFIG_GPIO_SYSFS ... that will make the interface impossible to provide without gpiolib, but I'm not much concerned about that. Note that putting it *here* covers the case of platforms that provide standard GPIO interfaces but don't use the newish gpiolib code to do that ... which is why putting it in an #else (above) didn't suffice. --
Yes, it would be a standalone patch. The sort which generates oceans of useful feedback ;) The sort which also generates hundreds of Yeah, I couldn't think of a decent name. I do think it should return true on finding a match so callers don't need to use ! or ==0. So its name shouldn't look anything like "strcmp". --
Heh, sounds good to me :) Becides, with linux-next, that merge mess is my problem now, not -mm... thanks, greg k-h --
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 <dbrownell@users.sourceforge.net>
---
include/linux/string.h | 2 ++
lib/string.c | 27 +++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
--- g26.orig/include/linux/string.h 2008-04-29 05:45:53.000000000 -0700
+++ g26/include/linux/string.h 2008-04-29 05:55:14.000000000 -0700
@@ -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_ */
--- g26.orig/lib/string.c 2008-04-29 05:15:52.000000000 -0700
+++ g26/lib/string.c 2008-04-29 05:55:32.000000000 -0700
@@ -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++;
+ }
+
+ if (*s1 == *s2)
+ return tru...On Tue, 29 Apr 2008 11:45:13 -0700
And
sysfs_streq("a\n", "a\n") ==> true
Looks good to me. I'll plan on sliding it into 2.6.26 a few days
hence.
--
| Artem Bityutskiy | [PATCH take 2 06/28] UBIFS: add journal replay |
| Greg Kroah-Hartman | [PATCH 009/196] Chinese: add translation of sparse.txt |
| Gregory Haskins | [PATCH 0/2] [RFC] RT: Optionally allow IRQF_NODELAY on serial console |
| Andrew Morton | Re: [RFC/PATCH] Documentation of kernel messages |
git: | |
| ir0s | Local branch ahead of tracked remote branch but git push claims everything up-to-d... |
| Matthieu Moy | git push to a non-bare repository |
| Rocco Rutte | mercurial to git |
| Bill Lear | Git rescue mission |
| Sunnz | radeon driver in -current Xorg 7.2? |
| Neko | reliable, dd over simple ip network |
| GVG GVG | ssh_exchange_identification: Connection closed by remote host |
| Siju George | This is what Linus Torvalds calls openBSD crowd |
| Inaky Perez-Gonzalez | [PATCH 00/39] merge request for WiMAX kernel stack and i2400m driver |
| Linus Torvalds | Re: [GIT]: Networking |
| Jarek Poplawski | [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
| Evgeniy Polyakov | Re: [bug, netconsole, SLUB] BUG skbuff_head_cache: Poison overwritten |
