michaelh/zephyr
1
0
Fork 0
Code Releases Activity

portability: cmsis: Bugfix osThreadFlagsWait behavior

Browse source 
The osFlagsWaitAny option is not handled correctly.  It returns once any
flag at all is set, whereas it is supposed to only return once one of
the flags specified in flags is set.

Signed-off-by: Utsav Munendra <utsavm@meta.com>
This commit is contained in:
Utsav Munendra 2025-04-07 11:49:26 -07:00 • committed by Benjamin Cabé
commit 12cba7addf
2 changed files with 97 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

53
subsys/portability/cmsis_rtos_v2/thread_flags.c
View file

@ -109,6 +109,24 @@ uint32_t osThreadFlagsWait(uint32_t flags, uint32_t options, uint32_t timeout)
time_stamp_start = (uint64_t)k_cycle_get_32();
sig = tid->signal_results & flags;
if (options & osFlagsWaitAll) {
/* Check if all events we are waiting on have
* been signalled
*/
if (sig == flags) {
break;
}
} else {
/* Check if any events we are waiting on have
* been signalled
*/
if (sig != 0) {
break;
}
}
switch (timeout) {
case 0:
retval = k_poll(&tid->poll_event, 1, K_NO_WAIT);
@ -138,40 +156,27 @@ uint32_t osThreadFlagsWait(uint32_t flags, uint32_t options, uint32_t timeout)
tid->poll_event.signal->signaled = 0U;
tid->poll_event.state = K_POLL_STATE_NOT_READY;
if (options & osFlagsWaitAll) {
/* Check if all events we are waiting on have
* been signalled
*/
if ((tid->signal_results & flags) == flags) {
break;
}
/* If we need to wait on more signals, we need to
* adjust the timeout value accordingly based on
* the time that has already elapsed.
*/
hwclk_cycles_delta = (uint64_t)k_cycle_get_32() - time_stamp_start;
/* If we need to wait on more signals, we need to
* adjust the timeout value accordingly based on
* the time that has already elapsed.
*/
hwclk_cycles_delta = (uint64_t)k_cycle_get_32() - time_stamp_start;
time_delta_ns = (uint32_t)k_cyc_to_ns_floor64(hwclk_cycles_delta);
time_delta_ns = (uint32_t)k_cyc_to_ns_floor64(hwclk_cycles_delta);
time_delta_ms = (uint32_t)time_delta_ns / NSEC_PER_MSEC;
time_delta_ms = (uint32_t)time_delta_ns / NSEC_PER_MSEC;
if (timeout_ms > time_delta_ms) {
timeout_ms -= time_delta_ms;
} else {
timeout_ms = 0U;
}
if (timeout_ms > time_delta_ms) {
timeout_ms -= time_delta_ms;
} else {
break;
timeout_ms = 0U;
}
}
sig = tid->signal_results;
if (!(options & osFlagsNoClear)) {
/* Clear signal flags as the thread is ready now */
key = irq_lock();
tid->signal_results &= ~(flags);
tid->signal_results &= ~(sig);
irq_unlock(key);
}

70
tests/subsys/portability/cmsis_rtos_v2/src/thread_flags.c
View file

@ -6,14 +6,16 @@
#include <zephyr/ztest.h>
#include <zephyr/kernel.h>
#include <cmsis_os2.h>
#include <zephyr/portability/cmsis_os2.h>
#include <zephyr/portability/cmsis_types.h>
#include <zephyr/irq_offload.h>
#include <zephyr/kernel_structs.h>
#define TIMEOUT_TICKS (10)
#define TIMEOUT_TICKS (1000)
#define FLAG1 (0x00000020)
#define FLAG2 (0x00000004)
#define FLAG3 (0x00000100)
#define FLAG (FLAG1 | FLAG2)
#define ISR_FLAG (0x50)
#define STACKSZ CONFIG_CMSIS_V2_THREAD_MAX_STACK_SIZE
@ -34,6 +36,10 @@ static void thread1(void *arg)
flags = osThreadFlagsGet();
zassert_equal(flags & FLAG1, FLAG1, "");
/* We should be able to get the exact same flags again as they were not cleared */
flags = osThreadFlagsWait(FLAG1, osFlagsWaitAny | osFlagsNoClear, 0);
zassert_equal(flags & FLAG1, FLAG1, "");
/* Clear the Flag explicitly */
flags = osThreadFlagsClear(FLAG1);
zassert_not_equal(flags, osFlagsErrorParameter, "ThreadFlagsClear failed");
@ -161,4 +167,64 @@ ZTEST(cmsis_thread_flags, test_thread_flags_isr)
osDelay(TIMEOUT_TICKS);
}
static K_THREAD_STACK_DEFINE(test_stack4, STACKSZ);
static struct cmsis_rtos_thread_cb test_cb4;
static osThreadAttr_t thread4_attr = {
.name = "Thread4",
.cb_mem = &test_cb4,
.cb_size = sizeof(test_cb4),
.stack_mem = &test_stack4,
.stack_size = STACKSZ,
.priority = osPriorityHigh,
};
static bool m_thread_4_is_blocked;
static void thread4(void *arg)
{
uint32_t flags;
/* Nothing will trigger FLAG1 to this thread, so the following should timeout */
flags = osThreadFlagsWait(FLAG1, osFlagsWaitAny, 0);
zassert_equal(flags, osFlagsErrorTimeout,
"ThreadFlagsWait unexpected found 0x%x flags were set");
flags = osThreadFlagsWait(FLAG1, osFlagsWaitAll, TIMEOUT_TICKS / 10);
zassert_equal(flags, osFlagsErrorTimeout,
"ThreadFlagsWait unexpected found 0x%x flags were set");
flags = osThreadFlagsWait(FLAG1, osFlagsWaitAny | osFlagsNoClear, 0);
zassert_equal(flags, osFlagsErrorTimeout,
"ThreadFlagsWait unexpected found 0x%x flags were set");
/* Nothing will trigger FLAG1 to this thread, so it should remain blocked here */
m_thread_4_is_blocked = true;
flags = osThreadFlagsWait(FLAG1, osFlagsWaitAny, osWaitForever);
zassert_unreachable();
}
ZTEST(cmsis_thread_flags, test_thread_flags_set_flags_not_waited_upon)
{
osThreadId_t id;
uint32_t flags;
m_thread_4_is_blocked = false;
id = osThreadNew(thread4, NULL, &thread4_attr);
zassert_true(id != NULL, "Failed creating thread3");
/* The thread will wait on any of FLAG1. Signal something it is not waiting for. */
flags = osThreadFlagsSet(id, FLAG3);
zassert_equal(flags & FLAG3, FLAG3, "");
osThreadYield();
/* Wait a bit, but thread4 should remain blocked */
osDelay(TIMEOUT_TICKS);
zassert_true(m_thread_4_is_blocked, "Thread 4 did run till expected point");
/* Kill the thread */
osThreadTerminate(id);
}
ZTEST_SUITE(cmsis_thread_flags, NULL, NULL, NULL, NULL, NULL);