| Message ID | 20221125112105.427045-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 |
Hey Anup, For the future, could you please CC me on all patches in a series that I have previously reviewed? On Fri, Nov 25, 2022 at 04:51:04PM +0530, 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> > --- > .../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. I'm really not sure about this... I would be inclined to think that if someone does not specify then we should assume that they took the scroogiest view of the spec and so do not get events during suspend. I suppose you could then argue that their DT is wrong & it's their fault though. Plus the existing platforms behave this way & we avoid having to retrofit stuff here. > + > +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 Fri, Nov 25, 2022 at 6:40 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > Hey Anup, > > For the future, could you please CC me on all patches in a series that I > have previously reviewed? Okay. > > On Fri, Nov 25, 2022 at 04:51:04PM +0530, 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> > > --- > > .../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. > > I'm really not sure about this... I would be inclined to think that if > someone does not specify then we should assume that they took the > scroogiest view of the spec and so do not get events during suspend. > > I suppose you could then argue that their DT is wrong & it's their fault > though. Plus the existing platforms behave this way & we avoid having to > retrofit stuff here. Yes, the DT property is defined to keep things working for existing platforms. IMO, people should always read the DT bindings document at time of creating DT for their platform. If there are queries then they can always shoot email to the maintainers on LKML. > > > + > > +additionalProperties: false > > + > > +required: > > + - compatible > > + - interrupts-extended > > + > > +examples: > > + - | > > + timer { > > + compatible = "riscv,timer"; > > + interrupts-extended = <&cpu1intc 5>, > > + <&cpu2intc 5>, > > + <&cpu3intc 5>, > > + <&cpu4intc 5>; > > + }; > > +... > > -- > > 2.34.1 > > Regards, Anup
On Fri, Nov 25, 2022 at 07:18:48PM +0530, Anup Patel wrote: > On Fri, Nov 25, 2022 at 6:40 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > Hey Anup, > > > > For the future, could you please CC me on all patches in a series that I > > have previously reviewed? > > Okay. > > > > > On Fri, Nov 25, 2022 at 04:51:04PM +0530, 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> > > > --- > > > .../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. > > > > I'm really not sure about this... I would be inclined to think that if > > someone does not specify then we should assume that they took the > > scroogiest view of the spec and so do not get events during suspend. > > > > I suppose you could then argue that their DT is wrong & it's their fault > > though. Plus the existing platforms behave this way & we avoid having to > > retrofit stuff here. > > Yes, the DT property is defined to keep things working for > existing platforms. > > IMO, people should always read the DT bindings document at time of > creating DT for their platform. If there are queries then they can always > shoot email to the maintainers on LKML. Aye, I suppose so. For every platform that may exist that this change hurts, there's likely another one that it fixes a timer for... /shrug Binding itself looks grand though & we are in lessor of two evils territory, so: Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks for turning around a v3 promptly Anup! Conor. > > > + > > > +additionalProperties: false > > > + > > > +required: > > > + - compatible > > > + - interrupts-extended > > > + > > > +examples: > > > + - | > > > + timer { > > > + compatible = "riscv,timer"; > > > + interrupts-extended = <&cpu1intc 5>, > > > + <&cpu2intc 5>, > > > + <&cpu3intc 5>, > > > + <&cpu4intc 5>; > > > + }; > > > +... > > > -- > > > 2.34.1 > > > > > 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>; + }; +...
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> --- .../bindings/timer/riscv,timer.yaml | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/timer/riscv,timer.yaml