[PATCH] clocksource/drivers/timer-mediatek: Don't override the arch timer sched_clock

[PATCH] clocksource/drivers/timer-mediatek: Don't override the arch timer sched_clock

[Chen-Yu Tsai]

Since the driver was converted to a platform driver, the GPT probes
late, after the architected timer has registered itself as sched_clock.
sched_clock_register() only refuses counters with a lower rate, and
sched_clock has no notion of priority, so the 32-bit GPT counter, which
runs at the same 13 MHz as the arch timer on MT8173, replaces the 56-bit
arch timer counter as sched_clock:

  sched_clock: 56 bits at 13MHz, resolution 76ns, wraps every 4398046511101ns
  ...
  sched_clock: 32 bits at 13MHz, resolution 76ns, wraps every 165191050201ns

Before the conversion, both drivers used TIMER_OF_DECLARE and
timer-mediatek.o is linked before arm_arch_timer.o, so the arch timer
registered last and won.

The narrower GPT counter wraps more often and is slower to read over
MMIO than the arch timer system register. Doing the switch at runtime
also runs enable_sched_clock_irqtime(), which may sleep, with interrupts
disabled, causing a splat.

Other drivers worked around the same problem in the same way, see

  commit 37b8b003c483 ("clocksource/drivers/sun4i-timer: Only register a sched_clock on sun4i and sun5i")
  commit 10720e120e2b ("clocksource/drivers/imx-tpm: Exclude sched clock for ARM64")

Only register the GPT as sched_clock if the ARM architected timer is not
present. This currently includes MT6572, MT6580, MT6582, and MT6592, as
seen in their respective .dtsi files. This follows the example of
sun4i-timer.

The analysis of the boot log and git history, and drafting of this
change, were done with an AI agent. The agent originally proposed using
arch_timer_get_rate(), which was replaced by the author.

Manually tested on a MT8173 Hana (Telesu).

Fixes: 7a097aba7255 ("clocksource/drivers/timer-mediatek: Convert to OF probing")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---

For the ARMv7 SoCs such as the MT2701, MT8127, and MT8135, the firmware
has to correctly set the arch timer frequency, as their device trees do
not have the clock-frequency property.

OTOH we could let sched_clock_register() reject replacing the current
counter if the new one runs at the same rate. But that would be a
separate discussion: if more drivers are converted to standard platform
drivers, then the probe order is no longer easily known like before
by the link order.

The splat is a separate issue that I already reported [1].

[1] https://lore.kernel.org/all/20261109095036.GA722431@google.com/
---
 drivers/clocksource/timer-mediatek.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
index 1b2cbcc2795a..ef67cc783635 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/sched_clock.h>
@@ -334,8 +335,20 @@ static int mtk_gpt_init(struct platform_device *pdev)
 	clocksource_mmio_init(timer_of_base(&to) + GPT_CNT_REG(TIMER_CLK_SRC),
 			      node->name, timer_of_rate(&to), 300, 32,
 			      clocksource_mmio_readl_up);
-	gpt_sched_reg = timer_of_base(&to) + GPT_CNT_REG(TIMER_CLK_SRC);
-	sched_clock_register(mtk_gpt_read_sched_clock, 32, timer_of_rate(&to));
+
+	/*
+	 * sched_clock has no priorities; a counter with the same rate
+	 * replaces the current one. Only these SoCs lack the ARM architected
+	 * timer, which is otherwise the better sched_clock.
+	 */
+	if (of_machine_is_compatible("mediatek,mt6572") ||
+	    of_machine_is_compatible("mediatek,mt6580") ||
+	    of_machine_is_compatible("mediatek,mt6582") ||
+	    of_machine_is_compatible("mediatek,mt6592")) {
+		gpt_sched_reg = timer_of_base(&to) + GPT_CNT_REG(TIMER_CLK_SRC);
+		sched_clock_register(mtk_gpt_read_sched_clock, 32,
+				     timer_of_rate(&to));
+	}
 
 	/* Configure clock event */
 	mtk_gpt_setup(&to, TIMER_CLK_EVT, GPT_CTRL_OP_REPEAT);
-- 
2.56.0.455.g6c5a503b38-goog

Re: [PATCH] clocksource/drivers/timer-mediatek: Don't override the arch timer sched_clock

[Ryan Brue]

Hi Chen-Yu,

On 11/9/26 6:07 AM, Chen-Yu Tsai wrote:
> Before the conversion, both drivers used TIMER_OF_DECLARE and
> timer-mediatek.o is linked before arm_arch_timer.o, so the arch timer
> registered last and won.

On MT8173 the GPT already replaced the arch timer as sched_clock before 
7a097aba7255. Every boot log I have from 7.3 on the Fire HD 10 shows the 
same swap, for example on 7.3.0-rc5:

   [    0.000000] arch_timer: cp15 timer running at 13.00MHz (phys).
   [    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff 
max_cycles: 0x2ff89eacb, max_idle_ns: 440795202429 ns
   [    0.000001] sched_clock: 56 bits at 13MHz, resolution 76ns, wraps 
every 4398046511101ns
   [    0.000366] clocksource: timer: mask: 0xffffffff max_cycles: 
0xffffffff, max_idle_ns: 147020034397 ns
   [    0.000378] sched_clock: 32 bits at 13MHz, resolution 76ns, wraps 
every 165191050201ns

and on 7.3.0-rc6:

   [    0.000001] sched_clock: 56 bits at 13MHz, resolution 76ns, wraps 
every 4398046511101ns
   [    0.000192] clocksource: timer: mask: 0xffffffff max_cycles: 
0xffffffff, max_idle_ns: 147020034397 ns
   [    0.000201] sched_clock: 32 bits at 13MHz, resolution 76ns, wraps 
every 165191050201ns

Neither kernel had changes to drivers/clocksource or kernel/time iirc.

I think the reason is that timer_probe() walks the device tree nodes in 
order and matches each one against __timer_of_table, so link order only 
decides which entry a node matches, not which timer initializes first.

In mt8173.dtsi the arm,armv8-timer node comes before soc/timer@10008000, 
so the GPT registered second then too, and with the same rate it 
replaced the arch timer.

So what the conversion changed is when the swap happens, late enough to 
trigger the splat, rather than whether it happens.

The patch still LGTM, and it gives MT8173 the 56-bit arch counter back. 
It's just the history paragraph that I wanted to give my anecdote for.

On the Fire HD 10 (2017), v7.4-rc2 with this patch prints only 
"sched_clock: 56 bits at 13MHz", where the rc1 boot before it switched 
to the GPT's 32 bits. Boot, suspend/resume, and cpufreq are fine.

Tested-by: Ryan Brue <ryanbrue.dev@gmail.com> # on Amazon Fire HD 10 
(MT8173)

Thanks!
Ryan Brue