Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling #74183

cvinayak · 2024-06-12T16:44:51Z

Use NRF_GRTC, Global real-time counter, instead of NRF_RTC.

NRF_GRTC is present in a different power domain in the SoC.
Also, NRF_GRTC provide microsecond resolution timing units,
in the future use of NRF_TIMER for packet timer can also be
replaced.

Use of NRF_RTC would keep Radio power domain ON in addition
to already ON NRF_GRTC's power domain, hence switch to using
NRF_GRTC on nRF54Lx SoC to have lower power consumptions.

Add ticker implementation support for free running counter use.

Thalley · 2024-06-14T08:22:42Z

tests/bluetooth/ctrl_isoal/src/sub_sets/isoal_test_rx.c

+	time_diff = (time_wrapping_point == UINT32_MAX) ?
+		    INT32_MIN :
+		    -((int64_t)ISOAL_TIME_WRAPPING_POINT_US);


time_diff is int32_t, so it doesn't make sense to cast to int64_t

Compiler did not like when ISOAL_TIME_WRAPPING_POINT_US is 32-bit unsigned int, as signed values need an extra bit to represent them in 2's compliment, hence have to typecast to 64-bit, then the compiler is ok to assign it to int32_t.

@mtpr-ot or @niag-Oticon or @Tronil can comment here.

I think it looks alright. With this timer I would expect time_wrapping_point == UINT32_MAX to be true so the value actually assigned should be INT32_MIN.

Compiler did not like when ISOAL_TIME_WRAPPING_POINT_US is 32-bit unsigned int, as signed values need an extra bit to represent them in 2's compliment, hence have to typecast to 64-bit, then the compiler is ok to assign it to int32_t.

What is the compiler error/warning?

Depending on what parameters is provided to e.g. GCC, it would also give an error/warning if you assigned a int64_t to a int32_t, so I would suggest to do it properly than just replacing one compiler error/warning with a future error/warning.

One easy solution would be to simply have time_diff as int64_t and then if the result actually fits in a int32_t, then we can cast the result.

The compile error was:

/home/jack/workspace/zephyrproject/zephyr/tests/bluetooth/ctrl_isoal/src/sub_sets/isoal_test_rx.c: In function 'test_rx_unframed_test_rx_time_wrapping':
/home/jack/workspace/zephyrproject/zephyr/tests/bluetooth/ctrl_isoal/src/sub_sets/isoal_test_rx.c:928:21: warning: overflow in conversion from 'long long unsigned int' to 'int32_t' {aka 'int'} changes value from '18446744073197551647' to '-511999969' [-Woverflow]
928 | time_diff = (time_wrapping_point == UINT32_MAX ? INT32_MIN : -ISOAL_TIME_WRAPPING_POINT_US);
| ^
/home/jack/workspace/zephyrproject/zephyr/tests/bluetooth/ctrl_isoal/src/sub_sets/isoal_test_rx.c:935:21: warning: overflow in conversion from 'long long unsigned int' to 'int32_t' {aka 'int'} changes value from '18446744073197551647' to '-511999969' [-Woverflow]
935 | time_diff = (time_wrapping_point == UINT32_MAX ? INT32_MIN : -ISOAL_TIME_WRAPPING_POINT_US);
| ^

Thalley · 2024-06-14T08:22:50Z

tests/bluetooth/ctrl_isoal/src/sub_sets/isoal_test_rx.c

+	time_diff = (time_wrapping_point == UINT32_MAX) ?
+		    INT32_MIN :
+		    -((int64_t)ISOAL_TIME_WRAPPING_POINT_US);


same as in https://github.com/zephyrproject-rtos/zephyr/pull/74183/files#r1639463489

Same as above.

subsys/bluetooth/controller/Kconfig.ll_sw_split

subsys/bluetooth/controller/ticker/ticker.c

subsys/bluetooth/controller/Kconfig.ll_sw_split

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

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

subsys/bluetooth/controller/ll_sw/nordic/lll/lll.c

samples/bluetooth/hci_uart/boards/nrf54l15pdk_nrf54l15_cpuapp_df.overlay

