From: "Uwe Kleine-König" <Uwe.Kleine-Koenig@digi.com>
To: <linux-kernel@vger.kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>
Subject: [PATCH v2] handle failure of irqchip->set_type in setup_irq
Date: Fri, 4 Jul 2008 12:46:34 +0200 [thread overview]
Message-ID: <20080704104634.GA31634@digi.com> (raw)
In-Reply-To: <20080625131101.GA6205@digi.com>
set_type returns an int indicating success or failure, but up to now
setup_irq ignores that.
In my case this resulted in a machine hang:
gpio-keys requested IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, but
arm/ns9xxx can only trigger on one direction so set_type didn't touch
the configuration which happens do default to a level sensitiveness and
returned -EINVAL. setup_irq ignored that and unmasked the irq. This
resulted in an endless triggering of the gpio-key interrupt service
routine which effectively killed the machine.
With this patch applied setup_irq propagates the error to the caller.
Note that before in the case
chip && !chip->set_type && !chip->name
a NULL pointer was feed to printk. This is fixed, too.
Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
---
Hello,
Changes since initial post:
- improve commit log (hopefully)
- move code to a dedicated function to improve readability and code
line length.
- include the symbolic name of the failing callback in the error
message.
Best regards
Uwe
kernel/irq/manage.c | 72 ++++++++++++++++++++++++++++++++++----------------
1 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 46d6611..178b966 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -270,6 +270,35 @@ void compat_irq_chip_set_default_handler(struct irq_desc *desc)
desc->handle_irq = NULL;
}
+static int __irq_set_trigger(struct irq_chip *chip, unsigned int irq,
+ unsigned long flags)
+{
+ int ret;
+
+ if (!chip || !chip->set_type) {
+ /*
+ * IRQF_TRIGGER_* but the PIC does not support multiple
+ * flow-types?
+ */
+ pr_warning("No set_type function for IRQ %d (%s)\n", irq,
+ chip ? (chip->name ? : "unknown") : "unknown");
+ return 0;
+ }
+
+ ret = chip->set_type(irq, flags & IRQF_TRIGGER_MASK);
+
+ if (ret) {
+ char buf[100];
+
+ snprintf(buf, sizeof(buf), KERN_ERR
+ "setting flow type for irq %u failed (%%s)\n",
+ irq);
+ print_fn_descriptor_symbol(buf, chip->set_type);
+ }
+
+ return ret;
+}
+
/*
* Internal function to register an irqaction - typically used to
* allocate special interrupts that are part of the architecture.
@@ -281,6 +310,7 @@ int setup_irq(unsigned int irq, struct irqaction *new)
const char *old_name = NULL;
unsigned long flags;
int shared = 0;
+ int ret;
if (irq >= NR_IRQS)
return -EINVAL;
@@ -338,37 +368,26 @@ int setup_irq(unsigned int irq, struct irqaction *new)
shared = 1;
}
- *p = new;
-
- /* Exclude IRQ from balancing */
- if (new->flags & IRQF_NOBALANCING)
- desc->status |= IRQ_NO_BALANCING;
-
if (!shared) {
irq_chip_set_defaults(desc->chip);
-#if defined(CONFIG_IRQ_PER_CPU)
- if (new->flags & IRQF_PERCPU)
- desc->status |= IRQ_PER_CPU;
-#endif
-
/* 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);
- else
- /*
- * IRQF_TRIGGER_* but the PIC does not support
- * multiple flow-types?
- */
- printk(KERN_WARNING "No IRQF_TRIGGER set_type "
- "function for IRQ %d (%s)\n", irq,
- desc->chip ? desc->chip->name :
- "unknown");
+ ret = __irq_set_trigger(desc->chip, irq, new->flags);
+
+ if (ret) {
+ spin_unlock_irqrestore(&desc->lock, flags);
+ return ret;
+ }
+
} else
compat_irq_chip_set_default_handler(desc);
+#if defined(CONFIG_IRQ_PER_CPU)
+ if (new->flags & IRQF_PERCPU)
+ desc->status |= IRQ_PER_CPU;
+#endif
+
desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING |
IRQ_INPROGRESS | IRQ_SPURIOUS_DISABLED);
@@ -383,6 +402,13 @@ int setup_irq(unsigned int irq, struct irqaction *new)
/* Undo nested disables: */
desc->depth = 1;
}
+
+ *p = new;
+
+ /* Exclude IRQ from balancing */
+ if (new->flags & IRQF_NOBALANCING)
+ desc->status |= IRQ_NO_BALANCING;
+
/* Reset broken irq detection when installing new handler */
desc->irq_count = 0;
desc->irqs_unhandled = 0;
--
1.5.6
next prev parent reply other threads:[~2008-07-04 10:46 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-25 13:11 [PATCH] handle failure of irqchip->set_type in setup_irq Uwe Kleine-König
2008-07-02 9:17 ` Uwe Kleine-König
2008-07-02 9:49 ` Andrew Morton
2008-07-02 10:09 ` Uwe Kleine-König
2008-07-04 10:46 ` Uwe Kleine-König [this message]
2008-07-04 17:17 ` [PATCH v2] " Andrew Morton
2008-07-04 18:43 ` Uwe Kleine-König
2008-07-04 19:08 ` Andrew Morton
2008-07-09 13:13 ` Uwe Kleine-König
2008-07-09 21:52 ` Andrew Morton
2008-07-10 8:23 ` Uwe Kleine-König
2008-07-10 8:28 ` Andrew Morton
[not found] ` <20080704111540.ddffd241.akpm@linux-foundation.org>
[not found] ` <alpine.LFD.1.10.0807041147450.2815@woody.linux-foundation.org>
[not found] ` <alpine.LFD.1.10.0807041250220.2815@woody.linux-foundation.org>
[not found] ` <20080704132716.f1e12554.akpm@linux-foundation.org>
[not found] ` <20080704204252.GM14894@parisc-linux.org>
2008-07-04 22:01 ` the printk problem Andrew Morton
2008-07-05 2:03 ` Matthew Wilcox
2008-07-22 10:05 ` [PATCH] Make u64 long long on all architectures (was: the printk problem) Andrew Morton
2008-07-22 10:36 ` Michael Ellerman
2008-07-22 10:53 ` Andrew Morton
2008-07-22 11:36 ` Benjamin Herrenschmidt
2008-07-22 11:35 ` Benjamin Herrenschmidt
2008-07-05 10:20 ` the printk problem Denys Vlasenko
2008-07-05 11:33 ` Jan Engelhardt
2008-07-05 12:52 ` Vegard Nossum
2008-07-05 13:24 ` Jan Engelhardt
2008-07-05 13:50 ` Vegard Nossum
2008-07-05 14:07 ` Jan Engelhardt
2008-07-05 17:56 ` Linus Torvalds
2008-07-05 18:40 ` Jan Engelhardt
2008-07-05 18:44 ` Linus Torvalds
2008-07-05 18:41 ` Vegard Nossum
2008-07-05 18:52 ` Matthew Wilcox
2008-07-06 0:02 ` Pekka Enberg
2008-07-06 5:17 ` Randy Dunlap
[not found] ` <1215212420.8970.8.camel@pasglop>
[not found] ` <alpine.LFD.1.10.0807041622270.2815@woody.linux-foundation.org>
[not found] ` <alpine.LFD.1.10.0807051523180.2847@woody.linux-foundation.org>
[not found] ` <20080706052741.GA18928@elte.hu>
2008-07-06 5:37 ` Linus Torvalds
2008-07-06 5:53 ` Ingo Molnar
2008-07-06 6:13 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080704104634.GA31634@digi.com \
--to=uwe.kleine-koenig@digi.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox