Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1317724 > unrolled thread

[PATCH] genirq: fix trigger flags check for shared irqs

Started byBrian Starkey <brian.starkey@arm.com>
First post2016-01-26 11:40 +0100
Last post2016-01-26 21:50 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] genirq: fix trigger flags check for shared irqs Brian Starkey <brian.starkey@arm.com> - 2016-01-26 11:40 +0100
    Re: [PATCH] genirq: fix trigger flags check for shared irqs Thomas Gleixner <tglx@linutronix.de> - 2016-01-26 21:50 +0100

#1317724 — [PATCH] genirq: fix trigger flags check for shared irqs

FromBrian Starkey <brian.starkey@arm.com>
Date2016-01-26 11:40 +0100
Subject[PATCH] genirq: fix trigger flags check for shared irqs
Message-ID<qV9cu-2Od-19@gated-at.bofh.it>
For shared interrupts, if one requester passes in any IRQF_TRIGGER_*
flags whilst another doesn't, __setup_irq() can erroneously fail.

The no-flags case should be treated as "already configured", so change
__setup_irq() to only check that the flags match if any have been
provided.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---

 kernel/irq/manage.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index a79d267..032c8fa 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -995,14 +995,10 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 	old = *old_ptr;
 	if (old) {
 		/*
-		 * Can't share interrupts unless both agree to and are
-		 * the same type (level, edge, polarity). So both flag
-		 * fields must have IRQF_SHARED set and the bits which
-		 * set the trigger type must match. Also all must
-		 * agree on ONESHOT.
+		 * Can't share interrupts unless both agree to and all
+		 * must agree on ONESHOT
 		 */
 		if (!((old->flags & new->flags) & IRQF_SHARED) ||
-		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
 		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
 			goto mismatch;
 
@@ -1014,6 +1010,15 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 		/* add new interrupt at end of irq queue */
 		do {
 			/*
+			 * If 'new' requests a trigger type it must match all
+			 * previously requested trigger types.
+			 */
+			if (((new->flags & IRQF_TRIGGER_MASK) &&
+			     (old->flags & IRQF_TRIGGER_MASK)) &&
+			    ((new->flags ^ old->flags) & IRQF_TRIGGER_MASK))
+				goto mismatch;
+
+			/*
 			 * Or all existing action->thread_mask bits,
 			 * so we can find the next zero bit for this
 			 * new action.
-- 
1.7.9.5

[toc] | [next] | [standalone]


#1318351

FromThomas Gleixner <tglx@linutronix.de>
Date2016-01-26 21:50 +0100
Message-ID<qViIN-1ad-5@gated-at.bofh.it>
In reply to#1317724
On Tue, 26 Jan 2016, Brian Starkey wrote:

> For shared interrupts, if one requester passes in any IRQF_TRIGGER_*
> flags whilst another doesn't, __setup_irq() can erroneously fail.
> 
> The no-flags case should be treated as "already configured", so change
> __setup_irq() to only check that the flags match if any have been
> provided.

What happens if that "already configured", i.e. the default setting, is
conflicting with the newly requested interrupt?

I rather prefer the failure than the resulting silent wreckage.
 
Thanks,

	tglx

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web