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

fix rom start & size for CY8CKIT_062_WIFI_BT & CY8CPROTO_062_4343W #11138

Merged
merged 1 commit into from Aug 6, 2019

Conversation

@maclobdell
Copy link
Contributor

commented Jul 31, 2019

Description

This fixes a mistake that was merged to master in #11053

This corrects the flash base address and size, and makes it compatible with the new method that was introduced in 5.13.1 for merging the Cortex M4 and Cortex M0+ images. Essentially, instead of setting the rom start to be the start of the M4 area, it will now be set to the start of the M0+ area which precedes the M4. The linker file handles adding the M0+ area instead of post-build scripts.

This does not fix bootloader support. That will be introduced in a future PR.

This has been tested with CY8CKIT_062_WIFI_BT & CY8CPROTO_062_4343W, using GCC_ARM, and mbed-os-example-blinky. The application starts up fine on each board after this change.

Please merge urgently as this fixes master for these targets and unblocks other PRs.

cc @ARMmbed/team-cypress

Pull request type

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

Reviewers

Release Notes

@ciarmcom ciarmcom requested a review from ARMmbed/mbed-os-maintainers Jul 31, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@maclobdell, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@vmedcy

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

@maclobdell:
thank you, does this fix address issue raised in #10892 (comment)?

The bootloader use-case was not covered by Cypress internal testing of PR that introduced a change to CM0+ image placement flow.

@vmedcy
vmedcy approved these changes Jul 31, 2019
@hennadiykytsun

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

I feel you are going to do something wrong that is not compatible with the current Cypress linker scripts and current dual core strategy, or probably I have missed something.
Are you going to replace CM0+ core image with the bootloader with this PR?

@hennadiykytsun

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

I feel you are going to do something wrong that is not compatible with the current Cypress linker scripts and current dual core strategy, or probably I have missed something.
Are you going to replace CM0+ core image with the bootloader with this PR?

The mbed_overrides.c file handle situation with the device peripheral initialization if bootloader is enabled (CM4 does it), otherwise - this does CM0+ if bootloader is disabled for the PSA targets.

@maclobdell

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

@hennadiykytsun @vmedcy - This change paves the way for managed bootloader support (for the Cortex M4 application) by providing values that the mbed os build tools use to set and verify the start of flash. It is compatible with the new method that you are using to merge the CM0+. The flow that I believe can work (if logic added to the linker files) is that if building a bootloader (for M4) or application (for M4) that does not plan to use a bootloader, then include the CM0+ area. If building an application that will use a bootloader, then do not include the CM0+ area. If there are different settings in mbed_overrides.c for when building a bootloader or not, then perhaps these can be wrapped in a config macro.

@SeppoTakalo SeppoTakalo added needs: CI and removed needs: review labels Aug 5, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 5, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@SeppoTakalo SeppoTakalo merged commit 1c2b050 into ARMmbed:master Aug 6, 2019

26 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-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR 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-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8685 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.