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 <[email protected]>
>>
>> ---
>>
>> 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 <[email protected]>
>> + - Bjorn Andersson <[email protected]>
>> + - Krzysztof Kozlowski <[email protected]>
>> +
>> +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
On Thu, Mar 31, 2022 at 09:40:08PM +0200, Krzysztof Kozlowski wrote:
> 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 <[email protected]>
[...]
> >> + 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.
Hi Krzysztof,
Power-domains, operating-points etc. makes sense, but reg-names?
With quick search, sm8150.dtsi is the only user of reg-names. Moreover,
there is no reference in driver to incorporate it.
Driver(drivers/spi/spi-geni-qcom.c) looks for "se" as clock name and not
as reg-name. If there's one, kindly help in pointing out.
It seems you are adding reg-names because some DT is using it and no
other reason. And with no reference even in driver, that's surely not a
way to go ahead as you have been advising the same in past.
-Kuldeep