linux-kernel - Re: [PATCH/RFC] hardware irq debouncing support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081012144638.4dfdbff8@hskinnemo-gx745.norway.atmel.com>
Date:	Sun, 12 Oct 2008 14:46:38 +0200
From:	Haavard Skinnemoen <haavard.skinnemoen@...el.com>
To:	David Brownell <david-b@...bell.net>
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

David Brownell <david-b@...bell.net> 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
--
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/