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.

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

Reduce 32KB from LPC55S69_S binary size #10047

Merged
merged 2 commits into from Mar 13, 2019

Conversation

@mikisch81
Copy link
Contributor

commented Mar 12, 2019

Description

Reduce the LPC55S69_S targets binary size from 192KB to 160KB.

Tested on LPC55S69 target, all PSA and compliance tests pass.

Targeted for rc3.

Pull request type

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

Reviewers

@orenc17 @mmahadevan108

Release Notes

@ciarmcom ciarmcom requested review from maclobdell, MarceloSalazar, orenc17 and ARMmbed/mbed-os-maintainers Mar 12, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 12, 2019

@mmahadevan108

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

Thank you for your changes, please update the below as well as I could not find a way to include the header file into IAR linker script as is done for GCC_ARM and ARMC toolchains
https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC55S69/TARGET_M33_NS/device/TOOLCHAIN_IAR/LPC55S69_cm33_core0_flash.icf#L26

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

@theotherjimmy @bridadan @deepikabhavnani @orenc17 @alzix
Opened IOTSEC-1123 regarding our on-going discussion on PSA targets addresses.

This PR is an example of the need for it (a small change in the split between secure/non-secure forced us to change couple of files).

@alzix
Copy link
Contributor

left a comment

Can we use values from JSON file directly instead of hardcoded values?

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

Can we use values from JSON file directly instead of hardcoded values?

It is something which requires more work, can you move the discussion to the new JIRA task?

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

@orenc17 please review

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Mar 12, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

CI started

@orenc17 orenc17 force-pushed the kfnta:nxp_secure_code_size branch to 546e33d Mar 13, 2019

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

rebased on top of master for #10086

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

@0xc0170 can CI be restarted?

@orenc17

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

@mikisch81 i think we are going to need a roll-up PR here @davidsaada has conflicts between this PR and #10086 and #10068

@mikisch81

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

@orenc17 this PR is ready to be merged, it was only waiting for rc2 to finally happen, no need to hold this because of another PR

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

@mikisch81 i think we are going to need a roll-up PR here @davidsaada has conflicts between this PR and #10086 and #10068

Oh boy.

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

No need for a rollup PR. My bootloader PR is dependent on this one and not vice versa. All I needed was either this PR to be merged in or rebased.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 13, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@mbed-ci

This comment has been minimized.

Copy link

commented Mar 13, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 2
Build artifacts

@cmonr cmonr added ready for merge and removed needs: CI labels Mar 13, 2019

@cmonr cmonr merged commit fe59870 into ARMmbed:master Mar 13, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9160 cycles (-260 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Mar 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.