Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pal timers #86

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
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 <emil.obalski@nordicsemi.no>
  • Loading branch information
Emil Obalski committed Jun 30, 2021
commit 52c7cded8c2cc4291614041092fef0f9cd6524fe
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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);

Expand All @@ -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);
}

Expand All @@ -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);

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down