mpsl: Add integration layer for MPSL external clock driver #19498

ppryga-nordic · 2024-12-13T09:27:35Z

The MPLS is enhanced with possibility to use it external clock driver.
That feature requires implementation of integration layer between nrf-clock-control for nRF52, nRF53 SoC series and nrf2-clock-control for nRF54H SoC series (the nRF54L SoC series is not available yet).

The integration with nrf-clock-control is experimental.
The integration with nrf2-clock-control is supported and enabled by default for nRF54H SoC series.

nrf2-clock-control required some changes to MSPL and SDC HCI driver initialization. The nrf2-clock-control depends on nRFS
so MPLS and SDC HCI driver that depend on MPLS need to be initialized in POST_KERNEL system initialization level and with priority lower than nRFS initialization priority.

Besides that there were a change in radio-notification-cb host extension that was dependent on RTC0 and RTC1 start order. With changed initialization level of MPSL the RTC1 (system timer) is started before the RTC0 used by MPSL.

NordicBuilder · 2024-12-13T09:29:05Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
dragoon	11f5e5d895cf2408403e58341e8c07f0baa5b4ef	4343578d1f304df7fdb80b97b7a33da28b800594	N/A

⛔ DNM label due to: 1 unreachable repo

Note: This message is automatically posted and updated by the Manifest GitHub Action.

NordicBuilder · 2024-12-13T09:34:00Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 15

Inputs:

Sources:

sdk-nrf: PR head: 425076c49bdc34e5039db6628f713218a5e6be03

more details

sdk-nrf:

PR head: 425076c49bdc34e5039db6628f713218a5e6be03
merge base: 68623068aed6482998f1eff6df828b051c7bb48f
target head (main): 68623068aed6482998f1eff6df828b051c7bb48f
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (14)

drivers
│  ├── mpsl
│  │  ├── clock_control
│  │  │  │ Kconfig
lib
│  ├── boot_banner
│  │  │ banner.c
subsys
│  ├── bluetooth
│  │  ├── controller
│  │  │  ├── Kconfig
│  │  │  │ hci_driver.c
│  │  ├── host_extensions
│  │  │  │ radio_notification_conn_cb.c
│  ├── mpsl
│  │  ├── CMakeLists.txt
│  │  ├── Kconfig
│  │  ├── clock_ctrl
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── mpsl_clock_ctrl.c
│  │  │  │ mpsl_clock_ctrl.h
│  │  ├── init
│  │  │  ├── Kconfig
│  │  │  │ mpsl_init.c
west.yml

Outputs:

Toolchain

Version: b77d8c1312
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:b77d8c1312_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
❌ Build twister
- sdk-nrf test count: 1106
❌ Integration tests
- ❌ desktop52_verification
- ❌ test_ble_nrf_config
- ❌ test-fw-nrfconnect-ble_samples
- ❌ test-fw-nrfconnect-chip
- ❌ test-fw-nrfconnect-rs
- ❌ test-fw-nrfconnect-fem
- ❌ test-fw-nrfconnect-thread
- ❌ test-fw-nrfconnect-zigbee
- ❌ test-sdk-find-my
- ❌ test-sdk-sidewalk

Disabled integration tests

- doc-internal
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-boot
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_mosh
- test-fw-nrfconnect-nrf-iot_nrf_provisioning
- test-fw-nrfconnect-nrf-iot_positioning
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_serial_lte_modem
- test-fw-nrfconnect-nrf-iot_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-tfm
- test-low-level
- test-sdk-audio
- test-sdk-dfu
- test-sdk-mcuboot
- test-sdk-pmic-samples
- test-sdk-wifi
- test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-12-16T10:27:52Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

rugeGerritsen · 2024-12-16T12:29:18Z

subsys/bluetooth/host_extensions/radio_notification_conn_cb.c

+/* In case of use of nrf clock control MPSL is stared on POST_KERNEL level, hence NRF_RTC0 is
+ * started after system clock which is NRF_RTC1. In case MPLS is configured as clock control driver
+ * then NRF_RTC0 is started on PRE_KERNEL_1 init level.
+ */
+#if defined(CONFIG_MPSL_USE_ZEPHYR_CLOCK_CONTROL)
+#define REF_RTC NRF_RTC1
+#define CMP_RTC NRF_RTC0
+#else
+#define REF_RTC NRF_RTC0
+#define CMP_RTC NRF_RTC1
+#endif /* CONFIG_MPSL_USE_ZEPHYR_CLOCK_CONTROL */


This code change is unnecessary if set_sys_clock_start_to_bt_clk_start_us() is updated to set sys_clock_start_to_bt_clk_start_us as a signed value. I suggest to change that instead to avoid extra complexity in this module.

That change can be separated out to a separate PR and be merged independently

drivers/mpsl/clock_control/Kconfig

rugeGerritsen · 2024-12-16T12:32:25Z

include/mpsl/mpsl_clock_ctrl.h

+ * @retval -NRF_EPERM Clock control is already initialized.
+ * @retval -NRF_EINVAL Invalid parameters supplied.
+ */
+int32_t mpsl_clock_ctrl_init(void);


The APIs mpsl_clock_ctrl_init() and mpsl_clock_ctrl_uninit() are not supposed to be called from application layer, only from MPSL-internal components. We should therefore move this header file from include/mpsl to subsys/mpsl

Then mpsl_pm.h should also be moved to mpsl/pm, it is also used internally by the mpsl subsystem.

File moved.

rugeGerritsen · 2024-12-16T12:33:09Z

subsys/bluetooth/controller/Kconfig

@@ -611,5 +611,13 @@ config BT_CTLR_SDC_LE_POWER_CLASS_1
 	  See Bluetooth Core Specification, Vol 6, Part A, Section 3
 	  Transmitter Characteristics for more information.

+config BT_LL_SOFTDEVICE_HCI_SYS_INIT_PRIO_LEVEL


Discussed offline:

Suggested change

config BT_LL_SOFTDEVICE_HCI_SYS_INIT_PRIO_LEVEL

config BT_LL_SOFTDEVICE_INIT_PRIO_LEVEL

rugeGerritsen · 2024-12-16T12:33:52Z

subsys/bluetooth/controller/Kconfig

+	  This option configures LL_SOFTDEVICE system init priority level. The priority must be lower
+	  than the CONFIG_MPLS_SYS_INIT_PRIO_LEVEL due to dependency on the MPSL library.


Suggested change

This option configures LL_SOFTDEVICE system init priority level. The priority must be lower

than the CONFIG_MPLS_SYS_INIT_PRIO_LEVEL due to dependency on the MPSL library.

This option configures the LL_SOFTDEVICE initialization priority level. The priority must be lower

than the CONFIG_MPSL_SYS_INIT_PRIO_LEVEL due to dependency on the MPSL library.

rugeGerritsen · 2024-12-16T13:12:08Z

subsys/mpsl/clock_ctrl/mpsl_clock_ctrl.c

+
+static int32_t m_lfclk_release(void)
+{
+	struct onoff_manager *mgr = z_nrf_clock_control_get_onoff(CLOCK_CONTROL_NRF_SUBSYS_LF);


Can we use clock_control_off() here?

No it is not supported for nRF SoCs. There are no device tree entries for clocks in 52/53 board DTS.

rugeGerritsen · 2024-12-16T13:15:06Z

subsys/mpsl/init/Kconfig

+config MPSL_SYS_INIT_LEVEL_POST_KERNEL
+	bool
+	default y if MPSL_USE_ZEPHYR_CLOCK_CONTROL
+	help
+	  Change MPSL system init level to POST_KERNEL. Default init level is PRE_KERNEL_1.
+	  The POST_KERNEL init level must be used in case of use of external clock driver.
+	  That gives integration layer possibilty to use kernel object for clocks handling.
+	  The option shall not be set in case of use of MPLS internal clock driver.
+	  Then the MPSL system init level must be set to PRE_KERNEL_1.


Do we need MPSL_SYS_INIT_LEVEL_POST_KERNEL` or should we always initialize mpsl post-kernel when MPSL_USE_ZEPHYR_CLOCK_CONTROL is set? Otherwise we need to add tests for both configurations

Good point, use of MPSL_USE_ZEPHYR_CLOCK_CONTROL should be enough.

Removed and used MPSL_USE_EXTERNAL_CLOCK_CONTROL instead

rugeGerritsen · 2024-12-16T13:18:16Z

subsys/mpsl/init/Kconfig

+	  The option shall not be set in case of use of MPLS internal clock driver.
+	  Then the MPSL system init level must be set to PRE_KERNEL_1.
+
+config MPSL_SYS_INIT_PRIO_LEVEL


I would consider naming this MPSL_INIT_PRIORITY. Other drivers tend to use the convention <drvier>_INIT_PRIO_LEVEL or <driver>_INIT_PRIORITY

rugeGerritsen · 2024-12-16T13:20:00Z

subsys/mpsl/init/mpsl_init.c

@@ -390,7 +395,14 @@ static int32_t mpsl_lib_init_internal(void)
 	 * 500ppm or better regardless of the value passed in clock_cfg.
 	 */
 	memset(&clock_cfg, 0, sizeof(clock_cfg));


I would consider passing in NULL when CONFIG_MPSL_USE_ZEPHYR_CLOCK_CONTROL is used. That makes it more clear the configuration is not used and also saves some code size

rugeGerritsen · 2024-12-16T13:20:55Z

subsys/mpsl/init/mpsl_init.c

+#endif /* CONFIG_MPLS_SYS_INIT_LEVEL_POST_KERNEL */
+
+#if defined(CONFIG_MPSL_USE_ZEPHYR_CLOCK_CONTROL) && defined(CONFIG_SOC_SERIES_NRF54HX)
+BUILD_ASSERT(CONFIG_MPLS_SYS_INIT_PRIO_LEVEL > CONFIG_NRFS_BACKEND_IPC_SERVICE_INIT_PRIO,


Some typos here: MPLS -> MPSL

Please also fix the commit message and title of this PR :)

rugeGerritsen · 2024-12-17T07:25:14Z

subsys/bluetooth/host_extensions/radio_notification_conn_cb.c

+#if !defined(CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL)
 	__ASSERT_NO_MSG(ticks_difference >= 0);
+#endif /* CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL */


This assertion is no longer needed

bjarki-andreasen

Looks good, definately like the introduction of MPSL_USE_EXTERNAL_CLOCK_CONTROL

In the past MPSL used its own implementation of clock control. The approach changes due to lack of acces to clocks from Radio core in nRF54h20 SoC. The new module is an integration layer between new MPSL API that allows for registration of external clock driver and nRF clock control driver. The implementation in this commit provides integration with MPSL for nrf clock control for nRF52 and nRF53 SoC series. Note: The support for nRF52 and nRF53 SoC series is experimental and is not enabled by default. Signed-off-by: Piotr Pryga <[email protected]>

Add integration layer for MPSL external clock driver and nrf2 clock control for nRF5420. This is mandatory for the nRF54H20. The nrf2 clock control requires change to MPSL initialization level and its priority. The nrf2 clock control depends on nRFS that is initialized at POST_KENREL init level. Its init priority is CONFIG_NRFS_BACKEND_IPC_SERVICE_INIT_PRIO that is lower than former MPSL init level. To make sure the mpsl lfclk request and response is handled corrently we must make the MPSL is initialized after it. The change in MPSL init level and priority affects BT_LL_SOFTDEVIDE- _HCI_SYS_INIT_PRIO_LEVEL. The HCI driver for SDC depends on MPSL so it must be initialized after the MPSL. Signed-off-by: Piotr Pryga <[email protected]>

Radio notification callback host extension checks the offset between system clock and second RTC instance. The order of RTCx startup depends on MPSL being used as clock control driver. If the MPSL is the driver it is started at PRE_KERNEL_1 init level that makes RTC0 instance to start first. In case of use of nrf clock control the MPSL is initialized at POST_KERNEL init level that makes the RTC1 to be started first. To make the timer correction to be right, change unsigned types of variables used to store the difference to be integer. Then we don't depend on the RTC initialization order. Signed-off-by: Piotr Pryga <[email protected]>

The nrf clock control driver doesn't enable HFXO when LFSYNTH is selected as a source of LFCLK. That causes the accuracy of LFCLK to be not within the expected by BT Core Specification range up to 500 ppm. To fix the problem the mpsl clock control integration layer has to request the hfxo in case the lfclk is requested with LFSYNTH as a source. Use of z_nrf_clock_bt_ctlr_hf_release makes the call to be faster. That unfortunately requires reference counting do avoid release of HFXO later in runtime by MPSL. For that reason the m_hfclk_request and m_hfclk_release are used. Signed-off-by: Piotr Pryga <[email protected]>

There were no API in nrf2 clock control that allows to requrest the HFXO from zero latency interrupts. That was a blocker for MPSL. Since the API was added, change the implementation of clock control integration layer to use it. Mind that the new API is similar to nRF52/nRF53 APIs: z_nrf_clock_bt_ctlr_hf_request and z_nrf_clock_bt_ctlr_hf_release. It does not provide feedback if the HFXO ramp-up is done. Signed-off-by: Piotr Pryga <[email protected]>

DNM Signed-off-by: Piotr Pryga <[email protected]>

knutel-nordic · 2024-12-30T08:49:49Z

subsys/mpsl/clock_ctrl/mpsl_clock_ctrl.c

+
+static bool m_hfclk_is_running(void)
+{
+	/* As of now assume the HFCLK is runnig after the request was put */


Is this assumption valid?

knutel-nordic · 2024-12-30T09:21:40Z

subsys/bluetooth/host_extensions/radio_notification_conn_cb.c

+/* In case of use of nrf clock control MPSL is stared on POST_KERNEL level, hence NRF_RTC0 is
+ * started after system clock which is NRF_RTC1. In case MPLS is configured as clock control driver
+ * then NRF_RTC0 is started on PRE_KERNEL_1 init level.


Suggested change

/* In case of use of nrf clock control MPSL is stared on POST_KERNEL level, hence NRF_RTC0 is

* started after system clock which is NRF_RTC1. In case MPLS is configured as clock control driver

* then NRF_RTC0 is started on PRE_KERNEL_1 init level.

/* In case of use of nrf clock control MPSL is started on POST_KERNEL level, hence NRF_RTC0 is

* started after system clock which is NRF_RTC1. In case MPSL is configured as clock control driver

* then NRF_RTC0 is started on PRE_KERNEL_1 init level.

ppryga-nordic requested review from a team as code owners December 13, 2024 09:27

github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Dec 13, 2024

NordicBuilder added manifest-zephyr manifest-dragoon DNM labels Dec 13, 2024

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch 3 times, most recently from 9ae3f8c to a5d7313 Compare December 16, 2024 10:09

ppryga-nordic changed the title ~~[WIP] Mpsl nrf clock control integration~~ mpsl: Add integration layer for MPLS external clock driver Dec 16, 2024

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch from a5d7313 to f712fd7 Compare December 16, 2024 10:50

ppryga-nordic added this to the 2.9.0-nRF54H20 milestone Dec 16, 2024

rugeGerritsen requested changes Dec 16, 2024

View reviewed changes

rugeGerritsen self-requested a review December 16, 2024 14:04

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch from f712fd7 to 06d2cc1 Compare December 16, 2024 17:34

ppryga-nordic changed the title ~~mpsl: Add integration layer for MPLS external clock driver~~ mpsl: Add integration layer for MPSL external clock driver Dec 16, 2024

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch 4 times, most recently from 10bb5d8 to 379cb4d Compare December 16, 2024 19:19

rugeGerritsen requested changes Dec 17, 2024

View reviewed changes

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch 2 times, most recently from e0be461 to 9d9adf0 Compare December 17, 2024 11:32

bjarki-andreasen approved these changes Dec 17, 2024

View reviewed changes

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch from 9d9adf0 to 3e8750d Compare December 19, 2024 13:04

ppryga-nordic added 6 commits December 27, 2024 15:45

nrf: lib: boo_banner: changed boot banner to print reset reason

425076c

DNM Signed-off-by: Piotr Pryga <[email protected]>

ppryga-nordic force-pushed the mpsl-nrf-clock-control-integration branch from 3e8750d to 425076c Compare December 27, 2024 15:16

ppryga-nordic requested a review from nordicjm as a code owner December 27, 2024 15:16

knutel-nordic reviewed Dec 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mpsl: Add integration layer for MPSL external clock driver #19498

mpsl: Add integration layer for MPSL external clock driver #19498

ppryga-nordic commented Dec 13, 2024 •

edited

Loading

NordicBuilder commented Dec 13, 2024 •

edited

Loading

NordicBuilder commented Dec 13, 2024 •

edited

Loading

NordicBuilder commented Dec 16, 2024

rugeGerritsen Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024 •

edited

Loading

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 16, 2024

rugeGerritsen Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 16, 2024

ppryga-nordic Dec 16, 2024

rugeGerritsen Dec 17, 2024

ppryga-nordic Dec 17, 2024

bjarki-andreasen left a comment

knutel-nordic Dec 30, 2024

knutel-nordic Dec 30, 2024

	config BT_LL_SOFTDEVICE_HCI_SYS_INIT_PRIO_LEVEL
	config BT_LL_SOFTDEVICE_INIT_PRIO_LEVEL

		This option configures LL_SOFTDEVICE system init priority level. The priority must be lower
		than the CONFIG_MPLS_SYS_INIT_PRIO_LEVEL due to dependency on the MPSL library.

mpsl: Add integration layer for MPSL external clock driver #19498

Are you sure you want to change the base?

mpsl: Add integration layer for MPSL external clock driver #19498

Conversation

ppryga-nordic commented Dec 13, 2024 • edited Loading

NordicBuilder commented Dec 13, 2024 • edited Loading

NordicBuilder commented Dec 13, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Dec 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppryga-nordic Dec 16, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjarki-andreasen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppryga-nordic commented Dec 13, 2024 •

edited

Loading

NordicBuilder commented Dec 13, 2024 •

edited

Loading

NordicBuilder commented Dec 13, 2024 •

edited

Loading

ppryga-nordic Dec 16, 2024 •

edited

Loading