| Message ID | 20221129140313.886192-3-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve CLOCK_EVT_FEAT_C3STOP feature setting | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be fixes |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | warning | WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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
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>; + }; +...