Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactoring memory regions definitions for Fast Models MPS2 targets #7706

Merged
merged 5 commits into from
Aug 17, 2018
Merged

Refactoring memory regions definitions for Fast Models MPS2 targets #7706

merged 5 commits into from
Aug 17, 2018

Conversation

jamesbeyond
Copy link
Contributor

@jamesbeyond jamesbeyond commented Aug 6, 2018 •
edited
Loading

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 Stack unification and test #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

Qinghao Shi added 5 commits August 6, 2018 01:41
Refactoring memory regions definitions for MPS2_M3 targets
a9f4481 
* 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
7159329 
* 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
8fc3842 
* 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
6f92504 
* 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
f2aae22 
* 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 August 6, 2018 14:13
mprse
mprse reviewed Aug 8, 2018
View reviewed changes
targets/TARGET_ARM_FM/TARGET_FVP_MPS2/TARGET_FVP_MPS2_M3/device/TOOLCHAIN_GCC_ARM/MPS2.ld
@@ -62,11 +66,10 @@ MEMORY
*/
ENTRY(Reset_Handler)

HEAP_SIZE = 0x4000;
STACK_SIZE = 0x1000;
STACK_SIZE = 0x400;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 mentioned this pull request Aug 8, 2018
Closed
mprse
mprse approved these changes Aug 9, 2018
View reviewed changes
Copy link
Contributor

@mprse mprse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph build

@mbed-ci
Copy link

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
Copy link

mbed-ci commented Aug 14, 2018

Exporter Build : SUCCESS

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

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 15, 2018

Test : FAILURE

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

@jamesbeyond
Copy link
Contributor Author

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

@cmonr
Copy link
Contributor

cmonr commented Aug 15, 2018

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

@jamesbeyond jamesbeyond mentioned this pull request Aug 16, 2018
Merged
@cmonr
Copy link
Contributor

cmonr commented Aug 16, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 17, 2018

Test : SUCCESS

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

@cmonr cmonr merged commit f15dbf2 into ARMmbed:master Aug 17, 2018
@jamesbeyond jamesbeyond deleted the fm_mem branch August 17, 2018 16:11
pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018
Merge pull request ARMmbed#7706 from jamesbeyond/fm_mem
55f723d 
 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
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@mprse mprse mprse approved these changes

Assignees
No one assigned
Labels
release-version: 5.9.6
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jamesbeyond @cmonr @mbed-ci @0xc0170 @mprse