Messages in this thread | | | Date | Tue, 12 Jun 2012 12:24:00 +0200 (CEST) | From | Guennadi Liakhovetski <> | Subject | Re: [GIT PULL] irq/core changes for v3.5 |
| |
Hi Ingo
On Tue, 22 May 2012, Ingo Molnar wrote:
> Linus, > > Please pull the latest irq-core-for-linus git tree from: > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-core-for-linus > > HEAD: e0d8ffd1df44518cb9ac9b1807d1f13cc100fc2f hexagon: Remove select of not longer existing Kconfig switches > > A collection of small fixes. > > Thanks, > > Ingo > > ------------------> > Thomas Gleixner (7): > genirq: Streamline irq_action > genirq: Reject bogus threaded irq requests
Correct me if I'm wrong, but the above patch, however good and correct in principle, breaks all existing drivers, that currently use request_threaded_irq() in this "bogus" way, namely, with NULL handler and without the IRQF_ONESHOT flag? Of those drivers I counted 73 in today's Linus' tree with a total of 92 call occurrences. Isn't this a bit too brutal? How about either doing it more gently (first just issue a warning) or first fixing them all? The current approach seems to just cause a bunch of regressions to me? I can provide a list of affected files, if someone volunteers to fix them all;-)
Thanks Guennadi
> genirq: Be more informative on irq type mismatch > genirq: Allow check_wakeup_irqs to notice level-triggered interrupts > genirq: Do not consider disabled wakeup irqs > arm: Select core options instead of redefining them > hexagon: Remove select of not longer existing Kconfig switches > > > arch/arm/Kconfig | 10 +------- > arch/hexagon/Kconfig | 2 - > include/linux/interrupt.h | 8 +++--- > kernel/irq/chip.c | 4 ++- > kernel/irq/manage.c | 46 ++++++++++++++++++++++++++++++-------------- > kernel/irq/pm.c | 7 +++++- > kernel/irq/resend.c | 7 ++++- > 7 files changed, 51 insertions(+), 33 deletions(-)
[snip]
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 89a3ea8..585f638 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -565,8 +565,8 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq, > * IRQF_TRIGGER_* but the PIC does not support multiple > * flow-types? > */ > - pr_debug("No set_type function for IRQ %d (%s)\n", irq, > - chip ? (chip->name ? : "unknown") : "unknown"); > + pr_debug("genirq: No set_type function for IRQ %d (%s)\n", irq, > + chip ? (chip->name ? : "unknown") : "unknown"); > return 0; > } > > @@ -600,7 +600,7 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq, > ret = 0; > break; > default: > - pr_err("setting trigger mode %lu for irq %u failed (%pF)\n", > + pr_err("genirq: Setting trigger mode %lu for irq %u failed (%pF)\n", > flags, irq, chip->irq_set_type); > } > if (unmask) > @@ -837,8 +837,7 @@ void exit_irq_thread(void) > > action = kthread_data(tsk); > > - printk(KERN_ERR > - "exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n", > + pr_err("genirq: exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n", > tsk->comm ? tsk->comm : "", tsk->pid, action->irq); > > desc = irq_to_desc(action->irq); > @@ -878,7 +877,6 @@ static int > __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > { > struct irqaction *old, **old_ptr; > - const char *old_name = NULL; > unsigned long flags, thread_mask = 0; > int ret, nested, shared = 0; > cpumask_var_t mask; > @@ -972,10 +970,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > */ > if (!((old->flags & new->flags) & IRQF_SHARED) || > ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) || > - ((old->flags ^ new->flags) & IRQF_ONESHOT)) { > - old_name = old->name; > + ((old->flags ^ new->flags) & IRQF_ONESHOT)) > goto mismatch; > - } > > /* All handlers must agree on per-cpuness */ > if ((old->flags & IRQF_PERCPU) != > @@ -1031,6 +1027,27 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > * all existing action->thread_mask bits. > */ > new->thread_mask = 1 << ffz(thread_mask); > + > + } else if (new->handler == irq_default_primary_handler) { > + /* > + * The interrupt was requested with handler = NULL, so > + * we use the default primary handler for it. But it > + * does not have the oneshot flag set. In combination > + * with level interrupts this is deadly, because the > + * default primary handler just wakes the thread, then > + * the irq lines is reenabled, but the device still > + * has the level irq asserted. Rinse and repeat.... > + * > + * While this works for edge type interrupts, we play > + * it safe and reject unconditionally because we can't > + * say for sure which type this interrupt really > + * has. The type flags are unreliable as the > + * underlying chip implementation can override them. > + */ > + pr_err("genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq %d\n", > + irq); > + ret = -EINVAL; > + goto out_mask; > } > > if (!shared) { > @@ -1078,7 +1095,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > > if (nmsk != omsk) > /* hope the handler works with current trigger mode */ > - pr_warning("IRQ %d uses trigger mode %u; requested %u\n", > + pr_warning("genirq: irq %d uses trigger mode %u; requested %u\n", > irq, nmsk, omsk); > } > > @@ -1115,14 +1132,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > return 0; > > mismatch: > -#ifdef CONFIG_DEBUG_SHIRQ > if (!(new->flags & IRQF_PROBE_SHARED)) { > - printk(KERN_ERR "IRQ handler type mismatch for IRQ %d\n", irq); > - if (old_name) > - printk(KERN_ERR "current handler: %s\n", old_name); > + pr_err("genirq: Flags mismatch irq %d. %08x (%s) vs. %08x (%s)\n", > + irq, new->flags, new->name, old->flags, old->name); > +#ifdef CONFIG_DEBUG_SHIRQ > dump_stack(); > - } > #endif > + } > ret = -EBUSY; > > out_mask:
[snip]
--- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
| |