Skip to content

Changes required by the ST low power ticker wrapper. #10536

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 19, 2019
Merged

Changes required by the ST low power ticker wrapper. #10536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
874ff1f
tests-mbed_hal-sleep: add exception for STM LPTIM targets.
mprse Apr 30, 2019
48fdd33
tests-mbed_drivers-lp_timeout: Rework "Zero delay" test case
mprse May 31, 2019
869585a
Ticker common layer: run interrupt reschedule if the inserted event h…
mprse May 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions TESTS/mbed_drivers/timeout/timeout_tests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,14 @@ void test_multiple(void)
template<typename T>
void test_no_wait(void)
{
Semaphore sem(0, 1);
T timeout;
timeout.attach_callback(mbed::callback(sem_callback, &sem), 0ULL);

bool acquired = sem.try_acquire();
TEST_ASSERT_TRUE(acquired);
timeout.detach();
for (int i = 0; i < 100; i++) {
Semaphore sem(0, 1);
T timeout;
timeout.attach_callback(mbed::callback(sem_callback, &sem), 0ULL);
int32_t sem_slots = sem.wait(0);
TEST_ASSERT_EQUAL(1, sem_slots);
timeout.detach();
}
}

/** Template for tests: accuracy of timeout delay
Expand Down
26 changes: 24 additions & 2 deletions TESTS/mbed_hal/sleep/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,27 @@ void deepsleep_lpticker_test()

lp_ticker_set_interrupt(next_match_timestamp);

/* On some targets like STM family boards with LPTIM enabled there is a required delay (~100 us) before we are able to
reprogram LPTIM_COMPARE register back to back. This is handled by the low level lp ticker wrapper which uses LPTIM_CMPOK interrupt.
CMPOK fires when LPTIM_COMPARE register can be safely reprogrammed again. During this period deep-sleep is locked.
This means that on these platforms we have additional interrupt (CMPOK) fired always ~100 us after programming lp ticker.
Since this interrupt wakes-up the board from the sleep we need to go to sleep after CMPOK is handled. */
TEST_ASSERT_TRUE(sleep_manager_can_deep_sleep_test_check());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this handled in the target code? Does not need to be there - scheduler schedule sleep, deep sleep would be locked, ~100us wake up, nothing to do, goes again to sleep - this time deep sleep.

Do I understand this correctly ?

Because seeing this in the common test file raises questions if a target does nto handle this internally and how to write generic sleep handling.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this a HAL test, there is no scheduler involved here I think
Here is how I commented the same for sleep manager test
https://github.com/ARMmbed/mbed-os/pull/10700/files#diff-5d28bad27b5517915180d28407cb8c79R125

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with scheduler anything above HAL, we are all good and this checks are not needed - will wake up - nothing to do, goes to sleep again (causing one false wake up only).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly !


sleep();

/* On some targets like STM family boards with LPTIM enabled an interrupt is triggered on counter rollover.
We need special handling for cases when next_match_timestamp < start_timestamp (interrupt is to be fired after rollover).
In such case after first wake-up we need to reset interrupt and go back to sleep waiting for the valid one.
NOTE: Above comment (CMPOK) applies also here.*/
#if MBED_CONF_TARGET_LPTICKER_LPTIM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this very target specific config (_LPTIM) is here in the test? Is this documented anywhere - this "unwanted" wake up from sleep

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in the case of CMPOK I was able to use a fact that while waiting on CMPOK, deep-sleep is locked, so I used sleep_manager_can_deep_sleep_test_check(). This is a generic solution for all targets. STM targets which have lp tricker driven by LPTIM will wait on sleep_manager_can_deep_sleep_test_check() about ~100us and for other targets, this extra delay will not be performed.

The extra interrupt on 0xFFFF is the second special case for STM targets which have lp tricker driven by LPTIM, but I couldn't find the generic solution for this case in the test. More information about this workaround can be found in the description of PR #10701:

In particular, the LP TIMER ticker cannot be programmed for a timestamp in the past, so we need to program a wake-up interrupt at the wrap-around (0xFFFF), then the next interrupt will be programmed again.

if ((next_match_timestamp < start_timestamp) && lp_ticker_read() < next_match_timestamp) {
lp_ticker_set_interrupt(next_match_timestamp);
wait_ns(200000);
sleep();
}
#endif

const timestamp_t wakeup_timestamp = lp_ticker_read();

sprintf(info, "Delta ticks: %u, Ticker width: %u, Expected wake up tick: %d, Actual wake up tick: %d, delay ticks: %d, wake up after ticks: %d",
Expand Down Expand Up @@ -154,11 +173,14 @@ void deepsleep_high_speed_clocks_turned_off_test()

TEST_ASSERT_TRUE_MESSAGE(sleep_manager_can_deep_sleep(), "deep sleep should not be locked");

const unsigned int us_ticks_before_sleep = us_ticker_read();

const timestamp_t wakeup_time = lp_ticker_read() + us_to_ticks(20000, lp_ticker_freq);
lp_ticker_set_interrupt(wakeup_time);

/* Wait for CMPOK */
TEST_ASSERT_TRUE(sleep_manager_can_deep_sleep_test_check());

const unsigned int us_ticks_before_sleep = us_ticker_read();

sleep();

const unsigned int us_ticks_after_sleep = us_ticker_read();
Expand Down
5 changes: 4 additions & 1 deletion hal/mbed_ticker_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,14 @@ void ticker_insert_event_us(const ticker_data_t *const ticker, ticker_event_t *o
/* if prev is NULL we're at the head */
if (prev == NULL) {
ticker->queue->head = obj;
schedule_interrupt(ticker);
} else {
prev->next = obj;
}

if (prev == NULL || timestamp <= ticker->queue->present_time) {
schedule_interrupt(ticker);
}

core_util_critical_section_exit();
}

Expand Down