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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> writes:

> On Mon, Oct 12, 2009 at 07:09:50PM +0200, Ferenc Wagner wrote:
>
>> Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> writes:
>>
>>> On Tue, Sep 22, 2009 at 09:06:05PM +0200, Ferenc Wagner wrote:
>>>
>>>> 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.

I'd appreciate hearing your opinion on the above, too.

>> 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.

And on this...

>>>> 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.

This scheme depends on triggering on the falling edge or otherwise
polling until the line goes inactive again, so that new interrupts can
be registered.  Sounds not so good.  But anyway, it's mostly academic
pondering, as we're dealing with level-triggered interrupts now.

>>>>> On Tue, Sep 22, 2009 at 05:14:22PM +0200, Ferenc Wagner wrote:
>>>>>
>>>>>> 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)...
>
> 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.

I see, but I also feel like the simplification is at most negligible.
The very same code which installs the one-button-handlers could be
used in a single handler to test each gpio in turn.  And we don't go
against common practice.
-- 
Thanks,
Feri.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux