RE: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCDBacklight and Keypad Input Device Driver

[Hennerich, Michael]

>From: uclinux-dist-devel-bounces@xxxxxxxxxxxxxxxxxxxx
[mailto:uclinux-dist-devel-
>bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of Hennerich, Michael
>Device Driver
>
>Hi Samuel,
>
>>From: Samuel Ortiz [mailto:sameo@xxxxxxxxxxxxxxx]
>>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.

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: 

>>> +	disable_irq_nosync(irq);
>>> +	schedule_work(&chip->irq_work);

Best regards,
Michael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/