Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource
@ 2026-05-14 11:14 Enric Balletbo i Serra
  2026-05-14 19:05 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Enric Balletbo i Serra @ 2026-05-14 11:14 UTC (permalink / raw)
  To: Russell King, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Daniel Lezcano, Thomas Gleixner
  Cc: linux-arm-kernel, imx, linux-kernel, Enric Balletbo i Serra

From: Enric Balletbo i Serra <eballetb@redhat.com>

The Kconfig logic for selecting the scheduler clocksource on
NXP Vybrid (VF610) uses a `choice` block restricted to 32-bit ARM. This
prevents 64-bit architectures, such as the NXP S32 family, from enabling
the NXP Periodic Interrupt Timer (PIT) driver (CONFIG_NXP_PIT_TIMER).

Relocate the NXP clocksource selection from arch/arm/mach-imx/Kconfig to
drivers/clocksource/Kconfig. This allows the configuration to be shared
across different architectures.

Update the selection to include support for ARCH_S32 and add a "None"
option restricted to ARCH_S32, since Vybrid lacks the ARM Architected
Timer. The Vybrid Global Timer option is restricted to ARCH_MULTI_V7
SOC_VF610 platforms to prevent it from being visible on Cortex-M4 builds,
which lack the ARM Global Timer hardware.

Fixes: bee33f22d7c3 ("clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
Changes in v3:
- Restrict VF_TIMER_NONE to ARCH_S32 to prevent selecting it on Vybrid
  platforms which lack the ARM Architected Timer
- Link to v2: https://lore.kernel.org/r/20260513-fix-nxp-timer-v2-1-533b99c57b67@redhat.com

Changes in v2:
- Fix VF_USE_ARM_GLOBAL_TIMER dependency: use ARCH_MULTI_V7 instead of
  ARM to prevent the option from being visible on Cortex-M4 builds
  (Sashiko AI review)
- Link to v1: https://lore.kernel.org/r/20260302-fix-nxp-timer-v1-1-af4bc62d4ffa@redhat.com
---
 arch/arm/mach-imx/Kconfig   | 21 ---------------------
 drivers/clocksource/Kconfig | 31 +++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 6ea1bd55acf8..a361840d7a04 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -227,27 +227,6 @@ config SOC_VF610
 	help
 	  This enables support for Freescale Vybrid VF610 processor.
 
-choice
-	prompt "Clocksource for scheduler clock"
-	depends on SOC_VF610
-	default VF_USE_ARM_GLOBAL_TIMER
-
-	config VF_USE_ARM_GLOBAL_TIMER
-		bool "Use ARM Global Timer"
-		depends on ARCH_MULTI_V7
-		select ARM_GLOBAL_TIMER
-		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
-		help
-		  Use the ARM Global Timer as clocksource
-
-	config VF_USE_PIT_TIMER
-		bool "Use PIT timer"
-		select NXP_PIT_TIMER
-		help
-		  Use SoC Periodic Interrupt Timer (PIT) as clocksource
-
-endchoice
-
 endif
 
 endif
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index d1a33a231a44..d9c76dd443f8 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -793,4 +793,35 @@ config RTK_SYSTIMER
 	  this option only when building for a Realtek platform or for compilation
 	  testing.
 
+choice
+	prompt "NXP clocksource for scheduler clock"
+	depends on SOC_VF610 || ARCH_S32
+	# Default to Global Timer for Vybrid (32-bit)
+	default VF_USE_ARM_GLOBAL_TIMER if SOC_VF610
+	# Default to None for S32 (64-bit)
+	default VF_TIMER_NONE if ARCH_S32
+
+	config VF_USE_ARM_GLOBAL_TIMER
+		bool "Use NXP Vybrid Global Timer"
+		depends on ARCH_MULTI_V7 && SOC_VF610
+		select ARM_GLOBAL_TIMER
+		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
+		help
+		  Use the NXP Vybrid Global Timer as clocksource.
+
+	config VF_USE_PIT_TIMER
+		bool "Use NXP PIT timer"
+		select NXP_PIT_TIMER
+		help
+		  Use NXP Periodic Interrupt Timer (PIT) as clocksource.
+
+	config VF_TIMER_NONE
+		bool "None (Use standard Arch Timer)"
+		depends on ARCH_S32
+		help
+		  Do not use any specific NXP timer driver. Use the standard
+		  ARM Architected Timer instead.
+
+endchoice
+
 endmenu

---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260302-fix-nxp-timer-9cb1fbd7afcd

Best regards,
-- 
Enric Balletbo i Serra <eballetb@redhat.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource
  2026-05-14 11:14 [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource Enric Balletbo i Serra
@ 2026-05-14 19:05 ` sashiko-bot
  2026-05-15  7:39   ` Enric Balletbo Serra
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-05-14 19:05 UTC (permalink / raw)
  To: Enric Balletbo i Serra; +Cc: imx, Frank.Li

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Shoehorning ARCH_S32 (ARM64) into the Vybrid scheduler clock choice block risks replacing the native ARM Architected Timer with a slower 32-bit MMIO timer.
--

commit 37ddcc4eb82e3bfe1a0ebf5e407f22b4f2b1bef7
Author: Enric Balletbo i Serra <eballetb@redhat.com>

clocksource: move NXP timer selection to drivers/clocksource

This commit relocates the NXP clocksource Kconfig logic from
arch/arm/mach-imx/Kconfig to drivers/clocksource/Kconfig. It updates
the selection to include support for the 64-bit ARCH_S32 platform,
adding a "None" option to fallback to the standard ARM Architected Timer.

[ ... ]

> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index d1a33a231a44c..d9c76dd443f8a 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -793,4 +793,35 @@ config RTK_SYSTIMER
>  	  this option only when building for a Realtek platform or for compilation
>  	  testing.
>  
> +choice
> +	prompt "NXP clocksource for scheduler clock"
> +	depends on SOC_VF610 || ARCH_S32
> +	# Default to Global Timer for Vybrid (32-bit)
> +	default VF_USE_ARM_GLOBAL_TIMER if SOC_VF610
> +	# Default to None for S32 (64-bit)
> +	default VF_TIMER_NONE if ARCH_S32
> +
> +	config VF_USE_ARM_GLOBAL_TIMER
> +		bool "Use NXP Vybrid Global Timer"
> +		depends on ARCH_MULTI_V7 && SOC_VF610
> +		select ARM_GLOBAL_TIMER
> +		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> +		help
> +		  Use the NXP Vybrid Global Timer as clocksource.
> +
> +	config VF_USE_PIT_TIMER
> +		bool "Use NXP PIT timer"
> +		select NXP_PIT_TIMER
> +		help
> +		  Use NXP Periodic Interrupt Timer (PIT) as clocksource.

Could this replace the native ARM Architected Timer with a slower 32-bit MMIO
timer on S32 systems?

Since NXP_PIT_TIMER has no prompt of its own, an S32 user needing the PIT
timer for other valid purposes (like clock events) is forced to select
VF_USE_PIT_TIMER from this choice block.

If selected, the timer-nxp-pit.c driver unconditionally calls
sched_clock_register() during initialization. This automatically overrides the
existing scheduler clock if the new clock's rate is higher than or equal to
the current one.

Because the PIT timer is typically driven by a high-speed system bus, its
frequency will likely exceed the ARM Architected Timer's standard 25-50 MHz
rate. Overriding the heavily optimized 64-bit system-register Architected
Timer with a 32-bit memory-mapped PIT timer introduces massive read latency
for every sched_clock() call.

Would it be better to expose NXP_PIT_TIMER as a standalone option for S32,
and modify the driver to avoid registering as sched_clock on ARM64
architectures?

> +
> +	config VF_TIMER_NONE
> +		bool "None (Use standard Arch Timer)"
> +		depends on ARCH_S32
> +		help
> +		  Do not use any specific NXP timer driver. Use the standard
> +		  ARM Architected Timer instead.
> +
> +endchoice
> +
>  endmenu

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-fix-nxp-timer-v3-1-a3e68fdb505e@redhat.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource
  2026-05-14 19:05 ` sashiko-bot
@ 2026-05-15  7:39   ` Enric Balletbo Serra
  0 siblings, 0 replies; 3+ messages in thread
From: Enric Balletbo Serra @ 2026-05-15  7:39 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Enric Balletbo i Serra, imx, Frank.Li

Hi,

For some reason this doesn't appear as a thread in my original email.
Answering your question below.

Missatge de l'adreça <sashiko-bot@kernel.org> del dia dj., 14 de maig
2026 a les 21:05:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Shoehorning ARCH_S32 (ARM64) into the Vybrid scheduler clock choice block risks replacing the native ARM Architected Timer with a slower 32-bit MMIO timer.
> --
>
> commit 37ddcc4eb82e3bfe1a0ebf5e407f22b4f2b1bef7
> Author: Enric Balletbo i Serra <eballetb@redhat.com>
>
> clocksource: move NXP timer selection to drivers/clocksource
>
> This commit relocates the NXP clocksource Kconfig logic from
> arch/arm/mach-imx/Kconfig to drivers/clocksource/Kconfig. It updates
> the selection to include support for the 64-bit ARCH_S32 platform,
> adding a "None" option to fallback to the standard ARM Architected Timer.
>
> [ ... ]
>
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index d1a33a231a44c..d9c76dd443f8a 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -793,4 +793,35 @@ config RTK_SYSTIMER
> >         this option only when building for a Realtek platform or for compilation
> >         testing.
> >
> > +choice
> > +     prompt "NXP clocksource for scheduler clock"
> > +     depends on SOC_VF610 || ARCH_S32
> > +     # Default to Global Timer for Vybrid (32-bit)
> > +     default VF_USE_ARM_GLOBAL_TIMER if SOC_VF610
> > +     # Default to None for S32 (64-bit)
> > +     default VF_TIMER_NONE if ARCH_S32
> > +
> > +     config VF_USE_ARM_GLOBAL_TIMER
> > +             bool "Use NXP Vybrid Global Timer"
> > +             depends on ARCH_MULTI_V7 && SOC_VF610
> > +             select ARM_GLOBAL_TIMER
> > +             select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> > +             help
> > +               Use the NXP Vybrid Global Timer as clocksource.
> > +
> > +     config VF_USE_PIT_TIMER
> > +             bool "Use NXP PIT timer"
> > +             select NXP_PIT_TIMER
> > +             help
> > +               Use NXP Periodic Interrupt Timer (PIT) as clocksource.
>
> Could this replace the native ARM Architected Timer with a slower 32-bit MMIO
> timer on S32 systems?
>
> Since NXP_PIT_TIMER has no prompt of its own, an S32 user needing the PIT
> timer for other valid purposes (like clock events) is forced to select
> VF_USE_PIT_TIMER from this choice block.
>
> If selected, the timer-nxp-pit.c driver unconditionally calls
> sched_clock_register() during initialization. This automatically overrides the
> existing scheduler clock if the new clock's rate is higher than or equal to
> the current one.
>
> Because the PIT timer is typically driven by a high-speed system bus, its
> frequency will likely exceed the ARM Architected Timer's standard 25-50 MHz
> rate. Overriding the heavily optimized 64-bit system-register Architected
> Timer with a 32-bit memory-mapped PIT timer introduces massive read latency
> for every sched_clock() call.
>
> Would it be better to expose NXP_PIT_TIMER as a standalone option for S32,
> and modify the driver to avoid registering as sched_clock on ARM64
> architectures?
>

This is pre-existing driver behavior that applies equally on Vybrid
today. The choice block defaults to VF_TIMER_NONE on S32, so the PIT
is only selected if the user explicitly requests it. The choice prompt
("NXP clocksource for scheduler clock") makes the intent clear.

Decoupling NXP_PIT_TIMER from sched_clock registration on arm64 would
be a worthwhile improvement to the driver itself, but I think that's a
separate change from this Kconfig reorganization.

Does it make sense?

Cheers,
  Enric

> > +
> > +     config VF_TIMER_NONE
> > +             bool "None (Use standard Arch Timer)"
> > +             depends on ARCH_S32
> > +             help
> > +               Do not use any specific NXP timer driver. Use the standard
> > +               ARM Architected Timer instead.
> > +
> > +endchoice
> > +
> >  endmenu
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260514-fix-nxp-timer-v3-1-a3e68fdb505e@redhat.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-15  7:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 11:14 [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource Enric Balletbo i Serra
2026-05-14 19:05 ` sashiko-bot
2026-05-15  7:39   ` Enric Balletbo Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox