nRF52: Properly configure nRF SDK for nRF52-series targets #12160

AGlass0fMilk · 2019-12-20T19:01:14Z

Summary of changes

Properly configure Nordic platform resource sharing (PRS) API so it actually delegates the shared interrupt slot for I2C/SPI to the appropriate peripheral.

Before this, the PRS system is enabled but none of the "boxes" are. This caused only the SPI interrupt handler to be properly installed in the NVIC table.

Also updated serial API to be fully migrated to using the UARTE peripheral driver (as opposed to the deprecated UART driver). Disabled the UART and TWIM drivers as these are not used by either serial_api.c nor i2c_api.c and were causing build issues.

Fixes #12159

Impact of changes

Migration actions required

Documentation

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

@desmond-blue

ciarmcom · 2019-12-20T20:00:43Z

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

AGlass0fMilk · 2019-12-20T20:06:14Z

@maclobdell

desmond-blue

Looks good to me.

bulislaw · 2019-12-23T09:42:49Z

CI started

mbed-ci · 2019-12-23T09:58:28Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

adbridge · 2019-12-23T13:37:26Z

Looks like it could be an ARM licence issue , restarted

mbed-ci · 2019-12-23T14:05:20Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

adbridge · 2019-12-24T11:21:12Z

Looks like there is a genuine ARMC error as the following targets all failed:

    Build ARM / MTB_LAIRD_BL652 / Build MTB_LAIRD_BL652 / MTB_LAIRD_BL652-ARM.build.build-greentea_tests
    Build ARM / SDT52832B / Build SDT52832B / SDT52832B-ARM.build.build-greentea_tests
    Build ARM / MTB_MURATA_WSM_BL241 / Build MTB_MURATA_WSM_BL241 / MTB_MURATA_WSM_BL241-ARM.build.build-greentea_tests
    Build ARM / NRF52_DK / Build NRF52_DK / NRF52_DK-ARM.build.build-greentea_tests
    Build ARM / UBLOX_EVK_NINA_B1 / Build UBLOX_EVK_NINA_B1 / UBLOX_EVK_NINA_B1-ARM.build.build-greentea_tests
    Build ARM / MTB_ACONNO_ACN52832 / Build MTB_ACONNO_ACN52832 / MTB_ACONNO_ACN52832-ARM.build.build-greentea_tests
    Build ARM / MTB_UBLOX_NINA_B1 / Build MTB_UBLOX_NINA_B1 / MTB_UBLOX_NINA_B1-ARM.build.build-greentea_tests
    Build ARM / MTB_LAIRD_BL654 / Build MTB_LAIRD_BL654 / MTB_LAIRD_BL654-ARM.build.build-greentea_tests
    Build ARM / EP_AGORA / Build EP_AGORA / EP_AGORA-ARM.build.build-greentea_tests
    Build ARM / NRF52840_DK / Build NRF52840_DK / NRF52840_DK-ARM.build.build-greentea_tests
    Build ARM / SDT52832B / Build SDT52832B / SDT52832B-ARM.build.build-examples
    Build ARM / UBLOX_EVK_NINA_B1 / Build UBLOX_EVK_NINA_B1 / UBLOX_EVK_NINA_B1-ARM.build.build-examples
    Build ARM / MTB_MURATA_WSM_BL241 / Build MTB_MURATA_WSM_BL241 / MTB_MURATA_WSM_BL241-ARM.build.build-examples
    Build ARM / MTB_ACONNO_ACN52832 / Build MTB_ACONNO_ACN52832 / MTB_ACONNO_ACN52832-ARM.build.build-examples
    Build ARM / MTB_LAIRD_BL652 / Build MTB_LAIRD_BL652 / MTB_LAIRD_BL652-ARM.build.build-examples
    Build ARM / MTB_UBLOX_NINA_B1 / Build MTB_UBLOX_NINA_B1 / MTB_UBLOX_NINA_B1-ARM.build.build-examples
    Build ARM / NRF52840_DK / Build NRF52840_DK / NRF52840_DK-ARM.build.build-examples
    Build ARM / MTB_LAIRD_BL654 / Build MTB_LAIRD_BL654 / MTB_LAIRD_BL654-ARM.build.build-examples
    Build ARM / EP_AGORA / Build EP_AGORA / EP_AGORA-ARM.build.build-examples
    Build ARM / NRF52_DK / Build NRF52_DK / NRF52_DK-ARM.build.build-examples

But I cannot see why they failed! @VeliMattiLahtela any idea why these failures are not giving any details ?

0Grit · 2019-12-27T19:56:32Z

@marcuschangarm in case this sounds familiar

AGlass0fMilk · 2020-01-02T19:40:31Z

@adbridge Any update on the source of the build failures? It's probably related to the macros I changed in this PR...

0xc0170 · 2020-01-03T14:21:35Z

@AGlass0fMilk You can review build artifacts, there are multiple defined symbols errors.

[Error] @0,0: L6218E: Undefined symbol nrfx_twim_1_irq_handler (referred from BUILD/EP_AGORA/ARM/mbed-os/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_SDK_15_0/modules/nrfx/drivers/src/nrfx_twim.o).

Same file as changed here.

AGlass0fMilk · 2020-01-06T21:38:48Z

@0xc0170 We actually don't use nrfx_twim.c as far as I know. The I2C driver used for master mode is nrfx_twi.c. I'll look into this more.

AGlass0fMilk · 2020-01-09T21:14:50Z

Alright, I think I found the issue. For some reason only ARMC6 was failing even though the Nordic SDK was still pretty misconfigured.

Serial API/UART Fix

It seems at some point the serial_api.c implementation migrated from using the (deprecated) UART peripheral and started using the UARTE (low-power UART) peripheral present on nRF52-series devices.

Whoever did that migration never changed all the types to use the corresponding types from nrf_uarte.h (they are identical to nrf_uart.h, but it's the principle!). Thus they also left NRFX_UART and NRF_UART, etc, enabled (which the serial_api.c implementation no longer uses in lieu of NRFX_UARTE).

It seems the PRS system doesn't allow the use of the UART and UARTE peripherals at the same time (they are functionally the same and share the same memory-mapped location in some cases) so that was causing the undefined symbol the linker was looking for. This has been fixed by completely migrating from UART to UARTE and disabling the deprecated UART peripheral driver.

I2C Fix

Similar to the UART sitatuion, there are several peripherals that support I2C master mode. Namely, TWI and TWIM. In Mbed's case, we're using TWI (the older, deprecated peripheral supporting I2C master). The developer implementing the i2c_api.c support for nRF52 must have misunderstood the distinction and kept both TWI and TWIM peripherals enabled -- causing conflicts.

~~In the same way, the PRS system does not allow TWI and TWIM to be used at the same time.~~

This issue was fixed by simply disabling the TWIM peripheral driver in the SDK. The build runs fine, still need to test on a real target but the CI should do that...

Since the TWI peripheral used by Mbed-OS's nRF52 I2C master implementation is "deprecated" it should eventually be migrated to use the TWIM. The I2CSlave implementation is already based on the (newer) TWIS peripheral so no migration effort is needed there.

I don't think there's much priority to migrating until Mbed starts supporting the newer Nordic chips (91-series, 53-series, etc)

@0xc0170

…upt slot to the appropriate peripheral

…nused TWIM peripheral (deprecated TWI peripheral is currently used for I2C master mode operation).

AGlass0fMilk · 2020-01-09T21:46:38Z

Momentarily screwed up the history with the rebase but I think I fixed it 😅

AGlass0fMilk · 2020-01-09T23:54:19Z

The striked out assumptions above are wrong upon closer examination:

What's really happening is when the PRS system is disabled, the IRQs for each peripheral driver actually conflict since they all point to the same function definition (see excerpt below in the case of nRF52840):

mbed-os/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_SDK_15_0/modules/nrfx/soc/nrfx_irqs_nrf52840.h

Lines 62 to 84 in 19e7622

    
           // SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQn 
        
           #if NRFX_CHECK(NRFX_PRS_ENABLED) && NRFX_CHECK(NRFX_PRS_BOX_0_ENABLED) 
        
           #define nrfx_prs_box_0_irq_handler  SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #else 
        
           #define nrfx_spim_0_irq_handler     SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #define nrfx_spis_0_irq_handler     SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #define nrfx_twim_0_irq_handler     SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #define nrfx_twis_0_irq_handler     SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #define nrfx_spi_0_irq_handler      SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #define nrfx_twi_0_irq_handler      SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler 
        
           #endif 
        
           // SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQn 
        
           #if NRFX_CHECK(NRFX_PRS_ENABLED) && NRFX_CHECK(NRFX_PRS_BOX_1_ENABLED) 
        
           #define nrfx_prs_box_1_irq_handler  SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #else 
        
           #define nrfx_spim_1_irq_handler     SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #define nrfx_spis_1_irq_handler     SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #define nrfx_twim_1_irq_handler     SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #define nrfx_twis_1_irq_handler     SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #define nrfx_spi_1_irq_handler      SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #define nrfx_twi_1_irq_handler      SPIM1_SPIS1_TWIM1_TWIS1_SPI1_TWI1_IRQHandler 
        
           #endif

With the PRS system enabled properly, the interrupts handlers then become locally-declared functions that are then installed by the PRS when nrfx_prs_acquire is called. This avoids an IRQ symbol conflict if multiple peripherals are used that share the same underlying memory region.

Once I enabled the PRS "boxes" then all the IRQ handlers for each driver became locally-declared functions.

The failure we were seeing here was that the previous developer had preprocessored-out (using #if 0) the local declarations of each handler so the symbols no longer existed:

mbed-os/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_SDK_15_0/modules/nrfx/drivers/src/nrfx_twi.c

Lines 709 to 725 in ea3ead0

    
           #if 0 
        
           #if NRFX_CHECK(NRFX_TWI0_ENABLED) 
        
           void nrfx_twi_0_irq_handler(void) 
        
           { 
        
               twi_irq_handler(NRF_TWI0, &m_cb[NRFX_TWI0_INST_IDX]); 
        
           } 
        
           #endif 
        
           #endif 
        
           #if 0 
        
           #if NRFX_CHECK(NRFX_TWI1_ENABLED) 
        
           void nrfx_twi_1_irq_handler(void) 
        
           { 
        
               twi_irq_handler(NRF_TWI1, &m_cb[NRFX_TWI1_INST_IDX]); 
        
           } 
        
           #endif 
        
           #endif

The reason it would build before was because none of the boxes were enabled so the symbols were all redirected to the global interrupt symbol for each "box" (eg: SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandler).

I am surprised we haven't seen any failures/issues related to this. It would only affect users trying to use asynchronous I2C and asynchronous SPI on the same instance I think...

Regardless, I think the PR fixes all these issues and resources should be shared appropriately now.

0xc0170 · 2020-01-10T14:20:10Z

CI started

mbed-ci · 2020-01-10T14:46:32Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2020-01-10T15:05:48Z

[Error] pal_uart.c@55,0: [Pe094]: the size of an array must be greater than zero

IAR failed , please review

… port uses UARTE peripheral)

AGlass0fMilk · 2020-01-10T17:06:21Z

Fixed outdated references to UARTx_ENABLED preprocessor macros in lieu of new NRFX_UARTEx_ENABLED macros.

@0xc0170

mbed-ci · 2020-01-13T11:00:33Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 4
Build artifacts

AGlass0fMilk mentioned this pull request Dec 20, 2019

nRF52 I2CSlave Implementation #12161

Merged

ciarmcom requested review from desmond-blue and a team December 20, 2019 20:00

ciarmcom added the needs: review label Dec 20, 2019

desmond-blue approved these changes Dec 23, 2019

View reviewed changes

bulislaw approved these changes Dec 23, 2019

View reviewed changes

bulislaw added needs: CI and removed needs: review labels Dec 23, 2019

bulislaw added the CI started label Dec 23, 2019

adbridge added needs: work and removed CI started needs: CI labels Dec 24, 2019

AGlass0fMilk changed the title ~~[nRF52] Properly configure PRS API~~ [nRF52] Properly configure nRF SDK for nRF52-series targets Jan 9, 2020

AGlass0fMilk added 2 commits January 9, 2020 16:35

Properly configure PRS API so it actually delegates the shared interr…

b6befee

…upt slot to the appropriate peripheral

Remove use of deprecated UART driver (as opposed to UARTE). Disable u…

dc19460

…nused TWIM peripheral (deprecated TWI peripheral is currently used for I2C master mode operation).

AGlass0fMilk force-pushed the fix-prs-config branch from da5708f to dc19460 Compare January 9, 2020 21:45

AGlass0fMilk mentioned this pull request Jan 9, 2020

[Nordic] TWI/I2C interrupts never get installed #12159

Closed

0xc0170 added needs: CI and removed needs: work labels Jan 10, 2020

0xc0170 added needs: work and removed needs: CI labels Jan 10, 2020

AGlass0fMilk added 3 commits January 10, 2020 11:30

Replace preprocessor "UARTx_ENABLED" with "NRFX_UARTEx_ENABLED".

cb4f3db

Update SDK config for nRF52840 to disable UART1 instance (Mbed serial…

dd98d01

… port uses UARTE peripheral)

Replace macro "UART1_ENABLED" with corresponding "NRFX_UARTE1_ENABLED".

d8fbfbf

0xc0170 added needs: CI and removed needs: work labels Jan 13, 2020

0xc0170 changed the title ~~[nRF52] Properly configure nRF SDK for nRF52-series targets~~ nRF52: Properly configure nRF SDK for nRF52-series targets Jan 13, 2020

0xc0170 added ready for merge release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 and removed needs: CI ready for merge labels Jan 13, 2020

0xc0170 merged commit cbca99d into ARMmbed:master Jan 15, 2020

AGlass0fMilk deleted the fix-prs-config branch January 16, 2020 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nRF52: Properly configure nRF SDK for nRF52-series targets #12160

nRF52: Properly configure nRF SDK for nRF52-series targets #12160

AGlass0fMilk commented Dec 20, 2019 •

edited

Loading

ciarmcom commented Dec 20, 2019

AGlass0fMilk commented Dec 20, 2019

desmond-blue left a comment

bulislaw commented Dec 23, 2019

mbed-ci commented Dec 23, 2019

adbridge commented Dec 23, 2019

mbed-ci commented Dec 23, 2019

adbridge commented Dec 24, 2019

0Grit commented Dec 27, 2019

AGlass0fMilk commented Jan 2, 2020

0xc0170 commented Jan 3, 2020 •

edited

Loading

AGlass0fMilk commented Jan 6, 2020

AGlass0fMilk commented Jan 9, 2020 •

edited

Loading

AGlass0fMilk commented Jan 9, 2020

AGlass0fMilk commented Jan 9, 2020

0xc0170 commented Jan 10, 2020

mbed-ci commented Jan 10, 2020

0xc0170 commented Jan 10, 2020

AGlass0fMilk commented Jan 10, 2020

mbed-ci commented Jan 13, 2020

nRF52: Properly configure nRF SDK for nRF52-series targets #12160

nRF52: Properly configure nRF SDK for nRF52-series targets #12160

Conversation

AGlass0fMilk commented Dec 20, 2019 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Dec 20, 2019

AGlass0fMilk commented Dec 20, 2019

desmond-blue left a comment

Choose a reason for hiding this comment

bulislaw commented Dec 23, 2019

mbed-ci commented Dec 23, 2019

Test run: FAILED

adbridge commented Dec 23, 2019

mbed-ci commented Dec 23, 2019

Test run: FAILED

adbridge commented Dec 24, 2019

0Grit commented Dec 27, 2019

AGlass0fMilk commented Jan 2, 2020

0xc0170 commented Jan 3, 2020 • edited Loading

AGlass0fMilk commented Jan 6, 2020

AGlass0fMilk commented Jan 9, 2020 • edited Loading

Serial API/UART Fix

I2C Fix

AGlass0fMilk commented Jan 9, 2020

AGlass0fMilk commented Jan 9, 2020

0xc0170 commented Jan 10, 2020

mbed-ci commented Jan 10, 2020

Test run: FAILED

0xc0170 commented Jan 10, 2020

AGlass0fMilk commented Jan 10, 2020

mbed-ci commented Jan 13, 2020

Test run: SUCCESS

AGlass0fMilk commented Dec 20, 2019 •

edited

Loading

0xc0170 commented Jan 3, 2020 •

edited

Loading

AGlass0fMilk commented Jan 9, 2020 •

edited

Loading