[GIT PULL] irq/core changes for v3.5

[GIT PULL] irq/core changes for v3.5

[Ingo Molnar]

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
      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(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cf006d4..1f51676 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -31,6 +31,8 @@ config ARM
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_GENERIC_HARDIRQS
 	select GENERIC_IRQ_SHOW
+	select GENERIC_IRQ_PROBE
+	select HARDIRQS_SW_RESEND
 	select CPU_PM if (SUSPEND || CPU_IDLE)
 	select GENERIC_PCI_IOMAP
 	select HAVE_BPF_JIT if NET
@@ -126,14 +128,6 @@ config TRACE_IRQFLAGS_SUPPORT
 	bool
 	default y
 
-config HARDIRQS_SW_RESEND
-	bool
-	default y
-
-config GENERIC_IRQ_PROBE
-	bool
-	default y
-
 config GENERIC_LOCKBREAK
 	bool
 	default y
diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 9059e39..3126fc6 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -18,8 +18,6 @@ config HEXAGON
 	select GENERIC_ATOMIC64
 	select HAVE_PERF_EVENTS
 	select HAVE_GENERIC_HARDIRQS
-	select GENERIC_HARDIRQS_NO__DO_IRQ
-	select GENERIC_HARDIRQS_NO_DEPRECATED
 	# GENERIC_ALLOCATOR is used by dma_alloc_coherent()
 	select GENERIC_ALLOCATOR
 	select GENERIC_IRQ_SHOW
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 2aea5d2..c911715 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -93,27 +93,27 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
 /**
  * struct irqaction - per interrupt action descriptor
  * @handler:	interrupt handler function
- * @flags:	flags (see IRQF_* above)
  * @name:	name of the device
  * @dev_id:	cookie to identify the device
  * @percpu_dev_id:	cookie to identify the device
  * @next:	pointer to the next irqaction for shared interrupts
  * @irq:	interrupt number
- * @dir:	pointer to the proc/irq/NN/name entry
+ * @flags:	flags (see IRQF_* above)
  * @thread_fn:	interrupt handler function for threaded interrupts
  * @thread:	thread pointer for threaded interrupts
  * @thread_flags:	flags related to @thread
  * @thread_mask:	bitmask for keeping track of @thread activity
+ * @dir:	pointer to the proc/irq/NN/name entry
  */
 struct irqaction {
 	irq_handler_t		handler;
-	unsigned long		flags;
 	void			*dev_id;
 	void __percpu		*percpu_dev_id;
 	struct irqaction	*next;
-	int			irq;
 	irq_handler_t		thread_fn;
 	struct task_struct	*thread;
+	unsigned int		irq;
+	unsigned int		flags;
 	unsigned long		thread_flags;
 	unsigned long		thread_mask;
 	const char		*name;
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6080f6b..741f836 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -379,8 +379,10 @@ handle_level_irq(unsigned int irq, struct irq_desc *desc)
 	 * If its disabled or no action available
 	 * keep it masked and get out of here
 	 */
-	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data)))
+	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+		desc->istate |= IRQS_PENDING;
 		goto out_unlock;
+	}
 
 	handle_irq_event(desc);
 
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:
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 15e53b1..cb228bf 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -103,8 +103,13 @@ int check_wakeup_irqs(void)
 	int irq;
 
 	for_each_irq_desc(irq, desc) {
+		/*
+		 * Only interrupts which are marked as wakeup source
+		 * and have not been disabled before the suspend check
+		 * can abort suspend.
+		 */
 		if (irqd_is_wakeup_set(&desc->irq_data)) {
-			if (desc->istate & IRQS_PENDING)
+			if (desc->depth == 1 && desc->istate & IRQS_PENDING)
 				return -EBUSY;
 			continue;
 		}
diff --git a/kernel/irq/resend.c b/kernel/irq/resend.c
index 14dd576..6454db7 100644
--- a/kernel/irq/resend.c
+++ b/kernel/irq/resend.c
@@ -58,10 +58,13 @@ void check_irq_resend(struct irq_desc *desc, unsigned int irq)
 	/*
 	 * We do not resend level type interrupts. Level type
 	 * interrupts are resent by hardware when they are still
-	 * active.
+	 * active. Clear the pending bit so suspend/resume does not
+	 * get confused.
 	 */
-	if (irq_settings_is_level(desc))
+	if (irq_settings_is_level(desc)) {
+		desc->istate &= ~IRQS_PENDING;
 		return;
+	}
 	if (desc->istate & IRQS_REPLAY)
 		return;
 	if (desc->istate & IRQS_PENDING) {

Re: [GIT PULL] irq/core changes for v3.5

[Guennadi Liakhovetski]

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/

Re: [GIT PULL] irq/core changes for v3.5

[Thomas Gleixner]

On Tue, 12 Jun 2012, Guennadi Liakhovetski wrote:
> 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;-)

Yeah, I guess I was a bit too fast with that after Linus ranting about
the missing check. :)

Thinking more about it, it's probably the best thing to simply force
the IRQF_ONESHOT flag if it's missing. I'd love to omit it for non
level type interrupts to avoid the overhead in the irq thread, but
there is no way to figure that out at this point.

Does that fix it for you ?

Thanks,

	tglx
---
 kernel/irq/manage.c |   39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

Index: tip/kernel/irq/manage.c
===================================================================
--- tip.orig/kernel/irq/manage.c
+++ tip/kernel/irq/manage.c
@@ -959,6 +959,24 @@ __setup_irq(unsigned int irq, struct irq
 		goto out_thread;
 	}
 
+	if (new->handler == irq_default_primary_handler ) {
+		/*
+		 * The interrupt was requested with handler = NULL, so
+		 * we use the default primary handler for it.
+		 *
+		 * This requires the IRQF_ONESHOT flag set especially
+		 * for level type interrupts, 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....
+		 *
+		 * Omitting the flag works for edge type interrupts,
+		 * but we can't say for sure which type this interrupt
+		 * really has at this point. So we simply enforce it.
+		 */
+		new->flags |= IRQF_ONESHOT;
+	}
+
 	/*
 	 * The following block of code has to be executed atomically
 	 */
@@ -1032,27 +1050,6 @@ __setup_irq(unsigned int irq, struct irq
 		 * 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("Threaded irq requested with handler=NULL and !ONESHOT for irq %d\n",
-		       irq);
-		ret = -EINVAL;
-		goto out_mask;
 	}
 
 	if (!shared) {

Re: [GIT PULL] irq/core changes for v3.5

[Linus Torvalds]

On Tue, Jun 12, 2012 at 5:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Thinking more about it, it's probably the best thing to simply force
> the IRQF_ONESHOT flag if it's missing.

No, that's just crazy.

Now you make broken shit code work, and then you break the *correct*
code that didn't want threading and didn't set IRQF_ONESHOT.

Just face it: if somebody doesn't have an interrupt-time function
pointer, they need to rethink their code. It's a mistake. It's broken
shit.

Why pander to crap? What is the advantage of allowing people to think
that they don't need an interrupt-time function? It's a fundamentaly
broken model, since it *cannot* work tgether with another driver that
wants to have the normal semantics and happens to share the irq.

               Linus

Re: [GIT PULL] irq/core changes for v3.5

[Thomas Gleixner]

On Tue, 12 Jun 2012, Linus Torvalds wrote:

> On Tue, Jun 12, 2012 at 5:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Thinking more about it, it's probably the best thing to simply force
> > the IRQF_ONESHOT flag if it's missing.
> 
> No, that's just crazy.
> 
> Now you make broken shit code work, and then you break the *correct*
> code that didn't want threading and didn't set IRQF_ONESHOT.

That correct code is totally unaffected. We only force set it for the
case which *requests* a threaded irq w/o a primary handler and w/o
supplying the ONESHOT flag.

> Just face it: if somebody doesn't have an interrupt-time function
> pointer, they need to rethink their code. It's a mistake. It's broken
> shit.

We explicitely made it that way to prevent people from implementing
the same function which just returns IRQ_WAKE_THREAD over and over.

This was done in the first place for irqs where the device which
raised the interrupt cannot be accessed from hard interrupt context
(stuff behind serial busses, i2c, spi ....)

The only choice those people had before we provided threaded
interrupts in the core was having a primary handler which called
disable_irq_nosync() and then woke an handling thread, which then
reenabled the irq line with enable_irq().

That allowed us to remove a lot of crappy, broken and racy kthread
code from drivers that way and consolidated the handling into the core
code.

Yes, in hindsight I should have enforced IRQF_ONESHOT for those
callers which have a NULL primary handler right from the beginning,
but that doesn't help much now.

> Why pander to crap? What is the advantage of allowing people to think
> that they don't need an interrupt-time function? It's a fundamentaly
> broken model, since it *cannot* work tgether with another driver that
> wants to have the normal semantics and happens to share the irq.

The vast majority of that stuff is embedded. Shared interupt lines are
almost non existent in that space. It's just x86 which is full of that
shared nonsense.

If it's shared between a normal semantics device and one which
requires the oneshot semantics, the core has already a sanity check
for those cases and always had.

If you really need a threaded handler which shares the interrupt line
with a non threaded one, then you definitely need a primary handler
which can disable the irq at the device level, but that was always
a requirement.

I just checked all the drivers in question with coccinelle.

Only a total of 8 drivers set IRQF_SHARED. 5 of them are for the same
ARM platform on which none of the irqs can be shared. So IRQF_SHARED
there is bogus. One other is for ARM stuff as well, which does not
have shared interrupts either. The two remaining are "general purpose"
drivers which also originated from embedded and are unlikely to show
up on a PC platform.

I did not bother to check the few ones which use platform data, but
those are definitely embedded ones as well, which won't have shared
interrupts either.

So now we have the choice of:

   - Leaving the current check and regress 90+ drivers

   - Leaving the current check and fix 90+ broken drivers

   - Reverting it and end up with no protection at all

   - Forcing the flag and risking the wreckage of two oddball drivers
     _IF_ they ever show up on a PC platform.

I definitely prefer option #4 as it makes the most sense.

Thanks,

	tglx

Re: [GIT PULL] irq/core changes for v3.5

[Linus Torvalds]

On Tue, Jun 12, 2012 at 7:47 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> So now we have the choice of:
>
>   - Leaving the current check and regress 90+ drivers
>
>   - Leaving the current check and fix 90+ broken drivers
>
>   - Reverting it and end up with no protection at all
>
>   - Forcing the flag and risking the wreckage of two oddball drivers
>     _IF_ they ever show up on a PC platform.

So I'd really prefer #2.

I'd rather have a nice sane generic irq layer that really makes it an
*error* to do stupid things that make no sense, and then fix the
drivers that do them.

I would assume that fixing the drivers should be simple, and could
even be done largely by simply just grepping for the pattern of "NULL
irq-time handler without the proper markers to show that the driver
author knew what the hell they were doing".

Because I really do think that your suggested "let's work around it"
approach breaks the *wrong* driver, and does so very subtly. If you
have a buggy driver that first registers a threaded interrupt and that
silently gets turned into a IRQF_ONESHOT, and then you have the case
of a driver later coming in that would like to share the irq (but
doesn't use threading, and wants the sane shared level semantics), you
now disallow that second (non-buggy) request.

And clearly that kind of sharing cannot work, but the driver author
that looks at his (non-buggy) driver and then maybe even understands
what the problem is, and starts looking at the *buggy* driver, doesn't
actually *see* that the buggy drver uses IRQF_ONESHOT.

See what I'm trying to say? The "implicit IRQF_ONESHOT" model
basically hides a failure point in a subtle way. If we just make it a
hard requirement that drivers have to use sane models, the driver that
wants the one-shot behavior (and *depends* on it, because it doesn't
have an interrupt-time routine at all), will have to explicitly mark
itself that way, so that *other* drivers - that might be impacted by
that choice - can see it, and can see why they can't share the irq.

I relaize that the current drivers that rely on our old fast-and-loose
behavior actually *work*, and I realize that apparently there aren't
any shared irq issues in existence today, but I'd like our generic irq
code to do the RightThing(tm), and I think that implies that it is the
drivers that should be fixed, not the core irq layer that should work
around the problem.

Driver authors that see the error should be able to easily fix their
drivers, no?

                Linus

Re: [GIT PULL] irq/core changes for v3.5

[Guennadi Liakhovetski]

Hi all

On Wed, 13 Jun 2012, Linus Torvalds wrote:

> On Tue, Jun 12, 2012 at 7:47 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > So now we have the choice of:
> >
> >   - Leaving the current check and regress 90+ drivers
> >
> >   - Leaving the current check and fix 90+ broken drivers
> >
> >   - Reverting it and end up with no protection at all
> >
> >   - Forcing the flag and risking the wreckage of two oddball drivers
> >     _IF_ they ever show up on a PC platform.
> 
> So I'd really prefer #2.
> 
> I'd rather have a nice sane generic irq layer that really makes it an
> *error* to do stupid things that make no sense, and then fix the
> drivers that do them.

To make this simpler I'm attaching below a list of (I think) all 
occurrences of the invalid request_threaded_irq() uses.

[snip]

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

drivers/regulator/wm831x-dcdc.c
ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING, dcdc->name, dcdc);
ret = request_threaded_irq(irq, NULL, wm831x_dcdc_oc_irq, IRQF_TRIGGER_RISING, dcdc->name, dcdc);
ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING, dcdc->name, dcdc);
ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING, dcdc->name, dcdc);

drivers/regulator/wm831x-isink.c
ret = request_threaded_irq(irq, NULL, wm831x_isink_irq, IRQF_TRIGGER_RISING, isink->name, isink);

drivers/regulator/wm831x-ldo.c
ret = request_threaded_irq(irq, NULL, wm831x_ldo_uv_irq, IRQF_TRIGGER_RISING, ldo->name, ldo);
ret = request_threaded_irq(irq, NULL, wm831x_ldo_uv_irq, IRQF_TRIGGER_RISING, ldo->name, ldo);

drivers/gpio/gpio-ab8500.c
ret = request_threaded_irq(irq_to_rising(irq), NULL, handle_rising, IRQF_TRIGGER_RISING, "ab8500-gpio-r", ab8500_gpio);

drivers/gpio/gpio-sx150x.c
err = request_threaded_irq(irq_summary, NULL, sx150x_irq_thread_fn, IRQF_SHARED | IRQF_TRIGGER_FALLING, chip->irq_chip.name, chip);

drivers/rtc/rtc-wm831x.c
ret = request_threaded_irq(alm_irq, NULL, wm831x_alm_irq, IRQF_TRIGGER_RISING, "RTC alarm", wm831x_rtc);

drivers/rtc/rtc-max8998.c
ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0, "rtc-alarm0", info);

drivers/rtc/rtc-twl.c
ret = request_threaded_irq(irq, NULL, twl_rtc_interrupt, IRQF_TRIGGER_RISING, dev_name(&rtc->dev), rtc);

drivers/rtc/rtc-ab8500.c
err = request_threaded_irq(irq, NULL, rtc_alarm_handler, IRQF_NO_SUSPEND, "ab8500-rtc", rtc);

drivers/rtc/rtc-isl1208.c
rc = request_threaded_irq(client->irq, NULL, isl1208_rtc_interrupt, IRQF_SHARED, isl1208_driver.driver.name, client);

drivers/mmc/core/cd-gpio.c
ret = request_threaded_irq(irq, NULL, mmc_cd_gpio_irqt, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, cd->label, host);

drivers/mmc/host/sdhci-s3c.c
request_threaded_irq(sc->ext_cd_irq, NULL, sdhci_s3c_gpio_card_detect_thread, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, dev_name(dev), sc) == 0) { int status = gpio_get_value(sc->ext_cd_gpio);

drivers/mmc/host/of_mmc_spi.c
return request_threaded_irq(oms->detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc);

drivers/net/can/mcp251x.c
ret = request_threaded_irq(spi->irq, NULL, mcp251x_can_ist, pdata->irq_flags ? pdata->irq_flags : IRQF_TRIGGER_FALLING, DEVICE_NAME, priv);

drivers/misc/mei/main.c
err = request_threaded_irq(pdev->irq, NULL, mei_interrupt_thread_handler, 0, mei_driver_name, dev);
err = request_threaded_irq(pdev->irq, NULL, mei_interrupt_thread_handler, 0, mei_driver_name, dev);

drivers/input/misc/ad714x.c
error = request_threaded_irq(ad714x->irq, NULL, ad714x_interrupt_thread, plat_data->irqflags ? plat_data->irqflags : IRQF_TRIGGER_FALLING, "ad714x_captouch", ad714x);

drivers/input/misc/wm831x-on.c
ret = request_threaded_irq(irq, NULL, wm831x_on_irq, IRQF_TRIGGER_RISING, "wm831x_on", wm831x_on);

drivers/input/misc/twl6040-vibra.c
ret = request_threaded_irq(info->irq, NULL, twl6040_vib_irq_handler, 0, "twl6040_irq_vib", info);

drivers/input/misc/twl4030-pwrbutton.c
err = request_threaded_irq(irq, NULL, powerbutton_irq, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "twl4030_pwrbutton", pwr);

drivers/input/misc/dm355evm_keys.c
status = request_threaded_irq(keys->irq, NULL, dm355evm_keys_irq, IRQF_TRIGGER_FALLING, dev_name(&pdev->dev), keys);

drivers/input/keyboard/twl4030_keypad.c
error = request_threaded_irq(kp->irq, NULL, do_kp_irq, 0, pdev->name, kp);

drivers/input/keyboard/mpr121_touchkey.c
error = request_threaded_irq(client->irq, NULL, mpr_touchkey_interrupt, IRQF_TRIGGER_FALLING, client->dev.driver->name, mpr121);

drivers/input/keyboard/mcs_touchkey.c
error = request_threaded_irq(client->irq, NULL, mcs_touchkey_interrupt, IRQF_TRIGGER_FALLING, client->dev.driver->name, data);

drivers/input/keyboard/tnetv107x-keypad.c
error = request_threaded_irq(kp->irq_press, NULL, keypad_irq, 0, dev_name(dev), kp);
error = request_threaded_irq(kp->irq_release, NULL, keypad_irq, 0, dev_name(dev), kp);

drivers/input/keyboard/tc3589x-keypad.c
error = request_threaded_irq(irq, NULL, tc3589x_keypad_irq, plat->irqtype, "tc3589x-keypad", keypad);

drivers/input/keyboard/tca6416-keypad.c
error = request_threaded_irq(chip->irqnum, NULL, tca6416_keys_isr, IRQF_TRIGGER_FALLING, "tca6416-keypad", chip);

drivers/input/keyboard/tca8418_keypad.c
error = request_threaded_irq(client->irq, NULL, tca8418_irq_handler, IRQF_TRIGGER_FALLING, client->name, keypad_data);

drivers/input/keyboard/qt1070.c
err = request_threaded_irq(client->irq, NULL, qt1070_interrupt, IRQF_TRIGGER_NONE, client->dev.driver->name, data);

drivers/input/touchscreen/tnetv107x-ts.c
error = request_threaded_irq(ts->tsc_irq, NULL, tsc_irq, 0, dev_name(dev), ts);

drivers/input/touchscreen/bu21013_ts.c
error = request_threaded_irq(pdata->irq, NULL, bu21013_gpio_irq, IRQF_TRIGGER_FALLING | IRQF_SHARED, DRIVER_TP, bu21013_data);

drivers/input/touchscreen/intel-mid-touch.c
err = request_threaded_irq(tsdev->irq, NULL, mrstouch_pendet_irq, 0, "mrstouch", tsdev);

drivers/input/touchscreen/tsc2005.c
error = request_threaded_irq(spi->irq, NULL, tsc2005_irq_thread, IRQF_TRIGGER_RISING, "tsc2005", ts);

drivers/input/touchscreen/atmel_mxt_ts.c
error = request_threaded_irq(client->irq, NULL, mxt_interrupt, pdata->irqflags, client->dev.driver->name, data);

drivers/input/touchscreen/cy8ctmg110_ts.c
err = request_threaded_irq(client->irq, NULL, cy8ctmg110_irq_thread, IRQF_TRIGGER_RISING, "touch_reset_key", ts);

drivers/input/touchscreen/ad7879.c
err = request_threaded_irq(ts->irq, NULL, ad7879_irq, IRQF_TRIGGER_FALLING, dev_name(dev), ts);

drivers/input/touchscreen/pixcir_i2c_ts.c
error = request_threaded_irq(client->irq, NULL, pixcir_ts_isr, IRQF_TRIGGER_FALLING, client->name, tsdata);

drivers/input/joystick/as5011.c
error = request_threaded_irq(as5011->button_irq, NULL, as5011_button_interrupt, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, "as5011_button", as5011);
error = request_threaded_irq(as5011->axis_irq, NULL, as5011_axis_interrupt, plat_data->axis_irqflags, "as5011_joystick", as5011);

drivers/nfc/pn544.c
r = request_threaded_irq(client->irq, NULL, pn544_irq_thread_fn, IRQF_TRIGGER_RISING, PN544_DRIVER_NAME, info);

drivers/nfc/pn544_hci.c
r = request_threaded_irq(client->irq, NULL, pn544_hci_irq_thread_fn, IRQF_TRIGGER_RISING, PN544_HCI_DRIVER_NAME, info);

drivers/power/twl4030_charger.c
ret = request_threaded_irq(bci->irq_chg, NULL, twl4030_charger_interrupt, 0, pdev->name, bci);
ret = request_threaded_irq(bci->irq_bci, NULL, twl4030_bci_interrupt, 0, pdev->name, bci);

drivers/power/ab8500_charger.c
ret = request_threaded_irq(irq, NULL, ab8500_charger_irq[i].isr, IRQF_SHARED | IRQF_NO_SUSPEND, ab8500_charger_irq[i].name, di);

drivers/power/wm831x_power.c
ret = request_threaded_irq(irq, NULL, wm831x_syslo_irq, IRQF_TRIGGER_RISING, "System power low", power);
ret = request_threaded_irq(irq, NULL, wm831x_pwr_src_irq, IRQF_TRIGGER_RISING, "Power source", power);
ret = request_threaded_irq(irq, NULL, wm831x_bat_irq, IRQF_TRIGGER_RISING, wm831x_bat_irqs[i], power);

drivers/power/lp8727_charger.c
return request_threaded_irq(pchg->client->irq, NULL, lp8727_isr_func, IRQF_TRIGGER_FALLING, "lp8727_irq", pchg);

drivers/power/max8903_charger.c
ret = request_threaded_irq(gpio_to_irq(pdata->dok), NULL, max8903_dcin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "MAX8903 DC IN", data);
ret = request_threaded_irq(gpio_to_irq(pdata->uok), NULL, max8903_usbin, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "MAX8903 USB IN", data);
ret = request_threaded_irq(gpio_to_irq(pdata->flt), NULL, max8903_fault, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "MAX8903 Fault", data);

drivers/power/max17042_battery.c
ret = request_threaded_irq(client->irq, NULL, max17042_thread_handler, IRQF_TRIGGER_FALLING, chip->battery.name, chip);

drivers/power/smb347-charger.c
ret = request_threaded_irq(irq, NULL, smb347_interrupt, IRQF_TRIGGER_FALLING, client->name, smb);

drivers/power/ab8500_btemp.c
ret = request_threaded_irq(irq, NULL, ab8500_btemp_irq[i].isr, IRQF_SHARED | IRQF_NO_SUSPEND, ab8500_btemp_irq[i].name, di);

drivers/power/ab8500_fg.c
ret = request_threaded_irq(irq, NULL, ab8500_fg_irq[i].isr, IRQF_SHARED | IRQF_NO_SUSPEND, ab8500_fg_irq[i].name, di);

drivers/base/regmap/regmap-irq.c
ret = request_threaded_irq(irq, NULL, regmap_irq_thread, irq_flags, chip->name, d);

drivers/staging/cptm1217/clearpad_tm1217.c
retval = request_threaded_irq(client->irq, NULL, cp_tm1217_sample_thread, IRQF_TRIGGER_FALLING, "cp_tm1217_touch", ts);

drivers/staging/iio/accel/sca3000_core.c
ret = request_threaded_irq(spi->irq, NULL, &sca3000_event_handler, IRQF_TRIGGER_FALLING, "sca3000", indio_dev);

drivers/staging/iio/adc/adt7410.c
ret = request_threaded_irq(client->irq, NULL, &adt7410_event_handler, IRQF_TRIGGER_LOW, id->name, indio_dev);
ret = request_threaded_irq(adt7410_platform_data[0], NULL, &adt7410_event_handler, adt7410_platform_data[1], id->name, indio_dev);

drivers/staging/iio/adc/ad7816.c
ret = request_threaded_irq(spi_dev->irq, NULL, &ad7816_event_handler, IRQF_TRIGGER_LOW, indio_dev->name, indio_dev);

drivers/staging/iio/adc/adt7310.c
ret = request_threaded_irq(spi_dev->irq, NULL, &adt7310_event_handler, irq_flags, indio_dev->name, indio_dev);
ret = request_threaded_irq(adt7310_platform_data[0], NULL, &adt7310_event_handler, adt7310_platform_data[1], indio_dev->name, indio_dev);

drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
retval = request_threaded_irq(platformdata->irq_number, NULL, synaptics_rmi4_irq, platformdata->irq_type, DRIVER_NAME, rmi4_data);

drivers/media/radio/si470x/radio-si470x-i2c.c
retval = request_threaded_irq(client->irq, NULL, si470x_i2c_interrupt, IRQF_TRIGGER_FALLING, DRIVER_NAME, radio);

drivers/extcon/extcon-max8997.c
ret = request_threaded_irq(pdata->irq_base + muic_irq->irq, NULL, max8997_muic_irq_handler, 0, muic_irq->name, info);

drivers/mfd/tps65912-irq.c
ret = request_threaded_irq(irq, NULL, tps65912_irq, flags, "tps65912", tps65912);

drivers/mfd/wm831x-auxadc.c
ret = request_threaded_irq(wm831x_irq(wm831x, WM831X_IRQ_AUXADC_DATA), NULL, wm831x_auxadc_irq, 0, "auxadc", wm831x);

drivers/mfd/twl4030-irq.c
status = request_threaded_irq(irq, NULL, handle_twl4030_sih, 0, agent->irq_name ?: sih->name, NULL);

drivers/mfd/ab8500-gpadc.c
ret = request_threaded_irq(gpadc->irq, NULL, ab8500_bm_gpswadcconvend_handler, IRQF_NO_SUSPEND | IRQF_SHARED, "ab8500-gpadc", gpadc);

drivers/mfd/htc-i2cpld.c
ret = request_threaded_irq(htcpld->chained_irq, NULL, htcpld_handler, flags, pdev->name, htcpld);

drivers/mfd/wm8350-core.c
ret = request_threaded_irq(wm8350->irq_base + WM8350_IRQ_AUXADC_DATARDY, NULL, wm8350_auxadc_irq, 0, "auxadc", wm8350);

drivers/mfd/twl4030-madc.c
ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL, twl4030_madc_threaded_irq_handler, IRQF_TRIGGER_RISING, "twl4030_madc", madc);

drivers/mfd/88pm860x-core.c
ret = request_threaded_irq(chip->core_irq, NULL, pm860x_irq, flags, "88pm860x", chip);

drivers/mfd/twl6040-core.c
ret = request_threaded_irq(twl6040->irq_base + TWL6040_IRQ_READY, NULL, twl6040_naudint_handler, 0, "twl6040_irq_ready", twl6040);

drivers/mfd/tps65910-irq.c
ret = request_threaded_irq(irq, NULL, tps65910_irq, flags, "tps65910", tps65910);

drivers/mfd/ti-ssp.c
error = request_threaded_irq(ssp->irq, NULL, ti_ssp_interrupt, 0, dev_name(dev), ssp);

drivers/mfd/max8925-core.c
ret = request_threaded_irq(irq, NULL, max8925_irq, flags, "max8925", chip);
ret = request_threaded_irq(chip->tsc_irq, NULL, max8925_tsc_irq, flags, "max8925-tsc", chip);

drivers/mfd/wm8350-irq.c
ret = request_threaded_irq(irq, NULL, wm8350_irq, flags, "wm8350", wm8350);

drivers/usb/otg/twl4030-usb.c
status = request_threaded_irq(twl->irq, NULL, twl4030_usb_irq, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "twl4030_usb", twl);

