Messages in this thread | | | From | David Brownell <> | Subject | Re: [PATCH] gpiolib: allow poll on gpio value | Date | Fri, 5 Jun 2009 23:01:39 -0700 |
| |
On Friday 05 June 2009, Ben Nizette wrote: > > Hey, good stuff Daniel. There's a fair few common features missing but > they can be added at a later date. > > - Ability to honour rising and falling filters even if the hardware only > supports both-edge (as lots of gpio interrupts do)
I suspect that's inadvisable. Userspace code will need to know what trigger model it's using, yes? Lies are doubleplusungood.
> - Support for polling the gpio at some interval for gpios which don't > support irqs at all
I had that thought. Units ... seconds? Milliseconds mapped to HZ? Could come later.
> - Debounce support
Software? Hardware capabilties vary *widely* ... three cases that come quickly to mind: (a) twl4030 fixed 30 msec delays, (b) at91 and avr32 "deglitch" filter, just syncs to a clock that's likely from 30-100 MHz, (c) omap2/omap3, up to 255 cycles of 32 KiHz clock but appplies entire GPIO banks, (d) DaVinci, no hardware support.
I can imagine a standard software filter option, but that would need to be a separate sysfs mechanism since it wouldn't always be desired. (And separate patch, if needed.)
For hardware options ... do that by hardware-specific sysfs hooks if they're really needed.
> - Reporting of number of changes since last read
Feels a more than bit overkilled by now. ;)
> These are all things which exist in many out-of-tree or > platform-specific implementations of this kind of thing and until > they're there I reckon people will largely stick with what they've got. > But that's really their problem of course, this is still valuable. > > Regarding the code itself, not much but: > > On Fri, 2009-06-05 at 16:36 +0200, Daniel Glöckner wrote: > > > > + "poll_edge" ... reads as either "none", "rising", "falling", or > > IMO this is misleading, sounds like you're polling the gpio.
So, just name the sysfs attribute "edge"?
> > + > > + struct sysfs_dirent *value_sd; > > }; > > No CONFIG_ option to turn all this off? > > What's the .text and .data impact of this? Sure it's going to be small, > but to some people (especially those likely to care about gpio) 1k > of .data is something worth being able to turn off.
I think it's probably OK to have this covered by the current GPIO_SYSFS flag.
> Using an IDR keyed to the gpio value and just allocating your useful > data structures when poll_edge != "none" would help too.
Can do that without an IDR, I think...
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |