max3100: added raise_threaded_irq
in [Kernel]
|
Prev: mfd: Add in XIIC to some configurations of timberdale
Next: [PATCH 4/4] Staging:rt2860: fix coding style issue in rt_main_dev.c This is a patch to the rt_main_dev.c file that fixes up errors found by the checkpatch.pl tool
From: Grant Likely on 19 Mar 2010 13:50 On Fri, Mar 19, 2010 at 2:38 AM, Christian Pellegrin <chripell(a)fsfe.org> wrote: > raise_threaded_irq schedules the execution of an interrupt thread > > Signed-off-by: Christian Pellegrin <chripell(a)fsfe.org> You should cc: Thomas and Ingo and lkml (which I just did) on patches to the threaded interrupt code. g. > --- > �include/linux/interrupt.h | � �3 +++ > �kernel/irq/manage.c � � � | � 27 +++++++++++++++++++++++++++ > �2 files changed, 30 insertions(+), 0 deletions(-) > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index 75f3f00..14c0c13 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -144,6 +144,9 @@ request_threaded_irq(unsigned int irq, irq_handler_t handler, > �static inline void exit_irq_thread(void) { } > �#endif > > +extern int raise_threaded_irq(unsigned int irq); > + > + > �extern void free_irq(unsigned int, void *); > > �struct device; > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index eb6078c..a7d21e0 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -1088,3 +1088,30 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, > � � � �return retval; > �} > �EXPORT_SYMBOL(request_threaded_irq); > + > +/** > + * � � raise_threaded_irq - triggers a threded interrupt > + * � � @irq: Interrupt line to trigger > + */ > +int raise_threaded_irq(unsigned int irq) > +{ > + � � � struct irq_desc *desc = irq_to_desc(irq); > + � � � struct irqaction *action; > + > + � � � if (!desc) > + � � � � � � � return -ENOENT; > + � � � action = desc->action; > + � � � if (!action) > + � � � � � � � return -ENOENT; > + � � � if (unlikely(!action->thread_fn)) > + � � � � � � � return -EINVAL; > + � � � if (likely(!test_bit(IRQTF_DIED, > + � � � � � � � � � � � � � �&action->thread_flags))) { > + � � � � � � � set_bit(IRQTF_RUNTHREAD, &action->thread_flags); > + � � � � � � � wake_up_process(action->thread); > + � � � } else { > + � � � � � � � return -ECHILD; > + � � � } > + � � � return 0; > +} > +EXPORT_SYMBOL(raise_threaded_irq); > -- > 1.5.6.5 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: christian pellegrin on 21 Mar 2010 03:40 Hi, On Fri, Mar 19, 2010 at 6:48 PM, Grant Likely <grant.likely(a)secretlab.ca> wrote: > You should cc: Thomas and Ingo and lkml (which I just did) on patches > to the threaded interrupt code. > ok, let me explain the reason for this function. The move from worqueues to the threaded interrupts was motivated by a reduction of latency in answering to RX buffer full interrupts. Threaded interrupts are SCHED_FIFO instead worqueues being SCHED_OTHER. In the case of MAX31x0 when we transmit a character we could have to receive one (this is efficient because SPI transfers are quite always bidirectional). The same routine is used both for tx and rx (an other things like changing parameters): this makes locking really simple. With this routine the thread interrupt handler could be the only kind of deferred work a driver for a simple hardware may ever need. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on 15 Apr 2010 19:30 On Tue, 23 Mar 2010, Christian Pellegrin wrote: > raise_threaded_irq schedules the execution of an interrupt thread I really have a hard time to understand _WHY_ we want to have that function. Interrupt threads are woken up either by the primary handler or by a interrupt demultiplexer and the code has all interfaces for that already. Can you please explain, what you are trying to achieve and why it can't be done with the existing interfaces ? > + > +/** > + * raise_threaded_irq - triggers a threded interrupt > + * @irq: Interrupt line to trigger > + */ > +int raise_threaded_irq(unsigned int irq) > +{ > + struct irq_desc *desc = irq_to_desc(irq); > + struct irqaction *action; > + > + if (!desc) > + return -ENOENT; > + action = desc->action; That's racy. You cannot access desc->action w/o holding desc->lock or having set the IRQ_INPROGRESS flag in desc->status under desc->lock. > + if (!action) > + return -ENOENT; > + if (unlikely(!action->thread_fn)) > + return -EINVAL; > + if (likely(!test_bit(IRQTF_DIED, > + &action->thread_flags))) { > + set_bit(IRQTF_RUNTHREAD, &action->thread_flags); > + wake_up_process(action->thread); > + } else { > + return -ECHILD; > + } > + return 0; > +} > +EXPORT_SYMBOL(raise_threaded_irq); EXPORT_SYMBOL_GPL if at all. Aside of that the name of of the function sucks: irq_wake_thread() perhaps ? But I still have no idea why we would need it at all. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: christian pellegrin on 16 Apr 2010 12:20 Hi, On Fri, Apr 16, 2010 at 1:22 AM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > >> raise_threaded_irq schedules the execution of an interrupt thread > > I really have a hard time to understand _WHY_ we want to have that > function. > ...... > > Can you please explain, what you are trying to achieve and why it > can't be done with the existing interfaces ? > The idea was that by using this function we just need one kind of deferred work (interrupt threads) instead of two (for example interrupt threads and workqueues) in some situations. This is very handy with devices that do transmission and reception at the same time, for example many SPI devices. The user case is the max3100 UART on SPI driver. The same SPI instruction both receives and sends chars. So when we need to send a char we just start the interrupt thread instead of having another kind of deferred work doing the job. This greatly simplifies locking and avoids duplication of functionality (otherwise we must have an interrupt thread that does reception and a workqueue that does sending and receiving for example) because everything is done in just one point. The move from worqueues to interrupt threads was motivated by the much smaller latency under load of the latter because they are scheduled as RT processes. I hope this doesn't sound like a terrible abuse of threaded interrupts. Let me know before I try to fix other problems you mentioned. Thanks -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on 16 Apr 2010 18:10
Christian, On Fri, 16 Apr 2010, christian pellegrin wrote: > Hi, > > On Fri, Apr 16, 2010 at 1:22 AM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > > > >> raise_threaded_irq schedules the execution of an interrupt thread > > > > I really have a hard time to understand _WHY_ we want to have that > > function. > > > ..... > > > > Can you please explain, what you are trying to achieve and why it > > can't be done with the existing interfaces ? > > > > The idea was that by using this function we just need one kind of > deferred work (interrupt threads) instead of two (for example > interrupt threads and workqueues) in some situations. This is very > handy with devices that do transmission and reception at the same > time, for example many SPI devices. The user case is the max3100 UART > on SPI driver. The same SPI instruction both receives and sends chars. > So when we need to send a char we just start the interrupt thread > instead of having another kind of deferred work doing the job. This > greatly simplifies locking and avoids duplication of functionality > (otherwise we must have an interrupt thread that does reception and a > workqueue that does sending and receiving for example) because > everything is done in just one point. The move from worqueues to > interrupt threads was motivated by the much smaller latency under load > of the latter because they are scheduled as RT processes. I hope this > doesn't sound like a terrible abuse of threaded interrupts. Let me > know before I try to fix other problems you mentioned. Thanks for the explanation. Now, that makes a lot of sense and I can see that it removes a lot of serialization issues and duplicated code pathes. So what you want is a mechanism to "inject" interrupts by software. I wonder whether we should restrict this mechanism to threaded handlers or just implement it in the following way: int irq_inject(unsigned int irq) { struct irq_desc *desc = irq_to_desc(irq); if (!desc) return -EINVAL; local_irq_disable(); desc->handle_irq(irq, desc); local_irq_enable(); return 0; } That would call the real interupt code path as it is called from the arch/*/kernel/irq.c:entry code and take care of all serialization issues. The drawback is that it will increase the irq statistics, but I think that's really a pure cosmetic problem. That requires that the primary interrupt handler code knows about the software "interrupt" event, but that's easy to solve. So the primary handler would just return IRQ_WAKE_THREAD as it would do for a real hardware triggered interrupt. But as a goodie that would work for non threaded interrupts as well. There is another thing which needs some care: This will not work out of the box when the irq is nested into some demultiplexing thread handler (IRQF_NESTED_THREAD). I'm too tried to look at this now, but I don't see a real showstopper there. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |