David Brownell <[email protected]> wrote:
> Hardware IRQ debouncing is common for IRQ controllers which are
> part of GPIO modules ... they often deal with mechanical switches,
> buttons, and so forth. This patch:
>
> - Provides simple support for that in genirq
>
> - Includes sample implementations for some Linux systems
> which already include non-generic support for this:
>
> * Atmel SOCs (AT91, AT32 -- the same GPIO module)
> * OMAP2/OMAP3 (not quite as simple)
>
> Control over how long to debounce is less common, and often applies
> to banks of GPIOs not individual signals ... not addressed here.
>
> Drivers can request this (where available) with a new IRQF_DEBOUNCED
> flag/hint passed to request_irq():
>
> IF that flag is set when a handler is registered
> AND the relevant irq_chip supports debouncing
> AND the IRQ isn't already flagged as being debounced
> THEN the irq_chip is asked to enable debouncing for this IRQ
> UNTIL the IRQ's last handler is unregistered.
> ELSE
> nothing is done ... the hint is ignored
> FI
>
> Comments?
Note that at least for AT91 and AVR32, the terminology used is "glitch
filtering", not "debouncing". The filtering period is very short (half
a clock cycle), so it probably won't help much when dealing with
mechanical switches and buttons.
What kind of guarantees should IRQF_DEBOUNCE provide? Filtering short
glitches may be useful, but if drivers start assuming it will do real
debouncing of badly filtered switches and buttons, I think we're in for
some trouble...
> Having this mechanism in genirq would let boards remove a bunch of
> nonportable code, and would let drivers like gpio_keys, gpio_mouse,
> and various touchscreens work more reliably. It'd also let various
> SOC-specific MMC and CF card drivers switch over to more standard
> (and widely understandable) mechanisms.
>
> I'd like to submit such updates for the 2.6.28 merge window, in
> part to let mainline avoid needing yet another driver-specific
> programming interface for IRQ debouncing. (For TWL4030/TPS659x0,
> as used in most OMAP3 boards including the Gumstix Overo and the
> BeagleBoard.)
Given that the limitations of this interface are clearly documented, I'm
all for it.
What would be perhaps even more useful is generic software debouncing
support. Something like
int request_debounced_irq(int irq, unsigned long debounce_us,
void (*handler)(void *data), void *data);
which would set up a handler which disables the interrupt and sets up a
timer which will ack/unmask the interrupt and run the handler.
This would mean the "interrupt handler" really gets run in softirq
context, and shared interrupt would probably be impossible to support,
but I think it would be useful for certain kinds of interrupts.
What do you think?
Haavard
On Friday 03 October 2008, Haavard Skinnemoen wrote:
> David Brownell <[email protected]> wrote:
> > Hardware IRQ debouncing is common for IRQ controllers which are
> > part of GPIO modules ... they often deal with mechanical switches,
> > buttons, and so forth. This patch:
> >
> > - Provides simple support for that in genirq
> >
> > - Includes sample implementations for some Linux systems
> > which already include non-generic support for this:
> >
> > * Atmel SOCs (AT91, AT32 -- the same GPIO module)
> > * OMAP2/OMAP3 (not quite as simple)
> >
> > ...
>
> Note that at least for AT91 and AVR32, the terminology used is "glitch
> filtering", not "debouncing". The filtering period is very short (half
> a clock cycle), so it probably won't help much when dealing with
> mechanical switches and buttons.
Right re mechanical switching -- still needs more debouncing -- but it
would at least help with some of the contact chatter. Quoting from one
manual (at91sam9263):
When the glitch filter is enabled, a glitch with a duration of less
than 1/2 Master Clock (MCK) cycle is automatically rejected, while a
pulse with a duration of 1 Master Clock cycle or more is accepted.
For pulse durations between 1/2 Master Clock cycle and 1 Master Clock
cycle the pulse may or may not be taken into account, depending on
the precise timing of its occurrence. Thus for a pulse to be visible
it must exceed 1 Master Clock cycle, whereas for a glitch to be reliably
filtered out, its duration must not exceed 1/2 Master Clock cycle.
The mechanism is fairly typical, except for (a) duration of "one" clock
cycle, which is sometimes configurable and not infrequently more than
just one cycle, and (b) "Master" clock, which on those chips is the same
as the memory clock -- e.g. 100 MHz in normal operation, or maybe half
that on some boards -- but on other chips is a lot slower.
Example: OMAP2/OMAP3 has a configurable duration of 1 to 256 in units
of what I'll guess are really 32 KiHZ clock ticks. The default is one
tick (31 usec), so it can support up to almost 8 msec.
And on the chip I'm fighting with just now, the debounce is 30 msec,
take it or leave it.
> What kind of guarantees should IRQF_DEBOUNCE provide? Filtering short
> glitches may be useful, but if drivers start assuming it will do real
> debouncing of badly filtered switches and buttons, I think we're in for
> some trouble...
The quality-of-service question rears its ugly head ... ;)
If QOS is exposed (e.g. "unsigned debounce_usec" in the irq_desc),
that sort of begs the question of how to *change* that. I had
hoped to let someone else address the issue of such interfaces...
What would you think about advice to debounce by default on the
order of one millisecond, where hardware allows? Later, ways
to query and update that QOS could be defined.
> > Having this mechanism in genirq would let boards remove a bunch of
> > nonportable code, and would let drivers like gpio_keys, gpio_mouse,
> > and various touchscreens work more reliably. It'd also let various
> > SOC-specific MMC and CF card drivers switch over to more standard
> > (and widely understandable) mechanisms.
> >
> > I'd like to submit such updates for the 2.6.28 merge window, in
> > part to let mainline avoid needing yet another driver-specific
> > programming interface for IRQ debouncing. (For TWL4030/TPS659x0,
> > as used in most OMAP3 boards including the Gumstix Overo and the
> > BeagleBoard.)
>
> Given that the limitations of this interface are clearly documented, I'm
> all for it.
What changes would you suggest in the $SUBJECT patch then?
I notice that Documentation/DocBook/genericirq.tmpl doesn't
do a pretty job of formatting the IRQF_* parameters, but
that seems fixable.
> What would be perhaps even more useful is generic software debouncing
> support. Something like
>
> int request_debounced_irq(int irq, unsigned long debounce_us,
> void (*handler)(void *data), void *data);
>
> which would set up a handler which disables the interrupt and sets up a
> timer which will ack/unmask the interrupt and run the handler.
Why require "software debouncing" if perhaps the hardware could do
it all for you?
> This would mean the "interrupt handler" really gets run in softirq
> context, and shared interrupt would probably be impossible to support,
> but I think it would be useful for certain kinds of interrupts.
>
> What do you think?
Seems plausible.
I won't volunteer to write such a thing myself, but I can easily
imagine it starting to grow users. At least, in the embedded
Linux space ... the server/desktop crowd seems unlikely to run
with that sort of hardware.
- Dave
David Brownell <[email protected]> wrote:
> On Friday 03 October 2008, Haavard Skinnemoen wrote:
> > David Brownell <[email protected]> wrote:
> > > Hardware IRQ debouncing is common for IRQ controllers which are
> > > part of GPIO modules ... they often deal with mechanical switches,
> > > buttons, and so forth. This patch:
> > >
> > > - Provides simple support for that in genirq
> > >
> > > - Includes sample implementations for some Linux systems
> > > which already include non-generic support for this:
> > >
> > > * Atmel SOCs (AT91, AT32 -- the same GPIO module)
> > > * OMAP2/OMAP3 (not quite as simple)
> > >
> > > ...
> >
> > Note that at least for AT91 and AVR32, the terminology used is "glitch
> > filtering", not "debouncing". The filtering period is very short (half
> > a clock cycle), so it probably won't help much when dealing with
> > mechanical switches and buttons.
>
> Right re mechanical switching -- still needs more debouncing -- but it
> would at least help with some of the contact chatter. Quoting from one
> manual (at91sam9263):
>
> When the glitch filter is enabled, a glitch with a duration of less
> than 1/2 Master Clock (MCK) cycle is automatically rejected, while a
> pulse with a duration of 1 Master Clock cycle or more is accepted.
> For pulse durations between 1/2 Master Clock cycle and 1 Master Clock
> cycle the pulse may or may not be taken into account, depending on
> the precise timing of its occurrence. Thus for a pulse to be visible
> it must exceed 1 Master Clock cycle, whereas for a glitch to be reliably
> filtered out, its duration must not exceed 1/2 Master Clock cycle.
>
> The mechanism is fairly typical, except for (a) duration of "one" clock
> cycle, which is sometimes configurable and not infrequently more than
> just one cycle, and (b) "Master" clock, which on those chips is the same
> as the memory clock -- e.g. 100 MHz in normal operation, or maybe half
> that on some boards -- but on other chips is a lot slower.
Good point. I'll suggest switching this mechanism over to use a 32 kHz
clock in future designs.
> Example: OMAP2/OMAP3 has a configurable duration of 1 to 256 in units
> of what I'll guess are really 32 KiHZ clock ticks. The default is one
> tick (31 usec), so it can support up to almost 8 msec.
>
> And on the chip I'm fighting with just now, the debounce is 30 msec,
> take it or leave it.
Ok, so the limitations of various chips vary a lot...which means that
it's difficult to predict what IRQF_DEBOUNCED actually does.
> > What kind of guarantees should IRQF_DEBOUNCE provide? Filtering short
> > glitches may be useful, but if drivers start assuming it will do real
> > debouncing of badly filtered switches and buttons, I think we're in for
> > some trouble...
>
> The quality-of-service question rears its ugly head ... ;)
>
> If QOS is exposed (e.g. "unsigned debounce_usec" in the irq_desc),
> that sort of begs the question of how to *change* that. I had
> hoped to let someone else address the issue of such interfaces...
>
> What would you think about advice to debounce by default on the
> order of one millisecond, where hardware allows? Later, ways
> to query and update that QOS could be defined.
I suppose a good start would be to add a comment saying that.
On the other hand, I don't see how drivers can even tell if the
hardware supports IRQF_DEBOUNCED at all...
> > > Having this mechanism in genirq would let boards remove a bunch of
> > > nonportable code, and would let drivers like gpio_keys, gpio_mouse,
> > > and various touchscreens work more reliably. It'd also let various
> > > SOC-specific MMC and CF card drivers switch over to more standard
> > > (and widely understandable) mechanisms.
> > >
> > > I'd like to submit such updates for the 2.6.28 merge window, in
> > > part to let mainline avoid needing yet another driver-specific
> > > programming interface for IRQ debouncing. (For TWL4030/TPS659x0,
> > > as used in most OMAP3 boards including the Gumstix Overo and the
> > > BeagleBoard.)
> >
> > Given that the limitations of this interface are clearly documented, I'm
> > all for it.
>
> What changes would you suggest in the $SUBJECT patch then?
Just a comment, really. But I realize that it's difficult to specify
any guarantees since hardware may give you anything from a few
nanoseconds to 30 ms...
> I notice that Documentation/DocBook/genericirq.tmpl doesn't
> do a pretty job of formatting the IRQF_* parameters, but
> that seems fixable.
I'm not all that concerned with cosmetics...just having the information
somewhere would be nice.
> > What would be perhaps even more useful is generic software debouncing
> > support. Something like
> >
> > int request_debounced_irq(int irq, unsigned long debounce_us,
> > void (*handler)(void *data), void *data);
> >
> > which would set up a handler which disables the interrupt and sets up a
> > timer which will ack/unmask the interrupt and run the handler.
>
> Why require "software debouncing" if perhaps the hardware could do
> it all for you?
Because of the "perhaps" part of your sentence.
But ok, drivers really shouldn't have to care, so let's call it
"generic debouncing support".
> > This would mean the "interrupt handler" really gets run in softirq
> > context, and shared interrupt would probably be impossible to support,
> > but I think it would be useful for certain kinds of interrupts.
> >
> > What do you think?
>
> Seems plausible.
>
> I won't volunteer to write such a thing myself, but I can easily
> imagine it starting to grow users. At least, in the embedded
> Linux space ... the server/desktop crowd seems unlikely to run
> with that sort of hardware.
Maybe we should just add this interface and drop the flag? A flag will
never be able to convey some important parameters like how long to
debounce. Then a irq chip implementation can decide to do it in
hardware if the requested debounce delay matches what the hardware can
provide.
Maybe we should let drivers provide a range of acceptable delays so
that the irq chip driver won't have to guess at how long it is
acceptable to deviate from the specified delay.
Haavard
On Wednesday 08 October 2008, Haavard Skinnemoen wrote:
>
> Ok, so the limitations of various chips vary a lot...which means that
> it's difficult to predict what IRQF_DEBOUNCED actually does.
The specific QOS achieved is system-specific; the term for
that kind of mechanism is "hinting". It's very clearly defined
what the hint means .. but a given system might not use it.
The madvise(2) system call is a userspace example of hinting.
> > > What kind of guarantees should IRQF_DEBOUNCE provide? Filtering short
> > > glitches may be useful, but if drivers start assuming it will do real
> > > debouncing of badly filtered switches and buttons, I think we're in for
> > > some trouble...
> >
> > The quality-of-service question rears its ugly head ... ;)
> >
> > If QOS is exposed (e.g. "unsigned debounce_usec" in the irq_desc),
> > that sort of begs the question of how to *change* that. I had
> > hoped to let someone else address the issue of such interfaces...
> >
> > What would you think about advice to debounce by default on the
> > order of one millisecond, where hardware allows? Later, ways
> > to query and update that QOS could be defined.
>
> I suppose a good start would be to add a comment saying that.
>
> On the other hand, I don't see how drivers can even tell if the
> hardware supports IRQF_DEBOUNCED at all...
That is, "On the other hand, 'later' is not yet..." ?
Are you suggesting that debouncing support shouldn't
be provided without QOS query/update support?
> > > > Having this mechanism in genirq would let boards remove a bunch of
> > > > nonportable code, and would let drivers like gpio_keys, gpio_mouse,
> > > > and various touchscreens work more reliably. It'd also let various
> > > > SOC-specific MMC and CF card drivers switch over to more standard
> > > > (and widely understandable) mechanisms.
> > > >
> > > > I'd like to submit such updates for the 2.6.28 merge window, in
> > > > part to let mainline avoid needing yet another driver-specific
> > > > programming interface for IRQ debouncing. (For TWL4030/TPS659x0,
> > > > as used in most OMAP3 boards including the Gumstix Overo and the
> > > > BeagleBoard.)
> > >
> > > Given that the limitations of this interface are clearly documented, I'm
> > > all for it.
> >
> > What changes would you suggest in the $SUBJECT patch then?
>
> Just a comment, really. But I realize that it's difficult to specify
> any guarantees since hardware may give you anything from a few
> nanoseconds to 30 ms...
Done: "as close to 1 msec as hardware allows". (I think less
than that is probably too little, and more would likely be OK.)
> > > What would be perhaps even more useful is generic software debouncing
> > > support. Something like
> > >
> > > int request_debounced_irq(int irq, unsigned long debounce_us,
> > > void (*handler)(void *data), void *data);
Note by the way what I think is a problematic assumption there:
that this *exact* debounce period matters. It seems to be more
usual that it just fit into a range of reasonable values; a bit
more or less won't matter, almost by definition.
(And also, that routine is less functional than request_irq ...)
> > > which would set up a handler which disables the interrupt and sets up a
> > > timer which will ack/unmask the interrupt and run the handler.
> >
> > Why require "software debouncing" if perhaps the hardware could do
> > it all for you?
>
> Because of the "perhaps" part of your sentence.
I'm not sure which sentence you refer too, but the first
"perhaps" above is yours! :)
> But ok, drivers really shouldn't have to care, so let's call it
> "generic debouncing support".
OK..
> > > This would mean the "interrupt handler" really gets run in softirq
> > > context, and shared interrupt would probably be impossible to support,
> > > but I think it would be useful for certain kinds of interrupts.
> > >
> > > What do you think?
> >
> > Seems plausible.
> >
> > I won't volunteer to write such a thing myself, but I can easily
> > imagine it starting to grow users. At least, in the embedded
> > Linux space ... the server/desktop crowd seems unlikely to run
> > with that sort of hardware.
>
> Maybe we should just add this interface and drop the flag?
What I like about the flag is that it's really simple, a
"fire and forget" model. Easy for drivers to use. And it
need not be incompatible with a fancier interface...
The debounce() method might usefully be changed to take the
requested delay in microseconds, instead of a boolean. And
to return the actual delay. That would make it easier to
support fancier calls later, maybe just exposing the raw
irq_chip call like
usecs = set_irq_debounce(irq, range_spec);
The notion of a request_debounced_irq() needs more cooking
yet though, IMO.
> A flag will
> never be able to convey some important parameters like how long to
> debounce.
But how important *is* that detail to most drivers? In practice.
I susct pethey pick numbers today since they have to debounce with
software timers, which require numbers.
> Then a irq chip implementation can decide to do it in
> hardware if the requested debounce delay matches what the hardware can
> provide.
I think irq_chip calls should be limited to hardware support.
Keep them really simple; put layers on top of them if needed.
> Maybe we should let drivers provide a range of acceptable delays so
> that the irq chip driver won't have to guess at how long it is
> acceptable to deviate from the specified delay.
I can't see it working otherwise. Of course, maybe there should
just be generic ranges rather than expecting drivers to understand
how springy contacts might be on a given board, or how dirty they
may be to cause other kinds of chatter.
- Dave
David Brownell <[email protected]> wrote:
> On Wednesday 08 October 2008, Haavard Skinnemoen wrote:
> >
> > Ok, so the limitations of various chips vary a lot...which means that
> > it's difficult to predict what IRQF_DEBOUNCED actually does.
>
> The specific QOS achieved is system-specific; the term for
> that kind of mechanism is "hinting". It's very clearly defined
> what the hint means .. but a given system might not use it.
I know that, but is "hinting" really what drivers need?
> The madvise(2) system call is a userspace example of hinting.
That's different. Ignoring madvise() hints might hurt performance
slightly, but it won't result in any fundamentally different behaviour.
On the other hand, lack of debouncing might cause the gpio_keys driver
to report 1000 keypresses instead of one when the user pushes a button.
That's much more harmful.
So if someone goes and replaces the debounce timer in gpio_keys with
this IRQF_DEBOUNCE flag, it might work very well on hardware which
happens to use a 30 ms debounce interval, but will break horribly on
hardware with shorter debounce intervals.
> > > What would you think about advice to debounce by default on the
> > > order of one millisecond, where hardware allows? Later, ways
> > > to query and update that QOS could be defined.
> >
> > I suppose a good start would be to add a comment saying that.
> >
> > On the other hand, I don't see how drivers can even tell if the
> > hardware supports IRQF_DEBOUNCED at all...
>
> That is, "On the other hand, 'later' is not yet..." ?
Right.
> Are you suggesting that debouncing support shouldn't
> be provided without QOS query/update support?
Sort of. I'm just wondering how drivers can rely on a feature which
provides such vague promises.
> > Just a comment, really. But I realize that it's difficult to specify
> > any guarantees since hardware may give you anything from a few
> > nanoseconds to 30 ms...
>
> Done: "as close to 1 msec as hardware allows". (I think less
> than that is probably too little, and more would likely be OK.)
It could be zero on hardware which doesn't support debouncing at all.
> > > > What would be perhaps even more useful is generic software debouncing
> > > > support. Something like
> > > >
> > > > int request_debounced_irq(int irq, unsigned long debounce_us,
> > > > void (*handler)(void *data), void *data);
>
> Note by the way what I think is a problematic assumption there:
> that this *exact* debounce period matters. It seems to be more
> usual that it just fit into a range of reasonable values; a bit
> more or less won't matter, almost by definition.
Yes, I actually came to that conclusion myself, as you've already seen
below.
> (And also, that routine is less functional than request_irq ...)
I guess we could add a "flags" parameter if that's what you mean.
> > > > which would set up a handler which disables the interrupt and sets up a
> > > > timer which will ack/unmask the interrupt and run the handler.
> > >
> > > Why require "software debouncing" if perhaps the hardware could do
> > > it all for you?
> >
> > Because of the "perhaps" part of your sentence.
>
> I'm not sure which sentence you refer too, but the first
> "perhaps" above is yours! :)
I mean that it's difficult to rely on hardware that "perhaps" can do
debouncing for you. I think many drivers need to know for sure.
> > Maybe we should just add this interface and drop the flag?
>
> What I like about the flag is that it's really simple, a
> "fire and forget" model. Easy for drivers to use. And it
> need not be incompatible with a fancier interface...
Could be. But I think the examples you provided (gpio_keys and
gpio_mouse) need stronger guarantees before they can drop their
debouncing timers.
> The debounce() method might usefully be changed to take the
> requested delay in microseconds, instead of a boolean. And
> to return the actual delay. That would make it easier to
> support fancier calls later, maybe just exposing the raw
> irq_chip call like
>
> usecs = set_irq_debounce(irq, range_spec);
Maybe that's better. Just request the interrupt as normal, and then try
to enable debouncing. If the actual delay is too short, the driver can
set up its own timer.
> The notion of a request_debounced_irq() needs more cooking
> yet though, IMO.
Yes, it was only a suggestion, I didn't mean to provide a fully-cooked
interface. But I think something like that (or set_irq_debounce) would
be useful to many drivers.
> > A flag will
> > never be able to convey some important parameters like how long to
> > debounce.
>
> But how important *is* that detail to most drivers? In practice.
> I susct pethey pick numbers today since they have to debounce with
> software timers, which require numbers.
I think it's very important to drivers that have to deal with
mechanical buttons and switches. In many cases, only the board code
knows what kind of switch is hooked up to the interrupt, so the driver
just passes on that information from the platform data.
> > Then a irq chip implementation can decide to do it in
> > hardware if the requested debounce delay matches what the hardware can
> > provide.
>
> I think irq_chip calls should be limited to hardware support.
> Keep them really simple; put layers on top of them if needed.
Ok, so let's just pass the range spec to the debounce() method, and if
it fails, the genirq core can decide to do debouncing in software.
> > Maybe we should let drivers provide a range of acceptable delays so
> > that the irq chip driver won't have to guess at how long it is
> > acceptable to deviate from the specified delay.
>
> I can't see it working otherwise. Of course, maybe there should
> just be generic ranges rather than expecting drivers to understand
> how springy contacts might be on a given board, or how dirty they
> may be to cause other kinds of chatter.
Maybe. I'd prefer real numbers to generic-sounding names like
low/medium/high, but if someone can come up with a good way to express
this, I guess that could work.
And I don't really expect drivers to understand this, but drivers can
get help from the platform or board code.
Haavard
Hi!
> > > Ok, so the limitations of various chips vary a lot...which means that
> > > it's difficult to predict what IRQF_DEBOUNCED actually does.
> >
> > The specific QOS achieved is system-specific; the term for
> > that kind of mechanism is "hinting". It's very clearly defined
> > what the hint means .. but a given system might not use it.
>
> I know that, but is "hinting" really what drivers need?
>
> > The madvise(2) system call is a userspace example of hinting.
>
> That's different. Ignoring madvise() hints might hurt performance
> slightly, but it won't result in any fundamentally different behaviour.
>
> On the other hand, lack of debouncing might cause the gpio_keys driver
> to report 1000 keypresses instead of one when the user pushes a button.
> That's much more harmful.
>
> So if someone goes and replaces the debounce timer in gpio_keys with
> this IRQF_DEBOUNCE flag, it might work very well on hardware which
> happens to use a 30 ms debounce interval, but will break horribly on
> hardware with shorter debounce intervals.
Right. So you don't _replace_ debounce timer, you just do both timer
and IRQF_.
> > > > Why require "software debouncing" if perhaps the hardware could do
> > > > it all for you?
> > >
> > > Because of the "perhaps" part of your sentence.
> >
> > I'm not sure which sentence you refer too, but the first
> > "perhaps" above is yours! :)
>
> I mean that it's difficult to rely on hardware that "perhaps" can do
> debouncing for you. I think many drivers need to know for sure.
Why? You write code as if no debouncing exist...
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thursday 09 October 2008, Haavard Skinnemoen wrote:
>
> On the other hand, lack of debouncing might cause the gpio_keys driver
> to report 1000 keypresses instead of one when the user pushes a button.
> That's much more harmful.
>
> So if someone goes and replaces the debounce timer in gpio_keys with
> this IRQF_DEBOUNCE flag, it might work very well on hardware which
> happens to use a 30 ms debounce interval, but will break horribly on
> hardware with shorter debounce intervals.
Agreed that if you want to _replace_ software debouncing with a flag
that's a pure hint, it wouldn't work. But I didn't suggest that...
Instead, if you just added IRQF_DEBOUNCE to the existing gpio_keys,
what you'd get is a reduction in useless IRQs received. Which is
a behavioral improvement, coming at essentially zero system cost.
(After discounting the cost of "how to do it" discussions!)
(As Pavel also observed.)
> > > > What would you think about advice to debounce by default on the
> > > > order of one millisecond, where hardware allows? Later, ways
> > > > to query and update that QOS could be defined.
> > >
> > > I suppose a good start would be to add a comment saying that.
> > >
> > > On the other hand, I don't see how drivers can even tell if the
> > > hardware supports IRQF_DEBOUNCED at all...
> >
> > That is, "On the other hand, 'later' is not yet..." ?
>
> Right.
>
> > Are you suggesting that debouncing support shouldn't
> > be provided without QOS query/update support?
>
> Sort of. I'm just wondering how drivers can rely on a feature which
> provides such vague promises.
As with madvise(), by just hinting "here's a way to improve
behavior a bit, if hardware allows". As you noted, ignoring
such hints would mean some missed optimizations ... but the
system would still behave correctly.
> > The debounce() method might usefully be changed to take the
> > requested delay in microseconds, instead of a boolean. And
> > to return the actual delay. That would make it easier to
> > support fancier calls later, maybe just exposing the raw
> > irq_chip call like
> >
> > usecs = set_irq_debounce(irq, range_spec);
>
> Maybe that's better. Just request the interrupt as normal, and then try
> to enable debouncing. If the actual delay is too short, the driver can
> set up its own timer.
So we seem to be thinking about two mechanisms:
- I focussed on hinting, as a lightweight way to kick in a
common hardware mechanism to improve system behavior;
- You focussed on abstracting the whole debouncing problem,
leveraging those mechanisms more highly and (implicitly)
helping improve much messy software debounce logic.
Still seems to me there should be no problem having both
mechanisms, supported by one irq_chip.debounce() hook
that's a little different from the one in $PATCH. And I
think you agreed with that.
Specifically with respect to the Atmel GPIO modules, the
hinting would eliminate a few glitches but would not be as
powerful as on some other hardware. (As you had observed
earlier.) So if I were wearing the same corporate hat you
are, I'd want Linux to support that second mechanism too!!
> > > A flag will
> > > never be able to convey some important parameters like how long to
> > > debounce.
> >
> > But how important *is* that detail to most drivers? In practice.
> > I suspect they pick numbers today since they have to debounce with
> > software timers, which require numbers.
>
> I think it's very important to drivers that have to deal with
> mechanical buttons and switches. In many cases, only the board code
> knows what kind of switch is hooked up to the interrupt, so the driver
> just passes on that information from the platform data.
If the platform setup code knows the hardware debounce will
be requested, it can adjust its platform_data debounce parameter
accordingly.
- Dave
David Brownell <[email protected]> wrote:
> On Thursday 09 October 2008, Haavard Skinnemoen wrote:
> >
> > On the other hand, lack of debouncing might cause the gpio_keys driver
> > to report 1000 keypresses instead of one when the user pushes a button.
> > That's much more harmful.
> >
> > So if someone goes and replaces the debounce timer in gpio_keys with
> > this IRQF_DEBOUNCE flag, it might work very well on hardware which
> > happens to use a 30 ms debounce interval, but will break horribly on
> > hardware with shorter debounce intervals.
>
> Agreed that if you want to _replace_ software debouncing with a flag
> that's a pure hint, it wouldn't work. But I didn't suggest that...
>
> Instead, if you just added IRQF_DEBOUNCE to the existing gpio_keys,
> what you'd get is a reduction in useless IRQs received. Which is
> a behavioral improvement, coming at essentially zero system cost.
> (After discounting the cost of "how to do it" discussions!)
Ok, I see. I didn't realize that gpio_keys doesn't disable the
interrupt while debouncing.
> > Sort of. I'm just wondering how drivers can rely on a feature which
> > provides such vague promises.
>
> As with madvise(), by just hinting "here's a way to improve
> behavior a bit, if hardware allows". As you noted, ignoring
> such hints would mean some missed optimizations ... but the
> system would still behave correctly.
Right.
> > Maybe that's better. Just request the interrupt as normal, and then try
> > to enable debouncing. If the actual delay is too short, the driver can
> > set up its own timer.
>
> So we seem to be thinking about two mechanisms:
>
> - I focussed on hinting, as a lightweight way to kick in a
> common hardware mechanism to improve system behavior;
>
> - You focussed on abstracting the whole debouncing problem,
> leveraging those mechanisms more highly and (implicitly)
> helping improve much messy software debounce logic.
Yes, you're right.
> Still seems to me there should be no problem having both
> mechanisms, supported by one irq_chip.debounce() hook
> that's a little different from the one in $PATCH. And I
> think you agreed with that.
>
> Specifically with respect to the Atmel GPIO modules, the
> hinting would eliminate a few glitches but would not be as
> powerful as on some other hardware. (As you had observed
> earlier.) So if I were wearing the same corporate hat you
> are, I'd want Linux to support that second mechanism too!!
I just want a mechanism where you'll end up with the same behaviour
regardless of what the hardware supports, at the cost of a timer if the
hardware doesn't support the requested debounce delay. As an added
benefit, this would eliminate the need for drivers to set up their own
debounce timers. It would also make such "more powerful" hardware
features easier to utilize from generic drivers, wouldn't it?
But you've convinced me that your IRQF_DEBOUNCE hint might be useful
too.
> > I think it's very important to drivers that have to deal with
> > mechanical buttons and switches. In many cases, only the board code
> > knows what kind of switch is hooked up to the interrupt, so the driver
> > just passes on that information from the platform data.
>
> If the platform setup code knows the hardware debounce will
> be requested, it can adjust its platform_data debounce parameter
> accordingly.
I guess it could, but why would it do that? The debounce delay
shouldn't depend on the mechanism used to implement it, should it?
Or are you thinking about compensating for any delays introduced by
IRQF_DEBOUNCE?
Haavard
On Sunday 12 October 2008, Haavard Skinnemoen wrote:
> I just want a mechanism where you'll end up with the same behaviour
> regardless of what the hardware supports, at the cost of a timer if the
> hardware doesn't support the requested debounce delay. As an added
> benefit, this would eliminate the need for drivers to set up their own
> debounce timers. It would also make such "more powerful" hardware
> features easier to utilize from generic drivers, wouldn't it?
>
> But you've convinced me that your IRQF_DEBOUNCE hint might be useful
> too.
Just for the record ... I'm withdrawing this proposal, in the sense
that I won't be pursuing it any further. A lighter weight solution
is enough for now.
I think the framework we discussed -- lowlevel irq_chip mechanism
that can kick in hardware debouncing and report the duration of its
debounce delay, paired with upper level code that can kick in longer
software timers as needed -- is probably the right direction to go,
if anyone really needs this.
> > If the platform setup code knows the hardware debounce will
> > be requested, it can adjust its platform_data debounce parameter
> > accordingly.
>
> I guess it could, but why would it do that? The debounce delay
> shouldn't depend on the mechanism used to implement it, should it?
The overall delay shouldn't matter, no. But if an 10 msec hardware
debounce kicks in and you wanted 20 msec total, you'd want something
to conclude that a 10 msec software timer is more appropriate than
a 20 msec one. I was pointing out that in one construction, that
"something" would be platform setup code which knows more about how
the system is set up than a "dumb" software timer in that driver.
- Dave