Bluetooth: controller: nRF21540 FEM support for nRF52x and nRF53x Series #31336

cvinayak · 2021-01-15T04:52:21Z

Added nRF21540 FEM support for nRF52x and nRF53x Series.

Thanks to @bjornspock for the FEM contributions and @auroraslb for the DPPI refactoring.

~~Contains cherry-picked commit from @auroraslb PR: #30732~~

carlescufi · 2021-02-18T12:42:07Z

@cvinayak this FEM configuration needs to be in Devicetree, not in Kconfig. @KAGA164 might have some input here.

KAGA164 · 2021-02-18T14:07:26Z

I agree with @carlescufi the FEM hardware configuration like pins should be in the device tree. We already have bindings for the nRF21540 FEM in the Zephyr. Also the FEM configuration is added to the nrf21540dk_nrf52840 dts file

zephyr/boards/arm/nrf21540dk_nrf52840/nrf21540dk_nrf52840.dts

Line 113 in a4f0ca8

nrf_radio_fem: nrf21540_fem {

and

zephyr/boards/arm/nrf21540dk_nrf52840/nrf21540dk_nrf52840.dts

Line 216 in a4f0ca8

fem_spi: &spi3 {

. I would like to add also a support for nrf21540EK which is currently in the PR #31523 but it seems to be blocked because we have only use case for it in the NCS not in the Zephyr. The BLE Controller should automatically enable support for FEM if its configuration is in the devicetree then we could easy build samples for nrf21540dk and also when the EK shield is used we could enable it when west build is invoked

cvinayak · 2021-02-18T14:32:06Z

@carlescufi and @KAGA164 This PR is collection of other authors works. I do not have the knowledge or time in the near future to rework, there is already a related issue #30708, if someone wants to work on it, they can use this PR as base and add commits to port to DTS macros.

carlescufi · 2021-02-23T18:34:04Z

@auroraslb or @KAGA164 could you please pick this up and rework it accordingly?

mbolivar-nordic · 2021-02-23T23:57:16Z

@auroraslb or @KAGA164 could you please pick this up and rework it accordingly?

@carlescufi does that mean I should not work on #30708? Just clarifying.

cvinayak · 2021-02-24T02:17:40Z

@carlescufi This PR is relatively big in terms of feature support, should be reviewed as-is and merged. The DT porting should be addressed in #30708, which can be done by @mbolivar and I can assist in its review and testing.

Hope the functional changes in this PR gets some reviews, else this is now entering the merge conflicts due to other PRs modifying the files common to this PR.

carlescufi · 2021-03-31T09:16:55Z

@auroraslb or @KAGA164 could you please pick this up and rework it accordingly?

@carlescufi does that mean I should not work on #30708? Just clarifying.

Sorry for the delay. No, apparently no one is working on that, so you can feel free to pick that up in fact.

mbolivar-nordic · 2021-04-01T17:42:20Z

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_dppi.h

The commit log says the motivation is "so that DPPI 0-4 available for application's use" -- where is that going to be documented?

More generally, PPI values seem like something that should be moved into DT, since they are boot time hardware configuration values. Another benefit would be that users could override this macro with an overlay as they desire.

Would you agree? If so, that also would solve the documentation problem if there is no existing doc, because the pages in the DT bindings index would be the docs.

The commit log says the motivation is "so that DPPI 0-4 available for application's use" -- where is that going to be documented?

I thought this would document it:

zephyr/modules/hal_nordic/nrfx/nrfx_glue.h

Line 229 in 45cdc05

/** @brief Bitmask that defines PPI channels that are reserved for use outside of the nrfx library. */

PPI values seem like something that should be moved into DT

Agree.

(Just thinking aloud, PPI/DPPIs and other peripherals are used by controller at scheduled time periods, they can but are not currently been used outside the Zephyr controller. nRF Connect SD controller based application do use outside the controller's scheduled time periods).

mbolivar-nordic · 2021-04-01T17:48:25Z

subsys/bluetooth/controller/Kconfig.ll_sw_split

Forgive my ignorance, but are these Kconfig options being copy/pasted from somewhere else for a different SoC?

If not, they should definitely be in DT instead, along with the polarity (since we have the GPIO_ACTIVE_LOW flag in DTS), CSN pin, etc.

I am not against use of DT, it is just that original controller contribution was before use of DT, and not all of controller has been ported to DT use.

they should definitely be in DT

Agree. As a subsequent PR, please do work on the port.

mbolivar-nordic · 2021-04-01T17:48:53Z

subsys/bluetooth/controller/Kconfig.ll_sw_split

This looks like a copy/paste error.

yes. Will fix it.

mbolivar-nordic · 2021-04-01T17:49:07Z

subsys/bluetooth/controller/Kconfig.ll_sw_split

yes, will fix it.

carlescufi · 2021-04-06T11:59:09Z

@mbolivar-nordic @anangl @rugeGerritsen @KAGA164 could you please review?

mbolivar-nordic

Looks OK to the best of my limited ability to review. I'll take on conversion of pin Kconfigs to DT as follow up PRs.

anangl · 2021-04-20T07:08:02Z

subsys/bluetooth/controller/Kconfig.ll_sw_split

... PDN pin.

anangl · 2021-04-20T07:09:07Z

subsys/bluetooth/controller/Kconfig.ll_sw_split

Perhaps it would be good to mention what's the unit of this value?

anangl · 2021-04-20T07:11:01Z

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c

Suggested change

#define NRF_GPIO_CSN_PIN ((CONFIG_BT_CTLR_GPIO_PA_PIN) - 32)

#define NRF_GPIO_CSN_PIN ((CONFIG_BT_CTLR_GPIO_CSN_PIN) - 32)

anangl · 2021-04-20T08:30:39Z

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_dppi.h

These channels should be added to HAL_USED_PPI_CHANNELS, similarly to how it is done for the PA/LNA ones: https://github.com/zephyrproject-rtos/zephyr/blob/82a55328ccc96abeff252dc63e73b3577973abf0/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_dppi.h#L582-L588

Same applies to the corresponding change in radio_nrf5_ppi.h.

anangl · 2021-04-20T08:37:35Z

subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_dppi.h

It looks like it should be:

Suggested change

#define HAL_ENABLE_FEM_PPI 4

#define HAL_DISABLE_FEM_PPI HAL_ENABLE_PALNA_PPI

#define HAL_ENABLE_FEM_PPI 4

#define HAL_DISABLE_FEM_PPI HAL_ENABLE_FEM_PPI

Otherwise, the PDN and CSN pins will be toggled on NRF_TIMER_EVENT_COMPARE3 and NRF_RADIO_EVENT_DISABLED but also on NRF_TIMER_EVENT_COMPARE2 (what would be different from the PPI version) as this event is published to the same channel.

~~yes, you are right, this would not work. I need to discuss with @bjornspock~~

The solution was simple reusing existing GPIOTEs, but an additional DPPI... The implementation for PA/LNA will be same as before, when not using FEM then will use GPIOTE OUT Task Toggle mode. With FEM, GPIOTE SET and CLR task will be used for each GPIO PIN, i.e. 1 DPPI for PA or LNA. 1 for PDN+CSN and 1 to CLR all three GPIO on Radio Disable.

cvinayak

@anangl please re-review.

anangl

LGTM now.

Adjust the PPI used by nRF53x so that DPPI 0-4 available for application's use. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Ported the GPIO PA/LNA support for nRF53x by using a DPPI channel. Fixes zephyrproject-rtos#24142. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Added correct SOC_NRF5340_CPUNET pin range for BT_CTLR_GPIO_PA_PIN and BT_CTLR_GPIO_LNA_PIN. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Added Kconfig options for nRF21540 PDN and CSN pins. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Added GPIO control for nRF21540 CSN and PDN lines. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Added nRF21540 FEM support for nRF52x Series. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

cvinayak requested review from anangl and rugeGerritsen January 15, 2021 04:52

cvinayak requested review from carlescufi, kruithofa and thoh-ot as code owners January 15, 2021 04:52

github-actions bot added area: Bluetooth area: Bluetooth Controller labels Jan 15, 2021

cvinayak mentioned this pull request Jan 15, 2021

Bluetooth: controller: GPIO PA/LNA support for nRF53x #30950

Closed

cvinayak force-pushed the github_nrf21540_support branch from 589590c to 0e4563f Compare January 15, 2021 05:00

zephyrbot assigned cvinayak Jan 15, 2021

cvinayak added this to the v2.6.0 milestone Jan 28, 2021

carlescufi mentioned this pull request Feb 4, 2021

BLE SW controller PA/LNA pins configuration in device tree #30708

Closed

carlescufi requested a review from ioannisg February 18, 2021 12:41

carlescufi requested a review from KAGA164 February 18, 2021 12:42

cvinayak closed this Mar 1, 2021

cvinayak deleted the github_nrf21540_support branch March 1, 2021 00:33

cvinayak restored the github_nrf21540_support branch March 1, 2021 00:54

cvinayak reopened this Mar 1, 2021

cvinayak marked this pull request as draft March 1, 2021 00:55

cvinayak mentioned this pull request Mar 31, 2021

bluetooth: controller: radio: Use HAL API in radio_nrf5_ppi #30732

Merged

cvinayak force-pushed the github_nrf21540_support branch from 0e4563f to 66caaaf Compare March 31, 2021 09:48

cvinayak marked this pull request as ready for review March 31, 2021 09:48

cvinayak requested a review from mbolivar-nordic March 31, 2021 09:49

mbolivar-nordic reviewed Apr 1, 2021

View reviewed changes

cvinayak requested a review from mbolivar-nordic April 2, 2021 00:58

cvinayak force-pushed the github_nrf21540_support branch from 66caaaf to 82a5532 Compare April 3, 2021 01:47

cvinayak added the Enhancement Changes/Updates/Additions to existing features label Apr 6, 2021

cvinayak mentioned this pull request Apr 19, 2021

Bluetooth: controller: nRF5: Back-to-Back Radio Tx interface #33118

Merged

mbolivar-nordic approved these changes Apr 19, 2021

View reviewed changes

anangl requested changes Apr 20, 2021

View reviewed changes

cvinayak force-pushed the github_nrf21540_support branch from 82a5532 to 1d21721 Compare April 20, 2021 12:52

carlescufi requested a review from anangl April 20, 2021 15:21

cvinayak force-pushed the github_nrf21540_support branch from 1d21721 to cb86e59 Compare April 20, 2021 16:09

cvinayak commented Apr 20, 2021

View reviewed changes

anangl approved these changes Apr 20, 2021

View reviewed changes

cvinayak and others added 6 commits April 21, 2021 06:58

Bluetooth: controller: Adjust DPPI used for nRF53x

a250f30

Adjust the PPI used by nRF53x so that DPPI 0-4 available for application's use. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: controller: GPIO PA/LNA support for nRF53x

0ab7313

Ported the GPIO PA/LNA support for nRF53x by using a DPPI channel. Fixes zephyrproject-rtos#24142. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: controller: Add PA/LNA pin range for nRF5340 SoC

0f53b9c

Added correct SOC_NRF5340_CPUNET pin range for BT_CTLR_GPIO_PA_PIN and BT_CTLR_GPIO_LNA_PIN. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: controller: Add Kconfig options for nRF21540 PDN and CSN pins

c28f326

Added Kconfig options for nRF21540 PDN and CSN pins. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: controller: Add GPIO control for nRF21540 CSN and PDN lines

1b0e05c

Added GPIO control for nRF21540 CSN and PDN lines. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: controller: nRF21540 FEM support for nRF52x Series

440fc6f

Added nRF21540 FEM support for nRF52x Series. Signed-off-by: Bjørn Spockeli <[email protected]> Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

cvinayak force-pushed the github_nrf21540_support branch from cb86e59 to 440fc6f Compare April 21, 2021 01:29

carlescufi merged commit 9c871fb into zephyrproject-rtos:master Apr 21, 2021

	#define NRF_GPIO_CSN_PIN ((CONFIG_BT_CTLR_GPIO_PA_PIN) - 32)
	#define NRF_GPIO_CSN_PIN ((CONFIG_BT_CTLR_GPIO_CSN_PIN) - 32)

-#define HAL_ENABLE_FEM_PPI 4
-#define HAL_DISABLE_FEM_PPI HAL_ENABLE_PALNA_PPI
+#define HAL_ENABLE_FEM_PPI 4
+#define HAL_DISABLE_FEM_PPI HAL_ENABLE_FEM_PPI

Uh oh!

Bluetooth: controller: nRF21540 FEM support for nRF52x and nRF53x Series #31336

Bluetooth: controller: nRF21540 FEM support for nRF52x and nRF53x Series #31336

Uh oh!

Conversation

cvinayak commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlescufi commented Feb 18, 2021

Uh oh!

KAGA164 commented Feb 18, 2021

Uh oh!

cvinayak commented Feb 18, 2021

Uh oh!

carlescufi commented Feb 23, 2021

Uh oh!

mbolivar-nordic commented Feb 23, 2021

Uh oh!

cvinayak commented Feb 24, 2021

Uh oh!

carlescufi commented Mar 31, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlescufi commented Apr 6, 2021

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvinayak Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvinayak left a comment

Choose a reason for hiding this comment

Uh oh!

anangl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cvinayak commented Jan 15, 2021 •

edited

Loading

cvinayak Apr 20, 2021 •

edited

Loading