* [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).