* [PATCH v2 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms
2026-11-02 21:16 [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
@ 2026-11-02 21:16 ` Enric Balletbo i Serra
2026-11-02 21:16 ` [PATCH v2 2/3] clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event Enric Balletbo i Serra
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-02 21:16 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin,
Alexandre Torgue, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: linux-kernel, linux-stm32, linux-arm-kernel, linux-rt-devel,
Ghennadi Procopciuc, NXP S32 Linux Team, stable,
Enric Balletbo i Serra
The NXP STM timer has a 32-bit counter and 32-bit compare registers.
Using ULONG_MAX as the max_delta parameter for
clockevents_config_and_register() is incorrect on 64-bit platforms where
unsigned long is 64 bits, as this exceeds the hardware's capability.
Cap the max_delta to 0x7FFFFFFFU to match the 32-bit compare channel
width, avoiding potential timer event misses or unexpected delays caused
by programming an unreachable compare value.
Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/clocksource/timer-nxp-stm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index 6fe098a4a33f..ee249fed1b91 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -346,7 +346,7 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
* minimum ticks = (rate / MICRO) * 2
*/
clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
- (stm_timer->rate / MICRO) * 2, ULONG_MAX);
+ (stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
return 0;
}
--
2.56.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 2/3] clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event
2026-11-02 21:16 [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
2026-11-02 21:16 ` [PATCH v2 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
@ 2026-11-02 21:16 ` Enric Balletbo i Serra
2026-11-02 21:16 ` [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT Enric Balletbo i Serra
2026-11-03 15:42 ` [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Sebastian Andrzej Siewior
3 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-02 21:16 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin,
Alexandre Torgue, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: linux-kernel, linux-stm32, linux-arm-kernel, linux-rt-devel,
Ghennadi Procopciuc, NXP S32 Linux Team, stable,
Enric Balletbo i Serra
The NXP STM generates an interrupt only on exact match between STM_CNT
and STM_CMP0. If the counter passes the comparator value before the
overshoot check runs, the match is missed and the hardware must count
through all 2^32 ticks (~32.2s at 133 MHz) before firing, causing soft
lockups.
The existing overshoot check:
if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
simplifies to checking if counter_before > counter_after, which only
detects a 32-bit counter wrap and misses the normal overshoot case where
the counter advances past val.
Use a signed comparison on the difference between the target and current
counter values instead, following the established pattern in
timer-imx-gpt. If the counter has reached or passed val, the signed
difference goes to zero or negative and we return -ETIME so the
clockevents layer retries immediately.
Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/clocksource/timer-nxp-stm.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ee249fed1b91..ed7662a4e2bc 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -247,7 +247,7 @@ static int nxp_stm_clockevent_shutdown(struct clock_event_device *ced)
static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
{
struct stm_timer *stm_timer = ced_to_stm(ced);
- u32 val;
+ u32 val, now;
nxp_stm_clockevent_disable(stm_timer);
@@ -258,16 +258,12 @@ 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.
+ * If the counter reached or passed 'val' during or before the
+ * write to the comparator, the hardware match will be missed,
+ * resulting in a full 32-bit (32.2s) wraparound lockup.
*/
- if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
+ now = nxp_stm_clockevent_read_counter(stm_timer);
+ if ((int)(val - now) <= 0)
return -ETIME;
nxp_stm_clockevent_enable(stm_timer);
--
2.56.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
2026-11-02 21:16 [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
2026-11-02 21:16 ` [PATCH v2 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
2026-11-02 21:16 ` [PATCH v2 2/3] clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event Enric Balletbo i Serra
@ 2026-11-02 21:16 ` Enric Balletbo i Serra
2026-11-02 21:25 ` sashiko-bot
2026-11-03 15:42 ` [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Sebastian Andrzej Siewior
3 siblings, 1 reply; 7+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-02 21:16 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin,
Alexandre Torgue, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt
Cc: linux-kernel, linux-stm32, linux-arm-kernel, linux-rt-devel,
Ghennadi Procopciuc, NXP S32 Linux Team, stable,
Enric Balletbo i Serra
Under heavy bus contention on PREEMPT_RT kernels, a 2us min_delta is too
tight and causes ~32-second timer soft lockups.
Increase min_delta from 2us to 20us (~2,660 cycles at 133 MHz). This
provides sufficient margin under severe bus contention to ensure the
channel-enable write completes well before the counter reaches the match
value, resolving the 32-second soft lockup.
Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/clocksource/timer-nxp-stm.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ed7662a4e2bc..a1a0e23d3342 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -335,14 +335,18 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
/*
* 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.
+ * value 1100 ns at 133MHz rate frequency. However, under heavy
+ * bus contention on PREEMPT_RT, 2us is too tight and can allow
+ * the counter to pass the match value before the channel-enable
+ * write takes effect, leading to 32s wraparounds.
*
- * minimum ticks = (rate / MICRO) * 2
+ * Increase to 20us to guarantee the comparator channel is fully
+ * enabled before the match value is reached.
+ *
+ * minimum ticks = (rate / MICRO) * 20
*/
clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
- (stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
+ (stm_timer->rate / MICRO) * 20, 0x7FFFFFFFU);
return 0;
}
--
2.56.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
2026-11-02 21:16 ` [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT Enric Balletbo i Serra
@ 2026-11-02 21:25 ` sashiko-bot
2026-11-02 21:47 ` Enric Balletbo i Serra
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-11-02 21:25 UTC (permalink / raw)
To: Enric Balletbo i Serra; +Cc: linux-rt-devel, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clocksource/drivers/nxp-stm: overshoot check precedes channel enable
--
commit 1283cc4078852e277605e923b00287b73e75d175
Author: Enric Balletbo i Serra <eballetb@redhat.com>
clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
This commit increases the clockevent min_delta from 2us to 20us in
nxp_stm_clockevent_starting_cpu() to prevent 32-second timer wraparound
lockups under heavy bus contention on PREEMPT_RT kernels.
> diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> index ed7662a4e2bc4..a1a0e23d33426 100644
> --- a/drivers/clocksource/timer-nxp-stm.c
> +++ b/drivers/clocksource/timer-nxp-stm.c
> @@ -335,14 +335,18 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
> /*
> * 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.
> + * value 1100 ns at 133MHz rate frequency. However, under heavy
> + * bus contention on PREEMPT_RT, 2us is too tight and can allow
> + * the counter to pass the match value before the channel-enable
> + * write takes effect, leading to 32s wraparounds.
> *
> - * minimum ticks = (rate / MICRO) * 2
> + * Increase to 20us to guarantee the comparator channel is fully
> + * enabled before the match value is reached.
> + *
> + * minimum ticks = (rate / MICRO) * 20
> */
> clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
> - (stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
> + (stm_timer->rate / MICRO) * 20, 0x7FFFFFFFU);
[Severity: Medium]
As reported by Sashiko on v1 [1], does increasing min_delta to 20us in
nxp_stm_clockevent_starting_cpu() actually guarantee that the comparator
channel is enabled before the match value is reached?
Looking at drivers/clocksource/timer-nxp-stm.c:
static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
{
struct stm_timer *stm_timer = ced_to_stm(ced);
u32 val, now;
nxp_stm_clockevent_disable(stm_timer);
stm_timer->delta = delta;
val = nxp_stm_clockevent_read_counter(stm_timer) + delta;
writel(val, STM_CMP0(stm_timer->base));
...
now = nxp_stm_clockevent_read_counter(stm_timer);
if ((int)(val - now) <= 0)
return -ETIME;
nxp_stm_clockevent_enable(stm_timer);
return 0;
}
nxp_stm_clockevent_disable() clears STM_CCR_CEN in STM_CCR0, and the
hardware only generates a match event when STM_CNT equals STM_CMP0 while
STM_CCR0[CEN] is enabled. Because nxp_stm_clockevent_enable() is called
after reading now and checking (int)(val - now) <= 0, the latency of writing
STM_CCR_CEN to STM_CCR0 is not covered by the -ETIME overshoot check.
For any programmed delta (20us or larger), if bus contention, an NMI, or
hypervisor/firmware delay between the two counter reads leaves val - now as
a small positive number of ticks, the overshoot check passes while the
channel is still disabled, and STM_CNT can pass val before the subsequent
nxp_stm_clockevent_enable() write takes effect in hardware.
Should nxp_stm_clockevent_set_next_event() enable the comparator channel
before reading now for the overshoot check (and call
nxp_stm_clockevent_disable() if (int)(val - now) <= 0 before returning
-ETIME) so the counter read is ordered after the STM_CCR0 write on the bus?
[1] https://lore.kernel.org/r/20261026-nxp-stm-fixes-v1-3-20a64a4ea454@redhat.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-nxp-stm-fixes-v2-0-8ca2706f30ef@redhat.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
2026-11-02 21:25 ` sashiko-bot
@ 2026-11-02 21:47 ` Enric Balletbo i Serra
0 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-02 21:47 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-rt-devel, linux-kernel
Hi,
On Mon, Nov 2, 2026 at 10:25 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] clocksource/drivers/nxp-stm: overshoot check precedes channel enable
> --
>
> commit 1283cc4078852e277605e923b00287b73e75d175
> Author: Enric Balletbo i Serra <eballetb@redhat.com>
>
> clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
>
> This commit increases the clockevent min_delta from 2us to 20us in
> nxp_stm_clockevent_starting_cpu() to prevent 32-second timer wraparound
> lockups under heavy bus contention on PREEMPT_RT kernels.
>
> > diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> > index ed7662a4e2bc4..a1a0e23d33426 100644
> > --- a/drivers/clocksource/timer-nxp-stm.c
> > +++ b/drivers/clocksource/timer-nxp-stm.c
> > @@ -335,14 +335,18 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
> > /*
> > * 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.
> > + * value 1100 ns at 133MHz rate frequency. However, under heavy
> > + * bus contention on PREEMPT_RT, 2us is too tight and can allow
> > + * the counter to pass the match value before the channel-enable
> > + * write takes effect, leading to 32s wraparounds.
> > *
> > - * minimum ticks = (rate / MICRO) * 2
> > + * Increase to 20us to guarantee the comparator channel is fully
> > + * enabled before the match value is reached.
> > + *
> > + * minimum ticks = (rate / MICRO) * 20
> > */
> > clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
> > - (stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
> > + (stm_timer->rate / MICRO) * 20, 0x7FFFFFFFU);
>
> [Severity: Medium]
> As reported by Sashiko on v1 [1], does increasing min_delta to 20us in
> nxp_stm_clockevent_starting_cpu() actually guarantee that the comparator
> channel is enabled before the match value is reached?
>
> Looking at drivers/clocksource/timer-nxp-stm.c:
>
> static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
> {
> struct stm_timer *stm_timer = ced_to_stm(ced);
> u32 val, now;
>
> nxp_stm_clockevent_disable(stm_timer);
>
> stm_timer->delta = delta;
>
> val = nxp_stm_clockevent_read_counter(stm_timer) + delta;
>
> writel(val, STM_CMP0(stm_timer->base));
> ...
> now = nxp_stm_clockevent_read_counter(stm_timer);
> if ((int)(val - now) <= 0)
> return -ETIME;
>
> nxp_stm_clockevent_enable(stm_timer);
>
> return 0;
> }
>
> nxp_stm_clockevent_disable() clears STM_CCR_CEN in STM_CCR0, and the
> hardware only generates a match event when STM_CNT equals STM_CMP0 while
> STM_CCR0[CEN] is enabled. Because nxp_stm_clockevent_enable() is called
> after reading now and checking (int)(val - now) <= 0, the latency of writing
> STM_CCR_CEN to STM_CCR0 is not covered by the -ETIME overshoot check.
>
> For any programmed delta (20us or larger), if bus contention, an NMI, or
> hypervisor/firmware delay between the two counter reads leaves val - now as
> a small positive number of ticks, the overshoot check passes while the
> channel is still disabled, and STM_CNT can pass val before the subsequent
> nxp_stm_clockevent_enable() write takes effect in hardware.
>
> Should nxp_stm_clockevent_set_next_event() enable the comparator channel
> before reading now for the overshoot check (and call
> nxp_stm_clockevent_disable() if (int)(val - now) <= 0 before returning
> -ETIME) so the counter read is ordered after the STM_CCR0 write on the bus?
>
I answered this in the first version of the PATCH 2/3, this describes
the same race you pointed in the PATCH 2/3 and I think the race is
possible but I believe it cannot occur in practice with the current
min_delta:
set_next_event is called with local interrupts disabled, so
nothing can preempt or extend that window. The gap between reading
now and the writel in nxp_stm_clockevent_enable() is a
comparison, a branch, and a single MMIO write. With min_delta = 2us, the
check is still close to the write.
Reordering to enable-before-check introduces a different problem: if
the comparator match fires between nxp_stm_clockevent_enable() and
reading now, the interrupt handler delivers the event via
ced->event_handler(ced) and then we also return -ETIME, causing, if
I'm not mistaken
the clockevents layer to program another event, causing a double delivery.
Thanks,
Enric
> [1] https://lore.kernel.org/r/20261026-nxp-stm-fixes-v1-3-20a64a4ea454@redhat.com
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261102-nxp-stm-fixes-v2-0-8ca2706f30ef@redhat.com?part=3
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups
2026-11-02 21:16 [PATCH v2 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
` (2 preceding siblings ...)
2026-11-02 21:16 ` [PATCH v2 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT Enric Balletbo i Serra
@ 2026-11-03 15:42 ` Sebastian Andrzej Siewior
3 siblings, 0 replies; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-11-03 15:42 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin,
Alexandre Torgue, Clark Williams, Steven Rostedt, linux-kernel,
linux-stm32, linux-arm-kernel, linux-rt-devel,
Ghennadi Procopciuc, NXP S32 Linux Team, stable
On 2026-11-02 22:16:19 [+0100], Enric Balletbo i Serra wrote:
> This series fixes three bugs in the NXP STM clocksource driver that can
> cause soft lockups and missed timer events, particularly visible on
> PREEMPT_RT kernels running on S32G platforms.
>
> The NXP STM generates an interrupt only on exact match between the
> counter and comparator registers. If the counter passes the programmed
> value, the match is missed and the hardware must count through all 2^32
> ticks (~32.2s at 133 MHz) before firing, causing soft lockups.
>
> Patch 1 fixes max_delta: on 64-bit platforms, ULONG_MAX exceeds the
> hardware's 32-bit compare register width, making it possible to program
> an unreachable compare value.
This looks fine
> Patch 2 fixes the overshoot detection in set_next_event: the existing
> check only detected counter wraps, not the normal case where the counter
> advances past the programmed value during the read-modify-write window.
> Replace it with a signed comparison following the pattern used by
> timer-imx-gpt.
okay…
> Patch 3 increases min_delta from 2us to 20us: under bus contention on
> PREEMPT_RT, 2us is not enough margin and the counter frequently
> overshoots the programmed value, triggering repeated -ETIME retries.
This all sounds like the hpet trouble on x86. I'm not sure what heavy
busy contention means and why the device is enabled/ disable during
setting. I would assume that the programmed value hits the HW after the
wrap so you wait until the counter wraps again for the equal match.
Worst case would be you read back the expected value but the HW sees it
later (after the wrap).
hpet_clkevt_set_next_event() does in principle the same check as you in
#2 except it compares against the "min" value instead <=0.
Would this make sense?
Sebastian
^ permalink raw reply [flat|nested] 7+ messages in thread