realtek rtl8195am SLEEP features implementation and lib updates

[M-ichae-l]

Description

1, lib updates. Sync with the latest rtl8195am SDK
2, Add sleep features. Add "sleep_ex_api.h", "sleep.c" and "sleep_api.c".
3, edit api files to fit new lib and feature. Edit "targets.json"
4, Add license under bootloader folder. Add "LICENSE"
5, rebase to resolve the conflits from #7638

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[x] Feature
[ ] Breaking change

[0xc0170]

Why the commit f4a453755d96e77b589f510e30199134a5ef647c is a squash of multiple various commits (updating lib, adding sleep, fixes, etc) ?

[M-ichae-l]

@0xc0170
This PR is a rebase version of #7638. #7638 has multiple commits. I have applied all commits for this PR with only one commit (the first commit f4a453755d96e77b589f510e30199134a5ef647c).

[0xc0170]

Can we split them as you have done them ? The description lists the changes accordingly (I would assume you worked from 1st to 2nd, etc.) so should also commits (it is very hard to review 80 file changes within one commit when they are squashed like this)

Thanks for rebasing

[M-ichae-l]

Let me edit the description and point out the file I have changed for each commit. Except for the first lib updates, others only changed few files.
Is there any way to split them(the commit)?

[0xc0170]

Is there any way to split them(the commit)?

There are ways. You can git rebase -i to go back, edit each commit. Once you stop at the first one. You would split the changes to multiple commits. git reset HEAD~ - this would reset the commit you are changing (in case of 80 files would suddenly be uncommited). git add files that you want and create a new commit , and so on until you complete it and do git rebase --continue and done.

Answer like here should help https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-multiple-commits

If you need help, I can create a branch and split them as I understand the changes from the review

[M-ichae-l]

@0xc0170
Thank you for the help and explanation.
Please refer to the description of this PR. My first commit will cover 1 to 3, which is already 80 files changed. (as shown in #7638). 4 is another commit for just add a file. So even we reduced to the first commit, there will still be 80 files changed.

[0xc0170]

@M-ichae-l I pulled the changes locally, I understand the first commit better now. Looks fine as it is

In the future, updating libs and adding new functionality is better to be separated (own commit)

[cmonr]

@M-ichae-l Are the .a lib files a part of the SDK update?

[M-ichae-l]

@cmonr
Yes, indeed.

[0xc0170]

/morph build

[mbed-ci]

Build : SUCCESS

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

Triggering tests

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

[mbed-ci]

Exporter Build : SUCCESS

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

[mbed-ci]

Test : FAILURE

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

[cmonr]

@M-ichae-l Looks like this still needs some work.

Also, I corrected your PR description with the proper PR type formatting.

[M-ichae-l]

@cmonr
For the failure above, I have tried offline my side with greentea test for hal sleep test.
mbed test -t GCC_ARM -m REALTEK_RTL8195AM -n tests-mbed_hal-sleep* -c -v
mbed test -t ARM -m REALTEK_RTL8195AM -n tests-mbed_hal-sleep* -c -v
mbed test -t IAR -m REALTEK_RTL8195AM -n tests-mbed_hal-sleep* -c -v
There are no errors. Could you give some suggestions?

[0xc0170]

Build using compile with the following macros -DMBED_HEAP_STATS_ENABLED=1 -DMBED_STACK_STATS_ENABLED=1 -DMBED_TRAP_ERRORS_ENABLED=1 -DMBED_ALL_STATS_ENABLED

Then run tests. This should produce the same binaries than this job here

once you compile do mbedgt -n tests-name -v

[cmonr]

@M-ichae-l To elaborate on what @0xc0170 mentioned, CI builds and runs tests seperately.

That is to say, when we do a Morph Build, all examples, and tests for all targets in CI, for all compilers are built. When that succeeds, the success comment triggers a Morph Test which pulls the compiled binaries, unarchives them, and flashes and runs them.

To compile with all of the flags, and be a bit closer to how CI runs the tests, first do a mbed test --compile -m REALTEK_RTL8195AM -t <COMPILER>-DMBED_HEAP_STATS_ENABLED=1 -DMBED_STACK_STATS_ENABLED=1 -DMBED_TRAP_ERRORS_ENABLED=1 -DMBED_ALL_STATS_ENABLED followed by the mbedgt command. I think doing an mbed test --run <same parameters as before> should also work.

[M-ichae-l]

@cmonr
Thank you!
I have tested on my side. It looks that when the macros are opened the hal_sleep test will not pass.

-DMBED_HEAP_STATS_ENABLED=1 
-DMBED_STACK_STATS_ENABLED=1 
-DMBED_TRAP_ERRORS_ENABLED=1 
-DMBED_ALL_STATS_ENABLED

Could I know what are these macros used for? Are they affect the test hal_sleep?

[cmonr]

@deepikabhavnani @SenRamakri ^^^

[deepikabhavnani]

Could I know what are these macros used for? Are they affect the test hal_sleep?

-DMBED_ALL_STATS_ENABLED enables all the stats in mbed-os. It will internally set multiple macros of which MBED_CPU_STATS_ENABLED is used to measure idle thread (sleep) performance.

mbed-os/platform/mbed_stats.h

Line 38 in 56dd4bf

#define MBED_THREAD_STATS_ENABLED 1

Please set -DMBED_CPU_STATS_ENABLED and see if that is causing failures?

[deepikabhavnani]

Please note the change in test case to accommodate sleep stats timing. https://github.com/ARMmbed/mbed-os/blob/master/TESTS/mbed_hal/sleep/main.cpp#L58

Would like to know delta measurements for Realtek.

[M-ichae-l]

@deepikabhavnani @cmonr
Thank you for the explanation! "-DMBED_CPU_STATS_ENABLED" is causing failures
The delta measurements for Realtek is,
static const uint32_t sleep_mode_delta_us = (1000 + 32 + 5 + 25);
delta = default 10 us + worst ticker resolution + extra time for code execution
Rtl8195a need 1000us to wake up and the ticker resolution is 32us/tick (31.25).
With the above sleep_mode_delta_us setting, the test can be passed with -DMBED_CPU_STATS_ENABLED and all the other macros.

[deepikabhavnani]

@c1728p9 - Enabling cpu stats, adds call to get_lp_ticker_data twice. In most targets that adds 25us delta while measuring ticker resolution. But in case of rtl8195 we see additional 1000 us. Is that acceptable or points to some underlying issue?

@M-ichae-l - Is low power ticker supported in rtl8195?

Rtl8195a need 1000us to wake up

I am not sure why wakeup time is different in case stats are not enabled?

[c1728p9]

@deepikabhavnani that seems to point to a problem. The function get_lp_ticker_data should not block.

[M-ichae-l]

@deepikabhavnani
RTL8195A will add feature low power ticker but at this point, "LPTICKER" have not enabled yet.
Shall we try to fix the issue without enabling the lp_ticker?

[M-ichae-l]

I have finished the lp_ticker and us_ticker feature and tested. The test is under SLEEP, STCLK_OFF_DURING_SLEEP, USTICKER, LPTICKER and MBED_TICKLESS macros. The result shows that it is necessary to change to static const uint32_t sleep_mode_delta_us = (1000 + 32 + 5 + 25); in order to pass the test sleep_usticker_test

[deepikabhavnani]

@M-ichae-l - Sorry for the delayed response. If lpticker is not enabled, stats are not enabled and calls to read data are empty.

mbed-os/hal/mbed_sleep_manager.c

Line 43 in 4005887

#if defined(MBED_CPU_STATS_ENABLED) && defined(DEVICE_LPTICKER)

Also, I do see additional 10 ms for deepsleep mode. https://github.com/ARMmbed/mbed-os/blob/master/TESTS/mbed_hal/sleep/main.cpp#L65

Is there a chance that device enters deepsleep in sleep mode? Wake time of sleep mode should be less.

[M-ichae-l]

@deepikabhavnani
For RTL8195AM, the sleep is deepsleep. So, instead of entering deepsleep in sleep mode, the device is doing the other way.

I have committed an updated version of sleep, lpticker, and usticker. The test result still shows me that it is necessary to increase sleep_mode_delta_us.

[deepikabhavnani]

For RTL8195AM, the sleep is deepsleep. The test result still shows me that it is necessary to increase sleep_mode_delta_us.

@mprse @bulislaw @c1728p9 - tests-mbed_hal-sleep test fail as time to come out of deepsleep is long. Shall we update the test for special case of RTL8195AM or we have any other suggestions?

[c1728p9]

Sleep should wake up the CPU in 10us according to the sleep specification. Relaxing this requirement to +1000us for the RTL8195AM is a problem as users can no longer rely on sleep executing quickly and low interrupt latency.

@M-ichae-l have you investigated why the function get_lp_ticker_data() is taking 1000us? This should be fixed.

[bulislaw]

I agree with Russ we have introduced the specification for a reason - we had issues with boards behaving differently. Relaxing set requirements would put as in the same situation we worked hard to get out from.

[0xc0170]

I agree with Russ we have introduced the specification for a reason - we had issues with boards behaving differently. Relaxing set requirements would put as in the same situation we worked hard to get out from.

@M-ichae-l Can you fix the sleep functionality?

[M-ichae-l]

@0xc0170 @deepikabhavnani @c1728p9 @bulislaw
So for RTL8195AM, we can only support the DEEPSLEEP mode because of the weak-up time minima is 1ms. So when trying to use deepsleep as the sleep, it will not support the test. I have tried to empty the hal_sleep but the test will expect the system will sleep in different timesteps, not 0 timesteps.

Therefore,
1, hal_sleep cannot be skipped cause the test wants it to have sleep timestep.
2, SLEEP for RTL8195AM support wake-up in 1ms, but not 10us. RTL8195AM not support hal_sleep

[cmonr]

@M-ichae-l It looks like this is in need of a rebase.

@0xc0170 @deepikabhavnani @c1728p9 @bulislaw any other comments?

[M-ichae-l]

This PR has issues with us_ticker api and need to re-implementation. The issue is blocking some sleep tests to be passed.
For new lib, #8014 and #8266 are already merged.

[0xc0170]

@M-ichae-l As this PR was reworked (should only contain sleep now, as the update lib and MCU_ addition was merged), shall we close this PR and a new one will be created - to start with a clean table in this case ?

Another option is just close this one in the meantime, reopen with rebase/cleanup - only sleep implementiton addition

[M-ichae-l]

@0xc0170
Agree to close this PR. A new one will be created after us-ticker issue is solved.