Fix BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE Kconfig depends on. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Fix software tIFS switching using single timer from triggering spurious TXEN/RXEN if the first remainder is shorter than the tIFS delay calculated in the sw_switch() function. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Fix Extended Connection Creation when using single timer feature in the Controller. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Refactor sw_switch hal interface use and document why some PPI/DPPI channel group related subscriptions are not disabled explicitly. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Remove redundant HAL_TICKER_CNTR_CLK_FREQ_HZ define. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Add HAL_TICKER_TICKS_TO_US_64BIT define, to return microsecond value in 64-bits. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Add ticker implementation support for free running counter use. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

nRF54Lx have updated Data Whitening register settings, add implementation to correctly set them up. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use NRF_GRTC, Global real-time counter, instead of NRF_RTC. NRF_GRTC is present in a different power domain in the SoC. Also, NRF_GRTC provide microsecond resolution timing units, in the future use of NRF_TIMER for packet timer can also be replaced. Use of NRF_RTC would keep Radio power domain ON in addition to already ON NRF_GRTC's power domain, hence switch to using NRF_GRTC on nRF54Lx SoC to have lower power consumptions. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Add support for Radio fast ramp up for nRF54Lx SoCs. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Add preliminary direction finding support building hci_uart sample for nrf54l15pdk. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Review rework GRTC support for nRF54Lx SoCs. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use nrfx interface for register access with sideeffects to facilitate bsim use. 4 part commits: 1. Add nrf_grtc interface. 2. Remove CMSIS interface use. 3. Add nrf_grtc interface, once missing bsim port available. 4. Remove CMSIS interface use, replaced by bsim port. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use nrfx interface for register access with sideeffects to facilitate bsim use. Remove CMSIS interface use, was replaced with nrf_grtc interface in previous commit. 4 part commits: 1. Add nrf_grtc interface. 2. Remove CMSIS interface use. 3. Add nrf_grtc interface, once missing bsim port available. 4. Remove CMSIS interface use, replaced by bsim port. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use nrfx interface for register access with sideeffects to facilitate bsim use. 4 part commits: 1. Add nrf_grtc interface. 2. Remove CMSIS interface use. 3. Add nrf_grtc interface, once missing bsim port available. 4. Remove CMSIS interface use, replaced by bsim port. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use nrfx interface for register access with sideeffects to facilitate bsim use. Remove CMSIS interface use, was replaced with nrf_grtc, nrf_dppic and nrf_ppib interface in previous commit. 4 part commits: 1. Add nrf_grtc interface. 2. Remove CMSIS interface use. 3. Add nrf_grtc interface, once missing bsim port available. 4. Remove CMSIS interface use, replaced by bsim port. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use SW_SWITCH_SINGLE_TIMER for nRF54Lx so that fast ramp can be used, and hence support assymmetric PHYs in ACL connections. nRF54Lx radio domain does not have second timer instance to efficiently implement software switch unless a second time is used from a different power domain needed use of PPIB for the double buffered switch timer compares. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Use NRF_GRTC in nrf54l15bsim board builds. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

This reverts commit 3d72998. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Enable central HR peripheral HR samples for nrf54l15bsim. RealEncryption is BabbleSIM is not enabled as nRF54L port in Zephyr Controller is not yet supported and the test is also not using security (SMP pairing). Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

aescolar

Approving understanding that this PR will be followed up shortly with another one fixing the entropy source so we avoid randomness in CI.

aescolar · 2024-08-29T13:25:51Z

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

@@ -47,17 +116,89 @@ uint32_t cntr_stop(void)
 		return 1;
 	}

+#if defined(CONFIG_BT_CTLR_NRF_GRTC)
+	/* TODO: if we own and stop GRTC, implement stop here */


Note: I don't think anybody is meant to ever stop the GRTC

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

Thalley

It's passing existing tests, but are we testing all the new Kconfigs?

cvinayak force-pushed the github_bt_ctlr_nrf_grtc branch 2 times, most recently from 57a6546 to 4cc8e1a Compare June 13, 2024 04:07

cvinayak marked this pull request as ready for review June 13, 2024 05:36

zephyrbot added area: Bluetooth area: Bluetooth Host area: Bluetooth Controller area: Samples Samples labels Jun 13, 2024

zephyrbot requested review from alwa-nordic, asbjornsabo, carlescufi, erbr-ot, hermabe, jhedberg, jori-nordic, kruithofa, mtpr-ot, sjanc, Thalley, theob-pro, thoh-ot, Vudentz and wopu-ot June 13, 2024 05:37

zephyrbot assigned cvinayak Jun 13, 2024

cvinayak requested review from aescolar and removed request for theob-pro June 13, 2024 05:38

Thalley requested changes Jun 14, 2024

View reviewed changes

cvinayak force-pushed the github_bt_ctlr_nrf_grtc branch 3 times, most recently from aa2503b to f51ae20 Compare June 14, 2024 11:16

cvinayak changed the title ~~Bluetooth: Controller: nRF54Lx: Add GRTC support~~ Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling Jun 14, 2024

cvinayak force-pushed the github_bt_ctlr_nrf_grtc branch 3 times, most recently from 58da23c to 919b875 Compare August 28, 2024 09:16

cvinayak added 7 commits August 28, 2024 14:53

Bluetooth: Controller: Fix BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE depends

b71ab0b

Fix BT_CTLR_EARLY_ABORT_PREVIOUS_PREPARE Kconfig depends on. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: Fix ext conn create when using single timer

7bd9da3

Fix Extended Connection Creation when using single timer feature in the Controller. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: Refactor sw_switch hal interface use

b5ea478

Refactor sw_switch hal interface use and document why some PPI/DPPI channel group related subscriptions are not disabled explicitly. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: Remove redundant HAL_TICKER_CNTR_CLK_FREQ_HZ

7ecf622

Remove redundant HAL_TICKER_CNTR_CLK_FREQ_HZ define. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: Add HAL_TICKER_TICKS_TO_US_64BIT define

1e6244d

Add HAL_TICKER_TICKS_TO_US_64BIT define, to return microsecond value in 64-bits. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: Ticker support for free running counter

1ecbeb7

Add ticker implementation support for free running counter use. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

cvinayak force-pushed the github_bt_ctlr_nrf_grtc branch from 919b875 to fa2f85f Compare August 28, 2024 13:07

cvinayak added 13 commits August 28, 2024 15:19

Bluetooth: Controller: nRF54Lx: Port for data whitening register use

837de42

nRF54Lx have updated Data Whitening register settings, add implementation to correctly set them up. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: nRF54Lx: Support Radio fast ramp up

d781e6d

Add support for Radio fast ramp up for nRF54Lx SoCs. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

samples: Bluetooth: hci_uart: Add preliminary DF support for nrf54l15pdk

2016dd1

Add preliminary direction finding support building hci_uart sample for nrf54l15pdk. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: nRF54Lx: Review rework GRTC support

385ce38

Review rework GRTC support for nRF54Lx SoCs. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Bluetooth: Controller: nrf54L15bsim: Use NRF_GRTC

3050e06

Use NRF_GRTC in nrf54l15bsim board builds. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Revert "tests/bsim: Provisionally disable the nrf54l15 BT tests"

e9f6c8e

This reverts commit 3d72998. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

cvinayak force-pushed the github_bt_ctlr_nrf_grtc branch from fa2f85f to d1293ae Compare August 28, 2024 13:20

aescolar approved these changes Aug 29, 2024

View reviewed changes

cvinayak requested review from niag-Oticon and Thalley August 30, 2024 08:52

Thalley approved these changes Aug 30, 2024

View reviewed changes

nashif merged commit be7445d into zephyrproject-rtos:main Aug 30, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling #74183

Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling #74183

cvinayak commented Jun 12, 2024 •

edited

Loading

Thalley Jun 14, 2024

cvinayak Jun 14, 2024

niag-Oticon Jun 17, 2024

Thalley Jun 17, 2024

cvinayak Aug 30, 2024

Thalley Jun 14, 2024

cvinayak Jun 14, 2024

niag-Oticon Jun 17, 2024

aescolar left a comment

aescolar Aug 29, 2024

Thalley left a comment

Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling #74183

Bluetooth: Controller: nRF54Lx: Use NRF_GRTC for radio scheduling #74183

Conversation

cvinayak commented Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aescolar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thalley left a comment

Choose a reason for hiding this comment

cvinayak commented Jun 12, 2024 •

edited

Loading