[PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered

Stephen Eta Zhou posted 1 patch 6 months, 4 weeks ago
There is a newer version of this series
drivers/clocksource/timer-sp804.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
[PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by Stephen Eta Zhou 6 months, 4 weeks ago
Register a valid read_current_timer() function for the
SP804 timer on ARM32.

On ARM32 platforms, when the SP804 timer is selected as the clocksource,
the driver does not register a valid read_current_timer() function.
As a result, features that rely on this API—such as rdseed—consistently
return incorrect values.

To fix this, a delay_timer structure is registered during the SP804
driver's initialization. The read_current_timer() function is implemented
using the existing sp804_read() logic, and the timer frequency is reused
from the already-initialized clocksource.

Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@gmail.com>
---
Changes in v4:
- Dropped redundant `delay.freq = rate;` assignment in `sp804_clocksource_and_sched_clock_init()`
- Dropped redundant `delay.read_current_timer` and `register_current_timer_delay()` lines in `sp804_of_init()`
- No functional changes to the driver logic; these lines were unnecessary as per Daniel's feedback.
- Link to v3: https://lore.kernel.org/all/20250414-sp804-fix-read_current_timer-v3-1-53b3e80d7183@gmail.com

Changes in v3:
- Updated the commit message for clarity and structure
- Link to v2: https://lore.kernel.org/all/BYAPR12MB3205D7A2BAA2712C89E03C4FD5D42@BYAPR12MB3205.namprd12.prod.outlook.com

Changes in v2:
- Added static keyword to struct delay_timer delay
- Integrate sp804_read_delay_timer_read and
  struct delay_timer delay together
- I moved the acquisition of delay.freq to
  sp804_clocksource_and_sched_clock_init.
  sp804_clocksource_and_sched_clock_init has already
  acquired and judged freq, so I can use it directly,
  and in this way I don’t need to consider whether to
  use clk1 or clk2, which can ensure that the clock source
  is available and reliable.
- Added detailed description information in Commit
- Link to v1: https://lore.kernel.org/all/BYAPR12MB3205C9C87EB560CA0CC4984BD5FB2@BYAPR12MB3205.namprd12.prod.outlook.com
---
 drivers/clocksource/timer-sp804.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c
index cd1916c0532507fb3ce7a11bfab4815906e326d5..e82a95ea472478ae096b2bf7abea0d65a7bca480 100644
--- a/drivers/clocksource/timer-sp804.c
+++ b/drivers/clocksource/timer-sp804.c
@@ -21,6 +21,10 @@
 #include <linux/of_irq.h>
 #include <linux/sched_clock.h>
 
+#ifdef CONFIG_ARM
+#include <linux/delay.h>
+#endif
+
 #include "timer-sp.h"
 
 /* Hisilicon 64-bit timer(a variant of ARM SP804) */
@@ -102,6 +106,23 @@ static u64 notrace sp804_read(void)
 	return ~readl_relaxed(sched_clkevt->value);
 }
 
+#ifdef CONFIG_ARM
+static struct delay_timer delay;
+static unsigned long sp804_read_delay_timer_read(void)
+{
+	return sp804_read();
+}
+
+static void sp804_register_delay_timer(int freq)
+{
+	delay.freq = freq;
+	delay.read_current_timer = sp804_read_delay_timer_read;
+	register_current_timer_delay(&delay);
+}
+#else
+static inline void sp804_register_delay_timer(int freq) {}
+#endif
+
 static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base,
 							 const char *name,
 							 struct clk *clk,
@@ -114,6 +135,8 @@ static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base,
 	if (rate < 0)
 		return -EINVAL;
 
+	sp804_register_delay_timer(rate);
+
 	clkevt = sp804_clkevt_get(base);
 
 	writel(0, clkevt->ctrl);
@@ -318,6 +341,7 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time
 		if (ret)
 			goto err;
 	}
+
 	initialized = true;
 
 	return 0;

---
base-commit: 7ee983c850b40043ac4751836fbd9a2b4d0c5937
change-id: 20250411-sp804-fix-read_current_timer-1c7c5a448c6c

Best regards,
-- 
Stephen Eta Zhou <stephen.eta.zhou@gmail.com>

