I.MX35 GPIO IRQ + Preempt -> Oops

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi,

using 2.6.36-rc6 on i.MX35 (arm1136jf-s supported in mach-mx3/mach-cpuimx35.c), I can easily 
generate Oops when triggering several consecutive interrupts on a GPIO.

To trigger the problem, I simply launch ts_test and press the touchscreen (a TSC2007 connected to 
i.MX35 through i2c) to generate interrupts on the GPIO. This works fine for a (short) random time 
but then, I get sometimes an "undefined instruction 0" and often an "Unable to handle kernel paging 
request at virtual address" (several crash messages are attached at the end of this mail).

I tried several compilers / binutils combinations (4.3.3 + 2.18 generated using OpenEmbedded and 
4.4.1 + 2.19.51.20090709 from Codesourcery) and several kernel configurations :
- imx i2c controler/GPIO i2c controler,
- Preempt/No Preempt,
- CPU idle/No CPU idle support,
- Tickless/No Tickless,
- HRT/No HRT
and the only change which seems to solve the problem is when I disable preempt (CONFIG_PREEMPT_NONE=y).

When preempt is disabled I can tap the touchscreen for several minutes, when preempt is enabled it 
crashes in a few tenth of seconds.
When preempt is enabled, it seems that having a high CPU load increase the time to reproduce the 
problem (I have a heartbeat timer triggering a led using a GPIO on the same bank as the TSC2007's 
IRQ - this could have explained why a high CPU load changes the time to reproduce the problem if the 
problem is related to accesses to the GPIO registers but I tried to disable the led trigger and the 
problem is still present).

A similar problem was discussed here http://ns.spinics.net/lists/arm-kernel/msg75037.html

Do you have any idea of where to search for this problem's reason ?

Thanks
Eric

PS: lines length of this mail is over 80 in order to have a readable log below, sorry in advance if 
this was not a good idea.

