Re: [RFC PATCH v2 2/5] irqchip/realtek-rtl: fix off-by-one in routing
From: Marc Zyngier <maz@kernel.org>
Date: 2021-12-27 10:16:02
Also in:
lkml
On Sun, 26 Dec 2021 19:59:25 +0000, Sander Vanheule [off-list ref] wrote:
There is an offset between routing values (1..6) and the connected MIPS CPU interrupts (2..7), but no distinction was made between these two values. This issue was previously hidden during testing, because an interrupt mapping was used where for each required interrupt another (unused) routing was configured, with an offset of +1.
Where does this 'other routing' come from?
quoted hunk ↗ jump to hunk
Offset the CPU IRQ numbers by -1 to retrieve the correct routing value. Fixes: 9f3a0f34b84a ("irqchip: Add support for Realtek RTL838x/RTL839x interrupt controller") Signed-off-by: Sander Vanheule <sander@svanheule.net> --- drivers/irqchip/irq-realtek-rtl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c index d6788dd93c7b..568614edd88f 100644 --- a/drivers/irqchip/irq-realtek-rtl.c +++ b/drivers/irqchip/irq-realtek-rtl.c@@ -95,7 +95,8 @@ static void realtek_irq_dispatch(struct irq_desc *desc) * SoC interrupts are cascaded to MIPS CPU interrupts according to the * interrupt-map in the device tree. Each SoC interrupt gets 4 bits for * the CPU interrupt in an Interrupt Routing Register. Max 32 SoC interrupts - * thus go into 4 IRRs. + * thus go into 4 IRRs. A routing value of '0' means the interrupt is left + * disconnected. Routing values {1..15} connect to output lines {0..14}. */ static int __init map_interrupts(struct device_node *node, struct irq_domain *domain) {@@ -134,7 +135,7 @@ static int __init map_interrupts(struct device_node *node, struct irq_domain *do of_node_put(cpu_ictl); cpu_int = be32_to_cpup(imap + 2); - if (cpu_int > 7) + if (cpu_int > 7 || cpu_int < 2)
How many output lines do you have? The comment above says something about having 15 output lines, but you limit it to 7...
quoted hunk ↗ jump to hunk
return -EINVAL; if (!(mips_irqs_set & BIT(cpu_int))) {@@ -143,7 +144,8 @@ static int __init map_interrupts(struct device_node *node, struct irq_domain *do mips_irqs_set |= BIT(cpu_int); } - regs[(soc_int * 4) / 32] |= cpu_int << (soc_int * 4) % 32; + /* Use routing values (1..6) for CPU interrupts (2..7) */ + regs[(soc_int * 4) / 32] |= (cpu_int - 1) << (soc_int * 4) % 32; imap += 3; }
What I don't understand is how this worked so far if all mappings were off my one. Or the mapping really doesn't matter, because this is all under SW control? M. -- Without deviation from the norm, progress is not possible.