[PATCH v1 0/3] convert sifive's prci, plic and pwm bindings to yaml

[PATCH v1 0/3] convert sifive's prci, plic and pwm bindings to yaml

[Sagar Kadam]

The PRCI and PLIC controller bindings are in txt format. Here, we convert
them to yaml format.

These patches are tested on commit f4d51dffc6c0 ("Linux 5.9-rc4"). I have
added the log of dt_binding_check for these IP block's on 5.9-rc4 kernel
here [1] in case someone want's to refer it.

[1] https://paste.ubuntu.com/p/VHqqnXdrkJ

Additionally the default log of dt_binding_check on linux-5.9-rc4 without
these patches can be found here [2].

[2] https://paste.ubuntu.com/p/KNGJrJvvdt

Patch History:
============================
V1: Base version.

Sagar Kadam (3):
  dt-bindings: fu540: prci: convert PRCI bindings to json-schema
  dt-bindings: riscv: convert plic bindings to json-schema
  dt-bindings: riscv: convert pwm bindings to json-schema

 .../bindings/clock/sifive/fu540-prci.txt           |  46 ---------
 .../bindings/clock/sifive/fu540-prci.yaml          |  75 +++++++++++++++
 .../interrupt-controller/sifive,plic-1.0.0.txt     |  58 -----------
 .../interrupt-controller/sifive,plic-1.0.0.yaml    | 107 +++++++++++++++++++++
 .../devicetree/bindings/pwm/pwm-sifive.txt         |  33 -------
 .../devicetree/bindings/pwm/pwm-sifive.yaml        |  72 ++++++++++++++
 6 files changed, 254 insertions(+), 137 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt
 create mode 100644 Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
 delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.yaml

-- 
2.7.4

[PATCH v1 1/3] dt-bindings: fu540: prci: convert PRCI bindings to json-schema

[Sagar Kadam]

FU540-C000 SoC from SiFive has a PRCI block, here we convert
the device tree bindings from txt to YAML.

Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
---
 .../bindings/clock/sifive/fu540-prci.txt           | 46 -------------
 .../bindings/clock/sifive/fu540-prci.yaml          | 75 ++++++++++++++++++++++
 2 files changed, 75 insertions(+), 46 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt
 create mode 100644 Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml

diff --git a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt
deleted file mode 100644
index 349808f..0000000
--- a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.txt
+++ /dev/null
@@ -1,46 +0,0 @@
-SiFive FU540 PRCI bindings
-
-On the FU540 family of SoCs, most system-wide clock and reset integration
-is via the PRCI IP block.
-
-Required properties:
-- compatible: Should be "sifive,<chip>-prci".  Only one value is
-	supported: "sifive,fu540-c000-prci"
-- reg: Should describe the PRCI's register target physical address region
-- clocks: Should point to the hfclk device tree node and the rtcclk
-          device tree node.  The RTC clock here is not a time-of-day clock,
-	  but is instead a high-stability clock source for system timers
-	  and cycle counters.
-- #clock-cells: Should be <1>
-
-The clock consumer should specify the desired clock via the clock ID
-macros defined in include/dt-bindings/clock/sifive-fu540-prci.h.
-These macros begin with PRCI_CLK_.
-
-The hfclk and rtcclk nodes are required, and represent physical
-crystals or resonators located on the PCB.  These nodes should be present
-underneath /, rather than /soc.
-
-Examples:
-
-/* under /, in PCB-specific DT data */
-hfclk: hfclk {
-	#clock-cells = <0>;
-	compatible = "fixed-clock";
-	clock-frequency = <33333333>;
-	clock-output-names = "hfclk";
-};
-rtcclk: rtcclk {
-	#clock-cells = <0>;
-	compatible = "fixed-clock";
-	clock-frequency = <1000000>;
-	clock-output-names = "rtcclk";
-};
-
-/* under /soc, in SoC-specific DT data */
-prci: clock-controller@10000000 {
-	compatible = "sifive,fu540-c000-prci";
-	reg = <0x0 0x10000000 0x0 0x1000>;
-	clocks = <&hfclk>, <&rtcclk>;
-	#clock-cells = <1>;
-};
diff --git a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
new file mode 100644
index 0000000..49386cd
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 SiFive, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sifive/fu540-prci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive FU540 Power Reset Clock Interrupt Controller (PRCI)
+
+maintainers:
+  - Sagar Kadam <sagar.kadam@sifive.com>
+  - Paul Walmsley  <paul.walmsley@sifive.com>
+
+description:
+  On the FU540 family of SoCs, most system-wide clock and reset integration
+  is via the PRCI IP block.
+  The clock consumer should specify the desired clock via the clock ID
+  macros defined in include/dt-bindings/clock/sifive-fu540-prci.h.
+  These macros begin with PRCI_CLK_.
+
+  The hfclk and rtcclk nodes are required, and represent physical
+  crystals or resonators located on the PCB.  These nodes should be present
+  underneath /, rather than /soc.
+
+properties:
+  compatible:
+    enum:
+      - sifive,fu540-c000-prci
+    description:
+      Should have "sifive,<soc>-prci", only one value is supported
+
+  reg:
+    maxItems: 1
+    description: Describe the PRCI's register target physical address region
+
+  clocks:
+    description:
+      Should point to the hfclk device tree node and the rtcclk device tree node.
+      The RTC clock here is not a time-of-day clock, but is instead a high-stability
+      clock source for system timers and cycle counters.
+
+  "#clock-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    //hfclk and rtcclk present under /, in PCB-specific DT data
+    hfclk: hfclk {
+      #clock-cells = <0>;
+      compatible = "fixed-clock";
+      clock-frequency = <33333333>;
+      clock-output-names = "hfclk";
+    };
+    rtcclk: rtcclk {
+      #clock-cells = <0>;
+      compatible = "fixed-clock";
+      clock-frequency = <1000000>;
+      clock-output-names = "rtcclk";
+    };
+
+    //under /soc, in SoC-specific DT data
+    prci: clock-controller@10000000 {
+      compatible = "sifive,fu540-c000-prci";
+      reg = <0x10000000 0x1000>;
+      clocks = <&hfclk>, <&rtcclk>;
+      #clock-cells = <1>;
+    };
-- 
2.7.4

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

