2023-01-03 14:38:05

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 0/3] Improve CLOCK_EVT_FEAT_C3STOP feature setting

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

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

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

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

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

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

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

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

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

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

--
2.34.1


2023-01-03 14:39:29

by Anup Patel

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

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 <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
---
.../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml

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

2023-01-03 14:41:15

by Anup Patel

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

From: Conor Dooley <[email protected]>

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

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

Link: https://lore.kernel.org/linux-riscv/YzYTNQRxLr7Q9JR0@spud/
Fixes: 232ccac1bd9b ("clocksource/drivers/riscv: Events are stopped during CPU suspend")
Suggested-by: Samuel Holland <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
---
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

2023-01-04 13:10:17

by Daniel Lezcano

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


Hi Anup,

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


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

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

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

2023-01-04 14:14:15

by Anup Patel

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

On Wed, Jan 4, 2023 at 6:32 PM Daniel Lezcano <[email protected]> wrote:
>
>
> Hi Anup,
>
> shall I pick the entire series or just the bindings and the driver changes ?

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

Thanks,
Anup

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

2023-01-04 23:01:21

by Daniel Lezcano

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

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

Applied, thanks

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

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

Subject: [tip: timers/core] dt-bindings: timer: Add bindings for the RISC-V timer device

The following commit has been merged into the timers/core branch of tip:

Commit-ID: e2bcf2d876fd7ca6ecca09794ac58d7e3a544794
Gitweb: https://git.kernel.org/tip/e2bcf2d876fd7ca6ecca09794ac58d7e3a544794
Author: Anup Patel <[email protected]>
AuthorDate: Tue, 03 Jan 2023 19:41:01 +05:30
Committer: Daniel Lezcano <[email protected]>
CommitterDate: Mon, 13 Feb 2023 13:10:16 +01:00

dt-bindings: timer: Add bindings for the RISC-V timer device

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 <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Daniel Lezcano <[email protected]>
---
Documentation/devicetree/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 0000000..38d67e1
--- /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 <[email protected]>
+
+description: |+
+ RISC-V platforms always have a RISC-V timer device for the supervisor-mode
+ based on the time CSR defined by the RISC-V privileged specification. The
+ timer interrupts of this device are configured using the RISC-V SBI Time
+ extension or the RISC-V Sstc extension.
+
+ The clock frequency of RISC-V timer device is specified via the
+ "timebase-frequency" DT property of "/cpus" DT node which is described
+ in Documentation/devicetree/bindings/riscv/cpus.yaml
+
+properties:
+ compatible:
+ enum:
+ - riscv,timer
+
+ interrupts-extended:
+ minItems: 1
+ maxItems: 4096 # Should be enough?
+
+ riscv,timer-cannot-wake-cpu:
+ type: boolean
+ description:
+ If present, the timer interrupt cannot wake up the CPU from one or
+ more suspend/idle states.
+
+additionalProperties: false
+
+required:
+ - compatible
+ - interrupts-extended
+
+examples:
+ - |
+ timer {
+ compatible = "riscv,timer";
+ interrupts-extended = <&cpu1intc 5>,
+ <&cpu2intc 5>,
+ <&cpu3intc 5>,
+ <&cpu4intc 5>;
+ };
+...

Subject: [tip: timers/core] RISC-V: time: initialize hrtimer based broadcast clock event device

The following commit has been merged into the timers/core branch of tip:

Commit-ID: 8b3b8fbb4896984b5564789a42240e4b3caddb61
Gitweb: https://git.kernel.org/tip/8b3b8fbb4896984b5564789a42240e4b3caddb61
Author: Conor Dooley <[email protected]>
AuthorDate: Tue, 03 Jan 2023 19:41:00 +05:30
Committer: Daniel Lezcano <[email protected]>
CommitterDate: Mon, 13 Feb 2023 13:10:16 +01:00

RISC-V: time: initialize hrtimer based broadcast clock event device

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

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

Link: https://lore.kernel.org/linux-riscv/YzYTNQRxLr7Q9JR0@spud/
Fixes: 232ccac1bd9b ("clocksource/drivers/riscv: Events are stopped during CPU suspend")
Suggested-by: Samuel Holland <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Daniel Lezcano <[email protected]>
---
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 8217b0f..1cf21db 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)