RE: [PATCH v1 2/3] dt-bindings: riscv: convert plic bindings to json-schema

Hello Rob,

-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Wednesday, September 23, 2020 2:04 AM
To: Sagar Kadam <redacted>
Cc: linux-pwm@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
riscv@lists.infradead.org; devicetree@vger.kernel.org; linux-
clk@vger.kernel.org; mturquette@baylibre.com; sboyd@kernel.org; Paul
Walmsley ( Sifive) [off-list ref]; palmer@dabbelt.com;
tglx@linutronix.de; jason@lakedaemon.net; maz@kernel.org;
thierry.reding@gmail.com; u.kleine-koenig@pengutronix.de;
lee.jones@linaro.org; aou@eecs.berkeley.edu; Yash Shah
[off-list ref]
Subject: Re: [PATCH v1 2/3] dt-bindings: riscv: convert plic bindings to json-
schema

[External Email] Do not click links or attachments unless you recognize the
sender and know the content is safe

On Thu, Sep 10, 2020 at 04:14:03PM +0530, Sagar Kadam wrote:

quoted

Convert device tree bindings for SiFive's PLIC to YAML format

Signed-off-by: Sagar Kadam <redacted>
---
 .../interrupt-controller/sifive,plic-1.0.0.txt     |  58 -----------
 .../interrupt-controller/sifive,plic-1.0.0.yaml    | 107

+++++++++++++++++++++

quoted

 2 files changed, 107 insertions(+), 58 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-

controller/sifive,plic-1.0.0.txt

quoted

 create mode 100644 Documentation/devicetree/bindings/interrupt-

controller/sifive,plic-1.0.0.yaml

quoted

diff --git a/Documentation/devicetree/bindings/interrupt-

controller/sifive,plic-1.0.0.txt
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-
1.0.0.txt

quoted

deleted file mode 100644
index 6adf7a6..0000000

--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-

1.0.0.txt

quoted

+++ /dev/null

@@ -1,58 +0,0 @@
-SiFive Platform-Level Interrupt Controller (PLIC)
--------------------------------------------------
-
-SiFive SOCs include an implementation of the Platform-Level Interrupt

Controller

quoted

-(PLIC) high-level specification in the RISC-V Privileged Architecture
-specification.  The PLIC connects all external interrupts in the system to all
-hart contexts in the system, via the external interrupt source in each hart.
-
-A hart context is a privilege mode in a hardware execution thread.  For

example,

quoted

-in an 4 core system with 2-way SMT, you have 8 harts and probably at

least two

quoted

-privilege modes per hart; machine mode and supervisor mode.
-
-Each interrupt can be enabled on per-context basis.  Any context can claim
-a pending enabled interrupt and then release it once it has been handled.
-
-Each interrupt has a configurable priority.  Higher priority interrupts are
-serviced first.  Each context can specify a priority threshold. Interrupts
-with priority below this threshold will not cause the PLIC to raise its
-interrupt line leading to the context.
-
-While the PLIC supports both edge-triggered and level-triggered interrupts,
-interrupt handlers are oblivious to this distinction and therefore it is not
-specified in the PLIC device-tree binding.
-
-While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
-"sifive,plic-1.0.0" device is a concrete implementation of the PLIC that
-contains a specific memory layout, which is documented in chapter 8 of

the

quoted

-SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-

RVCoreIP.pdf>.

quoted

-
-Required properties:
-- compatible : "sifive,plic-1.0.0" and a string identifying the actual
-  detailed implementation in case that specific bugs need to be worked

around.

quoted

-- #address-cells : should be <0> or more.
-- #interrupt-cells : should be <1> or more.
-- interrupt-controller : Identifies the node as an interrupt controller.
-- reg : Should contain 1 register range (address and length).
-- interrupts-extended : Specifies which contexts are connected to the PLIC,
-  with "-1" specifying that a context is not present.  Each node pointed
-  to should be a riscv,cpu-intc node, which has a riscv node as parent.
-- riscv,ndev: Specifies how many external interrupts are supported by
-  this controller.
-
-Example:
-
-     plic: interrupt-controller@c000000 {
-             #address-cells = <0>;
-             #interrupt-cells = <1>;
-             compatible = "sifive,plic-1.0.0", "sifive,fu540-c000-plic";
-             interrupt-controller;
-             interrupts-extended = <
-                     &cpu0-intc 11
-                     &cpu1-intc 11 &cpu1-intc 9
-                     &cpu2-intc 11 &cpu2-intc 9
-                     &cpu3-intc 11 &cpu3-intc 9
-                     &cpu4-intc 11 &cpu4-intc 9>;
-             reg = <0xc000000 0x4000000>;
-             riscv,ndev = <10>;
-     };

diff --git a/Documentation/devicetree/bindings/interrupt-

controller/sifive,plic-1.0.0.yaml
b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-
1.0.0.yaml

quoted

new file mode 100644
index 0000000..95c8c85

--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-

1.0.0.yaml

quoted

@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+# Copyright (C) 2020 SiFive, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/sifive,plic-

1.0.0.yaml#

quoted

+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive Platform-Level Interrupt Controller (PLIC)
+
+description:
+  SiFive SOCs include an implementation of the Platform-Level Interrupt

Controller

quoted

+  (PLIC) high-level specification in the RISC-V Privileged Architecture
+  specification. The PLIC connects all external interrupts in the system to all
+  hart contexts in the system, via the external interrupt source in each

hart.

quoted

+
+  A hart context is a privilege mode in a hardware execution thread. For

example,

quoted

+  in an 4 core system with 2-way SMT, you have 8 harts and probably at

least two

quoted

+  privilege modes per hart; machine mode and supervisor mode.
+
+  Each interrupt can be enabled on per-context basis. Any context can

claim

quoted

+  a pending enabled interrupt and then release it once it has been

handled.

quoted

+
+  Each interrupt has a configurable priority. Higher priority interrupts are
+  serviced first.  Each context can specify a priority threshold. Interrupts
+  with priority below this threshold will not cause the PLIC to raise its
+  interrupt line leading to the context.
+
+  While the PLIC supports both edge-triggered and level-triggered

interrupts,

quoted

+  interrupt handlers are oblivious to this distinction and therefore it is not
+  specified in the PLIC device-tree binding.
+
+  While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
+  "sifive,plic-1.0.0" device is a concrete implementation of the PLIC that
+  contains a specific memory layout, which is documented in chapter 8 of

the

quoted

+  SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-

RVCoreIP.pdf>.

quoted

+
+maintainers:
+  - Sagar Kadam [off-list ref]
+  - Paul Walmsley  [off-list ref]
+  - Palmer Dabbelt [off-list ref]
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#

Don't need this. It gets selected matching on node name.

Thanks for your suggestions. I will incorporate your suggestions and send the v2.

quoted

+
+properties:
+  compatible:
+    items:
+      - const: sifive,plic-1.0.0
+      - const: sifive,fu540-c000-plic

Somehow these ended up in the wrong order. Should be most specific to
least specific.

Yes, will rectify this.

quoted

+
+    description:
+      Should be "sifive,plic-1.0.0" and a string identifying the actual
+      detailed implementation in case that specific bugs need to be worked

around.

Drop this.

quoted

+
+  reg:
+    maxItems: 1
+    description:  Should contain 1 register range (address and length).

Drop this. The schema says this...

quoted

+
+  '#address-cells':
+    const: 0
+    description: Should be <0> or more.

Drop. 'or more' is wrong. If there's a case with more, it will need to
be documented.

quoted

+
+  '#interrupt-cells':
+    const: 1
+    description: Should be <1> or more.

Same here.

Okay. 
Will remove as suggested above

Thanks & BR,
Sagar

quoted

+
+  interrupt-controller: true
+
+  interrupts-extended:
+    minItems: 1
+    description:
+      Specifies which contexts are connected to the PLIC, with "-1" specifying
+      that a context is not present. Each node pointed to should be a
+      riscv,cpu-intc node, which has a riscv node as parent.
+
+  riscv,ndev:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description:
+      Specifies how many external interrupts are supported by this

controller.

quoted

+
+required:
+  - compatible
+  - '#address-cells'
+  - '#interrupt-cells'
+  - interrupt-controller
+  - reg
+  - interrupts-extended
+  - riscv,ndev
+
+additionalProperties: false
+
+examples:
+  - |
+    plic: interrupt-controller@c000000 {
+      #address-cells = <0>;
+      #interrupt-cells = <1>;
+      compatible = "sifive,plic-1.0.0", "sifive,fu540-c000-plic";
+      interrupt-controller;
+      interrupts-extended = <
+        &cpu0_intc 11
+        &cpu1_intc 11 &cpu1_intc 9
+        &cpu2_intc 11 &cpu2_intc 9
+        &cpu3_intc 11 &cpu3_intc 9
+        &cpu4_intc 11 &cpu4_intc 9>;
+      reg = <0xc000000 0x4000000>;
+      riscv,ndev = <10>;
+    };
--
2.7.4