public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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


  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