On Fri, Jul 21, 2023 at 04:33:20PM +0530, Mrinmay Sarkar wrote:

On 7/17/2023 12:55 PM, Krzysztof Kozlowski wrote:

quoted

On 14/07/2023 07:08, Mrinmay Sarkar wrote:

quoted

Add devicetree YAML binding for Qualcomm QMP PCIe PHY
for SA8775p platform.

Signed-off-by: Mrinmay Sarkar <redacted>
---
  .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml      | 19 ++++++++++++++++++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
index a0407fc..ca55ed9 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml

@@ -16,6 +16,8 @@ description:
  properties:
    compatible:
      enum:
+      - qcom,sa8775p-qmp-gen4x2-pcie-phy
+      - qcom,sa8775p-qmp-gen4x4-pcie-phy
        - qcom,sc8280xp-qmp-gen3x1-pcie-phy
        - qcom,sc8280xp-qmp-gen3x2-pcie-phy
        - qcom,sc8280xp-qmp-gen3x4-pcie-phy

@@ -30,7 +32,7 @@ properties:
    clocks:
      minItems: 5
-    maxItems: 6
+    maxItems: 7
    clock-names:
      minItems: 5

@@ -41,6 +43,7 @@ properties:
        - const: rchng
        - const: pipe
        - const: pipediv2
+      - const: phy_aux
    power-domains:
      maxItems: 1

@@ -141,6 +144,20 @@ allOf:
          compatible:
            contains:
              enum:
+              - qcom,sa8775p-qmp-gen4x2-pcie-phy
+              - qcom,sa8775p-qmp-gen4x4-pcie-phy
+    then:
+      properties:
+        clocks:
+          minItems: 7
+        clock-names:
+          minItems: 7
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:

This probably works but is not obvious and easy to read. You have here
if:then:else: block, so else applies to your variant. Change all these
if clauses for clocks into separate clauses per matching variant
(if:then: ... if:then:... if:then:...)

As far as I can tell, this actually doesn't work :(

quoted

Best regards,
Krzysztof

My Bad here, This patch already applied we will take care this in next patch
set.

Thanks,
Mrinmay

Mrinmay, do you plan on spinning what Krzysztof suggested? I grabbed
linux-next today and ran into this (looks like clocks, clock-names in
binding is broken and looks like we're either missing the required
power-domain in the dts or it isn't actually required):

    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % git diff
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=1 DT_SCHEMA_FILES=phy/qcom,sc8280xp-qmp-pcie-phy.yaml qcom/sa8775p-ride.dtb
      UPD     include/config/kernel.release
      LINT    Documentation/devicetree/bindings
      CHKDT   Documentation/devicetree/bindings/processed-schema.json
      SCHEMA  Documentation/devicetree/bindings/processed-schema.json
    /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml: ignoring, error parsing file
      DTC_CHK arch/arm64/boot/dts/qcom/sa8775p-ride.dtb
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c04000: 'power-domains' is a required property
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c04000: clocks: [[31, 66], [31, 68], [31, 94], [31, 72], [31, 74], [31, 77], [31, 70]] is too long
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c04000: clock-names: ['aux', 'cfg_ahb', 'ref', 'rchng', 'pipe', 'pipediv2', 'phy_aux'] is too long
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c14000: 'power-domains' is a required property
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c14000: clocks: [[31, 80], [31, 82], [31, 94], [31, 86], [31, 88], [31, 91], [31, 84]] is too long
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    /home/ahalaney/git/linux-next/arch/arm64/boot/dts/qcom/sa8775p-ride.dtb: phy@1c14000: clock-names: ['aux', 'cfg_ahb', 'ref', 'rchng', 'pipe', 'pipediv2', 'phy_aux'] is too long
        from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
    ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=1    7.65s user 0.52s system 99% cpu 8.231 total
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % 
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % 
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % 
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % # Total hack just to show our issues in current binding
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % git diff
    diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
    index ca55ed9d74ac..5476cf2422da 100644
    --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
    +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
    @@ -87,7 +87,6 @@ required:
       - reg
       - clocks
       - clock-names
    -  - power-domains
       - resets
       - reset-names
       - vdda-phy-supply
    @@ -132,12 +131,6 @@ allOf:
               maxItems: 5
             clock-names:
               maxItems: 5
    -    else:
    -      properties:
    -        clocks:
    -          minItems: 6
    -        clock-names:
    -          minItems: 6
     
       - if:
           properties:
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=1 DT_SCHEMA_FILES=phy/qcom,sc8280xp-qmp-pcie-phy.yaml qcom/sa8775p-ride.dtb
      UPD     include/config/kernel.release
      LINT    Documentation/devicetree/bindings
      CHKDT   Documentation/devicetree/bindings/processed-schema.json
      SCHEMA  Documentation/devicetree/bindings/processed-schema.json
    /home/ahalaney/git/linux-next/Documentation/devicetree/bindings/power/qcom,kpss-acc-v2.yaml: ignoring, error parsing file
      DTC_CHK arch/arm64/boot/dts/qcom/sa8775p-ride.dtb
    ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=1    7.58s user 0.87s system 98% cpu 8.618 total
    (dtb-checker) ahalaney@fedora ~/git/linux-next (git)-[tags/next-20230724] % 


Thanks,
Andrew