MIMXRT1050EVK: Fix Sleep support #7419

mmahadevan108 · 2018-07-04T13:09:16Z

Description

Add support for SLEEP in MXRT1050EVK. Support for low power modes on MXRT requires
1. Updates to the CCM LPCR register to set the power mode
2. Enabling wakeup source in the GPC module
3. Ensuring the low power functions related to powering off
FLEXx SPI and SDRAM are executed from internal memory
4. Save PLL state before disabling them
5. Restore PLL state on resume

All tests passed under GCC_ARM and IAR toolchains.

With ARM toolchain, all tests passed except I see the below green-tea crash when running mbed-os-tests-mbed_hal-lp_ticker test. I have debugged this and the test is passing, however it crashes on exit. Also this happens only with ARM toolchain. Any ideas??

mbedhtrun: mbedhtrun -m MIMXRT1050_EVK -p COM50:9600 -f "BUILD/tests/MIMXRT1050_EVK/ARM/mbed-os/TESTS/mbed_hal/lp_ticker/lp_ticker.bin" -e "mbed-os\TESTS\host_tests" -d D: -c default -t 0227000041114e450004300ac2040020df91000097969900 -r default -C 4 --sync 5 -P 60
mbedgt: mbed-host-test-runner: started
[1530709453.47][HTST][INF] host test executor ver. 1.3.1
[1530709453.47][HTST][INF] copy image onto target...
[1530709453.47][COPY][INF] Waiting up to 60 sec for '0227000041114e450004300ac2040020df91000097969900' mount point (current is 'D:')...
1 file(s) copied.
[1530709463.43][HTST][INF] starting host test process...
[1530709465.57][CONN][INF] starting connection process...
[1530709465.57][CONN][INF] notify event queue about extra 60 sec timeout for serial port pooling
[1530709465.57][CONN][INF] initializing serial port listener...
[1530709465.57][PLGN][INF] Waiting up to 60 sec for '0227000041114e450004300ac2040020df91000097969900' serial port (current is 'COM50')...
[1530709465.57][HTST][INF] setting timeout to: 60 sec
[1530709465.66][SERI][INF] serial(port=COM50, baudrate=9600, read_timeout=0.01, write_timeout=5)
[1530709465.66][SERI][INF] reset device using 'default' plugin...
[1530709465.92][SERI][INF] waiting 1.00 sec after reset
[1530709466.92][SERI][INF] wait for it...
[1530709466.92][SERI][TXD] mbedmbedmbedmbedmbedmbedmbedmbedmbedmbed
[1530709466.92][CONN][INF] sending up to 5 __sync packets (specified with --sync=5)
[1530709466.92][CONN][INF] sending preamble 'c550c4f6-142d-41dd-8848-ac685a497706'
[1530709466.92][SERI][TXD] {{__sync;c550c4f6-142d-41dd-8848-ac685a497706}}
[1530709467.06][CONN][RXD] mbedmbedmbedmbedmbedmbedmbedmbed
[1530709467.10][CONN][INF] found SYNC in stream: {{__sync;c550c4f6-142d-41dd-8848-ac685a497706}} it is #0 sent, queued..
.
[1530709467.11][HTST][INF] sync KV found, uuid=c550c4f6-142d-41dd-8848-ac685a497706, timestamp=1530709467.102000
[1530709467.12][CONN][INF] found KV pair in stream: {{__version;1.3.0}}, queued...
[1530709467.13][HTST][INF] DUT greentea-client version: 1.3.0
[1530709467.15][CONN][INF] found KV pair in stream: {{__timeout;20}}, queued...
[1530709467.15][HTST][INF] setting timeout to: 20 sec
[1530709467.18][CONN][INF] found KV pair in stream: {{__host_test_name;default_auto}}, queued...
[1530709467.18][HTST][INF] host test class: '<class 'mbed_host_tests.host_tests.default_auto.DefaultAuto'>'
[1530709467.18][HTST][INF] host test setup() call...
[1530709467.18][HTST][INF] CALLBACKs updated
[1530709467.18][HTST][INF] host test detected: default_auto
[1530709467.21][CONN][RXD] >>> Running 3 test cases...
[1530709467.26][CONN][INF] found KV pair in stream: {{__testcase_name;lp ticker info test}}, queued...
[1530709467.30][CONN][INF] found KV pair in stream: {{__testcase_name;lp ticker sleep test}}, queued...
[1530709467.35][CONN][RXD]
[1530709467.35][CONN][INF] found KV pair in stream: {{__testcase_name;lp ticker glitch test}}, queued...
[1530709467.39][CONN][RXD] >>> Running case #1: 'lp ticker info test'...
[1530709467.43][CONN][INF] found KV pair in stream: {{__testcase_start;lp ticker info test}}, queued...
[1530709467.48][CONN][INF] found KV pair in stream: {{__testcase_finish;lp ticker info test;1;0}}, queued...
[1530709467.53][CONN][RXD] >>> 'lp ticker info test': 1 passed, 0 failed
[1530709467.53][CONN][RXD]
[1530709467.58][CONN][RXD] >>> Running case #2: 'lp ticker sleep test'...
Exception in thread Thread-3:
Traceback (most recent call last):
File "c:\python27\lib\threading.py", line 801, in __bootstrap_inner
self.run()
File "c:\python27\lib\threading.py", line 754, in run
self.__target(*self.__args, **self.__kwargs)
File "c:\python27\lib\site-packages\mbed_greentea\mbed_greentea_cli.py", line 489, in run_test_thread
verbose=verbose)
File "c:\python27\lib\site-packages\mbed_greentea\mbed_test_api.py", line 318, in run_host_test
returncode, htrun_output = run_htrun(cmd, verbose)
File "c:\python27\lib\site-packages\mbed_greentea\mbed_test_api.py", line 134, in run_htrun
htrun_output += line.decode('utf-8')
File "c:\python27\lib\encodings\utf_8.py", line 16, in decode
return codecs.utf_8_decode(input, errors, True)
UnicodeDecodeError: 'utf8' codec can't decode byte 0xa1 in position 28: invalid start byte

mbedgt: could not generate test report
mbedgt: completed in 20.36 sec
mbedgt: exited with code -1000
[mbed] ERROR: "mbedgt" returned error code -1000.
[mbed] ERROR: Command "mbedgt --test-spec C:\mymbed\BUILD\tests\MIMXRT1050_EVK\ARM\test_spec.json -n mbed-os-tests-mbed_
hal-lp_ticker -V" in "C:\mymbed"

Pull request type

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

mmahadevan108 · 2018-07-04T13:09:44Z

cc @0xc0170

0xc0170 · 2018-07-04T13:20:00Z

With ARM toolchain, all tests passed except I see the below green-tea crash when running mbed-os-tests-mbed_hal-lp_ticker test. I have debugged this and the test is passing, however it crashes on exit. Also this happens only with ARM toolchain. Any ideas??

Is this connected to this changeset?

mmahadevan108 · 2018-07-04T13:23:01Z

I have looked closely at this changeset and cannot understand the cause of test framework crash. Can someone look at the green-tea failure log.

mprse · 2018-07-05T05:34:10Z

It looks like deep-sleep test fails. In the test there is a special delay used before entering deep-sleep, so the green tea transmission can be successfully finished:

mbed-os/TESTS/mbed_hal/lp_ticker/main.cpp

Line 101 in f12afde

wait_cycles(400000);

Maybe it is not sufficient for your target on ARM compiler and needs to be increased. Can you try to increase the delay?

mmahadevan108 · 2018-07-05T22:36:07Z

cc @maclobdell

mmahadevan108 · 2018-07-05T22:53:46Z

@mprse Thank you for your suggestion. I made the below change and don't see the crash anymore.

index 8c40e9c66..9b3f731bc 100644
--- a/TESTS/mbed_hal/lp_ticker/main.cpp
+++ b/TESTS/mbed_hal/lp_ticker/main.cpp
@@ -98,7 +98,7 @@ void lp_ticker_deepsleep_test()
     lp_ticker_init();

     /* Wait for green tea UART transmission before entering deep-sleep mode. */
-    wait_cycles(400000);
+    wait_cycles(1000000);

     overflow_protect();

@@ -128,6 +128,9 @@ void lp_ticker_glitch_test()
     uint32_t last = lp_ticker_read();
     const uint32_t start = last;

+    /* Give sometime for the counter to tick */
+    wait_cycles(1000000);
+
     while (last < (start + TICKER_GLITCH_TEST_TICKS)) {
         const uint32_t cur = lp_ticker_read();
         TEST_ASSERT(cur >= last);

0xc0170 · 2018-07-09T10:55:31Z

Maybe it is not sufficient for your target on ARM compiler and needs to be increased. Can you try to increase the delay?

@mprse How shall we address this issue?

mprse · 2018-07-09T12:04:45Z

@mprse How shall we address this issue?

This is workaround commonly used in tests. Since the required delay is target specific I had to increase the delay few times already.
Many tests use this approach and waits before entering deep-sleep mode (I can't see other option), but I think we should consider more elegant solution (some green tea functionality), so it can be handled properly.
But for the moment we should simply increase the delay. Shell I create the PR? @mmahadevan108 is 1000000 value sufficient?

TomoYamanaka · 2018-07-12T10:04:59Z

@mprse @mmahadevan108

I'm implementing LP Ticker feature for GR-PEACH and GR-LYCHEE, but I faced this issue(lp ticker sleep test is fail) when using ARMCC toolchain.
Same as the above, It seems that a special delay used before entering deep-sleep is short for my boards.

But for the moment we should simply increase the delay. Shell I create the PR? @mmahadevan108 is 1000000 value sufficient?

I will appreciate if you could create this PR.

mprse · 2018-07-12T11:22:46Z

I will appreciate if you could create this PR.

Fix can be found here: PR #7494.

Please let me know if this fix solves the issue.

TomoYamanaka · 2018-07-13T09:07:45Z

@mprse

Fix can be found here: PR #7494.

Please let me know if this fix solves the issue

Thank you. Your PR solved my issue.

0xc0170 · 2018-07-13T09:21:26Z

Depends on #7494 .

mmahadevan108 · 2018-08-01T16:40:31Z

Apologies for the delay, I have rebased this PR. I still see an issue with the glitch test, below change fixes it, could you please take a look.

--- a/TESTS/mbed_hal/lp_ticker/main.cpp
+++ b/TESTS/mbed_hal/lp_ticker/main.cpp
@@ -150,7 +150,12 @@ void lp_ticker_glitch_test()
     uint32_t last = lp_ticker_read();
     const uint32_t start = last;

     while (last < (start + TICKER_GLITCH_TEST_TICKS)) {
+        /* Wait a while - let the ticker to count. */
+        busy_wait_ms(SERIAL_FLUSH_TIME_MS);
+
         const uint32_t cur = lp_ticker_read();
         TEST_ASSERT(cur >= last);
         last = cur;

0xc0170 · 2018-08-02T11:28:47Z

Apologies for the delay, I have rebased this PR. I still see an issue with the glitch test, below change fixes it, could you please take a look.

@mprse Please review

mprse · 2018-08-02T12:06:35Z

@mmahadevan108 This test checks if lp ticker does not glitch backwards due to an incorrectly implemented ripple counter driver.
As far as I can see you added the delay in the while loop, so mainly you have reduced number of attempts when test is looking for the glitch. This is not the right way. The delay is only needed before going into deep-sleep. It looks like there is some problem with lp ticker driver. I see that lp ticker is based on 32 bit counter and operates at 32 kHz. So the rollover is likely not possible in this case. Could you please provide the test output with the failure?

Additionally please check the pseudo code implementation of lp_ticker_read() function which is checked by the failing test case:

mbed-os/hal/lp_ticker_api.h

Lines 131 to 158 in f9862b8

    
           /** Read the current tick 
        
            * 
        
            * If no rollover has occurred, the seconds passed since lp_ticker_init() 
        
            * was called can be found by dividing the ticks returned by this function 
        
            * by the frequency returned by ::lp_ticker_get_info. 
        
            * 
        
            * @return The current timer's counter value in ticks 
        
            * 
        
            * Pseudo Code: 
        
            * @code 
        
            * uint32_t lp_ticker_read() 
        
            * { 
        
            *     uint16_t count; 
        
            *     uint16_t last_count; 
        
            * 
        
            *     // Loop until the same tick is read twice since this 
        
            *     // is ripple counter on a different clock domain. 
        
            *     count = LPTMR_COUNT; 
        
            *     do { 
        
            *         last_count = count; 
        
            *         count = LPTMR_COUNT; 
        
            *     } while (last_count != count); 
        
            * 
        
            *     return count; 
        
            * } 
        
            * @endcode 
        
            */ 
        
           uint32_t lp_ticker_read(void);

As you can see counter is read in loop until the same value is read twice. Looking into implementation of lp_ticker_read() it simply returns counter value. Not sure if this is the case, but I think it is worth to check:

mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_IMX/lp_ticker.c

Lines 72 to 75 in f9862b8

    
           uint32_t lp_ticker_read(void) 
        
           { 
        
               return GPT_GetCurrentTimerCount(GPT2); 
        
           }

mprse

I see that this PR not only changes implementation of sleep functions and enables sleep support in targets.json, but also adds some files, changes linker scripts, fixing codding style, etc. This is all done in one commit and it is hard to understand the changes. Could you please split this commit and add some note to describe the purpose of the changes.

mmahadevan108 · 2018-08-02T13:47:52Z

All changes in this PR are related to adding sleep support on MRT1050

The sleep implementation on MXRT is a bit more involved than Kinetis and LPC devices. The changes to the linker script maps memory that stays active during DEEP SLEEP to hold certain functions.

mmahadevan108 · 2018-08-02T15:56:28Z

@mprse @0xc0170 . Apologies for the earlier report. I was testing on a wrong version of the board.

The lp ticker test passes with the current implementation.

0xc0170 · 2018-08-03T08:49:28Z

@mmahadevan108 Can you add that explanation to the commit ? How the commit fixes it (what problem there was and how it is addressed)

mmahadevan108 · 2018-08-03T13:03:55Z

@0xc0170 . Done, update commit message to include more information.

0xc0170

Update below in the next review

0xc0170

It would be better if this is separated to more commits as @mprse requested earlier. One commit changes multiple files for multiple reasons with one common goal - fix sleep.

Now the commit adds more details for fixing, it's more clear to me what is being changed however took me a while to go through changes.

Commits as follows (as I understand changes from this pull request - for illustration how it could be done):

add lpm init (why are we adding lpm files, is this new driver update?)
update sdk init: add copy function (what is being copied ? this requires some linker script changes?)
SKIP_SYSCLK_INIT addition (why is this needed now ?)
sleep: add pre/post processing steps
enable sleep

Note: my previous review was supposed to be needs work rather than approval, as I can't change it, adding this new one

This is needed to support different Low-Power modes available in MXRT1050 Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

…l memory Low power functions related to powering off FLEXSPI and SDRAM needs to be copied to internal memory Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

SKIP_SYSCLK_INIT prevents reinitializing the SYS_CLK PLL used by SDRAM. This PLL is setup during bootup by the ROM code. Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

mmahadevan108 · 2018-08-06T16:57:53Z

The PR has been broken to smaller commits as suggested earlier.

0xc0170

@mmahadevan108 Thanks ! Looks good , much easier to review and understand changes

0xc0170 · 2018-08-08T05:53:49Z

/morph build

mbed-ci · 2018-08-08T06:44:57Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-08-08T08:28:00Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-08-08T09:05:00Z

Test : SUCCESS

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

…upport MIMXRT1050EVK: Fix Sleep support

0xc0170 requested a review from a team July 4, 2018 13:20

0xc0170 added the needs: review label Jul 4, 2018

mprse mentioned this pull request Jul 12, 2018

tests-mbed_hal-lp_ticker: change implementation of the delay before deep-sleep. #7494

Merged

0xc0170 added needs: preceding PR and removed needs: review labels Jul 13, 2018

mmahadevan108 force-pushed the Add_MXRT_Sleep_Support branch from 83e53f1 to aefae1b Compare August 1, 2018 15:20

0xc0170 added needs: review and removed needs: preceding PR labels Aug 2, 2018

mprse suggested changes Aug 2, 2018

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Aug 3, 2018

mmahadevan108 force-pushed the Add_MXRT_Sleep_Support branch from aefae1b to 33e6ac9 Compare August 3, 2018 13:02

0xc0170 approved these changes Aug 6, 2018

View reviewed changes

0xc0170 requested changes Aug 6, 2018

View reviewed changes

mmahadevan108 added 4 commits August 6, 2018 10:36

MIMXRT1050_EVK: Add Low Power Manager files

a1d8298

This is needed to support different Low-Power modes available in MXRT1050 Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

MXRT1050_EVK: Ensure certain low power function are linked to interna…

e18e0f1

…l memory Low power functions related to powering off FLEXSPI and SDRAM needs to be copied to internal memory Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

MXRT1050_EVK: Sleep: add pre/post processing steps

74c96b6

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

MXRT1050_EVK: Enable Sleep function and add SKIP_SYSCLK_INIT macro

9cf2b76

SKIP_SYSCLK_INIT prevents reinitializing the SYS_CLK PLL used by SDRAM. This PLL is setup during bootup by the ROM code. Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>

mmahadevan108 force-pushed the Add_MXRT_Sleep_Support branch from 33e6ac9 to 9cf2b76 Compare August 6, 2018 16:57

0xc0170 approved these changes Aug 7, 2018

View reviewed changes

0xc0170 added needs: review and removed needs: work labels Aug 7, 2018

mprse approved these changes Aug 8, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Aug 8, 2018

0xc0170 added ready for merge and removed needs: CI labels Aug 8, 2018

cmonr added the release-version: 5.9.5 label Aug 8, 2018

cmonr merged commit d360da9 into ARMmbed:master Aug 8, 2018

0xc0170 removed the ready for merge label Aug 8, 2018

mmahadevan108 deleted the Add_MXRT_Sleep_Support branch August 8, 2018 16:33

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7419 from codeauroraforum/Add_MXRT_Sleep_S…

6ac0271

…upport MIMXRT1050EVK: Fix Sleep support

MIMXRT1050EVK: Fix Sleep support #7419

MIMXRT1050EVK: Fix Sleep support #7419

Uh oh!

Conversation

mmahadevan108 commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

mmahadevan108 commented Jul 4, 2018

Uh oh!

0xc0170 commented Jul 4, 2018

Uh oh!

mmahadevan108 commented Jul 4, 2018

Uh oh!

mprse commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmahadevan108 commented Jul 5, 2018

Uh oh!

mmahadevan108 commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Jul 9, 2018

Uh oh!

mprse commented Jul 9, 2018

Uh oh!

TomoYamanaka commented Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mprse commented Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomoYamanaka commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Jul 13, 2018

Uh oh!

mmahadevan108 commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mprse commented Aug 2, 2018

Uh oh!

mprse left a comment

Choose a reason for hiding this comment

Uh oh!

mmahadevan108 commented Aug 2, 2018

Uh oh!

mmahadevan108 commented Aug 2, 2018

Uh oh!

0xc0170 commented Aug 3, 2018

Uh oh!

mmahadevan108 commented Aug 3, 2018

Uh oh!

0xc0170 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmahadevan108 commented Aug 6, 2018

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Aug 8, 2018

Uh oh!

mbed-ci commented Aug 8, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Aug 8, 2018

Exporter Build : SUCCESS

Uh oh!

mbed-ci commented Aug 8, 2018

mmahadevan108 commented Jul 4, 2018 •

edited

Loading

mprse commented Jul 5, 2018 •

edited

Loading

mmahadevan108 commented Jul 5, 2018 •

edited

Loading

TomoYamanaka commented Jul 12, 2018 •

edited

Loading

mprse commented Jul 12, 2018 •

edited

Loading

TomoYamanaka commented Jul 13, 2018 •

edited

Loading

mmahadevan108 commented Aug 1, 2018 •

edited

Loading

0xc0170 commented Aug 2, 2018 •

edited

Loading

0xc0170 left a comment •

edited

Loading

0xc0170 left a comment •

edited

Loading