Internal error: Oops - undefined instruction: 0 [#1] PREEMPT
last sysfs file: /sys/class/vc/vcs3/dev
Modules linked in:
CPU: 0    Not tainted  (2.6.36-rc6-00050-g4ac6ae6-dirty #33)
PC is at default_idle+0x24/0x28
LR is at default_idle+0x20/0x28
pc : [<c0026fa8>]    lr : [<c0026fa4>]    psr: 60000013
sp : c0425fc8  ip : c7be8044  fp : 00000000
r10: 8001d9a0  r9 : 4117b363  r8 : 8001da08
r7 : c0427ba0  r6 : c001ef04  r5 : c001ef08  r4 : c0424000
r3 : 00000000  r2 : c0425fc8  r1 : 00000000  r0 : 00000001
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 87b70008  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc0424268)
Stack: (0xc0425fc8 to 0xc0426000)
5fc0:                   c0026f84 c002748c c0453054 c00088f0 c00084a8 00000000
5fe0: 00000000 c001ef08 00000000 00c5387d c044b9e0 80008034 00000000 00000000
[<c0026fa8>] (default_idle+0x24/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
[<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
[<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)
Code: e3130002 1a000000 eb001f1f f1080080 (e8bd8008)
---[ end trace b72dcd3cf24a984c ]---
Kernel panic - not syncing: Attempted to kill the idle task!
[<c002b528>] (unwind_backtrace+0x0/0xe4) from [<c0317b34>] (panic+0x58/0x174)
[<c0317b34>] (panic+0x58/0x174) from [<c003b730>] (do_exit+0x68/0x61c)
[<c003b730>] (do_exit+0x68/0x61c) from [<c0029828>] (die+0x1b4/0x1e0)
[<c0029828>] (die+0x1b4/0x1e0) from [<c00251e8>] (do_undefinstr+0x154/0x174)
[<c00251e8>] (do_undefinstr+0x154/0x174) from [<c0025b44>] (__und_svc+0x44/0x60)
Exception stack(0xc0425f80 to 0xc0425fc8)
5f80: 00000001 00000000 c0425fc8 00000000 c0424000 c001ef08 c001ef04 c0427ba0
5fa0: 8001da08 4117b363 8001d9a0 00000000 c7be8044 c0425fc8 c0026fa4 c0026fa8
5fc0: 60000013 ffffffff
[<c0025b44>] (__und_svc+0x44/0x60) from [<c0026fa8>] (default_idle+0x24/0x28)
[<c0026fa8>] (default_idle+0x24/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
[<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
[<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)


Unable to handle kernel paging request at virtual address 80020054
pgd = c0004000
[80020054] *pgd=00000000
Internal error: Oops: 805 [#1] PREEMPT
last sysfs file: /sys/class/i2c-dev/i2c-0/dev
Modules linked in:
CPU: 0    Not tainted  (2.6.36-rc6-00046-g6e4172c-dirty #4)
PC is at default_idle+0x24/0x2c
LR is at default_idle+0x20/0x2c
pc : [<c002a008>]    lr : [<c002a004>]    psr: 60000013
sp : c043bfc0  ip : c0440438  fp : 00000000
r10: 8001ffb8  r9 : 4117b363  r8 : 80020020
r7 : c043daf8  r6 : c045e850  r5 : c043db04  r4 : c043a000
r3 : 00000000  r2 : 60000013  r1 : 00000000  r0 : 2625a000
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 87018008  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc043a268)
Stack: (0xc043bfc0 to 0xc043c000)
bfc0: 00000000 c002a4f8 c0465f5c c045e7c4 c0021520 c0008964 c00084c0 00000000
bfe0: 00000000 c0021520 00c5387d c045e890 c0021924 80008034 00000000 00000000
[<c002a008>] (default_idle+0x24/0x2c) from [<c002a4f8>] (cpu_idle+0x44/0x9c)
[<c002a4f8>] (cpu_idle+0x44/0x9c) from [<c0008964>] (start_kernel+0x21c/0x26c)
[<c0008964>] (start_kernel+0x21c/0x26c) from [<80008034>] (0x80008034)
Code: e3130002 1a000000 eb0020e7 f1080080 (e28dd004)
---[ end trace 6bbb53675faf5922 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
[<c002e824>] (unwind_backtrace+0x0/0xec) from [<c03318d0>] (panic+0x60/0x18c)
[<c03318d0>] (panic+0x60/0x18c) from [<c0042ecc>] (do_exit+0x64/0x634)
[<c0042ecc>] (do_exit+0x64/0x634) from [<c002cb78>] (die+0x2bc/0x2fc)
[<c002cb78>] (die+0x2bc/0x2fc) from [<c002f6b4>] (__do_kernel_fault+0x64/0x84)
[<c002f6b4>] (__do_kernel_fault+0x64/0x84) from [<c002f8ac>] (do_page_fault+0x1d8/0x1f0)
[<c002f8ac>] (do_page_fault+0x1d8/0x1f0) from [<c00282c4>] (do_DataAbort+0x34/0x94)
[<c00282c4>] (do_DataAbort+0x34/0x94) from [<c0028a4c>] (__dabt_svc+0x4c/0x80)
Exception stack(0xc043bf78 to 0xc043bfc0)
bf60:                                                       2625a000 00000000
bf80: 60000013 00000000 c043a000 c043db04 c045e850 c043daf8 80020020 4117b363
bfa0: 8001ffb8 00000000 c0440438 c043bfc0 c002a004 c002a008 60000013 ffffffff
[<c0028a4c>] (__dabt_svc+0x4c/0x80) from [<c002a008>] (default_idle+0x24/0x2c)
[<c002a008>] (default_idle+0x24/0x2c) from [<c002a4f8>] (cpu_idle+0x44/0x9c)
[<c002a4f8>] (cpu_idle+0x44/0x9c) from [<c0008964>] (start_kernel+0x21c/0x26c)
[<c0008964>] (start_kernel+0x21c/0x26c) from [<80008034>] (0x80008034)

Unable to handle kernel NULL pointer dereference at virtual address 00000085
pgd = c0004000
[00000085] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT
last sysfs file: /sys/class/vc/vcs3/dev
Modules linked in:
CPU: 0    Not tainted  (2.6.36-rc6-00050-g4ac6ae6-dirty #35)
PC is at arch_randomize_brk+0x8/0x24
LR is at default_idle+0x20/0x28
pc : [<c0026fb4>]    lr : [<c0026fa4>]    psr: 60000013
sp : c0425fc0  ip : c753e044  fp : 00000000
r10: 8001d99c  r9 : 4117b363  r8 : 8001da04
r7 : c0427ba0  r6 : c001ef04  r5 : c001ef08  r4 : 00000001
r3 : 00000000  r2 : c0425fc8  r1 : 00000000  r0 : 00000001
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 00c5387d  Table: 874e4008  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc0424268)
Stack: (0xc0425fc0 to 0xc0426000)
5fc0: c0424000 c0026fa4 c0026f84 c002748c c0453054 c00088f0 c00084a8 00000000
5fe0: 00000000 c001ef08 00000000 00c5387d c044b9e0 80008034 00000000 00000000
[<c0026fb4>] (arch_randomize_brk+0x8/0x24) from [<c0026fa4>] (default_idle+0x20/0x28)
[<c0026fa4>] (default_idle+0x20/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
[<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
[<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)
Code: f1080080 e8bd8008 e92d4010 e1a04000 (e5900084)
---[ end trace 876431db1d07c298 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
[<c002b528>] (unwind_backtrace+0x0/0xe4) from [<c0317b74>] (panic+0x58/0x174)
[<c0317b74>] (panic+0x58/0x174) from [<c003b768>] (do_exit+0x68/0x61c)
[<c003b768>] (do_exit+0x68/0x61c) from [<c0029828>] (die+0x1b4/0x1e0)
[<c0029828>] (die+0x1b4/0x1e0) from [<c002c3fc>] (__do_kernel_fault+0x64/0x84)
[<c002c3fc>] (__do_kernel_fault+0x64/0x84) from [<c002c5e0>] (do_page_fault+0x1c4/0x1d8)
[<c002c5e0>] (do_page_fault+0x1c4/0x1d8) from [<c00252d4>] (do_DataAbort+0x34/0x98)
[<c00252d4>] (do_DataAbort+0x34/0x98) from [<c0025a0c>] (__dabt_svc+0x4c/0x80)
Exception stack(0xc0425f78 to 0xc0425fc0)
5f60:                                                       00000001 00000000
5f80: c0425fc8 00000000 00000001 c001ef08 c001ef04 c0427ba0 8001da04 4117b363
5fa0: 8001d99c 00000000 c753e044 c0425fc0 c0026fa4 c0026fb4 60000013 ffffffff
[<c0025a0c>] (__dabt_svc+0x4c/0x80) from [<c0026fb4>] (arch_randomize_brk+0x8/0x24)
[<c0026fb4>] (arch_randomize_brk+0x8/0x24) from [<c0026fa4>] (default_idle+0x20/0x28)
[<c0026fa4>] (default_idle+0x20/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
[<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
[<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)

Unable to handle kernel paging request at virtual address eaf440c4
pgd = c7548000
[eaf440c4] *pgd=00000000
Internal error: Oops: 80000005 [#1] PREEMPT
last sysfs file: /sys/class/vc/vcs3/dev
Modules linked in:
CPU: 0    Not tainted  (2.6.36-rc6-00050-g4ac6ae6-dirty #39)
PC is at 0xeaf440c4
LR is at mxc_audmux_v2_configure_port+0x50/0xac
pc : [<eaf440c4>]    lr : [<c0030650>]    psr: a0000033
sp : c0421fc0  ip : c74ea044  fp : 00000000
r10: 8001dc88  r9 : 4117b363  r8 : 8001dcf0
r7 : c0423ba0  r6 : 00000000  r5 : c001f1dc  r4 : c0420000
r3 : eaf440c5  r2 : c0421fc8  r1 : 00000000  r0 : c0318094
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA Thumb  Segment kernel
Control: 00c5387d  Table: 87548008  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc0420268)
Stack: (0xc0421fc0 to 0xc0422000)
1fc0: 00000000 c0026fa4 c0026f84 c002748c c044edd4 c00088f0 c00084a8 00000000
1fe0: 00000000 c001f1dc 00000000 00c5387d c0447760 80008034 00000000 00000000
[<c0030650>] (mxc_audmux_v2_configure_port+0x50/0xac) from [<c002748c>] (cpu_idle+0x40/0x8c)
[<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
[<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)
Code: bad PC value
---[ end trace a30956c8778d07c3 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
[<c002b528>] (unwind_backtrace+0x0/0xe4) from [<c0315c54>] (panic+0x58/0x174)
[<c0315c54>] (panic+0x58/0x174) from [<c003b730>] (do_exit+0x68/0x61c)
[<c003b730>] (do_exit+0x68/0x61c) from [<c0029828>] (die+0x1b4/0x1e0)
[<c0029828>] (die+0x1b4/0x1e0) from [<c002c3fc>] (__do_kernel_fault+0x64/0x84)
[<c002c3fc>] (__do_kernel_fault+0x64/0x84) from [<c002c6f8>] (do_translation_fault+0x98/0xa4)
[<c002c6f8>] (do_translation_fault+0x98/0xa4) from [<c002523c>] (do_PrefetchAbort+0x34/0x98)
[<c002523c>] (do_PrefetchAbort+0x34/0x98) from [<c0025bb0>] (__pabt_svc+0x50/0xa0)
Exception stack(0xc0421f78 to 0xc0421fc0)
1f60:                                                       c0318094 00000000
1f80: c0421fc8 eaf440c5 c0420000 c001f1dc 00000000 c0423ba0 8001dcf0 4117b363
1fa0: 8001dc88 00000000 c74ea044 c0421fc0 c0030650 eaf440c4 a0000033 ffffffff
[<c0025bb0>] (__pabt_svc+0x50/0xa0) from [<eaf440c4>] (0xeaf440c4)

I.MX35 GPIO IRQ + Preempt -> Oops

[Russell King - ARM Linux]

On Sat, Oct 02, 2010 at 03:55:00PM +0200, Eric B?nard wrote:
> Do you have any idea of where to search for this problem's reason ?

Well...

> Internal error: Oops - undefined instruction: 0 [#1] PREEMPT
> last sysfs file: /sys/class/vc/vcs3/dev
> Modules linked in:
> CPU: 0    Not tainted  (2.6.36-rc6-00050-g4ac6ae6-dirty #33)
> PC is at default_idle+0x24/0x28
> LR is at default_idle+0x20/0x28
> pc : [<c0026fa8>]    lr : [<c0026fa4>]    psr: 60000013
> sp : c0425fc8  ip : c7be8044  fp : 00000000
> r10: 8001d9a0  r9 : 4117b363  r8 : 8001da08
> r7 : c0427ba0  r6 : c001ef04  r5 : c001ef08  r4 : c0424000
> r3 : 00000000  r2 : c0425fc8  r1 : 00000000  r0 : 00000001
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 00c5387d  Table: 87b70008  DAC: 00000017
> Process swapper (pid: 0, stack limit = 0xc0424268)
> Stack: (0xc0425fc8 to 0xc0426000)
> 5fc0:                   c0026f84 c002748c c0453054 c00088f0 c00084a8 00000000
> 5fe0: 00000000 c001ef08 00000000 00c5387d c044b9e0 80008034 00000000 00000000
> [<c0026fa8>] (default_idle+0x24/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
> [<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
> [<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)
> Code: e3130002 1a000000 eb001f1f f1080080 (e8bd8008)

This disassembles to:

   0:   e3130002        tst     r3, #2  ; 0x2
   4:   1a000000        bne     0xc
   8:   eb001f1f        bl      0x7c8c
   c:   f1080080        cpsie   i
  10:   e8bd8008        pop     {r3, pc}

Doesn't look like any undefined instructions there.

> Unable to handle kernel paging request at virtual address 80020054
> pgd = c0004000
> [80020054] *pgd=00000000
> Internal error: Oops: 805 [#1] PREEMPT
> last sysfs file: /sys/class/i2c-dev/i2c-0/dev
> Modules linked in:
> CPU: 0    Not tainted  (2.6.36-rc6-00046-g6e4172c-dirty #4)
> PC is at default_idle+0x24/0x2c
> LR is at default_idle+0x20/0x2c
> pc : [<c002a008>]    lr : [<c002a004>]    psr: 60000013
> sp : c043bfc0  ip : c0440438  fp : 00000000
> r10: 8001ffb8  r9 : 4117b363  r8 : 80020020
> r7 : c043daf8  r6 : c045e850  r5 : c043db04  r4 : c043a000
> r3 : 00000000  r2 : 60000013  r1 : 00000000  r0 : 2625a000
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 00c5387d  Table: 87018008  DAC: 00000017
> Process swapper (pid: 0, stack limit = 0xc043a268)
> Stack: (0xc043bfc0 to 0xc043c000)
> bfc0: 00000000 c002a4f8 c0465f5c c045e7c4 c0021520 c0008964 c00084c0 00000000
> bfe0: 00000000 c0021520 00c5387d c045e890 c0021924 80008034 00000000 00000000
> [<c002a008>] (default_idle+0x24/0x2c) from [<c002a4f8>] (cpu_idle+0x44/0x9c)
> [<c002a4f8>] (cpu_idle+0x44/0x9c) from [<c0008964>] (start_kernel+0x21c/0x26c)
> [<c0008964>] (start_kernel+0x21c/0x26c) from [<80008034>] (0x80008034)
> Code: e3130002 1a000000 eb0020e7 f1080080 (e28dd004)

This disassembles to:

   0:   e3130002        tst     r3, #2  ; 0x2
   4:   1a000000        bne     0xc
   8:   eb0020e7        bl      0x83ac
   c:   f1080080        cpsie   i
  10:   e28dd004        add     sp, sp, #4      ; 0x4

Doesn't make sense for that last instruction to cause a data abort.

> Unable to handle kernel NULL pointer dereference at virtual address 00000085
> pgd = c0004000
> [00000085] *pgd=00000000
> Internal error: Oops: 17 [#1] PREEMPT
> last sysfs file: /sys/class/vc/vcs3/dev
> Modules linked in:
> CPU: 0    Not tainted  (2.6.36-rc6-00050-g4ac6ae6-dirty #35)
> PC is at arch_randomize_brk+0x8/0x24
> LR is at default_idle+0x20/0x28
> pc : [<c0026fb4>]    lr : [<c0026fa4>]    psr: 60000013
> sp : c0425fc0  ip : c753e044  fp : 00000000
> r10: 8001d99c  r9 : 4117b363  r8 : 8001da04
> r7 : c0427ba0  r6 : c001ef04  r5 : c001ef08  r4 : 00000001
> r3 : 00000000  r2 : c0425fc8  r1 : 00000000  r0 : 00000001
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 00c5387d  Table: 874e4008  DAC: 00000017
> Process swapper (pid: 0, stack limit = 0xc0424268)
> Stack: (0xc0425fc0 to 0xc0426000)
> 5fc0: c0424000 c0026fa4 c0026f84 c002748c c0453054 c00088f0 c00084a8 00000000
> 5fe0: 00000000 c001ef08 00000000 00c5387d c044b9e0 80008034 00000000 00000000
> [<c0026fb4>] (arch_randomize_brk+0x8/0x24) from [<c0026fa4>] (default_idle+0x20/0x28)
> [<c0026fa4>] (default_idle+0x20/0x28) from [<c002748c>] (cpu_idle+0x40/0x8c)
> [<c002748c>] (cpu_idle+0x40/0x8c) from [<c00088f0>] (start_kernel+0x20c/0x250)
> [<c00088f0>] (start_kernel+0x20c/0x250) from [<80008034>] (0x80008034)
> Code: f1080080 e8bd8008 e92d4010 e1a04000 (e5900084)

Doesn't make sense for the PC to get into arch_randomize_brk() from
default_idle().

The common theme here looks like instruction cache corruption in
default_idle() - iow, the CPU isn't executing the code which is in
memory.

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi Russell,

Le 03/10/2010 13:41, Russell King - ARM Linux a ?crit :
> The common theme here looks like instruction cache corruption in
> default_idle() - iow, the CPU isn't executing the code which is in
> memory.
>
thanks for the analysis.
This problem seems to be related to the ARM11 bug described in page 4 of 
this PDF :
http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf?fpsp=1

ENGcm09472 ARM: WFI and interrupt problems

Description:
There are two issues:
? The behavior of the FIQ signal to the ARM11 core can cause a problem 
when exiting WFI mode. The FIQ signal toggles after being initially 
asserted, which, as ARM has confirmed, is unexpected behavior to the 
ARM11 core. ARM has stated that this is not a fully validated case
for their cores. This behavior occurs when core clocks continue to run 
and, along with particular caching and alignment schemes, can result in 
a corrupted cache line following a prefetch, as well as unexpected 
behavior in code. Also, the core can execute an instruction immediately
following the WFI instruction before servicing the FIQ. This behavior of 
FIQ is caused by the design of the interrupt controller in the 
synchronization circuit.
? The same extra pulse on the FIQ signal can cause the core to execute 
instructions immediately following the WFI, before entering the ISR. If 
an ISR executes too quickly, the FIQ/IRQ may not clear by the time the 
core returns to main code, and may enter ISR two or more times for the
same interrupt. This situation should only happen if the execution time 
of the code in the ISR that follows the initial write to the peripheral 
to clear the FIQ/IRQ, can execute in fewer than 25 hclk (AHB clock) cycles.

Projected Impact:
The first issue can result in a corrupted cache line following a 
prefetch, and thus unexpected behavior; the second issue can result in 
unexpected behavior of ISR execution.

Work Around:
The WFI routine should change the clocking mode to a 1:1 (ARM:AHB) 
ratio. This must be ensured by following the programming with dummy 
reads. On wake-up, the clocks can then be changed back to the original 
ratio.
This completely prevents the toggle on the interrupt line, and this code 
can now be located in a
cacheable region.
EXAMPLE:
mov r0, #0
ldr r1, =<clock_control_BASE>
ldr r2, [r1, #OFFSET]
orr r3, r2, #1TO1MODE
str r3, [r1, #OFFSET]
... // Delay while switch to 1:1 occurs
mcr p15, 0, r0, c7, c0, 4 //WFI
str r2, [r1, #OFFSET]
bx lr

Projected Solution:
No fix scheduled.

Eric

I.MX35 GPIO IRQ + Preempt -> Oops

[Russell King - ARM Linux]

On Sun, Oct 03, 2010 at 05:25:31PM +0200, Eric B?nard wrote:
> Hi Russell,
>
> Le 03/10/2010 13:41, Russell King - ARM Linux a ?crit :
>> The common theme here looks like instruction cache corruption in
>> default_idle() - iow, the CPU isn't executing the code which is in
>> memory.
>>
> thanks for the analysis.
> This problem seems to be related to the ARM11 bug described in page 4 of  
> this PDF :
> http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf?fpsp=1
>
> ENGcm09472 ARM: WFI and interrupt problems
>
> Description:
> There are two issues:
> ? The behavior of the FIQ signal to the ARM11 core can cause a problem  
> when exiting WFI mode.

Are you using FIQs?  The kernel normally uses IRQs unless you explicitly
do something that with FIQs.

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Le 03/10/2010 18:20, Russell King - ARM Linux a ?crit :
> On Sun, Oct 03, 2010 at 05:25:31PM +0200, Eric B?nard wrote:
>> Hi Russell,
>>
>> Le 03/10/2010 13:41, Russell King - ARM Linux a ?crit :
>>> The common theme here looks like instruction cache corruption in
>>> default_idle() - iow, the CPU isn't executing the code which is in
>>> memory.
>>>
>> thanks for the analysis.
>> This problem seems to be related to the ARM11 bug described in page 4 of
>> this PDF :
>> http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf?fpsp=1
>>
>> ENGcm09472 ARM: WFI and interrupt problems
>>
>> Description:
>> There are two issues:
>> ? The behavior of the FIQ signal to the ARM11 core can cause a problem
>> when exiting WFI mode.
>
> Are you using FIQs?  The kernel normally uses IRQs unless you explicitly
> do something that with FIQs.
>
the FIQ is used for the imx-ssi driver.

It seems that removing the cpu_do_idle() call in 
plat-mxc/include/mach/system.h "fix" the problem.

Eric

I.MX35 GPIO IRQ + Preempt -> Oops

[Uwe Kleine-König]

Hi Eric,

On Sun, Oct 03, 2010 at 07:15:21PM +0200, Eric B?nard wrote:
> Le 03/10/2010 18:20, Russell King - ARM Linux a ?crit :
>> On Sun, Oct 03, 2010 at 05:25:31PM +0200, Eric B?nard wrote:
>>> Hi Russell,
>>>
>>> Le 03/10/2010 13:41, Russell King - ARM Linux a ?crit :
>>>> The common theme here looks like instruction cache corruption in
>>>> default_idle() - iow, the CPU isn't executing the code which is in
>>>> memory.
>>>>
>>> thanks for the analysis.
>>> This problem seems to be related to the ARM11 bug described in page 4 of
>>> this PDF :
>>> http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf?fpsp=1
>>>
>>> ENGcm09472 ARM: WFI and interrupt problems
>>>
>>> Description:
>>> There are two issues:
>>> ? The behavior of the FIQ signal to the ARM11 core can cause a problem
>>> when exiting WFI mode.
>>
>> Are you using FIQs?  The kernel normally uses IRQs unless you explicitly
>> do something that with FIQs.
>>
> the FIQ is used for the imx-ssi driver.
>
> It seems that removing the cpu_do_idle() call in  
> plat-mxc/include/mach/system.h "fix" the problem.
hmmm

Does deselecting imx-ssi "fixes" the problem, too?

Do you have traffic on the ssi unit when the problem occurs?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi Uwe,

Le 04/10/2010 09:39, Uwe Kleine-K?nig a ?crit :
> On Sun, Oct 03, 2010 at 07:15:21PM +0200, Eric B?nard wrote:
>> Le 03/10/2010 18:20, Russell King - ARM Linux a ?crit :
>> the FIQ is used for the imx-ssi driver.
>>
>> It seems that removing the cpu_do_idle() call in
>> plat-mxc/include/mach/system.h "fix" the problem.
> hmmm
>
> Does deselecting imx-ssi "fixes" the problem, too?
>
not tested yet.

> Do you have traffic on the ssi unit when the problem occurs?
>
no but looking at the errata (and the i.MX31's one which is very 
similar) it's not clearly mentionned that the FIQ has to be used in 
order to trigger the problem (they talk of FIQ/IRQ).

Eric

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi Uwe,

Le 04/10/2010 10:08, Eric B?nard a ?crit :
 > Le 04/10/2010 09:39, Uwe Kleine-K?nig a ?crit :
 >> On Sun, Oct 03, 2010 at 07:15:21PM +0200, Eric B?nard wrote:
 >>> Le 03/10/2010 18:20, Russell King - ARM Linux a ?crit :
 >>> the FIQ is used for the imx-ssi driver.
 >>>
 >>> It seems that removing the cpu_do_idle() call in
 >>> plat-mxc/include/mach/system.h "fix" the problem.
 >> hmmm
 >>
 >> Does deselecting imx-ssi "fixes" the problem, too?
 >>
 > not tested yet.
 >
same problem with audio layer (and thus FIQ) disabled.

Also : same problem with Freescale's BSP (2.6.31 based) and same "fix" 
(removing the WFI).

Eric

I.MX35 GPIO IRQ + Preempt -> Oops

[Marc Reilly]

Hi,

For what it's worth...

> Also : same problem with Freescale's BSP (2.6.31 based) and same "fix"
> (removing the WFI).

My kernel won't even boot up (although I don't usually get a kernel oops, only 
very occasionally) unless i pass the jtag=on parameter (or nohlt for non 
freescale kernel). It just freezes.

I think both of those workarounds effectively bypass the cpu_do_idle() call. 

Its new hardware for me, so I haven't delved into it more than that.

Cheers
Marc

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi Marc,

Le 05/10/2010 07:06, Marc Reilly a ?crit :
> For what it's worth...
>
>> Also : same problem with Freescale's BSP (2.6.31 based) and same "fix"
>> (removing the WFI).
>
> My kernel won't even boot up (although I don't usually get a kernel oops, only
> very occasionally) unless i pass the jtag=on parameter (or nohlt for non
> freescale kernel). It just freezes.
>
> I think both of those workarounds effectively bypass the cpu_do_idle() call.
>
exactly and it seems the workaround existed in the past in Freescale's 
kernel ( TLSbo65953 is the errata number for i.MX31 ) :
http://svn.buglabs.net/svn/!source/9783/bug/trunk/bug-linux-2.6.27.2/arch/arm/mach-mx3/mxc_pm.c#359

Eric

I.MX35 GPIO IRQ + Preempt -> Oops

[Eric Bénard]

Hi,

Le 05/10/2010 09:28, Eric B?nard a ?crit :
> Hi Marc,
>
> Le 05/10/2010 07:06, Marc Reilly a ?crit :
>> For what it's worth...
>>
>>> Also : same problem with Freescale's BSP (2.6.31 based) and same "fix"
>>> (removing the WFI).
>>
>> My kernel won't even boot up (although I don't usually get a kernel
>> oops, only
>> very occasionally) unless i pass the jtag=on parameter (or nohlt for non
>> freescale kernel). It just freezes.
>>
>> I think both of those workarounds effectively bypass the cpu_do_idle()
>> call.
>>
> exactly and it seems the workaround existed in the past in Freescale's
> kernel ( TLSbo65953 is the errata number for i.MX31 ) :
> http://svn.buglabs.net/svn/!source/9783/bug/trunk/bug-linux-2.6.27.2/arch/arm/mach-mx3/mxc_pm.c#359
>
this workaround seems to fix the problem (Freescale's support just sent 
me a very similar one). And vs removing cpu_do_idle, this saves ~ 1/2 W 
when there is no intensive usage of the CPU.

Eric

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Without this exiting WFI can result in cache corruption.
Code taken from Freescale's 2.6.27 BSP and tested on i.MX35

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
 arch/arm/plat-mxc/include/mach/system.h |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
index 4acd114..1661b29 100644
--- a/arch/arm/plat-mxc/include/mach/system.h
+++ b/arch/arm/plat-mxc/include/mach/system.h
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999 ARM Limited
  *  Copyright (C) 2000 Deep Blue Solutions Ltd
- *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
+ *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,14 +22,36 @@
 
 static inline void arch_idle(void)
 {
+#if (defined (CONFIG_ARCH_MX35) || defined (CONFIG_ARCH_MX31))
+	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
+	unsigned long reg = 0;
+	__asm__ __volatile__(
+		"mrc p15, 0, %0, c1, c0, 0\n"
+		"bic %0, %0, #0x00001000\n"
+		"bic %0, %0, #0x00000004\n"
+		"mcr p15, 0, %0, c1, c0, 0\n"
+		"mov %0, #0\n"
+		"mcr p15, 0, %0, c7, c5, 0\n"
+		"mov %0, #0\n"
+		"mcr p15, 0, %0, c7, c14, 0\n"
+		"mov %0, #0\n"
+		"mcr p15, 0, %0, c7, c0, 4\n"
+		"nop\n" "nop\n" "nop\n" "nop\n"
+		"nop\n" "nop\n" "nop\n"
+		"mrc p15, 0, %0, c1, c0, 0\n"
+		"orr %0, %0, #0x00001000\n"
+		"orr %0, %0, #0x00000004\n"
+		"mcr p15, 0, %0, c1, c0, 0\n"
+		:: "r" (reg));
+#else
 #ifdef CONFIG_ARCH_MXC91231
 	if (cpu_is_mxc91231()) {
 		/* Need this to set DSM low-power mode */
 		mxc91231_prepare_idle();
 	}
 #endif
-
 	cpu_do_idle();
+#endif
 }
 
 void arch_reset(char mode, const char *cmd);
-- 
1.7.0.4

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Sascha Hauer]

Hi Eric,

On Tue, Oct 05, 2010 at 11:25:39AM +0200, Eric B?nard wrote:
> Without this exiting WFI can result in cache corruption.
> Code taken from Freescale's 2.6.27 BSP and tested on i.MX35
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  arch/arm/plat-mxc/include/mach/system.h |   26 ++++++++++++++++++++++++--
>  1 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
> index 4acd114..1661b29 100644
> --- a/arch/arm/plat-mxc/include/mach/system.h
> +++ b/arch/arm/plat-mxc/include/mach/system.h
> @@ -1,7 +1,7 @@
>  /*
>   *  Copyright (C) 1999 ARM Limited
>   *  Copyright (C) 2000 Deep Blue Solutions Ltd
> - *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
> + *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -22,14 +22,36 @@
>  
>  static inline void arch_idle(void)
>  {
> +#if (defined (CONFIG_ARCH_MX35) || defined (CONFIG_ARCH_MX31))

Please use cpu_is_mx31() || cpu_is_mx35() instead.

> +	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
> +	unsigned long reg = 0;
> +	__asm__ __volatile__(
> +		"mrc p15, 0, %0, c1, c0, 0\n"
> +		"bic %0, %0, #0x00001000\n"
> +		"bic %0, %0, #0x00000004\n"
> +		"mcr p15, 0, %0, c1, c0, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c5, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c14, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c0, 4\n"
> +		"nop\n" "nop\n" "nop\n" "nop\n"
> +		"nop\n" "nop\n" "nop\n"
> +		"mrc p15, 0, %0, c1, c0, 0\n"
> +		"orr %0, %0, #0x00001000\n"
> +		"orr %0, %0, #0x00000004\n"
> +		"mcr p15, 0, %0, c1, c0, 0\n"
> +		:: "r" (reg));
> +#else
>  #ifdef CONFIG_ARCH_MXC91231
>  	if (cpu_is_mxc91231()) {
>  		/* Need this to set DSM low-power mode */
>  		mxc91231_prepare_idle();
>  	}
>  #endif
> -
>  	cpu_do_idle();
> +#endif
>  }
>  
>  void arch_reset(char mode, const char *cmd);
> -- 
> 1.7.0.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Without this exiting WFI can result in cache corruption.
Code taken from Freescale's 2.6.27 BSP and tested on i.MX35

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
v2 : use cpu_ismx3x() and add comments.

 arch/arm/plat-mxc/include/mach/system.h |   32 ++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
index 4acd114..70e66d3 100644
--- a/arch/arm/plat-mxc/include/mach/system.h
+++ b/arch/arm/plat-mxc/include/mach/system.h
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999 ARM Limited
  *  Copyright (C) 2000 Deep Blue Solutions Ltd
- *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
+ *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -28,8 +28,34 @@ static inline void arch_idle(void)
 		mxc91231_prepare_idle();
 	}
 #endif
-
-	cpu_do_idle();
+	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
+	if (cpu_is_mx31() || cpu_is_mx35()) {
+		unsigned long reg = 0;
+		__asm__ __volatile__(
+			/* disable I and D cache */
+			"mrc p15, 0, %0, c1, c0, 0\n"
+			"bic %0, %0, #0x00001000\n"
+			"bic %0, %0, #0x00000004\n"
+			"mcr p15, 0, %0, c1, c0, 0\n"
+			/* invalidate I cache */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c5, 0\n"
+			/* clear and invalidate D cache */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c14, 0\n"
+			/* WFI */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c0, 4\n"
+			"nop\n" "nop\n" "nop\n" "nop\n"
+			"nop\n" "nop\n" "nop\n"
+			/* enable I and D cache */
+			"mrc p15, 0, %0, c1, c0, 0\n"
+			"orr %0, %0, #0x00001000\n"
+			"orr %0, %0, #0x00000004\n"
+			"mcr p15, 0, %0, c1, c0, 0\n"
+			:: "r" (reg));
+	} else
+		cpu_do_idle();
 }
 
 void arch_reset(char mode, const char *cmd);
-- 
1.7.0.4

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

Hello Eric,

On Tue, Oct 05, 2010 at 02:00:12PM +0200, Eric B?nard wrote:
> Without this exiting WFI can result in cache corruption.
> Code taken from Freescale's 2.6.27 BSP and tested on i.MX35
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
> v2 : use cpu_ismx3x() and add comments.
> 
>  arch/arm/plat-mxc/include/mach/system.h |   32 ++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
> index 4acd114..70e66d3 100644
> --- a/arch/arm/plat-mxc/include/mach/system.h
> +++ b/arch/arm/plat-mxc/include/mach/system.h
> @@ -1,7 +1,7 @@
>  /*
>   *  Copyright (C) 1999 ARM Limited
>   *  Copyright (C) 2000 Deep Blue Solutions Ltd
> - *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
> + *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -28,8 +28,34 @@ static inline void arch_idle(void)
>  		mxc91231_prepare_idle();
>  	}
>  #endif
> -
> -	cpu_do_idle();
> +	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
> +	if (cpu_is_mx31() || cpu_is_mx35()) {
> +		unsigned long reg = 0;
> +		__asm__ __volatile__(
> +			/* disable I and D cache */
> +			"mrc p15, 0, %0, c1, c0, 0\n"
> +			"bic %0, %0, #0x00001000\n"
> +			"bic %0, %0, #0x00000004\n"
> +			"mcr p15, 0, %0, c1, c0, 0\n"
> +			/* invalidate I cache */
> +			"mov %0, #0\n"
> +			"mcr p15, 0, %0, c7, c5, 0\n"
> +			/* clear and invalidate D cache */
> +			"mov %0, #0\n"
mcr doesn't change the value of %0, does it?  Then there's no need to
set it to 0 once more.

> +			"mcr p15, 0, %0, c7, c14, 0\n"
> +			/* WFI */
> +			"mov %0, #0\n"
ditto

> +			"mcr p15, 0, %0, c7, c0, 4\n"
> +			"nop\n" "nop\n" "nop\n" "nop\n"
> +			"nop\n" "nop\n" "nop\n"
> +			/* enable I and D cache */
> +			"mrc p15, 0, %0, c1, c0, 0\n"
If you spend two registers there is no need to reread this register.

> +			"orr %0, %0, #0x00001000\n"
> +			"orr %0, %0, #0x00000004\n"
> +			"mcr p15, 0, %0, c1, c0, 0\n"
> +			:: "r" (reg));
... and the s/:: "/: "=/ as I suggested earlier.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Hi Uwe,

Le 05/10/2010 20:33, Uwe Kleine-K?nig a ?crit :
>> +			/* invalidate I cache */
>> +			"mov %0, #0\n"
>> +			"mcr p15, 0, %0, c7, c5, 0\n"
>> +			/* clear and invalidate D cache */
>> +			"mov %0, #0\n"
> mcr doesn't change the value of %0, does it?  Then there's no need to
> set it to 0 once more.
>
>> +			"mcr p15, 0, %0, c7, c14, 0\n"
>> +			/* WFI */
>> +			"mov %0, #0\n"
> ditto
>
>> +			"mcr p15, 0, %0, c7, c0, 4\n"
>> +			"nop\n" "nop\n" "nop\n" "nop\n"
>> +			"nop\n" "nop\n" "nop\n"
>> +			/* enable I and D cache */
>> +			"mrc p15, 0, %0, c1, c0, 0\n"
> If you spend two registers there is no need to reread this register.
>
>> +			"orr %0, %0, #0x00001000\n"
>> +			"orr %0, %0, #0x00000004\n"
>> +			"mcr p15, 0, %0, c1, c0, 0\n"
>> +			:: "r" (reg));
> ... and the s/:: "/: "=/ as I suggested earlier.
>
well, this code comes from Freescale and if everything was perfect it 
wouldn't be necessary as this is an errata fix so I'm not sure this can 
be considered as a code to optimize.

Moreover it follows exactly what is suggested in the workaround 
described on page 8 of this PDF : 
http://cache.freescale.com/files/32bit/doc/errata/MCIMX31CE.pdf so I'm 
not sure we should modify it.

The other function they sent me had even more duplicated lines than this 
one so it's difficult to know what is necessary and what isn't.

I can test it with your suggestions but the extra mov & mrc may be there 
to add instructions cycle between each mcr to be sure the workaround 
does its job.

Best regards,
Eric

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

On Tue, Oct 05, 2010 at 09:31:27PM +0200, Eric B?nard wrote:
> Hi Uwe,
>
> Le 05/10/2010 20:33, Uwe Kleine-K?nig a ?crit :
>>> +			/* invalidate I cache */
>>> +			"mov %0, #0\n"
>>> +			"mcr p15, 0, %0, c7, c5, 0\n"
>>> +			/* clear and invalidate D cache */
>>> +			"mov %0, #0\n"
>> mcr doesn't change the value of %0, does it?  Then there's no need to
>> set it to 0 once more.
>>
>>> +			"mcr p15, 0, %0, c7, c14, 0\n"
>>> +			/* WFI */
>>> +			"mov %0, #0\n"
>> ditto
>>
>>> +			"mcr p15, 0, %0, c7, c0, 4\n"
>>> +			"nop\n" "nop\n" "nop\n" "nop\n"
>>> +			"nop\n" "nop\n" "nop\n"
>>> +			/* enable I and D cache */
>>> +			"mrc p15, 0, %0, c1, c0, 0\n"
>> If you spend two registers there is no need to reread this register.
>>
>>> +			"orr %0, %0, #0x00001000\n"
>>> +			"orr %0, %0, #0x00000004\n"
>>> +			"mcr p15, 0, %0, c1, c0, 0\n"
>>> +			:: "r" (reg));
>> ... and the s/:: "/: "=/ as I suggested earlier.
>>
> well, this code comes from Freescale and if everything was perfect it  
> wouldn't be necessary as this is an errata fix so I'm not sure this can  
> be considered as a code to optimize.
>
> Moreover it follows exactly what is suggested in the workaround  
> described on page 8 of this PDF :  
> http://cache.freescale.com/files/32bit/doc/errata/MCIMX31CE.pdf so I'm  
> not sure we should modify it.

> The other function they sent me had even more duplicated lines than this  
> one so it's difficult to know what is necessary and what isn't.
>
> I can test it with your suggestions but the extra mov & mrc may be there  
> to add instructions cycle between each mcr to be sure the workaround  
> does its job.
hmm, it's a copy of the code suggested there, yes.  The text before the
example doesn't suggest that timing is an issue for the work around.  I
interpret it that everything is fine when the caches are off before
executing the wfi instruction.  But OK, it wouldn't be the first time
hardware documentation isn't exact to the point.

Hmm, when the caches are off before entering wfi, does that mean that
all interrupt (and fiq) handlers run with the caches off, too?  That's
very bad, isn't it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Hi Uwe,

Le 05/10/2010 21:46, Uwe Kleine-K?nig a ?crit :
> hmm, it's a copy of the code suggested there, yes.  The text before the
> example doesn't suggest that timing is an issue for the work around.  I
> interpret it that everything is fine when the caches are off before
> executing the wfi instruction.  But OK, it wouldn't be the first time
> hardware documentation isn't exact to the point.
>
Well the doc says there is an other workaround (setting CPU freq = AHB 
freq, waiting clock to switch, WFI, restore original CPU freq) doesn't 
work for me and support seems not very confident with this other 
workaround as they immediatl directed me to this one ...

> Hmm, when the caches are off before entering wfi, does that mean that
> all interrupt (and fiq) handlers run with the caches off, too?  That's
> very bad, isn't it?
>
If I understand well, this case happens only when they run after WFI 
which means cpuidle was called (and thus CPU activity was null for a 
certain amount of time, which explains why it's easier to reproduce the 
problem with tslib than with qtdemo which takes 100% CPU and don't let 
cpuidle to execute very often).

That may seems bad, but I find this solution better than having an oops 
after a few IRQs which makes the CPU unusable for real life applications :-)

Eric

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

Hello Eric,

>> Hmm, when the caches are off before entering wfi, does that mean that
>> all interrupt (and fiq) handlers run with the caches off, too?  That's
>> very bad, isn't it?
>>
> If I understand well, this case happens only when they run after WFI  
> which means cpuidle was called (and thus CPU activity was null for a  
> certain amount of time, which explains why it's easier to reproduce the  
> problem with tslib than with qtdemo which takes 100% CPU and don't let  
> cpuidle to execute very often).
>
> That may seems bad, but I find this solution better than having an oops  
> after a few IRQs which makes the CPU unusable for real life applications 
> :-)
Ack, but it makes me think if the caches should be enabled in the irq
entry point, too.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Hi Uwe,

Le 05/10/2010 22:04, Uwe Kleine-K?nig a ?crit :
>> That may seems bad, but I find this solution better than having an oops
>> after a few IRQs which makes the CPU unusable for real life applications
>> :-)
> Ack, but it makes me think if the caches should be enabled in the irq
> entry point, too.

If I understand well the following link, you are right.

Adding checks to enable cache at the IRQ entry point will execute 
aditional code at each interrupt which may have more cost than executing 
one ISR after WFI without cache (I may be totally wrong here).

More details here (click c7, Cache Operations Register > The Wait For 
Interrupt operation in the left menu) :
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0211h/I1014942.html

which says :

MCR p15,0,<Rd>,c7,c0,4               ; Wait For Interrupt

This puts the processor into a low-power state and stops it executing 
following instructions until an interrupt, an imprecise external abort, 
or a debug request occurs, regardless of whether the interrupts or 
external imprecise aborts are disabled by the masks in the CPSR. When an 
interrupt does occur, the MCR instruction completes. If interrupts are 
enabled, the IRQ or FIQ handler is entered as normal. The return link in 
r14_irq or r14_fiq contains the address of the MCR instruction plus 8, 
so that the normal instruction used for interrupt return (SUBS 
PC,R14,#4) returns to the instruction following the MCR.

Eric

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Nicolas Pitre]

On Tue, 5 Oct 2010, Eric B?nard wrote:

> Hi Uwe,
> 
> Le 05/10/2010 22:04, Uwe Kleine-K?nig a ?crit :
> > > That may seems bad, but I find this solution better than having an oops
> > > after a few IRQs which makes the CPU unusable for real life applications
> > > :-)
> > Ack, but it makes me think if the caches should be enabled in the irq
> > entry point, too.
> 
> If I understand well the following link, you are right.
> 
> Adding checks to enable cache at the IRQ entry point will execute aditional
> code at each interrupt which may have more cost than executing one ISR after
> WFI without cache (I may be totally wrong here).
> 
> More details here (click c7, Cache Operations Register > The Wait For
> Interrupt operation in the left menu) :
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0211h/I1014942.html
> 
> which says :
> 
> MCR p15,0,<Rd>,c7,c0,4               ; Wait For Interrupt
> 
> This puts the processor into a low-power state and stops it executing
> following instructions until an interrupt, an imprecise external abort, or a
> debug request occurs, regardless of whether the interrupts or external
> imprecise aborts are disabled by the masks in the CPSR. When an interrupt does
> occur, the MCR instruction completes. If interrupts are enabled, the IRQ or
> FIQ handler is entered as normal. The return link in r14_irq or r14_fiq
> contains the address of the MCR instruction plus 8, so that the normal
> instruction used for interrupt return (SUBS PC,R14,#4) returns to the
> instruction following the MCR.

Note that, on Linux, the WFI instruction is always executed with IRQs 
off.  So you just have to turn on the cache right after the MCR before 
returning.


Nicolas

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Hi Nicolas,

Le 06/10/2010 04:28, Nicolas Pitre a ?crit :
> Note that, on Linux, the WFI instruction is always executed with IRQs
> off.  So you just have to turn on the cache right after the MCR before
> returning.
>
thanks for this information.

So if there is no more problem left I'll resubmit the fixed patch.

Eric

[PATCH v3] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Without this exiting WFI can result in cache corruption.
Code taken from Freescale's 2.6.27 BSP and tested on i.MX35

Signed-off-by: Eric B?nard <eric@eukrea.com>
---
- v3 : fix following UWE's review
- v2 : use cpu_ismx3x() and add comments. 

 arch/arm/plat-mxc/include/mach/system.h |   32 ++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
index 4acd114..95be51b 100644
--- a/arch/arm/plat-mxc/include/mach/system.h
+++ b/arch/arm/plat-mxc/include/mach/system.h
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999 ARM Limited
  *  Copyright (C) 2000 Deep Blue Solutions Ltd
- *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
+ *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -28,8 +28,34 @@ static inline void arch_idle(void)
 		mxc91231_prepare_idle();
 	}
 #endif
-
-	cpu_do_idle();
+	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
+	if (cpu_is_mx31() || cpu_is_mx35()) {
+		unsigned long reg = 0;
+		__asm__ __volatile__(
+			/* disable I and D cache */
+			"mrc p15, 0, %0, c1, c0, 0\n"
+			"bic %0, %0, #0x00001000\n"
+			"bic %0, %0, #0x00000004\n"
+			"mcr p15, 0, %0, c1, c0, 0\n"
+			/* invalidate I cache */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c5, 0\n"
+			/* clear and invalidate D cache */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c14, 0\n"
+			/* WFI */
+			"mov %0, #0\n"
+			"mcr p15, 0, %0, c7, c0, 4\n"
+			"nop\n" "nop\n" "nop\n" "nop\n"
+			"nop\n" "nop\n" "nop\n"
+			/* enable I and D cache */
+			"mrc p15, 0, %0, c1, c0, 0\n"
+			"orr %0, %0, #0x00001000\n"
+			"orr %0, %0, #0x00000004\n"
+			"mcr p15, 0, %0, c1, c0, 0\n"
+			: "=r" (reg));
+	} else
+		cpu_do_idle();
 }
 
 void arch_reset(char mode, const char *cmd);
-- 
1.7.0.4

[PATCH v2] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Russell King - ARM Linux]

On Tue, Oct 05, 2010 at 09:46:59PM +0200, Uwe Kleine-K?nig wrote:
> Hmm, when the caches are off before entering wfi, does that mean that
> all interrupt (and fiq) handlers run with the caches off, too?  That's
> very bad, isn't it?

IRQs are disabled before cpu_idle() calls the calling the chosen idle
function.  If we didn't, we'd miss waking up threads from idle mode.

FIQs aren't covered by the core code though - you have to deal with
any issues there yourself.

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

On Tue, Oct 05, 2010 at 11:25:39AM +0200, Eric B?nard wrote:
> Without this exiting WFI can result in cache corruption.
> Code taken from Freescale's 2.6.27 BSP and tested on i.MX35
> 
> Signed-off-by: Eric B?nard <eric@eukrea.com>
> ---
>  arch/arm/plat-mxc/include/mach/system.h |   26 ++++++++++++++++++++++++--
>  1 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/system.h b/arch/arm/plat-mxc/include/mach/system.h
> index 4acd114..1661b29 100644
> --- a/arch/arm/plat-mxc/include/mach/system.h
> +++ b/arch/arm/plat-mxc/include/mach/system.h
> @@ -1,7 +1,7 @@
>  /*
>   *  Copyright (C) 1999 ARM Limited
>   *  Copyright (C) 2000 Deep Blue Solutions Ltd
> - *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
> + *  Copyright 2004-2008 Freescale Semiconductor, Inc. All Rights Reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -22,14 +22,36 @@
>  
>  static inline void arch_idle(void)
>  {
> +#if (defined (CONFIG_ARCH_MX35) || defined (CONFIG_ARCH_MX31))
> +	/* fix i.MX31 errata TLSbo65953 and i.MX35 errata ENGcm09472 */
> +	unsigned long reg = 0;
> +	__asm__ __volatile__(
> +		"mrc p15, 0, %0, c1, c0, 0\n"
> +		"bic %0, %0, #0x00001000\n"
> +		"bic %0, %0, #0x00000004\n"
> +		"mcr p15, 0, %0, c1, c0, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c5, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c14, 0\n"
> +		"mov %0, #0\n"
> +		"mcr p15, 0, %0, c7, c0, 4\n"
> +		"nop\n" "nop\n" "nop\n" "nop\n"
> +		"nop\n" "nop\n" "nop\n"
> +		"mrc p15, 0, %0, c1, c0, 0\n"
> +		"orr %0, %0, #0x00001000\n"
> +		"orr %0, %0, #0x00000004\n"
> +		"mcr p15, 0, %0, c1, c0, 0\n"
> +		:: "r" (reg));
Isn't it possible to just say

		: "=r" (reg));

here?  (That is only a single : to make reg an output register and add
the = flag.)

> +#else
>  #ifdef CONFIG_ARCH_MXC91231
>  	if (cpu_is_mxc91231()) {
>  		/* Need this to set DSM low-power mode */
>  		mxc91231_prepare_idle();
>  	}
>  #endif
> -
>  	cpu_do_idle();
> +#endif
No.  When we start to have multi-soc support for imx and a kernel for
both mx35 and mxc91231 is compiled mxc91231_prepare_idle isn't called
anymore for mxc91231.  Ah, and cpu_do_idle isn't called anymore at all.

This needs to read:

	#if defined (CONFIG_ARCH_MX31) || defined (CONFIG_ARCH_MX35)
		if (cpu_is_mx31() || cpu_is_mx35()) {
			unsigned long reg = 0;
			__asm__ __volatile__(...);
		} else
	#endif
		{
		
#ifdef CONFIG_ARCH_MXC91231
			if (cpu_is_mxc91231()) {
				/* Need this to set DSM low-power mode */
				mxc91231_prepare_idle();
			}
#endif
			cpu_do_idle();
		}

Looks ugly.  If you come up with nice code (that still does the correct
thing) you get five extra points.  (Maybe add another function for the
last block.  I couldn't come up with a nice name though.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Eric Bénard]

Hi Uwe,

Le 05/10/2010 18:29, Uwe Kleine-K?nig a ?crit :
> No.  When we start to have multi-soc support for imx and a kernel for
> both mx35 and mxc91231 is compiled mxc91231_prepare_idle isn't called
> anymore for mxc91231.  Ah, and cpu_do_idle isn't called anymore at all.
>
isn't that patch better ?
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/027983.html

Eric

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

Hi Eric,

On Tue, Oct 05, 2010 at 06:48:35PM +0200, Eric B?nard wrote:
> Le 05/10/2010 18:29, Uwe Kleine-K?nig a ?crit :
>> No.  When we start to have multi-soc support for imx and a kernel for
>> both mx35 and mxc91231 is compiled mxc91231_prepare_idle isn't called
>> anymore for mxc91231.  Ah, and cpu_do_idle isn't called anymore at all.
>>
> isn't that patch better ?
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/027983.html
Yes, but it doesn't address all my comments.  Sorry, I only saw it when
I already sent my reply to the first version.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Daniel Mack]

On Tue, Oct 05, 2010 at 06:29:20PM +0200, Uwe Kleine-K?nig wrote:
> On Tue, Oct 05, 2010 at 11:25:39AM +0200, Eric B?nard wrote:
> > +#else
> >  #ifdef CONFIG_ARCH_MXC91231
> >  	if (cpu_is_mxc91231()) {
> >  		/* Need this to set DSM low-power mode */
> >  		mxc91231_prepare_idle();
> >  	}
> >  #endif
> > -
> >  	cpu_do_idle();
> > +#endif
> No.  When we start to have multi-soc support for imx and a kernel for
> both mx35 and mxc91231 is compiled mxc91231_prepare_idle isn't called
> anymore for mxc91231.  Ah, and cpu_do_idle isn't called anymore at all.
> 
> This needs to read:
> 
> 	#if defined (CONFIG_ARCH_MX31) || defined (CONFIG_ARCH_MX35)
> 		if (cpu_is_mx31() || cpu_is_mx35()) {
> 			unsigned long reg = 0;
> 			__asm__ __volatile__(...);

A "return" statement here would save you the else branch and
subsequently, one indentation level, right?

Also, I wonder whether the check for CONFIG_ARCH_MX3[15] is actually
needed at all, as cpu_is_mx3x() will default to 0 at compile time in
case those #defines are not set.

> 		} else
> 	#endif
> 		{
> 		
> #ifdef CONFIG_ARCH_MXC91231
> 			if (cpu_is_mxc91231()) {
> 				/* Need this to set DSM low-power mode */
> 				mxc91231_prepare_idle();
> 			}
> #endif

In this case, though, they seem mandatory due to the symbol
mxc91231_prepare_idle would be undefined otherwise.


Daniel

[PATCH/RFC] i.MX31 and i.MX35 : fix errate TLSbo65953 and ENGcm09472

[Uwe Kleine-König]

On Wed, Oct 06, 2010 at 08:35:25AM +0200, Daniel Mack wrote:
> On Tue, Oct 05, 2010 at 06:29:20PM +0200, Uwe Kleine-K?nig wrote:
> > On Tue, Oct 05, 2010 at 11:25:39AM +0200, Eric B?nard wrote:
> > > +#else
> > >  #ifdef CONFIG_ARCH_MXC91231
> > >  	if (cpu_is_mxc91231()) {
> > >  		/* Need this to set DSM low-power mode */
> > >  		mxc91231_prepare_idle();
> > >  	}
> > >  #endif
> > > -
> > >  	cpu_do_idle();
> > > +#endif
> > No.  When we start to have multi-soc support for imx and a kernel for
> > both mx35 and mxc91231 is compiled mxc91231_prepare_idle isn't called
> > anymore for mxc91231.  Ah, and cpu_do_idle isn't called anymore at all.
> > 
> > This needs to read:
> > 
> > 	#if defined (CONFIG_ARCH_MX31) || defined (CONFIG_ARCH_MX35)
> > 		if (cpu_is_mx31() || cpu_is_mx35()) {
> > 			unsigned long reg = 0;
> > 			__asm__ __volatile__(...);
> 
> A "return" statement here would save you the else branch and
> subsequently, one indentation level, right?
> 
> Also, I wonder whether the check for CONFIG_ARCH_MX3[15] is actually
> needed at all, as cpu_is_mx3x() will default to 0 at compile time in
> case those #defines are not set.
No, it's not needed, as Eric showed in his v2.  Let's continue to speak
about that one.

> 
> > 		} else
> > 	#endif
> > 		{
> > 		
> > #ifdef CONFIG_ARCH_MXC91231
> > 			if (cpu_is_mxc91231()) {
> > 				/* Need this to set DSM low-power mode */
> > 				mxc91231_prepare_idle();
> > 			}
> > #endif
> 
> In this case, though, they seem mandatory due to the symbol
> mxc91231_prepare_idle would be undefined otherwise.
yep.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |