Hi Dmitry On Thu, 1 Dec 2011 16:48:18 +0800 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > On Thu, Dec 01, 2011 at 02:30:12PM +0800, Feng Tang wrote: > > Hi Dmitry, > > > > On Thu, 1 Dec 2011 14:10:15 +0800 > > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > > > > On Wed, Nov 30, 2011 at 10:08:24AM +0800, Feng Tang wrote: > > > > Hi Dmitry, > > > > > > > > Thanks for the review. > > > > > > > > On Tue, 29 Nov 2011 17:22:08 +0800 > > > > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > > > > > > > > Hi Feng, > > > > > > > > > > On Tue, Nov 29, 2011 at 04:12:57PM +0800, Feng Tang wrote: > > > > > > The TSC2007 data sheet say in some case the HW may fire some > > > > > > false interrrupt, which I also met during integrating one > > > > > > TSC2007 device. So add the disable_irq/enable_irq protection > > > > > > around data handling. > > > > > > > > > > IRQF_ONESHOT should prevent IRQ from firing again while > > > > > thread is servicing it. Did you actually observe it not > > > > > working? > > > > > > > > You are right, the tsc's threaded IRQ function is not > > > > re-entered, and the driver is working actually. My bad not > > > > stating the problem clearly. The real problem I want solve is, > > > > many platforms including ours use a GPIO line as the tsc2007's > > > > IRQ line, and when these extra tsc2007 IRQ is triggered on the > > > > gpio line, that GPIO controller will fire up extra noise IRQ > > > > accordingly, causing its ISR to be called. And my patch is > > > > trying to let the GPIO controller driver disable that specific > > > > IRQ pin from tsc2007. As disable_irq will call GPIO irq_chip's > > > > irq_disable() or mask() hook. > > > > > > But ONESHOT interrupt handler will not unmask interrupt until > > > thead finishes servicing it so we should not be seeing these > > > extra IRQs. I'm adding Thomas in case I misunderstand how it > > > threaded IRQ supposed to work. > > > > I did see these extra IRQs. As the tsc2007 datasheet says, the > > PENIRQ may be falsely triggered, and that signal is passed to the > > GPIO controller, if the tsc2007 specific pin is not disabled in > > GPIOC level, then the GPIOC HW will send out a IRQ anyway. > > > > While calling the disable_irq(), it will call the irq_chip's > > (implemented by GPIO controller) irq_disable() or irq_mask() hook > > to disable that specific line for tsc2007. > > > > I did check the original tsc2007 driver, which used the > > disable_irq/enable_irq too, which means this problem is a general > > one and has been seen before. > > > > No, the original had disable/enable IRQ because it was the only way to > stop interrupt storm with combination of hard IRQ + workqueue or > thread. My understanding is, if the GPIO line used by tsc2007 is not disabled in GPIO controller level, the GPIOC will always be triggered by tsc2007 to fire those extra interrupts. Actually I did try the old version driver (no threaded irq version), which will see the extra interrupts if the disable_irq/enable_irq is removed. > > BTW, do you have it configured as level or edge interrupt? I'm now using the falling edge trigger, but I tried the level trigger which will have more interrupts storm. Thanks, Feng > > > Or should we have a another flag in tsc2007 platform data, to let > > each platform chose whether or not to use the disable/enable_irq > > according to their platform need. > > > > > > > > Also, there is clear_penirq() platform method that is called to > > > clean penirq state, if needed. > > > > Sadly we don't have a way to clear the irq from TSC2007 on our > > platform :( > > > > Thanks. > -- 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
