Low power ticker wrapper layer still used when suspended

[fkjagodzinski]

Description

The LowPowerTickerWrapper class is still used to handle lp ticker overflows even after it has been disabled with a call to lp_ticker_wrapper_suspend().

mbed-os/hal/mbed_lp_ticker_wrapper.h

Lines 52 to 57 in f1587b5

	* Suspend the wrapper layer code
	*
	* Pass through all interrupts to the low power ticker and stop using
	* the microsecond ticker.
	*/
	void lp_ticker_wrapper_suspend(void);

As a result, the us ticker which is used internally by the wrapper, periodically locks deepsleep sleep mode.

Targets

NUCLEO_F746ZG

Mbed OS SHA

b1b6399 -- current master

Steps to reproduce

Check out 76fc292 (fkjagodzinski@76fc292) and use a logic analyser to capture data from D0, D1, D2 & D3.

lp_ticker_wrapper-logic_dump.zip

---

The high level result of this issue can be seen here:

#include "mbed.h"
#include "mbed_lp_ticker_wrapper.h"

#define SLEEP_DURATION_US 100000ULL
#define SERIAL_FLUSH_TIME_MS 20

void wakeup_callback(volatile int *wakeup_flag)
{
    (*wakeup_flag)++;
}

int main()
{
    // Suspend the RTOS kernel scheduler to prevent interference with duration of sleep.
    osKernelSuspend();
    // Suspend the low power ticker wrapper to prevent interference with deep sleep lock.
    lp_ticker_wrapper_suspend();

    volatile int wakeup_flag;
    LowPowerTimeout lp_timeout;

    bool deepsleep_status1 = sleep_manager_can_deep_sleep();
    sleep_manager_lock_deep_sleep();
    bool deepsleep_status2 = sleep_manager_can_deep_sleep();

    wakeup_flag = 0;
    lp_timeout.attach_us(mbed::callback(wakeup_callback, &wakeup_flag), SLEEP_DURATION_US);

    while (wakeup_flag == 0) {
        sleep_manager_sleep_auto();
    }

    bool deepsleep_status3 = sleep_manager_can_deep_sleep();
    sleep_manager_unlock_deep_sleep();
    bool deepsleep_status4 = sleep_manager_can_deep_sleep();

    printf("------------------------------------------------------------\r\n");
    printf("deepsleep_status1 = %i, %i expected\r\n", deepsleep_status1, 1);
    printf("deepsleep_status2 = %i, %i expected\r\n", deepsleep_status2, 0);
    printf("deepsleep_status3 = %i, %i expected\r\n", deepsleep_status3, 0);
    printf("deepsleep_status4 = %i, %i expected\r\n", deepsleep_status4, 1);
    printf("\r\n");
    wait_ms(SERIAL_FLUSH_TIME_MS);
}

mbed compile -t GCC_ARM -m NUCLEO_F746ZG -f --sterm

Output:

------------------------------------------------------------
deepsleep_status1 = 1, 1 expected
deepsleep_status2 = 0, 0 expected
deepsleep_status3 = 0, 0 expected
deepsleep_status4 = 0, 1 expected

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug

CC @c1728p9 @mprse @jamesbeyond

[c1728p9]

This may not be a bug with the low power ticker wrapper. To prevent interrupts you'll need to disable all the users of the LowPowerTicker wrapper before you suspend the low power ticker wrapper, otherwise the behavior is undefined. In this case its the common ticker layer which is still active and causing the interrupts you are seeing. If you suspend the common layer with a call to ticker_suspend(get_lp_ticker_data()); then you should no longer see locked deep sleep.

[adbridge]

Internal Jira reference: https://jira.arm.com/browse/IOTHAL-363

[fkjagodzinski]

(...) To prevent interrupts you'll need to disable all the users of the LowPowerTicker wrapper before you suspend the low power ticker wrapper, otherwise the behavior is undefined. (...)

Ok, I assumed it's up to the developer to decide which parts are to be suspended. Our docs are missing the part about undefined behaviour. Shall I add a note with a warning here?

In this case its the common ticker layer which is still active and causing the interrupts you are seeing.

Correct. However, I expected these interrupts to pass through unconditionally, without using the us ticker and locking a deepsleep mode. This could be fixed with #8279.

[c1728p9]

Our docs are missing the part about undefined behaviour. Shall I add a note with a warning here?

Yeah, please add this info.

However, I expected these interrupts to pass through unconditionally, without using the us ticker and locking a deepsleep mode.

The current implementation pass these interrupts through unconditionally. Its just calling into the ticker warpper layer when it is suspended which causes undefined behavior.