Skip to content

M2351: Support memory custom partition #10004

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
Merged
merged 6 commits into from
Apr 9, 2019
Merged

M2351: Support memory custom partition #10004

merged 6 commits into from
Apr 9, 2019

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Mar 8, 2019 •
edited
Loading

Description

Relying on #10008, this PR tries to add back support for ROM/RAM custom partition and address related #8757 which updates M2351 CMSIS pack and changes memory spec from single secure ROM/RAM block to separate secure/non-secure ROM/RAM blocks.

Related issue or PR

#10008
#8757

Pull request type

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

ccli8 added 5 commits March 7, 2019 16:28
@ccli8
[M2351] Remove unnecessary toolchain directory TOOLCHAIN_ARM_MICRO
26ff067
@ccli8
[M2351] Rename TOOLCHAIN_ARM_STD TOOLCHAIN_ARMC6
c0bba74 
This is to express ARMC6 toolchain support more explicitly.
@ccli8
[M2351] Fix preprocessor command line in ARMC6 .sct file
894f71e
@ccli8
[M2351] Refactor for memory partittion support
14775cb 
Add partition_M2351_mem.h/partition_M2351_mem.icf to centralize memory partition
@ccli8
[M2351] Override CMSIS ROM/RAM spec in targets.json
c4bda55 
Old M2351 CMSIS pack reports single secure ROM/RAM spec. It is updated in new
version which reports secure/non-secure ROM/RAM spec. Override this memory spec
in targets.json regardless of CMSIS.
@ciarmcom ciarmcom requested review from Ronny-Liu and a team March 8, 2019 06:00
@ciarmcom
Copy link
Member

ciarmcom commented Mar 8, 2019

@ccli8, thank you for your changes.
@Ronny-Liu @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-tools please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 8, 2019

Commmit fa975ad4924b6f88d364ff62e4e15008515744ec - can this be sent separately? Tools team would review (the rest of commits are target related).

@ccli8 ccli8 mentioned this pull request Mar 8, 2019
Merged
@ccli8 ccli8 force-pushed the nuvoton_m2351_fix-memory-partition branch from fa975ad to c4bda55 Compare March 8, 2019 10:31
@ccli8
Copy link
Contributor Author

ccli8 commented Mar 8, 2019

@0xc0170 Commmit fa975ad - can this be sent separately? Tools team would review (the rest of commits are target related).

Separate to #10008.

@bridadan
Copy link
Contributor

bridadan commented Mar 8, 2019

@0xc0170 since that commit was removed from this PR I'll go ahead and dismiss the mbed-os-tools review.

@bridadan bridadan removed the request for review from a team March 8, 2019 16:16
0xc0170
0xc0170 previously requested changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

License addition to the new sct file

targets/TARGET_NUVOTON/TARGET_M2351/device/partition_M2351_mem.icf
@@ -0,0 +1,82 @@
/* See partition_M2351_mem.h for documentation */
Copy link
Contributor

Choose a reason for hiding this comment

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

please add license header to all new files if not yet in, like this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 Added license header.

@cmonr cmonr requested a review from 0xc0170 March 13, 2019 18:44
@ccli8
Copy link
Contributor Author

ccli8 commented Mar 27, 2019

With #10008 merged, any update?

@cmonr cmonr dismissed 0xc0170’s stale review March 27, 2019 21:45

License header added.

@cmonr
Copy link
Contributor

cmonr commented Mar 27, 2019

@ccli8 Who are you asking?

@ccli8
Copy link
Contributor Author

ccli8 commented Mar 28, 2019

@cmonr This PR relies on #10008. With #10008 having merged into mainline, how about this PR's progress?

@cmonr
Copy link
Contributor

cmonr commented Mar 28, 2019

I'm still not sure I understand. Are you saying that this is ready to progress now that #10008 is merged?

@ccli8
Copy link
Contributor Author

ccli8 commented Mar 28, 2019

@cmonr Yes, please go ahead.

@cmonr cmonr requested a review from a team March 28, 2019 01:56
@cmonr
Copy link
Contributor

cmonr commented Mar 28, 2019

@ARMmbed/team-nuvoton Please review.

cmonr
cmonr approved these changes Mar 28, 2019
View reviewed changes
Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

LGTM

@ccli8
Copy link
Contributor Author

ccli8 commented Apr 3, 2019

@ARMmbed/team-nuvoton Please review.

@cyliangtw

cyliangtw
cyliangtw approved these changes Apr 9, 2019
View reviewed changes
Copy link
Contributor

@cyliangtw cyliangtw left a comment

Choose a reason for hiding this comment

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

Some duplicate rom/ram default settings in partition_M2351_mem.h and target.json .
Remind to remove the one in partition_M2351_mem.h after tool mature enough.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 9, 2019

Ci started

@mbed-ci
Copy link

mbed-ci commented Apr 9, 2019

Test run: SUCCESS

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

@cmonr cmonr merged commit 73f1edd into ARMmbed:master Apr 9, 2019
@cyliangtw cyliangtw deleted the nuvoton_m2351_fix-memory-partition branch March 9, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@cyliangtw cyliangtw cyliangtw approved these changes

@Ronny-Liu Ronny-Liu Awaiting requested review from Ronny-Liu

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

Successfully merging this pull request may close these issues.
7 participants
@ccli8 @ciarmcom @0xc0170 @bridadan @cmonr @mbed-ci @cyliangtw