Re: [patch 3/3] genirq: introduce IRQF_ALLOW_NESTED flag for request_irq()

[Marc Zyngier]

On Sat, 13 Mar 2010 20:56:19 +0100 (CET), Thomas Gleixner
<tglx@xxxxxxxxxxxxx> wrote:

Hi Thomas,

> In general I have no objections, but one thing bothers me. We have no
> way to let a driver know whether it runs in a nested threaded context
> or in hard irq context. There might be (future) drivers which would be
> happy to know that to apply context dependent optimizations etc.
> 
> What about a new function which solves your problem and returns that
> information ? Something along the line:
> 
> int request_any_context_irq(....)
> {
> 	...
> 	if (desc->status & IRQ_NESTED_THREAD) {
> 	   	ret = request_threaded_irq();
> 		if (!ret)
> 			ret = IRQ_IS_NESTED;
> 
> 	} else {
> 		.....
> 			ret = IRQ_IS_NONTHREADED;
> 		else
> 			ret = IRQ_IS_THREADED;
> 
> 	}
> 	...
> 	return ret;
> }
> 
> You get the idea, right ?
> 
> It's a bit more code, but less magic and more flexible for further use
> cases.

What about the attached (sorry, webmail crap) patch? I deliberately left
IS_THREADED out of the picture, as I have the feeling that the caller has
to know if it really wants a threaded handler, and I couldn't see a way to
guess its intent.

Please note that this patch has only been compile-tested, as I'm traveling
for the rest of the week and don't have access to my boards.

Thanks,

        M.
-- 
Who you jivin' with that Cosmik Debris?From b776526f1a0f0b3f7b2e8fd1b9cfad49985635e2 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@xxxxxxxxxxxxxxx>
Date: Mon, 15 Mar 2010 22:56:33 +0000
Subject: [PATCH] genirq: introduce request_any_context_irq()

Now that we enjoy threaded interrupts, we're starting to see irq_chip
implementations (wm831x, pca953x) that make use of threaded interrupts
for the controller, and nested interrupts for the client interrupt. It
all works very well, with one drawback:

Drivers requesting an IRQ must now know whether the handler will
run in a thread context of not, and call request_threaded_irq() or
request_irq() accordingly.

The problem is that the requesting driver sometimes doesn't know
about the nature of the interrupt, specially when the interrupt
controller is a discrete chip (typically a GPIO expander connected
over I2C) that can be connected to a wide variety of otherwise perfectly
supported hardware.

This patch introduce the request_any_context_irq() function that mostly
mimics the usual request_irq(), except that it checks whether the irq
level is configured as nested or not, and calls the right backend.
On success, it also returns either IRQC_IS_HARDIRQ or IRQC_IS_NESTED.

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxxxxxxx>
---
 include/linux/interrupt.h |   26 ++++++++++++++++++++++++++
 kernel/irq/manage.c       |   45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 75f3f00..8081e40 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -107,6 +107,16 @@ struct irqaction {
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
 
+/**
+ * These flags can be returned by request_any_context_irq() and
+ * describe the context the interrupt will be run in.
+ *
+ * IRQC_IS_HARDIRQ - interrupt runs in hardirq context
+ * IRQC_IS_NESTED - interrupt runs in a nested threaded context
+ */
+#define IRQC_IS_HARDIRQ		0x00000000
+#define IRQC_IS_NESTED		0x00000001
+
 #ifdef CONFIG_GENERIC_HARDIRQS
 extern int __must_check
 request_threaded_irq(unsigned int irq, irq_handler_t handler,
@@ -120,6 +130,10 @@ request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
 	return request_threaded_irq(irq, handler, NULL, flags, name, dev);
 }
 
+extern int __must_check
+request_any_context_irq(unsigned int irq, irq_handler_t handler,
+			unsigned long flags, const char *name, void *dev_id);
+
 extern void exit_irq_thread(void);
 #else
 
@@ -141,6 +155,18 @@ request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	return request_irq(irq, handler, flags, name, dev);
 }
 
+static inline int __must_check
+request_any_context_irq(unsigned int irq, irq_handler_t handler,
+			unsigned long flags, const char *name, void *dev_id)
+{
+	int ret = request_irq(irq, handler, flags, name, dev_id);
+
+	if (!ret)
+		ret = IRQC_IS_HARDIRQ;
+
+	return ret;
+}
+
 static inline void exit_irq_thread(void) { }
 #endif
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index eb6078c..60b33bf 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1088,3 +1088,48 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	return retval;
 }
 EXPORT_SYMBOL(request_threaded_irq);
+
+/**
+ *	request_any_context_irq - allocate an interrupt line
+ *	@irq: Interrupt line to allocate
+ *	@handler: Function to be called when the IRQ occurs.
+ *		  Threaded handler for threaded interrupts.
+ *	@flags: Interrupt type flags
+ *	@name: An ascii name for the claiming device
+ *	@dev_id: A cookie passed back to the handler function
+ *
+ *	This call allocates interrupt resources and enables the
+ *	interrupt line and IRQ handling. It selects either a
+ *	hardirq or threaded handling method depending on the
+ *	context.
+ *
+ *	On failure, it returns a negative value. On success,
+ *	it returns either IRQC_IS_HARDIRQ or IRQC_IS_NESTED.
+ *
+ */
+int request_any_context_irq(unsigned int irq, irq_handler_t handler,
+			    unsigned long flags, const char *name, void *dev_id)
+{
+	struct irq_desc *desc;
+	int ret;
+
+	desc = irq_to_desc(irq);
+	if (!desc)
+		return -EINVAL;
+
+	if (desc->status & IRQ_NESTED_THREAD) {
+		ret = request_threaded_irq(irq, NULL, handler,
+					   flags, name, dev_id);
+		if (!ret)
+			ret = IRQC_IS_NESTED;
+
+		return ret;
+	}
+
+	ret = request_irq(irq, handler, flags, name, dev_id);
+	if (!ret)
+		ret = IRQC_IS_HARDIRQ;
+
+	return ret;
+}
+EXPORT_SYMBOL(request_any_context_irq);
-- 
1.7.0.2