RTC specification with test headers (branch feature-hal-spec-rtc) #5226

c1728p9 · 2017-09-29T21:52:30Z

Add the RTC HAL API specification along with tests which verify it.

c1728p9 · 2017-09-29T21:52:43Z

This is an alternative to #5008

LMESTM · 2017-10-04T13:05:15Z

@0xc0170 @c1728p9 @bulislaw @jeromecoutant @bcostm @adustm
One request / question about RTC which does not seem to be covered up to now:
Most of the RTC HW IPs will offer the ability to program a wake-up alarm to trig an interrupt, but this capability is not offered by MBED API so I’d like this to be available
An example of such possible API can be seen here :
https://os.mbed.com/users/Sissors/code/RTC/docs/tip/classRTC.html
The reason for having this API on top of tickers based timeout is that tickers rely on 32bits or 16bits counters so that they will regularly wrap-up and cause extra wake-ups. In case of 16 bits timers, it can be up to every 2 seconds … which is a big limitation if you only want to wake-up let’s say 24 hours later.
Moreover, not all MCUs will have a Low Power Timer / Ticker HW while most or all will have a RTC alarm.
For many application, having a wake alarm based on RTC could be considered a good alternative to LP timers.

LMESTM · 2017-10-10T08:18:37Z

@0xc0170 @c1728p9 @bulislaw any feedback on above proposal ?

mbed-ci · 2017-10-10T09:52:50Z

Build : FAILURE

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

0xc0170 · 2017-10-10T10:35:06Z

@0xc0170 @c1728p9 @bulislaw any feedback on above proposal ?

This would be a new extension. What you are interested in is attach() functionality that would have one argument, 32bit value in milliseconds? Can we achieve this differently without adding a new function? How does this map to the tickless , as I assume it would be used mainly there (schedule the next event) ?

This is probably better to capture to a new issue on github as a new API request.

c1728p9 · 2017-10-10T18:10:50Z

Hi @LMESTM, that request makes sense to me, but I have a few concerns.

Devices without low power ticker

Deep sleep cannot be entered without violating RTOS timing

Devices with low power ticker

Low power ticker used for tickless so RTOS timing is not violated
rollover logic in mbed_ticker_api code will keep the low power ticker firing to prevent overflow

Because of the above bold points, I don't think there would be much benefit for making if just an attach function is added to the RTC API.

To make the RTC useful for sleep I think a new sleep mode, something like powerdown mode is needed. This would be a sleep mode that is explicitly entered by users and would power down the whole system, leaving only the RTC and GPIO to wake the system. This would allow most devices to enter even deeper sleep mode. Additionally there would be no need to leave the low power ticker on. Because this would be a fairly big change, I would also advocate for this being tracked separately as an API enhancement.

bulislaw · 2017-10-10T19:23:59Z

We will be adding powerdown/hibernate mode to mbed soonish.

mbed-ci · 2017-10-10T19:49:08Z

Build : FAILURE

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

theotherjimmy

Python looks good.

Maybe remove the () after yield?

c1728p9 · 2017-10-10T21:51:23Z

Rebased to master and removed () from yield

mbed-ci · 2017-10-10T22:35:07Z

Build : FAILURE

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

mbed-ci · 2017-10-11T16:38:44Z

Build : SUCCESS

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

Triggering tests

/test mbed-os

mbed-ci · 2017-10-12T03:47:20Z

Test : FAILURE

Build number : 46
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5226/46

0xc0170 · 2017-10-12T07:43:58Z

@c1728p9 Failures are related, expected I assume? I restarted jenkins CI also

LMESTM · 2017-10-12T14:43:37Z

hi @0xc0170 @c1728p9

Devices without low power ticker
Deep sleep cannot be entered without violating RTOS timing

I don't understand this point - can you elaborate on "RTOS timing violation" ?

Devices with low power ticker
Low power ticker used for tickless so RTOS timing is not violated
rollover logic in mbed_ticker_api code will keep the low power ticker firing to prevent overflow
Because of the above bold points, I don't think there would be much benefit for making if just an attach function is added to the RTC API.

