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

STM: Fix us_ticker timestamp after deep sleep #8136

Merged

Conversation

fkjagodzinski
Copy link
Member

Description

Use the mbed_sdk_inited flag to correct the HAL_GetTick() behavior
after waking up from deep sleep mode. ticker_read_us() must not be
used to read us_ticker timestamp after waking up until the us_ticker
context is restored. More detailed description in issue #8117.

Fixes #8117

CC @jeromecoutant @c1728p9 @jamesbeyond @maciejbocianski

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@jeromecoutant
Copy link
Collaborator

@bcostm FYI

CI tests started

@@ -199,6 +200,7 @@ void hal_deepsleep(void)
#else /* TARGET_STM32L4 */
HAL_PWR_EnterSTOPMode(PWR_LOWPOWERREGULATOR_ON, PWR_STOPENTRY_WFI);
#endif /* TARGET_STM32L4 */
mbed_sdk_inited = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind to add a comment explaining why we do this here (copy your PR description for example) ? For future reference... Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry about that. 😉 Comments added as requested.

Use the `mbed_sdk_inited` flag to correct the `HAL_GetTick()` behavior
after waking up from deep sleep mode. `ticker_read_us()` must not be
used to read us_ticker timestamp after waking up until the us_ticker
context is restored. More detailed description in issue ARMmbed#8117.

Fixes ARMmbed#8117
@fkjagodzinski fkjagodzinski force-pushed the fix-stm-us_ticker_after_deepsleep branch from 09c365c to 6821556 Compare September 14, 2018 13:45
@@ -200,6 +201,10 @@ void hal_deepsleep(void)
HAL_PWR_EnterSTOPMode(PWR_LOWPOWERREGULATOR_ON, PWR_STOPENTRY_WFI);
#endif /* TARGET_STM32L4 */

/* Prevent HAL_GetTick() from using ticker_read_us() to read the
Copy link
Contributor

@LMESTM LMESTM Sep 17, 2018

Choose a reason for hiding this comment

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

What about moving restore_timer_ctx(); at this stage so that HAL_GetTick can be called in the next functions ?
Have you tried this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The following functions:

  1. ForceClockOutofDeepSleep(),
  2. SetSysClock(),
  3. restore_timer_ctx(),

have to be called in that order. If the timer context was restored earlier, it would have been reset anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok good point indeed. will approve as it is

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 17, 2018

Starting the build as oldest needs: CI PR depends on this fix

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 17, 2018

Build : SUCCESS

Build number : 3086
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8136/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Sep 17, 2018

@cmonr
Copy link
Contributor

cmonr commented Sep 17, 2018

Pausing Test CI (it was still in queue, not running) for now. Will restart later today.

@cmonr
Copy link
Contributor

cmonr commented Sep 19, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Sep 19, 2018

@0xc0170 0xc0170 merged commit 6d5cb6c into ARMmbed:master Sep 19, 2018
@fkjagodzinski fkjagodzinski deleted the fix-stm-us_ticker_after_deepsleep branch October 19, 2018 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants