[PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Krzysztof Kozlowski]

Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
Peripheral Interface (SPI) bindings to DT Schema.

The original bindings in TXT were not complete, so add during conversion
properties already used in DTS and/or in the driver: reg-names, dmas,
interconnects, operating points and power-domains.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
 .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
 2 files changed, 131 insertions(+), 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
 create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml

diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
deleted file mode 100644
index c8c1e913f4e7..000000000000
--- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
-
-The QUP v3 core is a GENI based AHB slave that provides a common data path
-(an output FIFO and an input FIFO) for serial peripheral interface (SPI)
-mini-core.
-
-SPI in master mode supports up to 50MHz, up to four chip selects, programmable
-data path from 4 bits to 32 bits and numerous protocol variants.
-
-Required properties:
-- compatible:		Must contain "qcom,geni-spi".
-- reg:			Must contain SPI register location and length.
-- interrupts:		Must contain SPI controller interrupts.
-- clock-names:		Must contain "se".
-- clocks:		Serial engine core clock needed by the device.
-- #address-cells:	Must be <1> to define a chip select address on
-			the SPI bus.
-- #size-cells:		Must be <0>.
-
-SPI Controller nodes must be child of GENI based Qualcomm Universal
-Peripharal. Please refer GENI based QUP wrapper controller node bindings
-described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
-
-SPI slave nodes must be children of the SPI master node and conform to SPI bus
-binding as described in Documentation/devicetree/bindings/spi/spi-bus.txt.
-
-Example:
-	spi0: spi@a84000 {
-		compatible = "qcom,geni-spi";
-		reg = <0xa84000 0x4000>;
-		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
-		clock-names = "se";
-		clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
-		pinctrl-names = "default", "sleep";
-		pinctrl-0 = <&qup_1_spi_2_active>;
-		pinctrl-1 = <&qup_1_spi_2_sleep>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-	};
diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
new file mode 100644
index 000000000000..a85ff02ba1db
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/qcom,spi-geni-qcom.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
+
+maintainers:
+  - Andy Gross <agross@kernel.org>
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
+
+description:
+  The QUP v3 core is a GENI based AHB slave that provides a common data path
+  (an output FIFO and an input FIFO) for serial peripheral interface (SPI)
+  mini-core.
+
+  SPI in master mode supports up to 50MHz, up to four chip selects,
+  programmable data path from 4 bits to 32 bits and numerous protocol variants.
+
+  SPI Controller nodes must be child of GENI based Qualcomm Universal
+  Peripharal. Please refer GENI based QUP wrapper controller node bindings
+  described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
+
+allOf:
+  - $ref: /spi/spi-controller.yaml#
+
+properties:
+  compatible:
+    const: qcom,geni-spi
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: se
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+  interconnects:
+    minItems: 2
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: qup-core
+      - const: qup-config
+
+  interrupts:
+    maxItems: 1
+
+  operating-points-v2: true
+
+  power-domains:
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  reg-names:
+    const: se
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - interrupts
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+    #include <dt-bindings/interconnect/qcom,sc7180.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    spi@880000 {
+        compatible = "qcom,geni-spi";
+        reg = <0x880000 0x4000>;
+        clock-names = "se";
+        clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&qup_spi0_default>;
+        interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        power-domains = <&rpmhpd SC7180_CX>;
+        operating-points-v2 = <&qup_opp_table>;
+        interconnects = <&qup_virt MASTER_QUP_CORE_0 0 &qup_virt SLAVE_QUP_CORE_0 0>,
+                        <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
+        interconnect-names = "qup-core", "qup-config";
+
+        tpm@0 {
+            compatible = "google,cr50";
+            reg = <0>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&h1_ap_int_odl>;
+            spi-max-frequency = <800000>;
+            interrupt-parent = <&tlmm>;
+            interrupts = <42 IRQ_TYPE_EDGE_RISING>;
+        };
+    };
+
+  - |
+    #include <dt-bindings/dma/qcom-gpi.h>
+
+    spi@884000 {
+        compatible = "qcom,geni-spi";
+        reg = <0x884000 0x4000>;
+        reg-names = "se";
+        clock-names = "se";
+        clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>;
+        dmas = <&gpi_dma0 0 1 QCOM_GPI_SPI>,
+               <&gpi_dma0 1 1 QCOM_GPI_SPI>;
+        dma-names = "tx", "rx";
+        pinctrl-names = "default";
+        pinctrl-0 = <&qup_spi1_default>;
+        interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
+        spi-max-frequency = <50000000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
-- 
2.32.0

[PATCH 2/2] dt-bindings: qcom: qcom,geni-se: refer to dtschema for SPI

[Krzysztof Kozlowski]

After adding DT schema for the SPI controller, the Qualcomm GENI Serial
Engine QUP Wrapper Controller bindings can reference it directly for
full schema validation.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/soc/qcom/qcom,geni-se.yaml       | 22 +------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
index a776cd37c297..1ca081b1664e 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
@@ -103,27 +103,7 @@ patternProperties:
                  supports up to 50MHz, up to four chip selects, programmable
                  data path from 4 bits to 32 bits and numerous protocol
                  variants.
-    $ref: /spi/spi-controller.yaml#
-
-    properties:
-      compatible:
-        enum:
-          - qcom,geni-spi
-
-      interrupts:
-        maxItems: 1
-
-      "#address-cells":
-        const: 1
-
-      "#size-cells":
-        const: 0
-
-    required:
-      - compatible
-      - interrupts
-      - "#address-cells"
-      - "#size-cells"
+    $ref: /schemas/spi/qcom,spi-geni-qcom.yaml#
 
   "i2c@[0-9a-f]+$":
     type: object
-- 
2.32.0

Re: [PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Rob Herring]

On Tue, 29 Mar 2022 13:27:16 +0200, Krzysztof Kozlowski wrote:
> Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
> Peripheral Interface (SPI) bindings to DT Schema.
> 
> The original bindings in TXT were not complete, so add during conversion
> properties already used in DTS and/or in the driver: reg-names, dmas,
> interconnects, operating points and power-domains.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
>  2 files changed, 131 insertions(+), 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>  create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.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:
Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.example.dt.yaml:0:0: /example-0/spi@880000/tpm@0: failed to match any schema with compatible: ['google,cr50']

doc reference errors (make refcheckdocs):

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

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 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Krzysztof Kozlowski]

On 29/03/2022 21:05, Rob Herring wrote:
> On Tue, 29 Mar 2022 13:27:16 +0200, Krzysztof Kozlowski wrote:
>> Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
>> Peripheral Interface (SPI) bindings to DT Schema.
>>
>> The original bindings in TXT were not complete, so add during conversion
>> properties already used in DTS and/or in the driver: reg-names, dmas,
>> interconnects, operating points and power-domains.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
>>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
>>  2 files changed, 131 insertions(+), 39 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>>  create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.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:
> Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.example.dt.yaml:0:0: /example-0/spi@880000/tpm@0: failed to match any schema with compatible: ['google,cr50']
> 

I will try to find some better example for the child device (or skip it?).


Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Kuldeep Singh]

On Tue, Mar 29, 2022 at 01:27:16PM +0200, Krzysztof Kozlowski wrote:
> Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
> Peripheral Interface (SPI) bindings to DT Schema.
> 
> The original bindings in TXT were not complete, so add during conversion
> properties already used in DTS and/or in the driver: reg-names, dmas,
> interconnects, operating points and power-domains.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
>  2 files changed, 131 insertions(+), 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>  create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
> deleted file mode 100644
> index c8c1e913f4e7..000000000000
> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
> -
> -The QUP v3 core is a GENI based AHB slave that provides a common data path
> -(an output FIFO and an input FIFO) for serial peripheral interface (SPI)
> -mini-core.
> -
> -SPI in master mode supports up to 50MHz, up to four chip selects, programmable
> -data path from 4 bits to 32 bits and numerous protocol variants.
> -
> -Required properties:
> -- compatible:		Must contain "qcom,geni-spi".
> -- reg:			Must contain SPI register location and length.
> -- interrupts:		Must contain SPI controller interrupts.
> -- clock-names:		Must contain "se".
> -- clocks:		Serial engine core clock needed by the device.
> -- #address-cells:	Must be <1> to define a chip select address on
> -			the SPI bus.
> -- #size-cells:		Must be <0>.
> -
> -SPI Controller nodes must be child of GENI based Qualcomm Universal
> -Peripharal. Please refer GENI based QUP wrapper controller node bindings
> -described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> -
> -SPI slave nodes must be children of the SPI master node and conform to SPI bus
> -binding as described in Documentation/devicetree/bindings/spi/spi-bus.txt.
> -
> -Example:
> -	spi0: spi@a84000 {
> -		compatible = "qcom,geni-spi";
> -		reg = <0xa84000 0x4000>;
> -		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
> -		clock-names = "se";
> -		clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
> -		pinctrl-names = "default", "sleep";
> -		pinctrl-0 = <&qup_1_spi_2_active>;
> -		pinctrl-1 = <&qup_1_spi_2_sleep>;
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -	};
> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> new file mode 100644
> index 000000000000..a85ff02ba1db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/qcom,spi-geni-qcom.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
> +
> +maintainers:
> +  - Andy Gross <agross@kernel.org>
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> +
> +description:
> +  The QUP v3 core is a GENI based AHB slave that provides a common data path
> +  (an output FIFO and an input FIFO) for serial peripheral interface (SPI)
> +  mini-core.
> +
> +  SPI in master mode supports up to 50MHz, up to four chip selects,
> +  programmable data path from 4 bits to 32 bits and numerous protocol variants.
> +
> +  SPI Controller nodes must be child of GENI based Qualcomm Universal
> +  Peripharal. Please refer GENI based QUP wrapper controller node bindings
> +  described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> +
> +allOf:
> +  - $ref: /spi/spi-controller.yaml#

Rob sometime back sent an update on how to refer absolute paths.
Please see below:
https://lore.kernel.org/linux-spi/20220325215652.525383-1-robh@kernel.org/

> +
> +properties:
> +  compatible:
> +    const: qcom,geni-spi
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: se
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  interconnects:
> +    minItems: 2

We can skip minItems here.
As minimim value defaults to maximum if not defined.

> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: qup-core
> +      - const: qup-config

Some properties like clocks, dmas, dma-names, interconnect etc. are
defined as common child properties of geni based qup.
Please see Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml

Shouldn't we skip these entities here? as spi reference will anyway be
used in geni-se.yaml.

-Kuldeep

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  reg-names:
> +    const: se
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
> +    #include <dt-bindings/interconnect/qcom,sc7180.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +
> +    spi@880000 {
> +        compatible = "qcom,geni-spi";
> +        reg = <0x880000 0x4000>;
> +        clock-names = "se";
> +        clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&qup_spi0_default>;
> +        interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        power-domains = <&rpmhpd SC7180_CX>;
> +        operating-points-v2 = <&qup_opp_table>;
> +        interconnects = <&qup_virt MASTER_QUP_CORE_0 0 &qup_virt SLAVE_QUP_CORE_0 0>,
> +                        <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
> +        interconnect-names = "qup-core", "qup-config";
> +
> +        tpm@0 {
> +            compatible = "google,cr50";
> +            reg = <0>;
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&h1_ap_int_odl>;
> +            spi-max-frequency = <800000>;
> +            interrupt-parent = <&tlmm>;
> +            interrupts = <42 IRQ_TYPE_EDGE_RISING>;
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/dma/qcom-gpi.h>
> +
> +    spi@884000 {
> +        compatible = "qcom,geni-spi";
> +        reg = <0x884000 0x4000>;
> +        reg-names = "se";
> +        clock-names = "se";
> +        clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>;
> +        dmas = <&gpi_dma0 0 1 QCOM_GPI_SPI>,
> +               <&gpi_dma0 1 1 QCOM_GPI_SPI>;
> +        dma-names = "tx", "rx";
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&qup_spi1_default>;
> +        interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
> +        spi-max-frequency = <50000000>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +    };
> -- 
> 2.32.0
> 

Re: [PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Krzysztof Kozlowski]

On 30/03/2022 08:23, Kuldeep Singh wrote:
> On Tue, Mar 29, 2022 at 01:27:16PM +0200, Krzysztof Kozlowski wrote:
>> Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
>> Peripheral Interface (SPI) bindings to DT Schema.
>>
>> The original bindings in TXT were not complete, so add during conversion
>> properties already used in DTS and/or in the driver: reg-names, dmas,
>> interconnects, operating points and power-domains.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
>>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
>>  2 files changed, 131 insertions(+), 39 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>>  create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>> deleted file mode 100644
>> index c8c1e913f4e7..000000000000
>> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
>> +++ /dev/null
>> @@ -1,39 +0,0 @@
>> -GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
>> -
>> -The QUP v3 core is a GENI based AHB slave that provides a common data path
>> -(an output FIFO and an input FIFO) for serial peripheral interface (SPI)
>> -mini-core.
>> -
>> -SPI in master mode supports up to 50MHz, up to four chip selects, programmable
>> -data path from 4 bits to 32 bits and numerous protocol variants.
>> -
>> -Required properties:
>> -- compatible:		Must contain "qcom,geni-spi".
>> -- reg:			Must contain SPI register location and length.
>> -- interrupts:		Must contain SPI controller interrupts.
>> -- clock-names:		Must contain "se".
>> -- clocks:		Serial engine core clock needed by the device.
>> -- #address-cells:	Must be <1> to define a chip select address on
>> -			the SPI bus.
>> -- #size-cells:		Must be <0>.
>> -
>> -SPI Controller nodes must be child of GENI based Qualcomm Universal
>> -Peripharal. Please refer GENI based QUP wrapper controller node bindings
>> -described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
>> -
>> -SPI slave nodes must be children of the SPI master node and conform to SPI bus
>> -binding as described in Documentation/devicetree/bindings/spi/spi-bus.txt.
>> -
>> -Example:
>> -	spi0: spi@a84000 {
>> -		compatible = "qcom,geni-spi";
>> -		reg = <0xa84000 0x4000>;
>> -		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
>> -		clock-names = "se";
>> -		clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
>> -		pinctrl-names = "default", "sleep";
>> -		pinctrl-0 = <&qup_1_spi_2_active>;
>> -		pinctrl-1 = <&qup_1_spi_2_sleep>;
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>> -	};
>> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> new file mode 100644
>> index 000000000000..a85ff02ba1db
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> @@ -0,0 +1,131 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/qcom,spi-geni-qcom.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
>> +
>> +maintainers:
>> +  - Andy Gross <agross@kernel.org>
>> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
>> +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> +
>> +description:
>> +  The QUP v3 core is a GENI based AHB slave that provides a common data path
>> +  (an output FIFO and an input FIFO) for serial peripheral interface (SPI)
>> +  mini-core.
>> +
>> +  SPI in master mode supports up to 50MHz, up to four chip selects,
>> +  programmable data path from 4 bits to 32 bits and numerous protocol variants.
>> +
>> +  SPI Controller nodes must be child of GENI based Qualcomm Universal
>> +  Peripharal. Please refer GENI based QUP wrapper controller node bindings
>> +  described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
>> +
>> +allOf:
>> +  - $ref: /spi/spi-controller.yaml#
> 
> Rob sometime back sent an update on how to refer absolute paths.
> Please see below:
> https://lore.kernel.org/linux-spi/20220325215652.525383-1-robh@kernel.org/

Yes, this is wrong. I copied other existing schema without checking. :(

> 
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,geni-spi
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    const: se
>> +
>> +  dmas:
>> +    maxItems: 2
>> +
>> +  dma-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +  interconnects:
>> +    minItems: 2
> 
> We can skip minItems here.
> As minimim value defaults to maximum if not defined.

True.

> 
>> +    maxItems: 2
>> +
>> +  interconnect-names:
>> +    items:
>> +      - const: qup-core
>> +      - const: qup-config
> 
> Some properties like clocks, dmas, dma-names, interconnect etc. are
> defined as common child properties of geni based qup.
> Please see Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
> 
> Shouldn't we skip these entities here? as spi reference will anyway be
> used in geni-se.yaml.

We could have them there, just like we could store all of this schema
there. Having something half-here-half-there will not work, because this
schema won't validate.

Therefore all of child properties from qcom,geni-se.yaml should be
rather moved to child schema (which is included directly in my patch #2).


Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: convert to dtschema

[Kuldeep Singh]

On Wed, Mar 30, 2022 at 08:48:38AM +0200, Krzysztof Kozlowski wrote:
> On 30/03/2022 08:23, Kuldeep Singh wrote:
> > On Tue, Mar 29, 2022 at 01:27:16PM +0200, Krzysztof Kozlowski wrote:
> >> Convert the GENI based Qualcomm Universal Peripheral (QUP) Serial
> >> Peripheral Interface (SPI) bindings to DT Schema.
> >>
> >> The original bindings in TXT were not complete, so add during conversion
> >> properties already used in DTS and/or in the driver: reg-names, dmas,
> >> interconnects, operating points and power-domains.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
> >>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 131 ++++++++++++++++++
> >>  2 files changed, 131 insertions(+), 39 deletions(-)
> >>  delete mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
> >>  create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
> >> deleted file mode 100644
> >> index c8c1e913f4e7..000000000000
> >> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.txt
> >> +++ /dev/null
> >> @@ -1,39 +0,0 @@
> >> -GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
> >> -
> >> -The QUP v3 core is a GENI based AHB slave that provides a common data path
> >> -(an output FIFO and an input FIFO) for serial peripheral interface (SPI)
> >> -mini-core.
> >> -
> >> -SPI in master mode supports up to 50MHz, up to four chip selects, programmable
> >> -data path from 4 bits to 32 bits and numerous protocol variants.
> >> -
> >> -Required properties:
> >> -- compatible:		Must contain "qcom,geni-spi".
> >> -- reg:			Must contain SPI register location and length.
> >> -- interrupts:		Must contain SPI controller interrupts.
> >> -- clock-names:		Must contain "se".
> >> -- clocks:		Serial engine core clock needed by the device.
> >> -- #address-cells:	Must be <1> to define a chip select address on
> >> -			the SPI bus.
> >> -- #size-cells:		Must be <0>.
> >> -
> >> -SPI Controller nodes must be child of GENI based Qualcomm Universal
> >> -Peripharal. Please refer GENI based QUP wrapper controller node bindings
> >> -described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> >> -
> >> -SPI slave nodes must be children of the SPI master node and conform to SPI bus
> >> -binding as described in Documentation/devicetree/bindings/spi/spi-bus.txt.
> >> -
> >> -Example:
> >> -	spi0: spi@a84000 {
> >> -		compatible = "qcom,geni-spi";
> >> -		reg = <0xa84000 0x4000>;
> >> -		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
> >> -		clock-names = "se";
> >> -		clocks = <&clock_gcc GCC_QUPV3_WRAP0_S0_CLK>;
> >> -		pinctrl-names = "default", "sleep";
> >> -		pinctrl-0 = <&qup_1_spi_2_active>;
> >> -		pinctrl-1 = <&qup_1_spi_2_sleep>;
> >> -		#address-cells = <1>;
> >> -		#size-cells = <0>;
> >> -	};
> >> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> >> new file mode 100644
> >> index 000000000000..a85ff02ba1db
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
> >> @@ -0,0 +1,131 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/spi/qcom,spi-geni-qcom.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: GENI based Qualcomm Universal Peripheral (QUP) Serial Peripheral Interface (SPI)
> >> +
> >> +maintainers:
> >> +  - Andy Gross <agross@kernel.org>
> >> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> >> +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> +
> >> +description:
> >> +  The QUP v3 core is a GENI based AHB slave that provides a common data path
> >> +  (an output FIFO and an input FIFO) for serial peripheral interface (SPI)
> >> +  mini-core.
> >> +
> >> +  SPI in master mode supports up to 50MHz, up to four chip selects,
> >> +  programmable data path from 4 bits to 32 bits and numerous protocol variants.
> >> +
> >> +  SPI Controller nodes must be child of GENI based Qualcomm Universal
> >> +  Peripharal. Please refer GENI based QUP wrapper controller node bindings
> >> +  described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> >> +
> >> +allOf:
> >> +  - $ref: /spi/spi-controller.yaml#
> > 
> > Rob sometime back sent an update on how to refer absolute paths.
> > Please see below:
> > https://lore.kernel.org/linux-spi/20220325215652.525383-1-robh@kernel.org/
> 
> Yes, this is wrong. I copied other existing schema without checking. :(

No issues.

> 
> > 
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: qcom,geni-spi
> >> +
> >> +  clocks:
> >> +    maxItems: 1
> >> +
> >> +  clock-names:
> >> +    const: se
> >> +
> >> +  dmas:
> >> +    maxItems: 2
> >> +
> >> +  dma-names:
> >> +    items:
> >> +      - const: tx
> >> +      - const: rx
> >> +
> >> +  interconnects:
> >> +    minItems: 2
> > 
> > We can skip minItems here.
> > As minimim value defaults to maximum if not defined.
> 
> True.
> 
> > 
> >> +    maxItems: 2
> >> +
> >> +  interconnect-names:
> >> +    items:
> >> +      - const: qup-core
> >> +      - const: qup-config
> > 
> > Some properties like clocks, dmas, dma-names, interconnect etc. are
> > defined as common child properties of geni based qup.
> > Please see Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
> > 
> > Shouldn't we skip these entities here? as spi reference will anyway be
> > used in geni-se.yaml.
> 
> We could have them there, just like we could store all of this schema
> there. Having something half-here-half-there will not work, because this
> schema won't validate.
> 
> Therefore all of child properties from qcom,geni-se.yaml should be
> rather moved to child schema (which is included directly in my patch #2).

Sure, this looks good.
Probably we can remove common properties from geni-se.yaml once all
child nodes have their respective schemas.

-Kuldeep