* [PATCH] irqdomain: print a warning if domains contain IRQ 0 @ 2012-04-18 13:40 Linus Walleij 2012-04-18 22:23 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Linus Walleij @ 2012-04-18 13:40 UTC (permalink / raw) To: Benjamin Herrenschmidt, Grant Likely; +Cc: linux-kernel, Linus Walleij From: Linus Walleij <linus.walleij@linaro.org> Some of the clients using IRQ domains from the ARM VIC (arch/arm/common/vic.c) don't know that their (hardware) IRQ 0 is silently ignored by the IRQ core, they will just notice that they're not getting this IRQ anymore. So print a warning if a domain contains IRQ 0 (NO_IRQ) so we get some noise about it atleast. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- kernel/irq/irqdomain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 0e0ba5f..1444454 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -136,8 +136,10 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node, int hwirq = first_hwirq + i; /* IRQ0 gets ignored */ - if (!irq) + if (!irq) { + pr_warn("trying to register IRQ 0 (NO_IRQ) in an irq domain\n"); continue; + } /* Legacy flags are left to default at this point, * one can then use irq_create_mapping() to -- 1.7.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] irqdomain: print a warning if domains contain IRQ 0 2012-04-18 13:40 [PATCH] irqdomain: print a warning if domains contain IRQ 0 Linus Walleij @ 2012-04-18 22:23 ` Benjamin Herrenschmidt 2012-04-18 22:32 ` Linus Walleij 2012-04-19 18:29 ` Grant Likely 0 siblings, 2 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-04-18 22:23 UTC (permalink / raw) To: Linus Walleij; +Cc: Grant Likely, linux-kernel, Linus Walleij On Wed, 2012-04-18 at 15:40 +0200, Linus Walleij wrote: > From: Linus Walleij <linus.walleij@linaro.org> > > Some of the clients using IRQ domains from the ARM VIC > (arch/arm/common/vic.c) don't know that their (hardware) IRQ 0 > is silently ignored by the IRQ core, they will just notice > that they're not getting this IRQ anymore. So print a warning > if a domain contains IRQ 0 (NO_IRQ) so we get some noise about > it atleast. I don't understand. HW IRQ 0 is not ignored and works perfectly fine on "normal" remapped domains. Or is this specific to "legacy domains" ? In this case pls make it clear in the subject :-) Cheers, Ben. > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > kernel/irq/irqdomain.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 0e0ba5f..1444454 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -136,8 +136,10 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node, > int hwirq = first_hwirq + i; > > /* IRQ0 gets ignored */ > - if (!irq) > + if (!irq) { > + pr_warn("trying to register IRQ 0 (NO_IRQ) in an irq domain\n"); > continue; > + } > > /* Legacy flags are left to default at this point, > * one can then use irq_create_mapping() to ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqdomain: print a warning if domains contain IRQ 0 2012-04-18 22:23 ` Benjamin Herrenschmidt @ 2012-04-18 22:32 ` Linus Walleij 2012-04-19 18:29 ` Grant Likely 1 sibling, 0 replies; 6+ messages in thread From: Linus Walleij @ 2012-04-18 22:32 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linus Walleij, Grant Likely, linux-kernel, Russell King - ARM Linux On Thu, Apr 19, 2012 at 12:23 AM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2012-04-18 at 15:40 +0200, Linus Walleij wrote: >> From: Linus Walleij <linus.walleij@linaro.org> >> >> Some of the clients using IRQ domains from the ARM VIC >> (arch/arm/common/vic.c) don't know that their (hardware) IRQ 0 >> is silently ignored by the IRQ core, they will just notice >> that they're not getting this IRQ anymore. So print a warning >> if a domain contains IRQ 0 (NO_IRQ) so we get some noise about >> it atleast. > > I don't understand. HW IRQ 0 is not ignored and works perfectly > fine on "normal" remapped domains. Or is this specific to "legacy > domains" ? In this case pls make it clear in the subject :-) I'm trying ... but maybe not succeeding. The ARM platforms using the VIC and its domain translation have been converted to use domains without also renumbering their Linux IRQ numbers. So they still have, in their low IRQ range, still a 1-1 mapping between HW and Linux IRQ. Including IRQ 0. E.g (as recently posted a patch for): (arch/arm/mach-u300/include/mach/irqs.h) #define IRQ_U300_INTCON0_START 0 #define IRQ_U300_INTCON1_START 32 /* These are on INTCON0 - 30 lines */ #define IRQ_U300_IRQ0_EXT 0 #define IRQ_U300_IRQ1_EXT 1 #define IRQ_U300_DMA 2 The kernel is using IRQ_U300_IRQ0_EXT which of course stopped working now. Another example, mach-nomadik/include/mach/irqs.h: #define IRQ_VIC_START 0 /* first VIC interrupt is 0 */ /* * Interrupt numbers generic for all Nomadik Chip cuts */ #define IRQ_WATCHDOG 0 #define IRQ_SOFTINT 1 #define IRQ_CRYPTO 2 #define IRQ_OWM 3 #define IRQ_MTU0 4 I hope nobody tries to use the watchdog now... Another example: arch/arm/mach-versatile/include/mach/irqs.h #define IRQ_VIC_START 0 #define IRQ_WDOGINT (IRQ_VIC_START + INT_WDOGINT) #define IRQ_SOFTINT (IRQ_VIC_START + INT_SOFTINT) #define IRQ_COMMRx (IRQ_VIC_START + INT_COMMRx) #define IRQ_COMMTx (IRQ_VIC_START + INT_COMMTx) This INT_WDOGINT is incidentally also 0. So all of these are effectively broken now, and needs to have their IRQ offsets changed. I just sent a patch to U300 fixing this for that platform, but if I had this warning I would have noticed that something was wrong much earlier. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqdomain: print a warning if domains contain IRQ 0 2012-04-18 22:23 ` Benjamin Herrenschmidt 2012-04-18 22:32 ` Linus Walleij @ 2012-04-19 18:29 ` Grant Likely 2012-04-20 6:38 ` Linus Walleij 1 sibling, 1 reply; 6+ messages in thread From: Grant Likely @ 2012-04-19 18:29 UTC (permalink / raw) To: Benjamin Herrenschmidt, Linus Walleij; +Cc: linux-kernel, Linus Walleij On Thu, 19 Apr 2012 08:23:12 +1000, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2012-04-18 at 15:40 +0200, Linus Walleij wrote: > > From: Linus Walleij <linus.walleij@linaro.org> > > > > Some of the clients using IRQ domains from the ARM VIC > > (arch/arm/common/vic.c) don't know that their (hardware) IRQ 0 > > is silently ignored by the IRQ core, they will just notice > > that they're not getting this IRQ anymore. So print a warning > > if a domain contains IRQ 0 (NO_IRQ) so we get some noise about > > it atleast. > > I don't understand. HW IRQ 0 is not ignored and works perfectly > fine on "normal" remapped domains. Or is this specific to "legacy > domains" ? In this case pls make it clear in the subject :-) This is indeed specific to the legacy domain. I think the patch is good and it will help weed out unintended irq0 users. However, it requires the following additional fix I think. It will need to be tested to make sure it doesn't break PowerPC ISA users. g. diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index d0995bd..31f1f88 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -138,7 +138,7 @@ static inline struct irq_domain *irq_domain_add_legacy_isa( const struct irq_domain_ops *ops, void *host_data) { - return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops, + return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS-1, 1, 1, ops, host_data); } extern struct irq_domain *irq_find_host(struct device_node *node); > > --- > > kernel/irq/irqdomain.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > > index 0e0ba5f..1444454 100644 > > --- a/kernel/irq/irqdomain.c > > +++ b/kernel/irq/irqdomain.c > > @@ -136,8 +136,10 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node, > > int hwirq = first_hwirq + i; > > > > /* IRQ0 gets ignored */ > > - if (!irq) > > + if (!irq) { > > + pr_warn("trying to register IRQ 0 (NO_IRQ) in an irq domain\n"); > > continue; > > + } > > > > /* Legacy flags are left to default at this point, > > * one can then use irq_create_mapping() to > > -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd. ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] irqdomain: print a warning if domains contain IRQ 0 2012-04-19 18:29 ` Grant Likely @ 2012-04-20 6:38 ` Linus Walleij 2012-04-27 18:56 ` Grant Likely 0 siblings, 1 reply; 6+ messages in thread From: Linus Walleij @ 2012-04-20 6:38 UTC (permalink / raw) To: Grant Likely; +Cc: Benjamin Herrenschmidt, Linus Walleij, linux-kernel On Thu, Apr 19, 2012 at 8:29 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > This is indeed specific to the legacy domain. I think the patch is > good and it will help weed out unintended irq0 users. However, it > requires the following additional fix I think. It will need to be > tested to make sure it doesn't break PowerPC ISA users. > > g. > > diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > index d0995bd..31f1f88 100644 > --- a/include/linux/irqdomain.h > +++ b/include/linux/irqdomain.h > @@ -138,7 +138,7 @@ static inline struct irq_domain *irq_domain_add_legacy_isa( > const struct irq_domain_ops *ops, > void *host_data) > { > - return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops, > + return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS-1, 1, 1, ops, > host_data); Hm, so what does this do? If I fold it into my patch I need some kind of blurb... I'm guessing it bumps the ISA IRQs with one to avoid using IRQ0 which seems like a valid patch on its own, and that the old code was used for actively ignoring IRQ 0 on ISA (not used or whatever)? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqdomain: print a warning if domains contain IRQ 0 2012-04-20 6:38 ` Linus Walleij @ 2012-04-27 18:56 ` Grant Likely 0 siblings, 0 replies; 6+ messages in thread From: Grant Likely @ 2012-04-27 18:56 UTC (permalink / raw) To: Linus Walleij; +Cc: Benjamin Herrenschmidt, Linus Walleij, linux-kernel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 1742 bytes --] On Fri, 20 Apr 2012 08:38:05 +0200, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Apr 19, 2012 at 8:29 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > > > This is indeed specific to the legacy domain. I think the patch is > > good and it will help weed out unintended irq0 users. However, it > > requires the following additional fix I think. It will need to be > > tested to make sure it doesn't break PowerPC ISA users. > > > > g. > > > > diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > > index d0995bd..31f1f88 100644 > > --- a/include/linux/irqdomain.h > > +++ b/include/linux/irqdomain.h > > @@ -138,7 +138,7 @@ static inline struct irq_domain *irq_domain_add_legacy_isa( > > const struct irq_domain_ops *ops, > > void *host_data) > > { > > - return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops, > > + return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS-1, 1, 1, ops, > > host_data); > > Hm, so what does this do? If I fold it into my patch I need some > kind of blurb... > > I'm guessing it bumps the ISA IRQs with one to avoid using IRQ0 which > seems like a valid patch on its own, and that the old code was > used for actively ignoring IRQ 0 on ISA (not used or whatever)? Yes, that is exactly what it does. The fact that 0 is allowed but never used is an artifact of how this code used to be only for ISA and both irq and hwirq base numbers were hard coded to 0 for that. The reworked legacy domain allowed both irq base and hwirq base to be non-zero, but the check for 0 code remained. g. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-27 18:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-18 13:40 [PATCH] irqdomain: print a warning if domains contain IRQ 0 Linus Walleij 2012-04-18 22:23 ` Benjamin Herrenschmidt 2012-04-18 22:32 ` Linus Walleij 2012-04-19 18:29 ` Grant Likely 2012-04-20 6:38 ` Linus Walleij 2012-04-27 18:56 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).