Messages in this thread | | | Date | Sat, 20 Mar 2010 05:44:40 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH] gpio: add interrupt handling capability to max732x |
| |
On Fri, 19 Mar 2010 16:41:56 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 16 Mar 2010 11:34:52 +0100 > Marc Zyngier <maz@misterjones.org> wrote: > > > +static irqreturn_t max732x_irq_handler(int irq, void *devid) > > +{ > > + struct max732x_chip *chip = devid; > > + uint8_t pending; > > + uint8_t level; > > + > > + pending = max732x_irq_pending(chip); > > + > > + if (!pending) > > + return IRQ_HANDLED; > > Should be IRQ_NONE?
The main problem is that some of the supported expanders do not have a hardware interrupt mask, so the IRQ will fire on each input pin toggle, including the ones we don't care about (pca953x has the same problem).
Returning IRQ_NONE will cause the irq core to quickly disable the line. Not good either. I could add a test for the mask feature bit, and return IRQ_NONE only in that case.
> If this device was on a shared interrupt line and the interrupt was > caused by some other device, and this handler is called first then the > incorrect IRQ_HANDLED return could cause the irq core to do wrong things.
This is exactly why the irq handler is requested without the SHARED flag:
+ ret = request_threaded_irq(client->irq, + NULL, + max732x_irq_handler, + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, + dev_name(&client->dev), chip);
The same mask feature bit could be used to allow shared interrupts with devices that support it.
Thanks,
M. -- I'm the slime oozin' out from your TV set...
| |