-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[NRF5] Bootloader for NRF52_DK @s132 and NRF51_DK @s130 #3376
[NRF5] Bootloader for NRF52_DK @s132 and NRF51_DK @s130 #3376
Conversation
+ either single and dual banks bootloaders for nRF51 and nRF52 (complided to *.hex) + new NRF51_DK_BOOT, NRF52_DK_BOOT, NRF51_DK_OTA, NRF52_DK_OTA for SoftDevice 13x + add support for choice bootloader from a list of a few. Reserve BLE dfu service for nrf5x (previously was for nrf51)
For curious: code with modification of used nrf5 bootloader https://github.com/nvlsianpu/nrf5_bootloader based on nRF5 sdk 11. Be aware of https://infocenter.nordicsemi.com/index.jsp?topic=%2Fcom.nordic.infocenter.sdk5.v11.0.0%2Fbledfu_bootloader_introduction.html Probably you will need to creating a zip with image and init packet. |
SysTick->CTRL = 0; | ||
#endif | ||
// disable all interrupts | ||
core_util_critical_section_enter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm happy with this PR (I need to test it) but I believe that it would be very helpful to link to Nordic documentation (here) and explain on the mbed Nordic page how to switch from single bank to dual bank.
I also wonder if it is possible to add size constraint in the linker script file when dual bank is used or at least explain step by step what has to be done.
It would also be nice to explain to users how to generate a new bootloader (outside this PR).
@@ -2387,11 +2387,12 @@ | |||
"MERGE_SOFT_DEVICE": true, | |||
"EXPECTED_SOFTDEVICES_WITH_OFFSETS": [ | |||
{ | |||
"boot": "", | |||
"boot": ["s130_nrf51_2.0.0_sb_bootloader.hex", "s130_nrf51_2.0.0_sb_bootloader.hex"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the single bank binary is referenced twice ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug
"EXPECTED_SOFTDEVICES_WITH_OFFSETS": [ | ||
{ | ||
"boot": "", | ||
"boot": ["s132_nrf52_2.0.0_db_bootloader.hex", "s132_nrf52_2.0.0_sb_bootloader.hex"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the dual bank bootloader name: s132_nrf52_2.0.0_db_bootloadre.hex
instead of s132_nrf52_2.0.0_db_bootloader.hex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug
I agree. I will update/create FOTA target web immediately after we done this PR
for now I planned to emphasize this in documentation - although I think it's very good idea. But due the time constrains I must wait for a spare time. |
…n 9instead of two single bank bootloaders) fix: typo in dual bank bootloader intel-hex file name fix: remove redundant interupts disabling on bootloader startup call
Maintainers: when running tests, please run a nightly on this since the softdevice will affect all of the tests. |
@nvlsianpu Do you believe this is ready now? All actions done? |
/morph test-nightly |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
It's ready to be merged. As changed are approved I'm going to update DK documentation. |
FYI: |
This will require an alignment with online tools deployment. @theotherjimmy @screamerbg need to sync about when it will be possible. @pan- Did you test this? |
@pan- bump |
@sg- I've tested this, the content of the PR works but it is not that simple to get it working. I would prefer to have this PR integrated in a feature branch rather than in master so the missing items needed to have a good DFU feature like a refresh of the update services and a good documentation will be completed before the integration into master. |
Thanks for the feedback @pan- @nvlsianpu Can you change this PR to be against https://github.com/ARMmbed/mbed-os/tree/feature-nrf5_dfu_s13x_v2 |
Reserve BLE dfu service for nrf5x (previously was for nrf51)
Legacy bootloader's targets (@s110) NRF51_DK_OTA and NRF51_DK_BOOT are renamed to NRF51_DK_LEGACY_OTA and NRF51_DK_LEGACY_BOOT
For support choosing of a bootloader for a target (for now dual bank or single bank solution) the value target.bootloader_select_index was added to target descriptions.
This could be override in a child target or in a application configuration file in order to make a another choice.