Skip to contentSkip to conversation

dt-bindings: riscv: Fix CPU schema errors

PATCH In review Rob Herring 2 participants 4 messages Oct 9, 2019 – Oct 10, 2019

Subsystems (2)
lore
Share & tools
lore

Rob Herring

author maintainer
#0 lore raw

PATCH v2 1/1 dt-bindings: riscv: Fix CPU schema errors

Fix the errors in the RiscV CPU DT schema:

Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@1: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible:0: 'riscv' is not one of ['sifive,rocket0', 'sifive,e5', 'sifive,e51', 'sifive,u54-mc', 'sifive,u54', 'sifive,u5'] Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible: ['riscv'] is too short Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property

The DT spec allows for 'timebase-frequency' to be in 'cpu' or 'cpus' node and RiscV is doing nothing special with it, so just drop the definition here and don't make it required.

Fixes 4fd669a8c487 ("dt-bindings: riscv: convert cpu binding to json-schema")
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
---
 .../devicetree/bindings/riscv/cpus.yaml       | 28 ++++++++-----------
 1 file changed, 11 insertions(+), 17 deletions(-)
Documentation/devicetree/bindings/riscv/cpus.yaml +11 -17
index b261a3015f84..925b531767bf 100644
@@ -24,15 +24,17 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - sifive,rocket0
-          - sifive,e5
-          - sifive,e51
-          - sifive,u54-mc
-          - sifive,u54
-          - sifive,u5
-      - const: riscv
+    oneOf:
+      - items:
+          - enum:
+              - sifive,rocket0
+              - sifive,e5
+              - sifive,e51
+              - sifive,u54-mc
+              - sifive,u54
+              - sifive,u5
+          - const: riscv
+      - const: riscv    # Simulator only
     description:
       Identifies that the hart uses the RISC-V instruction set
       and identifies the type of the hart.
@@ -66,13 +68,6 @@ properties:
       insensitive, letters in the riscv,isa string must be all
       lowercase to simplify parsing.
 
-  timebase-frequency:
-    type: integer
-    minimum: 1
-    description:
-      Specifies the clock frequency of the system timer in Hz.
-      This value is common to all harts on a single system image.
-
   interrupt-controller:
     type: object
     description: Describes the CPU's local interrupt controller
@@ -93,7 +88,6 @@ properties:
 
 required:
   - riscv,isa
-  - timebase-frequency
   - interrupt-controller
 
 examples:
Signature · 1 line
2.20.1
#1 lore raw
Quoted 11 lines · Rob Herring

Fix the errors in the RiscV CPU DT schema:

Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@1: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible:0: 'riscv' is not one of ['sifive,rocket0', 'sifive,e5', 'sifive,e51', 'sifive,u54-mc', 'sifive,u54', 'sifive,u5'] Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible: ['riscv'] is too short Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property

The DT spec allows for 'timebase-frequency' to be in 'cpu' or 'cpus' node and RiscV is doing nothing special with it, so just drop the definition here and don't make it required.

The RISC-V kernel code does in fact parse it and use it, and we currently rely on it being under /cpus:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/kernel/time.c#n19

The RISC-V user ISA specification also constrains the timebase-frequency to be the same across all CPUs, in section 10.1:

https://github.com/riscv/riscv-isa-manual/releases/download/draft-20190608-f467e5d/riscv-spec.pdf

So the right thing is to require 'timebase-frequency' at /cpus, and forbid it in the individual CPU nodes.

Quoted 68 lines
Fixes 4fd669a8c487 ("dt-bindings: riscv: convert cpu binding to json-schema")
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
---
 .../devicetree/bindings/riscv/cpus.yaml       | 28 ++++++++-----------
 1 file changed, 11 insertions(+), 17 deletions(-)
Documentation/devicetree/bindings/riscv/cpus.yaml +11 -17
index b261a3015f84..925b531767bf 100644
@@ -24,15 +24,17 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - sifive,rocket0
-          - sifive,e5
-          - sifive,e51
-          - sifive,u54-mc
-          - sifive,u54
-          - sifive,u5
-      - const: riscv
+    oneOf:
+      - items:
+          - enum:
+              - sifive,rocket0
+              - sifive,e5
+              - sifive,e51
+              - sifive,u54-mc
+              - sifive,u54
+              - sifive,u5
+          - const: riscv
+      - const: riscv    # Simulator only
     description:
       Identifies that the hart uses the RISC-V instruction set
       and identifies the type of the hart.
@@ -66,13 +68,6 @@ properties:
       insensitive, letters in the riscv,isa string must be all
       lowercase to simplify parsing.
 
-  timebase-frequency:
-    type: integer
-    minimum: 1
-    description:
-      Specifies the clock frequency of the system timer in Hz.
-      This value is common to all harts on a single system image.
-
   interrupt-controller:
     type: object
     description: Describes the CPU's local interrupt controller
@@ -93,7 +88,6 @@ properties:
 
 required:
   - riscv,isa
-  - timebase-frequency
   - interrupt-controller
 
 examples:
Signature · 1 line
2.20.1

- Paul

author maintainer
#2 lore raw
Quoted 20 lines · Paul Walmsley
Quoted 11 lines · Rob Herring

Fix the errors in the RiscV CPU DT schema:

Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@1: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible:0: 'riscv' is not one of ['sifive,rocket0', 'sifive,e5', 'sifive,e51', 'sifive,u54-mc', 'sifive,u54', 'sifive,u5'] Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible: ['riscv'] is too short Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property

The DT spec allows for 'timebase-frequency' to be in 'cpu' or 'cpus' node and RiscV is doing nothing special with it, so just drop the definition here and don't make it required.

The RISC-V kernel code does in fact parse it and use it, and we currently rely on it being under /cpus:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/kernel/time.c#n19

Quoted 7 lines

The RISC-V user ISA specification also constrains the timebase-frequency to be the same across all CPUs, in section 10.1:

https://github.com/riscv/riscv-isa-manual/releases/download/draft-20190608-f467e5d/riscv-spec.pdf

So the right thing is to require 'timebase-frequency' at /cpus, and forbid it in the individual CPU nodes.

Yes, but this schema only deals with 'cpu' nodes and we can't check /cpus here. We'd need to write another schema matching on a child cpu node having a RiscV compatible.

I can change this to 'timebase-frequency: false' to ban it here. That doesn't add too much as any undefined name is still allowed such as 'timbase-frequency'. There's a way to address this in json-schema draft8 with 'unevaluatedProperties', but that's not ready yet.

Rob

Acked-by
#3 lore raw
Quoted 26 lines · Rob Herring
Quoted 25 lines · Paul Walmsley
Quoted 11 lines · Rob Herring

Fix the errors in the RiscV CPU DT schema:

Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@1: 'timebase-frequency' is a required property Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible:0: 'riscv' is not one of ['sifive,rocket0', 'sifive,e5', 'sifive,e51', 'sifive,u54-mc', 'sifive,u54', 'sifive,u5'] Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: compatible: ['riscv'] is too short Documentation/devicetree/bindings/riscv/cpus.example.dt.yaml: cpu@0: 'timebase-frequency' is a required property

The DT spec allows for 'timebase-frequency' to be in 'cpu' or 'cpus' node and RiscV is doing nothing special with it, so just drop the definition here and don't make it required.

The RISC-V kernel code does in fact parse it and use it, and we currently rely on it being under /cpus:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/kernel/time.c#n19

The RISC-V user ISA specification also constrains the timebase-frequency to be the same across all CPUs, in section 10.1:

https://github.com/riscv/riscv-isa-manual/releases/download/draft-20190608-f467e5d/riscv-spec.pdf

Quoted 8 lines
Quoted 2 lines

So the right thing is to require 'timebase-frequency' at /cpus, and forbid it in the individual CPU nodes.

Yes, but this schema only deals with 'cpu' nodes and we can't check /cpus here. We'd need to write another schema matching on a child cpu node having a RiscV compatible.

I can change this to 'timebase-frequency: false' to ban it here.

Sounds good to me. (Might catch the occasional mistake.) With that change, the resulting patch would be

and thanks indeed for cleaning this up.

Quoted 3 lines

That doesn't add too much as any undefined name is still allowed such as 'timbase-frequency'. There's a way to address this in json-schema draft8 with 'unevaluatedProperties', but that's not ready yet.

OK

- Paul