* [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting
@ 2022-11-29 14:03 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, 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_v4 branch at:
https://github.com/avpatel/linux.git
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 broadcast hrtimer based clock event device
.../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++
arch/riscv/kernel/time.c | 3 ++
drivers/clocksource/timer-riscv.c | 12 ++++-
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting
@ 2022-11-29 14:03 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, 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_v4 branch at:
https://github.com/avpatel/linux.git
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 broadcast hrtimer based clock event device
.../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++
arch/riscv/kernel/time.c | 3 ++
drivers/clocksource/timer-riscv.c | 12 ++++-
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 1/3] RISC-V: time: initialize broadcast hrtimer based clock event device
2022-11-29 14:03 ` Anup Patel
@ 2022-11-29 14:03 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel
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
hrtimers 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")
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>
---
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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 1/3] RISC-V: time: initialize broadcast hrtimer based clock event device
@ 2022-11-29 14:03 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel
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
hrtimers 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")
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>
---
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
2022-11-29 14:03 ` Anup Patel
@ 2022-11-29 14:03 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, 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>
---
.../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..cf53dfff90bc
--- /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-cant-wake-cpu:
+ type: boolean
+ description:
+ If present, the timer interrupt can't wake up the CPU from
+ suspend/idle state.
+
+additionalProperties: false
+
+required:
+ - compatible
+ - interrupts-extended
+
+examples:
+ - |
+ timer {
+ compatible = "riscv,timer";
+ interrupts-extended = <&cpu1intc 5>,
+ <&cpu2intc 5>,
+ <&cpu3intc 5>,
+ <&cpu4intc 5>;
+ };
+...
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
@ 2022-11-29 14:03 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, 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>
---
.../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..cf53dfff90bc
--- /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-cant-wake-cpu:
+ type: boolean
+ description:
+ If present, the timer interrupt can't wake up the CPU from
+ suspend/idle state.
+
+additionalProperties: false
+
+required:
+ - compatible
+ - interrupts-extended
+
+examples:
+ - |
+ timer {
+ compatible = "riscv,timer";
+ interrupts-extended = <&cpu1intc 5>,
+ <&cpu2intc 5>,
+ <&cpu3intc 5>,
+ <&cpu4intc 5>;
+ };
+...
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 14:03 ` Anup Patel
@ 2022-11-29 14:03 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, Anup Patel
We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
when riscv,timer-cant-wake-up 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>
---
drivers/clocksource/timer-riscv.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
static int riscv_clock_next_event(unsigned long delta,
struct clock_event_device *ce)
@@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
static unsigned int riscv_clock_event_irq;
static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
.name = "riscv_timer_clockevent",
- .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
+ .features = CLOCK_EVT_FEAT_ONESHOT,
.rating = 100,
.set_next_event = riscv_clock_next_event,
};
@@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
+ "riscv,timer-cant-wake-cpu");
+ of_node_put(child);
+ }
+
domain = NULL;
child = of_get_compatible_child(n, "riscv,cpu-intc");
if (!child) {
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 14:03 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 14:03 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Samuel Holland, Conor Dooley,
Anup Patel, devicetree, linux-riscv, linux-kernel, Anup Patel
We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
when riscv,timer-cant-wake-up 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>
---
drivers/clocksource/timer-riscv.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
static int riscv_clock_next_event(unsigned long delta,
struct clock_event_device *ce)
@@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
static unsigned int riscv_clock_event_irq;
static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
.name = "riscv_timer_clockevent",
- .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
+ .features = CLOCK_EVT_FEAT_ONESHOT,
.rating = 100,
.set_next_event = riscv_clock_next_event,
};
@@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
+ "riscv,timer-cant-wake-cpu");
+ of_node_put(child);
+ }
+
domain = NULL;
child = of_get_compatible_child(n, "riscv,cpu-intc");
if (!child) {
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 14:03 ` Anup Patel
@ 2022-11-29 14:36 ` Conor Dooley
-1 siblings, 0 replies; 26+ messages in thread
From: Conor Dooley @ 2022-11-29 14:36 UTC (permalink / raw)
To: Anup Patel
Cc: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner, Andrew Jones, Atish Patra,
Samuel Holland, Anup Patel, devicetree, linux-riscv,
linux-kernel
On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> when riscv,timer-cant-wake-up 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>
I thought I had left an R-b on this one?
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Also, I think that we need to backport *something* that disables C3STOP
which is why I had suggested keeping the revert in place.
Patch 1 of this series only solves the timer issues but does not restore
sleep states to their prior behaviour, right?
Either this patch or the revert needs to go to stable IMO.
Thanks,
Conor.
> ---
> drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
>
> static int riscv_clock_next_event(unsigned long delta,
> struct clock_event_device *ce)
> @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> static unsigned int riscv_clock_event_irq;
> static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> .name = "riscv_timer_clockevent",
> - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> .rating = 100,
> .set_next_event = riscv_clock_next_event,
> };
> @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> + "riscv,timer-cant-wake-cpu");
> + of_node_put(child);
> + }
> +
> domain = NULL;
> child = of_get_compatible_child(n, "riscv,cpu-intc");
> if (!child) {
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 14:36 ` Conor Dooley
0 siblings, 0 replies; 26+ messages in thread
From: Conor Dooley @ 2022-11-29 14:36 UTC (permalink / raw)
To: Anup Patel
Cc: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner, Andrew Jones, Atish Patra,
Samuel Holland, Anup Patel, devicetree, linux-riscv,
linux-kernel
On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> when riscv,timer-cant-wake-up 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>
I thought I had left an R-b on this one?
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Also, I think that we need to backport *something* that disables C3STOP
which is why I had suggested keeping the revert in place.
Patch 1 of this series only solves the timer issues but does not restore
sleep states to their prior behaviour, right?
Either this patch or the revert needs to go to stable IMO.
Thanks,
Conor.
> ---
> drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
>
> static int riscv_clock_next_event(unsigned long delta,
> struct clock_event_device *ce)
> @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> static unsigned int riscv_clock_event_irq;
> static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> .name = "riscv_timer_clockevent",
> - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> .rating = 100,
> .set_next_event = riscv_clock_next_event,
> };
> @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> + "riscv,timer-cant-wake-cpu");
> + of_node_put(child);
> + }
> +
> domain = NULL;
> child = of_get_compatible_child(n, "riscv,cpu-intc");
> if (!child) {
> --
> 2.34.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 14:36 ` Conor Dooley
@ 2022-11-29 17:11 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 17:11 UTC (permalink / raw)
To: Conor Dooley
Cc: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner, Andrew Jones,
Atish Patra, Samuel Holland, devicetree, linux-riscv,
linux-kernel
On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > when riscv,timer-cant-wake-up 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>
>
> I thought I had left an R-b on this one?
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> Also, I think that we need to backport *something* that disables C3STOP
> which is why I had suggested keeping the revert in place.
> Patch 1 of this series only solves the timer issues but does not restore
> sleep states to their prior behaviour, right?
> Either this patch or the revert needs to go to stable IMO.
Since it works for you with the C3STOP set and broadcast timer enabled,
we can directly go with this patch. I am fine including the revert as well.
Regards,
Anup
>
> Thanks,
> Conor.
>
> > ---
> > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> >
> > static int riscv_clock_next_event(unsigned long delta,
> > struct clock_event_device *ce)
> > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > static unsigned int riscv_clock_event_irq;
> > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > .name = "riscv_timer_clockevent",
> > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > .rating = 100,
> > .set_next_event = riscv_clock_next_event,
> > };
> > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > + "riscv,timer-cant-wake-cpu");
> > + of_node_put(child);
> > + }
> > +
> > domain = NULL;
> > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > if (!child) {
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 17:11 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 17:11 UTC (permalink / raw)
To: Conor Dooley
Cc: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner, Andrew Jones,
Atish Patra, Samuel Holland, devicetree, linux-riscv,
linux-kernel
On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > when riscv,timer-cant-wake-up 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>
>
> I thought I had left an R-b on this one?
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> Also, I think that we need to backport *something* that disables C3STOP
> which is why I had suggested keeping the revert in place.
> Patch 1 of this series only solves the timer issues but does not restore
> sleep states to their prior behaviour, right?
> Either this patch or the revert needs to go to stable IMO.
Since it works for you with the C3STOP set and broadcast timer enabled,
we can directly go with this patch. I am fine including the revert as well.
Regards,
Anup
>
> Thanks,
> Conor.
>
> > ---
> > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> >
> > static int riscv_clock_next_event(unsigned long delta,
> > struct clock_event_device *ce)
> > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > static unsigned int riscv_clock_event_irq;
> > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > .name = "riscv_timer_clockevent",
> > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > .rating = 100,
> > .set_next_event = riscv_clock_next_event,
> > };
> > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > + "riscv,timer-cant-wake-cpu");
> > + of_node_put(child);
> > + }
> > +
> > domain = NULL;
> > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > if (!child) {
> > --
> > 2.34.1
> >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 17:11 ` Anup Patel
@ 2022-11-29 17:17 ` Conor Dooley
-1 siblings, 0 replies; 26+ messages in thread
From: Conor Dooley @ 2022-11-29 17:17 UTC (permalink / raw)
To: Anup Patel
Cc: Conor Dooley, Anup Patel, Rob Herring, Krzysztof Kozlowski,
Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner,
Andrew Jones, Atish Patra, Samuel Holland, devicetree,
linux-riscv, linux-kernel
On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
> On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > > when riscv,timer-cant-wake-up 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>
> >
> > I thought I had left an R-b on this one?
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Also, I think that we need to backport *something* that disables C3STOP
> > which is why I had suggested keeping the revert in place.
> > Patch 1 of this series only solves the timer issues but does not restore
> > sleep states to their prior behaviour, right?
> > Either this patch or the revert needs to go to stable IMO.
>
> Since it works for you with the C3STOP set and broadcast timer enabled,
> we can directly go with this patch. I am fine including the revert as well.
I don't mind which gets backported. To me, this one is preferable as it
is more "complete" but it is a bit on the new feature side of things,
no?
Whoever applies it can decide, and I'll backport the revert if they
decide that this patch is not stable material :)
Thanks again for helping sort this mess out, I see it helped with your
IPI series too!
Conor.
> > > ---
> > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> > >
> > > static int riscv_clock_next_event(unsigned long delta,
> > > struct clock_event_device *ce)
> > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > > static unsigned int riscv_clock_event_irq;
> > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > > .name = "riscv_timer_clockevent",
> > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > > .rating = 100,
> > > .set_next_event = riscv_clock_next_event,
> > > };
> > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > > + "riscv,timer-cant-wake-cpu");
> > > + of_node_put(child);
> > > + }
> > > +
> > > domain = NULL;
> > > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > > if (!child) {
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 17:17 ` Conor Dooley
0 siblings, 0 replies; 26+ messages in thread
From: Conor Dooley @ 2022-11-29 17:17 UTC (permalink / raw)
To: Anup Patel
Cc: Conor Dooley, Anup Patel, Rob Herring, Krzysztof Kozlowski,
Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner,
Andrew Jones, Atish Patra, Samuel Holland, devicetree,
linux-riscv, linux-kernel
On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
> On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > > when riscv,timer-cant-wake-up 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>
> >
> > I thought I had left an R-b on this one?
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Also, I think that we need to backport *something* that disables C3STOP
> > which is why I had suggested keeping the revert in place.
> > Patch 1 of this series only solves the timer issues but does not restore
> > sleep states to their prior behaviour, right?
> > Either this patch or the revert needs to go to stable IMO.
>
> Since it works for you with the C3STOP set and broadcast timer enabled,
> we can directly go with this patch. I am fine including the revert as well.
I don't mind which gets backported. To me, this one is preferable as it
is more "complete" but it is a bit on the new feature side of things,
no?
Whoever applies it can decide, and I'll backport the revert if they
decide that this patch is not stable material :)
Thanks again for helping sort this mess out, I see it helped with your
IPI series too!
Conor.
> > > ---
> > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> > >
> > > static int riscv_clock_next_event(unsigned long delta,
> > > struct clock_event_device *ce)
> > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > > static unsigned int riscv_clock_event_irq;
> > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > > .name = "riscv_timer_clockevent",
> > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > > .rating = 100,
> > > .set_next_event = riscv_clock_next_event,
> > > };
> > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > > + "riscv,timer-cant-wake-cpu");
> > > + of_node_put(child);
> > > + }
> > > +
> > > domain = NULL;
> > > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > > if (!child) {
> > > --
> > > 2.34.1
> > >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 17:17 ` Conor Dooley
@ 2022-11-29 17:22 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 17:22 UTC (permalink / raw)
To: Conor Dooley
Cc: Anup Patel, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner,
Andrew Jones, Atish Patra, Samuel Holland, devicetree,
linux-riscv, linux-kernel
On Tue, Nov 29, 2022 at 10:47 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
> > On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > > > when riscv,timer-cant-wake-up 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>
> > >
> > > I thought I had left an R-b on this one?
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > Also, I think that we need to backport *something* that disables C3STOP
> > > which is why I had suggested keeping the revert in place.
> > > Patch 1 of this series only solves the timer issues but does not restore
> > > sleep states to their prior behaviour, right?
> > > Either this patch or the revert needs to go to stable IMO.
> >
> > Since it works for you with the C3STOP set and broadcast timer enabled,
> > we can directly go with this patch. I am fine including the revert as well.
>
> I don't mind which gets backported. To me, this one is preferable as it
> is more "complete" but it is a bit on the new feature side of things,
> no?
>
> Whoever applies it can decide, and I'll backport the revert if they
> decide that this patch is not stable material :)
>
> Thanks again for helping sort this mess out, I see it helped with your
> IPI series too!
Yes, I was surprised to see that it helped the IPI series as well.
Thanks for your patch.
Regards,
Anup
>
> Conor.
>
> > > > ---
> > > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> > > >
> > > > static int riscv_clock_next_event(unsigned long delta,
> > > > struct clock_event_device *ce)
> > > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > > > static unsigned int riscv_clock_event_irq;
> > > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > > > .name = "riscv_timer_clockevent",
> > > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > > > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > > > .rating = 100,
> > > > .set_next_event = riscv_clock_next_event,
> > > > };
> > > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > > > + "riscv,timer-cant-wake-cpu");
> > > > + of_node_put(child);
> > > > + }
> > > > +
> > > > domain = NULL;
> > > > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > > > if (!child) {
> > > > --
> > > > 2.34.1
> > > >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 17:22 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-11-29 17:22 UTC (permalink / raw)
To: Conor Dooley
Cc: Anup Patel, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner,
Andrew Jones, Atish Patra, Samuel Holland, devicetree,
linux-riscv, linux-kernel
On Tue, Nov 29, 2022 at 10:47 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
> > On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
> > > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
> > > > when riscv,timer-cant-wake-up 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>
> > >
> > > I thought I had left an R-b on this one?
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > Also, I think that we need to backport *something* that disables C3STOP
> > > which is why I had suggested keeping the revert in place.
> > > Patch 1 of this series only solves the timer issues but does not restore
> > > sleep states to their prior behaviour, right?
> > > Either this patch or the revert needs to go to stable IMO.
> >
> > Since it works for you with the C3STOP set and broadcast timer enabled,
> > we can directly go with this patch. I am fine including the revert as well.
>
> I don't mind which gets backported. To me, this one is preferable as it
> is more "complete" but it is a bit on the new feature side of things,
> no?
>
> Whoever applies it can decide, and I'll backport the revert if they
> decide that this patch is not stable material :)
>
> Thanks again for helping sort this mess out, I see it helped with your
> IPI series too!
Yes, I was surprised to see that it helped the IPI series as well.
Thanks for your patch.
Regards,
Anup
>
> Conor.
>
> > > > ---
> > > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
> > > >
> > > > static int riscv_clock_next_event(unsigned long delta,
> > > > struct clock_event_device *ce)
> > > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
> > > > static unsigned int riscv_clock_event_irq;
> > > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
> > > > .name = "riscv_timer_clockevent",
> > > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
> > > > + .features = CLOCK_EVT_FEAT_ONESHOT,
> > > > .rating = 100,
> > > > .set_next_event = riscv_clock_next_event,
> > > > };
> > > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
> > > > + "riscv,timer-cant-wake-cpu");
> > > > + of_node_put(child);
> > > > + }
> > > > +
> > > > domain = NULL;
> > > > child = of_get_compatible_child(n, "riscv,cpu-intc");
> > > > if (!child) {
> > > > --
> > > > 2.34.1
> > > >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
2022-11-29 17:22 ` Anup Patel
@ 2022-11-29 18:43 ` Palmer Dabbelt
-1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2022-11-29 18:43 UTC (permalink / raw)
To: apatel
Cc: Conor Dooley, anup, Conor Dooley, robh+dt,
krzysztof.kozlowski+dt, Paul Walmsley, daniel.lezcano, tglx,
ajones, atishp, samuel, devicetree, linux-riscv, linux-kernel
On Tue, 29 Nov 2022 09:22:22 PST (-0800), apatel@ventanamicro.com wrote:
> On Tue, Nov 29, 2022 at 10:47 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
>> > On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>> > >
>> > > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
>> > > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
>> > > > when riscv,timer-cant-wake-up 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>
>> > >
>> > > I thought I had left an R-b on this one?
>> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> > >
>> > > Also, I think that we need to backport *something* that disables C3STOP
>> > > which is why I had suggested keeping the revert in place.
>> > > Patch 1 of this series only solves the timer issues but does not restore
>> > > sleep states to their prior behaviour, right?
>> > > Either this patch or the revert needs to go to stable IMO.
>> >
>> > Since it works for you with the C3STOP set and broadcast timer enabled,
>> > we can directly go with this patch. I am fine including the revert as well.
>>
>> I don't mind which gets backported. To me, this one is preferable as it
>> is more "complete" but it is a bit on the new feature side of things,
>> no?
>>
>> Whoever applies it can decide, and I'll backport the revert if they
>> decide that this patch is not stable material :)
IIRC the clock folks took the original C3 patch, so that's probably the
best way to take these as well?
>>
>> Thanks again for helping sort this mess out, I see it helped with your
>> IPI series too!
>
> Yes, I was surprised to see that it helped the IPI series as well.
> Thanks for your patch.
>
> Regards,
> Anup
>
>>
>> Conor.
>>
>> > > > ---
>> > > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
>> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
>> > > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
>> > > >
>> > > > static int riscv_clock_next_event(unsigned long delta,
>> > > > struct clock_event_device *ce)
>> > > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
>> > > > static unsigned int riscv_clock_event_irq;
>> > > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
>> > > > .name = "riscv_timer_clockevent",
>> > > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
>> > > > + .features = CLOCK_EVT_FEAT_ONESHOT,
>> > > > .rating = 100,
>> > > > .set_next_event = riscv_clock_next_event,
>> > > > };
>> > > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
>> > > > + "riscv,timer-cant-wake-cpu");
>> > > > + of_node_put(child);
>> > > > + }
>> > > > +
>> > > > domain = NULL;
>> > > > child = of_get_compatible_child(n, "riscv,cpu-intc");
>> > > > if (!child) {
>> > > > --
>> > > > 2.34.1
>> > > >
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
@ 2022-11-29 18:43 ` Palmer Dabbelt
0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2022-11-29 18:43 UTC (permalink / raw)
To: apatel
Cc: Conor Dooley, anup, Conor Dooley, robh+dt,
krzysztof.kozlowski+dt, Paul Walmsley, daniel.lezcano, tglx,
ajones, atishp, samuel, devicetree, linux-riscv, linux-kernel
On Tue, 29 Nov 2022 09:22:22 PST (-0800), apatel@ventanamicro.com wrote:
> On Tue, Nov 29, 2022 at 10:47 PM Conor Dooley <conor@kernel.org> wrote:
>>
>> On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote:
>> > On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>> > >
>> > > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote:
>> > > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only
>> > > > when riscv,timer-cant-wake-up 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>
>> > >
>> > > I thought I had left an R-b on this one?
>> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> > >
>> > > Also, I think that we need to backport *something* that disables C3STOP
>> > > which is why I had suggested keeping the revert in place.
>> > > Patch 1 of this series only solves the timer issues but does not restore
>> > > sleep states to their prior behaviour, right?
>> > > Either this patch or the revert needs to go to stable IMO.
>> >
>> > Since it works for you with the C3STOP set and broadcast timer enabled,
>> > we can directly go with this patch. I am fine including the revert as well.
>>
>> I don't mind which gets backported. To me, this one is preferable as it
>> is more "complete" but it is a bit on the new feature side of things,
>> no?
>>
>> Whoever applies it can decide, and I'll backport the revert if they
>> decide that this patch is not stable material :)
IIRC the clock folks took the original C3 patch, so that's probably the
best way to take these as well?
>>
>> Thanks again for helping sort this mess out, I see it helped with your
>> IPI series too!
>
> Yes, I was surprised to see that it helped the IPI series as well.
> Thanks for your patch.
>
> Regards,
> Anup
>
>>
>> Conor.
>>
>> > > > ---
>> > > > drivers/clocksource/timer-riscv.c | 12 +++++++++++-
>> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
>> > > > index 969a552da8d2..0c8bdd168a45 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_cant_wake_cpu;
>> > > >
>> > > > static int riscv_clock_next_event(unsigned long delta,
>> > > > struct clock_event_device *ce)
>> > > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta,
>> > > > static unsigned int riscv_clock_event_irq;
>> > > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
>> > > > .name = "riscv_timer_clockevent",
>> > > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP,
>> > > > + .features = CLOCK_EVT_FEAT_ONESHOT,
>> > > > .rating = 100,
>> > > > .set_next_event = riscv_clock_next_event,
>> > > > };
>> > > > @@ -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_cant_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_cant_wake_cpu = of_property_read_bool(child,
>> > > > + "riscv,timer-cant-wake-cpu");
>> > > > + of_node_put(child);
>> > > > + }
>> > > > +
>> > > > domain = NULL;
>> > > > child = of_get_compatible_child(n, "riscv,cpu-intc");
>> > > > if (!child) {
>> > > > --
>> > > > 2.34.1
>> > > >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting
2022-11-29 14:03 ` Anup Patel
@ 2022-11-29 18:43 ` Palmer Dabbelt
-1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2022-11-29 18:43 UTC (permalink / raw)
To: apatel
Cc: robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, daniel.lezcano,
tglx, ajones, atishp, samuel, Conor Dooley, anup, devicetree,
linux-riscv, linux-kernel, apatel
On Tue, 29 Nov 2022 06:03:10 PST (-0800), apatel@ventanamicro.com 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_v4 branch at:
> https://github.com/avpatel/linux.git
>
> 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 broadcast hrtimer based clock event device
>
> .../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++
> arch/riscv/kernel/time.c | 3 ++
> drivers/clocksource/timer-riscv.c | 12 ++++-
> 3 files changed, 66 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
IIRC the main issue here were the DT bindings, though, so I think we'll
need to make sure that's sorted out.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting
@ 2022-11-29 18:43 ` Palmer Dabbelt
0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2022-11-29 18:43 UTC (permalink / raw)
To: apatel
Cc: robh+dt, krzysztof.kozlowski+dt, Paul Walmsley, daniel.lezcano,
tglx, ajones, atishp, samuel, Conor Dooley, anup, devicetree,
linux-riscv, linux-kernel, apatel
On Tue, 29 Nov 2022 06:03:10 PST (-0800), apatel@ventanamicro.com 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_v4 branch at:
> https://github.com/avpatel/linux.git
>
> 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 broadcast hrtimer based clock event device
>
> .../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++
> arch/riscv/kernel/time.c | 3 ++
> drivers/clocksource/timer-riscv.c | 12 ++++-
> 3 files changed, 66 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
IIRC the main issue here were the DT bindings, though, so I think we'll
need to make sure that's sorted out.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 1/3] RISC-V: time: initialize broadcast hrtimer based clock event device
2022-11-29 14:03 ` Anup Patel
@ 2022-11-30 4:19 ` Samuel Holland
-1 siblings, 0 replies; 26+ messages in thread
From: Samuel Holland @ 2022-11-30 4:19 UTC (permalink / raw)
To: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Conor Dooley, Anup Patel, devicetree,
linux-riscv, linux-kernel
On 11/29/22 08:03, Anup Patel wrote:
> 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
> hrtimers before C3STOP can be used. Otherwise, the introduction of C3STOP
Specifically it is the hrtimer-based broadcast clockevent that we need
to initialize, not hrtimers as a whole.
> for the RISC-V arch timer in commit 232ccac1bd9b
> ("clocksource/drivers/riscv: Events are stopped during CPU suspend")
Maybe add some more details here:
... 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>
Either way:
Reviewed-by: Samuel Holland <samuel@sholland.org>
> ---
> 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)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 1/3] RISC-V: time: initialize broadcast hrtimer based clock event device
@ 2022-11-30 4:19 ` Samuel Holland
0 siblings, 0 replies; 26+ messages in thread
From: Samuel Holland @ 2022-11-30 4:19 UTC (permalink / raw)
To: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Conor Dooley, Anup Patel, devicetree,
linux-riscv, linux-kernel
On 11/29/22 08:03, Anup Patel wrote:
> 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
> hrtimers before C3STOP can be used. Otherwise, the introduction of C3STOP
Specifically it is the hrtimer-based broadcast clockevent that we need
to initialize, not hrtimers as a whole.
> for the RISC-V arch timer in commit 232ccac1bd9b
> ("clocksource/drivers/riscv: Events are stopped during CPU suspend")
Maybe add some more details here:
... 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>
Either way:
Reviewed-by: Samuel Holland <samuel@sholland.org>
> ---
> 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)
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
2022-11-29 14:03 ` Anup Patel
@ 2022-11-30 4:45 ` Samuel Holland
-1 siblings, 0 replies; 26+ messages in thread
From: Samuel Holland @ 2022-11-30 4:45 UTC (permalink / raw)
To: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Conor Dooley, Anup Patel, devicetree,
linux-riscv, linux-kernel
On 11/29/22 08:03, Anup Patel wrote:
> 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>
> ---
> .../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..cf53dfff90bc
> --- /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-cant-wake-cpu:
I don't want to derail getting this merged, but if you do end up sending
another version, could you please spell out the word "cannot" here and
in the code? The missing apostrophe makes this jarring (and an entirely
different word).
> + type: boolean
> + description:
> + If present, the timer interrupt can't wake up the CPU from
> + suspend/idle state.
And in that case I would also suggest clarifying this as "one or more
suspend/idle states", since the limitation does not apply to all idle
states. At least it should never apply to the architectural WFI state;
for the SBI idle state binding, it only applies to those with the
"local-timer-stop" property.
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - interrupts-extended
> +
> +examples:
> + - |
> + timer {
> + compatible = "riscv,timer";
> + interrupts-extended = <&cpu1intc 5>,
> + <&cpu2intc 5>,
> + <&cpu3intc 5>,
> + <&cpu4intc 5>;
The CLINT and PLIC bindings also include the M-mode interrupts. Should
we do the same here?
Regards,
Samuel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
@ 2022-11-30 4:45 ` Samuel Holland
0 siblings, 0 replies; 26+ messages in thread
From: Samuel Holland @ 2022-11-30 4:45 UTC (permalink / raw)
To: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner
Cc: Andrew Jones, Atish Patra, Conor Dooley, Anup Patel, devicetree,
linux-riscv, linux-kernel
On 11/29/22 08:03, Anup Patel wrote:
> 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>
> ---
> .../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..cf53dfff90bc
> --- /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-cant-wake-cpu:
I don't want to derail getting this merged, but if you do end up sending
another version, could you please spell out the word "cannot" here and
in the code? The missing apostrophe makes this jarring (and an entirely
different word).
> + type: boolean
> + description:
> + If present, the timer interrupt can't wake up the CPU from
> + suspend/idle state.
And in that case I would also suggest clarifying this as "one or more
suspend/idle states", since the limitation does not apply to all idle
states. At least it should never apply to the architectural WFI state;
for the SBI idle state binding, it only applies to those with the
"local-timer-stop" property.
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - interrupts-extended
> +
> +examples:
> + - |
> + timer {
> + compatible = "riscv,timer";
> + interrupts-extended = <&cpu1intc 5>,
> + <&cpu2intc 5>,
> + <&cpu3intc 5>,
> + <&cpu4intc 5>;
The CLINT and PLIC bindings also include the M-mode interrupts. Should
we do the same here?
Regards,
Samuel
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
2022-11-30 4:45 ` Samuel Holland
@ 2022-12-01 5:56 ` Anup Patel
-1 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-12-01 5:56 UTC (permalink / raw)
To: Samuel Holland
Cc: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner, Andrew Jones,
Atish Patra, Conor Dooley, devicetree, linux-riscv, linux-kernel
On Wed, Nov 30, 2022 at 10:15 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 11/29/22 08:03, Anup Patel wrote:
> > 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>
> > ---
> > .../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..cf53dfff90bc
> > --- /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-cant-wake-cpu:
>
> I don't want to derail getting this merged, but if you do end up sending
> another version, could you please spell out the word "cannot" here and
> in the code? The missing apostrophe makes this jarring (and an entirely
> different word).
Okay, I will update.
>
> > + type: boolean
> > + description:
> > + If present, the timer interrupt can't wake up the CPU from
> > + suspend/idle state.
>
> And in that case I would also suggest clarifying this as "one or more
> suspend/idle states", since the limitation does not apply to all idle
> states. At least it should never apply to the architectural WFI state;
> for the SBI idle state binding, it only applies to those with the
> "local-timer-stop" property.
Okay, I will update.
>
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - interrupts-extended
> > +
> > +examples:
> > + - |
> > + timer {
> > + compatible = "riscv,timer";
> > + interrupts-extended = <&cpu1intc 5>,
> > + <&cpu2intc 5>,
> > + <&cpu3intc 5>,
> > + <&cpu4intc 5>;
>
> The CLINT and PLIC bindings also include the M-mode interrupts. Should
> we do the same here?
The RISC-V timer uses SBI time extension or RISC-V Sstc extension hence
it is only for S-mode software. In other words, the RISC-V timer is a S-mode
only timer.
The M-mode software is supposed to have its own platform specific MMIO
based timer.
Regards,
Anup
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device
@ 2022-12-01 5:56 ` Anup Patel
0 siblings, 0 replies; 26+ messages in thread
From: Anup Patel @ 2022-12-01 5:56 UTC (permalink / raw)
To: Samuel Holland
Cc: Anup Patel, Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt,
Paul Walmsley, Daniel Lezcano, Thomas Gleixner, Andrew Jones,
Atish Patra, Conor Dooley, devicetree, linux-riscv, linux-kernel
On Wed, Nov 30, 2022 at 10:15 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 11/29/22 08:03, Anup Patel wrote:
> > 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>
> > ---
> > .../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..cf53dfff90bc
> > --- /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-cant-wake-cpu:
>
> I don't want to derail getting this merged, but if you do end up sending
> another version, could you please spell out the word "cannot" here and
> in the code? The missing apostrophe makes this jarring (and an entirely
> different word).
Okay, I will update.
>
> > + type: boolean
> > + description:
> > + If present, the timer interrupt can't wake up the CPU from
> > + suspend/idle state.
>
> And in that case I would also suggest clarifying this as "one or more
> suspend/idle states", since the limitation does not apply to all idle
> states. At least it should never apply to the architectural WFI state;
> for the SBI idle state binding, it only applies to those with the
> "local-timer-stop" property.
Okay, I will update.
>
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - interrupts-extended
> > +
> > +examples:
> > + - |
> > + timer {
> > + compatible = "riscv,timer";
> > + interrupts-extended = <&cpu1intc 5>,
> > + <&cpu2intc 5>,
> > + <&cpu3intc 5>,
> > + <&cpu4intc 5>;
>
> The CLINT and PLIC bindings also include the M-mode interrupts. Should
> we do the same here?
The RISC-V timer uses SBI time extension or RISC-V Sstc extension hence
it is only for S-mode software. In other words, the RISC-V timer is a S-mode
only timer.
The M-mode software is supposed to have its own platform specific MMIO
based timer.
Regards,
Anup
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-12-01 5:57 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 14:03 [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting Anup Patel
2022-11-29 14:03 ` Anup Patel
2022-11-29 14:03 ` [PATCH v4 1/3] RISC-V: time: initialize broadcast hrtimer based clock event device Anup Patel
2022-11-29 14:03 ` Anup Patel
2022-11-30 4:19 ` Samuel Holland
2022-11-30 4:19 ` Samuel Holland
2022-11-29 14:03 ` [PATCH v4 2/3] dt-bindings: timer: Add bindings for the RISC-V timer device Anup Patel
2022-11-29 14:03 ` Anup Patel
2022-11-30 4:45 ` Samuel Holland
2022-11-30 4:45 ` Samuel Holland
2022-12-01 5:56 ` Anup Patel
2022-12-01 5:56 ` Anup Patel
2022-11-29 14:03 ` [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT Anup Patel
2022-11-29 14:03 ` Anup Patel
2022-11-29 14:36 ` Conor Dooley
2022-11-29 14:36 ` Conor Dooley
2022-11-29 17:11 ` Anup Patel
2022-11-29 17:11 ` Anup Patel
2022-11-29 17:17 ` Conor Dooley
2022-11-29 17:17 ` Conor Dooley
2022-11-29 17:22 ` Anup Patel
2022-11-29 17:22 ` Anup Patel
2022-11-29 18:43 ` Palmer Dabbelt
2022-11-29 18:43 ` Palmer Dabbelt
2022-11-29 18:43 ` [PATCH v4 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting Palmer Dabbelt
2022-11-29 18:43 ` Palmer Dabbelt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.