Since v4.11-rc1, 3 regressions have been observed on the Malta platform,
using malta_defconfig. which prevent it booting. These patches fix 2 of
them. The third one is that malta_defconfig, which uses SMP-MT, no
longer sets up its IPIs correctly resulting is a string of messages
like:
irq 23: nobody cared (try booting with the "irqpoll" option)
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.11.0-rc4 #421
Stack : 00000000 00000000 00000000 00000000 807cdff2 00000047 00000000 0000003d
80741327 8f093194 806c191c 00000000 00000001 807c9acc 80756078 807d0000
807cdbe4 80177c78 00000003 0000003c 00000006 80177a04 806c70a8 8f02be8c
00000006 801b4c8c 00000000 00000000 ffffffff 00000000 8f02be8c 80740000
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
...
Call Trace:
[<8010c6c0>] show_stack+0x88/0xa4
[<80380fb8>] dump_stack+0x88/0xd0
[<8017cf64>] __report_bad_irq+0x48/0x108
[<8017d2d4>] note_interrupt+0x1c0/0x2fc
[<80179ed4>] handle_irq_event_percpu+0x4c/0x64
[<8017eafc>] handle_percpu_irq+0x88/0xb8
[<801791c0>] generic_handle_irq+0x40/0x58
[<80108664>] do_IRQ+0x18/0x24
[<803b83fc>] plat_irq_dispatch+0x54/0xa8
handlers:
Disabling IRQ #23
This regression is fixed by Paul Burtons series "MIPS/irqchip: Use IPI
IRQ domains for CPU interrupt controller IPIs", but it is a large change
for this stage in the cycle so I don't know how best to proceed with
that one.
Matt Redfearn (2):
MIPS: Malta: Fix i8259 irqchip setup
irqchip/mips-gic: Fix Local compare interrupt
arch/mips/mti-malta/malta-int.c | 13 +++++++++++++
drivers/irqchip/irq-mips-gic.c | 4 ++++
2 files changed, 17 insertions(+)
--
2.7.4
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). 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 <[email protected]>
---
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);
}
--
2.7.4
Since commit 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts"),
the gic driver has been allocating virq's for local interrupts during
its initialisation. Unfortunately on Malta platforms, these are the
first IRQs to be allocated and so are allocated virqs 1-3. The i8259
driver uses a legacy irq domain which expects to map virqs 0-15. Probing
of that driver therefore fails because some of those virqs are already
taken, with the warning:
WARNING: CPU: 0 PID: 0 at kernel/irq/irqdomain.c:344
irq_domain_associate+0x1e8/0x228
error: virq1 is already associated
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.10.0-rc6-00011-g4cfffcfa5106 #368
Stack : 00000000 00000000 807ae03a 0000004d 00000000 806c1010 0000000b ffff0a01
80725467 807258f4 806a64a4 00000000 00000000 807a9acc 00000100 80713e68
806d5598 8017593c 8072bf90 8072bf94 806ac358 00000000 806abb60 80713ce4
00000100 801b22d4 806d5598 8017593c 807ae03a 00000000 80713ce4 80720000
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
...
Call Trace:
[<8010c480>] show_stack+0x88/0xa4
[<80376758>] dump_stack+0x88/0xd0
[<8012c4a8>] __warn+0x104/0x118
[<8012c4ec>] warn_slowpath_fmt+0x30/0x3c
[<8017edfc>] irq_domain_associate+0x1e8/0x228
[<8017efd0>] irq_domain_add_legacy+0x7c/0xb0
[<80764c50>] __init_i8259_irqs+0x64/0xa0
[<80764ca4>] i8259_of_init+0x18/0x74
[<8076ddc0>] of_irq_init+0x19c/0x310
[<80752dd8>] arch_init_irq+0x28/0x19c
[<80750a08>] start_kernel+0x2a8/0x434
Fix this by reserving the required i8259 virqs in malta platform code
before probing any irq chips.
Fixes: 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts")
Signed-off-by: Matt Redfearn <[email protected]>
---
arch/mips/mti-malta/malta-int.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/mips/mti-malta/malta-int.c b/arch/mips/mti-malta/malta-int.c
index cb675ec6f283..474b372e0dd9 100644
--- a/arch/mips/mti-malta/malta-int.c
+++ b/arch/mips/mti-malta/malta-int.c
@@ -232,6 +232,19 @@ void __init arch_init_irq(void)
{
int corehi_irq;
+#ifdef CONFIG_I8259
+ /*
+ * Preallocate the i8259's expected virq's here. Since irqchip_init()
+ * will probe the irqchips in hierarchial order, i8259 is probed last.
+ * If anything allocates a virq before the i8259 is probed, it will
+ * be given one of the i8259's expected range and consequently setup
+ * of the i8259 will fail.
+ */
+ WARN(irq_alloc_descs(I8259A_IRQ_BASE, I8259A_IRQ_BASE,
+ 16, numa_node_id()) < 0,
+ "Cannot reserve i8259 virqs at IRQ%d\n", I8259A_IRQ_BASE);
+#endif /* CONFIG_I8259 */
+
i8259_set_poll(mips_pcibios_iack);
irqchip_init();
--
2.7.4
Hi Matt,
On 31/03/17 12:05, Matt Redfearn wrote:
>
> Since v4.11-rc1, 3 regressions have been observed on the Malta platform,
> using malta_defconfig. which prevent it booting. These patches fix 2 of
> them. The third one is that malta_defconfig, which uses SMP-MT, no
> longer sets up its IPIs correctly resulting is a string of messages
> like:
>
> irq 23: nobody cared (try booting with the "irqpoll" option)
> CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.11.0-rc4 #421
> Stack : 00000000 00000000 00000000 00000000 807cdff2 00000047 00000000 0000003d
> 80741327 8f093194 806c191c 00000000 00000001 807c9acc 80756078 807d0000
> 807cdbe4 80177c78 00000003 0000003c 00000006 80177a04 806c70a8 8f02be8c
> 00000006 801b4c8c 00000000 00000000 ffffffff 00000000 8f02be8c 80740000
> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> ...
> Call Trace:
> [<8010c6c0>] show_stack+0x88/0xa4
> [<80380fb8>] dump_stack+0x88/0xd0
> [<8017cf64>] __report_bad_irq+0x48/0x108
> [<8017d2d4>] note_interrupt+0x1c0/0x2fc
> [<80179ed4>] handle_irq_event_percpu+0x4c/0x64
> [<8017eafc>] handle_percpu_irq+0x88/0xb8
> [<801791c0>] generic_handle_irq+0x40/0x58
> [<80108664>] do_IRQ+0x18/0x24
> [<803b83fc>] plat_irq_dispatch+0x54/0xa8
> handlers:
> Disabling IRQ #23
>
> This regression is fixed by Paul Burtons series "MIPS/irqchip: Use IPI
> IRQ domains for CPU interrupt controller IPIs", but it is a large change
> for this stage in the cycle so I don't know how best to proceed with
> that one.
>
>
>
> Matt Redfearn (2):
> MIPS: Malta: Fix i8259 irqchip setup
> irqchip/mips-gic: Fix Local compare interrupt
>
> arch/mips/mti-malta/malta-int.c | 13 +++++++++++++
> drivers/irqchip/irq-mips-gic.c | 4 ++++
> 2 files changed, 17 insertions(+)
>
I can take the GIC patch through the irq tree if that's convenient (I
was about to send a PR anyway). Just let me know.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Fri, Mar 31, 2017 at 12:05:32PM +0100, 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). 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 <[email protected]>
Acked-by: Ralf Baechle <[email protected]>
Ralf
On Fri, Mar 31, 2017 at 12:05:31PM +0100, Matt Redfearn wrote:
> diff --git a/arch/mips/mti-malta/malta-int.c b/arch/mips/mti-malta/malta-int.c
> index cb675ec6f283..474b372e0dd9 100644
> --- a/arch/mips/mti-malta/malta-int.c
> +++ b/arch/mips/mti-malta/malta-int.c
> @@ -232,6 +232,19 @@ void __init arch_init_irq(void)
> {
> int corehi_irq;
>
> +#ifdef CONFIG_I8259
> + /*
> + * Preallocate the i8259's expected virq's here. Since irqchip_init()
> + * will probe the irqchips in hierarchial order, i8259 is probed last.
> + * If anything allocates a virq before the i8259 is probed, it will
> + * be given one of the i8259's expected range and consequently setup
> + * of the i8259 will fail.
> + */
> + WARN(irq_alloc_descs(I8259A_IRQ_BASE, I8259A_IRQ_BASE,
> + 16, numa_node_id()) < 0,
> + "Cannot reserve i8259 virqs at IRQ%d\n", I8259A_IRQ_BASE);
> +#endif /* CONFIG_I8259 */
> +
> i8259_set_poll(mips_pcibios_iack);
CONFIG_I8259 is always defined on Malta so the #ifdef is pointless.
Ralf
Hi Ralf,
On 31/03/17 13:49, Ralf Baechle wrote:
> On Fri, Mar 31, 2017 at 12:05:31PM +0100, Matt Redfearn wrote:
>
>> diff --git a/arch/mips/mti-malta/malta-int.c b/arch/mips/mti-malta/malta-int.c
>> index cb675ec6f283..474b372e0dd9 100644
>> --- a/arch/mips/mti-malta/malta-int.c
>> +++ b/arch/mips/mti-malta/malta-int.c
>> @@ -232,6 +232,19 @@ void __init arch_init_irq(void)
>> {
>> int corehi_irq;
>>
>> +#ifdef CONFIG_I8259
>> + /*
>> + * Preallocate the i8259's expected virq's here. Since irqchip_init()
>> + * will probe the irqchips in hierarchial order, i8259 is probed last.
>> + * If anything allocates a virq before the i8259 is probed, it will
>> + * be given one of the i8259's expected range and consequently setup
>> + * of the i8259 will fail.
>> + */
>> + WARN(irq_alloc_descs(I8259A_IRQ_BASE, I8259A_IRQ_BASE,
>> + 16, numa_node_id()) < 0,
>> + "Cannot reserve i8259 virqs at IRQ%d\n", I8259A_IRQ_BASE);
>> +#endif /* CONFIG_I8259 */
>> +
>> i8259_set_poll(mips_pcibios_iack);
> CONFIG_I8259 is always defined on Malta so the #ifdef is pointless.
>
> Ralf
Ah, true. I was looking at arch/mips/include/asm/mach-generic/irq.h
where I8259A_IRQ_BASE is defined, it's wrapped in that CONFIG so I kept
it here, but you're right of course that Malta always defines it. Would
you like a v2 with that removed?
Thanks,
Matt
Hi Marc,
On 31/03/17 13:04, Marc Zyngier wrote:
> Hi Matt,
>
> On 31/03/17 12:05, Matt Redfearn wrote:
>> Since v4.11-rc1, 3 regressions have been observed on the Malta platform,
>> using malta_defconfig. which prevent it booting. These patches fix 2 of
>> them. The third one is that malta_defconfig, which uses SMP-MT, no
>> longer sets up its IPIs correctly resulting is a string of messages
>> like:
>>
>> irq 23: nobody cared (try booting with the "irqpoll" option)
>> CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.11.0-rc4 #421
>> Stack : 00000000 00000000 00000000 00000000 807cdff2 00000047 00000000 0000003d
>> 80741327 8f093194 806c191c 00000000 00000001 807c9acc 80756078 807d0000
>> 807cdbe4 80177c78 00000003 0000003c 00000006 80177a04 806c70a8 8f02be8c
>> 00000006 801b4c8c 00000000 00000000 ffffffff 00000000 8f02be8c 80740000
>> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> ...
>> Call Trace:
>> [<8010c6c0>] show_stack+0x88/0xa4
>> [<80380fb8>] dump_stack+0x88/0xd0
>> [<8017cf64>] __report_bad_irq+0x48/0x108
>> [<8017d2d4>] note_interrupt+0x1c0/0x2fc
>> [<80179ed4>] handle_irq_event_percpu+0x4c/0x64
>> [<8017eafc>] handle_percpu_irq+0x88/0xb8
>> [<801791c0>] generic_handle_irq+0x40/0x58
>> [<80108664>] do_IRQ+0x18/0x24
>> [<803b83fc>] plat_irq_dispatch+0x54/0xa8
>> handlers:
>> Disabling IRQ #23
>>
>> This regression is fixed by Paul Burtons series "MIPS/irqchip: Use IPI
>> IRQ domains for CPU interrupt controller IPIs", but it is a large change
>> for this stage in the cycle so I don't know how best to proceed with
>> that one.
>>
>>
>>
>> Matt Redfearn (2):
>> MIPS: Malta: Fix i8259 irqchip setup
>> irqchip/mips-gic: Fix Local compare interrupt
>>
>> arch/mips/mti-malta/malta-int.c | 13 +++++++++++++
>> drivers/irqchip/irq-mips-gic.c | 4 ++++
>> 2 files changed, 17 insertions(+)
>>
> I can take the GIC patch through the irq tree if that's convenient (I
> was about to send a PR anyway). Just let me know.
>
> Thanks,
>
> M.
Yes that would be great, please. Ralf has just acked it.
Thanks,
Matt
On 31/03/17 13:55, Matt Redfearn wrote:
> Hi Marc,
>
>
> On 31/03/17 13:04, Marc Zyngier wrote:
>> Hi Matt,
>>
>> On 31/03/17 12:05, Matt Redfearn wrote:
>>> Since v4.11-rc1, 3 regressions have been observed on the Malta platform,
>>> using malta_defconfig. which prevent it booting. These patches fix 2 of
>>> them. The third one is that malta_defconfig, which uses SMP-MT, no
>>> longer sets up its IPIs correctly resulting is a string of messages
>>> like:
>>>
>>> irq 23: nobody cared (try booting with the "irqpoll" option)
>>> CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 4.11.0-rc4 #421
>>> Stack : 00000000 00000000 00000000 00000000 807cdff2 00000047 00000000 0000003d
>>> 80741327 8f093194 806c191c 00000000 00000001 807c9acc 80756078 807d0000
>>> 807cdbe4 80177c78 00000003 0000003c 00000006 80177a04 806c70a8 8f02be8c
>>> 00000006 801b4c8c 00000000 00000000 ffffffff 00000000 8f02be8c 80740000
>>> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> ...
>>> Call Trace:
>>> [<8010c6c0>] show_stack+0x88/0xa4
>>> [<80380fb8>] dump_stack+0x88/0xd0
>>> [<8017cf64>] __report_bad_irq+0x48/0x108
>>> [<8017d2d4>] note_interrupt+0x1c0/0x2fc
>>> [<80179ed4>] handle_irq_event_percpu+0x4c/0x64
>>> [<8017eafc>] handle_percpu_irq+0x88/0xb8
>>> [<801791c0>] generic_handle_irq+0x40/0x58
>>> [<80108664>] do_IRQ+0x18/0x24
>>> [<803b83fc>] plat_irq_dispatch+0x54/0xa8
>>> handlers:
>>> Disabling IRQ #23
>>>
>>> This regression is fixed by Paul Burtons series "MIPS/irqchip: Use IPI
>>> IRQ domains for CPU interrupt controller IPIs", but it is a large change
>>> for this stage in the cycle so I don't know how best to proceed with
>>> that one.
>>>
>>>
>>>
>>> Matt Redfearn (2):
>>> MIPS: Malta: Fix i8259 irqchip setup
>>> irqchip/mips-gic: Fix Local compare interrupt
>>>
>>> arch/mips/mti-malta/malta-int.c | 13 +++++++++++++
>>> drivers/irqchip/irq-mips-gic.c | 4 ++++
>>> 2 files changed, 17 insertions(+)
>>>
>> I can take the GIC patch through the irq tree if that's convenient (I
>> was about to send a PR anyway). Just let me know.
>>
>> Thanks,
>>
>> M.
>
> Yes that would be great, please. Ralf has just acked it.
OK. I'll you and Ralf deal with the i8259 patch separately.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
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?
If so I think this would be much more cleanly fixed by moving to probe the
clocksource using DT 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 <[email protected]>
> ---
>
> 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);
> }
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 <[email protected]>
>> ---
>>
>> 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);
>> }
Hi Matt,
On Tuesday, 11 April 2017 01:20:35 PDT Matt Redfearn wrote:
> On 10/04/17 23:06, Paul Burton wrote:
> > 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?
Well the problem is that Malta expects a particular VIRQ - if it instead
probed the clock event driver via DT, which specifies a hardware interrupt
number, then that problem goes away & it doesn't matter which VIRQ is used.
For example the generic platform (including the Malta board support for it in
the downstream engineering kernels) ought to work fine without this because
there isn't an expectation for a particular VIRQ. Hopefully I'll have time to
submit some more of that code for the v4.13 cycle & we can eventually scrap
all of these hardcoded VIRQs along with the mti-malta board code that commits
the sins.
An alternative might be to have the Malta board code use irq_create_mapping()
to map the hardware IRQ to a VIRQ rather than hardcoding the VIRQ, which would
be an improvement but would then require that the board code have access to
the GIC's struct irq_domain. Given that we want to move Malta towards DT
anyway that doesn't seem worth much effort.
Thanks,
Paul