Sleep: Disallow sleep for targets turning off the systick clock at sleep entry #5086

pan- · 2017-09-13T09:34:22Z

Description

When the RTOS is present and the tickless mode is not implemented, it is
expected that the next tick issued by the Systick timer will wake up the MCU.
However nothing prevents an implementation of the ARM architecture to gate the
systick clock signal upon sleep entry.

Therefore on those targets sleep shall be prohibited if the RTOS is present and the
tickless mode is not implemented.

To ease life of porters , a new option has been added in the device add list:
STCLK_OF_DURING_SLEEP. This option expose that the target turn of the systick
clock during sleep.

Targets which exhibit such behavior shall add this define in their device_has list.

Status

READY

Migrations

Silicon vendor has to be informed of this change and add STCLK_OF_DURING_SLEEP to the device_has list of their targets which turn of (or alter) the Systick clock at sleep entry.

YES

Related issues

#4927 and #4893

…ep entry. When the RTOS is present and the tickless mode is not implemented, it is expected that the next tick issued by the Systick timer will wake up the MCU. However nothing prevents an implementation of the ARM architecture to gate the systick clock signal upon sleep entry. Therefore on those targets sleep shall be prohibited if the RTOS is present and the tickless mode is not implemented. To ease life of porters , a new option has been added in the device add list: STCLK_OF_DURING_SLEEP. This option expose that the target turn of the systick clock during sleep. Targets which exhibit such behavior shall add this define in their device_has list.

0xc0170 · 2017-09-19T06:25:32Z

targets/targets.json

        "release_versions": ["2", "5"],
        "device_name": "nRF52832_xxAA"
    },
    "DELTA_DFBM_NQ620": {
        "supported_form_factors": ["ARDUINO"],
        "inherits": ["MCU_NRF52"],
        "macros_add": ["BOARD_PCA10040", "NRF52_PAN_12", "NRF52_PAN_15", "NRF52_PAN_58", "NRF52_PAN_55", "NRF52_PAN_54", "NRF52_PAN_31", "NRF52_PAN_30", "NRF52_PAN_51", "NRF52_PAN_36", "NRF52_PAN_53", "S132", "CONFIG_GPIO_AS_PINRESET", "BLE_STACK_SUPPORT_REQD", "SWI_DISABLE0", "NRF52_PAN_20", "NRF52_PAN_64", "NRF52_PAN_62", "NRF52_PAN_63"],
-        "device_has": ["ANALOGIN", "I2C", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPI_ASYNCH", "SPISLAVE"],
+        "device_has_had": ["ANALOGIN", "I2C", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPI_ASYNCH", "SPISLAVE"],


typo: add ?

These targets in this change-set , they extend the parent right? If we do this, add, should we clean? I assume most of them are already defined as device_has in the parent ?

If you look at the diff the parent was not defining any device_has. Now it has one which define STCLK_OFF_DURING_SLEEP hence the use of device_has_add in children,

I missed that one, then just typo :)

Can you fix that type on 3065 line? or device_has_had is correct?

0xc0170 · 2017-09-19T06:28:22Z

platform/mbed_sleep.h

@@ -123,7 +127,9 @@ __INLINE static void sleep(void)
 {
 #if !(defined(FEATURE_UVISOR) && defined(TARGET_UVISOR_SUPPORTED))
 #if DEVICE_SLEEP
+#if (MBED_CONF_RTOS_PRESENT == 0) || (DEVICE_STCLK_OFF_DURING_SLEEP == 0) || defined(MBED_TICKLESS)


Should this be more descriptive DEVICE_SYSTICKCLK..... or DEVICE_SYSTCLK.... ? If I did not read the above comment, I would be guessing what ST clk stands for.

STCLK is used in the ARM architecture manuals. I choose to stick with it in the sake of consistency but I can change it.

c1728p9

Are the target.json changes needed for the nrf52 since tickless is now turned on for this target with #4991?

theotherjimmy · 2017-09-25T15:21:11Z

@pan- could you comment?

Replace `device_has_had` by `device_has_add`

pan- · 2017-09-25T16:54:02Z

Are the target.json changes needed for the nrf52 since tickless is now turned on for this target with #4991?

I believe changes in target.json are still required even if the tickless mode is turned on on NRF52 targets because those targets stop the Systick clock during sleep and this information might be valuable latter.
Tickless presence/absence is taken into account in the sleep function.

@theotherjimmy I've addressed Martin remarks on the typo in targets.json.

nvlsianpu · 2017-09-27T08:49:10Z

@pan- I want to confirm that nRF52840 behaves similar to nRF52832 so It turn off clock for SysTick while is sleeping as well.

pan- · 2017-09-28T08:46:05Z

@nvlsianpu I've updated the PR so NRF52840 behavior during sleep is taken into account. Do you think the tickless mode as defined for the NRF52832 would work on NRF52840 ?

nvlsianpu · 2017-09-28T09:22:38Z

@pan- I think yes - it Should work out of the box. nRF52840 in many areas is just a nRF52832 on steroids :D

0xc0170 · 2017-09-29T13:22:07Z

tools/targets/lint.py

@@ -82,7 +82,7 @@ def check_inherits(dict):
                      "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT",
                      "PWMOUT", "RTC", "TRNG","SERIAL", "SERIAL_ASYNCH",
                      "SERIAL_FC", "SLEEP", "SPI", "SPI_ASYNCH", "SPISLAVE",
-                      "STORAGE"]
+                      "STORAGE", "STCLK_OFF_DURING_SLEEP"]


looking at this again, is this a proper placement for this? Is it device_ ? Wouldnt this be rather a targets config? MBED_CONF_TARGET_STCLK..... - better place/definition for this?

I think it's a device characteristic not a config.

screamerbg · 2017-10-03T11:14:18Z

@0xc0170 What is gating this PR?

0xc0170 · 2017-10-06T13:04:37Z

@bulislaw @c1728p9 happy with this patch?

0xc0170 · 2017-10-09T09:05:21Z

/morph test-nightly

mbed-bot · 2017-10-09T20:42:11Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1552

All builds and test passed!

mbed-ci · 2017-10-09T21:07:06Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

pan- · 2017-10-10T12:36:24Z

Will this be part of 5.6.3 ?

0xc0170 · 2017-10-10T13:01:26Z

Will this be part of 5.6.3 ?

Yes, it should be

mbed-ci · 2017-10-10T14:37:16Z

Build : ABORTED

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

studavekar · 2017-10-11T17:23:31Z

This actually had passed nightly test, So build was aborted and marked the status as passed.

mbed-ci · 2017-10-11T17:38:55Z

Build : FAILURE

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

pan- requested review from c1728p9 and 0xc0170 September 13, 2017 09:35

theotherjimmy added the needs: review label Sep 14, 2017

pan- mentioned this pull request Sep 15, 2017

Blinky doesn't work when using develop/release profile with external power #5070

Closed

0xc0170 changed the title ~~Sleep: Disallow sleep for targets turning of the systick clock at sleep entry.~~ Sleep: Disallow sleep for targets turning off the systick clock at sleep entry Sep 19, 2017

0xc0170 reviewed Sep 19, 2017

View reviewed changes

c1728p9 reviewed Sep 21, 2017

View reviewed changes

Fix typo in DELTA_DFBM_NQ620 target

d76a5a7

Replace `device_has_had` by `device_has_add`

NRF52840: add the device configuration STCLK_OFF_DURING_SLEEP

6cb1a23

0xc0170 reviewed Sep 29, 2017

View reviewed changes

0xc0170 approved these changes Oct 6, 2017

View reviewed changes

c1728p9 approved these changes Oct 6, 2017

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Oct 9, 2017

theotherjimmy added ready for merge and removed needs: CI labels Oct 9, 2017

theotherjimmy merged commit a67737f into ARMmbed:master Oct 13, 2017

theotherjimmy added release-version: 5.6.3 and removed ready for merge labels Oct 13, 2017

pan- deleted the systick_sleep_fix branch July 3, 2018 11:04

Sleep: Disallow sleep for targets turning off the systick clock at sleep entry #5086

Sleep: Disallow sleep for targets turning off the systick clock at sleep entry #5086

Uh oh!

Conversation

pan- commented Sep 13, 2017

Description

Status

Migrations

Related issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c1728p9 left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Sep 25, 2017

Uh oh!

pan- commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvlsianpu commented Sep 27, 2017

Uh oh!

pan- commented Sep 28, 2017

Uh oh!

nvlsianpu commented Sep 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

screamerbg commented Oct 3, 2017

Uh oh!

0xc0170 commented Oct 6, 2017

Uh oh!

0xc0170 commented Oct 9, 2017

Uh oh!

mbed-bot commented Oct 9, 2017

Output

Uh oh!

mbed-ci commented Oct 9, 2017

Triggering tests

Uh oh!

pan- commented Oct 10, 2017

Uh oh!

0xc0170 commented Oct 10, 2017

Uh oh!

mbed-ci commented Oct 10, 2017

Uh oh!

studavekar commented Oct 11, 2017

Uh oh!

mbed-ci commented Oct 11, 2017

Uh oh!

Uh oh!

pan- commented Sep 25, 2017 •

edited

Loading