Skip to contentSkip to conversation

TI-Davinci 6446 oops on interrupts

DISCUSSION Holger Freyther 6 participants 17 messages Jun 6, 2011 – Jun 10, 2011

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

Hi,

my not mainlined with custom drivers board support seems to suffer from an issue introduced by moving the irq.c to the generic GPIO interrupt code (aac4dd1dab8acfc244d697473d2a5f4424a5746c, reverting it makes the oops go away). I think this is caused by either interrupt #46 or #47 on this system which is on the second 'bank'.

At first I thought that the usage of IRQ_ENT_REG1_OFFSET and IRQ_REG1_OFFSET was simply omitted but there is the for loop that counts j by four (offset between REG0 and REG1).

Is this issue known to anyone? I feel try to understand this a bit more as well and try to come up with a patch.

Unable to handle kernel paging request at virtual address 5ffffc1b
[5ffffc1b] *pgd=00000000
Internal error: Oops: 801 [#1] PREEMPT
Modules linked in: ...
CPU: 0    Tainted: G         C   (3.0.0-rc2-00006-gff5f4d6-dirty #68)
PC is at irq_gc_mask_clr_bit+0x38/0x40
LR is at gpio_irq_handler+0x44/0xdc
pc : [<c0d6f3a4>]    lr : [<c0d29a74>]    psr: 60000093

sp : c10abe68 ip : 00000001 fp : c10abe74
r10: 00000000 r9 : c10b69d0 r8 : 00000001
r7 : fec67010 r6 : 0000ffff r5 : 00000038 r4 : 00000038 r3 : 0000001c r2 : 00000400 r1 : 5ffffbff r0 : c70050f0
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: 85cfc000 DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc10aa270)

Stack: (0xc10abe68 to 0xc10ac000)

be60: c10abeac c10abe78 c0d29a74 c0d6f37c c0d3c028 c0d2c79c
be80: 00000000 c10c0b80 00000038 00000000 00000002 00000001 c10aa000 00000000
bea0: c10abebc c10abeb0 c0d6c0d8 c0d29a40 c10abedc c10abec0 c0d1e038 c0d6c0b4
bec0: 60000013 ffffffff fec48000 60000013 c10abf8c c10abee0 c0d1eb4c c0d1e010
bee0: 00000001 00000000 0075287d 00000000 c10b52e0 c10b52e0 60000013 60000013
bf00: ffff9214 c10b4508 00000000 c10abf8c c10abec0 c10abf28 c0d56068 c0d610a4
bf20: 60000013 ffffffff 00000002 c0d20e24 c0d20e44 00000004 4f482f80 ffff920b
bf40: 055d4a80 00000000 4f6b9d83 00000004 c10b0d00 60000013 4f6b9d83 00000004
bf60: 00000000 c10aa000 c10cc2c4 c10b0514 c10b0698 80004000 41069265 80021e74
bf80: c10abfac c10abf90 c0d2003c c0d60dc8 00000002 c10cc220 c0d1924c c11dc480
bfa0: c10abfc4 c10abfb0 c0fd10fc c0d20014 00000000 c10ac8f4 c10abff4 c10abfc8
bfc0: c00089c4 c0fd1088 c00083d4 00000000 00000000 c0d1924c 00053175 c10ac010
bfe0: c0d19248 c10b050c 00000000 c10abff8 8000803c c0008740 00000000 00000000

Backtrace:
[<c0d6f36c>] (irq_gc_mask_clr_bit+0x0/0x40) from [<c0d29a74>]
(gpio_irq_handler+0x44/0xdc)
[<c0d29a30>] (gpio_irq_handler+0x0/0xdc) from [<c0d6c0d8>]
(generic_handle_irq+0x34/0x48)
[<c0d6c0a4>] (generic_handle_irq+0x0/0x48) from [<c0d1e038>]
(asm_do_IRQ+0x38/0x8c)
[<c0d1e000>] (asm_do_IRQ+0x0/0x8c) from [<c0d1eb4c>] (__irq_svc+0x4c/0x90)
#1 lore raw
Quoted 7 lines · Holger Freyther

Hi,

my not mainlined with custom drivers board support seems to suffer from an issue introduced by moving the irq.c to the generic GPIO interrupt code (aac4dd1dab8acfc244d697473d2a5f4424a5746c, reverting it makes the oops go away). I think this is caused by either interrupt #46 or #47 on this system which is on the second 'bank'.

I resorted to printf debugging, it is IRQ 56 which is the IRQ_GPIOBNK0.. so I wonder if this IRQ should end up in the GC GPIO code at all?

#2 lore raw
Quoted 11 lines · Holger Hans Peter Freyther
Quoted 7 lines · Holger Freyther

Hi,

my not mainlined with custom drivers board support seems to suffer from an issue introduced by moving the irq.c to the generic GPIO interrupt code (aac4dd1dab8acfc244d697473d2a5f4424a5746c, reverting it makes the oops go away). I think this is caused by either interrupt #46 or #47 on this system which is on the second 'bank'.

I resorted to printf debugging, it is IRQ 56 which is the IRQ_GPIOBNK0.. so I wonder if this IRQ should end up in the GC GPIO code at all?

That depends on davinci_soc_info.intc_irq_num.

#define DAVINCI_N_AINTC_IRQ 64
#define DA830_N_CP_INTC_IRQ 96
#define TNETV107X_N_CP_INTC_IRQ 96

No idea which one applies to your machine, but for all in tree boards 56 is in the range of intc interrupts.

Thanks,

tglx

#3 lore raw
Quoted 11 lines · Thomas Gleixner
Quoted 2 lines

I resorted to printf debugging, it is IRQ 56 which is the IRQ_GPIOBNK0.. so I wonder if this IRQ should end up in the GC GPIO code at all?

That depends on davinci_soc_info.intc_irq_num.

#define DAVINCI_N_AINTC_IRQ 64
#define DA830_N_CP_INTC_IRQ 96
#define TNETV107X_N_CP_INTC_IRQ 96

No idea which one applies to your machine, but for all in tree boards 56 is in the range of intc interrupts.

It should be the DAVINCI_N_AINTC_IRQ (dm6446.c:davinci_soc_info_dm644x), I have commented out the AINTC code in davinci_gpio_irq_setup and my crash is gone, so without knowing the code at all I assume one should not end where we end up.. or at least not with the parameters.

I have instrumented the GC IRQ code and I get:
IRQ40:

	Interrupt is 40 32
	RegBase is 0xfec48000 offset 28

IRQ56:

	Interrupt is 56...
	RegBase is 0x5ffffbff offset 28

and both interrupts should be on REG1.. so for IRQ56 it looks like this method is entered with bogus data. Again, I have no idea about the underlying code, but could there be an issue with chained irq and the GC IRC code?

going to dig deeper..
holger

kernel/irq/generic-chip.c +7 -0
index 31a9db7..fc505c3 100644
@@ -74,6 +74,13 @@ void irq_gc_mask_set_bit(struct irq_data *d)
 void irq_gc_mask_clr_bit(struct irq_data *d)
 {
        struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+
+       if (d->irq > 31) {
+               printk(KERN_ERR "Interrupt is %d %d\n", d->irq, gc->irq_base);
+               printk(KERN_ERR "RegBase is 0x%p offset %lu\n", gc->reg_base,
+		       cur_regs(d)->mask);
+       }
+
        u32 mask = 1 << (d->irq - gc->irq_base);
 
        irq_gc_lock(gc);
#4 lore raw
Quoted 5 lines · Holger Freyther

On 06/06/2011 04:54 PM, Thomas Gleixner wrote:

and both interrupts should be on REG1.. so for IRQ56 it looks like this method is entered with bogus data. Again, I have no idea about the underlying code, but could there be an issue with chained irq and the GC IRC code?

Crap, yes. The code which does the chained handler setup overwrites chip_data.

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

---
 arch/arm/mach-davinci/gpio.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
arch/arm/mach-davinci/gpio.c +4 -3
Index: linux-2.6/arch/arm/mach-davinci/gpio.c
===================================================================
@@ -252,9 +252,11 @@ static struct irq_chip gpio_irqchip = {
 static void
 gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 {
-	struct davinci_gpio_regs __iomem *g = irq2regs(irq);
+	struct davinci_gpio_regs __iomem *g;
 	u32 mask = 0xffff;
 
+	g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc);
+
 	/* we only care about one bank */
 	if (irq & 1)
 		mask <<= 16;
@@ -422,8 +424,7 @@ static int __init davinci_gpio_irq_setup
 
 		/* set up all irqs in this bank */
 		irq_set_chained_handler(bank_irq, gpio_irq_handler);
-		irq_set_chip_data(bank_irq, (__force void *)g);
-		irq_set_handler_data(bank_irq, (void *)irq);
+		irq_set_handler_data(bank_irq, (__force void *)g);
 
 		for (i = 0; i < 16 && gpio < ngpio; i++, irq++, gpio++) {
 			irq_set_chip(irq, &gpio_irqchip);
author Tested-by Holger Hans Peter Freyther
#5 lore raw
Quoted 3 lines · Thomas Gleixner

Crap, yes. The code which does the chained handler setup overwrites chip_data.

Ah, you were too fast in the end, I just finished reading linux/irq.h. :)

Quoted 5 lines

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

#6 lore raw

Kevin,

Quoted 10 lines · Holger Freyther
Quoted 3 lines · Thomas Gleixner

Crap, yes. The code which does the chained handler setup overwrites chip_data.

Ah, you were too fast in the end, I just finished reading linux/irq.h. :)

Quoted 7 lines
Quoted 5 lines

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

can you please pick that up? It should have a Cc: stable at kernel.org tag as well.

Thanks,

tglx

#7 lore raw
Quoted 18 lines · Thomas Gleixner

Kevin,

Quoted 14 lines · Holger Freyther
Quoted 3 lines · Thomas Gleixner

Crap, yes. The code which does the chained handler setup overwrites chip_data.

Ah, you were too fast in the end, I just finished reading linux/irq.h. :)

Quoted 4 lines

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

can you please pick that up? It should have a Cc: stable at kernel.org tag as well.

Yup, will do.

Thanks,

Kevin

#8 lore raw
In reply to Thomas Gleixner

Hi Thomas,

Quoted 10 lines · Thomas Gleixner
Quoted 7 lines
Quoted 5 lines

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

can you please pick that up? It should have a Cc: stable at kernel.org tag as well.

But generic IRQ chip support for DaVinci was merged only in v3.0-rc1.

$ git log --tags --source --oneline arch/arm/mach-davinci/irq.c
aac4dd1 v3.0-rc1 arm: davinci: Use generic irq chip

Thanks,
Sekhar

#9 lore raw
Quoted 12 lines · Sekhar

Hi Thomas,

Quoted 8 lines · Thomas Gleixner
Quoted 7 lines
Quoted 5 lines

Subject: arm: davinci: Fix fallout from generic irq chip conversion
From: Thomas Gleixner <[email protected]>
Date: Mon, 06 Jun 2011 11:51:43 +0200

Quoted 8 lines
Quoted 2 lines

can you please pick that up? It should have a Cc: stable at kernel.org tag as well.

But generic IRQ chip support for DaVinci was merged only in v3.0-rc1.

$ git log --tags --source --oneline arch/arm/mach-davinci/irq.c
aac4dd1 v3.0-rc1 arm: davinci: Use generic irq chip

Of course. Stupid me !

#10 lore raw
In reply to Thomas Gleixner
Quoted 9 lines · Thomas Gleixner
Quoted 5 lines · Holger Freyther

On 06/06/2011 04:54 PM, Thomas Gleixner wrote:

and both interrupts should be on REG1.. so for IRQ56 it looks like this method is entered with bogus data. Again, I have no idea about the underlying code, but could there be an issue with chained irq and the GC IRC code?

Crap, yes. The code which does the chained handler setup overwrites chip_data.

Hi Thomas,

I think the for loop above the one that was patched has the same kind of issue (changing the irq_chip_data)? One needs to have a dm365.c for this code to become active though.

In general what do you think about a patch like the one below, it is at least boot tested on a qemu-system-i386.

#11 lore raw
In reply to Holger Freyther
Quoted 24 lines · Holger Freyther

Hi,

my not mainlined with custom drivers board support seems to suffer from an issue introduced by moving the irq.c to the generic GPIO interrupt code (aac4dd1dab8acfc244d697473d2a5f4424a5746c, reverting it makes the oops go away). I think this is caused by either interrupt #46 or #47 on this system which is on the second 'bank'.

At first I thought that the usage of IRQ_ENT_REG1_OFFSET and IRQ_REG1_OFFSET was simply omitted but there is the for loop that counts j by four (offset between REG0 and REG1).

Is this issue known to anyone? I feel try to understand this a bit more as well and try to come up with a patch.

Unable to handle kernel paging request at virtual address 5ffffc1b
[5ffffc1b] *pgd=00000000
Internal error: Oops: 801 [#1] PREEMPT
Modules linked in: ...
CPU: 0    Tainted: G         C   (3.0.0-rc2-00006-gff5f4d6-dirty #68)
PC is at irq_gc_mask_clr_bit+0x38/0x40
LR is at gpio_irq_handler+0x44/0xdc
pc : [<c0d6f3a4>]    lr : [<c0d29a74>]    psr: 60000093
Quoted 4 lines

sp : c10abe68 ip : 00000001 fp : c10abe74
r10: 00000000 r9 : c10b69d0 r8 : 00000001
r7 : fec67010 r6 : 0000ffff r5 : 00000038 r4 : 00000038 r3 : 0000001c r2 : 00000400 r1 : 5ffffbff r0 : c70050f0

5ffffbff + 1c = 5ffffc1b. Looking at the disassembly:

0000014c <irq_gc_mask_clr_bit>:

 14c:   e5903014        ldr     r3, [r0, #20]
 150:   e590c000        ldr     ip, [r0]
 154:   e5932004        ldr     r2, [r3, #4]
 158:   e593100c        ldr     r1, [r3, #12]
 15c:   e062200c        rsb     r2, r2, ip
 160:   e3a0c001        mov     ip, #1
 164:   e1c1221c        bic     r2, r1, ip, lsl r2
 168:   e583200c        str     r2, [r3, #12]
 16c:   e590000c        ldr     r0, [r0, #12]
 170:   e5931000        ldr     r1, [r3]
 174:   e5903064        ldr     r3, [r0, #100]  ; 0x64
 178:   e7812003        str     r2, [r1, r3]
 17c:   e12fff1e        bx      lr

So it's the "str r2, [r1, r3]" and obviously r1 is pointing into nirwana. r3 makes me wonder as well. We write 0x18 into ct->regs.mask because we add 0x04 to the base pointer. So where comes the 0x1c from?

R1 and R3 are read from the generic chip data structure and I'm pretty sure that you are chasing some weird data corruption and that reverting that patch makes it invisible for whatever reason.

Thanks,

tglx

#12 lore raw
Quoted 15 lines · Thomas Gleixner

0000014c <irq_gc_mask_clr_bit>:

 14c:   e5903014        ldr     r3, [r0, #20]
 150:   e590c000        ldr     ip, [r0]
 154:   e5932004        ldr     r2, [r3, #4]
 158:   e593100c        ldr     r1, [r3, #12]
 15c:   e062200c        rsb     r2, r2, ip
 160:   e3a0c001        mov     ip, #1
 164:   e1c1221c        bic     r2, r1, ip, lsl r2
 168:   e583200c        str     r2, [r3, #12]
 16c:   e590000c        ldr     r0, [r0, #12]
 170:   e5931000        ldr     r1, [r3]
 174:   e5903064        ldr     r3, [r0, #100]  ; 0x64
 178:   e7812003        str     r2, [r1, r3]
 17c:   e12fff1e        bx      lr
Quoted 3 lines

So it's the "str r2, [r1, r3]" and obviously r1 is pointing into nirwana. r3 makes me wonder as well. We write 0x18 into ct->regs.mask because we add 0x04 to the base pointer. So where comes the 0x1c from?

Beware - I don't think you can build the file and work out what went wrong in this manner. Compilers like to change their register allocation for even the slightest difference of compiler options and preceding _unrelated_ code.

Unless we have the Code: line we don't know what the code being executed was. It's important that folk always report the entire oops dump, not extracts from it that they _think_ may be relevant.

#13 lore raw

On 06/06/2011 06:32 PM, Russell King - ARM Linux wrote:
Hi Russell,

thanks for your time and sorry for sending a truncated kernel oops. In the other part of the thread we found that the davinci GPIO code overrides the IRQ chip data.

Quoted 3 lines

Unless we have the Code: line we don't know what the code being executed was. It's important that folk always report the entire oops dump, not extracts from it that they _think_ may be relevant.

is it still useful to send a complete oops, or do you think the issue is fully understood?

holger

#14 lore raw
Quoted 8 lines · Holger Freyther

On 06/06/2011 06:32 PM, Russell King - ARM Linux wrote:
Hi Russell,

thanks for your time and sorry for sending a truncated kernel oops. In the other part of the thread we found that the davinci GPIO code overrides the IRQ chip data.

Quoted 6 lines
Quoted 3 lines

Unless we have the Code: line we don't know what the code being executed was. It's important that folk always report the entire oops dump, not extracts from it that they _think_ may be relevant.

is it still useful to send a complete oops, or do you think the issue is fully understood?

It is. Letting chip data point to something which is not expected by the generic chip is definitly causing that issue.

Thanks,

tglx

#15 lore raw
In reply to Russell King - ARM Linux
Quoted 16 lines · Russell King - ARM Linux
Quoted 15 lines · Thomas Gleixner

0000014c <irq_gc_mask_clr_bit>:

 14c:   e5903014        ldr     r3, [r0, #20]
 150:   e590c000        ldr     ip, [r0]
 154:   e5932004        ldr     r2, [r3, #4]
 158:   e593100c        ldr     r1, [r3, #12]
 15c:   e062200c        rsb     r2, r2, ip
 160:   e3a0c001        mov     ip, #1
 164:   e1c1221c        bic     r2, r1, ip, lsl r2
 168:   e583200c        str     r2, [r3, #12]
 16c:   e590000c        ldr     r0, [r0, #12]
 170:   e5931000        ldr     r1, [r3]
 174:   e5903064        ldr     r3, [r0, #100]  ; 0x64
 178:   e7812003        str     r2, [r1, r3]
 17c:   e12fff1e        bx      lr
Quoted 8 lines
Quoted 3 lines

So it's the "str r2, [r1, r3]" and obviously r1 is pointing into nirwana. r3 makes me wonder as well. We write 0x18 into ct->regs.mask because we add 0x04 to the base pointer. So where comes the 0x1c from?

Beware - I don't think you can build the file and work out what went wrong in this manner. Compilers like to change their register allocation for even the slightest difference of compiler options and preceding _unrelated_ code.

Just for fun I tried 5 different compilers and they all come up with the

     str     r2, [r1, r3]

which is the only possible culprit for the register content combo in this particular case.

Quoted 3 lines

Unless we have the Code: line we don't know what the code being executed was. It's important that folk always report the entire oops dump, not extracts from it that they _think_ may be relevant.

No argument.

#16 lore raw
In reply to Holger Freyther
Quoted 29 lines · Holger Freyther
Unable to handle kernel paging request at virtual address 5ffffc1b
[5ffffc1b] *pgd=00000000
Internal error: Oops: 801 [#1] PREEMPT
Modules linked in: ...
CPU: 0    Tainted: G         C   (3.0.0-rc2-00006-gff5f4d6-dirty #68)
PC is at irq_gc_mask_clr_bit+0x38/0x40
LR is at gpio_irq_handler+0x44/0xdc
pc : [<c0d6f3a4>]    lr : [<c0d29a74>]    psr: 60000093

sp : c10abe68 ip : 00000001 fp : c10abe74
r10: 00000000 r9 : c10b69d0 r8 : 00000001
r7 : fec67010 r6 : 0000ffff r5 : 00000038 r4 : 00000038 r3 : 0000001c r2 : 00000400 r1 : 5ffffbff r0 : c70050f0
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: 85cfc000 DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc10aa270)

Stack: (0xc10abe68 to 0xc10ac000)

be60: c10abeac c10abe78 c0d29a74 c0d6f37c c0d3c028 c0d2c79c
be80: 00000000 c10c0b80 00000038 00000000 00000002 00000001 c10aa000 00000000
bea0: c10abebc c10abeb0 c0d6c0d8 c0d29a40 c10abedc c10abec0 c0d1e038 c0d6c0b4
bec0: 60000013 ffffffff fec48000 60000013 c10abf8c c10abee0 c0d1eb4c c0d1e010
bee0: 00000001 00000000 0075287d 00000000 c10b52e0 c10b52e0 60000013 60000013
bf00: ffff9214 c10b4508 00000000 c10abf8c c10abec0 c10abf28 c0d56068 c0d610a4
bf20: 60000013 ffffffff 00000002 c0d20e24 c0d20e44 00000004 4f482f80 ffff920b
bf40: 055d4a80 00000000 4f6b9d83 00000004 c10b0d00 60000013 4f6b9d83 00000004
bf60: 00000000 c10aa000 c10cc2c4 c10b0514 c10b0698 80004000 41069265 80021e74
bf80: c10abfac c10abf90 c0d2003c c0d60dc8 00000002 c10cc220 c0d1924c c11dc480
bfa0: c10abfc4 c10abfb0 c0fd10fc c0d20014 00000000 c10ac8f4 c10abff4 c10abfc8
bfc0: c00089c4 c0fd1088 c00083d4 00000000 00000000 c0d1924c 00053175 c10ac010
bfe0: c0d19248 c10b050c 00000000 c10abff8 8000803c c0008740 00000000 00000000

Quoted 8 lines
Backtrace:
[<c0d6f36c>] (irq_gc_mask_clr_bit+0x0/0x40) from [<c0d29a74>]
(gpio_irq_handler+0x44/0xdc)
[<c0d29a30>] (gpio_irq_handler+0x0/0xdc) from [<c0d6c0d8>]
(generic_handle_irq+0x34/0x48)
[<c0d6c0a4>] (generic_handle_irq+0x0/0x48) from [<c0d1e038>]
(asm_do_IRQ+0x38/0x8c)
[<c0d1e000>] (asm_do_IRQ+0x0/0x8c) from [<c0d1eb4c>] (__irq_svc+0x4c/0x90)

Where is the code line for this oops dump?