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

[Kuldeep Singh <singh.kuldeep87k@gmail.com>]

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
>