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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 31/03/2022 19:58, Kuldeep Singh wrote:
> On Thu, Mar 31, 2022 at 06:02:47PM +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>
>>
>> ---
>>
>> Changes since v1:
>> 1. Correct $ref path and remove interconnect minItems (Kuldeep).
>> 2. Remove child tpm device from example.
>> 3. Pad reg hex addresses with 00.
>> ---
>>  .../bindings/spi/qcom,spi-geni-qcom.txt       |  39 ------
>>  .../bindings/spi/qcom,spi-geni-qcom.yaml      | 120 ++++++++++++++++++
>>  2 files changed, 120 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..62c4a9598e16
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml
>> @@ -0,0 +1,120 @@
>> +# 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: /schemas/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:
>> +    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
> 
> Why reg-names is required?
> Reg contain max 1 value, we can skip reg-names like other users.
> 
> Also, "se" is used as clock name and using it again for reg-names?
> I think this is wrong and reg-names shouldn't be documented.

reg-names are not required. If you ask why they are documented? As I
wrote in commit msg - bindings were not fully updated to DTSes being used.

Best regards,
Krzysztof