[PATCH RESEND v7 0/3] riscv: Use GCR.U timer device as clocksource

[PATCH RESEND v7 0/3] riscv: Use GCR.U timer device as clocksource

[Aleksa Paunovic via B4 Relay]

This series adds bindings for the GCR.U timer device and corresponding
driver support. Accessing the memory mapped shadow of the mtime register
in the GCR.U region should be faster
than trapping to M mode each time the timer needs to be read.
The timer device does not implement any interrupts, therefore the
timer-riscv clockevent implementation should suffice.

We tested the patchset both on QEMU and the Boston board with the P8700 bitfile:
- Coremark and timer kselftests on QEMU emulating an 8 core CPU
- Coremark and timer kselftests on the Boston board with a single core CPU.

Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
Signed-off-by: Chao-ying Fu <cfu@mips.com>
---
Changes in v7:
- Replace the previous implementation with a new timer driver for mips,p8700-gcru
- Add a patch for 64bit reads for timer mmio
- Link to v6: https://lore.kernel.org/r/20250806-riscv-time-mmio-v6-0-2df0e8219998@htecgroup.com

Changes in v6:
- Rename mti,gcru to mips,p8700-gcru
- Link to v5: https://lore.kernel.org/r/20250711-riscv-time-mmio-v5-0-9ed1f825ad5e@htecgroup.com

Changes in v5:
- Fixed build issues on 32-bit RISC-V and sparse warnings
- Remove clint_time_val and clint.h, replace with riscv_time_val
- Depend on RISCV_TIMER in Kconfig

Changes in v4:
- Remove "select" from mti,gcru.yaml.
- Refactor the driver to use function pointers instead of static keys.

Previous versions:
v1: https://lore.kernel.org/lkml/20241227150056.191794-1-arikalo@gmail.com/#t
v2: https://lore.kernel.org/linux-riscv/20250409143816.15802-1-aleksa.paunovic@htecgroup.com/
v3: https://lore.kernel.org/linux-riscv/DU0PR09MB61968695A2A3146EE83B7708F6BA2@DU0PR09MB6196.eurprd09.prod.outlook.com/
v4: https://lore.kernel.org/r/20250514-riscv-time-mmio-v4-0-cb0cf2922d66@htecgroup.com
v5: https://lore.kernel.org/r/20250711-riscv-time-mmio-v5-0-9ed1f825ad5e@htecgroup.com
v6: https://lore.kernel.org/r/20250806-riscv-time-mmio-v6-0-2df0e8219998@htecgroup.com

---
Aleksa Paunovic (3):
      dt-bindings: timer: mips,p8700-gcru
      riscv: clocksource: Add readq options to clocksource mmio
      riscv: clocksource: Add p8700-gcru driver

 .../devicetree/bindings/timer/mips,p8700-gcru.yaml | 38 ++++++++++++++++++
 drivers/clocksource/Kconfig                        |  9 +++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/mmio.c                         | 14 +++++++
 drivers/clocksource/timer-p8700.c                  | 45 ++++++++++++++++++++++
 include/linux/clocksource.h                        |  4 ++
 6 files changed, 111 insertions(+)
---
base-commit: ac3fd01e4c1efce8f2c054cdeb2ddd2fc0fb150d
change-id: 20250424-riscv-time-mmio-5628e0fca8af

Best regards,
-- 
Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

[PATCH RESEND v7 1/3] dt-bindings: timer: mips,p8700-gcru

[Aleksa Paunovic via B4 Relay]

From: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

Add dt-bindings for the GCR.U memory mapped timer device for RISC-V
platforms. The GCR.U memory region contains shadow copies of the RISC-V
mtime register and the hrtime Global Configuration Register.

Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/timer/mips,p8700-gcru.yaml | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml b/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..3498255762cce6b3f491292d340d9639bb573e6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/mips,p8700-gcru.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GCR.U timer device for the MIPS P8700 platform
+
+maintainers:
+  - Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
+
+description:
+  The GCR.U memory region contains memory mapped shadow copies of
+  mtime and hrtime Global Configuration Registers,
+  which software can choose to make accessible from user mode.
+
+properties:
+  compatible:
+    const: mips,p8700-gcru
+
+  reg:
+    items:
+      - description: Read-only shadow copy of the RISC-V mtime register.
+      - description: Read-only shadow copy of the P8700 high resolution timer register.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    timer@1617f000 {
+        compatible = "mips,p8700-gcru";
+        reg = <0x1617f050 0x8>,
+              <0x1617f090 0x8>;
+    };

-- 
2.43.0

Re: [PATCH RESEND v7 1/3] dt-bindings: timer: mips,p8700-gcru

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The unit-address in the example node name does not match the first address in the 'reg' property.
--

commit 0fc6965163fbf81e3cf1b47f4dd36f18d5ee7201
Author: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

dt-bindings: timer: mips,p8700-gcru

This commit adds dt-bindings for the GCR.U memory mapped timer device for
RISC-V platforms. The GCR.U memory region contains shadow copies of the
mtime and hrtime registers.

> diff --git a/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml b/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml
> new file mode 100644
> index 0000000000000..3498255762cce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/mips,p8700-gcru.yaml

[ ... ]

> +examples:
> +  - |
> +    timer@1617f000 {

This isn't a bug, but does the unit-address in the node name need to match
the first address in the reg property?

> +        compatible = "mips,p8700-gcru";
> +        reg = <0x1617f050 0x8>,

The node is named timer@1617f000, but the first address specified in the
reg property is 0x1617f050. This mismatch might trigger a unit_address_vs_reg
warning when running make dt_binding_check.

> +              <0x1617f090 0x8>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-riscv-time-mmio-v7-0-f8925ffc226b@htecgroup.com?part=1

[PATCH RESEND v7 2/3] riscv: clocksource: Add readq options to clocksource mmio

[Aleksa Paunovic via B4 Relay]

From: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

Add read functions for 64-bit register size to the generic
mmio clocksource, covering both up and down counters.

Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
---
 drivers/clocksource/mmio.c  | 14 ++++++++++++++
 include/linux/clocksource.h |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
index 9de75153183124cc8997c6ab61d0c01d9b2637bc..6329d8ce2c0911b5c6de34346b5ca8de40b93099 100644
--- a/drivers/clocksource/mmio.c
+++ b/drivers/clocksource/mmio.c
@@ -17,6 +17,20 @@ static inline struct clocksource_mmio *to_mmio_clksrc(struct clocksource *c)
 	return container_of(c, struct clocksource_mmio, clksrc);
 }
 
+#if defined(readq_relaxed)
+
+u64 clocksource_mmio_readq_up(struct clocksource *c)
+{
+	return (u64)readq_relaxed(to_mmio_clksrc(c)->reg);
+}
+
+u64 clocksource_mmio_readq_down(struct clocksource *c)
+{
+	return ~(u64)readq_relaxed(to_mmio_clksrc(c)->reg) & c->mask;
+}
+
+#endif
+
 u64 clocksource_mmio_readl_up(struct clocksource *c)
 {
 	return (u64)readl_relaxed(to_mmio_clksrc(c)->reg);
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 65b7c41471c390463770c2da13694e58e83b84ea..df8ea45ec60a28e0276020cb95ab5328bec89879 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -276,6 +276,10 @@ static inline void clocksource_arch_init(struct clocksource *cs) { }
 
 extern int timekeeping_notify(struct clocksource *clock);
 
+#if defined(readq_relaxed)
+extern u64 clocksource_mmio_readq_up(struct clocksource *c);
+extern u64 clocksource_mmio_readq_down(struct clocksource *c);
+#endif
 extern u64 clocksource_mmio_readl_up(struct clocksource *);
 extern u64 clocksource_mmio_readl_down(struct clocksource *);
 extern u64 clocksource_mmio_readw_up(struct clocksource *);

-- 
2.43.0

Re: [PATCH RESEND v7 2/3] riscv: clocksource: Add readq options to clocksource mmio

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `readq_relaxed()` to read a live 64-bit clocksource counter is racy and unsafe on 32-bit systems, leading to torn reads and broken timekeeping.
--

commit 21048c9043069be78a033341f4f13739837c7833
Author: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

riscv: clocksource: Add readq options to clocksource mmio

This commit adds generic read functions for 64-bit memory-mapped
clocksources covering both up and down counters.

> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> index 9de7515318312..6329d8ce2c091 100644
> --- a/drivers/clocksource/mmio.c
> +++ b/drivers/clocksource/mmio.c
[ ... ]
> @@ -17,6 +17,20 @@ static inline struct clocksource_mmio *to_mmio_clksrc(struct clocksource *c)
>  	return container_of(c, struct clocksource_mmio, clksrc);
>  }
>  
> +#if defined(readq_relaxed)

Is it safe to use readq_relaxed() directly for a live 64-bit clocksource
counter on 32-bit systems?

readq_relaxed() is not guaranteed to be atomic on 32-bit platforms. For
example, 32-bit SuperH implements __raw_readq as *(volatile u64 *), which
results in two separate 32-bit memory accesses. Other 32-bit platforms
might define it via <linux/io-64-nonatomic-lo-hi.h>.

> +
> +u64 clocksource_mmio_readq_up(struct clocksource *c)
> +{
> +	return (u64)readq_relaxed(to_mmio_clksrc(c)->reg);

If the lower 32 bits of the live hardware counter roll over between the two
32-bit reads, will the resulting 64-bit value be severely corrupted (torn)?

To safely support 64-bit MMIO clocksources on 32-bit platforms, a software
retry loop is typically used. Otherwise, shouldn't the generic functions be
strictly guarded by #ifdef CONFIG_64BIT to prevent accidental usage on
32-bit systems, rather than just #if defined(readq_relaxed)?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-riscv-time-mmio-v7-0-f8925ffc226b@htecgroup.com?part=2

[PATCH RESEND v7 3/3] riscv: clocksource: Add p8700-gcru driver

[Aleksa Paunovic via B4 Relay]

From: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

Add a clocksource driver for the P8700 GCRU.

Initialization uses helper functions
provided by clocksource/mmio.c and timer-of.c.

Since the GCRU does not support any kind of interrupts,
the default RISC-V clockevent implementation should suffice.

Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
---
 drivers/clocksource/Kconfig       |  9 ++++++++
 drivers/clocksource/Makefile      |  1 +
 drivers/clocksource/timer-p8700.c | 45 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index ffcd23668763fe7707a4e917bf240caadbb09a8c..861e7b8c93376b345e3a488dabe435d06a42f357 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -672,6 +672,15 @@ config CLINT_TIMER
 	  This option enables the CLINT timer for RISC-V systems.  The CLINT
 	  driver is usually used for NoMMU RISC-V systems.
 
+config P8700_TIMER
+	bool "MIPS P8700 timer driver"
+	depends on GENERIC_SCHED_CLOCK && RISCV && RISCV_SBI
+	select CLKSRC_MMIO
+	select TIMER_PROBE
+	select TIMER_OF
+	help
+	  Enables support for MIPS P8700 timer driver.
+
 config CSKY_MP_TIMER
 	bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
 	depends on CSKY
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index ec4452ee958f1a814c708aeba6412bea61d24892..fae9a58d6c8663a7c857b9ab7fdae05782b3551c 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -95,3 +95,4 @@ obj-$(CONFIG_CLKSRC_LOONGSON1_PWM)	+= timer-loongson1-pwm.o
 obj-$(CONFIG_EP93XX_TIMER)		+= timer-ep93xx.o
 obj-$(CONFIG_RALINK_TIMER)		+= timer-ralink.o
 obj-$(CONFIG_NXP_STM_TIMER)		+= timer-nxp-stm.o
+obj-$(CONFIG_P8700_TIMER)		+= timer-p8700.o
diff --git a/drivers/clocksource/timer-p8700.c b/drivers/clocksource/timer-p8700.c
new file mode 100644
index 0000000000000000000000000000000000000000..220ed8efdfe5544a3f925ad43b8faf2e0565557b
--- /dev/null
+++ b/drivers/clocksource/timer-p8700.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 MIPS.
+ */
+
+#include <linux/sched_clock.h>
+#include <linux/delay.h>
+#include <linux/of_address.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/clocksource.h>
+
+#include "timer-of.h"
+
+static struct timer_of gcru_of = { .flags = TIMER_OF_BASE };
+static u64 __iomem *p8700_time_val __ro_after_init;
+
+static u64 notrace p8700_timer_sched_read(void)
+{
+	return (u64)readq_relaxed(p8700_time_val);
+}
+
+static int __init p8700_timer_init(struct device_node *node)
+{
+	int error = 0;
+
+	error = timer_of_init(node, &gcru_of);
+	if (error)
+		return error;
+
+	p8700_time_val = timer_of_base(&gcru_of);
+	/* Now init the mmio timer with the address we got from DT */
+	error = clocksource_mmio_init(p8700_time_val, "mips,p8700-gcru",
+				      riscv_timebase, 450, 64,
+				      clocksource_mmio_readq_up);
+	if (error)
+		return error;
+
+	/* Sched clock */
+	sched_clock_register(p8700_timer_sched_read, 64, riscv_timebase);
+
+	return error;
+}
+
+TIMER_OF_DECLARE(p8700_timer, "mips,p8700-gcru", p8700_timer_init);

-- 
2.43.0

Re: [PATCH RESEND v7 3/3] riscv: clocksource: Add p8700-gcru driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Resource leak in `p8700_timer_init()` on the error path if `clocksource_mmio_init()` fails.
- [Low] The Kconfig entry lacks `depends on 64BIT`, causing compilation failures on 32-bit RISC-V architectures.
--

commit 4f34b087c1b81aba548d943831bdb092b55bf784
Author: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

riscv: clocksource: Add p8700-gcru driver

This commit adds a clocksource driver for the MIPS P8700 GCRU. It uses
timer-of and clocksource_mmio helpers for initialization.

> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index ffcd23668763f..861e7b8c93376 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig

[ ... ]

> @@ -672,6 +672,15 @@ config CLINT_TIMER
>  	  This option enables the CLINT timer for RISC-V systems.  The CLINT
>  	  driver is usually used for NoMMU RISC-V systems.
>  
> +config P8700_TIMER
> +	bool "MIPS P8700 timer driver"
> +	depends on GENERIC_SCHED_CLOCK && RISCV && RISCV_SBI

Does this driver also need to depend on 64BIT?

The driver unconditionally uses readq_relaxed() and clocksource_mmio_readq_up.
On 32-bit RISC-V platforms, readq_relaxed() is not defined, which will cause
undeclared identifier errors during compilation.

If we try to fix the build by including linux/io-64-nonatomic-lo-hi.h, it
could introduce torn reads when reading the 64-bit hardware counter with two
non-atomic 32-bit reads, leading to massive sched_clock() time jumps.

> +	select CLKSRC_MMIO
> +	select TIMER_PROBE
> +	select TIMER_OF
> +	help
> +	  Enables support for MIPS P8700 timer driver.

[ ... ]

> diff --git a/drivers/clocksource/timer-p8700.c b/drivers/clocksource/timer-p8700.c
> new file mode 100644
> index 0000000000000..220ed8efdfe55
> --- /dev/null
> +++ b/drivers/clocksource/timer-p8700.c

[ ... ]

> +static int __init p8700_timer_init(struct device_node *node)
> +{
> +	int error = 0;
> +
> +	error = timer_of_init(node, &gcru_of);
> +	if (error)
> +		return error;
> +
> +	p8700_time_val = timer_of_base(&gcru_of);
> +	/* Now init the mmio timer with the address we got from DT */
> +	error = clocksource_mmio_init(p8700_time_val, "mips,p8700-gcru",
> +				      riscv_timebase, 450, 64,
> +				      clocksource_mmio_readq_up);
> +	if (error)
> +		return error;

If clocksource_mmio_init() fails, does this code leak the I/O memory mapped
by timer_of_init()?

It looks like timer_of_init() maps the memory via of_iomap(), but this error
path directly returns without calling timer_of_cleanup() to release those
mappings and other associated timer_of resources.

> +
> +	/* Sched clock */
> +	sched_clock_register(p8700_timer_sched_read, 64, riscv_timebase);
> +
> +	return error;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-riscv-time-mmio-v7-0-f8925ffc226b@htecgroup.com?part=3