Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stm32l152re: hard-fault unless power-cycled after flash, or depending on optimization #11820

Closed
fjmolinas opened this issue Jul 9, 2019 · 23 comments · Fixed by #11919
Closed
Labels
Area: pm Area: (Low) power management Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@fjmolinas
Copy link
Contributor

fjmolinas commented Jul 9, 2019

Description

#8518 introduced a _NOP() after _WFI to avoid a hard-fault because of the loss of a register content after exiting sleep (or at least that was the best possible cause at the time).

When #11159 was introduced the way pm_set_lowest() was called changed because pm_set() was now implemented for STM32L1, this made the problem re-appear. At the time the PR was merged I did not see the bug because I was using a different version of gcc and for some reason the bug didn't appear under those conditions. I did see it when @aabadie tested #11489 and I switched compiler versions.

As for what I can tell the hardfault still occurs at the same point.

There are many puzzling things about this:

  • I did not have the issue when using different gcc versions (see bottom of post).
  • The issue appears after flashing but if a power-cycle is performed (soft reset is not enough), the issue is gone.
  • I also checked out cpu/cortexm: add __NOP(); after __WFI(); for stm32l152 to avoid hardfault #8518 and removed the change it introduced and I get the exact same behavior, if a power-cycle is performed, no hardfault!
  • Removing the NOP solves the issue now...

Summing the possible fixes I have found:

  1. remove the _NOP()
  2. add more _NOP() after and even after irqs are enabled again
  3. change optimization from -Os to -O2
  4. declare state as volatile (when calling irq_disable())
  5. lower optimization to O2 only for cortexm_sleep

I've tried to find a proper formal explanation for this but I haven't succeeded. I'm specially intrigued by the fact that after a power-cycle it works again I feel that could be an important insight into the problem #8518 tried to fix.

I can post the disassembly for any of these solutions, I haven't been able to find something that could explain it (but I'm not very knowledged in assembler).

Steps to reproduce the issue

Checkout upstream master and flash an application (different than hello-world), I usually do:

make -C tests/xtimer_usleep BOARD=nucleo-l152re clean -j3 flash term

2019-07-09 11:47:35,335 - INFO # �main(): This is RIOT! (Version: 2019.10-devel-11-gecdcc-HEAD)
2019-07-09 11:47:35,337 - INFO # Running test 5 times with 7 distinct sleep times
2019-07-09 11:47:35,341 - INFO # Please hit any key and then ENTER to continue
a
2019-07-09 11:47:36,301 - INFO # *** RIOT kernel panic:
2019-07-09 11:47:36,307 - INFO # Context before hardfault:

Plug and un-plug the board (or remove IDD jumper on nucleo boards), in the same terminal and without flashing again run the application again:


2019-07-09 11:48:03,429 - INFO # Please hit any key and then ENTER to continue

2019-07-09 11:48:04,868 - INFO # �main(): This is RIOT! (Version: 2019.10-devel-11-gecdcc-HEAD)
2019-07-09 11:48:04,876 - INFO # Running test 5 times with 7 distinct sleep times
2019-07-09 11:48:04,877 - INFO # Please hit any key and then ENTER to continue
a
2019-07-09 11:48:06,340 - INFO # Slept for 10023 us (expected: 10000 us) Offset: 23 us
2019-07-09 11:48:06,393 - INFO # Slept for 50023 us (expected: 50000 us) Offset: 23 us
2019-07-09 11:48:06,411 - INFO # Slept for 10257 us (expected: 10234 us) Offset: 23 us
2019-07-09 11:48:06,472 - INFO # Slept for 56803 us (expected: 56780 us) Offset: 23 us

It works as expected.

Expected results

Should work fine after flashing with or without power-cycle.

Actual results

Crashes unless complete power-cycle is performed.

Versions

No issues toolchain:

Operating System Environment
-----------------------------
       Operating System: "Ubuntu" "16.04.2 LTS (Xenial Xerus)"
                 Kernel: Linux 4.13.0-32-generic x86_64 x86_64

Installed compiler toolchains
-----------------------------
             native gcc: gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609
      arm-none-eabi-gcc: arm-none-eabi-gcc (15:4.9.3+svn231177-1) 4.9.3 20150529 (prerelease)
                avr-gcc: missing
       mips-mti-elf-gcc: missing
             msp430-gcc: msp430-gcc (GCC) 4.6.3 20120301 (mspgcc LTS 20120406 unpatched)
   riscv-none-embed-gcc: missing
   xtensa-esp32-elf-gcc: missing
   xtensa-lx106-elf-gcc: missing
                  clang: missing

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: "2.2.0"
    mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
xtensa-esp32-elf-newlib: missing
xtensa-lx106-elf-newlib: missing
               avr-libc: missing (missing)

Installed development tools
---------------------------
                  cmake: cmake version 3.5.1
               cppcheck: missing
                doxygen: 1.8.11
                 flake8: missing
                    git: git version 2.7.4
                   make: GNU Make 4.1
                openocd: Open On-Chip Debugger 0.10.0+dev-00166-g0e4fbfb (2017-07-05-19:14)
                 python: Python 3.6.8
                python2: Python 2.7.12
                python3: Python 3.6.8
             coccinelle: missing

Issues toolchain:

Operating System Environment
-----------------------------
       Operating System: "Ubuntu" "18.04.2 LTS (Bionic Beaver)"
                 Kernel: Linux 4.18.0-25-generic x86_64 x86_64

Installed compiler toolchains
-----------------------------
             native gcc: gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
      arm-none-eabi-gcc: arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 8-2018-q4-major) 8.2.1 20181213 (release) [gcc-8-branch revision 267074]
                avr-gcc: missing
       mips-mti-elf-gcc: missing
             msp430-gcc: missing
   riscv-none-embed-gcc: missing
   xtensa-esp32-elf-gcc: missing
   xtensa-lx106-elf-gcc: missing
                  clang: missing

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: "3.0.0"
    mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
xtensa-esp32-elf-newlib: missing
xtensa-lx106-elf-newlib: missing
               avr-libc: missing (missing)

Installed development tools
---------------------------
                  cmake: cmake version 3.14.0-rc3
               cppcheck: Cppcheck 1.82
                doxygen: 1.8.16
                 flake8: missing
                    git: git version 2.21.0
                   make: GNU Make 4.1
                openocd: Open On-Chip Debugger 0.10.0+dev-00920-g6ea43726-dirty (2019-07-05-08:43)
                 python: Python 3.6.7
                python2: Python 2.7.15rc1
                python3: Python 3.6.7
             coccinelle: missing
@fjmolinas fjmolinas added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: pm Area: (Low) power management labels Jul 9, 2019
@fjmolinas
Copy link
Contributor Author

@cladmi @kaspar030 @kYc0o you were all highly involved in #8518, would you have more clues, suggestions, or insight in the problem I could use to try and find a explanation to this?

@kaspar030
Copy link
Contributor

Do you have any clue which exact instruction causes the hard fault?

If a cold-boot mitigates the issue, it means that this is dependent on some state change that happens since the last cold boot. As this happens after flashing, I'd expect that the flash logic does something weird. Are you using openocd? Can you try st-flash?

@fjmolinas
Copy link
Contributor Author

Are you using openocd? Can you try st-flash?

@kaspar030 I'm using opencod. The issue is NOT present when flashing with st-flash! (thanks for the suggestion).

@fjmolinas
Copy link
Contributor Author

The issue is NOT present when flashing with st-flash! (thanks for the suggestion).

Flashing with st-flash does not fix the issue if it has already been flashed before with openocd without a cold boot in between, but if a cold boot is performed, subsequent flash work without a cold-boot (unless st-flash is doing one underneath?)

Also, flashing the .bin or .hex with openocd doesn't change anything, the issue is still present.

@kaspar030
Copy link
Contributor

kaspar030 commented Jul 9, 2019

@kaspar030 I'm using opencod. The issue is NOT present when flashing with st-flash! (thanks for the suggestion).

That means that openocd's flashing code puts the MCU in a state that breaks our code.

Looking at what my openocd does: /usr/share/openocd/scripts/target/stm32l1.cfg, what might be an issue is its HSI handling:

proc stm32l_enable_HSI {} {                                                                                                      
›       # Enable HSI as clock source                                                                                             
›       echo "STM32L: Enabling HSI"                                                                                              
                                                                                                                                 
›       # Set HSION in RCC_CR                                                                                                    
›       mww 0x40023800 0x00000101                                                                                                
                                                                                                                                 
›       # Set HSI as SYSCLK                                                                                                      
›       mww 0x40023808 0x00000001                                                                                                
}                                                                                                                                

Maybe some of the stmclk assumptions are not valid after openocd fiddled with the registers.
Does gdb work on your board? Can you try stepping through the stmclk code and see if it takes a different path after openocd / after cold boot?

@fjmolinas
Copy link
Contributor Author

Do you have any clue which exact instruction causes the hard fault?

According to the debug output it would be irq_restore()

2019-07-09 15:27:44,953 - INFO # Context before hardfault:
2019-07-09 15:27:44,954 - INFO #    r0: 0x00000001
2019-07-09 15:27:44,954 - INFO #    r1: 0x00000001
2019-07-09 15:27:44,955 - INFO #    r2: 0xe000ed00
2019-07-09 15:27:44,960 - INFO #    r3: 0x00000010
2019-07-09 15:27:44,960 - INFO #   r12: 0x00000000
2019-07-09 15:27:44,961 - INFO #    lr: 0x08000693
2019-07-09 15:27:44,961 - INFO #    pc: 0x08422d92
2019-07-09 15:27:44,971 - INFO #   psr: 0x61000000
2019-07-09 15:27:44,972 - INFO # 
2019-07-09 15:27:44,972 - INFO # FSR/FAR:
2019-07-09 15:27:44,972 - INFO #  CFSR: 0x00000100
2019-07-09 15:27:44,973 - INFO #  HFSR: 0x40000000
2019-07-09 15:27:44,973 - INFO #  DFSR: 0x00000008
2019-07-09 15:27:44,974 - INFO #  AFSR: 0x00000000
2019-07-09 15:27:44,974 - INFO # Misc
2019-07-09 15:27:44,974 - INFO # EXC_RET: 0xfffffffd
2019-07-09 15:27:44,979 - INFO # Attempting to reconstruct state for debugging...
2019-07-09 15:27:44,981 - INFO # In GDB:
2019-07-09 15:27:44,981 - INFO #   set $pc=0x8422d92
2019-07-09 15:27:44,984 - INFO #   frame 0
2019-07-09 15:27:44,984 - INFO #   bt
2019-07-09 15:27:44,984 - INFO # 

    at /home/francisco/workspace/RIOT/cpu/cortexm_common/vectors_cortexm.c:360
360	    __BKPT(1);
(gdb) set $pc=0x8422d92
(gdb) frame 0
#0  0x08422d92 in ?? ()
(gdb) bt
#0  0x08422d92 in ?? ()
#1  0x08000692 in irq_restore (state=1) at /home/francisco/workspace/RIOT/cpu/cortexm_common/include/vendor/cmsis_gcc.h:414
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) 

@fjmolinas
Copy link
Contributor Author

Can you try stepping through the stmclk code and see if it takes a different path after openocd / after cold boot?

When steping threw the code the registers RCC_CR RCC_CFGR have the same values. I can't find anything different yet.

BTW make reset and make debug seems to also reset the hardfault behaviour, even after a cold boot (not the case when using st-util).

@cladmi
Copy link
Contributor

cladmi commented Jul 9, 2019

Do you have info of what is at 0x08422d92 ?

@fjmolinas
Copy link
Contributor Author

Do you have info of what is at 0x08422d92 ?

In this case I think its just a corrupted addres, depending on the application I get different values. If flashing tests/xtimer_drift I get:

2019-07-09 17:38:58,314 - INFO # Attempting to reconstruct state for debugging...
2019-07-09 17:38:58,315 - INFO # In GDB:
2019-07-09 17:38:58,316 - INFO #   set $pc=0x7822d5e
2019-07-09 17:38:58,317 - INFO #   frame 0
2019-07-09 17:38:58,317 - INFO #   bt
2019-07-09 17:38:58,318 - INFO # 
2019-07-09 17:38:58,319 - INFO # ISR stack overflowed by at least 16 bytes.

Which is outside of the flash region.

@cladmi
Copy link
Contributor

cladmi commented Jul 9, 2019

Ideas for bisecting, as you said it changes with optimization

  • did you compare the generated low level code to find the difference ?
  • does it also crash when compiling only the low level with optimization (by manually adding CFLAGS in the module Makefile)

Note as I just thought about it, if there is re-ordering, a memory barrier may be necessary __asm__ volatile ("" : : : "memory");
It was the case in #9068 where waiting on a variable was ignored. It was solved through a function call instead of the mem barrier there.

@fjmolinas
Copy link
Contributor Author

@cladmi Thanks for the suggestions!

does it also crash when compiling only the low level with optimization (by manually adding CFLAGS in the module Makefile)

It does not, I lowered optimization just for cortexm_sleep() this way:

#pragma GCC push_options
#pragma GCC optimize ("O2")
static inline void cortexm_sleep(int deep)
{
    if (deep) {
        SCB->SCR |=  (SCB_SCR_SLEEPDEEP_Msk);
    }
    else {
        SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk);
    }

    /* ensure that all memory accesses have completed and trigger sleeping */
    unsigned state = irq_disable();
    __DSB();
    __WFI();
#if defined(CPU_MODEL_STM32L152RE)
    /* STM32L152RE crashes without this __NOP(). See #8518. */
    __NOP();
#endif
    irq_restore(state);
}
#pragma GCC pop_options

But it hardfaults if the __NOP() is removed.

@dylad
Copy link
Member

dylad commented Jul 10, 2019

I don't have any knowledge in ST MCU but is this issue related to this somehow ? (section 2.1.13)

@fjmolinas
Copy link
Contributor Author

I don't have any knowledge in ST MCU but is this issue related to this somehow ? (section 2.1.13)

Hi @dylad thanks for the suggestion, I had looked at this some time ago but couldn't find a reason why this would be related, because we never set RUN_PD bit so it should never be in Power Down. I will look into it again though since, the fact that flashing with openocd exposes the hardfault and st-flash doesn't might mean that bit is set somewhere else??, worth another look!

@fjmolinas
Copy link
Contributor Author

did you compare the generated low level code to find the difference ?

So far I've been looking only at the assembler code for pm_set. When comparing current code output in master with fixes 1 & 2 (I numbered the fixes) the code is pretty much the same, the only difference (bedsides the one introduced by the change in 1 &2) is that a nop is sometimes added at the end of the function disassemble, just after the last instruction.

When comparing the base case with 3 there is indeed some re-ordering but functionally the code seems to be the same.

Master:

080006a4 <pm_set>:
 80006a4:       b510            push    {r4, lr}
 80006a6:       b1a0            cbz     r0, 80006d2 <pm_set+0x2e>
 80006a8:       2801            cmp     r0, #1
 80006aa:       d028            beq.n   80006fe <pm_set+0x5a>
 80006ac:       2400            movs    r4, #0
 80006ae:       4a1b            ldr     r2, [pc, #108]  ; (800071c <pm_set+0x78>)
 80006b0:       6913            ldr     r3, [r2, #16]
 80006b2:       f023 0304       bic.w   r3, r3, #4
 80006b6:       6113            str     r3, [r2, #16]
 80006b8:       f7ff ffc6       bl      8000648 <irq_disable>
 80006bc:       f3bf 8f4f       dsb     sy
 80006c0:       bf30            wfi
 80006c2:       bf00            nop
 80006c4:       f7ff ffc8       bl      8000658 <irq_restore>
 80006c8:       b33c            cbz     r4, 800071a <pm_set+0x76>
 80006ca:       e8bd 4010       ldmia.w sp!, {r4, lr}
 80006ce:       f000 bc8b       b.w     8000fe8 <stmclk_init_sysclk>
 80006d2:       4b13            ldr     r3, [pc, #76]   ; (8000720 <pm_set+0x7c>)
 80006d4:       681a            ldr     r2, [r3, #0]
 80006d6:       f422 7203       bic.w   r2, r2, #524    ; 0x20c
 80006da:       f022 0203       bic.w   r2, r2, #3
 80006de:       601a            str     r2, [r3, #0]
 80006e0:       681a            ldr     r2, [r3, #0]
 80006e2:       f442 7203       orr.w   r2, r2, #524    ; 0x20c
 80006e6:       f042 0202       orr.w   r2, r2, #2
 80006ea:       601a            str     r2, [r3, #0]
 80006ec:       685a            ldr     r2, [r3, #4]
 80006ee:       605a            str     r2, [r3, #4]
 80006f0:       4a0a            ldr     r2, [pc, #40]   ; (800071c <pm_set+0x78>)
 80006f2:       2401            movs    r4, #1
 80006f4:       6913            ldr     r3, [r2, #16]
 80006f6:       f043 0304       orr.w   r3, r3, #4
 80006fa:       6113            str     r3, [r2, #16]
 80006fc:       e7dc            b.n     80006b8 <pm_set+0x14>
 80006fe:       4a08            ldr     r2, [pc, #32]   ; (8000720 <pm_set+0x7c>)
 8000700:       6813            ldr     r3, [r2, #0]
 8000702:       f423 7303       bic.w   r3, r3, #524    ; 0x20c
 8000706:       f023 0303       bic.w   r3, r3, #3
 800070a:       6013            str     r3, [r2, #0]
 800070c:       6813            ldr     r3, [r2, #0]
 800070e:       f443 7301       orr.w   r3, r3, #516    ; 0x204
 8000712:       f043 0301       orr.w   r3, r3, #1
 8000716:       6013            str     r3, [r2, #0]
 8000718:       e7ea            b.n     80006f0 <pm_set+0x4c>
 800071a:       bd10            pop     {r4, pc}
 800071c:       e000ed00        .word   0xe000ed00
 8000720:       40007000        .word   0x40007000

-O2:

08000704 <pm_set>:
 8000704:       b508            push    {r3, lr}
 8000706:       b370            cbz     r0, 8000766 <pm_set+0x62>
 8000708:       2801            cmp     r0, #1
 800070a:       d11d            bne.n   8000748 <pm_set+0x44>
 800070c:       4a1e            ldr     r2, [pc, #120]  ; (8000788 <pm_set+0x84>)
 800070e:       6813            ldr     r3, [r2, #0]
 8000710:       f423 7303       bic.w   r3, r3, #524    ; 0x20c
 8000714:       f023 0303       bic.w   r3, r3, #3
 8000718:       6013            str     r3, [r2, #0]
 800071a:       6813            ldr     r3, [r2, #0]
 800071c:       f443 7301       orr.w   r3, r3, #516    ; 0x204
 8000720:       f043 0301       orr.w   r3, r3, #1
 8000724:       6013            str     r3, [r2, #0]
 8000726:       4a19            ldr     r2, [pc, #100]  ; (800078c <pm_set+0x88>)
 8000728:       6913            ldr     r3, [r2, #16]
 800072a:       f043 0304       orr.w   r3, r3, #4
 800072e:       6113            str     r3, [r2, #16]
 8000730:       f7ff ffba       bl      80006a8 <irq_disable>
 8000734:       f3bf 8f4f       dsb     sy
 8000738:       bf30            wfi
 800073a:       bf00            nop
 800073c:       f7ff ffbc       bl      80006b8 <irq_restore>
 8000740:       e8bd 4008       ldmia.w sp!, {r3, lr}
 8000744:       f000 bd54       b.w     80011f0 <stmclk_init_sysclk>
 8000748:       4a10            ldr     r2, [pc, #64]   ; (800078c <pm_set+0x88>)
 800074a:       6913            ldr     r3, [r2, #16]
 800074c:       f023 0304       bic.w   r3, r3, #4
 8000750:       6113            str     r3, [r2, #16]
 8000752:       f7ff ffa9       bl      80006a8 <irq_disable>
 8000756:       f3bf 8f4f       dsb     sy
 800075a:       bf30            wfi
 800075c:       bf00            nop
 800075e:       e8bd 4008       ldmia.w sp!, {r3, lr}
 8000762:       f7ff bfa9       b.w     80006b8 <irq_restore>
 8000766:       4b08            ldr     r3, [pc, #32]   ; (8000788 <pm_set+0x84>)
 8000768:       681a            ldr     r2, [r3, #0]
 800076a:       f422 7203       bic.w   r2, r2, #524    ; 0x20c
 800076e:       f022 0203       bic.w   r2, r2, #3
 8000772:       601a            str     r2, [r3, #0]
 8000774:       681a            ldr     r2, [r3, #0]
 8000776:       f442 7203       orr.w   r2, r2, #524    ; 0x20c
 800077a:       f042 0202       orr.w   r2, r2, #2
 800077e:       601a            str     r2, [r3, #0]
 8000780:       685a            ldr     r2, [r3, #4]
 8000782:       605a            str     r2, [r3, #4]
 8000784:       e7cf            b.n     8000726 <pm_set+0x22>
 8000786:       bf00            nop
 8000788:       40007000        .word   0x40007000
 800078c:       e000ed00        .word   0xe000ed00

@fjmolinas
Copy link
Contributor Author

@cladmi I'm quite puzzled at why make reset with openocd restores the hardfault behaviour but flashing different applications with st-flash doesn't (or debugging with st-util). what exactly happens in openocd when -c 'init' is called?

@cladmi
Copy link
Contributor

cladmi commented Jul 10, 2019

For me it looks like it moved the if (deep) part from before disabling interrupts to after. I will look into it.

@cladmi
Copy link
Contributor

cladmi commented Jul 10, 2019

And no idea about how openocd works… It is something I lack for a long time and never managed to compensate.

@fjmolinas
Copy link
Contributor Author

For me it looks like it moved the if (deep) part from before disabling interrupts to after. I will look into it.

I don't think that is the case, to me SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk); is between 8000728-800072e and SCB->SCR |= (SCB_SCR_SLEEPDEEP_Msk); is between 800074a-8000750, in both cases before disabling interrupts (unless I have misunderstood part of the assembly code).

@fjmolinas
Copy link
Contributor Author

fjmolinas commented Jul 10, 2019

@kaspar030 @cladmi I think I found the culprit. Openocd has the following difference with st-flash:

$_TARGETNAME configure -event examine-end {
	# DBGMCU_CR |= DBG_STANDBY | DBG_STOP | DBG_SLEEP
	mmw 0xE0042004 0x00000007 0

	# Stop watchdog counters during halt
	# DBGMCU_APB1_FZ |= DBG_IWDG_STOP | DBG_WWDG_STOP
	mmw 0xE0042008 0x00001800 0
}

And interestingly:

DBGMCU_CR register
Address: 0xE004 2004
Only 32-bit access supported
POR Reset: 0x0000 0000 (not reset by system reset)

Which explains why it works after POR, and why openocd triggers the hardfault. When removing mmw 0xE0042004 0x00000007 0 from the openocd config it now works at every time. DBG_STOP, DBG_SLEEP and DBG_SLEEP have to do with which clock is used.

Alternatively I can also add on start up I add DBGMCU->CR = 0.

I both cases all issues are gone. :)

@fjmolinas
Copy link
Contributor Author

I'm looking into how to implement this in the best way so these bit are only set when debugging. There is also something to be done in the stmclk sequence to handle the debugging setup.

@cladmi
Copy link
Contributor

cladmi commented Jul 10, 2019

For me it looks like it moved the if (deep) part from before disabling interrupts to after. I will look into it.

I don't think that is the case, to me SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk); is between 8000728-800072e and SCB->SCR |= (SCB_SCR_SLEEPDEEP_Msk); is between 800074a-8000750, in both cases before disabling interrupts (unless I have misunderstood part of the assembly code).

Indeed, I read a bit the output with more details, and the code PWR_CR_REG &= ~(PM_STOP_CONFIG | PM_STANDBY_CONFIG); moved but it looks like with -O2 it is inlined instead of being called and put at the end or something.

I tried blindly adding many __asm__ volatile ("" : : : "memory"); and it did not really change the generated code. Except removing the nop in the -O2 case :(


For my analysis, I used objdump on the object with all the annotations options I found :D

arm-none-eabi-objdump -D -e -a -g -S tests/xtimer_usleep/bin/nucleo-l152re/stm32_common_periph/pm.o | sed -n '/<pm_set>:/,/Disassembly of section/p'

Comparing building normally and with CFLAGS_OPT=-O2 BOARD=nucleo-l152re make -C tests/xtimer_usleep BOARD=nucleo-l152re clean all

master
00000000 <pm_set>:
{
   0:	b510      	push	{r4, lr}
    switch (mode) {
   2:	b1a0      	cbz	r0, 2e <pm_set+0x2e>
   4:	2801      	cmp	r0, #1
   6:	d028      	beq.n	5a <pm_set+0x5a>
            deep = 0;
   8:	2400      	movs	r4, #0
{
    if (deep) {
        SCB->SCR |=  (SCB_SCR_SLEEPDEEP_Msk);
    }
    else {
        SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk);
   a:	4a1b      	ldr	r2, [pc, #108]	; (78 <pm_set+0x78>)
   c:	6913      	ldr	r3, [r2, #16]
   e:	f023 0304 	bic.w	r3, r3, #4
  12:	6113      	str	r3, [r2, #16]
    }

    /* ensure that all memory accesses have completed and trigger sleeping */
    unsigned state = irq_disable();
  14:	f7ff fffe 	bl	0 <irq_disable>
  \details Acts as a special kind of Data Memory Barrier.
           It completes when all explicit memory accesses before this instruction complete.
 */
__STATIC_FORCEINLINE void __DSB(void)
{
  __ASM volatile ("dsb 0xF":::"memory");
  18:	f3bf 8f4f 	dsb	sy
    __DSB();
    __WFI();
  1c:	bf30      	wfi
#if defined(CPU_MODEL_STM32L152RE)
    /* STM32L152RE crashes without this __NOP(). See #8518. */
    __NOP();
  1e:	bf00      	nop
#endif
    irq_restore(state);
  20:	f7ff fffe 	bl	0 <irq_restore>
            break;
    }

    cortexm_sleep(deep);

    if (deep) {
  24:	b33c      	cbz	r4, 76 <pm_set+0x76>
        /* Re-init clock after STOP */
        stmclk_init_sysclk();
    }
}
  26:	e8bd 4010 	ldmia.w	sp!, {r4, lr}
        stmclk_init_sysclk();
  2a:	f7ff bffe 	b.w	0 <stmclk_init_sysclk>
            PWR_CR_REG &= ~(PM_STOP_CONFIG | PM_STANDBY_CONFIG);
  2e:	4b13      	ldr	r3, [pc, #76]	; (7c <pm_set+0x7c>)
  30:	681a      	ldr	r2, [r3, #0]
  32:	f422 7203 	bic.w	r2, r2, #524	; 0x20c
  36:	f022 0203 	bic.w	r2, r2, #3
  3a:	601a      	str	r2, [r3, #0]
            PWR_CR_REG |= PM_STANDBY_CONFIG;
  3c:	681a      	ldr	r2, [r3, #0]
  3e:	f442 7203 	orr.w	r2, r2, #524	; 0x20c
  42:	f042 0202 	orr.w	r2, r2, #2
  46:	601a      	str	r2, [r3, #0]
            PWR_WUP_REG |= PM_EWUP_CONFIG;
  48:	685a      	ldr	r2, [r3, #4]
  4a:	605a      	str	r2, [r3, #4]
        SCB->SCR |=  (SCB_SCR_SLEEPDEEP_Msk);
  4c:	4a0a      	ldr	r2, [pc, #40]	; (78 <pm_set+0x78>)
  4e:	2401      	movs	r4, #1
  50:	6913      	ldr	r3, [r2, #16]
  52:	f043 0304 	orr.w	r3, r3, #4
  56:	6113      	str	r3, [r2, #16]
  58:	e7dc      	b.n	14 <pm_set+0x14>
            PWR_CR_REG &= ~(PM_STOP_CONFIG | PM_STANDBY_CONFIG);
  5a:	4a08      	ldr	r2, [pc, #32]	; (7c <pm_set+0x7c>)
  5c:	6813      	ldr	r3, [r2, #0]
  5e:	f423 7303 	bic.w	r3, r3, #524	; 0x20c
  62:	f023 0303 	bic.w	r3, r3, #3
  66:	6013      	str	r3, [r2, #0]
            PWR_CR_REG |= PM_STOP_CONFIG;
  68:	6813      	ldr	r3, [r2, #0]
  6a:	f443 7301 	orr.w	r3, r3, #516	; 0x204
  6e:	f043 0301 	orr.w	r3, r3, #1
  72:	6013      	str	r3, [r2, #0]
  74:	e7ea      	b.n	4c <pm_set+0x4c>
}
  76:	bd10      	pop	{r4, pc}
  78:	e000ed00 	and	lr, r0, r0, lsl #26
  7c:	40007000 	andmi	r7, r0, r0

Disassembly of section .text.pm_off:
CFLAGS_OPT=-O2
00000000 <pm_set>:
{
   0:	b508      	push	{r3, lr}
    switch (mode) {
   2:	b370      	cbz	r0, 62 <pm_set+0x62>
   4:	2801      	cmp	r0, #1
   6:	d11d      	bne.n	44 <pm_set+0x44>
            PWR_CR_REG &= ~(PM_STOP_CONFIG | PM_STANDBY_CONFIG);
   8:	4a1e      	ldr	r2, [pc, #120]	; (84 <pm_set+0x84>)
   a:	6813      	ldr	r3, [r2, #0]
   c:	f423 7303 	bic.w	r3, r3, #524	; 0x20c
  10:	f023 0303 	bic.w	r3, r3, #3
  14:	6013      	str	r3, [r2, #0]
            PWR_CR_REG |= PM_STOP_CONFIG;
  16:	6813      	ldr	r3, [r2, #0]
  18:	f443 7301 	orr.w	r3, r3, #516	; 0x204
  1c:	f043 0301 	orr.w	r3, r3, #1
  20:	6013      	str	r3, [r2, #0]
 * @param[in] deep      !=0 for deep sleep, 0 for light sleep
 */
static inline void cortexm_sleep(int deep)
{
    if (deep) {
        SCB->SCR |=  (SCB_SCR_SLEEPDEEP_Msk);
  22:	4a19      	ldr	r2, [pc, #100]	; (88 <pm_set+0x88>)
  24:	6913      	ldr	r3, [r2, #16]
  26:	f043 0304 	orr.w	r3, r3, #4
  2a:	6113      	str	r3, [r2, #16]
    else {
        SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk);
    }

    /* ensure that all memory accesses have completed and trigger sleeping */
    unsigned state = irq_disable();
  2c:	f7ff fffe 	bl	0 <irq_disable>
  \details Acts as a special kind of Data Memory Barrier.
           It completes when all explicit memory accesses before this instruction complete.
 */
__STATIC_FORCEINLINE void __DSB(void)
{
  __ASM volatile ("dsb 0xF":::"memory");
  30:	f3bf 8f4f 	dsb	sy
    __DSB();
    __WFI();
  34:	bf30      	wfi
#if defined(CPU_MODEL_STM32L152RE)
    /* STM32L152RE crashes without this __NOP(). See #8518. */
    __NOP();
  36:	bf00      	nop
#endif
    irq_restore(state);
  38:	f7ff fffe 	bl	0 <irq_restore>

    if (deep) {
        /* Re-init clock after STOP */
        stmclk_init_sysclk();
    }
}
  3c:	e8bd 4008 	ldmia.w	sp!, {r3, lr}
        stmclk_init_sysclk();
  40:	f7ff bffe 	b.w	0 <stmclk_init_sysclk>
        SCB->SCR &= ~(SCB_SCR_SLEEPDEEP_Msk);
  44:	4a10      	ldr	r2, [pc, #64]	; (88 <pm_set+0x88>)
  46:	6913      	ldr	r3, [r2, #16]
  48:	f023 0304 	bic.w	r3, r3, #4
  4c:	6113      	str	r3, [r2, #16]
    unsigned state = irq_disable();
  4e:	f7ff fffe 	bl	0 <irq_disable>
  52:	f3bf 8f4f 	dsb	sy
    __WFI();
  56:	bf30      	wfi
    __NOP();
  58:	bf00      	nop
}
  5a:	e8bd 4008 	ldmia.w	sp!, {r3, lr}
    irq_restore(state);
  5e:	f7ff bffe 	b.w	0 <irq_restore>
            PWR_CR_REG &= ~(PM_STOP_CONFIG | PM_STANDBY_CONFIG);
  62:	4b08      	ldr	r3, [pc, #32]	; (84 <pm_set+0x84>)
  64:	681a      	ldr	r2, [r3, #0]
  66:	f422 7203 	bic.w	r2, r2, #524	; 0x20c
  6a:	f022 0203 	bic.w	r2, r2, #3
  6e:	601a      	str	r2, [r3, #0]
            PWR_CR_REG |= PM_STANDBY_CONFIG;
  70:	681a      	ldr	r2, [r3, #0]
  72:	f442 7203 	orr.w	r2, r2, #524	; 0x20c
  76:	f042 0202 	orr.w	r2, r2, #2
  7a:	601a      	str	r2, [r3, #0]
            PWR_WUP_REG |= PM_EWUP_CONFIG;
  7c:	685a      	ldr	r2, [r3, #4]
  7e:	605a      	str	r2, [r3, #4]
  80:	e7cf      	b.n	22 <pm_set+0x22>
  82:	bf00      	nop
  84:	40007000 	andmi	r7, r0, r0
  88:	e000ed00 	and	lr, r0, r0, lsl #26

Disassembly of section .text.pm_off:

@fjmolinas
Copy link
Contributor Author

I can confirm setting:

DBGMCU->CR &= ~(DBGMCU_CR_DBG_STANDBY | DBGMCU_CR_DBG_STOP | DBGMCU_CR_DBG_SLEEP);

Fixes the issue in all cases, but there is no debugging in sleep modes.

@cladmi
Copy link
Contributor

cladmi commented Jul 10, 2019

I remember that debugging with sleep mode could require some debugger special operation.

For the IoT-LAB tutorials we were recommending to remove wfi as it was turning the JTAG off https://www.iot-lab.info/tutorials/debugger-on-m3-nodes/
And we never put a workaround by default due to not really understanding the consequence.

It could be configured in the openocd script and they configure the CR register in this example as you do:
https://sourceforge.net/p/openocd/mailman/message/33251349/

And to connect to a sleeping device it may need to reset the device. connect_assert_srst This is why it fixes the flasher getting stuck I guess. https://visualgdb.com/tutorials/arm/stm32/sleep/

@fjmolinas
Copy link
Contributor Author

fjmolinas commented Jul 11, 2019

It could be configured in the openocd script and they configure the CR register in this example as you do:
https://sourceforge.net/p/openocd/mailman/message/33251349/

@cladmi I would propose changing the openocd config for all stm32 to add this:

# We override openocd default configuration for this event so to not enable debug
# when flashing or resetting device
$_TARGETNAME configure -event examine-end {
    # Stop watchdog counters during halt
	# DBGMCU_APB1_FZ |= DBG_IWDG_STOP | DBG_WWDG_STOP
	mmw 0xE0042008 0x00001800 0
}

$_TARGETNAME configure -event gdb-attach {
	# DBGMCU_CR |= DBG_STANDBY | DBG_STOP | DBG_SLEEP
	mmw 0xE0042004 0x00000007 0

	# DBGMCU_APB1_FZ |= DBG_IWDG_STOP | DBG_WWDG_STOP
	mmw 0xE0042008 0x00001800 0
}

$_TARGETNAME configure -event gdb-detach  {
	# DBGMCU_CR &= ~(DBG_STANDBY | DBG_STOP | DBG_SLEEP)
	mmw 0xE0042004 0x00000000 0x00000007

	# DBGMCU_APB1_FZ &= ~(DBG_IWDG_STOP | DBG_WWDG_STOP)
	mmw 0xE0042008 0x00000000 0x00001800
}

That way the DBGMCU_CR_DBG_STANDBY | DBGMCU_CR_DBG_STOP | DBGMCU_CR_DBG_SLEEP bits would only be set when needed, i.e. when debugging. I tested briefly on stm32l1.

This still wouldn't fix this issue because there is still a problem with wakeups when DBGMCU_CR_DBG_STANDBY | DBGMCU_CR_DBG_STOP | DBGMCU_CR_DBG_SLEEP are set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pm Area: (Low) power management Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
4 participants