STM: replace C++ low power ticker wrapper with a low level wrapper #10701

LMESTM · 2019-05-29T09:40:23Z

Description

This series of commits aims at removing the latency limitation of the LP ticker wrapper.

Current situation is that workaround from #9785 makes TICKLESS actually work without real deep sleep usage. #9785 was introduced because of latency issue with the C++ wrapper layer.

With this Pull request the C+ wrapper layer is removed and replaced by a low level wrapper in lp ticker driver of STM targets. This direction was suggested by @c1728p9 and supported by @mprse

Implementation of this low level wrapper require many specific implementation detailed in separate commits because of the way the LP TIMER HW is designed.

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. Also the LP TIMER cannot cope with programming an interrupt within the next few low power clock 32KHz cycles, corresponding to the time it takes for CMPOK flag to be set, which means new comparator value is taken into account. Those design constraints are taken into account in this new implementation.

The changes in this pull request have dependencies to a few test cases updates:
#10536 #10698 #10700

Also changes to IDLE stack size are needed:
#10702 #10781

Also this PR supersedes #10537 that will be closed. The commit from #10537 is actually included in this new PR.

This PR should also solve #9962 and it reverts #10572 by enabling back LPTIM on NUCLEO_L073RZ.

Pull request type

[ ] Fix
[ ] Refactor
[ x] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@c1728p9 @mprse @jeromecoutant

Release Notes

The test results are good and show significant improvements on latencies.
@mprse has verified that interrupt latencie is now lower than it was, allowing reliable serial communication @115200kbps on NUCLEO_L476RG target and 57600kbps on NUCLEO_L073RZ.

Complete test suites have been run on
TOOL_CHAIN=ARMC6,GCC_ARM,IAR
Executed targets : ['NUCLEO_WB55RG']
Executed targets : ['NUCLEO_L476RG']
Executed targets : ['NUCLEO_L073RZ']
And there is still a pending issue to be understood that might come from common ticker layer and was not seen before. @mprse is looking at it.

ciarmcom · 2019-05-29T11:00:43Z

@LMESTM, thank you for your changes.
@mprse @c1728p9 @ARMmbed/mbed-os-maintainers please review.

LMESTM · 2019-06-04T08:29:12Z

Updated the changes in targets.json to disable the wrapper only for STM32 targets that use the LPTIM HW (not RTC HW) and have TICKLESS enabled (L0, L4, WB, F7, H7 and few F4 boards).

mprse

LGTM

I added only a few minor suggestions.

targets/TARGET_STM/lp_ticker.c

LMESTM · 2019-06-19T11:32:01Z

@mprse @adbridge I think I addressed the comments so unless there are new comments, the needs:work label could be removed.

adbridge · 2019-06-19T14:37:19Z

@mprse could you please review the latest updates ?

mprse

Looks good

LMESTM · 2019-06-26T11:49:58Z

and rebased to solve conflicts ...

LMESTM · 2019-06-28T12:32:16Z

@adbridge @0xc0170 @mprse all preceding PRs have been merged :-)

LMESTM · 2019-06-28T12:33:51Z

Note: once this gets merged, Tickless will be fully active on the STM32 targets impacted by this PR, ie with dynamic Deep sleep entry / exit by default ...

There is an errata in LPTIM specification that explains that CMP Flag condition is not an exact match (COUNTER = MATCH) but rather a comparison (COUNTER >= MATCH). As a consequence the interrupt is firing early than expected when programing a timestamp after the 0xFFFF wrap-around. In order to work-around this issue, we implement the below work-around. In case timestamp is after the work-around, let's decide to program the CMP value to 0xFFFF, which is the wrap-around value. There would anyway be a wake-up at the time of wrap-around to let the OS update the system time. When the wrap-around interrupt happen, OS will check the current time and program again the timestamp to the proper value.

LP TICKER mbed-os wrapper needs to be disabled as it introduces too much latencies. LP TICKER wrapper has been disabled and we need to managed the HW constraints at low level: - main HW constraint is that once the comparator has been programmed once, driver cannot program it again before CMPOK HW flag is set, which takes about 3 30us cycles. To make it even more complex, the driver also needs to cope with "LP ticker workaround" See commit: LP ticker workaround There is an errata in LPTIM specification that explains that CMP Flag condition is not an exact match (COUNTER = MATCH) but rather a comparison (COUNTER >= MATCH). Also the disable interrupt is more complete now: - always check sleep manager status and restore it - remove irq_handler as comparator is always programed and might get called eventually when LP TICK is restarted - reset delayed_prog Also in set_interrupt, make sure interrupt does not fire early. If needed, we decide to slightly delay the tick to cope with the HW limitation to make sure it will fire as soon as HW is capable. Functions are called under critical section as they may be called from the IRQ handler now, not only from driver layer.

For L0/L4/H7/F7/WB targets that have tickless enabled, remove the tickless from us ticker and the delay ticks as the C++ wrapper layer is being removed and replaced by the low layer handling. For now, the few F4 targets with LPTIM are left with previous configuration as test results are showing a few instabilities not yet understood.

Now we'd rather not use this wrapper and use instead the low level wrapper implemented in this driver.

LMESTM · 2019-07-01T07:35:44Z

Rebased again to solve conflicts on targets.json ...

Thanks @c1728p9 for approving !

0xc0170 · 2019-07-01T20:38:58Z

CI started

mbed-ci · 2019-07-01T22:10:50Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

mbed-ci · 2019-07-02T09:07:27Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

bulislaw · 2019-07-02T11:48:31Z

Ready to merge?

0xc0170 · 2019-07-02T11:49:28Z

Just reviewing all needs: CI, a sec !

LMESTM · 2019-07-02T11:51:51Z

Thank you all - let's Tickless now :-)

mprse · 2019-07-03T06:29:26Z

I just came back from holidays and I can see that this one has passed CI and has been merged.
@LMESTM Congratulations! Great stuff!

LMESTM · 2019-07-03T07:41:25Z

@mprse thanks to you !

This was referenced May 29, 2019

Update HAL Sleep manager test to cope with STM32 LPTIM HW #10700

Merged

timeout_tests: do not call sleep from test thread #10698

Merged

STM32 LP ticker workaround #10537

Closed

lp timeout test fails on NUCLEO_L073RZ #9962

Closed

ciarmcom requested review from c1728p9, mprse and a team May 29, 2019 11:00

ciarmcom added the needs: review label May 29, 2019

mprse mentioned this pull request May 31, 2019

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

Merged

LMESTM mentioned this pull request Jun 3, 2019

Fixes in Sys Timer test #10744

Merged

LMESTM changed the title ~~Stm lp ticker low level wrapper~~ STM: replace C++ low power ticker wrapper with a low level wrapper Jun 3, 2019

LMESTM force-pushed the STM_lp_ticker_low_level_wrapper branch from 365dcb2 to ca90405 Compare June 4, 2019 08:23

0xc0170 requested a review from a team June 4, 2019 08:33

mprse approved these changes Jun 4, 2019

View reviewed changes

targets/TARGET_STM/lp_ticker.c Outdated Show resolved Hide resolved

targets/TARGET_STM/lp_ticker.c Outdated Show resolved Hide resolved

targets/TARGET_STM/lp_ticker.c Outdated Show resolved Hide resolved

0xc0170 added needs: preceding PR needs: work and removed needs: review labels Jun 4, 2019

LMESTM force-pushed the STM_lp_ticker_low_level_wrapper branch 2 times, most recently from 20c89e3 to 4b69cd7 Compare June 6, 2019 16:40

LMESTM mentioned this pull request Jun 6, 2019

Include TICKLESS stack size increase even without LPTICKER_DELAY_TICKS #10781

Merged

adbridge added needs: review and removed needs: work labels Jun 19, 2019

mprse approved these changes Jun 19, 2019

View reviewed changes

bulislaw approved these changes Jun 20, 2019

View reviewed changes

adbridge added needs: CI and removed needs: review labels Jun 20, 2019

LMESTM force-pushed the STM_lp_ticker_low_level_wrapper branch from 7315031 to fbb38c0 Compare June 26, 2019 11:49

c1728p9 approved these changes Jun 28, 2019

View reviewed changes

LMESTM added 8 commits July 1, 2019 09:31

Update warning now that C++ wrapper is not needed anymore

501881b

Now we'd rather not use this wrapper and use instead the low level wrapper implemented in this driver.

Update comments and fix typo

8616cd7

STM32: lp_ticker optim: remove irq_handler variable

ee95c7e

STM32: lp_ticker optim: initialize handle once only

58a4685

STM32 with low level ticker wrapper require Extra Idle stack

6331034

LMESTM force-pushed the STM_lp_ticker_low_level_wrapper branch from fbb38c0 to 6331034 Compare July 1, 2019 07:34

0xc0170 removed the needs: preceding PR label Jul 1, 2019

0xc0170 added ready for merge release-version: 5.13.1 and removed needs: CI labels Jul 2, 2019

0xc0170 merged commit a84c239 into ARMmbed:master Jul 2, 2019

0xc0170 removed the ready for merge label Jul 2, 2019

LMESTM mentioned this pull request Nov 29, 2019

STM32L486RG/MTB_ADV_WISE_1570 losing data when mbed tickless is enabled #11954

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM: replace C++ low power ticker wrapper with a low level wrapper #10701

STM: replace C++ low power ticker wrapper with a low level wrapper #10701

LMESTM commented May 29, 2019 •

edited

Loading

ciarmcom commented May 29, 2019

LMESTM commented Jun 4, 2019

mprse left a comment

LMESTM commented Jun 19, 2019

adbridge commented Jun 19, 2019

mprse left a comment

LMESTM commented Jun 26, 2019

LMESTM commented Jun 28, 2019

LMESTM commented Jun 28, 2019

LMESTM commented Jul 1, 2019

0xc0170 commented Jul 1, 2019

mbed-ci commented Jul 1, 2019

mbed-ci commented Jul 2, 2019

bulislaw commented Jul 2, 2019

0xc0170 commented Jul 2, 2019

LMESTM commented Jul 2, 2019

mprse commented Jul 3, 2019

LMESTM commented Jul 3, 2019

STM: replace C++ low power ticker wrapper with a low level wrapper #10701

STM: replace C++ low power ticker wrapper with a low level wrapper #10701

Conversation

LMESTM commented May 29, 2019 • edited Loading

Description

Pull request type

Reviewers

Release Notes

ciarmcom commented May 29, 2019

LMESTM commented Jun 4, 2019

mprse left a comment

Choose a reason for hiding this comment

LMESTM commented Jun 19, 2019

adbridge commented Jun 19, 2019

mprse left a comment

Choose a reason for hiding this comment

LMESTM commented Jun 26, 2019

LMESTM commented Jun 28, 2019

LMESTM commented Jun 28, 2019

LMESTM commented Jul 1, 2019

0xc0170 commented Jul 1, 2019

mbed-ci commented Jul 1, 2019

Test run: SUCCESS

mbed-ci commented Jul 2, 2019

Test run: SUCCESS

bulislaw commented Jul 2, 2019

0xc0170 commented Jul 2, 2019

LMESTM commented Jul 2, 2019

mprse commented Jul 3, 2019

LMESTM commented Jul 3, 2019

LMESTM commented May 29, 2019 •

edited

Loading