When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
not take into account the new virq offset, the interrupt is no longer
translated, leading to an unhandled interrupt.
Fix this by taking into account the virq offset when translating
cascaded IRL interrupts.
Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
Reported-by: Guenter Roeck <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Highlander and Dreamcast probably have the same issue.
I'll send patches for these later...
---
arch/sh/boards/mach-r2d/irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
index e34f81e9ae813b8d..c37b40398c5bc83e 100644
--- a/arch/sh/boards/mach-r2d/irq.c
+++ b/arch/sh/boards/mach-r2d/irq.c
@@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
int rts7751r2d_irq_demux(int irq)
{
- if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
+ if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
return irq;
- return irl2irq[irq];
+ return irl2irq[irq - 16];
}
/*
--
2.34.1
Hi Geert!
On Sun, 2023-07-09 at 13:15 +0200, Geert Uytterhoeven wrote:
> When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> not take into account the new virq offset, the interrupt is no longer
> translated, leading to an unhandled interrupt.
>
> Fix this by taking into account the virq offset when translating
> cascaded IRL interrupts.
>
> Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Highlander and Dreamcast probably have the same issue.
> I'll send patches for these later...
> ---
> arch/sh/boards/mach-r2d/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
Funny, this is actually almost what I did myself when trying to fix this
issue. Only difference was that I applied the offset of 16 only to one
of the instances at a time and it never occurred to me that it needs to
be applied to all instances.
Thanks for fixing this!
Reviewed-by: John Paul Adrian Glaubitz <[email protected]>
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Geert!
On Sun, 2023-07-09 at 13:15 +0200, Geert Uytterhoeven wrote:
> When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> not take into account the new virq offset, the interrupt is no longer
> translated, leading to an unhandled interrupt.
>
> Fix this by taking into account the virq offset when translating
> cascaded IRL interrupts.
>
> Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Highlander and Dreamcast probably have the same issue.
> I'll send patches for these later...
> ---
> arch/sh/boards/mach-r2d/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
I can also confirm that this fixes the hang on QEMU.
Tested-by: John Paul Adrian Glaubitz <[email protected]>
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On 7/9/23 2:58 PM, John Paul Adrian Glaubitz wrote:
[...]
>> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
>> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
>> --- a/arch/sh/boards/mach-r2d/irq.c
>> +++ b/arch/sh/boards/mach-r2d/irq.c
>> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>>
>> int rts7751r2d_irq_demux(int irq)
>> {
>> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
>> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
>> return irq;
>>
>> - return irl2irq[irq];
>> + return irl2irq[irq - 16];
>> }
>>
>> /*
>
> Btw, I think this needs to be adjusted to test for "ret <= 0" since IRQs cannot
> be zero anymore, correct?
>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/sm501.c#n1389
No, just ignore IRQ0 from now on, it can't be returned. Else you'd just complicate
your code as you'd have to add a separate check for IRQ0 in order to return -EINVAL
in this case (you can't return 0 from probe in case of ret == 0 as that would mean
successful probe when it's not). My patch to platfrom_get_irq() ensures that IRQ0
check in its users is never needed, in order to avoid the (badly scaling) checks)...
> Adrian
MBR, Sergey
Hi!
On Sun, 2023-07-09 at 13:15 +0200, Geert Uytterhoeven wrote:
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
Btw, I think this needs to be adjusted to test for "ret <= 0" since IRQs cannot
be zero anymore, correct?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/sm501.c#n1389
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Sun, Jul 09, 2023 at 01:15:49PM +0200, Geert Uytterhoeven wrote:
> When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> not take into account the new virq offset, the interrupt is no longer
> translated, leading to an unhandled interrupt.
>
> Fix this by taking into account the virq offset when translating
> cascaded IRL interrupts.
>
> Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Tested-by: Guenter Roeck <[email protected]>
Guenter
> ---
> Highlander and Dreamcast probably have the same issue.
> I'll send patches for these later...
> ---
> arch/sh/boards/mach-r2d/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
> --
> 2.34.1
>
On 7/9/23 04:15, Geert Uytterhoeven wrote:
> When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> not take into account the new virq offset, the interrupt is no longer
> translated, leading to an unhandled interrupt.
>
> Fix this by taking into account the virq offset when translating
> cascaded IRL interrupts.
>
> Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Highlander and Dreamcast probably have the same issue.
> I'll send patches for these later...
dreamcast doesn't build in linux-next, just in case you didn't notice.
Guenter
> ---
> arch/sh/boards/mach-r2d/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
Hi Günter,
On Mon, Jul 10, 2023 at 3:13 AM Guenter Roeck <[email protected]> wrote:
> On 7/9/23 04:15, Geert Uytterhoeven wrote:
> > When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> > an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> > interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> > Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> > not take into account the new virq offset, the interrupt is no longer
> > translated, leading to an unhandled interrupt.
> >
> > Fix this by taking into account the virq offset when translating
> > cascaded IRL interrupts.
> >
> > Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> > Reported-by: Guenter Roeck <[email protected]>
> > Closes: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > Highlander and Dreamcast probably have the same issue.
> > I'll send patches for these later...
>
> dreamcast doesn't build in linux-next, just in case you didn't notice.
Indeed, I hadn't tested that.
My current tree isn't based on linux-next, but did have a build
failure in the cdrom code, for which I had found your fix (thanks!) in
linux-next...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Guenter!
On Sun, 2023-07-09 at 18:13 -0700, Guenter Roeck wrote:
> dreamcast doesn't build in linux-next, just in case you didn't notice.
Didn't the person who introduced the regression a notification from the CI?
Maybe we could configure the CI to send a mail to the linux-sh mailing list
in case of such failures, so that the people involved in the SH port are
being notified.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Geert!
On Mon, 2023-07-10 at 09:03 +0200, Geert Uytterhoeven wrote:
> Indeed, I hadn't tested that.
> My current tree isn't based on linux-next, but did have a build
> failure in the cdrom code, for which I had found your fix (thanks!) in
> linux-next...
So, there is a patch for this already? Is it going to be included for 6.5?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Geert!
On Mon, 2023-07-10 at 09:54 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
>
> On Mon, Jul 10, 2023 at 9:44 AM John Paul Adrian Glaubitz
> <[email protected]> wrote:
> > On Mon, 2023-07-10 at 09:03 +0200, Geert Uytterhoeven wrote:
> > > Indeed, I hadn't tested that.
> > > My current tree isn't based on linux-next, but did have a build
> > > failure in the cdrom code, for which I had found your fix (thanks!) in
> > > linux-next...
> >
> > So, there is a patch for this already? Is it going to be included for 6.5?
>
> The cdrom fix is commit a587b046ce921cc1 ("cdrom/gdrom: Fix build
> error") in v6.5-rc1, which builds dreamcast_defconfig fine.
>
> That config is still broken in linux-next, but the breakage hasn't\
> entered v6.5-rc1 (yet?).
OK, so we're talking about two different regressions here?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Adrian,
On Mon, Jul 10, 2023 at 9:44 AM John Paul Adrian Glaubitz
<[email protected]> wrote:
> On Mon, 2023-07-10 at 09:03 +0200, Geert Uytterhoeven wrote:
> > Indeed, I hadn't tested that.
> > My current tree isn't based on linux-next, but did have a build
> > failure in the cdrom code, for which I had found your fix (thanks!) in
> > linux-next...
>
> So, there is a patch for this already? Is it going to be included for 6.5?
The cdrom fix is commit a587b046ce921cc1 ("cdrom/gdrom: Fix build
error") in v6.5-rc1, which builds dreamcast_defconfig fine.
That config is still broken in linux-next, but the breakage hasn't\
entered v6.5-rc1 (yet?).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, Jul 10, 2023 at 9:57 AM John Paul Adrian Glaubitz
<[email protected]> wrote:
> On Mon, 2023-07-10 at 09:54 +0200, Geert Uytterhoeven wrote:
> > On Mon, Jul 10, 2023 at 9:44 AM John Paul Adrian Glaubitz
> > <[email protected]> wrote:
> > > On Mon, 2023-07-10 at 09:03 +0200, Geert Uytterhoeven wrote:
> > > > Indeed, I hadn't tested that.
> > > > My current tree isn't based on linux-next, but did have a build
> > > > failure in the cdrom code, for which I had found your fix (thanks!) in
> > > > linux-next...
> > >
> > > So, there is a patch for this already? Is it going to be included for 6.5?
> >
> > The cdrom fix is commit a587b046ce921cc1 ("cdrom/gdrom: Fix build
> > error") in v6.5-rc1, which builds dreamcast_defconfig fine.
> >
> > That config is still broken in linux-next, but the breakage hasn't\
> > entered v6.5-rc1 (yet?).
>
> OK, so we're talking about two different regressions here?
Yes.
https://lore.kernel.org/r/CAMuHMdWmv-Jdvi7a04JGXuA2QARj8c8mpUvY7TOcetPkG4pW7A@mail.gmail.com
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Adrian,
On Mon, Jul 10, 2023 at 10:20 AM John Paul Adrian Glaubitz
<[email protected]> wrote:
> On Mon, 2023-07-10 at 10:18 +0200, Geert Uytterhoeven wrote:
> > > OK, so we're talking about two different regressions here?
> > https://lore.kernel.org/r/CAMuHMdWmv-Jdvi7a04JGXuA2QARj8c8mpUvY7TOcetPkG4pW7A@mail.gmail.com
>
> Link doesn't work for me, unfortunately.
-EAGAIN ;-)
Lore-archiving is not instantaneous. Please retry.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert!
On Mon, 2023-07-10 at 10:18 +0200, Geert Uytterhoeven wrote:
> > OK, so we're talking about two different regressions here?
>
> Yes.
>
> https://lore.kernel.org/r/CAMuHMdWmv-Jdvi7a04JGXuA2QARj8c8mpUvY7TOcetPkG4pW7A@mail.gmail.com
Link doesn't work for me, unfortunately.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Sun, 2023-07-09 at 13:15 +0200, Geert Uytterhoeven wrote:
> When booting rts7751r2dplus_defconfig on QEMU, the system hangs due to
> an interrupt storm on IRQ 20. IRQ 20 aka event 0x280 is a cascaded IRL
> interrupt, which maps to IRQ_VOYAGER, the interrupt used by the Silicon
> Motion SM501 multimedia companion chip. As rts7751r2d_irq_demux() does
> not take into account the new virq offset, the interrupt is no longer
> translated, leading to an unhandled interrupt.
>
> Fix this by taking into account the virq offset when translating
> cascaded IRL interrupts.
>
> Fixes: a8ac2961148e8c72 ("sh: Avoid using IRQ0 on SH3 and SH4")
> Reported-by: Guenter Roeck <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Highlander and Dreamcast probably have the same issue.
> I'll send patches for these later...
> ---
> arch/sh/boards/mach-r2d/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sh/boards/mach-r2d/irq.c b/arch/sh/boards/mach-r2d/irq.c
> index e34f81e9ae813b8d..c37b40398c5bc83e 100644
> --- a/arch/sh/boards/mach-r2d/irq.c
> +++ b/arch/sh/boards/mach-r2d/irq.c
> @@ -117,10 +117,10 @@ static unsigned char irl2irq[R2D_NR_IRL];
>
> int rts7751r2d_irq_demux(int irq)
> {
> - if (irq >= R2D_NR_IRL || irq < 0 || !irl2irq[irq])
> + if (irq >= 16 + R2D_NR_IRL || irq < 16 || !irl2irq[irq - 16])
> return irq;
>
> - return irl2irq[irq];
> + return irl2irq[irq - 16];
> }
>
> /*
Applied to my for-linus branch.
Thanks,
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913