-
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 + NRF52840]: Merge nrf52840 to [NRF5] sources #4245
Conversation
TARGET_NRF5_SDK13/sdk -> TARGET_NRF5/TARGET_SDK13 TARGET_NRF5/sdk -> TARGET_NRF5/TARGET_SDK11 TARGET_NRF5_SDK13/TARGET_MCU_NRF52840 -> TARGET_NRF5/TARGET_MCU_NRF52840
HAL driver: Add changes from needad for nrf52840 support us_ticker, spi, sleep, serial, pwmout, pinmap, object, i2c, gpio, analogin Add compatibility patches for: - SoftDevice headers renamed (redirec by a few h files) - sdk configuration (redirect by sdk_config.h files) - renaming of func in softdevice handler module
….x,5.0.0-1.alpha by Copy of changes from features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5_SDK13/source to features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source
…g to NRFFOETT-1674.
I tested BLE applications for all Nordic Soc witch success. mbed test are blocked by #4237. Although this tests pass if unwanted intel-hex record was removed from hex file "by hand". |
A couple of NRF52 DK builds are failing. Neither build has the BLE feature included.
Btw @theotherjimmy, getting |
The patch is already for review, will be fixed soon |
Cool, I figured you'd probably be aware of it already. |
@pan- could you review? |
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.
Thanks, it looks good. I leaved few comments you can address.
I still need to run the test suite on these changes.
@@ -46,15 +56,15 @@ gpio_cfg_t m_gpio_cfg[GPIO_PIN_COUNT]; | |||
|
|||
static gpio_irq_handler m_irq_handler; | |||
static uint32_t m_channel_ids[GPIO_PIN_COUNT] = {0}; | |||
uint32_t m_gpio_irq_enabled; | |||
static uint32_t m_gpio_irq_enabled; |
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.
The type should be gpio_mask_t
.
@@ -79,20 +89,26 @@ void gpio_init(gpio_t *obj, PinName pin) | |||
m_gpio_cfg[obj->pin].used_as_gpio = true; | |||
} | |||
|
|||
#ifdef TARGET_SDK11 |
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.
Is it a polyfill for SDK V11 ? If so, can it be explicitly indicated.
#error not recognized gpio count for mcu | ||
#endif | ||
|
||
#define GPIO_PORT_COUNT (sizeof(m_gpio_pin_count)/sizeof(uint32_t)) |
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.
little note, (sizeof(m_gpio_pin_count)/sizeof(m_gpio_pin_count[0]))
is more correct because it doesn't assume the type of m_gpio_pin_count
as uint32_t
.
US_TICKER_INT_MASK | | ||
NRF_RTC_INT_OVERFLOW_MASK); | ||
#if DEVICE_LOWPOWERTIMER | ||
LP_TICKER_INT_MASK | |
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.
Could you fix the indentation ?
#if DEVICE_LOWPOWERTIMER | ||
LP_TICKER_INT_MASK | | ||
#endif | ||
US_TICKER_INT_MASK | |
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.
Indentation
@@ -0,0 +1 @@ | |||
#include "nrf_ble_hci.h" |
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.
Please remove this header.
@@ -0,0 +1 @@ | |||
#include "nrf_ble_l2cap.h" |
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.
Please remove this header.
@@ -0,0 +1 @@ | |||
#include "nrf_ble_ranges.h" |
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.
Please remove this header.
@@ -0,0 +1 @@ | |||
#include "nrf_ble_types.h" |
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.
Please remove this header.
@@ -167,7 +173,11 @@ ble_error_t nRF5xGap::startAdvertising(const GapAdvertisingParams ¶ms) | |||
(params.getTimeout() > GapAdvertisingParams::GAP_ADV_PARAMS_TIMEOUT_MAX)) { | |||
return BLE_ERROR_PARAM_OUT_OF_RANGE; | |||
} | |||
|
|||
uint32_t err; |
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.
Is it possible to move back err
and adv_para
variables ?
s140 headers renamed form ble_* to nrf_ble_*, Removed s130 and s132 headers named form ble_* (Them had been added by #2ff572682798562e812015dc775b5896e0fda5a4) Headers inclusinons were changed in order to meet above changes. Revrted bad change in us_ticker.c: use __disable_irq lock instead of core_util_critical_section_enter lock for setting rtc1 tick for systick emulation as was good before.
/morph test |
test ble {
"targets": ["NRF51_DK", "NRF52_DK"],
"toolchains": ["GCC_ARM", "ARM"]
} |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
@pan- Did IAR get tested? |
This PR didn't have examples or exporters tested |
https://github.com/ARMmbed/mbed-os-example-ble work fine on nRF52840_DK (although BLE_Button and BLE_GAPButton require new definition of buton pin in mbed_app.json):
|
Description
Former nRF52840 was based on its private api hal drivers and ble implementation. This PR merge those nRF52840 changes to NRF5.
nRF5 SDK base codes and softdevice versions are not changed for mcu targets.
changes:
renamed dir. NRF5/sdk -> NRF5/TARGET_SDK11
moved dir. NRF5_SDK13/sdk -> NR5/TARGET_SDK13
moved dir. NRF5_SDK13/TARGET_MCU_NRF52840 -> NRF5/TARGET_MCU_NRF52840
merged changes introduce to BLE port for compatibility with newest SD API
changes FEATURE_BLE(...)/NRF5_SDK13/.(c,h,cpp) -> FEATURE_BLE(...)/NRF5/.(c,h,cpp)
merged changes introduce to device hal driver port for compatibility with nRF5 SDK13
NRF5_SDK13/.(c,h) -> NRF5/.(c,h)
Added backward compatibility code to API implementation of gpio, pwm, serial, port, spi
Added backward compatibility code to BLE implementation of GAP.
Modified targets description in order to use moved files.
Purpose of changes
This PR purpose is to reduce Nordic maintenance effort. It introduce coherence between NRF51, NRF52, NRF52840 ports.
Status
READY