Re: [PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by Guenter Roeck 3 days, 15 hours ago
On Sun, May 25, 2025 at 04:43:28PM +0800, Stephen Eta Zhou wrote:
> Register a valid read_current_timer() function for the
> SP804 timer on ARM32.
> 
> On ARM32 platforms, when the SP804 timer is selected as the clocksource,
> the driver does not register a valid read_current_timer() function.
> As a result, features that rely on this API—such as rdseed—consistently
> return incorrect values.
> 
> To fix this, a delay_timer structure is registered during the SP804
> driver's initialization. The read_current_timer() function is implemented
> using the existing sp804_read() logic, and the timer frequency is reused
> from the already-initialized clocksource.
> 
> Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@gmail.com>

This patch results in a crash when trying to boot integratorcp in qemu.

8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 0000000c when read
[0000000c] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.19.0-rc1 #1 PREEMPT
Hardware name: ARM Integrator/CP (Device Tree)
PC is at sp804_read_delay_timer_read+0x8/0x1c
LR is at read_current_timer+0x24/0x44
pc : [<c0af98d0>]    lr : [<c0cd9010>]    psr: a00001d3
sp : c1211fa0  ip : c0dfd818  fp : 00000000
r10: 000003e6  r9 : c10080f8  r8 : 00000000
r7 : c1217940  r6 : c6fffe60  r5 : c11ad510  r4 : c1211fb0
r3 : 00000000  r2 : 00000000  r1 : ffffffff  r0 : c1211fb0
Flags: NzCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
Control: 00093177  Table: 00004000  DAC: 00000053
Register r0 information: non-slab/vmalloc memory
Register r1 information: non-paged memory
Register r2 information: NULL pointer
Register r3 information: NULL pointer
Register r4 information: non-slab/vmalloc memory
Register r5 information: non-slab/vmalloc memory
Register r6 information: 0-page vmalloc region starting at 0xc7000000 allocated at iotable_init+0x0/0xc4
Register r7 information: non-slab/vmalloc memory
Register r8 information: NULL pointer
Register r9 information: non-slab/vmalloc memory
Register r10 information: non-paged memory
Register r11 information: NULL pointer
Register r12 information: non-slab/vmalloc memory
Process swapper/0 (pid: 0, stack limit = 0xc1210000)
Stack: (0xc1211fa0 to 0xc1212000)
1fa0: c1324000 c11d8708 00000000 c121794c c1324000 c11ad510 c6fffe60 c121794c
1fc0: 00000000 c1324000 c11ad510 c11adf28 ffffffff ffffffff 00000000 c11ad57c
1fe0: c11eca2c c121794c 00000000 00000113 00000000 00000000 00000000 00000000
Call trace:
 sp804_read_delay_timer_read from read_current_timer+0x24/0x44

This is not surprising since sp804_read_delay_timer_read() calls
sp804_read() which dereferences sched_clkevt. sched_clkevt is not
initialized for integratorcp.

static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base,
                                                         const char *name,
                                                         struct clk *clk,
                                                         int use_sched_clock)
{
...
	if (use_sched_clock) {
                sched_clkevt = clkevt;
                sched_clock_register(sp804_read, 32, rate);
        }
...
static int __init integrator_cp_of_init(struct device_node *np)
{
...
		ret = sp804_clocksource_and_sched_clock_init(base,
                                                             name, clk, 0);
							                ^
Guenter
RE: [PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by stephen.eta.zhou@gmail.com 3 days, 9 hours ago
Hi Guenter,

On Mon, 15 Dec 2025 15:18:37 -0800, Guenter Roeck wrote:
> This patch results in a crash when trying to boot integratorcp in qemu.

Thank you for pointing out the problem.

> sp804_read_delay_timer_read from read_current_timer+0x24/0x44
>
> This is not surprising since sp804_read_delay_timer_read() calls
> sp804_read() which dereferences sched_clkevt. sched_clkevt is not
> initialized for integratorcp.

Thank you for pointing out this issue.
I will work on fixing it and submit a new patch as soon as possible.

> static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base,
>                                                          const char *name,
>                                                          struct clk *clk,
>                                                          int use_sched_clock)
> {
> ...
> 	if (use_sched_clock) {
>                 sched_clkevt = clkevt;
>                 sched_clock_register(sp804_read, 32, rate);
>         }
> ...
> static int __init integrator_cp_of_init(struct device_node *np)
> {
> ...
> 		ret = sp804_clocksource_and_sched_clock_init(base,
>                                                              name, clk, 0);

Thanks.

Best regards,  
Stephen
Re: [PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by Guenter Roeck 3 days, 8 hours ago
On 12/15/25 21:38, stephen.eta.zhou@gmail.com wrote:
> Hi Guenter,
> 
> On Mon, 15 Dec 2025 15:18:37 -0800, Guenter Roeck wrote:
>> This patch results in a crash when trying to boot integratorcp in qemu.
> 
> Thank you for pointing out the problem.
> 
>> sp804_read_delay_timer_read from read_current_timer+0x24/0x44
>>
>> This is not surprising since sp804_read_delay_timer_read() calls
>> sp804_read() which dereferences sched_clkevt. sched_clkevt is not
>> initialized for integratorcp.
> 
> Thank you for pointing out this issue.
> I will work on fixing it and submit a new patch as soon as possible.
> 

The realview-pbx-a9 target it also affected. Bisect log below.

Guenter

---
# bad: [559e608c46553c107dbba19dae0854af7b219400] Merge tag 'ntfs3_for_6.19' of https://github.com/Paragon-Software-Group/linux-ntfs3
# good: [7d0a66e4bb9081d75c82ec4957c50034cb0ea449] Linux 6.18
git bisect start '559e608c4655' 'v6.18'
# bad: [015e7b0b0e8e51f7321ec2aafc1d7fc0a8a5536f] Merge tag 'bpf-next-6.19' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
git bisect bad 015e7b0b0e8e51f7321ec2aafc1d7fc0a8a5536f
# bad: [2547f79b0b0cd969ae6f736890af4ebd9368cda5] Merge tag 's390-6.19-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect bad 2547f79b0b0cd969ae6f736890af4ebd9368cda5
# good: [63e6995005be8ceb8a1d56a18df1a1a40c28356d] Merge tag 'objtool-core-2025-12-01' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 63e6995005be8ceb8a1d56a18df1a1a40c28356d
# good: [15b87bec89cb227b55b3689bf5de31b85cf88559] Merge tag 'irq-drivers-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 15b87bec89cb227b55b3689bf5de31b85cf88559
# bad: [54de197c9a5e8f522cb0a472e68e3e9888c91aa3] Merge tag 'x86_sgx_for_6.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 54de197c9a5e8f522cb0a472e68e3e9888c91aa3
# bad: [49219bba0149157774b7091c3ea9ad22b2114285] Merge tag 'edac_updates_for_v6.19_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras
git bisect bad 49219bba0149157774b7091c3ea9ad22b2114285
# bad: [d42e504a555d0da2a10001e697f0c8a7f633fb05] Merge tag 'timers-core-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad d42e504a555d0da2a10001e697f0c8a7f633fb05
# bad: [5028f42416eaec08d3f6aa4f98ccca669b3f8ab3] Merge tag 'timers-clocksource-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 5028f42416eaec08d3f6aa4f98ccca669b3f8ab3
# bad: [627f3f3716a3591f5e6a6bd124c95eef85444080] clocksource/drivers/rda: Add sched_clock_register for RDA8810PL SoC
git bisect bad 627f3f3716a3591f5e6a6bd124c95eef85444080
# bad: [62524f285c11d6e6168ad31b586143755b27b2e5] clocksource/drivers/sh_cmt: Always leave device running after probe
git bisect bad 62524f285c11d6e6168ad31b586143755b27b2e5
# bad: [640594a04f119338019b0aeed70c7301216595b3] clocksource/drivers/timer-sp804: Fix read_current_timer() issue when clock source is not registered
git bisect bad 640594a04f119338019b0aeed70c7301216595b3
# good: [576c564ec3bb60e571c705a71907d7c0c039e6c0] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
git bisect good 576c564ec3bb60e571c705a71907d7c0c039e6c0
# first bad commit: [640594a04f119338019b0aeed70c7301216595b3] clocksource/drivers/timer-sp804: Fix read_current_timer() issue when clock source is not registered
RE: [PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by stephen.eta.zhou@gmail.com 3 days, 8 hours ago
Hi Guenter,

On Mon, 15 Dec 2025 21:48:39 -0800, Guenter Roeck wrote:
> The realview-pbx-a9 target it also affected. Bisect log below.

Thanks so much for the feedback!
My plan is to fix the issue by properly initializing sched_clkevt
before calling sp804_read() in read_current_timer().
I'll submit the patch soon to address the problem on both the
integratorcp and realview-pbx-a9 platforms.

> # bad: [559e608c46553c107dbba19dae0854af7b219400] Merge tag 'ntfs3_for_6.19' of https://github.com/Paragon-Software-Group/linux-ntfs3
> # good: [7d0a66e4bb9081d75c82ec4957c50034cb0ea449] Linux 6.18
> git bisect start '559e608c4655' 'v6.18'
> # bad: [015e7b0b0e8e51f7321ec2aafc1d7fc0a8a5536f] Merge tag 'bpf-next-6.19' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
> git bisect bad 015e7b0b0e8e51f7321ec2aafc1d7fc0a8a5536f
> # bad: [2547f79b0b0cd969ae6f736890af4ebd9368cda5] Merge tag 's390-6.19-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> git bisect bad 2547f79b0b0cd969ae6f736890af4ebd9368cda5
> # good: [63e6995005be8ceb8a1d56a18df1a1a40c28356d] Merge tag 'objtool-core-2025-12-01' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect good 63e6995005be8ceb8a1d56a18df1a1a40c28356d
> # good: [15b87bec89cb227b55b3689bf5de31b85cf88559] Merge tag 'irq-drivers-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect good 15b87bec89cb227b55b3689bf5de31b85cf88559
> # bad: [54de197c9a5e8f522cb0a472e68e3e9888c91aa3] Merge tag 'x86_sgx_for_6.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect bad 54de197c9a5e8f522cb0a472e68e3e9888c91aa3
> # bad: [49219bba0149157774b7091c3ea9ad22b2114285] Merge tag 'edac_updates_for_v6.19_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras
> git bisect bad 49219bba0149157774b7091c3ea9ad22b2114285
> # bad: [d42e504a555d0da2a10001e697f0c8a7f633fb05] Merge tag 'timers-core-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect bad d42e504a555d0da2a10001e697f0c8a7f633fb05
> # bad: [5028f42416eaec08d3f6aa4f98ccca669b3f8ab3] Merge tag 'timers-clocksource-2025-11-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect bad 5028f42416eaec08d3f6aa4f98ccca669b3f8ab3
> # bad: [627f3f3716a3591f5e6a6bd124c95eef85444080] clocksource/drivers/rda: Add sched_clock_register for RDA8810PL SoC
> git bisect bad 627f3f3716a3591f5e6a6bd124c95eef85444080
> # bad: [62524f285c11d6e6168ad31b586143755b27b2e5] clocksource/drivers/sh_cmt: Always leave device running after probe
> git bisect bad 62524f285c11d6e6168ad31b586143755b27b2e5
> # bad: [640594a04f119338019b0aeed70c7301216595b3] clocksource/drivers/timer-sp804: Fix read_current_timer() issue when clock source is not registered
> git bisect bad 640594a04f119338019b0aeed70c7301216595b3
> # good: [576c564ec3bb60e571c705a71907d7c0c039e6c0] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
> git bisect good 576c564ec3bb60e571c705a71907d7c0c039e6c0
> # first bad commit: [640594a04f119338019b0aeed70c7301216595b3] clocksource/drivers/timer-sp804: Fix read_current_timer() issue when clock source is not registered

Thanks~

Best regards,  
Stephen
Re: [PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered
Posted by Daniel Lezcano 1 month, 2 weeks ago
Hi Stephen,

On 10/23/25 08:55, stephen.eta.zhou@gmail.com wrote:
> Hi,
> 
> I wanted to follow up on my `[PATCH v4] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered` patch,
> which I submitted on Sun, 25 May 2025.
> 
> I haven't received any feedback yet.
> 
> so just ping....
> 
> If any updates or modifications are required, please let me know.


Your patch looks correct, I picked it up

Thanks

   -- Daniel

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog