Messages in this thread | | | Date | Sat, 19 Jul 2008 00:18:27 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH] IRQ : bug fix in setup_irq |
| |
On Fri, 18 Jul 2008, sasa sasa wrote: > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 1f31422..ca92f97 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -339,6 +339,7 @@ int setup_irq(unsigned int irq, struct irqaction *new) > desc->status |= IRQ_NO_BALANCING; > > if (!shared) { > + int trigger_type = 0;
No need to initialize to zero.
> irq_chip_set_defaults(desc->chip); > > #if defined(CONFIG_IRQ_PER_CPU) > @@ -346,11 +347,22 @@ int setup_irq(unsigned int irq, struct irqaction *new) > desc->status |= IRQ_PER_CPU; > #endif > > + trigger_type = new->flags & IRQF_TRIGGER_MASK; > + > /* Setup the type (level, edge polarity) if configured: */ > - if (new->flags & IRQF_TRIGGER_MASK) { > - if (desc->chip && desc->chip->set_type) > - desc->chip->set_type(irq, > - new->flags & IRQF_TRIGGER_MASK); > + if (trigger_type) { > + if (desc->chip && desc->chip->set_type) { > + if ((desc->chip->set_type(irq, trigger_type))) { > + /* Action handler gets freed but > + * desc->action also needs to be > + * set to NULL > + */
That's not desc->action. That's the end of the desc->action list
> + *p = NULL;
Can we have a printk here, which tells us what went wrong ?
> + spin_unlock_irqrestore(&desc->lock, > + flags); > + return -EBUSY;
Please use the already existing error exit via "goto ..." instead
> + } > + } > else > /* > * IRQF_TRIGGER_* but the PIC does not support > @@ -571,8 +583,10 @@ int request_irq(unsigned int irq, irq_handler_t handler, > #endif > > retval = setup_irq(irq, action); > - if (retval) > + if (retval) { > + printk(KERN_WARNING"Failed to allocate interrupt %d\n", retval); > kfree(action);
We dont want to know that the setup function returned -EBUSY. We rather want to have a printk in the path above.
Thanks, tglx
| |