* [RFC PATCH] irq: handle private interrupt registration @ 2010-05-26 20:29 ` adharmap 0 siblings, 0 replies; 10+ messages in thread From: adharmap at codeaurora.org @ 2010-05-26 20:29 UTC (permalink / raw) To: linux-arm-kernel From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> The current code fails to register a handler for the same irq without taking in to account that it could be a per cpu interrupt. If the IRQF_PERCPU flag is set, enable the interrupt on that cpu and return success. Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> --- On systems with an interrupt controller that supports private interrupts per core, it is not possible to call request_irq/setup_irq from multiple cores for the same irq. This is because the second+ invocation of __setup_irq checks if the previous hndler had a IRQ_SHARED flag set and errors out if not. The current irq handling code doesnt take in to account what cpu it is executing on. Usually the local interrupt controller registers are banked per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked registers. One way to get around this problem is to call the setup_irq on a single cpu while other cpus simply enable their private interrupts by writing to their banked registers For eg. code in arch/arm/time/smp_twd.c /* Make sure our local interrupt controller has this enabled */ local_irq_save(flags); get_irq_chip(clk->irq)->unmask(clk->irq); local_irq_restore(flags); This looks like a hacky way to get local interrupts working on multiple cores. The patch adds a check for PERCPU flag in __setup_irq - if an handler is present it simply enables that interrupt for that core and returns 0. arch/arm/Kconfig | 5 +++++ kernel/irq/manage.c | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fcb79c9..ceefffd 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -979,6 +979,11 @@ config SMP If you don't know what to do here, say N. +config IRQ_PER_CPU + bool + depends on SMP + default y + config HAVE_ARM_SCU bool depends on SMP diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index bde4c66..9592a2d 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -683,6 +683,37 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) old_ptr = &desc->action; old = *old_ptr; if (old) { +#if defined(CONFIG_IRQ_PER_CPU) + /* All handlers must agree on per-cpuness */ + if ((old->flags & IRQF_PERCPU) != + (new->flags & IRQF_PERCPU)) + goto mismatch; + + if (old->flags & IRQF_PERCPU) { + /* the chip must have been set for this interrupt*/ + if (!(desc->status & IRQ_NOAUTOEN)) { + desc->depth = 0; + desc->status &= ~IRQ_DISABLED; + desc->chip->startup(irq); + } else + /* Undo nested disables: */ + desc->depth = 1; + + spin_unlock_irqrestore(&desc->lock, flags); + if (new->thread) + wake_up_process(new->thread); + return 0; + } +#endif + + /* they are the same types and same handler + * perhaps it is a private cpu interrupt + */ + if (old->flags == new->flags + && old->handler == new->handler) + setup_affinity(irq, desc); + return 0; + /* * Can't share interrupts unless both agree to and are * the same type (level, edge, polarity). So both flag @@ -695,13 +726,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) goto mismatch; } -#if defined(CONFIG_IRQ_PER_CPU) - /* All handlers must agree on per-cpuness */ - if ((old->flags & IRQF_PERCPU) != - (new->flags & IRQF_PERCPU)) - goto mismatch; -#endif - /* add new interrupt at end of irq queue */ do { old_ptr = &old->next; -- 1.5.6.3 -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH] irq: handle private interrupt registration @ 2010-05-26 20:29 ` adharmap 0 siblings, 0 replies; 10+ messages in thread From: adharmap @ 2010-05-26 20:29 UTC (permalink / raw) To: tglx Cc: Abhijeet Dharmapurikar, Mark Brown, Dmitry Torokhov, Trilok Soni, Pavel Machek, Brian Swetland, Joonyoung Shim, m.szyprowski, t.fujak, kyungmin.park, David Brownell, Daniel Ribeiro, arve, Barry Song, Russell King, Bryan Huntsman, Iliyan Malchev, Michael Buesch, Bruno Premont, linux-arm-kernel, linux-kernel From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> The current code fails to register a handler for the same irq without taking in to account that it could be a per cpu interrupt. If the IRQF_PERCPU flag is set, enable the interrupt on that cpu and return success. Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> --- On systems with an interrupt controller that supports private interrupts per core, it is not possible to call request_irq/setup_irq from multiple cores for the same irq. This is because the second+ invocation of __setup_irq checks if the previous hndler had a IRQ_SHARED flag set and errors out if not. The current irq handling code doesnt take in to account what cpu it is executing on. Usually the local interrupt controller registers are banked per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked registers. One way to get around this problem is to call the setup_irq on a single cpu while other cpus simply enable their private interrupts by writing to their banked registers For eg. code in arch/arm/time/smp_twd.c /* Make sure our local interrupt controller has this enabled */ local_irq_save(flags); get_irq_chip(clk->irq)->unmask(clk->irq); local_irq_restore(flags); This looks like a hacky way to get local interrupts working on multiple cores. The patch adds a check for PERCPU flag in __setup_irq - if an handler is present it simply enables that interrupt for that core and returns 0. arch/arm/Kconfig | 5 +++++ kernel/irq/manage.c | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fcb79c9..ceefffd 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -979,6 +979,11 @@ config SMP If you don't know what to do here, say N. +config IRQ_PER_CPU + bool + depends on SMP + default y + config HAVE_ARM_SCU bool depends on SMP diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index bde4c66..9592a2d 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -683,6 +683,37 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) old_ptr = &desc->action; old = *old_ptr; if (old) { +#if defined(CONFIG_IRQ_PER_CPU) + /* All handlers must agree on per-cpuness */ + if ((old->flags & IRQF_PERCPU) != + (new->flags & IRQF_PERCPU)) + goto mismatch; + + if (old->flags & IRQF_PERCPU) { + /* the chip must have been set for this interrupt*/ + if (!(desc->status & IRQ_NOAUTOEN)) { + desc->depth = 0; + desc->status &= ~IRQ_DISABLED; + desc->chip->startup(irq); + } else + /* Undo nested disables: */ + desc->depth = 1; + + spin_unlock_irqrestore(&desc->lock, flags); + if (new->thread) + wake_up_process(new->thread); + return 0; + } +#endif + + /* they are the same types and same handler + * perhaps it is a private cpu interrupt + */ + if (old->flags == new->flags + && old->handler == new->handler) + setup_affinity(irq, desc); + return 0; + /* * Can't share interrupts unless both agree to and are * the same type (level, edge, polarity). So both flag @@ -695,13 +726,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) goto mismatch; } -#if defined(CONFIG_IRQ_PER_CPU) - /* All handlers must agree on per-cpuness */ - if ((old->flags & IRQF_PERCPU) != - (new->flags & IRQF_PERCPU)) - goto mismatch; -#endif - /* add new interrupt at end of irq queue */ do { old_ptr = &old->next; -- 1.5.6.3 -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH] irq: handle private interrupt registration 2010-05-26 20:29 ` adharmap @ 2010-06-01 22:26 ` Andrew Morton -1 siblings, 0 replies; 10+ messages in thread From: Andrew Morton @ 2010-06-01 22:26 UTC (permalink / raw) To: linux-arm-kernel On Wed, 26 May 2010 13:29:54 -0700 adharmap at codeaurora.org wrote: > From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > The current code fails to register a handler for the same irq > without taking in to account that it could be a per cpu interrupt. > If the IRQF_PERCPU flag is set, enable the interrupt on that cpu > and return success. > > Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 > Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > --- > > On systems with an interrupt controller that supports > private interrupts per core, it is not possible to call > request_irq/setup_irq from multiple cores for the same irq. This is because > the second+ invocation of __setup_irq checks if the previous > hndler had a IRQ_SHARED flag set and errors out if not. > > The current irq handling code doesnt take in to account what cpu it > is executing on. Usually the local interrupt controller registers are banked > per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked > registers. > > One way to get around this problem is to call the setup_irq on a single cpu > while other cpus simply enable their private interrupts by writing to their > banked registers > > For eg. code in arch/arm/time/smp_twd.c > /* Make sure our local interrupt controller has this enabled */ > local_irq_save(flags); > get_irq_chip(clk->irq)->unmask(clk->irq); > local_irq_restore(flags); > > This looks like a hacky way to get local interrupts working on > multiple cores. > > The patch adds a check for PERCPU flag in __setup_irq - if an handler is > present it simply enables that interrupt for that core and returns 0. > > ... > > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -683,6 +683,37 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > old_ptr = &desc->action; > old = *old_ptr; > if (old) { > +#if defined(CONFIG_IRQ_PER_CPU) > + /* All handlers must agree on per-cpuness */ > + if ((old->flags & IRQF_PERCPU) != > + (new->flags & IRQF_PERCPU)) > + goto mismatch; > + > + if (old->flags & IRQF_PERCPU) { > + /* the chip must have been set for this interrupt*/ > + if (!(desc->status & IRQ_NOAUTOEN)) { > + desc->depth = 0; > + desc->status &= ~IRQ_DISABLED; > + desc->chip->startup(irq); > + } else > + /* Undo nested disables: */ > + desc->depth = 1; > + > + spin_unlock_irqrestore(&desc->lock, flags); The rest of the code uses raw_spin_unlock_irqrestore(). I don't know _why_ is uses this. There are no code comments, and the 239007b8440abff689632f50cdf0f2b9e895b534 changelog is: : genirq: Convert irq_desc.lock to raw_spinlock : : Convert locks which cannot be sleeping locks in preempt-rt to : raw_spinlocks. which is pathetically useless. But I suppose we should ignorantly copy it and hope we're not screwing something up. > + if (new->thread) > + wake_up_process(new->thread); > + return 0; > + } > +#endif > + > + /* they are the same types and same handler > + * perhaps it is a private cpu interrupt > + */ > + if (old->flags == new->flags > + && old->handler == new->handler) > + setup_affinity(irq, desc); > + return 0; And this appears to have forgotten to undo the lock altogether, which makes one wonder about the testing coverage. It also embeds a `return' statement deep inside a huge and complex function, which is invariably bad. And in so doing it bypasses the register_irq_proc() and register_handler_proc() calls. I have no way of knowing whether that was deliberate or whether it was a bug. If it was deliberate then some code and'/or changelog commentary is needed, so that others don't think that it is a bug too. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] irq: handle private interrupt registration @ 2010-06-01 22:26 ` Andrew Morton 0 siblings, 0 replies; 10+ messages in thread From: Andrew Morton @ 2010-06-01 22:26 UTC (permalink / raw) To: adharmap Cc: tglx, Mark Brown, Dmitry Torokhov, Trilok Soni, Pavel Machek, Brian Swetland, Joonyoung Shim, m.szyprowski, t.fujak, kyungmin.park, David Brownell, Daniel Ribeiro, arve, Barry Song, Russell King, Bryan Huntsman, Iliyan Malchev, Michael Buesch, Bruno Premont, linux-arm-kernel, linux-kernel On Wed, 26 May 2010 13:29:54 -0700 adharmap@codeaurora.org wrote: > From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > The current code fails to register a handler for the same irq > without taking in to account that it could be a per cpu interrupt. > If the IRQF_PERCPU flag is set, enable the interrupt on that cpu > and return success. > > Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 > Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > --- > > On systems with an interrupt controller that supports > private interrupts per core, it is not possible to call > request_irq/setup_irq from multiple cores for the same irq. This is because > the second+ invocation of __setup_irq checks if the previous > hndler had a IRQ_SHARED flag set and errors out if not. > > The current irq handling code doesnt take in to account what cpu it > is executing on. Usually the local interrupt controller registers are banked > per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked > registers. > > One way to get around this problem is to call the setup_irq on a single cpu > while other cpus simply enable their private interrupts by writing to their > banked registers > > For eg. code in arch/arm/time/smp_twd.c > /* Make sure our local interrupt controller has this enabled */ > local_irq_save(flags); > get_irq_chip(clk->irq)->unmask(clk->irq); > local_irq_restore(flags); > > This looks like a hacky way to get local interrupts working on > multiple cores. > > The patch adds a check for PERCPU flag in __setup_irq - if an handler is > present it simply enables that interrupt for that core and returns 0. > > ... > > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -683,6 +683,37 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > old_ptr = &desc->action; > old = *old_ptr; > if (old) { > +#if defined(CONFIG_IRQ_PER_CPU) > + /* All handlers must agree on per-cpuness */ > + if ((old->flags & IRQF_PERCPU) != > + (new->flags & IRQF_PERCPU)) > + goto mismatch; > + > + if (old->flags & IRQF_PERCPU) { > + /* the chip must have been set for this interrupt*/ > + if (!(desc->status & IRQ_NOAUTOEN)) { > + desc->depth = 0; > + desc->status &= ~IRQ_DISABLED; > + desc->chip->startup(irq); > + } else > + /* Undo nested disables: */ > + desc->depth = 1; > + > + spin_unlock_irqrestore(&desc->lock, flags); The rest of the code uses raw_spin_unlock_irqrestore(). I don't know _why_ is uses this. There are no code comments, and the 239007b8440abff689632f50cdf0f2b9e895b534 changelog is: : genirq: Convert irq_desc.lock to raw_spinlock : : Convert locks which cannot be sleeping locks in preempt-rt to : raw_spinlocks. which is pathetically useless. But I suppose we should ignorantly copy it and hope we're not screwing something up. > + if (new->thread) > + wake_up_process(new->thread); > + return 0; > + } > +#endif > + > + /* they are the same types and same handler > + * perhaps it is a private cpu interrupt > + */ > + if (old->flags == new->flags > + && old->handler == new->handler) > + setup_affinity(irq, desc); > + return 0; And this appears to have forgotten to undo the lock altogether, which makes one wonder about the testing coverage. It also embeds a `return' statement deep inside a huge and complex function, which is invariably bad. And in so doing it bypasses the register_irq_proc() and register_handler_proc() calls. I have no way of knowing whether that was deliberate or whether it was a bug. If it was deliberate then some code and'/or changelog commentary is needed, so that others don't think that it is a bug too. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH] irq: handle private interrupt registration 2010-06-01 22:26 ` Andrew Morton @ 2010-06-01 23:14 ` Thomas Gleixner -1 siblings, 0 replies; 10+ messages in thread From: Thomas Gleixner @ 2010-06-01 23:14 UTC (permalink / raw) To: linux-arm-kernel On Tue, 1 Jun 2010, Andrew Morton wrote: > On Wed, 26 May 2010 13:29:54 -0700 > adharmap at codeaurora.org wrote: > > > From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > > > The current code fails to register a handler for the same irq > > without taking in to account that it could be a per cpu interrupt. > > If the IRQF_PERCPU flag is set, enable the interrupt on that cpu > > and return success. > > > > Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 > > Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > --- > > > > On systems with an interrupt controller that supports > > private interrupts per core, it is not possible to call > > request_irq/setup_irq from multiple cores for the same irq. This is because > > the second+ invocation of __setup_irq checks if the previous > > hndler had a IRQ_SHARED flag set and errors out if not. > > > > The current irq handling code doesnt take in to account what cpu it > > is executing on. Usually the local interrupt controller registers are banked > > per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked > > registers. > > > > One way to get around this problem is to call the setup_irq on a single cpu > > while other cpus simply enable their private interrupts by writing to their > > banked registers > > > > For eg. code in arch/arm/time/smp_twd.c > > /* Make sure our local interrupt controller has this enabled */ > > local_irq_save(flags); > > get_irq_chip(clk->irq)->unmask(clk->irq); > > local_irq_restore(flags); > > > > This looks like a hacky way to get local interrupts working on > > multiple cores. Yes, it is. But it's saner than your aproach to trick the setup_irq() to handle that case. There are two sane solutions: 1) Use PER_CPU offsets for the irq numbers. The generic irq code does not care whether the interrupt number is matching any physical numbering scheme in the hardware, as long as the arch specific chip implementation knows how to deal with it, which is not rocket science to do. 2) Let the boot CPU setup the interrupt and provide a generic enable_per_cpu_irq() / disable_per_cpu_irq() interface, which has sanity checking in place. That has a couple of interesting implications as well, but they can be dealt with. Thanks, tglx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] irq: handle private interrupt registration @ 2010-06-01 23:14 ` Thomas Gleixner 0 siblings, 0 replies; 10+ messages in thread From: Thomas Gleixner @ 2010-06-01 23:14 UTC (permalink / raw) To: Andrew Morton Cc: adharmap, Mark Brown, Dmitry Torokhov, Trilok Soni, Pavel Machek, Brian Swetland, Joonyoung Shim, m.szyprowski, t.fujak, kyungmin.park, David Brownell, Daniel Ribeiro, arve, Barry Song, Russell King, Bryan Huntsman, Iliyan Malchev, Michael Buesch, Bruno Premont, linux-arm-kernel, linux-kernel On Tue, 1 Jun 2010, Andrew Morton wrote: > On Wed, 26 May 2010 13:29:54 -0700 > adharmap@codeaurora.org wrote: > > > From: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > > > The current code fails to register a handler for the same irq > > without taking in to account that it could be a per cpu interrupt. > > If the IRQF_PERCPU flag is set, enable the interrupt on that cpu > > and return success. > > > > Change-Id: I748b3aa08d794342ad74cbd0bb900cc599f883a6 > > Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org> > > --- > > > > On systems with an interrupt controller that supports > > private interrupts per core, it is not possible to call > > request_irq/setup_irq from multiple cores for the same irq. This is because > > the second+ invocation of __setup_irq checks if the previous > > hndler had a IRQ_SHARED flag set and errors out if not. > > > > The current irq handling code doesnt take in to account what cpu it > > is executing on. Usually the local interrupt controller registers are banked > > per cpu a.k.a. a cpu can enable its local interrupt by writing to its banked > > registers. > > > > One way to get around this problem is to call the setup_irq on a single cpu > > while other cpus simply enable their private interrupts by writing to their > > banked registers > > > > For eg. code in arch/arm/time/smp_twd.c > > /* Make sure our local interrupt controller has this enabled */ > > local_irq_save(flags); > > get_irq_chip(clk->irq)->unmask(clk->irq); > > local_irq_restore(flags); > > > > This looks like a hacky way to get local interrupts working on > > multiple cores. Yes, it is. But it's saner than your aproach to trick the setup_irq() to handle that case. There are two sane solutions: 1) Use PER_CPU offsets for the irq numbers. The generic irq code does not care whether the interrupt number is matching any physical numbering scheme in the hardware, as long as the arch specific chip implementation knows how to deal with it, which is not rocket science to do. 2) Let the boot CPU setup the interrupt and provide a generic enable_per_cpu_irq() / disable_per_cpu_irq() interface, which has sanity checking in place. That has a couple of interesting implications as well, but they can be dealt with. Thanks, tglx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH] irq: handle private interrupt registration 2010-06-01 23:14 ` Thomas Gleixner @ 2010-06-02 5:41 ` Felipe Balbi -1 siblings, 0 replies; 10+ messages in thread From: Felipe Balbi @ 2010-06-02 5:41 UTC (permalink / raw) To: linux-arm-kernel Hi, On Wed, Jun 02, 2010 at 01:14:32AM +0200, ext Thomas Gleixner wrote: >1) Use PER_CPU offsets for the irq numbers. The generic irq code does > not care whether the interrupt number is matching any physical > numbering scheme in the hardware, as long as the arch specific chip > implementation knows how to deal with it, which is not rocket > science to do. FWIW, I think (1) is a better approach as the problem will vanish altogether and to me it sounds like the simpler approach as well. Archs which have more than one IRQ chip (like OMAP with the twl4030 family) will already use sequencial numbering anyway, so using the same approach for N cpus, to me, sounds like a good deal. While at that, a question from my side: do we have a generic way of fetching the last IRQ number so we can easily use that to calculate the physical number of the IRQ line on the chip ? On OMAP, we have been passing that number down to twl4030 via platform_data, but it would be better to ask genirq to tell us which was the last irq number "claimed". -- balbi DefectiveByDesign.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] irq: handle private interrupt registration @ 2010-06-02 5:41 ` Felipe Balbi 0 siblings, 0 replies; 10+ messages in thread From: Felipe Balbi @ 2010-06-02 5:41 UTC (permalink / raw) To: ext Thomas Gleixner Cc: Andrew Morton, adharmap@codeaurora.org, Mark Brown, Dmitry Torokhov, Trilok Soni, Pavel Machek, Brian Swetland, Joonyoung Shim, m.szyprowski@samsung.com, t.fujak@samsung.com, kyungmin.park@samsung.com, David Brownell, Daniel Ribeiro, arve@android.com, Barry Song, Russell King, Bryan Huntsman, Iliyan Malchev, Michael Buesch, Bruno Premont, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi, On Wed, Jun 02, 2010 at 01:14:32AM +0200, ext Thomas Gleixner wrote: >1) Use PER_CPU offsets for the irq numbers. The generic irq code does > not care whether the interrupt number is matching any physical > numbering scheme in the hardware, as long as the arch specific chip > implementation knows how to deal with it, which is not rocket > science to do. FWIW, I think (1) is a better approach as the problem will vanish altogether and to me it sounds like the simpler approach as well. Archs which have more than one IRQ chip (like OMAP with the twl4030 family) will already use sequencial numbering anyway, so using the same approach for N cpus, to me, sounds like a good deal. While at that, a question from my side: do we have a generic way of fetching the last IRQ number so we can easily use that to calculate the physical number of the IRQ line on the chip ? On OMAP, we have been passing that number down to twl4030 via platform_data, but it would be better to ask genirq to tell us which was the last irq number "claimed". -- balbi DefectiveByDesign.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH] irq: handle private interrupt registration 2010-06-02 5:41 ` Felipe Balbi @ 2010-06-02 13:06 ` Thomas Gleixner -1 siblings, 0 replies; 10+ messages in thread From: Thomas Gleixner @ 2010-06-02 13:06 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2 Jun 2010, Felipe Balbi wrote: > Hi, > > On Wed, Jun 02, 2010 at 01:14:32AM +0200, ext Thomas Gleixner wrote: > > 1) Use PER_CPU offsets for the irq numbers. The generic irq code does > > not care whether the interrupt number is matching any physical > > numbering scheme in the hardware, as long as the arch specific chip > > implementation knows how to deal with it, which is not rocket > > science to do. > > FWIW, I think (1) is a better approach as the problem will vanish altogether > and to me it sounds like the simpler approach as well. Archs which have more > than one IRQ chip (like OMAP with the twl4030 family) will already use > sequencial numbering anyway, so using the same approach for N cpus, to me, > sounds like a good deal. > > While at that, a question from my side: do we have a generic way of fetching > the last IRQ number so we can easily use that to calculate the physical number > of the IRQ line on the chip ? > > On OMAP, we have been passing that number down to twl4030 via platform_data, > but it would be better to ask genirq to tell us which was the last irq number > "claimed". No, we don't, but it might be a good idea to move the virq management from powerpc into generic code so other archs can use it as well. Needs some thought. Thanks tglx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] irq: handle private interrupt registration @ 2010-06-02 13:06 ` Thomas Gleixner 0 siblings, 0 replies; 10+ messages in thread From: Thomas Gleixner @ 2010-06-02 13:06 UTC (permalink / raw) To: Felipe Balbi Cc: Andrew Morton, adharmap@codeaurora.org, Mark Brown, Dmitry Torokhov, Trilok Soni, Pavel Machek, Brian Swetland, Joonyoung Shim, m.szyprowski@samsung.com, t.fujak@samsung.com, kyungmin.park@samsung.com, David Brownell, Daniel Ribeiro, arve@android.com, Barry Song, Russell King, Bryan Huntsman, Iliyan Malchev, Michael Buesch, Bruno Premont, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Wed, 2 Jun 2010, Felipe Balbi wrote: > Hi, > > On Wed, Jun 02, 2010 at 01:14:32AM +0200, ext Thomas Gleixner wrote: > > 1) Use PER_CPU offsets for the irq numbers. The generic irq code does > > not care whether the interrupt number is matching any physical > > numbering scheme in the hardware, as long as the arch specific chip > > implementation knows how to deal with it, which is not rocket > > science to do. > > FWIW, I think (1) is a better approach as the problem will vanish altogether > and to me it sounds like the simpler approach as well. Archs which have more > than one IRQ chip (like OMAP with the twl4030 family) will already use > sequencial numbering anyway, so using the same approach for N cpus, to me, > sounds like a good deal. > > While at that, a question from my side: do we have a generic way of fetching > the last IRQ number so we can easily use that to calculate the physical number > of the IRQ line on the chip ? > > On OMAP, we have been passing that number down to twl4030 via platform_data, > but it would be better to ask genirq to tell us which was the last irq number > "claimed". No, we don't, but it might be a good idea to move the virq management from powerpc into generic code so other archs can use it as well. Needs some thought. Thanks tglx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-06-02 13:08 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-26 20:29 [RFC PATCH] irq: handle private interrupt registration adharmap at codeaurora.org 2010-05-26 20:29 ` adharmap 2010-06-01 22:26 ` Andrew Morton 2010-06-01 22:26 ` Andrew Morton 2010-06-01 23:14 ` Thomas Gleixner 2010-06-01 23:14 ` Thomas Gleixner 2010-06-02 5:41 ` Felipe Balbi 2010-06-02 5:41 ` Felipe Balbi 2010-06-02 13:06 ` Thomas Gleixner 2010-06-02 13:06 ` Thomas Gleixner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.