| Message ID | 2c99d5df41c40691f6c407b7b6a040d406bc81ac.1688901306.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFT] sh: mach-r2d: Handle virq offset in cascaded IRL demux | expand |
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 <linux@roeck-us.net> > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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 <glaubitz@physik.fu-berlin.de> Adrian
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 <linux@roeck-us.net> > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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 <glaubitz@physik.fu-berlin.de> Adrian
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
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
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 <linux@roeck-us.net> > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Tested-by: Guenter Roeck <linux@roeck-us.net> 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 <linux@roeck-us.net> > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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 <linux@roeck-us.net> 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 <linux@roeck-us.net> > > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > 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
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
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
Hi Adrian, On Mon, Jul 10, 2023 at 9:44 AM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> 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
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 > <glaubitz@physik.fu-berlin.de> 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
On Mon, Jul 10, 2023 at 9:57 AM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> 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 > > <glaubitz@physik.fu-berlin.de> 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
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
Hi Adrian, On Mon, Jul 10, 2023 at 10:20 AM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> 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
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 <linux@roeck-us.net> > Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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
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]; } /*
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 <linux@roeck-us.net> Closes: https://lore.kernel.org/r/fbfea3ad-d327-4ad5-ac9c-648c7ca3fe1f@roeck-us.net Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- 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(-)