mpsl: pm: Change init API and adds uninit API MPSL PM

Refactor initialization API and add uninitialization.

Change the initialization API to do not allow call it
when it is already initialized or if uninitialization
is started but not completed. The uninitialization
API will wait for scheduled work queue uninit item
to complete. If that is late the API will timeout.

Uninitialize API will fail when the MPSL PM is not
in initialized state.

Unit tests are changed to cover changes in the API.

Signed-off-by: Knut Eldhuset <[email protected]>
Signed-off-by: Piotr Pryga <[email protected]>
Co-authored-by: Piotr Pryga <[email protected]>

Author: 2 people

include/mpsl/mpsl_work.h

@@ -7,9 +7,9 @@
 /**
  * @file mpsl_work.h
  *
- * @defgroup mpsl_work Multiprotocol Service Layer workqueue.
+ * @defgroup mpsl_work Multiprotocol Service Layer work queue.
  *
- * @brief Internal MPSL workqueue.
+ * @brief Internal MPSL work queue.
  * @{
  */
 
@@ -24,23 +24,22 @@ extern "C" {
 
 extern struct k_work_q mpsl_work_q;
 
-/** @brief Submit a work item to the MPSL workqueue.
+/** @brief Submit a work item to the MPSL work queue.
  *
  * This routine submits work item @a work to be processed by the MPSL
- * workqueue. If the work item is already pending in the MPSL workqueue or
- * any other workqueue as a result of an earlier submission, this routine
+ * work queue. If the work item is already pending in the MPSL work queue or
+ * any other work queue as a result of an earlier submission, this routine
  * has no effect on the work item. If the work item has already been
  * processed, or is currently being processed, its work is considered
  * complete and the work item can be resubmitted.
  *
- * @note
- * Work items submitted to the MPSL workqueue should avoid using handlers
- * that block or yield since this may prevent the MPSL workqueue from
- * processing other work items in a timely manner.
+ * @note Work items submitted to the MPSL work queue should avoid using handlers
+ *       that block or yield since this may prevent the MPSL work queue from
+ *       processing other work items in a timely manner.
  *
  * @note Can be called by ISRs.
  *
- * @param work Address of work item.
+ * @param work address of work item.
  */
 static inline void mpsl_work_submit(struct k_work *work)
 {
@@ -49,23 +48,22 @@ static inline void mpsl_work_submit(struct k_work *work)
 	}
 }
 
-/** @brief Submit an idle work item to the MPSL workqueue after a delay.
+/** @brief Submit an idle work item to the MPSL work queue after a delay.
  *
  * @note Can be called by ISRs.
  *
- * @note
- * Work items submitted to the MPSL workqueue should avoid using handlers
- * that block or yield since this may prevent the MPSL workqueue from
- * processing other work items in a timely manner.
+ * @note Work items submitted to the MPSL work queue should avoid using handlers
+ *       that block or yield since this may prevent the MPSL work queue from
+ *       processing other work items in a timely manner.
  *
  * @note This is a no-op if the work item is already scheduled or submitted,
- * even if @p delay is @c K_NO_WAIT.
+ *       even if @p delay is @c K_NO_WAIT. See @ref mpsl_work_reschedule().
  *
- * @param dwork Address of delayable work item.
+ * @param dwork address of delayable work item.
  *
- * @param delay the time to wait before submitting the work item.  If @c
- * K_NO_WAIT and the work is not pending this is equivalent to
- * mpsl_work_submit().
+ * @param delay the time to wait before submitting the work item.
+ *        If @c K_NO_WAIT and the work is not pending this is equivalent to
+ *        mpsl_work_submit().
  */
 static inline void mpsl_work_schedule(struct k_work_delayable *dwork, k_timeout_t delay)
 {
@@ -74,6 +72,30 @@ static inline void mpsl_work_schedule(struct k_work_delayable *dwork, k_timeout_
 	}
 }
 
+/** @brief Reschedule a work item to the MPSL work queue after a delay.
+ *
+ * @note Can be called by ISRs.
+ *
+ * @note Work items submitted to the MPSL work queue should avoid using handlers
+ *       that block or yield since this may prevent the MPSL work queue from
+ *       processing other work items in a timely manner.
+ *
+ * @note If the work item has not been scheduled before, this behavior is
+ *       the same as @ref mpsl_work_schedule().
+ *
+ * @param dwork address of delayable work item.
+ *
+ * @param delay the time to wait before submitting the work item.
+ *        If @c K_NO_WAIT and the work is not pending this is equivalent to
+ *        mpsl_work_submit().
+ */
+static inline void mpsl_work_reschedule(struct k_work_delayable *dwork, k_timeout_t delay)
+{
+	if (k_work_reschedule_for_queue(&mpsl_work_q, dwork, delay) < 0) {
+		__ASSERT(false, "k_work_reschedule_for_queue() failed.");
+	}
+}
+
 #ifdef __cplusplus
 }
 #endif

subsys/mpsl/init/mpsl_init.c

@@ -26,7 +26,7 @@
 #endif
 
 #if IS_ENABLED(CONFIG_MPSL_USE_ZEPHYR_PM)
-#include <mpsl/mpsl_pm_utils.h>
+#include "../pm/mpsl_pm_utils.h"
 #endif
 
 #if IS_ENABLED(CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL)
@@ -398,6 +398,13 @@ static int32_t mpsl_lib_init_internal(void)
 	}
 #endif /* CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL */
 
+#if defined(CONFIG_MPSL_USE_ZEPHYR_PM)
+	err = mpsl_pm_utils_init();
+	if (err) {
+		return err;
+	}
+#endif /* CONFIG_MPSL_USE_ZEPHYR_PM */
+
 #if defined(CONFIG_SOC_SERIES_NRF54HX)
 	/* Secure domain no longer enables DPPI channels for local domains,
 	 * MPSL now has to enable the ones it uses.
@@ -442,10 +449,6 @@ static int mpsl_lib_init_sys(void)
 		return err;
 	}
 
-#if IS_ENABLED(CONFIG_MPSL_USE_ZEPHYR_PM)
-	mpsl_pm_utils_init();
-#endif
-
 #if IS_ENABLED(CONFIG_MPSL_DYNAMIC_INTERRUPTS)
 	/* Ensure IRQs are disabled before attaching. */
 	mpsl_lib_irq_disable();
