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

Reply via email to