[PATCH 0/2] Fix v4.11 malta_defconfig regressions

[PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Matt Redfearn]

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

[PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Matt Redfearn]

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

[PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup

[Matt Redfearn]

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 <matt.redfearn@imgtec.com>

---

 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

[PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup

[Matt Redfearn]

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 <matt.redfearn@imgtec.com>

---

 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

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

[Matt Redfearn]

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 <matt.redfearn@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);
 }
 
-- 
2.7.4

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

[Matt Redfearn]

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 <matt.redfearn@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);
 }
 
-- 
2.7.4

Re: [PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Marc Zyngier]

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

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

[Ralf Baechle]

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 <matt.redfearn@imgtec.com>

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

Re: [PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup

[Ralf Baechle]

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

Re: [PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup

[Matt Redfearn]

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

Re: [PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup

[Matt Redfearn]

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

Re: [PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Matt Redfearn]

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

Re: [PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Matt Redfearn]

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

Re: [PATCH 0/2] Fix v4.11 malta_defconfig regressions

[Marc Zyngier]

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

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

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

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 <matt.redfearn@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);
>  }


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

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 <matt.redfearn@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);
>  }


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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.redfearn@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);
>>   }

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.redfearn@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);
>>   }

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

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 3007 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Paul Burton]

[-- Attachment #1: Type: text/plain, Size: 3007 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]