portability: cmsis: Base Event Flags on k_event

Updates the implementation of the osEventFlags APIs to use
Zephyr kernel events instead of polling.

Fixes #92899

Signed-off-by: Peter Mitsis <[email protected]>

Author: peter-mitsis

include/zephyr/portability/cmsis_types.h

@@ -102,9 +102,7 @@ struct cmsis_rtos_msgq_cb {
  * event flag control block. Control block is initiazed within `osEventFlagsNew()`.
  */
 struct cmsis_rtos_event_cb {
-	struct k_poll_signal poll_signal;
-	struct k_poll_event poll_event;
-	uint32_t signal_results;
+	struct k_event z_event;
 	bool is_cb_dynamic_allocation;
 	const char *name;
 };

subsys/portability/cmsis_rtos_v2/Kconfig

@@ -9,6 +9,7 @@ config CMSIS_RTOS_V2
 	depends on THREAD_MONITOR
 	depends on INIT_STACKS
 	depends on NUM_PREEMPT_PRIORITIES >= 56
+	select EVENTS
 	help
 	  This enables CMSIS RTOS v2 API support. This is an OS-integration
 	  layer which allows applications using CMSIS RTOS V2 APIs to build

subsys/portability/cmsis_rtos_v2/event_flags.c

@@ -43,13 +43,9 @@ osEventFlagsId_t osEventFlagsNew(const osEventFlagsAttr_t *attr)
 		return NULL;
 	}
 	memset(events, 0, sizeof(struct cmsis_rtos_event_cb));
-	events->is_cb_dynamic_allocation = attr->cb_mem == NULL;
-
-	k_poll_signal_init(&events->poll_signal);
-	k_poll_event_init(&events->poll_event, K_POLL_TYPE_SIGNAL, K_POLL_MODE_NOTIFY_ONLY,
-			  &events->poll_signal);
-	events->signal_results = 0U;
 
+	k_event_init(&events->z_event);
+	events->is_cb_dynamic_allocation = (attr->cb_mem == NULL);
 	events->name = (attr->name == NULL) ? init_event_flags_attrs.name : attr->name;
 
 	return (osEventFlagsId_t)events;
@@ -61,19 +57,13 @@ osEventFlagsId_t osEventFlagsNew(const osEventFlagsAttr_t *attr)
 uint32_t osEventFlagsSet(osEventFlagsId_t ef_id, uint32_t flags)
 {
 	struct cmsis_rtos_event_cb *events = (struct cmsis_rtos_event_cb *)ef_id;
-	unsigned int key;
-
 	if ((ef_id == NULL) || (flags & osFlagsError)) {
 		return osFlagsErrorParameter;
 	}
 
-	key = irq_lock();
-	events->signal_results |= flags;
-	irq_unlock(key);
+	k_event_post(&events->z_event, flags);
 
-	k_poll_signal_raise(&events->poll_signal, DONT_CARE);
-
-	return events->signal_results;
+	return k_event_test(&events->z_event, 0xFFFFFFFF);
 }
 
 /**
@@ -82,19 +72,16 @@ uint32_t osEventFlagsSet(osEventFlagsId_t ef_id, uint32_t flags)
 uint32_t osEventFlagsClear(osEventFlagsId_t ef_id, uint32_t flags)
 {
 	struct cmsis_rtos_event_cb *events = (struct cmsis_rtos_event_cb *)ef_id;
-	unsigned int key;
-	uint32_t sig;
+	uint32_t rv;
 
 	if ((ef_id == NULL) || (flags & osFlagsError)) {
 		return osFlagsErrorParameter;
 	}
 
-	key = irq_lock();
-	sig = events->signal_results;
-	events->signal_results &= ~(flags);
-	irq_unlock(key);
+	rv = k_event_test(&events->z_event, 0xFFFFFFFF);
+	k_event_clear(&events->z_event, flags);
 
-	return sig;
+	return rv;
 }
 
 /**
@@ -104,101 +91,44 @@ uint32_t osEventFlagsWait(osEventFlagsId_t ef_id, uint32_t flags, uint32_t optio
 			  uint32_t timeout)
 {
 	struct cmsis_rtos_event_cb *events = (struct cmsis_rtos_event_cb *)ef_id;
-	int retval, key;
-	uint32_t sig;
-	k_timeout_t poll_timeout;
-	uint64_t time_stamp_start, ticks_elapsed;
-	bool flags_are_set;
-
-	/* Can be called from ISRs only if timeout is set to 0 */
-	if (timeout > 0 && k_is_in_isr()) {
+	uint32_t rv;
+	k_timeout_t event_timeout;
+
+	/*
+	 * Return unknown error if called from ISR with a non-zero timeout
+	 * or if flags is zero.
+	 */
+	if (((timeout > 0U) && k_is_in_isr()) || (flags == 0U)) {
 		return osFlagsErrorUnknown;
 	}
 
 	if ((ef_id == NULL) || (flags & osFlagsError)) {
 		return osFlagsErrorParameter;
 	}
 
-	time_stamp_start = (uint64_t)k_uptime_ticks();
-
-	for (;;) {
-
-		flags_are_set = false;
-
-		key = irq_lock();
-
-		if (options & osFlagsWaitAll) {
-			/* Check if all events we are waiting on have
-			 * been signalled
-			 */
-			if ((events->signal_results & flags) == flags) {
-				flags_are_set = true;
-			}
-		} else {
-			/* Check if any of events we are waiting on have
-			 * been signalled
-			 */
-			if (events->signal_results & flags) {
-				flags_are_set = true;
-			}
-		}
-
-		if (flags_are_set) {
-			sig = events->signal_results;
-
-			if (!(options & osFlagsNoClear)) {
-				/* Clear signal flags as the thread is ready now */
-				events->signal_results &= ~(flags);
-			}
-
-			irq_unlock(key);
-
-			break;
-		}
-
-		/* Reset the states to facilitate the next trigger */
-		events->poll_event.signal->signaled = 0U;
-		events->poll_event.state = K_POLL_STATE_NOT_READY;
-
-		irq_unlock(key);
-
-		if (timeout == 0) {
-			return osFlagsErrorTimeout;
-		} else if (timeout == osWaitForever) {
-			poll_timeout = Z_FOREVER;
-		} else {
-			/* If we need to wait on more signals, we need to
-			 * adjust the timeout value accordingly based on
-			 * the time that has already elapsed.
-			 */
-			ticks_elapsed = (uint64_t)k_uptime_ticks() - time_stamp_start;
-
-			if (ticks_elapsed < (uint64_t)timeout) {
-				poll_timeout = Z_TIMEOUT_TICKS(
-					(k_ticks_t)(timeout - (uint32_t)ticks_elapsed));
-			} else {
-				return osFlagsErrorTimeout;
-			}
-		}
-
-		retval = k_poll(&events->poll_event, 1, poll_timeout);
-
-		if (retval == -EAGAIN) {
-			/* k_poll signaled timeout. */
-			return osFlagsErrorTimeout;
-		} else if (retval != 0) {
-			return osFlagsErrorUnknown;
-		}
-
-		/* retval is zero.
-		 * k_poll found some raised signal then loop again and check flags.
-		 */
-		__ASSERT(events->poll_event.state == K_POLL_STATE_SIGNALED,
-			 "event state not signalled!");
-		__ASSERT(events->poll_event.signal->signaled == 1U, "event signaled is not 1");
-	}
-
-	return sig;
+	if (timeout == osWaitForever) {
+		event_timeout = K_FOREVER;
+	} else if (timeout == 0U) {
+		event_timeout = K_NO_WAIT;
+	} else {
+		event_timeout = K_TICKS(timeout);
+	}
+
+	if ((options & osFlagsWaitAll) != 0) {
+		rv = k_event_wait_all(&events->z_event, flags, false, event_timeout);
+	} else {
+		rv = k_event_wait(&events->z_event, flags, false, event_timeout);
+	}
+
+	if ((options & osFlagsNoClear) == 0) {
+		k_event_clear(&events->z_event, flags);
+	}
+
+	if (rv != 0U) {
+		return rv;
+	}
+
+	return (timeout == 0U) ? osFlagsErrorResource : osFlagsErrorTimeout;
 }
 
 /**
@@ -226,7 +156,7 @@ uint32_t osEventFlagsGet(osEventFlagsId_t ef_id)
 		return 0;
 	}
 
-	return events->signal_results;
+	return k_event_test(&events->z_event, 0xFFFFFFFF);
 }
 
 /**

tests/subsys/portability/cmsis_rtos_v2/prj.conf

@@ -16,3 +16,4 @@ CONFIG_CMSIS_V2_THREAD_DYNAMIC_MAX_COUNT=10
 CONFIG_CMSIS_V2_THREAD_MAX_STACK_SIZE=1024
 CONFIG_CMSIS_V2_THREAD_DYNAMIC_STACK_SIZE=1024
 CONFIG_MP_MAX_NUM_CPUS=1
+CONFIG_EVENTS=y

tests/subsys/portability/cmsis_rtos_v2/src/event_flags.c

@@ -232,4 +232,68 @@ ZTEST(cmsis_event_flags, test_event_flags_static_allocation)
 
 	zassert_true(osEventFlagsDelete(evt_id) == osOK, "EventFlagsDelete failed");
 }
+
+static K_THREAD_STACK_DEFINE(test_wait_stack1, STACKSZ);
+static osThreadAttr_t thread_wait_attr1 = {
+	.name = "ThrWait1",
+	.stack_mem = &test_wait_stack1,
+	.stack_size = STACKSZ,
+	.priority = osPriorityHigh,
+};
+
+static K_THREAD_STACK_DEFINE(test_wait_stack2, STACKSZ);
+static osThreadAttr_t thread_wait_attr2 = {
+	.name = "ThrWait2",
+	.stack_mem = &test_wait_stack2,
+	.stack_size = STACKSZ,
+	.priority = osPriorityHigh,
+};
+
+static osEventFlagsAttr_t event_wait_attrs = {
+	.name = "MyEvent",
+	.attr_bits = 0,
+	.cb_mem = NULL,
+	.cb_size = 0,
+};
+
+static void thread_wait1(void *arg)
+{
+	osEventFlagsWait(evt_id, FLAG2, osFlagsWaitAny, osWaitForever);
+}
+
+static void thread_wait2(void *arg)
+{
+	osEventFlagsWait(evt_id, FLAG1, osFlagsWaitAny, osWaitForever);
+}
+
+/*
+ * This test creates two threads that wait on the event flags. It previously
+ * discovered that it would fail on the old CMSIS V2 implementation of event
+ * flags that used polling. This test has been added to show that it no longer
+ * fails.
+ */
+ZTEST(cmsis_event_flags, test_concurrent_wait)
+{
+	evt_id = osEventFlagsNew(&event_wait_attrs);
+
+	/* Create 2 waiting threads */
+	osThreadId_t id1 = osThreadNew(thread_wait1, evt_id, &thread_wait_attr1);
+	osThreadId_t id2 = osThreadNew(thread_wait2, evt_id, &thread_wait_attr2);
+
+	zassert_true(id1 != NULL, "Failed creating thread1");
+	zassert_true(id2 != NULL, "Failed creating thread2");
+	osDelay(10);
+
+	int flags1 = osEventFlagsSet(evt_id, FLAG1);
+	int flags2 = osEventFlagsSet(evt_id, FLAG2);
+
+	osThreadJoin(id1);
+	osThreadJoin(id2);
+
+	zassert_true(flags1 & FLAG1, "");
+	zassert_true(flags2 & FLAG2, "");
+
+	osEventFlagsDelete(evt_id);
+}
+
 ZTEST_SUITE(cmsis_event_flags, NULL, NULL, NULL, NULL, NULL);