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

Adjust STMF411xE IAR linker file to mbed-os memory needs. #7909

Merged
merged 1 commit into from Sep 12, 2018

Conversation

Projects
None yet
9 participants
@KariHaapalehto
Contributor

KariHaapalehto commented Aug 28, 2018

Description

mbed cloud client needs more heap, so heap size increased.
8k is enough for stack, so stack size decreased.
New linker file configuration tested with HW, mbed cloud client is able to register to cloud

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@0xc0170 0xc0170 requested a review from ARMmbed/team-st-mcd Aug 28, 2018

define symbol __size_cstack__ = 0x4000;
define symbol __size_heap__ = 0x8000;
/*Heap 84kB and stack 8kB */
define symbol __size_cstack__ = 0x2000;

This comment has been minimized.

@0xc0170

0xc0170 Aug 28, 2018

Member

There's alignment for stack to be 1k @mprse please confirm

This comment has been minimized.

@mprse

mprse Sep 5, 2018

Member

I confirm, stack size should be 1k.

@0xc0170

Please fix the stack size

@0xc0170 0xc0170 added needs: work and removed needs: review labels Sep 5, 2018

Adjust STMF411xE IAR linker file to mbed-os memory needs.
Stack size decreased and heap size increased.

@KariHaapalehto KariHaapalehto force-pushed the KariHaapalehto:f411re_iar branch from 62fb351 to 86f966f Sep 6, 2018

@KariHaapalehto

This comment has been minimized.

Contributor

KariHaapalehto commented Sep 6, 2018

Stack size changed

@0xc0170

0xc0170 approved these changes Sep 6, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 6, 2018

@KariHaapalehto Why is the PR change so specific? A single target against a single compiler?

Also, is this required for 5.10?

@KariHaapalehto

This comment has been minimized.

Contributor

KariHaapalehto commented Sep 10, 2018

@cmonr This error only occur with IAR 7.x compiler, since it isn't supporting dynamic heap. This correction has been done already to stm32f439xx_flash.icf and it might be be good idea to check also other target in separate PR. Cloud client isn't working without this change, so it would be good to have in 5.10

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Sep 10, 2018

This is covering same area as #7238, and is not inconsistent with it. That's covering multiple targets, but appears to be not fully complete, and doesn't currently touch this one.

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Sep 10, 2018

So @kjbracey-arm ,
you suggest to close this PR and make #7238 complete (all targets, all toolchain update)?

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Sep 10, 2018

If this is urgently needed for 5.10 because it's a key cloud client target, I'd say it can go now, and other platforms can get mopped up by #7238 during 5.11. (Although that is only covering stack sizes, not the additional heap size increase here.)

This error only occur with IAR 7.x compiler

BTW @KariHaapalehto - are you suggesting IAR 8 does have dynamic heap? Would be good news if true.

@cmonr cmonr added needs: review and removed needs: work labels Sep 10, 2018

@adbridge

This comment has been minimized.

Contributor

adbridge commented Sep 10, 2018

Looks like we should definitely get this one in for RC2 especially as #7238 will not come in until 5.11 by the looks of it.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 10, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 10, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 11, 2018

Test CI needs to be restarted. Will restart aftetr 5.10-rc2 is generated.

We had to cutoff RC2 to continue testing with new fixes. Still planning on getting this into 5.10.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 11, 2018

/morph test

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 12, 2018

Would prefer for that heap size change to be tied to the application instead of making the change on the target itself, but I don't think we have that kind of mechanism to facilitate that.

@mprse @kjbracey-arm @ARMmbed/team-st-mcd Final thoughts/OKs?

@mprse

mprse approved these changes Sep 12, 2018

@bcostm

bcostm approved these changes Sep 12, 2018

@cmonr cmonr added ready for merge and removed needs: review labels Sep 12, 2018

@cmonr cmonr merged commit 8d9e2e5 into ARMmbed:master Sep 12, 2018

14 checks passed

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
jenkins-ci/cloud_client_smoke_test Test job: successful
Details
travis-ci/astyle Passed, 598 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10217 cycles (+990 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 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@KariHaapalehto KariHaapalehto deleted the KariHaapalehto:f411re_iar branch Sep 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment