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

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

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