[PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

[PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

[Anup Patel]

This series improves the RISC-V timer driver to set CLOCK_EVT_FEAT_C3STOP
feature based on RISC-V platform capabilities.

These patches can also be found in riscv_timer_dt_imp_v6 branch at:
https://github.com/avpatel/linux.git

Changes since v5:
 - Rebased on Linux-6.2-rc2

Changes since v4:
 - Update commit text of PATCH1 based on Samuel's comments
 - Renamed DT property "riscv,timer-can-wake-cpu" to
   "riscv,timer-cannot-wake-cpu" in PATCH2 and PATCH3
 - Updated description of DT property "riscv,timer-cannot-wake-cpu"
   in PATCH2

Changes since v3:
 - Rebased on Linux-6.1-rc7
 - Replaced PATCH1 with a patch to initialize broadcast timer

Changes since v2:
 - Include Conor's revert patch as the first patch and rebased other patches
 - Update PATCH2 to document bindings for separate RISC-V timer DT node
 - Update PATCH3 based on RISC-V timer DT node bindings

Changes since v1:
 - Rebased on Linux-5.19-rc8
 - Renamed "riscv,always-on" DT property to "riscv,timer-can-wake-cpu"

Anup Patel (2):
  dt-bindings: timer: Add bindings for the RISC-V timer device
  clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT

Conor Dooley (1):
  RISC-V: time: initialize hrtimer based broadcast clock event device

 .../bindings/timer/riscv,timer.yaml           | 52 +++++++++++++++++++
 arch/riscv/kernel/time.c                      |  3 ++
 drivers/clocksource/timer-riscv.c             | 10 ++++
 3 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml

-- 
2.34.1

[PATCH v6 1/3] RISC-V: time: initialize hrtimer based broadcast clock event device

[Anup Patel]

From: Conor Dooley <conor.dooley@microchip.com>

Similarly to commit 022eb8ae8b5e ("ARM: 8938/1: kernel: initialize
broadcast hrtimer based clock event device"), RISC-V needs to initiate
hrtimer based broadcast clock event device before C3STOP can be used.
Otherwise, the introduction of C3STOP for the RISC-V arch timer in
commit 232ccac1bd9b ("clocksource/drivers/riscv: Events are stopped
during CPU suspend") leaves us without any broadcast timer registered.
This prevents the kernel from entering oneshot mode, which breaks timer
behaviour, for example clock_nanosleep().

A test app that sleeps each cpu for 6, 5, 4, 3 ms respectively, HZ=250
& C3STOP enabled, the sleep times are rounded up to the next jiffy:
== CPU: 1 ==      == CPU: 2 ==      == CPU: 3 ==      == CPU: 4 ==
Mean: 7.974992    Mean: 7.976534    Mean: 7.962591    Mean: 3.952179
Std Dev: 0.154374 Std Dev: 0.156082 Std Dev: 0.171018 Std Dev: 0.076193
Hi: 9.472000      Hi: 10.495000     Hi: 8.864000      Hi: 4.736000
Lo: 6.087000      Lo: 6.380000      Lo: 4.872000      Lo: 3.403000
Samples: 521      Samples: 521      Samples: 521      Samples: 521

Link: https://lore.kernel.org/linux-riscv/YzYTNQRxLr7Q9JR0@spud/
Fixes: 232ccac1bd9b ("clocksource/drivers/riscv: Events are stopped during CPU suspend")
Suggested-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel@sholland.org>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 arch/riscv/kernel/time.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
index 8217b0f67c6c..1cf21db4fcc7 100644
--- a/arch/riscv/kernel/time.c
+++ b/arch/riscv/kernel/time.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/of_clk.h>
+#include <linux/clockchips.h>
 #include <linux/clocksource.h>
 #include <linux/delay.h>
 #include <asm/sbi.h>
@@ -29,6 +30,8 @@ void __init time_init(void)
 
 	of_clk_init(NULL);
 	timer_probe();
+
+	tick_setup_hrtimer_broadcast();
 }
 
 void clocksource_arch_init(struct clocksource *cs)
-- 
2.34.1

[PATCH v6 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device

[Anup Patel]

We add DT bindings for a separate RISC-V timer DT node which can
be used to describe implementation specific behaviour (such as
timer interrupt not triggered during non-retentive suspend).

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 .../bindings/timer/riscv,timer.yaml           | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/riscv,timer.yaml b/Documentation/devicetree/bindings/timer/riscv,timer.yaml
new file mode 100644
index 000000000000..38d67e1a5a79
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/riscv,timer.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/riscv,timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V timer
+
+maintainers:
+  - Anup Patel <anup@brainfault.org>
+
+description: |+
+  RISC-V platforms always have a RISC-V timer device for the supervisor-mode
+  based on the time CSR defined by the RISC-V privileged specification. The
+  timer interrupts of this device are configured using the RISC-V SBI Time
+  extension or the RISC-V Sstc extension.
+
+  The clock frequency of RISC-V timer device is specified via the
+  "timebase-frequency" DT property of "/cpus" DT node which is described
+  in Documentation/devicetree/bindings/riscv/cpus.yaml
+
+properties:
+  compatible:
+    enum:
+      - riscv,timer
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 4096   # Should be enough?
+
+  riscv,timer-cannot-wake-cpu:
+    type: boolean
+    description:
+      If present, the timer interrupt cannot wake up the CPU from one or
+      more suspend/idle states.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - interrupts-extended
+
+examples:
+  - |
+    timer {
+      compatible = "riscv,timer";
+      interrupts-extended = <&cpu1intc 5>,
+                            <&cpu2intc 5>,
+                            <&cpu3intc 5>,
+                            <&cpu4intc 5>;
+    };
+...
-- 
2.34.1

[PATCH v6 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT

[Anup Patel]

We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
when riscv,timer-cannot-wake-cpu DT property is present in the RISC-V
timer DT node.

This way CLOCK_EVT_FEAT_C3STOP feature is set for clock_event_device
based on RISC-V platform capabilities rather than having it set for
all RISC-V platforms.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/clocksource/timer-riscv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index a0d66fabf073..1b4b36df5484 100644
--- a/drivers/clocksource/timer-riscv.c
+++ b/drivers/clocksource/timer-riscv.c
@@ -28,6 +28,7 @@
 #include <asm/timex.h>
 
 static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available);
+static bool riscv_timer_cannot_wake_cpu;
 
 static int riscv_clock_next_event(unsigned long delta,
 		struct clock_event_device *ce)
@@ -85,6 +86,8 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
 
 	ce->cpumask = cpumask_of(cpu);
 	ce->irq = riscv_clock_event_irq;
+	if (riscv_timer_cannot_wake_cpu)
+		ce->features |= CLOCK_EVT_FEAT_C3STOP;
 	clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff);
 
 	enable_percpu_irq(riscv_clock_event_irq,
@@ -139,6 +142,13 @@ static int __init riscv_timer_init_dt(struct device_node *n)
 	if (cpuid != smp_processor_id())
 		return 0;
 
+	child = of_find_compatible_node(NULL, NULL, "riscv,timer");
+	if (child) {
+		riscv_timer_cannot_wake_cpu = of_property_read_bool(child,
+					"riscv,timer-cannot-wake-cpu");
+		of_node_put(child);
+	}
+
 	domain = NULL;
 	child = of_get_compatible_child(n, "riscv,cpu-intc");
 	if (!child) {
-- 
2.34.1

Re: [PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

[Daniel Lezcano]

Hi Anup,

shall I pick the entire series or just the bindings and the driver changes ?


On 03/01/2023 15:10, Anup Patel wrote:
> This series improves the RISC-V timer driver to set CLOCK_EVT_FEAT_C3STOP
> feature based on RISC-V platform capabilities.
> 
> These patches can also be found in riscv_timer_dt_imp_v6 branch at:
> https://github.com/avpatel/linux.git
> 
> Changes since v5:
>   - Rebased on Linux-6.2-rc2
> 
> Changes since v4:
>   - Update commit text of PATCH1 based on Samuel's comments
>   - Renamed DT property "riscv,timer-can-wake-cpu" to
>     "riscv,timer-cannot-wake-cpu" in PATCH2 and PATCH3
>   - Updated description of DT property "riscv,timer-cannot-wake-cpu"
>     in PATCH2
> 
> Changes since v3:
>   - Rebased on Linux-6.1-rc7
>   - Replaced PATCH1 with a patch to initialize broadcast timer
> 
> Changes since v2:
>   - Include Conor's revert patch as the first patch and rebased other patches
>   - Update PATCH2 to document bindings for separate RISC-V timer DT node
>   - Update PATCH3 based on RISC-V timer DT node bindings
> 
> Changes since v1:
>   - Rebased on Linux-5.19-rc8
>   - Renamed "riscv,always-on" DT property to "riscv,timer-can-wake-cpu"
> 
> Anup Patel (2):
>    dt-bindings: timer: Add bindings for the RISC-V timer device
>    clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
> 
> Conor Dooley (1):
>    RISC-V: time: initialize hrtimer based broadcast clock event device
> 
>   .../bindings/timer/riscv,timer.yaml           | 52 +++++++++++++++++++
>   arch/riscv/kernel/time.c                      |  3 ++
>   drivers/clocksource/timer-riscv.c             | 10 ++++
>   3 files changed, 65 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
> 

-- 
<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

Re: [PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

[Anup Patel]

On Wed, Jan 4, 2023 at 6:32 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
>
> Hi Anup,
>
> shall I pick the entire series or just the bindings and the driver changes ?

Yes, that would be great. Palmer has already ACKed this series.

Thanks,
Anup

>
>
> On 03/01/2023 15:10, Anup Patel wrote:
> > This series improves the RISC-V timer driver to set CLOCK_EVT_FEAT_C3STOP
> > feature based on RISC-V platform capabilities.
> >
> > These patches can also be found in riscv_timer_dt_imp_v6 branch at:
> > https://github.com/avpatel/linux.git
> >
> > Changes since v5:
> >   - Rebased on Linux-6.2-rc2
> >
> > Changes since v4:
> >   - Update commit text of PATCH1 based on Samuel's comments
> >   - Renamed DT property "riscv,timer-can-wake-cpu" to
> >     "riscv,timer-cannot-wake-cpu" in PATCH2 and PATCH3
> >   - Updated description of DT property "riscv,timer-cannot-wake-cpu"
> >     in PATCH2
> >
> > Changes since v3:
> >   - Rebased on Linux-6.1-rc7
> >   - Replaced PATCH1 with a patch to initialize broadcast timer
> >
> > Changes since v2:
> >   - Include Conor's revert patch as the first patch and rebased other patches
> >   - Update PATCH2 to document bindings for separate RISC-V timer DT node
> >   - Update PATCH3 based on RISC-V timer DT node bindings
> >
> > Changes since v1:
> >   - Rebased on Linux-5.19-rc8
> >   - Renamed "riscv,always-on" DT property to "riscv,timer-can-wake-cpu"
> >
> > Anup Patel (2):
> >    dt-bindings: timer: Add bindings for the RISC-V timer device
> >    clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
> >
> > Conor Dooley (1):
> >    RISC-V: time: initialize hrtimer based broadcast clock event device
> >
> >   .../bindings/timer/riscv,timer.yaml           | 52 +++++++++++++++++++
> >   arch/riscv/kernel/time.c                      |  3 ++
> >   drivers/clocksource/timer-riscv.c             | 10 ++++
> >   3 files changed, 65 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
> >
>
> --
> <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
>

Re: [PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

[Daniel Lezcano]

On 03/01/2023 15:10, Anup Patel wrote:
> This series improves the RISC-V timer driver to set CLOCK_EVT_FEAT_C3STOP
> feature based on RISC-V platform capabilities.
> 
> These patches can also be found in riscv_timer_dt_imp_v6 branch at:
> https://github.com/avpatel/linux.git
> 
> Changes since v5:
>   - Rebased on Linux-6.2-rc2

Applied, thanks

-- 
<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