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

Refactoring memory regions definitions for Fast Models MPS2 targets #7706

Merged
merged 5 commits into from Aug 17, 2018

Conversation

Projects
None yet
5 participants
@jamesbeyond
Contributor

jamesbeyond commented Aug 6, 2018

Description

This PR is about refactoring the memory regions definitions for fast models MPS2 targets

The reason for this changes are:

  • The old version had the memory size or regions defined hard coded in mulitiple places. In case you need to change some value, is very like the build is broken due to the multiple definations are not match each other.
  • The old vesion did not fully ultilized the 4MiB SRAM provided by the fast model. because the the hard coded 128K as HEAP_SIZE. running code coverage requires much larger heap.
  • The old version defined ISR stack size as 4K, but we want to limit it to 1K as #7238

The changes had been made:

  • added a memory_zones.h header file for each target. all the linker scripts (except IAR) or header files will reference the define in this memory_zones.h
  • IAR linker scripts have the same memory_zones coded in side icf file, as it is not supporting include a header file.
  • All tool chain linker scripts use 1K as ISR stack
  • ARMCC and GCC will auto calculate the heap size, IAR use 2MB as heap size.

Pull request type

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

cc @mprse

jamesbeyond added some commits Aug 6, 2018

Refactoring memory regions definitions for MPS2_M3 targets
* added memory_zones.h
* all linker scripts reference the definitions from memory_zones.h
* tool chains use predefined 1K as ISR Stack size
* ARM Complier 5 and GCC will auto calculated heap size
* IAR use predefined 2MiB as heap size
Refactoring memory regions definitions for MPS2_M4 targets
* added memory_zones.h
* all linker scripts reference the definitions from memory_zones.h
* tool chains use predefined 1K as ISR Stack size
* ARM Complier 5 and GCC will auto calculated heap size
* IAR use predefined 2MiB as heap size
Refactoring memory regions definitions for MPS2_M7 targets
* added memory_zones.h
* all linker scripts reference the definitions from memory_zones.h
* tool chains use predefined 1K as ISR Stack size
* ARM Complier 5 and GCC will auto calculated heap size
* IAR use predefined 2MiB as heap size
Refactoring memory regions definitions for MPS2_M0P targets
* added memory_zones.h
* all linker scripts reference the definitions from memory_zones.h
* tool chains use predefined 1K as ISR Stack size
* ARM Complier 5 and GCC will auto calculated heap size
* IAR use predefined 2MiB as heap size
Refactoring memory regions definitions for MPS2_M0 targets
* align MPS2_M0 FVP target with other MPS2 targets
* moved memory_zones.h
* chnage the flash_api.c where referencing the old memory_zones
* modify mbed_rtx.h to use the memory_zones definations as INITIAL_SP
* all linker scripts reference the definitions from memory_zones.h
* tool chains use predefined 1K as ISR Stack size
* ARM Complier 5 and GCC will auto calculated heap size
* IAR use predefined 2MiB as heap size

@0xc0170 0xc0170 requested a review from mprse Aug 6, 2018

@@ -62,11 +66,10 @@ MEMORY
*/
ENTRY(Reset_Handler)
HEAP_SIZE = 0x4000;
STACK_SIZE = 0x1000;
STACK_SIZE = 0x400;

This comment has been minimized.

@mprse

mprse Aug 8, 2018

Member

Stack size is hard-coded to 1KB not only for IAR, but also for GCC and ARM compilers. 1KB is the stack size required by MBED 5. And I see that there is no distinction for MBED 2 builds (which requires 4KB stack).
I see that FVP_MPS2 target has release_version 5 only. Probably MBED 2 builds are are not provided for Fast Model and that is why there is no distinction for stack required by MBED 2. Is it correct?

This comment has been minimized.

@jamesbeyond

jamesbeyond Aug 8, 2018

Contributor

Yes, hardcoded for 1KB is because Fastmodels only support mbed OS 5 at moment.
When mbed os 2 support is added, the linker scripts need to be adjusted based on the presence of MBED_CONF_RTOS_PRESENT macro

@mprse mprse referenced this pull request Aug 8, 2018

Closed

Stack unification and test #7238

@0xc0170

0xc0170 approved these changes Aug 9, 2018

@mprse

mprse approved these changes Aug 9, 2018

👍

@cmonr cmonr added needs: CI and removed needs: review labels Aug 14, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 14, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Aug 14, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 14, 2018

/morph test

@mbed-ci

This comment has been minimized.

@jamesbeyond

This comment has been minimized.

Contributor

jamesbeyond commented Aug 15, 2018

The sync failed the target was NRF51_DK, which total unrelated to the PR.
Seems a random HW failure. plase confirm @studavekar @cmonr

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 15, 2018

Interesting...
@jamesbeyond That looks correct. Will re-enqueue once CI load a bit lower.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 16, 2018

/morph test

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels Aug 17, 2018

@cmonr cmonr merged commit f15dbf2 into ARMmbed:master Aug 17, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0.0%) RAM(+0.0%)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 705 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10258 cycles (+82 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@jamesbeyond jamesbeyond deleted the jamesbeyond:fm_mem branch Aug 17, 2018

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

Merge pull request ARMmbed#7706 from jamesbeyond/fm_mem
 Refactoring memory regions definitions for Fast Models MPS2 targets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment