[PATCH v4 08/11] bindings: spmi: spmi-pmic-arb: mark interrupt properties as optional

[PATCH v4 08/11] bindings: spmi: spmi-pmic-arb: mark interrupt properties as optional

[Fenglin Wu]

From: David Collins <collinsd@codeaurora.org>

Mark all interrupt related properties as optional instead of
required.  Some boards do not required PMIC IRQ support and it
isn't needed to handle SPMI bus transactions, so specify it as
optional.

Signed-off-by: David Collins <collinsd@codeaurora.org>
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
index ca645e2..6332507 100644
--- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
@@ -29,6 +29,8 @@ Required properties:
 - #size-cells : must be set to 0
 - qcom,ee : indicates the active Execution Environment identifier (0-5)
 - qcom,channel : which of the PMIC Arb provided channels to use for accesses (0-5)
+
+Optional properties:
 - interrupts : interrupt list for the PMIC Arb controller, must contain a
                single interrupt entry for the peripheral interrupt
 - interrupt-names : corresponding interrupt names for the interrupts
-- 
2.7.4

[PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Fenglin Wu]

Convert the SPMI PMIC arbiter documentation to JSON/yaml.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
 .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 +++++++++++++++++++++
 2 files changed, 146 insertions(+), 67 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
 create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml

diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
deleted file mode 100644
index 6332507..0000000
--- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Qualcomm SPMI Controller (PMIC Arbiter)
-
-The SPMI PMIC Arbiter is found on Snapdragon chipsets.  It is an SPMI
-controller with wrapping arbitration logic to allow for multiple on-chip
-devices to control a single SPMI master.
-
-The PMIC Arbiter can also act as an interrupt controller, providing interrupts
-to slave devices.
-
-See Documentation/devicetree/bindings/spmi/spmi.yaml for the generic SPMI
-controller binding requirements for child nodes.
-
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
-generic interrupt controller binding documentation.
-
-Required properties:
-- compatible : should be "qcom,spmi-pmic-arb".
-- reg-names  : must contain:
-     "core" - core registers
-     "intr" - interrupt controller registers
-     "cnfg" - configuration registers
-   Registers used only for V2 PMIC Arbiter:
-     "chnls"  - tx-channel per virtual slave registers.
-     "obsrvr" - rx-channel (called observer) per virtual slave registers.
-
-- reg : address + size pairs describing the PMIC arb register sets; order must
-        correspond with the order of entries in reg-names
-- #address-cells : must be set to 2
-- #size-cells : must be set to 0
-- qcom,ee : indicates the active Execution Environment identifier (0-5)
-- qcom,channel : which of the PMIC Arb provided channels to use for accesses (0-5)
-
-Optional properties:
-- interrupts : interrupt list for the PMIC Arb controller, must contain a
-               single interrupt entry for the peripheral interrupt
-- interrupt-names : corresponding interrupt names for the interrupts
-                    listed in the 'interrupts' property, must contain:
-     "periph_irq" - summary interrupt for PMIC peripherals
-- interrupt-controller : boolean indicator that the PMIC arbiter is an interrupt controller
-- #interrupt-cells :  must be set to 4. Interrupts are specified as a 4-tuple:
-    cell 1: slave ID for the requested interrupt (0-15)
-    cell 2: peripheral ID for requested interrupt (0-255)
-    cell 3: the requested peripheral interrupt (0-7)
-    cell 4: interrupt flags indicating level-sense information, as defined in
-            dt-bindings/interrupt-controller/irq.h
-
-Example:
-
-	spmi {
-		compatible = "qcom,spmi-pmic-arb";
-		reg-names = "core", "intr", "cnfg";
-		reg = <0xfc4cf000 0x1000>,
-		      <0xfc4cb000 0x1000>,
-		      <0xfc4ca000 0x1000>;
-
-		interrupt-names = "periph_irq";
-		interrupts = <0 190 0>;
-
-		qcom,ee = <0>;
-		qcom,channel = <0>;
-
-		#address-cells = <2>;
-		#size-cells = <0>;
-
-		interrupt-controller;
-		#interrupt-cells = <4>;
-	};
diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
new file mode 100644
index 0000000..df8cfb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spmi/qcom,spmi-pmic-arb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SPMI PMIC Arbiter
+
+maintainers:
+  - Fenglin Wu <quic_fenglinw@quicinc.com>
+  - Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
+
+description: |
+  The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI
+  controller with wrapping arbitration logic to allow for multiple
+  on-chip devices to control a single SPMI master.
+
+  The PMIC Arbiter can also act as an interrupt controller, providing
+  interrupts to slave devices.
+
+  See Documentation/devicetree/bindings/spmi/spmi.yaml for the generic
+  SPMI controller binding requirements for child nodes.
+
+allOf:
+  - $ref: spmi.yaml#
+
+properties:
+  $nodename:
+    pattern: "^spmi@.*"
+
+  compatible:
+    const: qcom,spmi-pmic-arb
+
+  reg-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    anyOf:
+      - minItems: 3
+      - maxItems: 3
+      - enum: ["core", "intr", "cnfg"]
+
+      - minItems: 5
+      - maxItems: 5
+      - enum: ["core", "intr", "cnfg", "chnls", "obsrvr"]
+
+  reg:
+    minItems: 3
+    maxItems: 5
+    description: |
+      Specifies base physical address and size of the registers in SPMI PMIC
+      Arbiter HW module, with the following order.
+        - SPMI PMIC arbiter core registers (core)
+        - SPMI PMIC arbiter interrupt controller registers (intr)
+        - SPMI PMIC arbiter configuration registers (cnfg)
+        - SPMI PMIC arbiter tx-channel per virtual slave registers (chnls)
+        - SPMI PMIC arbiter rx-channel per virtual slave registers (obsrvr).
+      Register for "chnls" and "obsrvr" are only applicable for PMIC arbiter
+      with HW version greater than V2.
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 0
+
+  interrupts:
+    description: The summary interrupt for the PMIC Arb controller.
+    maxItems: 1
+
+  interrupt-names:
+    const: periph_irq
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 4
+    description: |
+      Specifies the number of cells needed to encode any interrupt source.
+      The 1st cell is the slave ID for the requested interrupt, its valid
+      range is [0-15].
+      The 2nd cell is the  peripheral ID for requested interrupt, its valid
+      range is [0-255].
+      The 3rd cell is the requested peripheral interrupt, its valid range
+      is [0-7].
+      The 4th cell is interrupt flags indicating level-sense information,
+      as defined in dt-bindings/interrupt-controller/irq.h
+
+  qcom,ee:
+    description: the active Execution Environment identifier
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3, 4, 5]
+
+  qcom,channel:
+    description: which of the PMIC Arbiter provided channels to use for accesses
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3, 4, 5]
+
+patternProperties:
+  "@[0-9a-f]$":
+    description: up to 16 child PMIC nodes
+    type: object
+
+    properties:
+      reg:
+        items:
+          - minItems: 1
+            items:
+              - minimum: 0
+                maximum: 0xf
+              - enum: [ 0 ]
+                description:
+                  0 means user ID address. 1 is reserved for group ID
+                  address.
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg-names
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - qcom,ee
+  - qcom,channel
+
+additionalProperties: false
+
+examples:
+  - |
+    spmi@fc4cf000 {
+          compatible = "qcom,spmi-pmic-arb";
+          reg-names = "core", "intr", "cnfg";
+          reg = <0xfc4cf000 0x1000>,
+                <0xfc4cb000 0x1000>,
+                <0xfc4ca000 0x1000>;
+          interrupt-names = "periph_irq";
+          interrupts = <0 190 0>;
+          interrupt-controller;
+          #interrupt-cells = <4>;
+
+          qcom,ee = <0>;
+          qcom,channel = <0>;
+
+          #address-cells = <2>;
+          #size-cells = <0>;
+    };
-- 
2.7.4

Re: [PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Rob Herring]

On Tue, 21 Dec 2021 15:20:09 +0800, Fenglin Wu wrote:
> Convert the SPMI PMIC arbiter documentation to JSON/yaml.
> 
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
>  .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
>  .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 +++++++++++++++++++++
>  2 files changed, 146 insertions(+), 67 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>  create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.example.dt.yaml: spmi@fc4cf000: reg: [[4232900608, 4096], [4232884224, 4096], [4232880128, 4096]] is too long
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/spmi.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.example.dt.yaml: spmi@fc4cf000: reg: [[4232900608, 4096], [4232884224, 4096], [4232880128, 4096]] is too long
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml

doc reference errors (make refcheckdocs):
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt: Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt

See https://patchwork.ozlabs.org/patch/1571409

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Rob Herring]

On Tue, Dec 21, 2021 at 03:20:09PM +0800, Fenglin Wu wrote:
> Convert the SPMI PMIC arbiter documentation to JSON/yaml.
> 
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
>  .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
>  .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 +++++++++++++++++++++
>  2 files changed, 146 insertions(+), 67 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>  create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
> 

> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
> new file mode 100644
> index 0000000..df8cfb7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spmi/qcom,spmi-pmic-arb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SPMI PMIC Arbiter
> +
> +maintainers:
> +  - Fenglin Wu <quic_fenglinw@quicinc.com>
> +  - Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
> +
> +description: |
> +  The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI
> +  controller with wrapping arbitration logic to allow for multiple
> +  on-chip devices to control a single SPMI master.
> +
> +  The PMIC Arbiter can also act as an interrupt controller, providing
> +  interrupts to slave devices.
> +
> +  See Documentation/devicetree/bindings/spmi/spmi.yaml for the generic
> +  SPMI controller binding requirements for child nodes.
> +
> +allOf:
> +  - $ref: spmi.yaml#
> +
> +properties:
> +  $nodename:
> +    pattern: "^spmi@.*"
> +
> +  compatible:
> +    const: qcom,spmi-pmic-arb
> +
> +  reg-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array

reg-names already has a type defined.

> +    anyOf:
> +      - minItems: 3
> +      - maxItems: 3
> +      - enum: ["core", "intr", "cnfg"]
> +
> +      - minItems: 5
> +      - maxItems: 5
> +      - enum: ["core", "intr", "cnfg", "chnls", "obsrvr"]

I think you want something like this:

minItems: 3
items:
  - const: core
  - const: intr
  - const: cnfg
  - const: chnls
  - const: obsrvr


> +
> +  reg:
> +    minItems: 3
> +    maxItems: 5
> +    description: |
> +      Specifies base physical address and size of the registers in SPMI PMIC
> +      Arbiter HW module, with the following order.
> +        - SPMI PMIC arbiter core registers (core)
> +        - SPMI PMIC arbiter interrupt controller registers (intr)
> +        - SPMI PMIC arbiter configuration registers (cnfg)
> +        - SPMI PMIC arbiter tx-channel per virtual slave registers (chnls)
> +        - SPMI PMIC arbiter rx-channel per virtual slave registers (obsrvr).
> +      Register for "chnls" and "obsrvr" are only applicable for PMIC arbiter
> +      with HW version greater than V2.
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 0
> +
> +  interrupts:
> +    description: The summary interrupt for the PMIC Arb controller.
> +    maxItems: 1
> +
> +  interrupt-names:
> +    const: periph_irq
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 4
> +    description: |
> +      Specifies the number of cells needed to encode any interrupt source.
> +      The 1st cell is the slave ID for the requested interrupt, its valid
> +      range is [0-15].
> +      The 2nd cell is the  peripheral ID for requested interrupt, its valid
> +      range is [0-255].
> +      The 3rd cell is the requested peripheral interrupt, its valid range
> +      is [0-7].
> +      The 4th cell is interrupt flags indicating level-sense information,
> +      as defined in dt-bindings/interrupt-controller/irq.h
> +
> +  qcom,ee:
> +    description: the active Execution Environment identifier
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5]
> +
> +  qcom,channel:
> +    description: which of the PMIC Arbiter provided channels to use for accesses
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5]
> +

> +patternProperties:
> +  "@[0-9a-f]$":
> +    description: up to 16 child PMIC nodes
> +    type: object
> +
> +    properties:
> +      reg:
> +        items:
> +          - minItems: 1
> +            items:
> +              - minimum: 0
> +                maximum: 0xf
> +              - enum: [ 0 ]
> +                description:
> +                  0 means user ID address. 1 is reserved for group ID
> +                  address.
> +
> +    required:
> +      - reg

All this should be covered by spmi.yaml

> +
> +required:
> +  - compatible
> +  - reg-names
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - qcom,ee
> +  - qcom,channel
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spmi@fc4cf000 {
> +          compatible = "qcom,spmi-pmic-arb";
> +          reg-names = "core", "intr", "cnfg";
> +          reg = <0xfc4cf000 0x1000>,
> +                <0xfc4cb000 0x1000>,
> +                <0xfc4ca000 0x1000>;
> +          interrupt-names = "periph_irq";
> +          interrupts = <0 190 0>;
> +          interrupt-controller;
> +          #interrupt-cells = <4>;
> +
> +          qcom,ee = <0>;
> +          qcom,channel = <0>;
> +
> +          #address-cells = <2>;
> +          #size-cells = <0>;
> +    };
> -- 
> 2.7.4
> 
> 

Re: [PATCH v4 08/11] bindings: spmi: spmi-pmic-arb: mark interrupt properties as optional

[Rob Herring]

On Tue, 21 Dec 2021 15:20:06 +0800, Fenglin Wu wrote:
> From: David Collins <collinsd@codeaurora.org>
> 
> Mark all interrupt related properties as optional instead of
> required.  Some boards do not required PMIC IRQ support and it
> isn't needed to handle SPMI bus transactions, so specify it as
> optional.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
>  Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

Re: [PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Fenglin Wu]

On 2021/12/21 19:11, Rob Herring wrote:
> On Tue, 21 Dec 2021 15:20:09 +0800, Fenglin Wu wrote:
>> Convert the SPMI PMIC arbiter documentation to JSON/yaml.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>>   .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
>>   .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 +++++++++++++++++++++
>>   2 files changed, 146 insertions(+), 67 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>>   create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.example.dt.yaml: spmi@fc4cf000: reg: [[4232900608, 4096], [4232884224, 4096], [4232880128, 4096]] is too long
> 	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/spmi.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.example.dt.yaml: spmi@fc4cf000: reg: [[4232900608, 4096], [4232884224, 4096], [4232880128, 4096]] is too long
> 	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
I re-based the change on the tip of spmi-next project which has this 
change included:
https://lore.kernel.org/all/20211119034613.32489-2-james.lo@mediatek.com/
With it, the constraint should be removed and this warning/error won't 
be seen.
> doc reference errors (make refcheckdocs):
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt: Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>
> See https://patchwork.ozlabs.org/patch/1571409
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

Re: [PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Fenglin Wu]

resend with plain text


On 2021/12/21 22:42, Rob Herring wrote:
> On Tue, Dec 21, 2021 at 03:20:09PM +0800, Fenglin Wu wrote:
>> Convert the SPMI PMIC arbiter documentation to JSON/yaml.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>>   .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
>>   .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 +++++++++++++++++++++
>>   2 files changed, 146 insertions(+), 67 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>>   create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>>
> 
>> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>> new file mode 100644
>> index 0000000..df8cfb7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>> @@ -0,0 +1,146 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spmi/qcom,spmi-pmic-arb.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SPMI PMIC Arbiter
>> +
>> +maintainers:
>> +  - Fenglin Wu <quic_fenglinw@quicinc.com>
>> +  - Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
>> +
>> +description: |
>> +  The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI
>> +  controller with wrapping arbitration logic to allow for multiple
>> +  on-chip devices to control a single SPMI master.
>> +
>> +  The PMIC Arbiter can also act as an interrupt controller, providing
>> +  interrupts to slave devices.
>> +
>> +  See Documentation/devicetree/bindings/spmi/spmi.yaml for the generic
>> +  SPMI controller binding requirements for child nodes.
>> +
>> +allOf:
>> +  - $ref: spmi.yaml#
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^spmi@.*"
>> +
>> +  compatible:
>> +    const: qcom,spmi-pmic-arb
>> +
>> +  reg-names:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
> 
> reg-names already has a type defined.
I understand there is a pattern property defined in dt-core.yaml and it 
defines ".*-names" as a "non-unique-string-array" type. But here, the 
strings in "reg-names" needs to be unique and it has to be ["core", 
"intr", "cnfg"] or ["core", "intr", "cnfg", "chnls", "obsrvr"] , that's 
why I redefined it as "string-array" type which requires each string to 
be unique. Otherwise, if any dtsi nodes define the "reg-name" as 
["core", "core", "core"] will not be caught as a fault.

> 
>> +    anyOf:
>> +      - minItems: 3
>> +      - maxItems: 3
>> +      - enum: ["core", "intr", "cnfg"]
>> +
>> +      - minItems: 5
>> +      - maxItems: 5
>> +      - enum: ["core", "intr", "cnfg", "chnls", "obsrvr"]
> 
> I think you want something like this:
> 
> minItems: 3
> items:
>    - const: core
>    - const: intr
>    - const: cnfg
>    - const: chnls
>    - const: obsrvr
> 
> 
As I said, the content for "reg-names" here only has two options , 
either ["core", "intr", "cnfg"] or ["core", "intr", "cnfg", "chnls", 
"obsrvr"]. In patch V3, I defined it as below and "make dtbs_check" 
threw out warnings because some of existing nodes defined "reg-names" 
with these strings are not having the same order as I defined here (I 
understood from the warnings that const items need to be followed 
strictly even in order wise, is this correct?), and I guess the order of 
the strings doesn't matter here and the schema here shouldn't have such 
limitation, so I updated it as the "array-string" type and specified the 
tuples can only be one of the strings defined in the enum. With this, 
the previous warning regarding "reg-names" in "make dtbs_check" are all 
fixed.

   reg-names:
     oneOf:
       - items:
           - const: core
           - const: intr
           - const: cnfg
       - items:
           - const: core
           - const: intr
           - const: cnfg
           - const: chnls
           - const: obsrvr


>> +
>> +  reg:
>> +    minItems: 3
>> +    maxItems: 5
>> +    description: |
>> +      Specifies base physical address and size of the registers in SPMI PMIC
>> +      Arbiter HW module, with the following order.
>> +        - SPMI PMIC arbiter core registers (core)
>> +        - SPMI PMIC arbiter interrupt controller registers (intr)
>> +        - SPMI PMIC arbiter configuration registers (cnfg)
>> +        - SPMI PMIC arbiter tx-channel per virtual slave registers (chnls)
>> +        - SPMI PMIC arbiter rx-channel per virtual slave registers (obsrvr).
>> +      Register for "chnls" and "obsrvr" are only applicable for PMIC arbiter
>> +      with HW version greater than V2.
>> +
>> +  "#address-cells":
>> +    const: 2
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +  interrupts:
>> +    description: The summary interrupt for the PMIC Arb controller.
>> +    maxItems: 1
>> +
>> +  interrupt-names:
>> +    const: periph_irq
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 4
>> +    description: |
>> +      Specifies the number of cells needed to encode any interrupt source.
>> +      The 1st cell is the slave ID for the requested interrupt, its valid
>> +      range is [0-15].
>> +      The 2nd cell is the  peripheral ID for requested interrupt, its valid
>> +      range is [0-255].
>> +      The 3rd cell is the requested peripheral interrupt, its valid range
>> +      is [0-7].
>> +      The 4th cell is interrupt flags indicating level-sense information,
>> +      as defined in dt-bindings/interrupt-controller/irq.h
>> +
>> +  qcom,ee:
>> +    description: the active Execution Environment identifier
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2, 3, 4, 5]
>> +
>> +  qcom,channel:
>> +    description: which of the PMIC Arbiter provided channels to use for accesses
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2, 3, 4, 5]
>> +
> 
>> +patternProperties:
>> +  "@[0-9a-f]$":
>> +    description: up to 16 child PMIC nodes
>> +    type: object
>> +
>> +    properties:
>> +      reg:
>> +        items:
>> +          - minItems: 1
>> +            items:
>> +              - minimum: 0
>> +                maximum: 0xf
>> +              - enum: [ 0 ]
>> +                description:
>> +                  0 means user ID address. 1 is reserved for group ID
>> +                  address.
>> +
>> +    required:
>> +      - reg
> 
> All this should be covered by spmi.yaml
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg-names
>> +  - reg
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - qcom,ee
>> +  - qcom,channel
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    spmi@fc4cf000 {
>> +          compatible = "qcom,spmi-pmic-arb";
>> +          reg-names = "core", "intr", "cnfg";
>> +          reg = <0xfc4cf000 0x1000>,
>> +                <0xfc4cb000 0x1000>,
>> +                <0xfc4ca000 0x1000>;
>> +          interrupt-names = "periph_irq";
>> +          interrupts = <0 190 0>;
>> +          interrupt-controller;
>> +          #interrupt-cells = <4>;
>> +
>> +          qcom,ee = <0>;
>> +          qcom,channel = <0>;
>> +
>> +          #address-cells = <2>;
>> +          #size-cells = <0>;
>> +    };
>> -- 
>> 2.7.4
>>
>>

Re: [PATCH v4 08/11] bindings: spmi: spmi-pmic-arb: mark interrupt properties as optional

[Fenglin Wu]

On 2021/12/21 23:13, Rob Herring wrote:
> On Tue, 21 Dec 2021 15:20:06 +0800, Fenglin Wu wrote:
>> From: David Collins <collinsd@codeaurora.org>
>>
>> Mark all interrupt related properties as optional instead of
>> required.  Some boards do not required PMIC IRQ support and it
>> isn't needed to handle SPMI bus transactions, so specify it as
>> optional.
>>
>> Signed-off-by: David Collins <collinsd@codeaurora.org>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
> 
> 
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
> 
> If a tag was not added on purpose, please state why and what changed.
> 
I will remember to add the Acked-by/Reviewed-by tags next time when 
sending any update in this series. Thanks for the notice!

Re: [PATCH v4 11/11] dt-bindings: convert qcom,spmi-pmic-arb binding to YAML format

[Fenglin Wu]

On 2021/12/22 8:45, Fenglin Wu wrote:
> resend with plain text
> 
> 
> On 2021/12/21 22:42, Rob Herring wrote:
>> On Tue, Dec 21, 2021 at 03:20:09PM +0800, Fenglin Wu wrote:
>>> Convert the SPMI PMIC arbiter documentation to JSON/yaml.
>>>
>>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>>> ---
>>>   .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  67 ----------
>>>   .../bindings/spmi/qcom,spmi-pmic-arb.yaml          | 146 
>>> +++++++++++++++++++++
>>>   2 files changed, 146 insertions(+), 67 deletions(-)
>>>   delete mode 100644 
>>> Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>>>
>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml 
>>> b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>>> new file mode 100644
>>> index 0000000..df8cfb7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
>>> @@ -0,0 +1,146 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/spmi/qcom,spmi-pmic-arb.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm SPMI PMIC Arbiter
>>> +
>>> +maintainers:
>>> +  - Fenglin Wu <quic_fenglinw@quicinc.com>
>>> +  - Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
>>> +
>>> +description: |
>>> +  The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI
>>> +  controller with wrapping arbitration logic to allow for multiple
>>> +  on-chip devices to control a single SPMI master.
>>> +
>>> +  The PMIC Arbiter can also act as an interrupt controller, providing
>>> +  interrupts to slave devices.
>>> +
>>> +  See Documentation/devicetree/bindings/spmi/spmi.yaml for the generic
>>> +  SPMI controller binding requirements for child nodes.
>>> +
>>> +allOf:
>>> +  - $ref: spmi.yaml#
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^spmi@.*"
>>> +
>>> +  compatible:
>>> +    const: qcom,spmi-pmic-arb
>>> +
>>> +  reg-names:
>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>
>> reg-names already has a type defined.
> I understand there is a pattern property defined in dt-core.yaml and it 
> defines ".*-names" as a "non-unique-string-array" type. But here, the 
> strings in "reg-names" needs to be unique and it has to be ["core", 
> "intr", "cnfg"] or ["core", "intr", "cnfg", "chnls", "obsrvr"] , that's 
> why I redefined it as "string-array" type which requires each string to 
> be unique. Otherwise, if any dtsi nodes define the "reg-name" as 
> ["core", "core", "core"] will not be caught as a fault.
> 
>>
>>> +    anyOf:
>>> +      - minItems: 3
>>> +      - maxItems: 3
>>> +      - enum: ["core", "intr", "cnfg"]
>>> +
>>> +      - minItems: 5
>>> +      - maxItems: 5
>>> +      - enum: ["core", "intr", "cnfg", "chnls", "obsrvr"]
>>
>> I think you want something like this:
>>
>> minItems: 3
>> items:
>>    - const: core
>>    - const: intr
>>    - const: cnfg
>>    - const: chnls
>>    - const: obsrvr
>>
>>
> As I said, the content for "reg-names" here only has two options , 
> either ["core", "intr", "cnfg"] or ["core", "intr", "cnfg", "chnls", 
> "obsrvr"]. In patch V3, I defined it as below and "make dtbs_check" 
> threw out warnings because some of existing nodes defined "reg-names" 
> with these strings are not having the same order as I defined here (I 
> understood from the warnings that const items need to be followed 
> strictly even in order wise, is this correct?), and I guess the order of 
> the strings doesn't matter here and the schema here shouldn't have such 
> limitation, so I updated it as the "array-string" type and specified the 
> tuples can only be one of the strings defined in the enum. With this, 
> the previous warning regarding "reg-names" in "make dtbs_check" are all 
> fixed.
> 
>    reg-names:
>      oneOf:
>        - items:
>            - const: core
>            - const: intr
>            - const: cnfg
>        - items:
>            - const: core
>            - const: intr
>            - const: cnfg
>            - const: chnls
>            - const: obsrvr
>
Can you help to confirm if I need to change this back to what has been 
defined in PATCH v3 but just ignore those "make dtbs_check" warnings?
Thanks

> 
>>> +
>>> +  reg:
>>> +    minItems: 3
>>> +    maxItems: 5
>>> +    description: |
>>> +      Specifies base physical address and size of the registers in 
>>> SPMI PMIC
>>> +      Arbiter HW module, with the following order.
>>> +        - SPMI PMIC arbiter core registers (core)
>>> +        - SPMI PMIC arbiter interrupt controller registers (intr)
>>> +        - SPMI PMIC arbiter configuration registers (cnfg)
>>> +        - SPMI PMIC arbiter tx-channel per virtual slave registers 
>>> (chnls)
>>> +        - SPMI PMIC arbiter rx-channel per virtual slave registers 
>>> (obsrvr).
>>> +      Register for "chnls" and "obsrvr" are only applicable for PMIC 
>>> arbiter
>>> +      with HW version greater than V2.
>>> +
>>> +  "#address-cells":
>>> +    const: 2
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +
>>> +  interrupts:
>>> +    description: The summary interrupt for the PMIC Arb controller.
>>> +    maxItems: 1
>>> +
>>> +  interrupt-names:
>>> +    const: periph_irq
>>> +
>>> +  interrupt-controller: true
>>> +
>>> +  "#interrupt-cells":
>>> +    const: 4
>>> +    description: |
>>> +      Specifies the number of cells needed to encode any interrupt 
>>> source.
>>> +      The 1st cell is the slave ID for the requested interrupt, its 
>>> valid
>>> +      range is [0-15].
>>> +      The 2nd cell is the  peripheral ID for requested interrupt, 
>>> its valid
>>> +      range is [0-255].
>>> +      The 3rd cell is the requested peripheral interrupt, its valid 
>>> range
>>> +      is [0-7].
>>> +      The 4th cell is interrupt flags indicating level-sense 
>>> information,
>>> +      as defined in dt-bindings/interrupt-controller/irq.h
>>> +
>>> +  qcom,ee:
>>> +    description: the active Execution Environment identifier
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2, 3, 4, 5]
>>> +
>>> +  qcom,channel:
>>> +    description: which of the PMIC Arbiter provided channels to use 
>>> for accesses
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2, 3, 4, 5]
>>> +
>>
>>> +patternProperties:
>>> +  "@[0-9a-f]$":
>>> +    description: up to 16 child PMIC nodes
>>> +    type: object
>>> +
>>> +    properties:
>>> +      reg:
>>> +        items:
>>> +          - minItems: 1
>>> +            items:
>>> +              - minimum: 0
>>> +                maximum: 0xf
>>> +              - enum: [ 0 ]
>>> +                description:
>>> +                  0 means user ID address. 1 is reserved for group ID
>>> +                  address.
>>> +
>>> +    required:
>>> +      - reg
>>
>> All this should be covered by spmi.yaml
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg-names
>>> +  - reg
>>> +  - "#address-cells"
>>> +  - "#size-cells"
>>> +  - qcom,ee
>>> +  - qcom,channel
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    spmi@fc4cf000 {
>>> +          compatible = "qcom,spmi-pmic-arb";
>>> +          reg-names = "core", "intr", "cnfg";
>>> +          reg = <0xfc4cf000 0x1000>,
>>> +                <0xfc4cb000 0x1000>,
>>> +                <0xfc4ca000 0x1000>;
>>> +          interrupt-names = "periph_irq";
>>> +          interrupts = <0 190 0>;
>>> +          interrupt-controller;
>>> +          #interrupt-cells = <4>;
>>> +
>>> +          qcom,ee = <0>;
>>> +          qcom,channel = <0>;
>>> +
>>> +          #address-cells = <2>;
>>> +          #size-cells = <0>;
>>> +    };
>>> -- 
>>> 2.7.4
>>>
>>>