[Sagar Kadam]

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

Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
---
 .../interrupt-controller/sifive,plic-1.0.0.txt     |  58 -----------
 .../interrupt-controller/sifive,plic-1.0.0.yaml    | 107 +++++++++++++++++++++
 2 files changed, 107 insertions(+), 58 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml

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
deleted file mode 100644
index 6adf7a6..0000000
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-SiFive Platform-Level Interrupt Controller (PLIC)
--------------------------------------------------
-
-SiFive SOCs include an implementation of the Platform-Level Interrupt Controller
-(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,
-in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
-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
-SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
-
-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.
-- #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
new file mode 100644
index 0000000..95c8c85
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -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#
+$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
+  (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,
+  in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
+  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
+  SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
+
+maintainers:
+  - Sagar Kadam <sagar.kadam@sifive.com>
+  - Paul Walmsley  <paul.walmsley@sifive.com>
+  - Palmer Dabbelt <palmer@dabbelt.com>
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: sifive,plic-1.0.0
+      - const: sifive,fu540-c000-plic
+
+    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.
+
+  reg:
+    maxItems: 1
+    description:  Should contain 1 register range (address and length).
+
+  '#address-cells':
+    const: 0
+    description: Should be <0> or more.
+
+  '#interrupt-cells':
+    const: 1
+    description: Should be <1> or more.
+
+  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.
+
+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

[PATCH v1 3/3] dt-bindings: riscv: convert pwm bindings to json-schema

[Sagar Kadam]

Convert device tree bindings for SiFive's PWM controller to YAML
format.

Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
---
 .../devicetree/bindings/pwm/pwm-sifive.txt         | 33 ----------
 .../devicetree/bindings/pwm/pwm-sifive.yaml        | 72 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 33 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.yaml

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
deleted file mode 100644
index 3d1dd7b0..0000000
--- a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-SiFive PWM controller
-
-Unlike most other PWM controllers, the SiFive PWM controller currently only
-supports one period for all channels in the PWM. All PWMs need to run at
-the same period. The period also has significant restrictions on the values
-it can achieve, which the driver rounds to the nearest achievable period.
-PWM RTL that corresponds to the IP block version numbers can be found
-here:
-
-https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/pwm
-
-Required properties:
-- compatible: Should be "sifive,<chip>-pwm" and "sifive,pwm<version>".
-  Supported compatible strings are: "sifive,fu540-c000-pwm" for the SiFive
-  PWM v0 as integrated onto the SiFive FU540 chip, and "sifive,pwm0" for the
-  SiFive PWM v0 IP block with no chip integration tweaks.
-  Please refer to sifive-blocks-ip-versioning.txt for details.
-- reg: physical base address and length of the controller's registers
-- clocks: Should contain a clock identifier for the PWM's parent clock.
-- #pwm-cells: Should be 3. See pwm.yaml in this directory
-  for a description of the cell format.
-- interrupts: one interrupt per PWM channel
-
-Examples:
-
-pwm:  pwm@10020000 {
-	compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
-	reg = <0x0 0x10020000 0x0 0x1000>;
-	clocks = <&tlclk>;
-	interrupt-parent = <&plic>;
-	interrupts = <42 43 44 45>;
-	#pwm-cells = <3>;
-};
diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
new file mode 100644
index 0000000..415d053
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 SiFive, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-sifive.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive PWM controller
+
+maintainers:
+  - Yash Shah <yash.shah@sifive.com>
+  - Sagar Kadam <sagar.kadam@sifive.com>
+  - Paul Walmsley <paul.walmsley@sifive.com>
+
+description:
+  Unlike most other PWM controllers, the SiFive PWM controller currently
+  only supports one period for all channels in the PWM. All PWMs need to
+  run at the same period. The period also has significant restrictions on
+  the values it can achieve, which the driver rounds to the nearest
+  achievable period. PWM RTL that corresponds to the IP block version
+  numbers can be found here -
+
+  https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/pwm
+
+properties:
+  compatible:
+    items:
+      - const: sifive,fu540-c000-pwm
+      - const: sifive,pwm0
+    description:
+      Should be "sifive,<chip>-pwm" and "sifive,pwm<version>". Supported
+      compatible strings are "sifive,fu540-c000-pwm" for the SiFive PWM v0
+      as integrated onto the SiFive FU540 chip, and "sifive,pwm0" for the
+      SiFive PWM v0 IP block with no chip integration tweaks.
+      Please refer to sifive-blocks-ip-versioning.txt for details.
+
+  reg:
+    maxItems: 1
+    description: Physical base address and length of the controller's registers
+
+  clocks:
+    description: Should contain a clock identifier for the PWM's parent clock.
+
+  "#pwm-cells":
+    const: 3
+    description:
+      Should be 3. See pwm.yaml in this directory for a description of the
+      cell format.
+
+  interrupts:
+    maxItems: 1
+    description: One interrupt per PWM channel.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#pwm-cells"
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    pwm:  pwm@10020000 {
+      compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
+      reg = <0x10020000 0x1000>;
+      clocks = <&tlclk>;
+      interrupt-parent = <&plic>;
+      interrupts = <42 43 44 45>;
+      #pwm-cells = <3>;
+    };
-- 
2.7.4

Re: [PATCH v1 1/3] dt-bindings: fu540: prci: convert PRCI bindings to json-schema

[Stephen Boyd]

Quoting Sagar Kadam (2020-09-10 03:44:02)
> diff --git a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
> new file mode 100644
> index 0000000..49386cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 SiFive, Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/sifive/fu540-prci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive FU540 Power Reset Clock Interrupt Controller (PRCI)
> +
> +maintainers:
> +  - Sagar Kadam <sagar.kadam@sifive.com>
> +  - Paul Walmsley  <paul.walmsley@sifive.com>
> +
> +description:
> +  On the FU540 family of SoCs, most system-wide clock and reset integration
> +  is via the PRCI IP block.
> +  The clock consumer should specify the desired clock via the clock ID
> +  macros defined in include/dt-bindings/clock/sifive-fu540-prci.h.
> +  These macros begin with PRCI_CLK_.
> +
> +  The hfclk and rtcclk nodes are required, and represent physical
> +  crystals or resonators located on the PCB.  These nodes should be present
> +  underneath /, rather than /soc.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sifive,fu540-c000-prci
> +    description:
> +      Should have "sifive,<soc>-prci", only one value is supported

Drop description and have 

    compatible:
      const: sifive,fu540-c000-prci

> +
> +  reg:
> +    maxItems: 1
> +    description: Describe the PRCI's register target physical address region

Drop description.

> +
> +  clocks:
> +    description:
> +      Should point to the hfclk device tree node and the rtcclk device tree node.

s/device tree node//g

> +      The RTC clock here is not a time-of-day clock, but is instead a high-stability
> +      clock source for system timers and cycle counters.

Better to have:

    clocks:
      items:
        - const: high frequency clock
	- const: RTC clock

Can you add clock-names too? Making it optional is OK.

> +  "#clock-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#clock-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    //hfclk and rtcclk present under /, in PCB-specific DT data
> +    hfclk: hfclk {
> +      #clock-cells = <0>;
> +      compatible = "fixed-clock";
> +      clock-frequency = <33333333>;
> +      clock-output-names = "hfclk";
> +    };

Add a newline here?

> +    rtcclk: rtcclk {
> +      #clock-cells = <0>;
> +      compatible = "fixed-clock";
> +      clock-frequency = <1000000>;
> +      clock-output-names = "rtcclk";
> +    };

These may not be necessary either, just have the clock-controller node
reference phandles?

> +
> +    //under /soc, in SoC-specific DT data

Don't think this comment is necessary.

> +    prci: clock-controller@10000000 {
> +      compatible = "sifive,fu540-c000-prci";
> +      reg = <0x10000000 0x1000>;
> +      clocks = <&hfclk>, <&rtcclk>;
> +      #clock-cells = <1>;
> +    };

RE: [PATCH v1 1/3] dt-bindings: fu540: prci: convert PRCI bindings to json-schema

[Sagar Kadam]

Hello Stephen,

> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, September 15, 2020 5:37 AM
> To: Sagar Kadam <sagar.kadam@openfive.com>;
> devicetree@vger.kernel.org; linux-clk@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-pwm@vger.kernel.org; linux-
> riscv@lists.infradead.org
> Cc: mturquette@baylibre.com; robh+dt@kernel.org; Paul Walmsley ( Sifive)
> <paul.walmsley@sifive.com>; 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 <yash.shah@openfive.com>; Sagar
> Kadam <sagar.kadam@openfive.com>
> Subject: Re: [PATCH v1 1/3] dt-bindings: fu540: prci: convert PRCI bindings
> to json-schema
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Quoting Sagar Kadam (2020-09-10 03:44:02)
> > diff --git
> > a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
> > b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
> > new file mode 100644
> > index 0000000..49386cd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
> > +(C) 2020 SiFive, Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/sifive/fu540-prci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiFive FU540 Power Reset Clock Interrupt Controller (PRCI)
> > +
> > +maintainers:
> > +  - Sagar Kadam <sagar.kadam@sifive.com>
> > +  - Paul Walmsley  <paul.walmsley@sifive.com>
> > +
> > +description:
> > +  On the FU540 family of SoCs, most system-wide clock and reset
> > +integration
> > +  is via the PRCI IP block.
> > +  The clock consumer should specify the desired clock via the clock
> > +ID
> > +  macros defined in include/dt-bindings/clock/sifive-fu540-prci.h.
> > +  These macros begin with PRCI_CLK_.
> > +
> > +  The hfclk and rtcclk nodes are required, and represent physical
> > + crystals or resonators located on the PCB.  These nodes should be
> > + present  underneath /, rather than /soc.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - sifive,fu540-c000-prci
> > +    description:
> > +      Should have "sifive,<soc>-prci", only one value is supported
> 
> Drop description and have
> 
>     compatible:
>       const: sifive,fu540-c000-prci
> 
Thank you for suggestion here, I will remove this.
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Describe the PRCI's register target physical address
> > + region
> 
> Drop description.
> 
Okay.
> > +
> > +  clocks:
> > +    description:
> > +      Should point to the hfclk device tree node and the rtcclk device tree
> node.
> 
> s/device tree node//g

Okay, will remove these.

> 
> > +      The RTC clock here is not a time-of-day clock, but is instead a high-
> stability
> > +      clock source for system timers and cycle counters.
> 
> Better to have:
> 
>     clocks:
>       items:
>         - const: high frequency clock
>         - const: RTC clock
> 
> Can you add clock-names too? Making it optional is OK.
Okay, I will include these optional properties as
            -const: "hfclk"
            -const: "rtcclk"
> 
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - "#clock-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    //hfclk and rtcclk present under /, in PCB-specific DT data
> > +    hfclk: hfclk {
> > +      #clock-cells = <0>;
> > +      compatible = "fixed-clock";
> > +      clock-frequency = <33333333>;
> > +      clock-output-names = "hfclk";
> > +    };
> 
> Add a newline here?
> 
Okay.
> > +    rtcclk: rtcclk {
> > +      #clock-cells = <0>;
> > +      compatible = "fixed-clock";
> > +      clock-frequency = <1000000>;
> > +      clock-output-names = "rtcclk";
> > +    };
> 
> These may not be necessary either, just have the clock-controller node
> reference phandles?
> 
Okay.
> > +
> > +    //under /soc, in SoC-specific DT data
> 
> Don't think this comment is necessary.
> 
Okay.

Thanks & BR,
Sagar

> > +    prci: clock-controller@10000000 {
> > +      compatible = "sifive,fu540-c000-prci";
> > +      reg = <0x10000000 0x1000>;
> > +      clocks = <&hfclk>, <&rtcclk>;
> > +      #clock-cells = <1>;
> > +    };

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

[Rob Herring]

On Thu, Sep 10, 2020 at 04:14:03PM +0530, Sagar Kadam wrote:
> Convert device tree bindings for SiFive's PLIC to YAML format
> 
> Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
> ---
>  .../interrupt-controller/sifive,plic-1.0.0.txt     |  58 -----------
>  .../interrupt-controller/sifive,plic-1.0.0.yaml    | 107 +++++++++++++++++++++
>  2 files changed, 107 insertions(+), 58 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> 
> 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
> deleted file mode 100644
> index 6adf7a6..0000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -SiFive Platform-Level Interrupt Controller (PLIC)
> --------------------------------------------------
> -
> -SiFive SOCs include an implementation of the Platform-Level Interrupt Controller
> -(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,
> -in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
> -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
> -SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> -
> -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.
> -- #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
> new file mode 100644
> index 0000000..95c8c85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> @@ -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#
> +$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
> +  (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,
> +  in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
> +  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
> +  SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> +
> +maintainers:
> +  - Sagar Kadam <sagar.kadam@sifive.com>
> +  - Paul Walmsley  <paul.walmsley@sifive.com>
> +  - Palmer Dabbelt <palmer@dabbelt.com>
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#

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

> +
> +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.

> +
> +    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.

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

Drop this. The schema says this...

> +
> +  '#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.

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

Same here.

> +
> +  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.
> +
> +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
> 

Re: [PATCH v1 3/3] dt-bindings: riscv: convert pwm bindings to json-schema

[Rob Herring]

On Thu, Sep 10, 2020 at 04:14:04PM +0530, Sagar Kadam wrote:
> Convert device tree bindings for SiFive's PWM controller to YAML
> format.
> 
> Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
> ---
>  .../devicetree/bindings/pwm/pwm-sifive.txt         | 33 ----------
>  .../devicetree/bindings/pwm/pwm-sifive.yaml        | 72 ++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 33 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> deleted file mode 100644
> index 3d1dd7b0..0000000
> --- a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -SiFive PWM controller
> -
> -Unlike most other PWM controllers, the SiFive PWM controller currently only
> -supports one period for all channels in the PWM. All PWMs need to run at
> -the same period. The period also has significant restrictions on the values
> -it can achieve, which the driver rounds to the nearest achievable period.
> -PWM RTL that corresponds to the IP block version numbers can be found
> -here:
> -
> -https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/pwm
> -
> -Required properties:
> -- compatible: Should be "sifive,<chip>-pwm" and "sifive,pwm<version>".
> -  Supported compatible strings are: "sifive,fu540-c000-pwm" for the SiFive
> -  PWM v0 as integrated onto the SiFive FU540 chip, and "sifive,pwm0" for the
> -  SiFive PWM v0 IP block with no chip integration tweaks.
> -  Please refer to sifive-blocks-ip-versioning.txt for details.
> -- reg: physical base address and length of the controller's registers
> -- clocks: Should contain a clock identifier for the PWM's parent clock.
> -- #pwm-cells: Should be 3. See pwm.yaml in this directory
> -  for a description of the cell format.
> -- interrupts: one interrupt per PWM channel
> -
> -Examples:
> -
> -pwm:  pwm@10020000 {
> -	compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
> -	reg = <0x0 0x10020000 0x0 0x1000>;
> -	clocks = <&tlclk>;
> -	interrupt-parent = <&plic>;
> -	interrupts = <42 43 44 45>;
> -	#pwm-cells = <3>;
> -};
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> new file mode 100644
> index 0000000..415d053
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 SiFive, Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/pwm-sifive.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive PWM controller
> +
> +maintainers:
> +  - Yash Shah <yash.shah@sifive.com>
> +  - Sagar Kadam <sagar.kadam@sifive.com>
> +  - Paul Walmsley <paul.walmsley@sifive.com>
> +
> +description:
> +  Unlike most other PWM controllers, the SiFive PWM controller currently
> +  only supports one period for all channels in the PWM. All PWMs need to
> +  run at the same period. The period also has significant restrictions on
> +  the values it can achieve, which the driver rounds to the nearest
> +  achievable period. PWM RTL that corresponds to the IP block version
> +  numbers can be found here -
> +
> +  https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/pwm
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: sifive,fu540-c000-pwm
> +      - const: sifive,pwm0
> +    description:
> +      Should be "sifive,<chip>-pwm" and "sifive,pwm<version>". Supported
> +      compatible strings are "sifive,fu540-c000-pwm" for the SiFive PWM v0
> +      as integrated onto the SiFive FU540 chip, and "sifive,pwm0" for the
> +      SiFive PWM v0 IP block with no chip integration tweaks.
> +      Please refer to sifive-blocks-ip-versioning.txt for details.
> +
> +  reg:
> +    maxItems: 1
> +    description: Physical base address and length of the controller's registers

Drop description.

> +
> +  clocks:
> +    description: Should contain a clock identifier for the PWM's parent clock.

How many clocks?

> +
> +  "#pwm-cells":
> +    const: 3
> +    description:
> +      Should be 3. See pwm.yaml in this directory for a description of the
> +      cell format.

Drop.

> +
> +  interrupts:
> +    maxItems: 1

Is it 1 or...

> +    description: One interrupt per PWM channel.

one per channel?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#pwm-cells"
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pwm:  pwm@10020000 {
> +      compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
> +      reg = <0x10020000 0x1000>;
> +      clocks = <&tlclk>;
> +      interrupt-parent = <&plic>;
> +      interrupts = <42 43 44 45>;

Split entries:

interrupts = <42>, <43>, <44>, <45>;

> +      #pwm-cells = <3>;
> +    };
> -- 
> 2.7.4
> 

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

[Sagar Kadam]

Hello Rob,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, September 23, 2020 2:04 AM
> To: Sagar Kadam <sagar.kadam@openfive.com>
> 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) <paul.walmsley@sifive.com>; 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
> <yash.shah@openfive.com>
> 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:
> > Convert device tree bindings for SiFive's PLIC to YAML format
> >
> > Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
> > ---
> >  .../interrupt-controller/sifive,plic-1.0.0.txt     |  58 -----------
> >  .../interrupt-controller/sifive,plic-1.0.0.yaml    | 107
> +++++++++++++++++++++
> >  2 files changed, 107 insertions(+), 58 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/interrupt-
> controller/sifive,plic-1.0.0.txt
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-
> controller/sifive,plic-1.0.0.yaml
> >
> > 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
> > deleted file mode 100644
> > index 6adf7a6..0000000
> > --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-
> 1.0.0.txt
> > +++ /dev/null
> > @@ -1,58 +0,0 @@
> > -SiFive Platform-Level Interrupt Controller (PLIC)
> > --------------------------------------------------
> > -
> > -SiFive SOCs include an implementation of the Platform-Level Interrupt
> Controller
> > -(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,
> > -in an 4 core system with 2-way SMT, you have 8 harts and probably at
> least two
> > -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
> > -SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-
> RVCoreIP.pdf>.
> > -
> > -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.
> > -- #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
> > new file mode 100644
> > index 0000000..95c8c85
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-
> 1.0.0.yaml
> > @@ -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#
> > +$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
> > +  (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,
> > +  in an 4 core system with 2-way SMT, you have 8 harts and probably at
> least two
> > +  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
> > +  SiFive U5 Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-
> RVCoreIP.pdf>.
> > +
> > +maintainers:
> > +  - Sagar Kadam <sagar.kadam@sifive.com>
> > +  - Paul Walmsley  <paul.walmsley@sifive.com>
> > +  - Palmer Dabbelt <palmer@dabbelt.com>
> > +
> > +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.
> > +
> > +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.
> > +
> > +    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.
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description:  Should contain 1 register range (address and length).
> 
> Drop this. The schema says this...
> 
> > +
> > +  '#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.
> 
> > +
> > +  '#interrupt-cells':
> > +    const: 1
> > +    description: Should be <1> or more.
> 
> Same here.
> 
Okay. 
Will remove as suggested above

Thanks & BR,
Sagar

> > +
> > +  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.
> > +
> > +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
> >

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

[Sagar Kadam]

Hello Rob,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, September 23, 2020 2:07 AM
> To: Sagar Kadam <sagar.kadam@openfive.com>
> 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) <paul.walmsley@sifive.com>; 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
> <yash.shah@openfive.com>
> Subject: Re: [PATCH v1 3/3] dt-bindings: riscv: convert pwm 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:04PM +0530, Sagar Kadam wrote:
> > Convert device tree bindings for SiFive's PWM controller to YAML
> > format.
> >
> > Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
> > ---
> >  .../devicetree/bindings/pwm/pwm-sifive.txt         | 33 ----------
> >  .../devicetree/bindings/pwm/pwm-sifive.yaml        | 72
> ++++++++++++++++++++++
> >  2 files changed, 72 insertions(+), 33 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> > b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> > deleted file mode 100644
> > index 3d1dd7b0..0000000
> > --- a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> > +++ /dev/null
> > @@ -1,33 +0,0 @@
> > -SiFive PWM controller
> > -
> > -Unlike most other PWM controllers, the SiFive PWM controller
> > currently only -supports one period for all channels in the PWM. All
> > PWMs need to run at -the same period. The period also has significant
> > restrictions on the values -it can achieve, which the driver rounds to the
> nearest achievable period.
> > -PWM RTL that corresponds to the IP block version numbers can be found
> > -here:
> > -
> > -https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/de
> > vices/pwm
> > -
> > -Required properties:
> > -- compatible: Should be "sifive,<chip>-pwm" and "sifive,pwm<version>".
> > -  Supported compatible strings are: "sifive,fu540-c000-pwm" for the
> > SiFive
> > -  PWM v0 as integrated onto the SiFive FU540 chip, and "sifive,pwm0"
> > for the
> > -  SiFive PWM v0 IP block with no chip integration tweaks.
> > -  Please refer to sifive-blocks-ip-versioning.txt for details.
> > -- reg: physical base address and length of the controller's registers
> > -- clocks: Should contain a clock identifier for the PWM's parent clock.
> > -- #pwm-cells: Should be 3. See pwm.yaml in this directory
> > -  for a description of the cell format.
> > -- interrupts: one interrupt per PWM channel
> > -
> > -Examples:
> > -
> > -pwm:  pwm@10020000 {
> > -     compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
> > -     reg = <0x0 0x10020000 0x0 0x1000>;
> > -     clocks = <&tlclk>;
> > -     interrupt-parent = <&plic>;
> > -     interrupts = <42 43 44 45>;
> > -     #pwm-cells = <3>;
> > -};
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> > b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> > new file mode 100644
> > index 0000000..415d053
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
> > +(C) 2020 SiFive, Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/pwm-sifive.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiFive PWM controller
> > +
> > +maintainers:
> > +  - Yash Shah <yash.shah@sifive.com>
> > +  - Sagar Kadam <sagar.kadam@sifive.com>
> > +  - Paul Walmsley <paul.walmsley@sifive.com>
> > +
> > +description:
> > +  Unlike most other PWM controllers, the SiFive PWM controller
> > +currently
> > +  only supports one period for all channels in the PWM. All PWMs need
> > +to
> > +  run at the same period. The period also has significant
> > +restrictions on
> > +  the values it can achieve, which the driver rounds to the nearest
> > +  achievable period. PWM RTL that corresponds to the IP block version
> > +  numbers can be found here -
> > +
> > +
> > + https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/d
> > + evices/pwm
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: sifive,fu540-c000-pwm
> > +      - const: sifive,pwm0
> > +    description:
> > +      Should be "sifive,<chip>-pwm" and "sifive,pwm<version>". Supported
> > +      compatible strings are "sifive,fu540-c000-pwm" for the SiFive PWM v0
> > +      as integrated onto the SiFive FU540 chip, and "sifive,pwm0" for the
> > +      SiFive PWM v0 IP block with no chip integration tweaks.
> > +      Please refer to sifive-blocks-ip-versioning.txt for details.
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Physical base address and length of the controller's
> > + registers
> 
> Drop description.
Okay.
> 
> > +
> > +  clocks:
> > +    description: Should contain a clock identifier for the PWM's parent
> clock.
> 
> How many clocks?
> 
PWM IP block instance is clocked with single clock (tlclk).
> > +
> > +  "#pwm-cells":
> > +    const: 3
> > +    description:
> > +      Should be 3. See pwm.yaml in this directory for a description of the
> > +      cell format.
> 
> Drop.
Okay, I will drop this description.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> Is it 1 or...
> 
> > +    description: One interrupt per PWM channel.
> 
> one per channel?
> 
Each PWM instance in FU540-C000 has 4 independent comparator's 
each capable of generating interrupts.  So maxItems need to be 4 and I can 
include it in description something like:
 " description: 
        Each PWM instance in FU540-C000 has 4 comparators. One interrupt per comparator" 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - "#pwm-cells"
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    pwm:  pwm@10020000 {
> > +      compatible = "sifive,fu540-c000-pwm", "sifive,pwm0";
> > +      reg = <0x10020000 0x1000>;
> > +      clocks = <&tlclk>;
> > +      interrupt-parent = <&plic>;
> > +      interrupts = <42 43 44 45>;
> 
> Split entries:
> 
> interrupts = <42>, <43>, <44>, <45>;
> 
Yes, I will split entries as suggested.

Thanks & BR,
Sagar
> > +      #pwm-cells = <3>;
> > +    };
> > --
> > 2.7.4
> >

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

[Geert Uytterhoeven]

Hi Sagar, Rob,

(replying to an old email, as this one seems to be the most appropriate)

On Tue, Sep 22, 2020 at 10:34 PM Rob Herring <robh@kernel.org> wrote:
> On Thu, Sep 10, 2020 at 04:14:03PM +0530, Sagar Kadam wrote:
> > Convert device tree bindings for SiFive's PLIC to YAML format
> >
> > Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml

> > +
> > +  '#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.

Why do we have the "'#address-cells': const: 0" at all...

> > +required:
> > +  - compatible
> > +  - '#address-cells'

... and why is it required?

> > +  - '#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>;
> > +    };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Rob Herring]

On Mon, Mar 22, 2021 at 9:38 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Sagar, Rob,
>
> (replying to an old email, as this one seems to be the most appropriate)
>
> On Tue, Sep 22, 2020 at 10:34 PM Rob Herring <robh@kernel.org> wrote:
> > On Thu, Sep 10, 2020 at 04:14:03PM +0530, Sagar Kadam wrote:
> > > Convert device tree bindings for SiFive's PLIC to YAML format
> > >
> > > Signed-off-by: Sagar Kadam <sagar.kadam@sifive.com>
>
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
>
> > > +
> > > +  '#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.
>
> Why do we have the "'#address-cells': const: 0" at all...
>
> > > +required:
> > > +  - compatible
> > > +  - '#address-cells'
>
> ... and why is it required?

It is only required if an 'interrupt-map' points to this node.
Currently dtc is warning if it is missing always, but there are plans
to relax dtc to only warn when 'interrupt-map' is present. Of course,
if you had 'interrupt-map' in an overlay, you'd want #address-cells in
the base dt and there's no other way to check that than making it
required.

Rob