Re: [PATCH 2/3] dt-bindings: imx6q-pcie: Handle various PD configurations

[Marek Vasut <marex@denx.de>]

On 11/3/22 13:32, Rob Herring wrote:
> On Thu, Nov 3, 2022 at 3:29 AM Alexander Stein
> <alexander.stein@ew.tq-group.com> wrote:
>>
>> Hi Marek,
>>
>> Am Mittwoch, 2. November 2022, 22:57:28 CET schrieb Marek Vasut:
>>> The i.MX SoCs have various power domain configurations routed into
>>> the PCIe IP. MX6SX is the only one which contains 2 domains and also
>>> uses power-domain-names. MX6QDL do not use any domains. All the rest
>>> uses one domain and does not use power-domain-names anymore.
>>>
>>> Document all those configurations in the DT binding document.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Lucas Stach <l.stach@pengutronix.de>
>>> Cc: Richard Zhu <hongxing.zhu@nxp.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: NXP Linux Team <linux-imx@nxp.com>
>>> To: devicetree@vger.kernel.org
>>> ---
>>>   .../bindings/pci/fsl,imx6q-pcie.yaml          | 47 ++++++++++++++-----
>>>   1 file changed, 34 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
>>> b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml index
>>> 1cfea8ca72576..fc8d4d7b80b38 100644
>>> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
>>> @@ -68,19 +68,6 @@ properties:
>>>       description: A phandle to an fsl,imx7d-pcie-phy node. Additional
>>>         required properties for imx7d-pcie and imx8mq-pcie.
>>>
>>> -  power-domains:
>>> -    items:
>>> -      - description: The phandle pointing to the DISPLAY domain for
>>> -          imx6sx-pcie, to PCIE_PHY power domain for imx7d-pcie and
>>> -          imx8mq-pcie.
>>> -      - description: The phandle pointing to the PCIE_PHY power domains
>>> -          for imx6sx-pcie.
>>> -
>>> -  power-domain-names:
>>> -    items:
>>> -      - const: pcie
>>> -      - const: pcie_phy
>>> -
>>>     resets:
>>>       maxItems: 3
>>>       description: Phandles to PCIe-related reset lines exposed by SRC
>>> @@ -241,6 +228,40 @@ allOf:
>>>                   - const: pcie_bus
>>>                   - const: pcie_phy
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,imx6sx-pcie
>>> +    then:
>>> +      properties:
>>> +        power-domains:
>>> +          items:
>>> +            - description: The phandle pointing to the DISPLAY domain for
>>> +                imx6sx-pcie, to PCIE_PHY power domain for imx7d-pcie and
>>> +                imx8mq-pcie.
>>> +            - description: The phandle pointing to the PCIE_PHY power
>>> domains +                for imx6sx-pcie.
>>> +        power-domain-names:
>>> +          items:
>>> +            - const: pcie
>>> +            - const: pcie_phy
>>> +    else:
>>> +      if:
>>> +        not:
>>> +          properties:
>>> +            compatible:
>>> +              contains:
>>> +                enum:
>>> +                  - fsl,imx6q-pcie
>>> +                  - fsl,imx6qp-pcie
>>> +      then:
>>> +        properties:
>>> +          power-domains:
>>> +            description: |
>>> +               The phandle pointing to the DISPLAY domain for imx6sx-pcie,
>>> to +               PCIE_PHY power domain for imx7d-pcie and imx8mq-pcie. +
>>
>> Doesn't it makes more sense to keep the power-domains descriptions in the
>> common part on top, as before, but adjust minItems/maxItems for each
>> compatible?
> 
> Yes. Keep properties defined at the top level.

The problem I keep running into here is that if I apply patch like below 
(basically what you and Alex are suggesting), I get this warning:

arch/arm64/boot/dts/freescale/imx8mm-board.dtb: pcie@33800000: 
power-domains: [[86]] is too short

I think that's because power-domains: contains items: and to validate 
that imx8mm.dtsi with pcie@33800000 { power-domains = <&pgc_pcie>; };, I 
would need to get rid of those items: ? Which is what I did in the 
aforementioned patch for imx8m, that's why I removed it from the common 
part.

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml 
b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 12c7baba489aa..ec5e8dfe541ea 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -68,6 +68,18 @@ properties:
      description: A phandle to an fsl,imx7d-pcie-phy node. Additional
        required properties for imx7d-pcie and imx8mq-pcie.

+  power-domains:
+    items:
+      - description: The phandle pointing to the DISPLAY domain for
+          imx6sx-pcie, to PCIE_PHY power domain for imx7d-pcie and
+          imx8mq-pcie.
+      - description: The phandle pointing to the PCIE_PHY power domains
+          for imx6sx-pcie.
+  power-domain-names:
+    items:
+      - const: pcie
+      - const: pcie_phy
+
    resets:
      maxItems: 2
      description: Phandles to PCIe-related reset lines exposed by SRC
@@ -235,16 +247,11 @@ allOf:
      then:
        properties:
          power-domains:
-          items:
-            - description: The phandle pointing to the DISPLAY domain for
-                imx6sx-pcie, to PCIE_PHY power domain for imx7d-pcie and
-                imx8mq-pcie.
-            - description: The phandle pointing to the PCIE_PHY power 
domains
-                for imx6sx-pcie.
+          minItems: 2
+          maxItems: 2
          power-domain-names:
-          items:
-            - const: pcie
-            - const: pcie_phy
+          minItems: 2
+          maxItems: 2
      else:
        if:
          not:
@@ -257,9 +264,8 @@ allOf:
        then:
          properties:
            power-domains:
-            description: |
-               The phandle pointing to the DISPLAY domain for 
imx6sx-pcie, to
-               PCIE_PHY power domain for imx7d-pcie and imx8mq-pcie.
+            minItems: 1
+            maxItems: 1

    - if:
        properties: