Messages in this thread | | | Date | Fri, 2 Oct 2009 16:05:18 +0200 | From | Samuel Ortiz <> | Subject | Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCDBacklight and Keypad Input Device Driver |
| |
Hi Michael,
On Fri, Oct 02, 2009 at 02:48:27PM +0100, Hennerich, Michael wrote: > >From: uclinux-dist-devel-bounces@blackfin.uclinux.org > [mailto:uclinux-dist-devel- > >bounces@blackfin.uclinux.org] On Behalf Of Hennerich, Michael > >Device Driver > > > >Hi Samuel, > > > >>From: Samuel Ortiz [mailto:sameo@linux.intel.com] > >>Hi Mike, > >> > >>Some comments on this patch: > >> > >>On Wed, Sep 23, 2009 at 01:11:04AM -0400, Mike Frysinger wrote: > >>> +static irqreturn_t adp5520_irq_handler(int irq, void *data) > >>> +{ > >>> + struct adp5520_chip *chip = data; > >>> + > >>> + disable_irq_nosync(irq); > >>> + schedule_work(&chip->irq_work); > >>Have you considered using a threaded irq handler here ? > > > >The Linux version I developed this driver on didn't feature threaded > irq > >handlers. > >But thanks I'm going to take a look here. > > Hi Samuel, > > Well the threaded irq handlers are no option here, since we use a Level > Sensitive Interrupt. > The work queue here is to schedule the main irq handler outside hardirq > context. > I2C can't we invoked form none sleepy context, so we can't clear the > interrupt. > This will cause that we execute the hardirq over and over again, > preventing the irq thread to be run. > > The threaded irqs with its current implementation also doesn't allow me > to disable the irq in the hardirq handler. Ok, that's the piece I missed. Thanks for the clarification.
> There have been some discussions about this on lkml recently. > Until there is a way to workaround this issue (handle_level_oneshot_irq, > etc.), > I like to stick with: Fine with me, yes.
Cheers, Samuel.
> >>> + disable_irq_nosync(irq); > >>> + schedule_work(&chip->irq_work); > > Best regards, > Michael >
-- Intel Open Source Technology Centre http://oss.intel.com/
| |