Possible advantage to reduce the number of wake-ups when the only wake-up is scheduled after a long time. Do you mean that you may need to update the RTOS time when wakeing-up ?

To make the RTC useful for sleep I think a new sleep mode, something like powerdown mode is needed. This would be a sleep mode that is explicitly entered by users and would power down the whole system, leaving only the RTC and GPIO to wake the system. This would allow most devices to enter even deeper sleep mode. Additionally there would be no need to leave the low power ticker on. Because this would be a fairly big change, I would also advocate for this being tracked separately as an API enhancement.

Same Here .. do you mean that you need to update RTOS time based on elapsed RTC time? Same could be done in the deepsleep don't you think ?

c1728p9 · 2017-10-12T16:26:38Z

I don't understand this point - can you elaborate on "RTOS timing violation" ?

Without the low power timer the RTOS won't be able to wake up the system from deep sleep. Because of this the rtos will sleep for an undefined amount of time whenever all the threads become suspended.

Possible advantage to reduce the number of wake-ups when the only wake-up is scheduled after a long time. Do you mean that you may need to update the RTOS time when wakeing-up ?

In the case where tickless is enabled, the RTOS will properly be suspended for as long as requested. The problem I mentioned is for devices with a small bit-width counter like 16 bit. To keep track of time an interrupt needs to keep firing to account for overflows. For 32 bit counters this isn't really a problem since it take a long time for an overflow to occur.

Same Here .. do you mean that you need to update RTOS time based on elapsed RTC time? Same could be done in the deepsleep don't you think ?

The RTC is intended to be a low resolution long running timer. Since its time base is in seconds it can't be used for updating the RTOS time which requires millisecond resolution.

Looking at the STM implementation of the low power ticker, it is already using the RTC. This allows going more than half an hour between wakeups with the existing low power API. Assuming deep sleep wakeup takes the worst case allow by the hal specification - 10ms this will correspond to being awake 0.00056% = 10ms / (30min + 10ms) of the time.

Using the STM32F411 as an example device, power numbers due to the extra 30 minute wakeup with the low power timer can be computed given:
Typical run mode current at 100MHz = 21.4ma
Typical stop mode current draw (under-drive mode) = 0.10mA

Theoretical current draw with 100% deep sleep = 0.1000000mA
Current draw with (100 - 0.00056)% deep sleep = 0.10mA + 21.4mA * 0.0000056 = 0.1001189mA

Because the extra current from waking up is so small compared stop mode current there isn't a big advantage to extending the RTC API to allow for the system to sleep for longer with deep sleep mode. This is because deep sleep mode requires almost all state to be maintained which prevents some of the deepest sleep modes.

The biggest advantage will be relaxing the requirement that peripherial and memory state is maintained by adding a new powerdown mode as mentioned above. With just the RTC and low speed oscillator running current draw can be much less. On the STM32F411 typical current draw in standby mode with these features enabled is 2.4uA which is 1/40th of the best case deep sleep current.

In summary, even without an RTC alarm function, deep sleep mode current draw is nearly as low as it can be, and adding this API would save very little power. The real power savings will come in the future when a new powerdown mode is added.

@LMESTM let me know what you think or if I made any math errors.

c1728p9 · 2017-10-12T18:09:58Z

Turned off the RTC for all devices. As devices conform to the new API they should re-enable RTC on this feature branch.

mbed-ci · 2017-10-12T18:17:25Z

Build : FAILURE

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

0xc0170 · 2017-10-17T13:19:02Z

There are CI failures, please resolve

0xc0170 · 2017-10-17T13:19:34Z

cc @bulislaw @pan- @scartmell-arm review please

c1728p9 · 2017-10-17T18:43:14Z

/morph build

c1728p9 · 2017-10-24T21:37:02Z

/morph build

mbed-ci · 2017-10-25T03:17:46Z

Build : FAILURE

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

bulislaw · 2017-10-25T14:34:08Z

@c1728p9 What else is there to do in this ticket? It seems that we are spinning in circles, can we just disable boards that are not working and merge it to the feature branch ASAP?

Keep the prototypes in rtc_api.h even when DEVICE_RTC is not defined. This allows devices that aren't fully compliant with the RTC API to still use the header and prototypes.

Keep the RTC code if either DEVICE_RTC or DEVICE_LOWPOWERTIMER is defined on the devices which use the RTC for both the rtc api and the low power timer api. This allows DEVICE_LOWPOWERTIMER to be enabled while DEVICE_RTC is turned off.

Add requirements, tests, an example implementation and additional function documentation to the HAL RTC API.

Turn off RTC for all devices. When support for a device is added this should be re-enabled.

c1728p9 · 2017-10-25T22:19:27Z

I cant disable the boards since CI fails when these boards aren't there. Yeah, I'm spinning in circles on this.

c1728p9 · 2017-10-25T22:19:30Z

/morph build

mbed-ci · 2017-10-25T22:36:11Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test

mbed-ci · 2017-10-26T00:56:30Z

Test : FAILURE

Build number : 151
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5226/151

c1728p9 · 2017-10-26T16:15:24Z

/morph test

mbed-ci · 2017-10-26T17:43:15Z

Test : FAILURE

Build number : 155
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5226/155

mbed-ci · 2017-10-27T01:14:52Z

Test : FAILURE

Build number : 160
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5226/160

mbed-ci · 2017-10-27T17:32:04Z

Test : SUCCESS

Build number : 167
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5226/167

bulislaw

@c1728p9 Could you put tests headers in matching place to create a 'standard'? https://github.com/ARMmbed/mbed-os/pull/5164/files did it differently than this PR.
I think it's more readable if they are in their separate tests directory, but I don't have very strong opinion.

c1728p9 · 2017-10-30T16:54:53Z

Hi @bulislaw, this PR puts the test header file along side the test itself, which should be the 'standard' way of doing things. The only reason this wasn't done in #5164 is because there aren't any tests yet so if the header files are put in a test directory without a main file they are picked up as tests that fail to build.

bulislaw · 2017-10-30T16:56:16Z

Thanks makes sense. @mprse can you move that in your PR please.

c1728p9 · 2017-10-30T23:19:03Z

This PR should be ready for merging unless any anyone has additional feedback.

bulislaw · 2017-10-31T10:08:57Z

Great job getting it all to pass!

bulislaw · 2017-11-01T10:12:27Z

Can we merge that?

theotherjimmy · 2017-11-01T14:14:31Z

Merging. No release label as it's not to master.

screamerbg · 2017-11-02T11:51:23Z

@0xc0170 @c1728p9 @theotherjimmy @bulislaw this PR has a major impact on RTC support across multiple Silicon Partners, which will also prevent using the impacted targets with Mbed Cloud.

To make this productive in the future, can you ensure that the following happens before a PR is merged:

As part of the PR description, a summary is provided about the impacted targets, silicon etc. The PR makes tons of changes and little effort to summarize this will help another 30+ ppl get to the point.
This is brought to the PE team attention so in timeline manner we can communicate this with Silicon Partners about the impact
CC the Mbed Cloud team so they understand the upcoming impact if (a) feature branch is merged and (b) the impacted targets do not receive a fix.

cc @JanneKiiskila @ChiefBureaucraticOfficer @MarceloSalazar

bulislaw · 2017-11-02T13:03:42Z

@screamerbg It's merged to a feature branch, to enable partners to add suport for the new API. We should discuss how we communicate it effectively to partners.

c1728p9 mentioned this pull request Sep 29, 2017

Doxygen for source #5213

Closed

0xc0170 added the needs: review label Oct 2, 2017

c1728p9 mentioned this pull request Oct 10, 2017

RTC specification (branch feature-hal-spec-rtc) #5008

Closed

theotherjimmy approved these changes Oct 10, 2017

View reviewed changes

c1728p9 force-pushed the rtc_specification_master_2 branch 2 times, most recently from ab9a024 to 8b1b47e Compare October 10, 2017 21:51

c1728p9 force-pushed the rtc_specification_master_2 branch from 8b1b47e to 00165c9 Compare October 11, 2017 14:23

mprse mentioned this pull request Oct 12, 2017

Update of RTC HAL API specification and tests. #5306

Merged

c1728p9 force-pushed the rtc_specification_master_2 branch from db39107 to dc42a98 Compare October 17, 2017 18:30

c1728p9 added the needs: CI label Oct 17, 2017

c1728p9 force-pushed the rtc_specification_master_2 branch from 047a2bf to 0663ade Compare October 24, 2017 21:30

c1728p9 added 4 commits October 25, 2017 17:16

Keep RTC API prototypes even when not enabled

690569f

Keep the prototypes in rtc_api.h even when DEVICE_RTC is not defined. This allows devices that aren't fully compliant with the RTC API to still use the header and prototypes.

Keep rtc code if low power timer is used

63cccd9

Keep the RTC code if either DEVICE_RTC or DEVICE_LOWPOWERTIMER is defined on the devices which use the RTC for both the rtc api and the low power timer api. This allows DEVICE_LOWPOWERTIMER to be enabled while DEVICE_RTC is turned off.

Add documentation and test the HAL RTC API

9244f70

Add requirements, tests, an example implementation and additional function documentation to the HAL RTC API.

Turn off RTC until updated

beb4ff3

Turn off RTC for all devices. When support for a device is added this should be re-enabled.

c1728p9 force-pushed the rtc_specification_master_2 branch from 0663ade to beb4ff3 Compare October 25, 2017 22:18

bulislaw reviewed Oct 30, 2017

View reviewed changes

c1728p9 removed the needs: work label Oct 31, 2017

adbridge added the needs: CI label Oct 31, 2017

theotherjimmy merged commit d312da9 into ARMmbed:feature-hal-spec-rtc Nov 1, 2017

theotherjimmy removed the needs: CI label Nov 1, 2017

RTC specification with test headers (branch feature-hal-spec-rtc) #5226

RTC specification with test headers (branch feature-hal-spec-rtc) #5226

Conversation

c1728p9 commented Sep 29, 2017

c1728p9 commented Sep 29, 2017 • edited

LMESTM commented Oct 4, 2017 • edited

LMESTM commented Oct 10, 2017

mbed-ci commented Oct 10, 2017

0xc0170 commented Oct 10, 2017

c1728p9 commented Oct 10, 2017

bulislaw commented Oct 10, 2017

mbed-ci commented Oct 10, 2017

theotherjimmy left a comment

Choose a reason for hiding this comment

c1728p9 commented Oct 10, 2017

mbed-ci commented Oct 10, 2017

mbed-ci commented Oct 11, 2017

Triggering tests

mbed-ci commented Oct 12, 2017

0xc0170 commented Oct 12, 2017

LMESTM commented Oct 12, 2017

c1728p9 commented Oct 12, 2017

c1728p9 commented Oct 12, 2017

mbed-ci commented Oct 12, 2017

0xc0170 commented Oct 17, 2017

0xc0170 commented Oct 17, 2017

c1728p9 commented Oct 17, 2017

c1728p9 commented Oct 24, 2017

mbed-ci commented Oct 25, 2017

bulislaw commented Oct 25, 2017

c1728p9 commented Oct 25, 2017 • edited

c1728p9 commented Oct 25, 2017

mbed-ci commented Oct 25, 2017

Triggering tests

mbed-ci commented Oct 26, 2017

c1728p9 commented Oct 26, 2017

mbed-ci commented Oct 26, 2017

mbed-ci commented Oct 27, 2017

mbed-ci commented Oct 27, 2017

bulislaw left a comment

Choose a reason for hiding this comment

c1728p9 commented Oct 30, 2017

bulislaw commented Oct 30, 2017

c1728p9 commented Oct 30, 2017

bulislaw commented Oct 31, 2017

bulislaw commented Nov 1, 2017

theotherjimmy commented Nov 1, 2017

screamerbg commented Nov 2, 2017

bulislaw commented Nov 2, 2017

c1728p9 commented Sep 29, 2017 •

edited

LMESTM commented Oct 4, 2017 •

edited

c1728p9 commented Oct 25, 2017 •

edited