Thread (2 messages) 2 messages, 2 authors, 2009-10-14

Re: [PATCH] input: make gpio-keys use IRQF_SHARED

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2009-10-14 08:10:38

On Mon, Oct 12, 2009 at 07:09:50PM +0200, Ferenc Wagner wrote:
Ferenc Wagner [off-list ref] writes:
quoted
Dmitry Torokhov [off-list ref] writes:
quoted
On Tue, Sep 22, 2009 at 09:06:05PM +0200, Ferenc Wagner wrote:
quoted
Dmitry Torokhov [off-list ref] writes:
quoted
On Tue, Sep 22, 2009 at 05:14:22PM +0200, Ferenc Wagner wrote:
quoted
    The gpio_get_value function may sleep, so it should not be
    called in a timer function.
So is drivers/staging/dream/gpio_input.c in error, too?
I guess so. Initially gpio method did not sleep but that has changed.
quoted
quoted
quoted
quoted
quoted
But I don't see why it could sleep, is that really the case?
There are things like i2c gpio extenders that require access to
slow buses and can sleep.
Please read my other reply in this thread before the following.
All this seems to mean that using level triggered interrupts on
such devices is impossible, unless we find a way to acknowledge the
interrupt without GPIO access.
You probably want to look into threaded interrupt handlers and
IRQF_ONESHOT. These can't be shared though, so it looks like you
need nested IRQ handlers infrastructure.
This sounds like a job for the irqchip setup code, if I understand
http://lkml.org/lkml/2009/8/15/133 correctly, that is, no driver
business.  Doesn't that apply only when the irqchip itself is on a
slow bus?  I find IRQF_ONESHOT more relevant, and sharing such a beast
would be possible in principle, although a little complicated, as
http://lkml.org/lkml/2009/8/15/131 asserts.  But still, how would the
somewhat more latency-sensitive serial port on the same interrupt line
tolerate its interrupt staying masked for a considerable period?  Even
if there was no hardware which shared interrupts between slow and fast
devices (which I hope), a driver blindly using oneshot interrupts
would unnecessarily add the scheduling delay to the masked period
instead of acknowledging and unmasking the line from hardirq context.
Please correct me if I got these wrong.

On the other hand, querying gpio_cansleep could be used to avoid this,
and I can't conceive how the IRQ core could find out and do what's
best for the driver in such cases.
quoted
quoted
But level triggering is needed for sharing.
I believe that both level and edge-triggered interrupts can be shared.
Sure, but all parties must agree on the trigger type, and level
triggering seems to be the norm (I've read
http://lkml.org/lkml/1998/8/7/30 on the unreliability of shared edge
triggered interrupts, but I don't buy Linus' argument, because the
system should keep asking the devices until none of them needs
servicing -- ineffective, but reliable).  Anyway, in my (and therefore
the most important) case the serial console grabs the interrupt first,
and although it's willing to share it, it uses level triggering, so
I've got no choice.
quoted
quoted
quoted
quoted
Also, commit 57ffe9d539e0eb741bb9ca8f2834d210e70ee2e3 removed the
possibility of telling apart different keys, so that should be
reverted during the process.  I already asked Uwe Kleine-König
about the whys, but didn't get a reply.
I don't see why you say that... You request IRQ per button and you get
that button structure as argument in the interrupt handler.
In practice, several buttons often share a single IRQ line, possibly
even with other hardware, like the serial port in my case (as
described in my other reply).  So generally you need the full platform
data for all GPIO buttons in the handler, to find out which generated
the interrupt.
Your interrupt handler will get called for every button on that IRQ line
and you can query button state I think.
Well, yes, if I register the handler once for each button.  Is that
really preferable?  It didn't occur to me as the current code does not
use shared interrupts, so it's out of question.  Sigh, the generic
GPIO interface is already rather inefficient, accessing only a single
bit per call (cf. the first block comment in tosakbd.c)...
Or did I get you wrong again?
I'd much appreciate some education on the above points as well, if
your time permits.
I think it simplifies thigs a bit. After all we are not talking about
device delivering interrupts constantly at a very high rate so
simplicity may be preferable over ultimate performance here.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help