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

EP Atlas Target Updates #14325

Merged
merged 6 commits into from
Mar 2, 2021
Merged

Conversation

AGlass0fMilk
Copy link
Member

@AGlass0fMilk AGlass0fMilk commented Feb 22, 2021

Summary of changes

This PR introduces several updates to the EP_ATLAS target.

Namely, the updates are:


Introduce subtarget_sdk_init startup hook.

The mbed_sdk_init startup hook is implemented at the NRF52-series level and so is unavailable for override. This commit adds an additional startup hook for NRF52 subtargets to perform any other startup initialization required.


Configure internal regulators at startup

This commit introduces an implementation of the subtarget_sdk_init startup hook (called during mbed_sdk_init) that configures the internal regulators of the nRF52840.

The configuration sets up the internal regulator to output 3.3V. If this is not done, the default system voltage may be too low for the on-board indicator LEDs to conduct (ie: system voltage is lower than LED forward voltage).


Add option to use USBSerial for stdio console

This commit introduces an option, ep-atlas.enable-usb-stdio-console, that will retarget the Mbed stdio console handle to a USBSerial instance if enabled.

Please note that if your application uses USB, it will conflict with this option. You should disable this option and implement a composite USB device in your application if you require stdio over USB.

This option is disabled by default so it will not cause issues with existing user code.


Add default app start and size limitations

This commit introduces a default application start address (0x1000) and size limitation (0xDF000) to accomodate the Nordic USB bootloader.

The bootloader consists of a master boot record in flash from address 0x0 to 0x1000 and the actual bootloader application starting at 0xE0000 to the end of flash (0x100000). The bootloader enables firmware updates over USB using nRF Connect for Desktop.

More documentation regarding the open bootloader can be found here: https://infocenter.nordicsemi.com/topic/sdk_nrf5_v17.0.2/ble_sdk_app_open_bootloader.html

Impact of changes

The USB stdio console option is disabled by default and so will not have any impact on existing code.

The regulator initialization at startup will fix issues with on-board LED indicators not lighting up due to the system voltage being set too low. Previously, users would have to execute this configuration from their own main function. This configuration is now implemented as part of the target's initialization code.

Migration actions required

Remove regulator initialization from user application, if required.

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)
[] Tests / results supplied as part of this PR

Reviewers

@farrenv @trowbridgec


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Feb 22, 2021
@ciarmcom ciarmcom requested review from farrenv, trowbridgec and a team February 22, 2021 21:00
@ciarmcom
Copy link
Member

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

@AGlass0fMilk AGlass0fMilk changed the title EP Atlas Target Updates - Regulator configuration and USB stdio [WIP] EP Atlas Target Updates - Regulator configuration and USB stdio Feb 22, 2021
@AGlass0fMilk AGlass0fMilk changed the title [WIP] EP Atlas Target Updates - Regulator configuration and USB stdio [WIP] EP Atlas Target Updates - Regulator configuration, USB stdio, application start address and size Feb 22, 2021
@AGlass0fMilk AGlass0fMilk changed the title [WIP] EP Atlas Target Updates - Regulator configuration, USB stdio, application start address and size EP Atlas Target Updates Feb 22, 2021
@AGlass0fMilk
Copy link
Member Author

Tested and building blinky/printf over USB on Atlas 👍

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.

Otherwise LGTM

@AGlass0fMilk AGlass0fMilk force-pushed the ep-atlas-reg-init branch 2 times, most recently from 1fc52b8 to d58ee83 Compare February 23, 2021 15:03
The `mbed_sdk_init` startup hook is implemented at the NRF52-series level and so is unavailable for override. This commit adds an additional startup hook for NRF52 subtargets to perform any other startup initialization required.
This commit introduces an implementation of the `subtarget_sdk_init` startup hook (called during `mbed_sdk_init`) that configures the internal regulators of the nRF52840.

The configuration sets up the internal regulator to output 3.3V. If this is not done, the default system voltage may be too low for the on-board indicator LEDs to conduct (ie: system voltage is lower than LED forward voltage).
This commit introduces an option, `ep-atlas.enable-usb-stdio-console`, that will retarget the Mbed stdio console handle to a USBSerial instance if enabled.

Please note that if your application uses USB, it will conflict with this option. You should disable this option and implement a composite USB device in your application if you require stdio over USB.

This option is disabled by default so it will not cause issues with existing user code.
This commit introduces a default application start address (`0x1000`) and size limitation (`0xDF000`) to accomodate the Nordic USB bootloader.

The bootloader consists of a master boot record in flash from address `0x0` to `0x1000` and the actual bootloader application starting at `0xE0000` to the end of flash (`0x100000`). The bootloader enables firmware updates over USB using nRF Connect for Desktop.

More documentation regarding the open bootloader can be found here: https://infocenter.nordicsemi.com/topic/sdk_nrf5_v17.0.2/ble_sdk_app_open_bootloader.html
@AGlass0fMilk
Copy link
Member Author

Otherwise LGTM

Alignment issue has been resolved.

#if MBED_CONF_EP_ATLAS_ENABLE_USB_STDIO_CONSOLE

/* Retarget stdio to USBSerial */
mbed::FileHandle *mbed::mbed_target_override_console(int fd) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you run astyle on these new files?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ran astyle -n --options=.astylerc on the new files and pushed up changes.

@mergify mergify bot dismissed 0xc0170’s stale review February 23, 2021 20:44

Pull request has been modified.

0xc0170
0xc0170 previously approved these changes Feb 23, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 23, 2021

Thanks for the fixes. Can you attach testing logs for both toolchains?

@mergify mergify bot added needs: CI and removed needs: review labels Feb 23, 2021
@AGlass0fMilk
Copy link
Member Author

AGlass0fMilk commented Feb 24, 2021

Thanks for the fixes. Can you attach testing logs for both toolchains?

This target is a bit abnormal. It does not have an on-board debugger. It works more like an nRF52840 dongle. The board is programmed with a bootloader that lets you update it over USB using the nRF Connect software.

I'm not sure if Mbed's automated testing tools would support flashing/testing this target...

It's mostly based on the nRF52840_DK so think that is sufficient. There are only some small changes (like this PR) to accommodate the bootloader and hardware regulator configuration.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 25, 2021

CI started

@mergify mergify bot added needs: work and removed needs: CI labels Feb 25, 2021
@mbed-ci
Copy link

mbed-ci commented Feb 25, 2021

Jenkins CI Test : ❌ FAILED

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

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM

@AGlass0fMilk
Copy link
Member Author

I think I need to add the new files to cmake... will have to learn the new tools.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 25, 2021

Please do. It should be fairly simple , if any questions, I can assist.

@AGlass0fMilk
Copy link
Member Author

@0xc0170 How does that look? 🙂 d00699c

@mergify mergify bot dismissed 0xc0170’s stale review February 25, 2021 15:35

Pull request has been modified.

@@ -25,6 +25,13 @@ target_include_directories(mbed-ep-atlas
TARGET_EP_ATLAS
)

target_sources(mbed-ep-atlas
Copy link
Contributor

@0xc0170 0xc0170 Feb 25, 2021

Choose a reason for hiding this comment

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

you need to create the library add_library(mbed-ep-atlas INTERFACE) and possibly later here to link it with nrf52xx library to get common drivers,sdks, etc.

dk board should do that below

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah wait, it's not visible in my view, but I can see it already there

@mergify mergify bot added needs: CI and removed needs: work labels Feb 25, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, please start CI to get the PR merged.

@ciarmcom ciarmcom added the stale Stale Pull Request label Feb 27, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 1, 2021

CI restarted

@mbed-ci
Copy link

mbed-ci commented Mar 1, 2021

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-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_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-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-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit f78c241 into ARMmbed:master Mar 2, 2021
@mergify mergify bot removed the ready for merge label Mar 2, 2021
@mbedmain mbedmain added release-version: 6.9.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Mar 15, 2021
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.

5 participants