Message-Id: <200810090234.41243.david-b@pacbell.net>
Date:	Thu, 9 Oct 2008 02:34:40 -0700
From:	David Brownell <david-b@...bell.net>
To:	Haavard Skinnemoen <haavard.skinnemoen@...el.com>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Andrew Victor <linux@...im.org.za>,
	Kevin Hilman <khilman@...prootsystems.com>,
	Tony Lindgren <tony@...mide.com>
Subject: Re: [PATCH/RFC] hardware irq debouncing support

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists