jlaitine commented on code in PR #15938: URL: https://github.com/apache/nuttx/pull/15938#discussion_r1984620387
########## arch/arm64/src/common/arm64_arch_timer.c: ########## @@ -265,34 +269,20 @@ static int arm64_tick_start(struct oneshot_lowerhalf_s *lower, { struct arm64_oneshot_lowerhalf_s *priv = (struct arm64_oneshot_lowerhalf_s *)lower; - uint64_t next_cycle; - DEBUGASSERT(priv != NULL && callback != NULL); /* Save the new handler and its argument */ priv->callback = callback; priv->arg = arg; - if (!priv->init[this_cpu()]) - { - /* Enable int */ - - up_enable_irq(ARM_ARCH_TIMER_IRQ); - - /* Start timer */ - - arm64_arch_timer_enable(true); - priv->init[this_cpu()] = true; - } - priv->running = this_cpu(); - next_cycle = - arm64_arch_timer_count() / priv->cycle_per_tick * priv->cycle_per_tick + - ticks * priv->cycle_per_tick; + /* Be careful of the multiply overflow */ + + DEBUGASSERT(ticks <= UINT64_MAX / priv->frequency); - arm64_arch_timer_set_compare(next_cycle); + arm64_arch_timer_set_relative(ticks * priv->frequency / TICK_PER_SEC); Review Comment: Suggestion: just use the same time base for getting the current time and setting the interrupt, and round up the tick starts to the tick boundary like this. This fixes the drift problem. ``` --- a/arch/arm64/src/common/arm64_arch_timer.c +++ b/arch/arm64/src/common/arm64_arch_timer.c @@ -269,6 +269,9 @@ static int arm64_tick_start(struct oneshot_lowerhalf_s *lower, { struct arm64_oneshot_lowerhalf_s *priv = (struct arm64_oneshot_lowerhalf_s *)lower; + + uint64_t count = arm64_arch_timer_count(); + DEBUGASSERT(priv != NULL && callback != NULL); /* Save the new handler and its argument */ @@ -282,7 +285,10 @@ static int arm64_tick_start(struct oneshot_lowerhalf_s *lower, DEBUGASSERT(ticks <= UINT64_MAX / priv->frequency); - arm64_arch_timer_set_relative(ticks * priv->frequency / TICK_PER_SEC); + /* Calculate absolute tick value and set to compare register */ + + ticks += count * TICK_PER_SEC / priv->frequency; + arm64_arch_timer_set_compare(ticks * priv->frequency / TICK_PER_SEC); arm64_arch_timer_set_irq_mask(false); return OK; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org