From a9e05bb8427db6aceadcac962843904814c8d713 Mon Sep 17 00:00:00 2001 From: Emil Obalski Date: Tue, 22 Jun 2021 16:26:35 +0200 Subject: [PATCH 1/2] zephyr: pal: Rename work to dwork where relevant Update variable name to better reflect its meaning. Signed-off-by: Emil Obalski --- .../OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c b/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c index 409d990e..4f71d6e0 100644 --- a/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c +++ b/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c @@ -181,7 +181,7 @@ palStatus_t pal_plat_osDelay(uint32_t milliseconds) } struct timer_data { - struct k_work_delayable work; + struct k_work_delayable dwork; palTimerFuncPtr callback; void *arg; struct k_mutex lock; @@ -193,7 +193,7 @@ struct timer_data { static void work_fn(struct k_work *work) { struct k_work_delayable *dwork = CONTAINER_OF(work, struct k_work_delayable, work); - struct timer_data *timer_data = CONTAINER_OF(dwork, struct timer_data, work); + struct timer_data *timer_data = CONTAINER_OF(dwork, struct timer_data, dwork); uint64_t uptime = k_uptime_get(); @@ -226,16 +226,17 @@ static void work_fn(struct k_work *work) delay = 0; } - k_work_reschedule(&timer_data->work, K_MSEC(delay)); + k_work_reschedule(&timer_data->dwork, K_MSEC(delay)); /* If timer was started reference is already taken. */ k_sem_take(&timer_data->busy, K_NO_WAIT); } } else { - if (!k_work_reschedule(&timer_data->work, + if (!k_work_reschedule(&timer_data->dwork, K_MSEC(timer_data->timeout - uptime))) { k_sem_take(&timer_data->busy, K_NO_WAIT); } + } /* Release the timer. */ @@ -267,7 +268,7 @@ palStatus_t pal_plat_osTimerCreate(palTimerFuncPtr function, k_sem_init(&timer_data->busy, 2, 2); k_mutex_init(&timer_data->lock); - k_work_init_delayable(&timer_data->work, work_fn); + k_work_init_delayable(&timer_data->dwork, work_fn); *timerID = (palTimerID_t)timer_data; return PAL_SUCCESS; @@ -295,11 +296,11 @@ palStatus_t pal_plat_osTimerStart(palTimerID_t timerID, uint32_t millisec) timer_data->period = millisec; } - if (!k_work_cancel_delayable(&timer_data->work)) { + if (!k_work_cancel_delayable(&timer_data->dwork)) { k_sem_give(&timer_data->busy); - k_work_reschedule(&timer_data->work, get_timeout(millisec)); + k_work_reschedule(&timer_data->dwork, get_timeout(millisec)); k_sem_take(&timer_data->busy, K_NO_WAIT); - } else if (!k_work_reschedule(&timer_data->work, get_timeout(millisec))) { + } else if (!k_work_reschedule(&timer_data->dwork, get_timeout(millisec))) { k_sem_take(&timer_data->busy, K_NO_WAIT); } else { /* Callback cannot be stopped or resubmitted. @@ -327,7 +328,7 @@ palStatus_t pal_plat_osTimerStop(palTimerID_t timerID) timer_data->period = UINT64_MAX; } - if (!k_work_cancel_delayable(&timer_data->work)) { + if (!k_work_cancel_delayable(&timer_data->dwork)) { k_sem_give(&timer_data->busy); } else { /* Callback cannot be stopped. From 52c7cded8c2cc4291614041092fef0f9cd6524fe Mon Sep 17 00:00:00 2001 From: Emil Obalski Date: Tue, 22 Jun 2021 13:13:35 +0200 Subject: [PATCH 2/2] zephyr: pal: Update timers implementation Update implementation of Timer API: - Use absolute k_timeout instead of calculated ones when scheduling works. One exception is when timer work is handled too early. This should not happen but could be handled. - Cancel sync when timer is deleted. In other cases let the timer callback finish the job. - Remove counting semaphore for timer usages. New API allows to better track work state and its not required now. Signed-off-by: Emil Obalski --- .../OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c | 62 +++++-------------- 1 file changed, 16 insertions(+), 46 deletions(-) diff --git a/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c b/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c index 4f71d6e0..e9427c27 100644 --- a/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c +++ b/mbed-client-pal/Source/Port/Reference-Impl/OS_Specific/ZephyrOS/RTOS/pal_plat_rtos.c @@ -182,10 +182,10 @@ palStatus_t pal_plat_osDelay(uint32_t milliseconds) struct timer_data { struct k_work_delayable dwork; + struct k_work_sync work_sync; palTimerFuncPtr callback; void *arg; struct k_mutex lock; - struct k_sem busy; uint64_t timeout; uint64_t period; }; @@ -196,6 +196,7 @@ static void work_fn(struct k_work *work) struct timer_data *timer_data = CONTAINER_OF(dwork, struct timer_data, dwork); uint64_t uptime = k_uptime_get(); + int ret = 0; k_mutex_lock(&timer_data->lock, K_FOREVER); @@ -219,28 +220,16 @@ static void work_fn(struct k_work *work) /* Timer was restarted - timeout was updated. */ } - /* Calculate work delay. */ - int64_t delay = timer_data->timeout - k_uptime_get(); - if (delay < 0) { - /* Timer is late. Can happen for long callbacks. */ - delay = 0; - } - - k_work_reschedule(&timer_data->dwork, K_MSEC(delay)); - - /* If timer was started reference is already taken. */ - k_sem_take(&timer_data->busy, K_NO_WAIT); + ret = k_work_reschedule(&timer_data->dwork, K_TIMEOUT_ABS_MS(timer_data->timeout)); + __ASSERT_NO_MSG(ret > 0); } } else { - if (!k_work_reschedule(&timer_data->dwork, - K_MSEC(timer_data->timeout - uptime))) { - k_sem_take(&timer_data->busy, K_NO_WAIT); - } - + /* Theoretically impossible but could be handled, reschedule remaining time. */ + ret = k_work_reschedule(&timer_data->dwork, K_MSEC(timer_data->timeout - uptime)); + __ASSERT_NO_MSG(ret > 0); } /* Release the timer. */ - k_sem_give(&timer_data->busy); k_mutex_unlock(&timer_data->lock); } @@ -260,13 +249,9 @@ palStatus_t pal_plat_osTimerCreate(palTimerFuncPtr function, timer_data->callback = function; /* Non-zero period marks periodic timer. */ - timer_data->period = (timerType == palOsTimerPeriodic) ? - UINT64_MAX : 0; + timer_data->period = (timerType == palOsTimerPeriodic) ? UINT64_MAX : 0; timer_data->timeout = 0; - /* Two references - extra one is for unstoppable callback. */ - k_sem_init(&timer_data->busy, 2, 2); - k_mutex_init(&timer_data->lock); k_work_init_delayable(&timer_data->dwork, work_fn); @@ -296,18 +281,8 @@ palStatus_t pal_plat_osTimerStart(palTimerID_t timerID, uint32_t millisec) timer_data->period = millisec; } - if (!k_work_cancel_delayable(&timer_data->dwork)) { - k_sem_give(&timer_data->busy); - k_work_reschedule(&timer_data->dwork, get_timeout(millisec)); - k_sem_take(&timer_data->busy, K_NO_WAIT); - } else if (!k_work_reschedule(&timer_data->dwork, get_timeout(millisec))) { - k_sem_take(&timer_data->busy, K_NO_WAIT); - } else { - /* Callback cannot be stopped or resubmitted. - * Let in-callback code to handle the restart. - */ - } - + int ret = k_work_reschedule(&timer_data->dwork, K_TIMEOUT_ABS_MS(timer_data->timeout)); + __ASSERT_NO_MSG(ret > 0); k_mutex_unlock(&timer_data->lock); return PAL_SUCCESS; @@ -328,14 +303,11 @@ palStatus_t pal_plat_osTimerStop(palTimerID_t timerID) timer_data->period = UINT64_MAX; } - if (!k_work_cancel_delayable(&timer_data->dwork)) { - k_sem_give(&timer_data->busy); - } else { - /* Callback cannot be stopped. - * Let in-callback code to handle the restart. - */ - } - + /* Return code is not relevant. + * If Stop was called from timer callback it cannot be stopped. + * Let in-callback code handle the stop. + */ + (void)k_work_cancel_delayable(&timer_data->dwork); k_mutex_unlock(&timer_data->lock); return PAL_SUCCESS; @@ -360,9 +332,7 @@ palStatus_t pal_plat_osTimerDelete(palTimerID_t *timerID) palStatus_t ret = pal_plat_osTimerStop(*timerID); __ASSERT_NO_MSG(ret == PAL_SUCCESS); - /* Wait until timer callback completes. */ - k_sem_take(&timer_data->busy, K_FOREVER); - k_sem_take(&timer_data->busy, K_FOREVER); + (void)k_work_cancel_delayable_sync(&timer_data->dwork, &timer_data->work_sync); /* Nothing is using the resources - delete the timer. */ free(timer_data);