timer: hpet: a few improvements

- That MIN_DELAY is a magic arbitrary number that is never going to be
  right for all cases. Get rid of it in favor of a smarter solution.

- `sys_clock_set_timeout()` should not base its next match value on the
  current time. Tracking the `last_tick` and `last_elapsed` values avoids
  the need for all the tick rounding computation.

- Clamp the next timeout to HPET_MAX_TICKS/2. This leaves room for the
  added elapsed time and any possible IRQ servicing delay.

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>

drivers/timer/hpet.c

@@ -37,11 +37,6 @@
  *
  * HPET_COUNTER_CLK_PERIOD can be overridden in soc.h if
  * COUNTER_CLK_PERIOD is not in femtoseconds (1e-15 sec).
- *
- * HPET_CMP_MIN_DELAY can be overridden in soc.h to better match
- * the frequency of the timers. Default is 1000 where the value
- * written to the comparator must be 1000 larger than the current
- * main counter value.
  */
 
 /* General Configuration register */
@@ -215,11 +210,6 @@ static inline void hpet_timer_comparator_set(uint64_t val)
 #define HPET_COUNTER_CLK_PERIOD		(1000000000000000ULL)
 #endif
 
-#ifndef HPET_CMP_MIN_DELAY
-/* Minimal delay for comparator before the next timer event */
-#define HPET_CMP_MIN_DELAY		(1000)
-#endif
-
 /*
  * HPET_INT_LEVEL_TRIGGER is used to set HPET interrupt as level trigger
  * for ARM CPU with NVIC like EHL PSE, whose DTS interrupt setting
@@ -237,6 +227,8 @@ __WARN("HPET_INT_LEVEL_TRIGGER has no effect, DTS setting is used instead")
 
 static __pinned_bss struct k_spinlock lock;
 static __pinned_bss uint64_t last_count;
+static __pinned_bss uint64_t last_tick;
+static __pinned_bss uint32_t last_elapsed;
 
 #ifdef CONFIG_TIMER_READS_ITS_FREQUENCY_AT_RUNTIME
 static __pinned_bss unsigned int cyc_per_tick;
@@ -261,6 +253,25 @@ static inline void hpet_int_sts_set(uint32_t val)
 }
 #endif
 
+/* ensure the comparator is always set ahead of the current counter value */
+static inline void hpet_timer_comparator_set_safe(uint64_t next)
+{
+	hpet_timer_comparator_set(next);
+
+	uint64_t now = hpet_counter_get();
+
+	if (unlikely((int64_t)(next - now) <= 0)) {
+		uint32_t bump = 1;
+
+		do {
+			next = now + bump;
+			bump *= 2;
+			hpet_timer_comparator_set(next);
+			now = hpet_counter_get();
+		} while ((int64_t)(next - now) <= 0);
+	}
+}
+
 __isr
 static void hpet_isr(const void *arg)
 {
@@ -295,14 +306,13 @@ static void hpet_isr(const void *arg)
 	uint32_t dticks = (uint32_t)((now - last_count) / cyc_per_tick);
 
 	last_count += (uint64_t)dticks * cyc_per_tick;
+	last_tick += dticks;
+	last_elapsed = 0;
 
 	if (!IS_ENABLED(CONFIG_TICKLESS_KERNEL)) {
 		uint64_t next = last_count + cyc_per_tick;
 
-		if ((int64_t)(next - now) < HPET_CMP_MIN_DELAY) {
+		hpet_timer_comparator_set_safe(next);
-			next = now + HPET_CMP_MIN_DELAY;
-		}
-		hpet_timer_comparator_set(next);
 	}
 
 	k_spin_unlock(&lock, key);
@@ -354,28 +364,12 @@ void sys_clock_set_timeout(int32_t ticks, bool idle)
 	}
 
 	ticks = ticks == K_TICKS_FOREVER ? HPET_MAX_TICKS : ticks;
-	ticks = CLAMP(ticks - 1, 0, HPET_MAX_TICKS);
+	ticks = CLAMP(ticks, 0, HPET_MAX_TICKS/2);
 
 	k_spinlock_key_t key = k_spin_lock(&lock);
-	uint64_t now = hpet_counter_get(), cyc, adj;
+	uint64_t cyc = (last_tick + last_elapsed + ticks) * cyc_per_tick;
-	uint64_t max_cyc = (uint64_t)HPET_MAX_TICKS * cyc_per_tick;
 
-	/* Round up to next tick boundary. */
+	hpet_timer_comparator_set_safe(cyc);
-	cyc = (uint64_t)ticks * cyc_per_tick;
-	adj = (now - last_count) + (cyc_per_tick - 1);
-	if (cyc <= max_cyc - adj) {
-		cyc += adj;
-	} else {
-		cyc = max_cyc;
-	}
-	cyc = (cyc / cyc_per_tick) * cyc_per_tick;
-	cyc += last_count;
-
-	if ((int64_t)(cyc - now) < HPET_CMP_MIN_DELAY) {
-		cyc = now + HPET_CMP_MIN_DELAY;
-	}
-
-	hpet_timer_comparator_set(cyc);
 	k_spin_unlock(&lock, key);
 #endif
 }
@@ -391,6 +385,7 @@ uint32_t sys_clock_elapsed(void)
 	uint64_t now = hpet_counter_get();
 	uint32_t ret = (uint32_t)((now - last_count) / cyc_per_tick);
 
+	last_elapsed = ret;
 	k_spin_unlock(&lock, key);
 	return ret;
 }
@@ -462,12 +457,9 @@ static int sys_clock_driver_init(const struct device *dev)
 
 	hpet_gconf_set(reg);
 
-	last_count = hpet_counter_get();
+	last_tick = hpet_counter_get() / cyc_per_tick;
-	if (cyc_per_tick >= HPET_CMP_MIN_DELAY) {
+	last_count = last_tick * cyc_per_tick;
-		hpet_timer_comparator_set(last_count + cyc_per_tick);
+	hpet_timer_comparator_set_safe(last_count + cyc_per_tick);
-	} else {
-		hpet_timer_comparator_set(last_count + HPET_CMP_MIN_DELAY);
-	}
 
 	return 0;
 }