Re: realtek,rtl-intc IRQ mapping broken on 5.16-rc1
From: Sander Vanheule
Date: Thu Nov 18 2021 - 14:45:34 EST
Hi Marc,
On Thu, 2021-11-18 at 19:19 +0000, Marc Zyngier wrote:
> Hi Sander,
>
> On Thu, 18 Nov 2021 15:56:06 +0000,
> Sander Vanheule <sander@xxxxxxxxxxxxx> wrote:
> >
> > Hi everyone,
> >
> > On 5.16-rc1, the realtek,rtl-intc interrupt controller driver for
> > Realtek RTL8380 SoCs (and related) appears broken. When booting, I
> > don't get a tty on the serial port, although serial output works.
>
> Thanks for the heads up.
>
> > The watchdog (currently under review) also cannot acquire the
> > required phase1 interrupt, and produces the following output:
>
> > [ 1.968228] realtek-otto-watchdog 18003150.watchdog: error -EINVAL: Failed to get
> > IRQ 4
> > for phase1
> > [ 1.978404] realtek-otto-watchdog: probe of 18003150.watchdog failed with error -22
> >
> > A bisects points to commit 041284181226 ("of/irq: Allow matching of
> > an interrupt-map local to an interrupt controller"). Reverting this
> > above commit and follow-up commit 10a20b34d735 ("of/irq: Don't
> > ignore interrupt-controller when interrupt-map failed") restores the
> > functionality from v5.15.
>
> OK, back to square one, we need to debug this one.
>
> [...]
>
> > cpuintc: cpuintc {
> > compatible = "mti,cpu-interrupt-controller";
> > #address-cells = <0>;
> > #interrupt-cells = <1>;
> > interrupt-controller;
> > };
> >
>
> [...]
>
> >
> > intc: interrupt-controller@3000 {
> > compatible = "realtek,rtl-intc";
> > reg = <0x3000 0x20>;
> > interrupt-controller;
> > #interrupt-cells = <1>;
> >
> > #address-cells = <0>;
> > interrupt-map =
> > <31 &cpuintc 2>, /* UART0 */
> > <20 &cpuintc 3>, /* SWCORE */
> > <19 &cpuintc 4>, /* WDT IP1 */
> > <18 &cpuintc 5>; /* WDT IP2 */
> > };
>
> Something looks pretty odd. With 5.15, this interrupt-map would be
> completely ignored. With 5.16-rc1, we should actually honour it.
>
> /me digs...
>
> Gah, I see. This driver has its own interrupt-map parser and invents
> something out of thin air. I will bang my own head on the wall for
> having merged this horror.
>
> Can you try applying the patch below and rename the interrupt-map
> property in your DT to "silly-interrupt-map" and let me know if that
> helps?
I've dropped the aforementioned reverts, and applied your suggested changes to the DTS and
irq-realtek-rtl. Interrupts now appear to work like before; UART console and watchdog work
as expected.
Best,
Sander
>
> That's of course not the right fix, but that's just to confirm the
> extent of the damage...
>
> M.
>
> diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
> index fd9f275592d2..3641cd2b1a2c 100644
> --- a/drivers/irqchip/irq-realtek-rtl.c
> +++ b/drivers/irqchip/irq-realtek-rtl.c
> @@ -114,7 +114,7 @@ static int __init map_interrupts(struct device_node *node, struct
> irq_domain *do
> if (ret || tmp)
> return -EINVAL;
>
> - imap = of_get_property(node, "interrupt-map", &imaplen);
> + imap = of_get_property(node, "silly-interrupt-map", &imaplen);
> if (!imap || imaplen % 3)
> return -EINVAL;
>
>