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. BTW, do you have it configured as level or edge interrupt? > 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. -- Dmitry -- 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