Support for irq chips on slow busses (i2c, spi) - V2
cover Support for irq chips on slow busses (i2c, spi) - V2
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
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.
--- 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(-)
include/linux/interrupt.h +6 -0
@@ -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
include/linux/irq.h +1 -0
@@ -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)
kernel/irq/chip.c +28 -3
@@ -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, strucspin_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, structkstat_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 ;
kernel/irq/manage.c +27 -2
@@ -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 , ¶m); 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 irqrand_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
Hi Thomas,
Quoted 12 lines · Thomas Gleixner
/**
+ *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
+ */
kernel/irq/chip.c static inline void mask_ack_irq(struct i · line 317
Quoted 5 lines
kernel/irq/chip.c +5
-0+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.
Signature · 1 line
Dmitry
Re: [RFC patch 1/3] genirq: Add oneshot support
Dmitry,
Quoted 18 lines · Dmitry Torokhov
Hi Thomas,
Quoted 15 lines · Thomas Gleixner
/**
+ *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
+ */kernel/irq/chip.c +3
-0+irqreturn_t irq_oneshot_primary_handler (int irq,void *dev_id) +{ +return IRQ_WAKE_THREAD ;
Quoted 8 lines
kernel/irq/chip.c static inline void mask_ack_irq(struct i · line 317
Quoted 2 lines
kernel/irq/chip.c +2
-0+} +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 inrequest_threaded_irq()?
Good point. If handler == NULL and thread_fn != NULL. That's reasonable. Will rework.
Quoted 4 lines
Also, if
IRQF_ONESHOTdefinition 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
RFC PATCH 2/3 genirq: Add buslock support
In reply to Thomas GleixnerSome 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 theirq_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.
--- include/linux/irq.h | 6 ++++++ kernel/irq/chip.c | 2 ++ kernel/irq/internals.h | 13 +++++++++++++ kernel/irq/manage.c | 19 ++++++++++++++++++- 4 files changed, 39 insertions(+), 1 deletion(-)
include/linux/irq.h +6 -0
@@ -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);
kernel/irq/chip.c +2 -0
@@ -597,6 +597,7 @@ __set_irq_handler(unsigned int irq, irq_ desc->chip = &dummy_irq_chip; } +chip_bus_lock (irq, desc);spin_lock_irqsave (&desc->lock, flags);/* Uninstall? */
@@ -616,6 +617,7 @@ __set_irq_handler(unsigned int irq, irq_ desc->chip->startup (irq); }spin_unlock_irqrestore (&desc->lock, flags); +chip_bus_sync_unlock (irq, desc); }EXPORT_SYMBOL_GPL (__set_irq_handler);
kernel/irq/internals.h +13 -0
@@ -45,6 +45,19 @@ extern int irq_select_affinity_usr(unsigextern void irq_set_thread_affinity (struct irq_desc *desc,const struct cpumask *cpumask); +/* Inline functions for support of irq chips on slow busses */ +static inline void chip_bus_lock (unsigned int irq,struct irq_desc *desc) +{ +if (unlikely (desc->chip->bus_lock)) + 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)) + desc->chip->bus_sync_unlock (irq); +} +/* * Debugging printout: */
kernel/irq/manage.c +18 -1
@@ -222,9 +222,11 @@ void disable_irq_nosync(unsigned int irqif (!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 +288,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 +299,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);
@@ -450,12 +455,14 @@ static int irq_wait_for_interrupt(struct*/ static void irq_finalize_oneshot (unsigned int irq,struct irq_desc *desc) { +chip_bus_lock (irq, 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); +chip_bus_sync_unlock (irq, desc); }/*
@@ -854,7 +861,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,7 +969,10 @@ 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); +chip_bus_sync_unlock (irq, desc); +if (retval)kfree (action);
RFC PATCH 3/3 genirq: Support nested threaded irq handling
In reply to Thomas GleixnerInterrupt 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 the easiest way is to run the sub device handlers in the context of the demultiplexing handler thread.
To avoid that a separate thread is created for the subdevices the function set_nested_irq_thread() is provided which sets the IRQ_NESTED_THREAD flag in the interrupt descriptor.
A driver which calls request_threaded_irq() must not be aware of the fact that the threaded handler is called in the context of the demultiplexing handler thread. The setup code checks the IRQ_NESTED_THREAD flag which was set from the irq chip setup code and does not setup a separate thread for the interrupt. The primary function which is provided by the device driver is replaced by an internal dummy function which warns when it is called.
For the demultiplexing handler a helper function handle_nested_irq() is provided which calls the demux interrupt thread function in the context of the caller and does the proper interrupt accounting and takes the interrupt disabled status of the demultiplexed subdevice into account.
--- include/linux/irq.h | 3 ++ kernel/irq/chip.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++ kernel/irq/internals.h | 2 + kernel/irq/manage.c | 24 ++++++++++++++-- 4 files changed, 99 insertions(+), 3 deletions(-)
include/linux/irq.h +3 -0
@@ -70,6 +70,7 @@ typedef void (*irq_flow_handler_t)(unsig#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 */ +#define IRQ_NESTED_THREAD 0x10000000/* IRQ is nested into another, no own handler thread */ #ifdef CONFIG_IRQ_PER_CPU # define CHECK_IRQ_PER_CPU (var) ((var) & IRQ_PER_CPU)
@@ -379,6 +380,8 @@ set_irq_chained_handler(unsigned int irq__set_irq_handler (irq, handle,1 ,NULL ); } +extern void set_irq_nested_thread (unsigned int irq,int nest); +extern void set_irq_noprobe (unsigned int irq);extern void set_irq_probe (unsigned int irq);
kernel/irq/chip.c +73 -0
85 lines not on this page · raw message
kernel/irq/internals.h +2 -0
@@ -58,6 +58,8 @@ static inline void chip_bus_sync_unlock( desc->chip->bus_sync_unlock (irq); } +extern irqreturn_t handle_nested_irq_primary (int irq,void *dev_id); +/* * Debugging printout: */
kernel/irq/manage.c +21 -3
@@ -547,7 +547,7 @@ __setup_irq(unsigned int irq, struct irqstruct irqaction *old, **old_ptr;const char *old_name =NULL ;unsigned long flags; -int shared =0 ; +int nested, shared =0 ;int ret;if (!desc)
@@ -577,9 +577,27 @@ __setup_irq(unsigned int irq, struct irqreturn -EINVAL ;/* -* Threaded handler ? +* Check whether the interrupt nests into another interrupt +* thread. */ -if (new->thread_fn) { + nested = desc->status &IRQ_NESTED_THREAD ; +if (nested) { +if (!new->thread_fn) +return -EINVAL ; +/* +* Replace the primary handler which was provided from +* the driver for non nested interrupt handling by the +* dummy function which warns when called. +*/ + new->handler = handle_nested_irq_primary; + } + +/* +* Create a handler thread when a thread function is supplied +* and the interrupt does not nest into another interrupt +* thread. +*/ +if (new->thread_fn && !nested) {struct task_struct *t; t = kthread_create(irq_thread, new, "irq/%d-%s", irq,
Hi!
Quoted 19 lines
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_THREADas 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
Quoted 7 lines
- 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
FWIW, it looks ok to me.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 2 lines · Thomas Gleixner
Please have a thorough look and hopefully a test ride on your favourite slowbus irq chip implementation so we can get this into .32
I've given this a spin with such a chip and it seems to be working OK for me, though I've only done very light testing. The only issue I ran into was a lack of a prototype for handle_nested_irq() - I've sent a patch for that.
Thanks a lot for working on this!