[RFC patch 0/3] Support for irq chips on slow busses (i2c, spi)

[RFC patch 0/3] Support for irq chips on slow busses (i2c, spi)

[Thomas Gleixner]

The support for irq chips on slow busses eg. i2c, spi has been
discussed to great length several times. Most of the details can be
found in this thread: http://lkml.org/lkml/2009/7/21/266

The following patch series is a round up of the various patch snippets
sent out during the discussion and the ideas we agreed on.

There are some subtle differences to the initial test patches:

1) Instead of using a separate flow handler we allow to request the
one shot feature with a flag on request_irq and integrate the oneshot
feature into the existing level/edge handlers.

2) Instead of conditional locking which makes the patch smaller but
hides the already known fact that a device sits behind a slow irq chip
I implemented a set of separate management functions (see patch
2/3). The initial idea with the conditional locking can be found here:
http://lkml.org/lkml/2009/7/22/163

3) patch 3/3 implements a helper function which allows the
demultiplexing thread to call the demux handlers in its own thread
context with out creating a separate thread for each subdevice
interrupt. The device driver needs to request the interrupt via
request_nested_slowbus_irq() to prevent the creation of the extra
thread.

Please have a thorough look and hopefully a test ride on your
favourite slowbus irq chip implementation so we can get this into .32

Thanks,

	tglx

[RFC patch 1/3] genirq: Add oneshot support

[Thomas Gleixner]

[-- Attachment #1: genirq-add-oneshot-support.patch --]
[-- Type: text/plain, Size: 5899 bytes --]

For threaded interrupt handlers we expect the hard interrupt handler
part to mask the interrupt on the originating device. The interrupt
line itself is reenabled after the hard interrupt handler has
executed.

This requires access to the originating device from hard interrupt
context which is not always possible. There are devices which can only
be accessed via a bus (i2c, spi, ...). The bus access requires thread
context. For such devices we need to keep the interrupt line masked
until the threaded handler has executed.

Add a new flag IRQF_ONESHOT which allows drivers to request that the
interrupt is not unmasked after the hard interrupt context handler has
been executed and the thread has been woken. The interrupt line is
unmasked after the thread handler function has been executed.

Note that for now IRQF_ONESHOT cannot be used with IRQF_SHARED to
avoid complex accounting mechanisms.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/interrupt.h |    4 ++++
 include/linux/irq.h       |    1 +
 kernel/irq/chip.c         |   14 +++++++++++---
 kernel/irq/manage.c       |   29 +++++++++++++++++++++++++++--
 4 files changed, 43 insertions(+), 5 deletions(-)

Index: linux-2.6-tip/include/linux/interrupt.h
===================================================================
--- linux-2.6-tip.orig/include/linux/interrupt.h
+++ linux-2.6-tip/include/linux/interrupt.h
@@ -49,6 +49,9 @@
  * IRQF_IRQPOLL - Interrupt is used for polling (only the interrupt that is
  *                registered first in an shared interrupt is considered for
  *                performance reasons)
+ * IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
+ *                Used by threaded interrupts which need to keep the
+ *                irq line disabled until the threaded handler has been run.
  */
 #define IRQF_DISABLED		0x00000020
 #define IRQF_SAMPLE_RANDOM	0x00000040
@@ -58,6 +61,7 @@
 #define IRQF_PERCPU		0x00000400
 #define IRQF_NOBALANCING	0x00000800
 #define IRQF_IRQPOLL		0x00001000
+#define IRQF_ONESHOT		0x00002000
 
 /*
  * Bits used by threaded handlers:
Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
@@ -69,6 +69,7 @@ typedef	void (*irq_flow_handler_t)(unsig
 #define IRQ_MOVE_PCNTXT		0x01000000	/* IRQ migration from process context */
 #define IRQ_AFFINITY_SET	0x02000000	/* IRQ affinity was set from userspace*/
 #define IRQ_SUSPENDED		0x04000000	/* IRQ has gone through suspend sequence */
+#define IRQ_ONESHOT		0x08000000	/* IRQ is not unmasked after hardirq */
 
 #ifdef CONFIG_IRQ_PER_CPU
 # define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
Index: linux-2.6-tip/kernel/irq/chip.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/chip.c
+++ linux-2.6-tip/kernel/irq/chip.c
@@ -382,7 +382,10 @@ handle_level_irq(unsigned int irq, struc
 
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
-	if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+
+	if (unlikely(desc->status & IRQ_ONESHOT))
+		desc->status |= IRQ_MASKED;
+	else if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
 		desc->chip->unmask(irq);
 out_unlock:
 	spin_unlock(&desc->lock);
@@ -478,8 +481,13 @@ handle_edge_irq(unsigned int irq, struct
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	/* Start handling the irq */
-	if (desc->chip->ack)
-		desc->chip->ack(irq);
+	if (unlikely(desc->status & IRQ_ONESHOT)) {
+		desc->status |= IRQ_MASKED;
+		mask_ack_irq(desc, irq);
+	} else {
+		if (desc->chip->ack)
+			desc->chip->ack(irq);
+	}
 
 	/* Mark the IRQ currently in progress.*/
 	desc->status |= IRQ_INPROGRESS;
Index: linux-2.6-tip/kernel/irq/manage.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/manage.c
+++ linux-2.6-tip/kernel/irq/manage.c
@@ -444,6 +444,21 @@ static int irq_wait_for_interrupt(struct
 }
 
 /*
+ * Oneshot interrupts keep the irq line masked until the threaded
+ * handler finished. unmask if the interrupt has not been disabled and
+ * is marked MASKED.
+ */
+static void irq_finalize_oneshot(unsigned int irq, struct irq_desc *desc)
+{
+	spin_lock_irq(&desc->lock);
+	if (!(desc->status & IRQ_DISABLED) && (desc->status & IRQ_MASKED)) {
+		desc->status &= ~IRQ_MASKED;
+		desc->chip->unmask(irq);
+	}
+	spin_unlock_irq(&desc->lock);
+}
+
+/*
  * Interrupt handler thread
  */
 static int irq_thread(void *data)
@@ -451,7 +466,7 @@ static int irq_thread(void *data)
 	struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO/2, };
 	struct irqaction *action = data;
 	struct irq_desc *desc = irq_to_desc(action->irq);
-	int wake;
+	int wake, oneshot = desc->status & IRQ_ONESHOT;
 
 	sched_setscheduler(current, SCHED_FIFO, &param);
 	current->irqaction = action;
@@ -475,6 +490,9 @@ static int irq_thread(void *data)
 			spin_unlock_irq(&desc->lock);
 
 			action->thread_fn(action->irq, action->dev_id);
+
+			if (oneshot)
+				irq_finalize_oneshot(action->irq, desc);
 		}
 
 		wake = atomic_dec_and_test(&desc->threads_active);
@@ -547,6 +565,10 @@ __setup_irq(unsigned int irq, struct irq
 		rand_initialize_irq(irq);
 	}
 
+	/* Oneshot interrupts are not allowed with shared */
+	if ((new->flags & IRQF_ONESHOT) && (new->flags & IRQF_SHARED))
+		return -EINVAL;
+
 	/*
 	 * Threaded handler ?
 	 */
@@ -620,9 +642,12 @@ __setup_irq(unsigned int irq, struct irq
 			desc->status |= IRQ_PER_CPU;
 #endif
 
-		desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING |
+		desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING | IRQ_ONESHOT |
 				  IRQ_INPROGRESS | IRQ_SPURIOUS_DISABLED);
 
+		if (new->flags & IRQF_ONESHOT)
+			desc->status |= IRQ_ONESHOT;
+
 		if (!(desc->status & IRQ_NOAUTOEN)) {
 			desc->depth = 0;
 			desc->status &= ~IRQ_DISABLED;

[RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Thomas Gleixner]

[-- Attachment #1: genirq-add-buslock-support.patch --]
[-- Type: text/plain, Size: 9628 bytes --]

Some interrupt chips are connected to a "slow" bus (i2c, spi ...). The
bus access needs to sleep and therefor cannot be called in atomic
contexts.

Some of the generic interrupt management functions like disable_irq(),
enable_irq() ... call interrupt chip functions with the irq_desc->lock
held and interrupts disabled. This does not work for such devices.

Provide a separate synchronization mechanism for such interrupt
chips. The irq_chip structure is extended by two optional functions
(bus_lock and bus_sync_and_unlock).

The idea is to serialize the bus access for those operations in the
core code so that drivers which are behind that bus operated interrupt
controller do not have to worry about it and just can use the normal
interfaces. To achieve this we add two function pointers to the
irq_chip: bus_lock and bus_sync_unlock.

bus_lock() is called to serialize access to the interrupt controller
bus.

Now the core code can issue chip->mask/unmask ... commands without
changing the fast path code at all. The chip implementation merily
stores that information in a chip private data structure and
returns. No bus interaction as these functions are called from atomic
context.

After that bus_sync_unlock() is called outside the atomic context. Now
the chip implementation issues the bus commands, waits for completion
and unlocks the interrupt controller bus.

The irq_chip implementation as pseudo code:

struct irq_chip_data {
       struct mutex   mutex;
       unsigned int   irq_offset;
       unsigned long  mask;
       unsigned long  mask_status;
}

static void bus_lock(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        mutex_lock(&data->mutex);
}

static void mask(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        irq -= data->irq_offset;
        data->mask |= (1 << irq);
}

static void unmask(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        irq -= data->irq_offset;
        data->mask &= ~(1 << irq);
}

static void bus_sync_unlock(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        if (data->mask != data->mask_status) {
                do_bus_magic_to_set_mask(data->mask);
                data->mask_status = data->mask;
        }
        mutex_unlock(&data->mutex);
}

For the device drivers a new set of functions is provided:
request_threaded_slowbus_irq, free_slowbus_irq, disable_slowbus_irq
and enable_slowbus_irq. They work the same as the !slowbus variants
with the only restriction that the calls need to come from non atomic
context.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/interrupt.h |    9 +++
 include/linux/irq.h       |    6 ++
 kernel/irq/manage.c       |  117 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 131 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/include/linux/interrupt.h
===================================================================
--- linux-2.6-tip.orig/include/linux/interrupt.h
+++ linux-2.6-tip/include/linux/interrupt.h
@@ -113,6 +113,11 @@ request_threaded_irq(unsigned int irq, i
 		     irq_handler_t thread_fn,
 		     unsigned long flags, const char *name, void *dev);
 
+extern int __must_check
+request_threaded_slowbus_irq(unsigned int irq, irq_handler_t handler,
+			     irq_handler_t thread_fn,
+			     unsigned long flags, const char *name, void *dev);
+
 static inline int __must_check
 request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
 	    const char *name, void *dev)
@@ -145,6 +150,7 @@ static inline void exit_irq_thread(void)
 #endif
 
 extern void free_irq(unsigned int, void *);
+extern void free_slowbus_irq(unsigned int irq, void *dev_id);
 
 struct device;
 
@@ -186,6 +192,9 @@ extern void disable_irq_nosync(unsigned 
 extern void disable_irq(unsigned int irq);
 extern void enable_irq(unsigned int irq);
 
+extern void disable_slowbus_irq(unsigned int irq);
+extern void enable_slowbus_irq(unsigned int irq);
+
 /* The following three functions are for the core kernel use only. */
 #ifdef CONFIG_GENERIC_HARDIRQS
 extern void suspend_device_irqs(void);
Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
@@ -101,6 +101,9 @@ struct msi_desc;
  * @set_type:		set the flow type (IRQ_TYPE_LEVEL/etc.) of an IRQ
  * @set_wake:		enable/disable power-management wake-on of an IRQ
  *
+ * @bus_lock:		function to lock access to slow bus (i2c) chips
+ * @bus_sync_unlock:	function to sync and unlock slow bus (i2c) chips
+ *
  * @release:		release function solely used by UML
  * @typename:		obsoleted by name, kept as migration helper
  */
@@ -124,6 +127,9 @@ struct irq_chip {
 	int		(*set_type)(unsigned int irq, unsigned int flow_type);
 	int		(*set_wake)(unsigned int irq, unsigned int on);
 
+	void		(*bus_lock)(unsigned int irq);
+	void		(*bus_sync_unlock)(unsigned int irq);
+
 	/* Currently used only by UML, might disappear one day.*/
 #ifdef CONFIG_IRQ_RELEASE_METHOD
 	void		(*release)(unsigned int irq, void *dev_id);
Index: linux-2.6-tip/kernel/irq/manage.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/manage.c
+++ linux-2.6-tip/kernel/irq/manage.c
@@ -253,6 +253,33 @@ void disable_irq(unsigned int irq)
 }
 EXPORT_SYMBOL(disable_irq);
 
+/**
+ *	disable_slowbus_irq - disable an slowbus irq and wait for completion
+ *	@irq: Interrupt to disable
+ *
+ *	Disable the selected interrupt line.  Enables and Disables are
+ *	nested.
+ *	This function waits for any pending IRQ handlers for this interrupt
+ *	to complete before returning. If you use this function while
+ *	holding a resource the IRQ handler may need you will deadlock.
+ *
+ *	This function must not be called from IRQ context.
+ */
+void disable_slowbus_irq(unsigned int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (!desc || !desc->chip || !desc->chip->bus_lock)
+		return;
+
+	desc->chip->bus_lock(irq);
+	disable_irq_nosync(irq);
+	if (desc->action)
+		synchronize_irq(irq);
+	desc->chip->bus_sync_unlock(irq);
+}
+EXPORT_SYMBOL(disable_slowbus_irq);
+
 void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
 {
 	if (resume)
@@ -302,6 +329,32 @@ void enable_irq(unsigned int irq)
 }
 EXPORT_SYMBOL(enable_irq);
 
+/**
+ *	enable_slowbus_irq - enable handling of a slowbus irq
+ *	@irq: Interrupt to enable
+ *
+ *	Undoes the effect of one call to disable_irq(). If this
+ *	matches the last disable, processing of interrupts on this
+ *	IRQ line is re-enabled.
+ *
+ *	This function must not be called from IRQ context
+ */
+void enable_slowbus_irq(unsigned int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+	unsigned long flags;
+
+	if (!desc || !desc->chip || !desc->chip->bus_lock)
+		return;
+
+	desc->chip->bus_lock(irq);
+	spin_lock_irqsave(&desc->lock, flags);
+	__enable_irq(desc, irq, false);
+	spin_unlock_irqrestore(&desc->lock, flags);
+	desc->chip->bus_sync_unlock(irq);
+}
+EXPORT_SYMBOL(enable_slowbus_irq);
+
 static int set_irq_wake_real(unsigned int irq, unsigned int on)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
@@ -859,6 +912,33 @@ void free_irq(unsigned int irq, void *de
 EXPORT_SYMBOL(free_irq);
 
 /**
+ *	free_slowbus_irq - free an interrupt allocated with request_slowbus_irq
+ *	@irq: Interrupt line to free
+ *	@dev_id: Device identity to free
+ *
+ *	Remove an interrupt handler. The handler is removed and if the
+ *	interrupt line is no longer in use by any driver it is disabled.
+ *	On a shared IRQ the caller must ensure the interrupt is disabled
+ *	on the card it drives before calling this function. The function
+ *	does not return until any executing interrupts for this IRQ
+ *	have completed.
+ *
+ *	This function must not be called from interrupt context.
+ */
+void free_slowbus_irq(unsigned int irq, void *dev_id)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (!desc || !desc->chip || !desc->chip->bus_lock)
+		return;
+
+	desc->chip->bus_lock(irq);
+	kfree(__free_irq(irq, dev_id));
+	desc->chip->bus_sync_unlock(irq);
+}
+EXPORT_SYMBOL(free_slowbus_irq);
+
+/**
  *	request_threaded_irq - allocate an interrupt line
  *	@irq: Interrupt line to allocate
  *	@handler: Function to be called when the IRQ occurs.
@@ -981,3 +1061,37 @@ int request_threaded_irq(unsigned int ir
 	return retval;
 }
 EXPORT_SYMBOL(request_threaded_irq);
+
+/**
+ *	request_slowbus_threaded_irq - allocate a slowbus interrupt line
+ *	@irq: Interrupt line to allocate
+ *	@handler: Function to be called when the IRQ occurs.
+ *		  Primary handler for threaded interrupts
+ *	@thread_fn: Function called from the irq handler thread
+ *		    If NULL, no irq thread is created
+ *	@irqflags: Interrupt type flags
+ *	@devname: An ascii name for the claiming device
+ *	@dev_id: A cookie passed back to the handler function
+ *
+ *	See request_threaded_irq for further information
+ */
+int request_slowbus_threaded_irq(unsigned int irq, irq_handler_t handler,
+				 irq_handler_t thread_fn,
+				 unsigned long irqflags,
+				 const char *devname, void *dev_id)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+	int res;
+
+	if (!desc || !desc->chip || !desc->chip->bus_lock)
+		return -EINVAL;
+
+	desc->chip->bus_lock(irq);
+	res = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
+				   dev_id);
+
+	desc->chip->bus_sync_unlock(irq);
+
+	return res;
+}
+EXPORT_SYMBOL(request_slowbus_threaded_irq);

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Peter Zijlstra]