@@ -518,6 +521,10 @@ int32_t mpsl_lib_init(void)
 int32_t mpsl_lib_uninit(void)
 {
 #if IS_ENABLED(CONFIG_MPSL_DYNAMIC_INTERRUPTS)
+#if defined(CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL) || defined(CONFIG_MPSL_USE_ZEPHYR_PM)
+	int err;
+#endif /* CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL || CONFIG_MPSL_USE_ZEPHYR_PM */
+
 #if defined(CONFIG_MPSL_CALIBRATION_PERIOD)
 	atomic_set(&do_calibration, 0);
 #endif /* CONFIG_MPSL_CALIBRATION_PERIOD */
@@ -527,14 +534,19 @@ int32_t mpsl_lib_uninit(void)
 	mpsl_uninit();
 
 #if defined(CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL)
-	int err;
-
 	err = mpsl_clock_ctrl_uninit();
 	if (err) {
 		return err;
 	}
 #endif /* CONFIG_MPSL_USE_EXTERNAL_CLOCK_CONTROL */
 
+#if defined(CONFIG_MPSL_USE_ZEPHYR_PM)
+	err = mpsl_pm_utils_uninit();
+	if (err) {
+		return err;
+	}
+#endif /* CONFIG_MPSL_USE_ZEPHYR_PM */
+
 	return 0;
 #else /* !IS_ENABLED(CONFIG_MPSL_DYNAMIC_INTERRUPTS) */
 	return -NRF_EPERM;

subsys/mpsl/pm/Kconfig

@@ -15,6 +15,20 @@ config MPSL_USE_ZEPHYR_PM
 
 if MPSL_USE_ZEPHYR_PM
 
+config MPSL_PM_UNINIT_WORK_WAIT_TIME_MS
+	int "Timeout value the MPSL PM uninit procedure will wait for work queue to complete, in milliseconds"
+	default 100
+	help
+	  The option specifies a timeout value in milliseconds, the MPLS PM uninit procedure
+	  will wait for MPSL work queue to complete handling of scheduled uninit work item.
+
+config MPSL_PM_NO_RADIO_EVENT_PERIOD_LATENCY_US
+	int "Latency value the MPSL PM allows in periods outside of radio events, in microseconds"
+	default 499999
+	help
+	  The option specifies the latency in microseconds that is allowed by MPSL PM
+	  outside of radio events.
+
 config MPSL_PM_USE_MRAM_LATENCY_SERVICE
 	bool "Use Zephyr's MRAM latency service"
 	select MRAM_LATENCY

subsys/mpsl/pm/mpsl_pm_utils.c

@@ -14,29 +14,42 @@
 #include <mram_latency.h>
 #endif /* CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE */
 
-#include <mpsl/mpsl_pm_utils.h>
+#include <mpsl/mpsl_work.h>
+#include "mpsl_pm_utils.h"
 
 LOG_MODULE_REGISTER(mpsl_pm_utils, CONFIG_MPSL_LOG_LEVEL);
 
-/* These constants must be updated once the Zephyr PM Policy API is updated
- * to handle low latency events. Ideally, the Policy API should be changed to use
- * absolute time instead of relative time. This would remove the need for safety
- * margins and allow optimal power savings.
- */
-#define TIME_TO_REGISTER_EVENT_IN_ZEPHYR_US 1000
-#define PM_MAX_LATENCY_HCI_COMMANDS_US 499999
+#define NO_RADIO_EVENT_PERIOD_LATENCY_US CONFIG_MPSL_PM_NO_RADIO_EVENT_PERIOD_LATENCY_US
+#define UNINIT_WORK_WAIT_TIMEOUT_MS	 K_MSEC(CONFIG_MPSL_PM_UNINIT_WORK_WAIT_TIME_MS)
 
-static uint8_t                          m_pm_prev_flag_value;
-static bool                             m_pm_event_is_registered;
-static uint32_t                         m_prev_lat_value_us;
+/* All MPSL PM event and latency actions are triggered inside the library.
+ * The uninitialization may be started outside of the library thgough public API.
+ * To avoid interference with other MPSL work items we need dedicated work
+ * item for uninitialization purpose.
+ */
+static void m_pm_uninit_work_handler(struct k_work *work);
+static K_WORK_DELAYABLE_DEFINE(m_pm_uninit_work, m_pm_uninit_work_handler);
+
+enum MPLS_PM_STATE {
+	MPSL_PM_UNINITIALIZED,
+	MPSL_PM_UNINITIALIZING,
+	MPSL_PM_INITIALIZED
+};
+
+static uint8_t m_pm_prev_flag_value;
+static bool m_pm_event_is_registered;
+static uint32_t m_prev_lat_value_us;
 static struct pm_policy_latency_request m_latency_req;
-static struct pm_policy_event           m_evt;
+static struct pm_policy_event m_evt;
+
+static atomic_t m_pm_state = (atomic_val_t)MPSL_PM_UNINITIALIZED;
+struct k_sem m_uninit_wait_sem;
 
 #if defined(CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE)
 #define LOW_LATENCY_ATOMIC_BITS_NUM 2
-#define LOW_LATENCY_PM_BIT 0
-#define LOW_LATENCY_MRAM_BIT 0
-#define LOW_LATENCY_BITS_MASK 0x3
+#define LOW_LATENCY_PM_BIT	    0
+#define LOW_LATENCY_MRAM_BIT	    0
+#define LOW_LATENCY_BITS_MASK	    0x3
 
 static ATOMIC_DEFINE(m_low_latency_req_state, LOW_LATENCY_ATOMIC_BITS_NUM);
 /* Variable must be global to use it in on-off service cancel or release API */
@@ -56,7 +69,7 @@ static void m_update_latency_request(uint32_t lat_value_us)
 
 static void m_register_event(void)
 {
-	mpsl_pm_params_t params	= {0};
+	mpsl_pm_params_t params = {0};
 	bool pm_param_valid = mpsl_pm_params_get(&params);
 
 	if (m_pm_prev_flag_value == params.cnt_flag) {
@@ -130,7 +143,7 @@ static void m_register_latency(void)
 #if defined(CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE)
 			m_mram_low_latency_release();
 #endif /* CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE */
-			m_update_latency_request(PM_MAX_LATENCY_HCI_COMMANDS_US);
+			m_update_latency_request(NO_RADIO_EVENT_PERIOD_LATENCY_US);
 #if defined(CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE)
 			atomic_clear_bit(m_low_latency_req_state, LOW_LATENCY_PM_BIT);
 #endif /* CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE*/
@@ -201,21 +214,88 @@ static void m_mram_low_latency_release(void)
 }
 #endif /* CONFIG_MPSL_PM_USE_MRAM_LATENCY_SERVICE */
 
+static void m_pm_uninit_work_handler(struct k_work *work)
+{
+	ARG_UNUSED(work);
+
+	mpsl_pm_utils_work_handler();
+}
+
 void mpsl_pm_utils_work_handler(void)
 {
-	m_register_event();
-	m_register_latency();
+	enum MPLS_PM_STATE pm_state = (enum MPLS_PM_STATE)atomic_get(&m_pm_state);
+
+	if (pm_state == MPSL_PM_INITIALIZED) {
+		m_register_event();
+		m_register_latency();
+	} else if (pm_state == MPSL_PM_UNINITIALIZING) {
+
+		/* The uninitialization is handled by all MPSL work items as well as by dedicated
+		 * uninit work item. In case regular MPSL work item cleans MPSL PM the uninit
+		 * work queue will not do anything.
+		 */
+		pm_policy_latency_request_remove(&m_latency_req);
+		pm_policy_event_unregister(&m_evt);
+
+		/* The MPSL PM status is updated here to make the code unit testable.
+		 * There is no work queue when running UTs, so the mpsl_pm_utils_work_handler()
+		 * is manualy executed after mpsl_pm_utils_uninit() returns.
+		 */
+		atomic_set(&m_pm_state, (atomic_val_t)MPSL_PM_UNINITIALIZED);
+
+		k_sem_give(&m_uninit_wait_sem);
+	}
 }
 
-void mpsl_pm_utils_init(void)
+int32_t mpsl_pm_utils_init(void)
 {
 	mpsl_pm_params_t params = {0};
 
-	pm_policy_latency_request_add(&m_latency_req, PM_MAX_LATENCY_HCI_COMMANDS_US);
-	m_prev_lat_value_us = PM_MAX_LATENCY_HCI_COMMANDS_US;
+	if (atomic_get(&m_pm_state) != (atomic_val_t)MPSL_PM_UNINITIALIZED) {
+		return -NRF_EPERM;
+	}
+
+	pm_policy_latency_request_add(&m_latency_req, NO_RADIO_EVENT_PERIOD_LATENCY_US);
+	m_prev_lat_value_us = NO_RADIO_EVENT_PERIOD_LATENCY_US;
 
 	mpsl_pm_init();
-	mpsl_pm_params_get(&params);
+	/* On init there should be no update from high-prio, returned value can be ignored */
+	(void)mpsl_pm_params_get(&params);
 	m_pm_prev_flag_value = params.cnt_flag;
 	m_pm_event_is_registered = false;
+
+	atomic_set(&m_pm_state, (atomic_val_t)MPSL_PM_INITIALIZED);
+
+	return 0;
+}
+
+int32_t mpsl_pm_utils_uninit(void)
+{
+	int err;
+
+	if (atomic_get(&m_pm_state) != (atomic_val_t)MPSL_PM_INITIALIZED) {
+		return -NRF_EPERM;
+	}
+
+	mpsl_pm_uninit();
+	atomic_set(&m_pm_state, (atomic_val_t)MPSL_PM_UNINITIALIZING);
+	/* In case there is any pended MPSL work item that was not handled, a dedicated
+	 * work item is used to remove PM policy event and unregister latency request.
+	 */
+	(void)k_sem_init(&m_uninit_wait_sem, 0, 1);
+
+	mpsl_work_reschedule(&m_pm_uninit_work, K_NO_WAIT);
+
+	/* Wait for completion of the uninit work item to make sure user can re-initialize
+	 * MPSL PM after return.
+	 */
+	err = k_sem_take(&m_uninit_wait_sem, UNINIT_WORK_WAIT_TIMEOUT_MS);
+	if (err == -EAGAIN) {
+		return -NRF_ETIMEDOUT;
+	} else if (err != 0) {
+		__ASSERT(false, "MPSL PM uninit failed to complete: %d", err);
+		return -NRF_EFAULT;
+	}
+
+	return 0;
 }

include/mpsl/mpsl_pm_utils.h renamed to subsys/mpsl/pm/mpsl_pm_utils.h

@@ -14,8 +14,22 @@ extern "C" {
 /** @brief Initialize MPSL Power Management
  *
  * This routine initializes MPSL PM (via `mpsl_pm_init`).
+ *
+ * @retval 0 MPSL PM was initialized successfully.
+ * @retval -NRF_EPERM MPSL PM is already initialized.
+ */
+int32_t mpsl_pm_utils_init(void);
+
+/** @brief Unitialize MPSL Power Management
+ *
+ * This routine uninitializes MPSL PM (via `mpsl_pm_uninit`).
+ *
+ * @retval 0 MPSL PM was uninitialized successfully.
+ * @retval -NRF_EPERM MPSL was not initialized before the call.
+ * @retval -NRF_ETIMEDOUT MPSL PM uninitialization timed out while waiting for completion.
+ * @retval -NRF_EFAULT MPSL PM uninitialization failed due to unknown reason.
  */
-void mpsl_pm_utils_init(void);
+int32_t mpsl_pm_utils_uninit(void);
 
 /** @brief Handles MPSL Power Management work
  *