Re: [PATCH v2] clocksource/drivers/arm_global_timer: Add auto-detection for initial prescaler values

[Patrice CHOTARD <patrice.chotard@foss.st.com>]

On 8/19/25 09:52, Markus Schneider-Pargmann wrote:
> am43xx has a clock tree where the global timer clock is an indirect child
> of the CPU clock used for frequency scaling:
> 
>   dpll_mpu_ck -- CPU/cpufreq
>         |
>         v
>   dpll_mpu_m2_ck -- divider
>         |
>         v
>   mpu_periphclk -- fixed divider by 2 used for global timer
> 
> When CPU frequency changes, the global timer's clock notifier rejects
> the change because the hardcoded prescaler (1 or 2) cannot accommodate
> the frequency range across all CPU OPPs (300, 600, 720, 800, 1000 MHz).
> 
> Add platform-specific prescaler auto-detection to solve this issue:
> 
> - am43xx: prescaler = 50 (calculated as initial_freq/GCD of all OPP
>   freqs) This allows the timer to work across all CPU frequencies after
>   the fixed divider by 2. Tested on am4372-idk-evm.
> 
> - zynq-7000: prescaler = 2 (preserves previous Kconfig default)
> 
> - Other platforms: prescaler = 1 (previous default)
> 
> The Kconfig option now defaults to 0 (auto-detection) but can still
> override the auto-detected value when set to a non-zero value,
> preserving existing customization workflows.
> 
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> Tested-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> Changes in v2:
> - Rebased to v6.17-rc1
> - Link to v1: https://lore.kernel.org/r/20250808-topic-am43-arm-global-timer-v6-16-v1-1-82067d327580@baylibre.com
> ---
>  drivers/clocksource/Kconfig            |  4 ++--
>  drivers/clocksource/arm_global_timer.c | 44 ++++++++++++++++++++++++++++++----
>  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 645f517a1ac26d0c12d4b2b46591321ddf6e20b3..92df191b25b1af339dffc791f81ff541b09a87a8 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -395,8 +395,7 @@ config ARM_GLOBAL_TIMER
>  
>  config ARM_GT_INITIAL_PRESCALER_VAL
>  	int "ARM global timer initial prescaler value"
> -	default 2 if ARCH_ZYNQ
> -	default 1
> +	default 0
>  	depends on ARM_GLOBAL_TIMER
>  	help
>  	  When the ARM global timer initializes, its current rate is declared
> @@ -406,6 +405,7 @@ config ARM_GT_INITIAL_PRESCALER_VAL
>  	  bounds about how much the parent clock is allowed to decrease or
>  	  increase wrt the initial clock value.
>  	  This affects CPU_FREQ max delta from the initial frequency.
> +	  Use 0 to use auto-detection in the driver.
>  
>  config ARM_TIMER_SP804
>  	bool "Support for Dual Timer SP804 module"
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index 2d86bbc2764a04944e453038013142b2495d26b9..5e3d6bb7e437badeba45d8bf2c1df14eb3427448 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -263,14 +263,13 @@ static void __init gt_delay_timer_init(void)
>  	register_current_timer_delay(&gt_delay_timer);
>  }
>  
> -static int __init gt_clocksource_init(void)
> +static int __init gt_clocksource_init(unsigned int psv)
>  {
>  	writel(0, gt_base + GT_CONTROL);
>  	writel(0, gt_base + GT_COUNTER0);
>  	writel(0, gt_base + GT_COUNTER1);
>  	/* set prescaler and enable timer on all the cores */
> -	writel(FIELD_PREP(GT_CONTROL_PRESCALER_MASK,
> -			  CONFIG_ARM_GT_INITIAL_PRESCALER_VAL - 1) |
> +	writel(FIELD_PREP(GT_CONTROL_PRESCALER_MASK, psv - 1) |
>  	       GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
>  
>  #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> @@ -338,11 +337,45 @@ static int gt_clk_rate_change_cb(struct notifier_block *nb,
>  	return NOTIFY_DONE;
>  }
>  
> +struct gt_prescaler_config {
> +	const char *compatible;
> +	unsigned long prescaler;
> +};
> +
> +static const struct gt_prescaler_config gt_prescaler_configs[] = {
> +	/*
> +	 * On am43 the global timer clock is a child of the clock used for CPU
> +	 * OPPs, so the initial prescaler has to be compatible with all OPPs
> +	 * which are 300, 600, 720, 800 and 1000 with a fixed divider of 2, this
> +	 * gives us a GCD of 10. Initial frequency is 1000, so the prescaler is
> +	 * 50.
> +	 */
> +	{ .compatible = "ti,am43", .prescaler = 50 },
> +	{ .compatible = "xlnx,zynq-7000", .prescaler = 2 },
> +	{ .compatible = NULL }
> +};
> +
> +static unsigned long gt_get_initial_prescaler_value(struct device_node *np)
> +{
> +	const struct gt_prescaler_config *config;
> +
> +	if (CONFIG_ARM_GT_INITIAL_PRESCALER_VAL != 0)
> +		return CONFIG_ARM_GT_INITIAL_PRESCALER_VAL;
> +
> +	for (config = gt_prescaler_configs; config->compatible; config++) {
> +		if (of_machine_is_compatible(config->compatible))
> +			return config->prescaler;
> +	}
> +
> +	return 1;
> +}
> +
>  static int __init global_timer_of_register(struct device_node *np)
>  {
>  	struct clk *gt_clk;
>  	static unsigned long gt_clk_rate;
>  	int err;
> +	unsigned long psv;
>  
>  	/*
>  	 * In A9 r2p0 the comparators for each processor with the global timer
> @@ -378,8 +411,9 @@ static int __init global_timer_of_register(struct device_node *np)
>  		goto out_unmap;
>  	}
>  
> +	psv = gt_get_initial_prescaler_value(np);
>  	gt_clk_rate = clk_get_rate(gt_clk);
> -	gt_target_rate = gt_clk_rate / CONFIG_ARM_GT_INITIAL_PRESCALER_VAL;
> +	gt_target_rate = gt_clk_rate / psv;
>  	gt_clk_rate_change_nb.notifier_call =
>  		gt_clk_rate_change_cb;
>  	err = clk_notifier_register(gt_clk, &gt_clk_rate_change_nb);
> @@ -404,7 +438,7 @@ static int __init global_timer_of_register(struct device_node *np)
>  	}
>  
>  	/* Register and immediately configure the timer on the boot CPU */
> -	err = gt_clocksource_init();
> +	err = gt_clocksource_init(psv);
>  	if (err)
>  		goto out_irq;
>  
> 
> ---
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> change-id: 20250808-topic-am43-arm-global-timer-v6-16-66fc0c6981ab
> 
> Best regards,

Tested on STMicroelectronics STiH410-B2260 board

Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice