Re: [PATCH v3] clocksource: move NXP timer selection to drivers/clocksource

[sashiko-bot@kernel.org]

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