Skip to content

MCU_NRF52832 (NRF52_DK, SDT52832B): use two-region memory model to support MicroLib #13951

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

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Nov 23, 2020

Summary of changes

Fixes: #13857

Arm toolchain's MicroLib, a lightweight C library used in conjunction with the bare metal profile to save code size, requires two separate memory regions for stack and heap. For MCU_NRF52832 (NRF52_DK, SDT52832B) such memory model is already used in the GCC_ARM linker script, and this PR adds it to the Arm Compiler scatter file.

This change is based on nRF52840.sct. Update: The heap size calculation of nRF52840 and many other targets contain a mistake, which will be fixed in a subsequent PR.

Impact of changes

MCU_NRF52832 targets (NRF52_DK, SDT52832B) now support MicroLib (i.e. "target.c_lib": "small" with TOOLCHAIN=ARM).

Migration actions required

To use MicroLib on MCU_NRF52832 targets, set "target.c_lib": "small" in mbed_app.json and compile with the ARM toolchain.

Documentation

None.


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

mbed-os-example-blinky-baremetal compiles and runs on NRF52_DK.
All Greentea tests with the bare metal profile pass:

mbed test -t ARM -m NRF52_DK --app-config TESTS/configs/baremetal.json

Both the above use MicroLib when compiled with the Arm Compiler.


Reviewers

@ARMmbed/mbed-os-core @MarceloSalazar


@LDong-Arm LDong-Arm changed the title MCU_NRF52832: Add a heap to support bare metal MCU_NRF52832 (NRF52_DK, SDT52832B): Add a heap to support bare metal Nov 23, 2020
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Nov 23, 2020
@ciarmcom ciarmcom requested review from MarceloSalazar and a team November 23, 2020 18:30
@ciarmcom
Copy link
Member

@LDong-Arm, thank you for your changes.
@MarceloSalazar @ARMmbed/team-nordic @ARMmbed/mbed-os-maintainers please review.

@LDong-Arm
Copy link
Contributor Author

LDong-Arm commented Nov 24, 2020

Manually pinging @ARMmbed/mbed-os-core for review, as the group was not added as reviewers by the system

MarceloSalazar
MarceloSalazar previously approved these changes Nov 24, 2020
Copy link

@MarceloSalazar MarceloSalazar left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot added needs: CI and removed needs: review labels Nov 24, 2020
0xc0170
0xc0170 previously approved these changes Nov 24, 2020
@LDong-Arm
Copy link
Contributor Author

@0xc0170 I need to edit the PR description and commit message later as they are not accurate, please keep it on hold for a bit, thanks!

@LDong-Arm LDong-Arm changed the title MCU_NRF52832 (NRF52_DK, SDT52832B): Add a heap to support bare metal MCU_NRF52832 (NRF52_DK, SDT52832B): use two-region memory layout to support MicroLib Nov 24, 2020
@mergify mergify bot dismissed stale reviews from MarceloSalazar and 0xc0170 November 24, 2020 13:43

Pull request has been modified.

@LDong-Arm LDong-Arm changed the title MCU_NRF52832 (NRF52_DK, SDT52832B): use two-region memory layout to support MicroLib MCU_NRF52832 (NRF52_DK, SDT52832B): use two-region memory model to support MicroLib Nov 24, 2020
@LDong-Arm
Copy link
Contributor Author

@evedon I've updated the PR description and commit message to clarify the two-region memory model is for MicroLib.
@0xc0170 Now it should be ready for CI once reapproved, thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 24, 2020

CI restarted

evedon
evedon previously requested changes Nov 24, 2020
@mergify mergify bot added needs: work and removed needs: CI labels Nov 24, 2020
@mbed-ci
Copy link

mbed-ci commented Nov 24, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️

…MicroLib

MicroLib is the lightweight C lib for the Arm toolchain and
used as the default C lib on bare metal builds with the Arm
toolchain. It requires two separate memory regions for stack
and heap.

The change is based on nRF52840.sct.
@mergify mergify bot dismissed evedon’s stale review November 24, 2020 16:35

Pull request has been modified.

@LDong-Arm LDong-Arm requested a review from 0xc0170 November 24, 2020 16:46
@LDong-Arm
Copy link
Contributor Author

Hopefully it's all good this time!

@mergify mergify bot added needs: CI and removed needs: work labels Nov 24, 2020
@LDong-Arm
Copy link
Contributor Author

@0xc0170 This PR is ready for CI (when there's capacity), cheers!

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 26, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 26, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest ✔️

@0xc0170 0xc0170 merged commit 22c1c6c into ARMmbed:master Nov 26, 2020
@mergify mergify bot removed the ready for merge label Nov 26, 2020
@mbedmain mbedmain added release-version: 6.6.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nRF52 target doesn't compile with blinky-baremetal
7 participants