* [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
@ 2021-11-12 14:36 Marc Zyngier
2021-11-12 14:57 ` Rob Herring
2021-11-12 15:04 ` Bjorn Helgaas
0 siblings, 2 replies; 8+ messages in thread
From: Marc Zyngier @ 2021-11-12 14:36 UTC (permalink / raw)
To: linux-kernel, devicetree; +Cc: Christian Zigotzky, Rob Herring, Bjorn Helgaas
Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
to an interrupt controller"), the irq code favors using an interrupt-map
over a interrupt-controller property if both are available, while the
earlier behaviour was to ignore the interrupt-map altogether.
However, we now end-up with the opposite behaviour, which is to
ignore the interrupt-controller property even if the interrupt-map
fails to match its input. This new behaviour breaks the AmigaOne
X1000 machine, which ships with an extremely "creative" (read:
broken) device tree.
Fix this by allowing the interrupt-controller property to be selected
when interrupt-map fails to match anything.
Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
Cc: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/of/irq.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 32be5a03951f..508fb1717de3 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -161,9 +161,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
* if it is then we are done, unless there is an
* interrupt-map which takes precedence.
*/
+ bool intc = of_property_read_bool(ipar, "interrupt-controller");
+
imap = of_get_property(ipar, "interrupt-map", &imaplen);
- if (imap == NULL &&
- of_property_read_bool(ipar, "interrupt-controller")) {
+ if (imap == NULL && intc) {
pr_debug(" -> got it !\n");
return 0;
}
@@ -244,8 +245,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
pr_debug(" -> imaplen=%d\n", imaplen);
}
- if (!match)
+ if (!match) {
+ if (intc) {
+ pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
+ return 0;
+ }
+
goto fail;
+ }
/*
* Successfully parsed an interrrupt-map translation; copy new
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 14:36 [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed Marc Zyngier
@ 2021-11-12 14:57 ` Rob Herring
2021-11-12 15:26 ` Marc Zyngier
2021-11-12 15:04 ` Bjorn Helgaas
1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2021-11-12 14:57 UTC (permalink / raw)
To: Marc Zyngier; +Cc: linux-kernel, devicetree, Christian Zigotzky, Bjorn Helgaas
On Fri, Nov 12, 2021 at 8:37 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
> to an interrupt controller"), the irq code favors using an interrupt-map
> over a interrupt-controller property if both are available, while the
> earlier behaviour was to ignore the interrupt-map altogether.
>
> However, we now end-up with the opposite behaviour, which is to
> ignore the interrupt-controller property even if the interrupt-map
> fails to match its input. This new behaviour breaks the AmigaOne
> X1000 machine, which ships with an extremely "creative" (read:
> broken) device tree.
>
> Fix this by allowing the interrupt-controller property to be selected
> when interrupt-map fails to match anything.
>
> Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
> Cc: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/of/irq.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 32be5a03951f..508fb1717de3 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -161,9 +161,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> * if it is then we are done, unless there is an
> * interrupt-map which takes precedence.
> */
> + bool intc = of_property_read_bool(ipar, "interrupt-controller");
> +
> imap = of_get_property(ipar, "interrupt-map", &imaplen);
> - if (imap == NULL &&
> - of_property_read_bool(ipar, "interrupt-controller")) {
> + if (imap == NULL && intc) {
> pr_debug(" -> got it !\n");
> return 0;
> }
> @@ -244,8 +245,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
>
> pr_debug(" -> imaplen=%d\n", imaplen);
> }
> - if (!match)
> + if (!match) {
> + if (intc) {
> + pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
Let's make this a WARN for !IS_ENABLED(CONFIG_PASEMI). (whatever the
right kconfig symbol is).
> + return 0;
> + }
> +
> goto fail;
> + }
>
> /*
> * Successfully parsed an interrrupt-map translation; copy new
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 14:36 [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed Marc Zyngier
2021-11-12 14:57 ` Rob Herring
@ 2021-11-12 15:04 ` Bjorn Helgaas
2021-11-12 15:28 ` Marc Zyngier
2021-11-12 15:43 ` Christian Zigotzky
1 sibling, 2 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2021-11-12 15:04 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, devicetree, Christian Zigotzky, Rob Herring, Bjorn Helgaas
On Fri, Nov 12, 2021 at 02:36:44PM +0000, Marc Zyngier wrote:
> Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
> to an interrupt controller"), the irq code favors using an interrupt-map
> over a interrupt-controller property if both are available, while the
> earlier behaviour was to ignore the interrupt-map altogether.
>
> However, we now end-up with the opposite behaviour, which is to
> ignore the interrupt-controller property even if the interrupt-map
> fails to match its input. This new behaviour breaks the AmigaOne
> X1000 machine, which ships with an extremely "creative" (read:
> broken) device tree.
>
> Fix this by allowing the interrupt-controller property to be selected
> when interrupt-map fails to match anything.
>
> Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
> Cc: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
I'm not qualified to review this, but since 041284181226 was merged
via my tree along with the rest of the Apple stuff, let me know if
you'd like me to merge this.
I see Rob has a comment, so if you want to take care merging it
yourself, that's certainly fine with me.
> ---
> drivers/of/irq.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 32be5a03951f..508fb1717de3 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -161,9 +161,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> * if it is then we are done, unless there is an
> * interrupt-map which takes precedence.
> */
> + bool intc = of_property_read_bool(ipar, "interrupt-controller");
> +
> imap = of_get_property(ipar, "interrupt-map", &imaplen);
> - if (imap == NULL &&
> - of_property_read_bool(ipar, "interrupt-controller")) {
> + if (imap == NULL && intc) {
> pr_debug(" -> got it !\n");
> return 0;
> }
> @@ -244,8 +245,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
>
> pr_debug(" -> imaplen=%d\n", imaplen);
> }
> - if (!match)
> + if (!match) {
> + if (intc) {
> + pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
> + return 0;
> + }
> +
> goto fail;
> + }
>
> /*
> * Successfully parsed an interrrupt-map translation; copy new
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 14:57 ` Rob Herring
@ 2021-11-12 15:26 ` Marc Zyngier
0 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2021-11-12 15:26 UTC (permalink / raw)
To: Rob Herring; +Cc: linux-kernel, devicetree, Christian Zigotzky, Bjorn Helgaas
On Fri, 12 Nov 2021 14:57:13 +0000,
Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Nov 12, 2021 at 8:37 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > @@ -244,8 +245,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> >
> > pr_debug(" -> imaplen=%d\n", imaplen);
> > }
> > - if (!match)
> > + if (!match) {
> > + if (intc) {
> > + pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
>
> Let's make this a WARN for !IS_ENABLED(CONFIG_PASEMI). (whatever the
> right kconfig symbol is).
I've folded this in.
M.
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 508fb1717de3..b10f015b2e37 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -247,7 +247,13 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
}
if (!match) {
if (intc) {
- pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
+ /*
+ * The PASEMI Nemo is a known offender, so
+ * let's only warn for anyone else.
+ */
+ WARN(!IS_ENABLED(CONFIG_PPC_PASEMI),
+ "%pOF interrupt-map failed, using interrupt-controller\n",
+ ipar);
return 0;
}
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 15:04 ` Bjorn Helgaas
@ 2021-11-12 15:28 ` Marc Zyngier
2021-11-12 15:55 ` Rob Herring
2021-11-12 15:43 ` Christian Zigotzky
1 sibling, 1 reply; 8+ messages in thread
From: Marc Zyngier @ 2021-11-12 15:28 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-kernel, devicetree, Christian Zigotzky, Rob Herring, Bjorn Helgaas
On Fri, 12 Nov 2021 15:04:15 +0000,
Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Nov 12, 2021 at 02:36:44PM +0000, Marc Zyngier wrote:
> > Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
> > to an interrupt controller"), the irq code favors using an interrupt-map
> > over a interrupt-controller property if both are available, while the
> > earlier behaviour was to ignore the interrupt-map altogether.
> >
> > However, we now end-up with the opposite behaviour, which is to
> > ignore the interrupt-controller property even if the interrupt-map
> > fails to match its input. This new behaviour breaks the AmigaOne
> > X1000 machine, which ships with an extremely "creative" (read:
> > broken) device tree.
> >
> > Fix this by allowing the interrupt-controller property to be selected
> > when interrupt-map fails to match anything.
> >
> > Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
> > Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
>
> I'm not qualified to review this, but since 041284181226 was merged
> via my tree along with the rest of the Apple stuff, let me know if
> you'd like me to merge this.
>
> I see Rob has a comment, so if you want to take care merging it
> yourself, that's certainly fine with me.
I have a couple of IRQ patches that need to go in, so happy to route
it via the irqchip tree if Rob gives his blessing.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 15:04 ` Bjorn Helgaas
2021-11-12 15:28 ` Marc Zyngier
@ 2021-11-12 15:43 ` Christian Zigotzky
2021-11-12 16:01 ` Marc Zyngier
1 sibling, 1 reply; 8+ messages in thread
From: Christian Zigotzky @ 2021-11-12 15:43 UTC (permalink / raw)
To: Bjorn Helgaas, Marc Zyngier
Cc: linux-kernel, devicetree, Rob Herring, Bjorn Helgaas,
R.T.Dickinson, Matthew Leaman, Darren Stevens, mad skateman
On 12 November 2021 at 04:04 pm, Bjorn Helgaas wrote:
> On Fri, Nov 12, 2021 at 02:36:44PM +0000, Marc Zyngier wrote:
>> Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
>> to an interrupt controller"), the irq code favors using an interrupt-map
>> over a interrupt-controller property if both are available, while the
>> earlier behaviour was to ignore the interrupt-map altogether.
>>
>> However, we now end-up with the opposite behaviour, which is to
>> ignore the interrupt-controller property even if the interrupt-map
>> fails to match its input. This new behaviour breaks the AmigaOne
>> X1000 machine, which ships with an extremely "creative" (read:
>> broken) device tree.
>>
>> Fix this by allowing the interrupt-controller property to be selected
>> when interrupt-map fails to match anything.
>>
>> Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
>> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> I'm not qualified to review this, but since 041284181226 was merged
> via my tree along with the rest of the Apple stuff, let me know if
> you'd like me to merge this.
>
> I see Rob has a comment, so if you want to take care merging it
> yourself, that's certainly fine with me.
>
>> ---
>> drivers/of/irq.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>> index 32be5a03951f..508fb1717de3 100644
>> --- a/drivers/of/irq.c
>> +++ b/drivers/of/irq.c
>> @@ -161,9 +161,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
>> * if it is then we are done, unless there is an
>> * interrupt-map which takes precedence.
>> */
>> + bool intc = of_property_read_bool(ipar, "interrupt-controller");
>> +
>> imap = of_get_property(ipar, "interrupt-map", &imaplen);
>> - if (imap == NULL &&
>> - of_property_read_bool(ipar, "interrupt-controller")) {
>> + if (imap == NULL && intc) {
>> pr_debug(" -> got it !\n");
>> return 0;
>> }
>> @@ -244,8 +245,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
>>
>> pr_debug(" -> imaplen=%d\n", imaplen);
>> }
>> - if (!match)
>> + if (!match) {
>> + if (intc) {
>> + pr_debug("%pOF interrupt-map failed, using interrupt-controller\n", ipar);
>> + return 0;
>> + }
>> +
>> goto fail;
>> + }
>>
>> /*
>> * Successfully parsed an interrrupt-map translation; copy new
>> --
>> 2.30.2
>>
Could you please merge it asap for the RC1? Then we can compile the RC1
without these patches.
Thanks to all for your help and sorry for bother you with all these
mails. I do this work alongside my main job and actually I don't have
time for bisecting.
-- Christian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 15:28 ` Marc Zyngier
@ 2021-11-12 15:55 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2021-11-12 15:55 UTC (permalink / raw)
To: Marc Zyngier
Cc: Bjorn Helgaas, linux-kernel, devicetree, Christian Zigotzky,
Bjorn Helgaas
On Fri, Nov 12, 2021 at 9:28 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Fri, 12 Nov 2021 15:04:15 +0000,
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Fri, Nov 12, 2021 at 02:36:44PM +0000, Marc Zyngier wrote:
> > > Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
> > > to an interrupt controller"), the irq code favors using an interrupt-map
> > > over a interrupt-controller property if both are available, while the
> > > earlier behaviour was to ignore the interrupt-map altogether.
> > >
> > > However, we now end-up with the opposite behaviour, which is to
> > > ignore the interrupt-controller property even if the interrupt-map
> > > fails to match its input. This new behaviour breaks the AmigaOne
> > > X1000 machine, which ships with an extremely "creative" (read:
> > > broken) device tree.
> > >
> > > Fix this by allowing the interrupt-controller property to be selected
> > > when interrupt-map fails to match anything.
> > >
> > > Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
> > > Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > Link: https://lore.kernel.org/r/78308692-02e6-9544-4035-3171a8e1e6d4@xenosoft.de
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> >
> > I'm not qualified to review this, but since 041284181226 was merged
> > via my tree along with the rest of the Apple stuff, let me know if
> > you'd like me to merge this.
> >
> > I see Rob has a comment, so if you want to take care merging it
> > yourself, that's certainly fine with me.
>
> I have a couple of IRQ patches that need to go in, so happy to route
> it via the irqchip tree if Rob gives his blessing.
I have stuff for rc1 too, but feel free to take it:
With the WARN added,
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed
2021-11-12 15:43 ` Christian Zigotzky
@ 2021-11-12 16:01 ` Marc Zyngier
0 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2021-11-12 16:01 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Bjorn Helgaas, linux-kernel, devicetree, Rob Herring,
Bjorn Helgaas, R.T.Dickinson, Matthew Leaman, Darren Stevens,
mad skateman
On Fri, 12 Nov 2021 15:43:12 +0000,
Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>
> Could you please merge it asap for the RC1? Then we can compile the
> RC1 without these patches.
I don't think there is any rush. Plenty of things are broken at this
stage, and that's completely expected after merging a few thousand
patches. It is not like we are at -rc7 with a critical issue to
address right before release.
The problem is understood, the fix identified, and we have a patch
under review. This really is as good as it gets.
> Thanks to all for your help and sorry for bother you with all these
> mails. I do this work alongside my main job and actually I don't have
> time for bisecting.
And we're grateful for your help in identifying the problem.
"Don't Panic!".
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-12 16:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 14:36 [PATCH] of/irq: Don't ignore interrupt-controller when interrupt-map failed Marc Zyngier
2021-11-12 14:57 ` Rob Herring
2021-11-12 15:26 ` Marc Zyngier
2021-11-12 15:04 ` Bjorn Helgaas
2021-11-12 15:28 ` Marc Zyngier
2021-11-12 15:55 ` Rob Herring
2021-11-12 15:43 ` Christian Zigotzky
2021-11-12 16:01 ` Marc Zyngier
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).