The existing overshoot check effectively compares two counter readings and only triggers on a counter wraparound between reads; the normal case where the counter advances past the programmed value during the read/write sequence is not detected. Replace it with the approach of hpet_clkevt_set_next_event(): read the counter after the comparator write and return -ETIME when less than STM_MIN_CYCLES remain until the match. Derive the minimum programming delta from that margin. See the cover letter for the measurement procedure and constant derivation. Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms") Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Enric Balletbo i Serra --- drivers/clocksource/timer-nxp-stm.c | 53 ++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c index ee249fed1b91..81527745b7e2 100644 --- a/drivers/clocksource/timer-nxp-stm.c +++ b/drivers/clocksource/timer-nxp-stm.c @@ -20,7 +20,6 @@ #include #include #include -#include #define STM_CR(__base) (__base) @@ -52,6 +51,17 @@ #define STM_ENABLE_MASK (STM_CR_FRZ | STM_CR_TEN) +/* + * The comparator fires only on an exact match with the counter. + * STM_MIN_CYCLES is the post-write overshoot margin used by + * nxp_stm_clockevent_set_next_event(), following the same approach + * as hpet_clkevt_set_next_event(). STM_MIN_PROG_DELTA is the + * minimum programming delta derived from it. Both values are + * empirically sized from measurements on S32G. + */ +#define STM_MIN_CYCLES 512 +#define STM_MIN_PROG_DELTA (STM_MIN_CYCLES * 3) + struct stm_timer { void __iomem *base; unsigned long rate; @@ -248,6 +258,7 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e { struct stm_timer *stm_timer = ced_to_stm(ced); u32 val; + s32 res; nxp_stm_clockevent_disable(stm_timer); @@ -258,16 +269,26 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e writel(val, STM_CMP0(stm_timer->base)); /* - * The counter is shared across the channels and can not be - * stopped while we are setting the next event. If the delta - * is very small it is possible the counter increases above - * the computed 'val'. The min_delta value specified when - * registering the clockevent will prevent that. The second - * case is if the counter wraps while we compute the 'val' and - * before writing the comparator register. We read the counter, - * check if we are back in time and abort the timer with -ETIME. + * The comparator generates an interrupt only on an exact match + * with the counter. If the counter reaches the programmed value + * before the write to the comparator takes effect, the match is + * missed and the next interrupt only fires after a full 32-bit + * counter wraparound (~32.2s at 133 MHz). + * + * The counter read below targets the same module as the + * comparator write, so it can only complete once the write has + * reached the device, and the signed difference between the + * programmed value and the counter reflects the hardware state at + * that point. If less than STM_MIN_CYCLES remain to the match, the + * write might not have made it into the comparator in time; + * return -ETIME so the clockevents core retries with a larger + * delta. + * + * This is the same approach as hpet_clkevt_set_next_event(). */ - if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta) + res = (s32)(val - nxp_stm_clockevent_read_counter(stm_timer)); + + if (res < STM_MIN_CYCLES) return -ETIME; nxp_stm_clockevent_enable(stm_timer); @@ -337,16 +358,12 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu) return ret; /* - * The timings measurement show reading the counter register - * and writing to the comparator register takes as a maximum - * value 1100 ns at 133MHz rate frequency. The timer must be - * set above this value and to be secure we set the minimum - * value equal to 2000ns, so 2us. - * - * minimum ticks = (rate / MICRO) * 2 + * The minimum programming delta absorbs the measured worst case + * read/write/read round trip and the overshoot margin, see the + * comment on STM_MIN_CYCLES. */ clockevents_config_and_register(&stm_timer->ced, stm_timer->rate, - (stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU); + STM_MIN_PROG_DELTA, 0x7FFFFFFFU); return 0; } -- 2.56.0