drivers/usb/otg/ab8500-usb.c
err = request_threaded_irq(ab->irq_num_id_rise, NULL, ab8500_usb_v1x_common_irq, IRQF_NO_SUSPEND | IRQF_SHARED, "usb-id-rise", ab);
err = request_threaded_irq(ab->irq_num_id_fall, NULL, ab8500_usb_v1x_common_irq, IRQF_NO_SUSPEND | IRQF_SHARED, "usb-id-fall", ab);
err = request_threaded_irq(ab->irq_num_vbus_rise, NULL, ab8500_usb_v1x_common_irq, IRQF_NO_SUSPEND | IRQF_SHARED, "usb-vbus-rise", ab);
err = request_threaded_irq(ab->irq_num_vbus_fall, NULL, ab8500_usb_v1x_vbus_fall_irq, IRQF_NO_SUSPEND | IRQF_SHARED, "usb-vbus-fall", ab);
err = request_threaded_irq(ab->irq_num_link_status, NULL, ab8500_usb_v20_irq, IRQF_NO_SUSPEND | IRQF_SHARED, "usb-link-status", ab);

drivers/platform/x86/intel_mid_powerbtn.c
error = request_threaded_irq(irq, NULL, mfld_pb_isr, IRQF_NO_SUSPEND, DRIVER_NAME, input);

