Re: [PATCH 2/2] irqchip/mips-gic: Fix Local compare interrupt

[Matt Redfearn]

Hi Paul,

On 10/04/17 23:06, Paul Burton wrote:
Hi Matt,

On Friday, 31 March 2017 04:05:32 PDT Matt Redfearn wrote:
Commit 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts") added
mapping of several local interrupts during initialisation of the gic
driver. This associates virq numbers with these interrupts.
Unfortunately, as not all of the interrupts are mapped in hardware
order, when drivers subsequently request these interrupts they conflict
with the mappings that have already been set up. For example, this
manifests itself in the gic clocksource driver, which fails to probe
with the message:

clocksource: GIC: mask: 0xffffffffffffffff max_cycles: 0x7350c9738,
max_idle_ns: 440795203769 ns
GIC timer IRQ 25 setup failed: -22

This is because virq 25 (the correct IRQ number specified via device
tree) was allocated to the PERFCTR interrupt (and 24 to the timer, 26 to
the FDC).
I'm confused by this - the DT doesn't specify VIRQs, it specifies hardware IRQ
numbers. Which VIRQ is used should be irrelevant. Is this on a system using
gic_clocksource_init() from platform code? (Malta?) and therefore relying on
MIPS_GIC_IRQ_BASE?

Yes, this is on Malta, which as you say, uses MIPS_GIC_IRQ_BASE. On 
Malta that ends up, through the definition of I8259A_IRQ_BASE and 
MIPS_CPU_IRQ_BASE, to be 24. Therefore hardware interrupt 1 of the GIC 
ends up expecting to be allocated at virq 25. But since 4cfffcfa5106, 
that virq number was allocated to the PERFCTR interrupt. Everything 
about the order-dependent and hardcoded bases of Maltas IRQs seems bad 
and needs looking at but this was the easiest fix for this cycle.


If so I think this would be much more cleanly fixed by moving to probe the
clocksource using DT

Not sure that would help if Maltas expected virq for this source had 
already been allocated?

Thanks,
Matt

than by adding more fragile order-dependent mappings in
the GIC driver. Perhaps we have to live with it for this cycle though...

Thanks,
     Paul

To fix this, map all of these local interrupts in the hardware
order so as to associate their virq numbers with the correct hw
interrupts.

Fixes: 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts")
Signed-off-by: Matt Redfearn <matt.redfe...@imgtec.com>
---

  drivers/irqchip/irq-mips-gic.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 11d12bccc4e7..cd20df12d63d 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -991,8 +991,12 @@ static void __init gic_map_single_int(struct
device_node *node,

  static void __init gic_map_interrupts(struct device_node *node)
  {
+       gic_map_single_int(node, GIC_LOCAL_INT_WD);
+       gic_map_single_int(node, GIC_LOCAL_INT_COMPARE);
        gic_map_single_int(node, GIC_LOCAL_INT_TIMER);
        gic_map_single_int(node, GIC_LOCAL_INT_PERFCTR);
+       gic_map_single_int(node, GIC_LOCAL_INT_SWINT0);
+       gic_map_single_int(node, GIC_LOCAL_INT_SWINT1);
        gic_map_single_int(node, GIC_LOCAL_INT_FDC);
  }