Re: [PATCH] OMAP2/3 Avoid GPIO pending irq status been set after irq_disable

[Kevin Hilman <khilman@deeprootsystems.com>]

"Wang Sawsd-A24013" <cqwang@motorola.com> writes:

>  
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org 
>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Wang 
>> Sawsd-A24013
>> Sent: 2009年6月5日 1:43
>> To: Kevin Hilman
>> Cc: linux-omap@vger.kernel.org; nm@ti.com; Mike Chan
>> Subject: RE: [PATCH] OMAP2/3 Avoid GPIO pending irq status 
>> been set after irq_disable
>> 
>> 
>> 
>> > -----Original Message-----
>> > From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>> > Sent: 2009年6月5日 1:04
>> > 
>> > Dumb question: Why use level?  Why not use falling edge for this?
>> > 
>> A good question, :-) We did use edge interrupt before, see 
>> the reason below.
>> 
>> > > The issue is, after the touch driver calls irq_disable, 
>> since it is
>> > > empty unless
>> > > Set the IRQ_DISABLED flag, so next time only the generic handler
>> > > function
>> > > handle_level_irq is called, this handler just ack to OMAP 
>> > GPIO IRQ that
>> > > is
>> > > To clear the IRQ status and mask the GPIO on OMAP side, 
>> but since NO
>> > > Touch driver IRQ action is called, so the touch Controller 
>> > can not gets
>> > > acknowledged, then the interrupt line will be always driven 
>> > low by the
>> > > external controller, 
>> > 
>> > If the GPIO is set to be edge triggered, the fact that it 
>> is still low
>> > won't matter, the genirq layer will have noticed a pending 
>> interrupt.
>> > 
>> If we use edge interrupt here, the potential issue is still 
>> existing, and also
>> We are liky to lose the interrupt.
>> After irq_disable and before HW suspend, if the interrupt 
>> line is driven low,
>> Then OMAP GPIO can catch this edge transition, so the IRQ is set,
>> Then the handle_edge_irq will clear the IRQ staus and mask the IRQ.
>> Since the controller is not ACKed, then the interrupt line is 
>> always driven low,
>> Then from then on, no edge can happen, and no more Touch 
>> interrupt can happen
>> Even when irq_enable is called, though we have the prepare 
>> for idle hooks,
>> But that only work when the edge transition happens after 
>> that prepare call,
>> Since it saves the GPIO data IN at that time, if the input 
>> level already changes
>> Before that time, then the workaround does not work. 
>> 
>> We ever made another patch to not only compare the data in, 
>> but also check
>> It is rising or falling edge, and check the CURRENT input 
>> level to decide whether to
>> Use LEVEL detect to manually trigger the interrupt, it works 
>> fine with our HW.
>> But later on, touch cotroller driver is updated to use level 
>> detect, then we
>> Met this issue. I think the patch we made to workaround the 
>> edge int lost is also needed
>> In current pm branch, currently we may still face the issue I 
>> mentioned above.
>
> I rechecked the code, seems the issue will not be there since
> The handle_edge_irq can resend irq during resume time, then

Yes, I was actually replying to ask you to check why the retrigger
wasn't happening in your kernel.  

> I checked our issue log and found, the reason that we lose
> The edge interrupt is because we were using omapzoom kernel

I was starting to think you were not actually using the linux-omap
kernel (looks like I was right.)

> And put PER to fully HW supervised mode and we didn't use
> The prepare idle hooks in our idle function call, then the issues
> Happens when PER is in RET/OFF state but the touch interrupt happens.
>
> With linux-omap kernel, seems using edge interrupt can just workaround
> The touch issue, 

Good.

>but I think the issue is still there, we can not expect All the GPIO
>interrupts to be edge type, and we can not expect all the edge
>Interrupts to fire only once, with open platforms, every kind of
>peripherals We may use, 

I completely agree.  You have definitely found a robustness problem in
the GPIO core that will be relatively easy to run into in the future.

>the change to root fix this problem should be still clearing The
>level/edge detection when irq_disable is called. This will not cause
>Extra interrupt loss since we still have the prepare/resume hooks to
>check Gpio input and retrigger the interrupts.

What do you think about disabling the level/edge detection when
disable_irq_wake() is called instead?  This seems more logical
and expected.

Kevin

P.S., are you wanting to use your touchscreen as a wakeup source?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html