[feature-nrf528xx] NRF52832 and NRF52840 upgraded to Nordic SDK 14 and SoftDevice 5.0 #5845
Conversation
marcuschangarm
changed the title
|
Please review. Note the peripherals are still buggy. I would like to get this into the feature branch so that we can tag-team the missing features. Overview:
|
|
this would be easier to review if it were split up into more commits. |
|
@jrobeson good point. I've split the initial commit into one big one and several smaller ones. This PR is mainly the insertion of a new SDK and the copying over of files already in mbed OS. Once we get this new structure into a feature branch the real porting work of the missing non-working peripherals can begin. |
|
Is there a reason to keep SDK13? I've not dealt with nordic devices long eough to know if that's a thing people do. I see at least one TARGET_SDK13 check in there. |
|
That would be at a later stage:
|
|
Thanks for your efor @marcuschangarm ! We are analyzing SDK/Softdevice updates of our ports. For Nordic port update we are expecting much more comitment - especiali BLE-port reworking, testing, hal driver fixing, hal driver testing. |
|
What I can suggestion Is that you can suspend this PR and wait for us a little (Can you answer to questions what your timeline?, which SoC do you want to use 1st with updated SDK? what company you represent?). |
|
BTW - I really like idea of renaming softdevice directories in the way that allow to chose proper softdevice using the mbed build system. |
|
Maintenance wise, I don't see how keeping the nrf51 and nrf52 HAL together will make things easier if that's what you mean by avoiding duplication? If you want to take full advantage of the nrf52's new features the two implementations are going to be vastly different anyway. |
|
Regarding the time constraint, we need this work to proceed right now. Can you offer a folder structure you would be happy with? |
|
This is only a PR to a feature branch that is served as discussion and collaboration with the Nordic team and the mutual developers/customers of this work. |
|
I'm glad to see this effort. It'd be nice to have signed boot images and the ability to use more code found on the internet that requires at least sdk 12. SDK 11 is just too old nowadays. |
Experience: We use to have separte ports for nrf51, nrf52823, nrf52840 before - amount of work needed for just supervising and bug-fixing was much more bigger than for currently one (TARGET_NRF5). Folder structure:
After you add SDK14_2, it is need to rewrite the porting layer for the included modules to map to the changes in API's between SDK 11 and 14.2. Obviously it is reasonably to use macro TARGET_SDK14_2 for #ifdef with changes. This could be useful information: |
We're refactoring the SecurityManager module and adding a platform adaptation layer for it in the process. We look at the pure soft device API's to implement the PAL SecurityManager. |
I would have to disagree. This only works if the mbed HAL implementation serves the lowest common denominator between the different MCUs and then you miss out on important features which is why people pick the more advanced MCU anyway. The current setup also makes it very difficult for people to contribute because you need knowledge of and access to all three devices to make any changes. How about this compromise instead:
Common mbed HAL implementations that are truly shareable between NRF51 and NRF52 go in the TARGET_NRF5x root folder. Peripherals that are different between NRF51 and NRF52 get their own mbed HAL implementation which goes into TARGET_NRF51 and TARGET_NRF52 respectively. This is basically the setup every other partner implementation uses. |
|
What @marcuschangarm explained above is also valid for the largest vendor family on mbed - STM32 (https://github.com/ARMmbed/mbed-os/tree/master/targets/TARGET_STM). Some of the implemented HAL features that are common for ALL STM32 families, while others are common per sub-family (STM32L0, STM32F4, etc), and some other are target specific. @nvlsianpu I doubt that the nRF chips are vastly different, otherwise we wouldn't have SDK11 covering nRF51-DK and nRF52-DK (nRF52832), but also SDK13 compatible with both nRF52-DK and nRF52840-DK. @0xc0170 This is a feature branch work and as such, this doesn't impact any of the release targets on the mbed OS official releases. As @MarceloSalazar described above, this is established process, similarly on how the Arm Mbed team is introducing new APIs and features on feature branches. We would like to use this feature branch as a collaborative effort to achieve better nRF5x support. Is there anything blocking this PR? |
|
/morph build |
Build : FAILUREBuild number : 884 |
Yes, exactly they are so similar. BTW Even now nrf5 port is close to that what was proposed above. |
So you are fine with the new structure I proposed? |
Initial commit for separating NRF52 code from the NRF51 and for combining the NRF52832 and NRF52840 SDK to version 14. nRF5_SDK_14.2.0_17b948a.zip has been used as baseline. The folders in SDK/components: * ble * boards * device * drivers_nrf * libraries * softdevice have been copied to TARGET_NORDIC/TARGET_NRF52/TARGET_SDK14. Each folder contains a README.md file describing any modifications made to that particular folder. Most common operation is deletion of files. If the need arise in the future the missing files can be added again. The SoftDevices have been moved to TARGET_SOFTDEVICE_x folders for flexible selection. ble.h has been renamed to nrf_ble.h to avoid namespace clash with mbed OS BLE.
Flash and RAM offsets have been modified to fit new SoftDevice
Flash and RAM offsets have been modified to fit new SoftDevice
Copied from previous location in targets/TARGET_NORDIC/TARGET_NRF5 Core functionality is working but some peripherals do not.
|
/morph build |
Build : SUCCESSBuild number : 888 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 561 |
Test : SUCCESSBuild number : 733 |
|
@marcuschangarm I don't understand why you created brand new TARGET_NRF5x - why not put changes to currently existed TARGET_NRF5 - apart for that directory structure reorganization you proposed is fine. |
|
I want to keep a clean separation during the porting efforts to untangle dependencies. Once everything is done we can rename the directory back to NRF5 if you want. |
|
Holding off on merging until @nvlsianpu gives the ok. Otherwise, this PR is good to go. |
@nvlsianpu Does it make sense to keep this separation for now, this will be cleaned later? We would like to continue the work on the branch |
|
I was waiting on answer regard this for my management :( . For me this merge is OK - I didn't think we could dismiss this, I appreciated this efforts.
That's fine! |
|
Curious on the timeframe when the porting for the MBED BLE layer will be done to accommodate these changes? |
|
@rerickson1 we are triaging the BLE part of the problem at the moment so unfortunately I don't have a timeframe yet. |
Initial commit for separating NRF52 code from the NRF51 and for
combining the NRF52832 and NRF52840 SDK to version 14.
nRF5_SDK_14.2.0_17b948a.zip has been used as baseline.
The folders in SDK/components:
have been copied to TARGET_NORDIC/TARGET_NRF52/TARGET_SDK14.
Each folder contains a README.md file describing any modifications
made to that particular folder. Most common operation is deletion
of files.
If the need arise in the future the missing files can be added again.
The SoftDevices have been moved to TARGET_SOFTDEVICE_x folders for
flexible selection. Each folder contains a separate version of
SDK/config/sdk_config.h