sound/soc/codecs/wm8996.c
ret = request_threaded_irq(i2c->irq, NULL, wm8996_edge_irq, irq_flags, "wm8996", codec);

sound/soc/codecs/wm5100.c
ret = request_threaded_irq(i2c->irq, NULL, wm5100_edge_irq, irq_flags, "wm5100", wm5100);

sound/soc/codecs/wm8994.c
ret = request_threaded_irq(wm8994->micdet_irq, NULL, wm8994_mic_irq, IRQF_TRIGGER_RISING, "Mic1 detect", wm8994);
ret = request_threaded_irq(wm8994->micdet_irq, NULL, wm8958_mic_irq, IRQF_TRIGGER_RISING, "Mic detect", wm8994);

sound/soc/codecs/twl6040.c
ret = request_threaded_irq(priv->plug_irq, NULL, twl6040_audio_handler, 0, "twl6040_irq_plug", codec);

sound/soc/codecs/max98095.c
ret = request_threaded_irq(client->irq, NULL, max98095_report_jack, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, "max98095", codec);

./include/linux/mfd/wm8994/core.h
return request_threaded_irq(regmap_irq_get_virq(wm8994->irq_data, irq), NULL, handler, IRQF_TRIGGER_RISING, name, data);

./include/linux/mfd/wm8350/core.h
return request_threaded_irq(irq + wm8350->irq_base, NULL, handler, flags, name, data);

Re: [GIT PULL] irq/core changes for v3.5

[Mark Brown]

On Wed, Jun 13, 2012 at 09:03:27AM +0200, Guennadi Liakhovetski wrote:

> To make this simpler I'm attaching below a list of (I think) all 
> occurrences of the invalid request_threaded_irq() uses.

Scanning through the list most of these are for internal IRQs from MFDs
where it'd be a bug if the primary IRQ ever existed in the first place
as we're already in threaded context, though obviously there's no
problem with them marking themselves as _ONESHOT.

Re: [GIT PULL] irq/core changes for v3.5

[Thomas Gleixner]

On Wed, 13 Jun 2012, Linus Torvalds wrote:
> I relaize that the current drivers that rely on our old fast-and-loose
> behavior actually *work*, and I realize that apparently there aren't
> any shared irq issues in existence today, but I'd like our generic irq
> code to do the RightThing(tm), and I think that implies that it is the
> drivers that should be fixed, not the core irq layer that should work
> around the problem.

I don't see much of a difference between

  request_threaded_irq(irq, NULL, thread_handler, 0,...)

and

  request_threaded_irq(irq, NULL, thread_handler, IRQF_ONESHOT,...)

The NULL primary handler is a clear indicator for the intention. So
the innocent author of that *correct* driver will see in both cases
what's going on.

But that's a distinction without a difference which we can discuss
forever :)

> Driver authors that see the error should be able to easily fix their
> drivers, no?

No argument about that, though I prefer not to break working stuff
just to add another indicator for something which is obvious already
and requires to have such a weird device on a machine with shared
interrupts.

A 5th option would be to emit a warning in case the flag is not set,
fix it up for now and schedule it for removal in 3.6. Anything what's
not fixed by then is broken for good.

Your choice, really.

Thanks,

	tglx

Re: [GIT PULL] irq/core changes for v3.5

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> A 5th option would be to emit a warning in case the flag is 
> not set, fix it up for now and schedule it for removal in 3.6. 
> Anything what's not fixed by then is broken for good.

Seems like the least invasive option to me. It's not like we are 
in a rush to break more stuff.

Thanks,

	Ingo

Re: [GIT PULL] irq/core changes for v3.5

[Thomas Gleixner]

On Wed, 13 Jun 2012, Ingo Molnar wrote:
> Seems like the least invasive option to me. It's not like we are 
> in a rush to break more stuff.

OTOH, when will we get the chance again to break lots of stuff Par
ordre de Mufti?