ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.68k stars 2.99k forks source link

STM32L0 MCU hang after some deepsleep/wakeup cycles #5830

Closed atoy40 closed 6 years ago

atoy40 commented 6 years ago

Description


Bug

Target NUCLEO_L073RZ

Toolchain: GCC_ARM

mbed-os sha: eca67ca7d (HEAD, tag: mbed-os-5.7.2, tag: latest, origin/mbed-os-5.7)

Problem

A gpio is used as an interrupt to wakeup after a deep sleep. After some sleep/wakeup cycle, the processor hangs (it can hangs after less than ten, or a lot more, no identified "rules").

#include "mbed.h"

DigitalOut led(LED1);
InterruptIn btn(PC_13);

void callback(void) {
    led = 1 - led;
}

int main() {
    btn.rise(&callback);
    while(1) {
        sleep();
    }
}

Initially, I've try the exact same code with a LowPowerTicker instead of the InteruptIn. It gives the exact same behavior.

0xc0170 commented 6 years ago

@ARMmbed/team-st-mcd please review

LMESTM commented 6 years ago

@atoy40 thanks for reporting the issue @0xc0170 with new sleep architecture, is it expected that sleep() is called from main thread ? Shouldn't it be called directly from background loop when thread don't have anything to do ? Basically, doing wait for a long time in the main instead of sleep() should result in the same sleep mode right ?

0xc0170 commented 6 years ago

@0xc0170 with new sleep architecture, is it expected that sleep() is called from main thread ? Shouldn't it be called directly from background loop when thread don't have anything to do ? Basically, doing wait for a long time in the main instead of sleep() should result in the same sleep mode right ?

I overlooked the sleep there ! True should not be. In case it is, it does not provide time for other threads, go to sleep immediately. I assume this should be functional though.

@atoy40 Please read https://os.mbed.com/docs/v5.7/reference/sleep-manager.html Instead of sleep, use wait function.

atoy40 commented 6 years ago

@0xc0170 @LMESTM one more information to add : I've tested the exact same code with a "mbedlib" (rev 159 ) project, it's had the same behavior : seems dead after some deepsleep/wakeup And stop me if I'm wrong, but if I take a look into the wait code in mbed5.7, it locks for deepsleep before calling Thread::wait. So when you ask to test with wait, you mean Thread::wait ? Thanks.

[update] I've just read the code of the default idle_hook (https://github.com/ARMmbed/mbed-os/blob/master/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp#L223) and it seems to lock deepsleep also, true ?

LMESTM commented 6 years ago

@atoy40 yes I think we should better use wait(100) that sleep now. wait would end up in Thread::wait indeed I think. I agree that this doesn't mean it would solve your issue - I'll have to test, but cannot do it now.

atoy40 commented 6 years ago

@LMESTM yes, but I think Thread::wait() - and so wait() - will never put the STM into deep sleep (STM STOP mode) as the idle code lock it and so the sample at the bottom of https://os.mbed.com/docs/v5.7/reference/sleep-manager.html sound false to me as it suggests it works (or may be I misundestood something in the code :))

0xc0170 commented 6 years ago

@0xc0170 @LMESTM one more information to add : I've tested the exact same code with a "mbedlib" (rev 159 ) project, it's had the same behavior : seems dead after some deepsleep/wakeup

Using mbed 2 then. sleep there looks fine then and you can ignore sleep manager as that one is involved in mbed OS 5.

https://os.mbed.com/docs/v5.7/reference/sleep-manager.html sound false to me as it suggests it works (or may be I misundestood something in the code :))

What is false?

[update] I've just read the code of the default idle_hook (https://github.com/ARMmbed/mbed-os/blob/master/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp#L223) and it seems to lock deepsleep also, true ?

That is correct, as this is default ide look as it was (backward compatible), as RTOS OS timer is running and does not allow deepsleep (tickless changes things, however on a line above is a bug that I'll address soon).

The code in your initial report looks correct to me. @LMESTM Can you confirm that this should work for STM32L0

atoy40 commented 6 years ago

@0xc0170 What sound false to me is the example, where :

// Deep sleep for 1 second
    printf("Deep sleep allowed: %i\r\n", sleep_manager_can_deep_sleep());
    wait(1.0);

the comment suggests the wait call will put the MCU into deepsleep, but it's not the case as it's lock into the idle hook itself.

jeromecoutant commented 6 years ago

Hi I have tried the code from the description, and it doesn't hang for me...

jeromecoutant commented 6 years ago

Thanks to sleep() command, target goes well in deepsleep mode. With some wait command, as deepsleep is locked, target goes only in sleep mode. Without any command (only a while(1);), target stays in normal mode.

I didn't try OS2 (non RTOS).

atoy40 commented 6 years ago

@jeromecoutant can you try this one please ? :


#include "mbed.h"

DigitalOut led(LED1);
LowPowerTicker ticker;

void callback(void) {
    led = 1 - led;
}

int main() {
    ticker.attach(&callback, 2.0f);
    while(1) {
        sleep();
    }
}

and wait a bunch of minutes to see if it continues to blink ? In my case it stop (led on or off, it depends) to blink.

For your description on how mbed choose STOP/SLEEP mode, I agree with you, but imho, SLEEP mode cannot be consider as a deep sleep, this is why I think the doc sample can be misleading.

0xc0170 commented 6 years ago

For your description on how mbed choose STOP/SLEEP mode, I agree with you, but imho, SLEEP mode cannot be consider as a deep sleep, this is why I think the doc sample can be misleading.

I'll fix the documentation now. Already proposed a fix for tickless deep sleep enable.

atoy40 commented 6 years ago

@0xc0170 you mean tickless will be enable on STM based targets ? in 5.7.2 I think only some targets enabled it.

0xc0170 commented 6 years ago

5866 - discussing how to address this what you were asking (deepsleep locked by default)

jeromecoutant commented 6 years ago

Hi

can you try this one please ? : and wait a bunch of minutes to see if it continues to blink ? In my case it stop (led on or off, it depends) to blink.

Issue is confirmed... ST side... Debug is on going

Thx

jeromecoutant commented 6 years ago

@atoy40

Could you check https://github.com/ARMmbed/mbed-os/commit/3ad9b01326c9f14352b10c23e11c18ac184c41f2

In parallel, I am waiting ST driver team analysis.

Regards,

jeromecoutant commented 6 years ago

ST_INTERNAL_REF 42806

atoy40 commented 6 years ago

@jeromecoutant I've applied your patch to the 5.7.2 mbed version, and it blinks for 1 or 2 hours now :) To be sure, i've tested the unpatched version before that, and It crashed after arround 2 minutes. Can you explain to me why this bug is coming after a "looking like random" time ?

I think I undersrtand it is when the clock is reconfigure after the wakeup (and try to set PLL config, even unchanged), but why it's a problem at wakeup "N" and it was not at wakeup "N-1" ?

thanks again.

atoy40 commented 6 years ago

@jeromecoutant I've pushed the exact same patched binary (the code with the ticker) on a second L073RZ board I own, and suprise ... no blinking ! But it's not the exact same problem (related to clock too). First look, it seems it never leaves STOP mode, but after some debugging, It is. It has a problem - again - with SetSysClock(). The call stack is :

In this last function, it loops indefinitely at line 308, waiting for the HSE to be ready. The timeout test seems to have no effect, and the HSE seems to never be ready.

The only difference between boards : the second one has the STLINK part cutted.

I Hope this description will help.

Anthony.

note : I'm (as far as I can) sure it's looping, because I've added a led on/led off into the loop, and the led looks 50% dimmed.

jeromecoutant commented 6 years ago

Hi

The only difference between boards : the second one has the STLINK part cutted

HSE is STLINK clock. So as clock source has been set in targets/targets.json file as HSE or HSI:

        "clock_source": {
            "value": "USE_PLL_HSE_EXTC|USE_PLL_HSI",

Init procedure should try HSE till timeout, and then HSI is configured.

atoy40 commented 6 years ago

@jeromecoutant Yes, it should, but after a STOP, as I said, it hangs trying to setup HSE (the initial clock setup is working). I thing the problem is with the timeout test, with HAL_getTick(). It looks like it always returns the same value, so the HAL_GetTick() - tickstart never reach RCC_HSE_TIMEOUT_VALUE.

I'll try to change clock_source to "HSI only" for this target in my json configuration file.

LMESTM commented 6 years ago

@atoy40 @jeromecoutant Sounds like the ticker isn't resumed yet so that HAL_getTick() cannot be used yet ... The proposal to test HSE then switch to HSI in case of timeout is provided for convenience, but you know for sure that HSE isn't available in your final HW (without STlink), then I would suggest to specifically set and always use HSI only as clock source.

jeromecoutant commented 6 years ago

https://os.mbed.com/teams/ST/wiki/Automatic-clock-configuration

atoy40 commented 6 years ago

@LMESTM I agree, I'll fix it and tell you, but I'm confident ! @jeromecoutant Thanks for pointing the doc. A question about your commit, is it in attached to a git branch ? because github disallow fetching of specific commits.

atoy40 commented 6 years ago

Just to confirm, "cutted" PCB wakeup like a charm using HSI clock source only.

cmonr commented 6 years ago

@atoy40, @jeromecoutant's PR has just landed inside of master. Inside of your mbed-os folder, you can run git pull master to pull the changes.

Once the next patch release occurs (5.7.4), you'll be able to also run mbed update in your projects to get the changes as well.

cmonr commented 6 years ago

Feel free to reopen or open a new issue if your problem still persists.