On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote:
>  
> +/**
> + *	disable_slowbus_irq - disable an slowbus irq and wait for completion
> + *	@irq: Interrupt to disable
> + *
> + *	Disable the selected interrupt line.  Enables and Disables are
> + *	nested.
> + *	This function waits for any pending IRQ handlers for this interrupt
> + *	to complete before returning. If you use this function while
> + *	holding a resource the IRQ handler may need you will deadlock.
> + *
> + *	This function must not be called from IRQ context.
> + */
> +void disable_slowbus_irq(unsigned int irq)
> +{
> +	struct irq_desc *desc = irq_to_desc(irq);
> +
> +	if (!desc || !desc->chip || !desc->chip->bus_lock)
> +		return;
> +
> +	desc->chip->bus_lock(irq);
> +	disable_irq_nosync(irq);
> +	if (desc->action)
> +		synchronize_irq(irq);
> +	desc->chip->bus_sync_unlock(irq);
> +}
> +EXPORT_SYMBOL(disable_slowbus_irq);

Should we also not check that desc->chip->bus_lock is not set for the
regular function disable_irq()?

It seems to me mixing disable_irq() and disable_slowbus_irq() is a
recipe for disaster.

Same for the other slowbus functions of course.

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Thomas Gleixner]

On Fri, 14 Aug 2009, Peter Zijlstra wrote:
> On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote:
> >  
> > +/**
> > + *	disable_slowbus_irq - disable an slowbus irq and wait for completion
> > + *	@irq: Interrupt to disable
> > + *
> > + *	Disable the selected interrupt line.  Enables and Disables are
> > + *	nested.
> > + *	This function waits for any pending IRQ handlers for this interrupt
> > + *	to complete before returning. If you use this function while
> > + *	holding a resource the IRQ handler may need you will deadlock.
> > + *
> > + *	This function must not be called from IRQ context.
> > + */
> > +void disable_slowbus_irq(unsigned int irq)
> > +{
> > +	struct irq_desc *desc = irq_to_desc(irq);
> > +
> > +	if (!desc || !desc->chip || !desc->chip->bus_lock)
> > +		return;
> > +
> > +	desc->chip->bus_lock(irq);
> > +	disable_irq_nosync(irq);
> > +	if (desc->action)
> > +		synchronize_irq(irq);
> > +	desc->chip->bus_sync_unlock(irq);
> > +}
> > +EXPORT_SYMBOL(disable_slowbus_irq);
> 
> Should we also not check that desc->chip->bus_lock is not set for the
> regular function disable_irq()?
> 
> It seems to me mixing disable_irq() and disable_slowbus_irq() is a
> recipe for disaster.
> 
> Same for the other slowbus functions of course.

Yeah, that's what I wanted to avoid with the first version, which did
the conditional locking and did not require a separate API, but Ingo
frowned upon the conditional lock.

Thanks,

	tglx

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Ingo Molnar]

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

> On Fri, 14 Aug 2009, Peter Zijlstra wrote:
> > On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote:
> > >  
> > > +/**
> > > + *	disable_slowbus_irq - disable an slowbus irq and wait for completion
> > > + *	@irq: Interrupt to disable
> > > + *
> > > + *	Disable the selected interrupt line.  Enables and Disables are
> > > + *	nested.
> > > + *	This function waits for any pending IRQ handlers for this interrupt
> > > + *	to complete before returning. If you use this function while
> > > + *	holding a resource the IRQ handler may need you will deadlock.
> > > + *
> > > + *	This function must not be called from IRQ context.
> > > + */
> > > +void disable_slowbus_irq(unsigned int irq)
> > > +{
> > > +	struct irq_desc *desc = irq_to_desc(irq);
> > > +
> > > +	if (!desc || !desc->chip || !desc->chip->bus_lock)
> > > +		return;
> > > +
> > > +	desc->chip->bus_lock(irq);
> > > +	disable_irq_nosync(irq);
> > > +	if (desc->action)
> > > +		synchronize_irq(irq);
> > > +	desc->chip->bus_sync_unlock(irq);
> > > +}
> > > +EXPORT_SYMBOL(disable_slowbus_irq);
> > 
> > Should we also not check that desc->chip->bus_lock is not set for the
> > regular function disable_irq()?
> > 
> > It seems to me mixing disable_irq() and disable_slowbus_irq() 
> > is a recipe for disaster.
> > 
> > Same for the other slowbus functions of course.
> 
> Yeah, that's what I wanted to avoid with the first version, which 
> did the conditional locking and did not require a separate API, 
> but Ingo frowned upon the conditional lock.

Mind posting that version too?

Conditional locking is really nasty but if the only other option is 
nastier there's not much we can do, is there?

	Ingo

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Peter Zijlstra]

On Fri, 2009-08-14 at 11:18 +0200, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Fri, 14 Aug 2009, Peter Zijlstra wrote:
> > > On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote:
> > > >  
> > > > +/**
> > > > + *	disable_slowbus_irq - disable an slowbus irq and wait for completion
> > > > + *	@irq: Interrupt to disable
> > > > + *
> > > > + *	Disable the selected interrupt line.  Enables and Disables are
> > > > + *	nested.
> > > > + *	This function waits for any pending IRQ handlers for this interrupt
> > > > + *	to complete before returning. If you use this function while
> > > > + *	holding a resource the IRQ handler may need you will deadlock.
> > > > + *
> > > > + *	This function must not be called from IRQ context.
> > > > + */
> > > > +void disable_slowbus_irq(unsigned int irq)
> > > > +{
> > > > +	struct irq_desc *desc = irq_to_desc(irq);
> > > > +
> > > > +	if (!desc || !desc->chip || !desc->chip->bus_lock)
> > > > +		return;
> > > > +
> > > > +	desc->chip->bus_lock(irq);
> > > > +	disable_irq_nosync(irq);
> > > > +	if (desc->action)
> > > > +		synchronize_irq(irq);
> > > > +	desc->chip->bus_sync_unlock(irq);
> > > > +}
> > > > +EXPORT_SYMBOL(disable_slowbus_irq);
> > > 
> > > Should we also not check that desc->chip->bus_lock is not set for the
> > > regular function disable_irq()?
> > > 
> > > It seems to me mixing disable_irq() and disable_slowbus_irq() 
> > > is a recipe for disaster.
> > > 
> > > Same for the other slowbus functions of course.
> > 
> > Yeah, that's what I wanted to avoid with the first version, which 
> > did the conditional locking and did not require a separate API, 
> > but Ingo frowned upon the conditional lock.
> 
> Mind posting that version too?
> 
> Conditional locking is really nasty but if the only other option is 
> nastier there's not much we can do, is there?

Ah what I meant was that we can make the posted version better by adding

  BUG_ON(desc && desc->chip && desc->chip->bus_lock);

to disable_irq/enable_irq/free_irq.

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Thomas Gleixner]

On Fri, 14 Aug 2009, Ingo Molnar wrote:
> > Yeah, that's what I wanted to avoid with the first version, which 
> > did the conditional locking and did not require a separate API, 
> > but Ingo frowned upon the conditional lock.
> 
> Mind posting that version too?
> 
> Conditional locking is really nasty but if the only other option is 
> nastier there's not much we can do, is there?

Here you go.

Thanks,

	tglx
----
Subject: genirq: Add buslock support
From: Thomas Gleixner <tglx@linutronix.de>
Date: Thu, 13 Aug 2009 12:17:48 +0200

Some interrupt chips are connected to a "slow" bus (i2c, spi ...). The
bus access needs to sleep and therefor cannot be called in atomic
contexts.

Some of the generic interrupt management functions like disable_irq(),
enable_irq() ... call interrupt chip functions with the irq_desc->lock
held and interrupts disabled. This does not work for such devices.

Provide a separate synchronization mechanism for such interrupt
chips. The irq_chip structure is extended by two optional functions
(bus_lock and bus_sync_and_unlock).

The idea is to serialize the bus access for those operations in the
core code so that drivers which are behind that bus operated interrupt
controller do not have to worry about it and just can use the normal
interfaces. To achieve this we add two function pointers to the
irq_chip: bus_lock and bus_sync_unlock.

bus_lock() is called to serialize access to the interrupt controller
bus.

Now the core code can issue chip->mask/unmask ... commands without
changing the fast path code at all. The chip implementation merily
stores that information in a chip private data structure and
returns. No bus interaction as these functions are called from atomic
context.

After that bus_sync_unlock() is called outside the atomic context. Now
the chip implementation issues the bus commands, waits for completion
and unlocks the interrupt controller bus.

The irq_chip implementation as pseudo code:

struct irq_chip_data {
       struct mutex   mutex;
       unsigned int   irq_offset;
       unsigned long  mask;
       unsigned long  mask_status;
}

static void bus_lock(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        mutex_lock(&data->mutex);
}

static void mask(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        irq -= data->irq_offset;
        data->mask |= (1 << irq);
}

static void unmask(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        irq -= data->irq_offset;
        data->mask &= ~(1 << irq);
}

static void bus_sync_unlock(unsigned int irq)
{
        struct irq_chip_data *data = get_irq_desc_chip_data(irq);

        if (data->mask != data->mask_status) {
                do_bus_magic_to_set_mask(data->mask);
                data->mask_status = data->mask;
        }
        mutex_unlock(&data->mutex);
}

The device drivers can use request_threaded_irq, free_irq, disable_irq
and enable_irq as usual with the only restriction that the calls need
to come from non atomic context.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/irq.h |    6 ++++++
 kernel/irq/manage.c |   34 +++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
@@ -101,6 +101,9 @@ struct msi_desc;
  * @set_type:		set the flow type (IRQ_TYPE_LEVEL/etc.) of an IRQ
  * @set_wake:		enable/disable power-management wake-on of an IRQ
  *
+ * @bus_lock:		function to lock access to slow bus (i2c) chips
+ * @bus_sync_unlock:	function to sync and unlock slow bus (i2c) chips
+ *
  * @release:		release function solely used by UML
  * @typename:		obsoleted by name, kept as migration helper
  */
@@ -124,6 +127,9 @@ struct irq_chip {
 	int		(*set_type)(unsigned int irq, unsigned int flow_type);
 	int		(*set_wake)(unsigned int irq, unsigned int on);
 
+	void		(*bus_lock)(unsigned int irq);
+	void		(*bus_sync_unlock)(unsigned int irq);
+
 	/* Currently used only by UML, might disappear one day.*/
 #ifdef CONFIG_IRQ_RELEASE_METHOD
 	void		(*release)(unsigned int irq, void *dev_id);
Index: linux-2.6-tip/kernel/irq/manage.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/manage.c
+++ linux-2.6-tip/kernel/irq/manage.c
@@ -17,6 +17,22 @@
 
 #include "internals.h"
 
+static inline void chip_bus_lock(unsigned int irq, struct irq_desc *desc)
+{
+	if (unlikely(desc->chip->bus_lock)) {
+		might_sleep();
+		desc->chip->bus_lock(irq);
+	}
+}
+
+static inline void chip_bus_sync_unlock(unsigned int irq, struct irq_desc *desc)
+{
+	if (unlikely(desc->chip->bus_sync_unlock)) {
+		might_sleep();
+		desc->chip->bus_sync_unlock(irq);
+	}
+}
+
 /**
  *	synchronize_irq - wait for pending IRQ handlers (on other CPUs)
  *	@irq: interrupt number to wait for
@@ -222,9 +238,11 @@ void disable_irq_nosync(unsigned int irq
 	if (!desc)
 		return;
 
+	chip_bus_lock(irq, desc);
 	spin_lock_irqsave(&desc->lock, flags);
 	__disable_irq(desc, irq, false);
 	spin_unlock_irqrestore(&desc->lock, flags);
+	chip_bus_sync_unlock(irq, desc);
 }
 EXPORT_SYMBOL(disable_irq_nosync);
 
@@ -286,7 +304,8 @@ void __enable_irq(struct irq_desc *desc,
  *	matches the last disable, processing of interrupts on this
  *	IRQ line is re-enabled.
  *
- *	This function may be called from IRQ context.
+ *	This function may be called from IRQ context only when
+ *	desc->chip->bus_lock and desc->chip->bus_sync_unlock are NULL !
  */
 void enable_irq(unsigned int irq)
 {
@@ -296,9 +315,11 @@ void enable_irq(unsigned int irq)
 	if (!desc)
 		return;
 
+	chip_bus_lock(irq, desc);
 	spin_lock_irqsave(&desc->lock, flags);
 	__enable_irq(desc, irq, false);
 	spin_unlock_irqrestore(&desc->lock, flags);
+	chip_bus_sync_unlock(irq, desc);
 }
 EXPORT_SYMBOL(enable_irq);
 
@@ -854,7 +875,14 @@ EXPORT_SYMBOL_GPL(remove_irq);
  */
 void free_irq(unsigned int irq, void *dev_id)
 {
+	struct irq_desc *desc = irq_to_desc(irq);
+
+	if (!desc)
+		return;
+
+	chip_bus_lock(irq, desc);
 	kfree(__free_irq(irq, dev_id));
+	chip_bus_sync_unlock(irq, desc);
 }
 EXPORT_SYMBOL(free_irq);
 
@@ -955,10 +983,14 @@ int request_threaded_irq(unsigned int ir
 	action->name = devname;
 	action->dev_id = dev_id;
 
+	chip_bus_lock(irq, desc);
+
 	retval = __setup_irq(irq, desc, action);
 	if (retval)
 		kfree(action);
 
+	chip_bus_sync_unlock(irq, desc);
+
 #ifdef CONFIG_DEBUG_SHIRQ
 	if (irqflags & IRQF_SHARED) {
 		/*

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Pavel Machek]

Hi!

> Index: linux-2.6-tip/include/linux/interrupt.h
> ===================================================================
> --- linux-2.6-tip.orig/include/linux/interrupt.h
> +++ linux-2.6-tip/include/linux/interrupt.h
> @@ -113,6 +113,11 @@ request_threaded_irq(unsigned int irq, i
>  		     irq_handler_t thread_fn,
>  		     unsigned long flags, const char *name, void *dev);
>  
> +extern int __must_check
> +request_threaded_slowbus_irq(unsigned int irq, irq_handler_t handler,
> +			     irq_handler_t thread_fn,
> +			     unsigned long flags, const char *name, void *dev);
> +
>  static inline int __must_check
>  request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
>  	    const char *name, void *dev)
> @@ -145,6 +150,7 @@ static inline void exit_irq_thread(void)
>  #endif
>  
>  extern void free_irq(unsigned int, void *);
> +extern void free_slowbus_irq(unsigned int irq, void *dev_id);
>  
>  struct device;
>  
> @@ -186,6 +192,9 @@ extern void disable_irq_nosync(unsigned 
>  extern void disable_irq(unsigned int irq);
>  extern void enable_irq(unsigned int irq);
>  
> +extern void disable_slowbus_irq(unsigned int irq);
> +extern void enable_slowbus_irq(unsigned int irq);
> +

AFAICT this means that driver would need to know what kind of IRQ it
is hooked to, right? That will lead to some ugly code in drivers that
can handle both normal and slowbus irqs, right?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Thomas Gleixner]

On Fri, 14 Aug 2009, Pavel Machek wrote:
> > +extern void disable_slowbus_irq(unsigned int irq);
> > +extern void enable_slowbus_irq(unsigned int irq);
> > +
> 
> AFAICT this means that driver would need to know what kind of IRQ it
> is hooked to, right? That will lead to some ugly code in drivers that
> can handle both normal and slowbus irqs, right?

Are there such drivers in reality ?

Thanks,

	tglx

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Pavel Machek]

On Fri 2009-08-14 12:20:49, Thomas Gleixner wrote:
> On Fri, 14 Aug 2009, Pavel Machek wrote:
> > > +extern void disable_slowbus_irq(unsigned int irq);
> > > +extern void enable_slowbus_irq(unsigned int irq);
> > > +
> > 
> > AFAICT this means that driver would need to know what kind of IRQ it
> > is hooked to, right? That will lead to some ugly code in drivers that
> > can handle both normal and slowbus irqs, right?
> 
> Are there such drivers in reality ?

I think so. Between Spitz and Akita, some gpios were moved to slow i2c
extender. For stuff like buttons...

Now... it is probably already fixed somehow for Spitz/Akita, but I
believe more hardware designers do such things....
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Mark Brown]

On Fri, Aug 14, 2009 at 12:20:49PM +0200, Thomas Gleixner wrote:
> On Fri, 14 Aug 2009, Pavel Machek wrote:

> > AFAICT this means that driver would need to know what kind of IRQ it
> > is hooked to, right? That will lead to some ugly code in drivers that
> > can handle both normal and slowbus irqs, right?

> Are there such drivers in reality ?

Yes.  The GPIO based stuff is the prime example but there's other
examples - one is the WM831x touchscreen (no driver in mainline yet)
which can use interrupts via the main interrupt controller on the CPU
but also has the option of bringing the interrupt signals out to
dedicated pins on the chip for direct connection to the CPU precisely to
avoid the overheads of these slow interrupt controllers.

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Ingo Molnar]

* Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Fri, Aug 14, 2009 at 12:20:49PM +0200, Thomas Gleixner wrote:
> > On Fri, 14 Aug 2009, Pavel Machek wrote:
> 
> > > AFAICT this means that driver would need to know what kind of IRQ it
> > > is hooked to, right? That will lead to some ugly code in drivers that
> > > can handle both normal and slowbus irqs, right?
> 
> > Are there such drivers in reality ?
> 
> Yes.  The GPIO based stuff is the prime example but there's other 
> examples - one is the WM831x touchscreen (no driver in mainline 
> yet) which can use interrupts via the main interrupt controller on 
> the CPU but also has the option of bringing the interrupt signals 
> out to dedicated pins on the chip for direct connection to the CPU 
> precisely to avoid the overheads of these slow interrupt 
> controllers.

This would call for Thomas's first version of the patch, that is 
transparent to drivers - the IRQ subsystem will know how to lock 
access to the line.

How about implementing that first patch in a cleaner way - can we 
somehow express the slow-bus property purely via the irqchip? Or is 
that too lowlevel?

	Ingo

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Thomas Gleixner]

On Sat, 15 Aug 2009, Ingo Molnar wrote:
> * Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > On Fri, Aug 14, 2009 at 12:20:49PM +0200, Thomas Gleixner wrote:
> > > On Fri, 14 Aug 2009, Pavel Machek wrote:
> > 
> > > > AFAICT this means that driver would need to know what kind of IRQ it
> > > > is hooked to, right? That will lead to some ugly code in drivers that
> > > > can handle both normal and slowbus irqs, right?
> > 
> > > Are there such drivers in reality ?
> > 
> > Yes.  The GPIO based stuff is the prime example but there's other 
> > examples - one is the WM831x touchscreen (no driver in mainline 
> > yet) which can use interrupts via the main interrupt controller on 
> > the CPU but also has the option of bringing the interrupt signals 
> > out to dedicated pins on the chip for direct connection to the CPU 
> > precisely to avoid the overheads of these slow interrupt 
> > controllers.
> 
> This would call for Thomas's first version of the patch, that is 
> transparent to drivers - the IRQ subsystem will know how to lock 
> access to the line.
> 
> How about implementing that first patch in a cleaner way - can we 
> somehow express the slow-bus property purely via the irqchip? Or is 
> that too lowlevel?

The problem here is that the management functions serialize via
irqdesc->lock and call the chip level function with the lock held
(preemption and interrupts disabled). So we can not access the slow
bus chips from these low level functions.

If the low level functions just store the information and schedule it
for bus access then there is no serialization anymore. Lets look at
disable_irq():

	spin_lock_irqsave(&desc->lock);
	....
	desc->chip->mask();
		schedule bus access;
	...
	spin_lock_irqsave(&desc->lock);

So now we return, but the mask has not reached the chip.

The idea of bus_lock/bus_sync_unlock() was to provide well defined
synchronization points for the chip level implementation to do the bus
update and have this serialized against other management functions.

	desc->chip->bus_lock();
		Take the chip->bus mutex

	spin_lock_irqsave(&desc->lock);
	....
	desc->chip->mask();
		store mask information
	...
	spin_lock_irqsave(&desc->lock);

	desc->chip->bus_sync_unlock();
		Update the mask via the slow bus
		Release chip->bus mutex

That way we have made sure that the change to the chip actually hits
the hardware before we allow further management operations and it
simplifies the code for the chip implementation as the bus access can
be done in the context of the caller w/o the need of an extra
thread/workqueue ...

Thanks,

	tglx

Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses

[Ingo Molnar]

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

> On Sat, 15 Aug 2009, Ingo Molnar wrote:
> > * Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > 
> > > On Fri, Aug 14, 2009 at 12:20:49PM +0200, Thomas Gleixner wrote:
> > > > On Fri, 14 Aug 2009, Pavel Machek wrote:
> > > 
> > > > > AFAICT this means that driver would need to know what kind of IRQ it
> > > > > is hooked to, right? That will lead to some ugly code in drivers that
> > > > > can handle both normal and slowbus irqs, right?
> > > 
> > > > Are there such drivers in reality ?
> > > 
> > > Yes.  The GPIO based stuff is the prime example but there's other 
> > > examples - one is the WM831x touchscreen (no driver in mainline 
> > > yet) which can use interrupts via the main interrupt controller on 
> > > the CPU but also has the option of bringing the interrupt signals 
> > > out to dedicated pins on the chip for direct connection to the CPU 
> > > precisely to avoid the overheads of these slow interrupt 
> > > controllers.
> > 
> > This would call for Thomas's first version of the patch, that is 
> > transparent to drivers - the IRQ subsystem will know how to lock 
> > access to the line.
> > 
> > How about implementing that first patch in a cleaner way - can we 
> > somehow express the slow-bus property purely via the irqchip? Or is 
> > that too lowlevel?
> 
> The problem here is that the management functions serialize via 
> irqdesc->lock and call the chip level function with the lock held 
> (preemption and interrupts disabled). So we can not access the 
> slow bus chips from these low level functions.
> 
> If the low level functions just store the information and schedule 
> it for bus access then there is no serialization anymore. Lets 
> look at disable_irq():
> 
> 	spin_lock_irqsave(&desc->lock);
> 	....
> 	desc->chip->mask();
> 		schedule bus access;
> 	...
> 	spin_lock_irqsave(&desc->lock);
> 
> So now we return, but the mask has not reached the chip.
> 
> The idea of bus_lock/bus_sync_unlock() was to provide well defined
> synchronization points for the chip level implementation to do the bus
> update and have this serialized against other management functions.
> 
> 	desc->chip->bus_lock();
> 		Take the chip->bus mutex
> 
> 	spin_lock_irqsave(&desc->lock);
> 	....
> 	desc->chip->mask();
> 		store mask information
> 	...
> 	spin_lock_irqsave(&desc->lock);
> 
> 	desc->chip->bus_sync_unlock();
> 		Update the mask via the slow bus
> 		Release chip->bus mutex
> 
> That way we have made sure that the change to the chip actually 
> hits the hardware before we allow further management operations 
> and it simplifies the code for the chip implementation as the bus 
> access can be done in the context of the caller w/o the need of an 
> extra thread/workqueue ...

Given the alternatives i'd prefer this one now ...

	Ingo

[RFC patch 3/3] genirq: Support nested threaded irq handling

[Thomas Gleixner]

[-- Attachment #1: genirq-support-nested-threaded-irq-handling.patch --]
[-- Type: text/plain, Size: 5881 bytes --]

Interrupt chips which are behind a slow bus (i2c, spi ...) and
demultiplex other interrupt sources need to run their interrupt
handler in a thread. 

The demultiplexed interrupt handlers need to run in thread context as
well and need to finish before the demux handler thread can reenable
the interrupt line. So instead of creating separate threads which need
to synchronize with the demux handler thread we provide a function
which calls the interrupt thread function of the sub device in the
context of the demux handler thread.

To avoid that a separate thread is created for the subdevices the
function request_nested_slowbus_irq() is provided which sets a dummy
handler for the primary handler which should never be called. A flag
prevents the creation of the separate handler thread in __setup_irq().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/interrupt.h |    9 +++++++++
 kernel/irq/chip.c         |   44 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/irq/internals.h    |    2 ++
 kernel/irq/manage.c       |   28 +++++++++++++++++++++++++++-
 4 files changed, 82 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/include/linux/interrupt.h
===================================================================
--- linux-2.6-tip.orig/include/linux/interrupt.h
+++ linux-2.6-tip/include/linux/interrupt.h
@@ -52,6 +52,9 @@
  * IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
  *                Used by threaded interrupts which need to keep the
  *                irq line disabled until the threaded handler has been run.
+ * IRQF_NESTED -  Interrupt is nested into another threaded interrupt. Handler
+ *		  is called in the parent interrupt thread context
+ *
  */
 #define IRQF_DISABLED		0x00000020
 #define IRQF_SAMPLE_RANDOM	0x00000040
@@ -62,6 +65,7 @@
 #define IRQF_NOBALANCING	0x00000800
 #define IRQF_IRQPOLL		0x00001000
 #define IRQF_ONESHOT		0x00002000
+#define IRQF_NESTED		0x00004000
 
 /*
  * Bits used by threaded handlers:
@@ -125,6 +129,11 @@ request_irq(unsigned int irq, irq_handle
 	return request_threaded_irq(irq, handler, NULL, flags, name, dev);
 }
 
+extern int __must_check
+request_nested_slowbus_irq(unsigned int irq, irq_handler_t thread_fn,
+			   unsigned long irqflags, const char *devname,
+			   void *dev_id);
+
 extern void exit_irq_thread(void);
 #else
 
Index: linux-2.6-tip/kernel/irq/chip.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/chip.c
+++ linux-2.6-tip/kernel/irq/chip.c
@@ -300,6 +300,50 @@ static inline void mask_ack_irq(struct i
 }
 
 /**
+ *	handle_nested_irq - Handle a nested irq from a irq thread
+ *	@irq:	the interrupt number
+ *
+ *	Handle interrupts which are nested into a threaded interrupt
+ *	handler. The handler function is called inside the calling
+ *	threads context.
+ */
+void handle_nested_irq(unsigned int irq)
+{
+	struct irq_desc *desc = irq_to_desc(irq);
+	struct irqaction *action;
+	irqreturn_t action_ret;
+
+	might_sleep();
+
+	spin_lock_irq(&desc->lock);
+
+	kstat_incr_irqs_this_cpu(irq, desc);
+
+	action = desc->action;
+	if (unlikely(!action || (desc->status & IRQ_DISABLED)))
+		goto out_unlock;
+
+	desc->status |= IRQ_INPROGRESS;
+	spin_unlock_irq(&desc->lock);
+
+	action_ret = action->thread_fn(action->irq, action->dev_id);
+	if (!noirqdebug)
+		note_interrupt(irq, desc, action_ret);
+
+	spin_lock_irq(&desc->lock);
+	desc->status &= ~IRQ_INPROGRESS;
+
+out_unlock:
+	spin_unlock_irq(&desc->lock);
+}
+
+irqreturn_t handle_nested_irq_primary(int irq, void *dev_id)
+{
+	WARN(1, "Primary handler called for nested irq %d\n", irq);
+	return IRQ_NONE;
+}
+
+/**
  *	handle_simple_irq - Simple and software-decoded IRQs.
  *	@irq:	the interrupt number
  *	@desc:	the interrupt description structure for this irq
Index: linux-2.6-tip/kernel/irq/internals.h
===================================================================
--- linux-2.6-tip.orig/kernel/irq/internals.h
+++ linux-2.6-tip/kernel/irq/internals.h
@@ -45,6 +45,8 @@ extern int irq_select_affinity_usr(unsig
 extern void
 irq_set_thread_affinity(struct irq_desc *desc, const struct cpumask *cpumask);
 
+extern irqreturn_t handle_nested_irq_primary(int irq, void *dev_id);
+
 /*
  * Debugging printout:
  */
Index: linux-2.6-tip/kernel/irq/manage.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/manage.c
+++ linux-2.6-tip/kernel/irq/manage.c
@@ -625,7 +625,7 @@ __setup_irq(unsigned int irq, struct irq
 	/*
 	 * Threaded handler ?
 	 */
-	if (new->thread_fn) {
+	if (new->thread_fn && !(new->flags & IRQF_NESTED)) {
 		struct task_struct *t;
 
 		t = kthread_create(irq_thread, new, "irq/%d-%s", irq,
@@ -1095,3 +1095,29 @@ int request_slowbus_threaded_irq(unsigne
 	return res;
 }
 EXPORT_SYMBOL(request_slowbus_threaded_irq);
+
+/**
+ *	request_nested_slowbus_irq - allocate an interrupt line for a nested irq
+ *	@irq: Interrupt line to allocate
+ *	@thread_fn: Function called from the irq handler thread
+ *	@irqflags: Interrupt type flags
+ *	@devname: An ascii name for the claiming device
+ *	@dev_id: A cookie passed back to the handler function
+ *
+ *	This function is used for interrupts which nest into another
+ *	threaded interrupt handler, e.g. subdevice interrupts of an
+ *	interrupt controller which is connected via i2c/spi or other
+ *	slow path mechanisms. The thread_fn is called from the
+ *	demultiplexing interrupt thread via handle_nested_irq().
+ */
+int request_nested_slowbus_irq(unsigned int irq, irq_handler_t thread_fn,
+			       unsigned long irqflags, const char *devname,
+			       void *dev_id)
+{
+	irqflags |= IRQF_NESTED;
+
+	return request_threaded_slowbus_irq(irq, handle_nested_irq_primary,
+					    thread_fn, irqflags, devname,
+					    dev_id);
+}
+EXPORT_SYMBOL(request_nested_slowbus_irq);

[RFC patch 0/3] Support for irq chips on slow busses (i2c, spi) - V2

[Thomas Gleixner]

The support for irq chips on slow busses eg. i2c, spi has been
discussed to great length several times. Most of the details can be
found in this thread: http://lkml.org/lkml/2009/7/21/266

The following patch series is a round up of the various patch snippets
sent out during the discussion and the ideas we agreed on.

This is version 2 of the patch series. The main changes vs. V1
(http://lkml.org/lkml/2009/8/13/348)

 - patch 1/3: Provide a generic primary handler function which just
   	      returns IRQ_WAKE_THREAD as this is all what oneshot
   	      threaded handlers need to avoid useless copies of that
   	      all over the place.

 - patch 2/3: Reverted to the initial idea of conditional locking to
   	      allow drivers to be used for both slowbus and standard
   	      interrupts without any magic in the driver code

 - patch 3/3: To avoid different driver code for nested or separate
   	      thread handling a new function is provided which allows
   	      to mark the interrupt nested. request_threaded_irq()
   	      creates a separate thread only when the flag is not set.

Please have a thorough look and hopefully a test ride on your
favourite slowbus irq chip implementation so we can get this into .32

Thanks,

	tglx

[RFC patch 1/3] genirq: Add oneshot support

[Thomas Gleixner]

[-- Attachment #1: genirq-add-oneshot-support.patch --]
[-- Type: text/plain, Size: 7707 bytes --]

For threaded interrupt handlers we expect the hard interrupt handler
part to mask the interrupt on the originating device. The interrupt
line itself is reenabled after the hard interrupt handler has
executed.

This requires access to the originating device from hard interrupt
context which is not always possible. There are devices which can only
be accessed via a bus (i2c, spi, ...). The bus access requires thread
context. For such devices we need to keep the interrupt line masked
until the threaded handler has executed.

Add a new flag IRQF_ONESHOT which allows drivers to request that the
interrupt is not unmasked after the hard interrupt context handler has
been executed and the thread has been woken. The interrupt line is
unmasked after the thread handler function has been executed.

Note that for now IRQF_ONESHOT cannot be used with IRQF_SHARED to
avoid complex accounting mechanisms.

For oneshot interrupts the primary handler simply returns
IRQ_WAKE_THREAD and does nothing else. A generic implementation
irq_oneshot_primary_handler() is provided to avoid useless copies all
over the place.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Trilok Soni <soni.trilok@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Brian Swetland <swetland@google.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: m.szyprowski@samsung.com
Cc: t.fujak@samsung.com
Cc: kyungmin.park@samsung.com,
Cc: David Brownell <david-b@pacbell.net>
Cc: Daniel Ribeiro <drwyrm@gmail.com>
Cc: arve@android.com
Cc: Barry Song <21cnbao@gmail.com>

---
 include/linux/interrupt.h |    6 ++++++
 include/linux/irq.h       |    1 +
 kernel/irq/chip.c         |   31 ++++++++++++++++++++++++++++---
 kernel/irq/manage.c       |   29 +++++++++++++++++++++++++++--
 4 files changed, 62 insertions(+), 5 deletions(-)

Index: linux-2.6-tip/include/linux/interrupt.h
===================================================================
--- linux-2.6-tip.orig/include/linux/interrupt.h
+++ linux-2.6-tip/include/linux/interrupt.h
@@ -49,6 +49,9 @@
  * IRQF_IRQPOLL - Interrupt is used for polling (only the interrupt that is
  *                registered first in an shared interrupt is considered for
  *                performance reasons)
+ * IRQF_ONESHOT - Interrupt is not reenabled after the hardirq handler finished.
+ *                Used by threaded interrupts which need to keep the
+ *                irq line disabled until the threaded handler has been run.
  */
 #define IRQF_DISABLED		0x00000020
 #define IRQF_SAMPLE_RANDOM	0x00000040
@@ -58,6 +61,7 @@
 #define IRQF_PERCPU		0x00000400
 #define IRQF_NOBALANCING	0x00000800
 #define IRQF_IRQPOLL		0x00001000
+#define IRQF_ONESHOT		0x00002000
 
 /*
  * Bits used by threaded handlers:
@@ -286,6 +290,8 @@ static inline int disable_irq_wake(unsig
 	return set_irq_wake(irq, 0);
 }
 
+extern irqreturn_t oneshot_irq_primary_handler(int irq, void *dev_id);
+
 #else /* !CONFIG_GENERIC_HARDIRQS */
 /*
  * NOTE: non-genirq architectures, if they want to support the lock
Index: linux-2.6-tip/include/linux/irq.h
===================================================================
--- linux-2.6-tip.orig/include/linux/irq.h
+++ linux-2.6-tip/include/linux/irq.h
@@ -69,6 +69,7 @@ typedef	void (*irq_flow_handler_t)(unsig
 #define IRQ_MOVE_PCNTXT		0x01000000	/* IRQ migration from process context */
 #define IRQ_AFFINITY_SET	0x02000000	/* IRQ affinity was set from userspace*/
 #define IRQ_SUSPENDED		0x04000000	/* IRQ has gone through suspend sequence */
+#define IRQ_ONESHOT		0x08000000	/* IRQ is not unmasked after hardirq */
 
 #ifdef CONFIG_IRQ_PER_CPU
 # define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
Index: linux-2.6-tip/kernel/irq/chip.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/chip.c
+++ linux-2.6-tip/kernel/irq/chip.c
@@ -300,6 +300,23 @@ static inline void mask_ack_irq(struct i
 }
 
 /**
+ *	irq_oneshot_primary_handler - Handle oneshot interrupt primary handler
+ *	@irq:		the interrupt number
+ *	@dev_id:	cookie to identify the device
+ *
+ *	For oneshot interrupts which keep the interrupt line masked
+ *	until the threaded handler has been executed, the only
+ *	functionality of the primary handler is to return
+ *	IRQ_WAKE_THREAD. This is the generic implementation which
+ *	avoids lots of duplicates all over the place
+ */
+irqreturn_t irq_oneshot_primary_handler(int irq, void *dev_id)
+{
+	return IRQ_WAKE_THREAD;
+}
+EXPORT_SYMBOL_GPL(irq_oneshot_primary_handler);
+
+/**
  *	handle_simple_irq - Simple and software-decoded IRQs.
  *	@irq:	the interrupt number
  *	@desc:	the interrupt description structure for this irq
@@ -382,7 +399,10 @@ handle_level_irq(unsigned int irq, struc
 
 	spin_lock(&desc->lock);
 	desc->status &= ~IRQ_INPROGRESS;
-	if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+
+	if (unlikely(desc->status & IRQ_ONESHOT))
+		desc->status |= IRQ_MASKED;
+	else if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
 		desc->chip->unmask(irq);
 out_unlock:
 	spin_unlock(&desc->lock);
@@ -478,8 +498,13 @@ handle_edge_irq(unsigned int irq, struct
 	kstat_incr_irqs_this_cpu(irq, desc);
 
 	/* Start handling the irq */
-	if (desc->chip->ack)
-		desc->chip->ack(irq);
+	if (unlikely(desc->status & IRQ_ONESHOT)) {
+		desc->status |= IRQ_MASKED;
+		mask_ack_irq(desc, irq);
+	} else {
+		if (desc->chip->ack)
+			desc->chip->ack(irq);
+	}
 
 	/* Mark the IRQ currently in progress.*/
 	desc->status |= IRQ_INPROGRESS;
Index: linux-2.6-tip/kernel/irq/manage.c
===================================================================
--- linux-2.6-tip.orig/kernel/irq/manage.c
+++ linux-2.6-tip/kernel/irq/manage.c
@@ -444,6 +444,21 @@ static int irq_wait_for_interrupt(struct
 }
 
 /*
+ * Oneshot interrupts keep the irq line masked until the threaded
+ * handler finished. unmask if the interrupt has not been disabled and
+ * is marked MASKED.
+ */
+static void irq_finalize_oneshot(unsigned int irq, struct irq_desc *desc)
+{
+	spin_lock_irq(&desc->lock);
+	if (!(desc->status & IRQ_DISABLED) && (desc->status & IRQ_MASKED)) {
+		desc->status &= ~IRQ_MASKED;
+		desc->chip->unmask(irq);
+	}
+	spin_unlock_irq(&desc->lock);
+}
+
+/*
  * Interrupt handler thread
  */
 static int irq_thread(void *data)
@@ -451,7 +466,7 @@ static int irq_thread(void *data)
 	struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO/2, };
 	struct irqaction *action = data;
 	struct irq_desc *desc = irq_to_desc(action->irq);
-	int wake;
+	int wake, oneshot = desc->status & IRQ_ONESHOT;
 
 	sched_setscheduler(current, SCHED_FIFO, &param);
 	current->irqaction = action;
@@ -475,6 +490,9 @@ static int irq_thread(void *data)
 			spin_unlock_irq(&desc->lock);
 
 			action->thread_fn(action->irq, action->dev_id);
+
+			if (oneshot)
+				irq_finalize_oneshot(action->irq, desc);
 		}
 
 		wake = atomic_dec_and_test(&desc->threads_active);
@@ -547,6 +565,10 @@ __setup_irq(unsigned int irq, struct irq
 		rand_initialize_irq(irq);
 	}
 
+	/* Oneshot interrupts are not allowed with shared */
+	if ((new->flags & IRQF_ONESHOT) && (new->flags & IRQF_SHARED))
+		return -EINVAL;
+
 	/*
 	 * Threaded handler ?
 	 */
@@ -620,9 +642,12 @@ __setup_irq(unsigned int irq, struct irq
 			desc->status |= IRQ_PER_CPU;
 #endif
 
-		desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING |
+		desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING | IRQ_ONESHOT |
 				  IRQ_INPROGRESS | IRQ_SPURIOUS_DISABLED);
 
+		if (new->flags & IRQF_ONESHOT)
+			desc->status |= IRQ_ONESHOT;
+
 		if (!(desc->status & IRQ_NOAUTOEN)) {
 			desc->depth = 0;
 			desc->status &= ~IRQ_DISABLED;

Re: [RFC patch 1/3] genirq: Add oneshot support

[Dmitry Torokhov]

Hi Thomas,

On Sat, Aug 15, 2009 at 05:48:33PM -0000, Thomas Gleixner wrote:
>  
>  /**
> + *	irq_oneshot_primary_handler - Handle oneshot interrupt primary handler
> + *	@irq:		the interrupt number
> + *	@dev_id:	cookie to identify the device
> + *
> + *	For oneshot interrupts which keep the interrupt line masked
> + *	until the threaded handler has been executed, the only
> + *	functionality of the primary handler is to return
> + *	IRQ_WAKE_THREAD. This is the generic implementation which
> + *	avoids lots of duplicates all over the place
> + */
> +irqreturn_t irq_oneshot_primary_handler(int irq, void *dev_id)
> +{
> +	return IRQ_WAKE_THREAD;
> +}
> +EXPORT_SYMBOL_GPL(irq_oneshot_primary_handler);

This kind of handler is useful not only for users of oneshot interrupts
but also other drivers using threaded IRQs. So maybe we should rename it
to default_threaded_irq_handler() and instead of exporting it simply
have it installed automatically when driver requests NULL in place of
IRQ handler in request_threaded_irq()?

Also, if IRQF_ONESHOT definition would make into mainline sooner than
later that would be great - then I'd be able to put all the drivers that
will end up using it into my next branch and not be concerned of
breaking linux-next.

-- 
Dmitry

Re: [RFC patch 1/3] genirq: Add oneshot support

[Thomas Gleixner]

Dmitry,

On Sat, 15 Aug 2009, Dmitry Torokhov wrote:
> Hi Thomas,
> 
> On Sat, Aug 15, 2009 at 05:48:33PM -0000, Thomas Gleixner wrote:
> >  
> >  /**
> > + *	irq_oneshot_primary_handler - Handle oneshot interrupt primary handler
> > + *	@irq:		the interrupt number
> > + *	@dev_id:	cookie to identify the device
> > + *
> > + *	For oneshot interrupts which keep the interrupt line masked
> > + *	until the threaded handler has been executed, the only
> > + *	functionality of the primary handler is to return
> > + *	IRQ_WAKE_THREAD. This is the generic implementation which
> > + *	avoids lots of duplicates all over the place
> > + */
> > +irqreturn_t irq_oneshot_primary_handler(int irq, void *dev_id)
> > +{
> > +	return IRQ_WAKE_THREAD;
> > +}
> > +EXPORT_SYMBOL_GPL(irq_oneshot_primary_handler);
> 
> This kind of handler is useful not only for users of oneshot interrupts
> but also other drivers using threaded IRQs. So maybe we should rename it
> to default_threaded_irq_handler() and instead of exporting it simply
> have it installed automatically when driver requests NULL in place of
> IRQ handler in request_threaded_irq()?

Good point. If handler == NULL and thread_fn != NULL. That's
reasonable. Will rework.
 
> Also, if IRQF_ONESHOT definition would make into mainline sooner than
> later that would be great - then I'd be able to put all the drivers that
> will end up using it into my next branch and not be concerned of
> breaking linux-next.

Hmm, the ONESHOT definition alone won't give you the testing you
want. If all involved folks agree on the patch series I can commit it
into a standalone branch which can be pulled into the development
branches of interested driver maintainers. Git will deal with that
just fine.

@Andrew: any opinion on that ?

Thanks,

	tglx