lkml.org 
[lkml]   [2021]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH v2 2/5] irqchip/realtek-rtl: fix off-by-one in routing
On Sun, 26 Dec 2021 19:59:25 +0000,
Sander Vanheule <sander@svanheule.net> 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?

>
> 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...

> 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.

\
 
 \ /
  Last update: 2021-12-27 11:17    [W:1.397 / U:0.952 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site