* [PATCH 0/2] Improve CLOCK_EVT_FEAT_C3STOP feature setting
@ 2022-07-19 5:47 Anup Patel
2022-07-19 5:47 ` [PATCH 1/2] dt-bindings: riscv: Add optional DT property riscv,timer-always-on Anup Patel
2022-07-19 5:47 ` [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property Anup Patel
0 siblings, 2 replies; 5+ messages in thread
From: Anup Patel @ 2022-07-19 5:47 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley,
Daniel Lezcano, Thomas Gleixner
Cc: Atish Patra, Samuel Holland, 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_v1 branch at:
https://github.com/avpatel/linux.git
Anup Patel (2):
dt-bindings: riscv: Add optional DT property riscv,timer-always-on
clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT
property
Documentation/devicetree/bindings/riscv/cpus.yaml | 6 ++++++
drivers/clocksource/timer-riscv.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletion(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] dt-bindings: riscv: Add optional DT property riscv,timer-always-on 2022-07-19 5:47 [PATCH 0/2] Improve CLOCK_EVT_FEAT_C3STOP feature setting Anup Patel @ 2022-07-19 5:47 ` Anup Patel 2022-07-19 5:47 ` [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property Anup Patel 1 sibling, 0 replies; 5+ messages in thread From: Anup Patel @ 2022-07-19 5:47 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner Cc: Atish Patra, Samuel Holland, Anup Patel, devicetree, linux-riscv, linux-kernel, Anup Patel We add an optional DT property riscv,timer-always-on which if present in CPU DT node then CPU timer is always powered-on and never loses context. Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- Documentation/devicetree/bindings/riscv/cpus.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml index d632ac76532e..33832b8dfaab 100644 --- a/Documentation/devicetree/bindings/riscv/cpus.yaml +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml @@ -78,6 +78,12 @@ properties: - rv64imac - rv64imafdc + riscv,timer-always-on: + type: boolean + description: + If present, the timer is powered through an always-on power + domain, therefore it never loses context. + # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here timebase-frequency: false -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property 2022-07-19 5:47 [PATCH 0/2] Improve CLOCK_EVT_FEAT_C3STOP feature setting Anup Patel 2022-07-19 5:47 ` [PATCH 1/2] dt-bindings: riscv: Add optional DT property riscv,timer-always-on Anup Patel @ 2022-07-19 5:47 ` Anup Patel 2022-07-19 6:42 ` Samuel Holland 1 sibling, 1 reply; 5+ messages in thread From: Anup Patel @ 2022-07-19 5:47 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner Cc: Atish Patra, Samuel Holland, 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-always-on DT property is not present for the corresponding CPU. 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 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 593d5a957b69..3015324f2b59 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -34,7 +34,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, }; @@ -65,9 +65,13 @@ static struct clocksource riscv_clocksource = { static int riscv_timer_starting_cpu(unsigned int cpu) { struct clock_event_device *ce = per_cpu_ptr(&riscv_clock_event, cpu); + struct device_node *np = of_get_cpu_node(cpu, NULL); ce->cpumask = cpumask_of(cpu); ce->irq = riscv_clock_event_irq; + if (!of_property_read_bool(np, "riscv,timer-always-on")) + ce->features |= CLOCK_EVT_FEAT_C3STOP; + of_node_put(np); clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); enable_percpu_irq(riscv_clock_event_irq, -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property 2022-07-19 5:47 ` [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property Anup Patel @ 2022-07-19 6:42 ` Samuel Holland 2022-07-21 9:45 ` Anup Patel 0 siblings, 1 reply; 5+ messages in thread From: Samuel Holland @ 2022-07-19 6:42 UTC (permalink / raw) To: Anup Patel, Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner Cc: Rob Herring, Krzysztof Kozlowski, Atish Patra, Anup Patel, devicetree, linux-riscv, linux-kernel Hi Anup, On 7/19/22 12:47 AM, Anup Patel wrote: > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only when > riscv,timer-always-on DT property is not present for the corresponding > CPU. The timer maintaining its context (and continuing to count) during non-retentive CPU suspend is not sufficient to drop CLOCK_EVT_FEAT_C3STOP. Another requirement is that the timer interrupt is generated and routed outside the CPU's power/reset domain, to whatever hardware is responsible for turning the CPU back on. It does not matter if the timer interrupt fires, if that interrupt cannot wake up the CPU. So something closer to "riscv,timer-can-wake-cpu" would be a more accurate property name for how you are using it. And even then, that ability is a property of the SBI implementation, not just the hardware. In the motivating example for the flag (Allwinner D1), the CLINT cannot wake the CPU from reset, but the SoC contains other MMIO timers that can. So the capability of the SBI timer extension depends on which timer hardware the SBI implementation chooses to use. So I am not sure that the property belongs in the CPU node. Maybe it makes sense to report this capability via a function in the SBI timer extension? Regards, Samuel > 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 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 593d5a957b69..3015324f2b59 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -34,7 +34,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, > }; > @@ -65,9 +65,13 @@ static struct clocksource riscv_clocksource = { > static int riscv_timer_starting_cpu(unsigned int cpu) > { > struct clock_event_device *ce = per_cpu_ptr(&riscv_clock_event, cpu); > + struct device_node *np = of_get_cpu_node(cpu, NULL); > > ce->cpumask = cpumask_of(cpu); > ce->irq = riscv_clock_event_irq; > + if (!of_property_read_bool(np, "riscv,timer-always-on")) > + ce->features |= CLOCK_EVT_FEAT_C3STOP; > + of_node_put(np); > clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); > > enable_percpu_irq(riscv_clock_event_irq, > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property 2022-07-19 6:42 ` Samuel Holland @ 2022-07-21 9:45 ` Anup Patel 0 siblings, 0 replies; 5+ messages in thread From: Anup Patel @ 2022-07-21 9:45 UTC (permalink / raw) To: Samuel Holland Cc: Palmer Dabbelt, Paul Walmsley, Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Atish Patra, Anup Patel, devicetree, linux-riscv, linux-kernel On Tue, Jul 19, 2022 at 12:12 PM Samuel Holland <samuel@sholland.org> wrote: > > Hi Anup, > > On 7/19/22 12:47 AM, Anup Patel wrote: > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only when > > riscv,timer-always-on DT property is not present for the corresponding > > CPU. > > The timer maintaining its context (and continuing to count) during non-retentive > CPU suspend is not sufficient to drop CLOCK_EVT_FEAT_C3STOP. > > Another requirement is that the timer interrupt is generated and routed outside > the CPU's power/reset domain, to whatever hardware is responsible for turning > the CPU back on. It does not matter if the timer interrupt fires, if that > interrupt cannot wake up the CPU. > > So something closer to "riscv,timer-can-wake-cpu" would be a more accurate > property name for how you are using it. I agree with your suggestion. Let's make this DT property name aligned with what it is doing. > > And even then, that ability is a property of the SBI implementation, not just > the hardware. In the motivating example for the flag (Allwinner D1), the CLINT > cannot wake the CPU from reset, but the SoC contains other MMIO timers that can. > So the capability of the SBI timer extension depends on which timer hardware the > SBI implementation chooses to use. So I am not sure that the property belongs in > the CPU node. > > Maybe it makes sense to report this capability via a function in the SBI timer > extension? Well, the timer interrupt not firing in non-retentive suspend is an attribute of underlying platform and not of the SBI implementation (firmware/hypervisor) hence should be described in DT or ACPI. Also, the timer interrupt not firing in non-retentive suspend is also possible with RISC-V Sstc extension (i.e stimecmp CSRs) so the proposed DT property will be useful for platforms with Sstc extension as well. Regards, Anup > > Regards, > Samuel > > > 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 | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > index 593d5a957b69..3015324f2b59 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -34,7 +34,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, > > }; > > @@ -65,9 +65,13 @@ static struct clocksource riscv_clocksource = { > > static int riscv_timer_starting_cpu(unsigned int cpu) > > { > > struct clock_event_device *ce = per_cpu_ptr(&riscv_clock_event, cpu); > > + struct device_node *np = of_get_cpu_node(cpu, NULL); > > > > ce->cpumask = cpumask_of(cpu); > > ce->irq = riscv_clock_event_irq; > > + if (!of_property_read_bool(np, "riscv,timer-always-on")) > > + ce->features |= CLOCK_EVT_FEAT_C3STOP; > > + of_node_put(np); > > clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); > > > > enable_percpu_irq(riscv_clock_event_irq, > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-21 9:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-19 5:47 [PATCH 0/2] Improve CLOCK_EVT_FEAT_C3STOP feature setting Anup Patel 2022-07-19 5:47 ` [PATCH 1/2] dt-bindings: riscv: Add optional DT property riscv,timer-always-on Anup Patel 2022-07-19 5:47 ` [PATCH 2/2] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT property Anup Patel 2022-07-19 6:42 ` Samuel Holland 2022-07-21 9:45 ` Anup Patel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox