diff mbox series

[v2,1/2] dt-bindings: PCI: mediatek-gen3: add SoC based clock config

Message ID 20221025072837.16591-2-linux@fw-web.de (mailing list archive)
State New, archived
Headers show
Series rework mtk pcie-gen3 bindings and support mt7986 | expand

Commit Message

Frank Wunderlich Oct. 25, 2022, 7:28 a.m. UTC 
From: Frank Wunderlich <frank-w@public-files.de>

The PCIe driver covers different SOC which needing different clock
configs. Define them based on compatible.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2:
- fix typo in mediatek,mt8192-pcie
---
 .../bindings/pci/mediatek-pcie-gen3.yaml      | 48 ++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)

Comments

Jianjun Wang (王建军) Oct. 28, 2022, 9:24 a.m. UTC | #1 
Hi Frank,

After apply this patch, we found some dtbs_check error with the
following patch which adds the PCIe node for MT8195:

https://lore.kernel.org/linux-pci/20221020111925.30002-3-tinghan.shen@mediatek.com/

arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f0000
: clock-names:        5: 'top_133m' was expected
    From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
gen3.yaml
arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f8000
: clock-names:        5: 'top_133m' was expected
    From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
gen3.yaml

Did you get the same error when adding the PCIe node for MT7986?

Thanks. 

On Tue, 2022-10-25 at 09:28 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> The PCIe driver covers different SOC which needing different clock
> configs. Define them based on compatible.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> v2:
> - fix typo in mediatek,mt8192-pcie
> ---
>  .../bindings/pci/mediatek-pcie-gen3.yaml      | 48 ++++++++++++++---
> --
>  1 file changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-
> gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-
> gen3.yaml
> index c00be39af64e..98d3f0f1cd76 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -43,9 +43,6 @@ description: |+
>    each set has its own address for MSI message, and supports 32 MSI
> vectors
>    to generate interrupt.
>  
> -allOf:
> -  - $ref: /schemas/pci/pci-bus.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -84,15 +81,7 @@ properties:
>      maxItems: 6
>  
>    clock-names:
> -    items:
> -      - const: pl_250m
> -      - const: tl_26m
> -      - const: tl_96m
> -      - const: tl_32k
> -      - const: peri_26m
> -      - enum:
> -          - top_133m        # for MT8192
> -          - peri_mem        # for MT8188/MT8195
> +    maxItems: 6
>  
>    assigned-clocks:
>      maxItems: 1
> @@ -138,6 +127,41 @@ required:
>    - '#interrupt-cells'
>    - interrupt-controller
>  
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8192-pcie
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: pl_250m
> +            - const: tl_26m
> +            - const: tl_96m
> +            - const: tl_32k
> +            - const: peri_26m
> +            - const: top_133m
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8188-pcie
> +              - mediatek,mt8195-pcie
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: pl_250m
> +            - const: tl_26m
> +            - const: tl_96m
> +            - const: tl_32k
> +            - const: peri_26m
> +            - const: peri_mem
> +
>  unevaluatedProperties: false
>  
>  examples:
Frank Wunderlich Oct. 28, 2022, 10:31 a.m. UTC | #2 
Am 28. Oktober 2022 11:24:36 MESZ schrieb Jianjun Wang <jianjun.wang@mediatek.com>:
>Hi Frank,
>
>After apply this patch, we found some dtbs_check error with the
>following patch which adds the PCIe node for MT8195:
>
>https://lore.kernel.org/linux-pci/20221020111925.30002-3-tinghan.shen@mediatek.com/
>
>arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f0000
>: clock-names:        5: 'top_133m' was expected
>    From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
>gen3.yaml
>arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f8000
>: clock-names:        5: 'top_133m' was expected
>    From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
>gen3.yaml
>
>Did you get the same error when adding the PCIe node for MT7986?
>
>Thanks. 
>
>On Tue, 2022-10-25 at 09:28 +0200, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 

As far as i see the problem is the fallback-node which requires different clockconfig than the main compatible.

6th clock was defined as this enum
  - top_133m        # for MT8192
  - peri_mem        # for MT8188/MT8195

By using lower compatible as main compatible and first one as fallback you cannot success all parts of allOf.

>>    clock-names:
>> -    items:
>> -      - const: pl_250m
>> -      - const: tl_26m
>> -      - const: tl_96m
>> -      - const: tl_32k
>> -      - const: peri_26m
>> -      - enum:
>> -          - top_133m        # for MT8192
>> -          - peri_mem        # for MT8188/MT8195

From my PoV the dts is wrong as the 2 SoC are not compatible to each other...mt8192 needs top_133m as 6th clock whereas mt8195 needs peri_mem. Of course we can change it back to enum in both branches,but imho fallback does not match to main compatible in the dts.

>> +allOf:
>> +  - $ref: /schemas/pci/pci-bus.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: mediatek,mt8192-pcie
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          items:
>> +            - const: pl_250m
>> +            - const: tl_26m
>> +            - const: tl_96m
>> +            - const: tl_32k
>> +            - const: peri_26m
>> +            - const: top_133m
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - mediatek,mt8188-pcie
>> +              - mediatek,mt8195-pcie
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          items:
>> +            - const: pl_250m
>> +            - const: tl_26m
>> +            - const: tl_96m
>> +            - const: tl_32k
>> +            - const: peri_26m
>> +            - const: peri_mem
>> +
>>  unevaluatedProperties: false
>>  
>>  examples:
>


regards Frank
Frank Wunderlich Oct. 28, 2022, 4:42 p.m. UTC | #3 
Hi
> Gesendet: Freitag, 28. Oktober 2022 um 11:24 Uhr
> Von: "Jianjun Wang" <jianjun.wang@mediatek.com>
> An: "Frank Wunderlich" <linux@fw-web.de>, linux-mediatek@lists.infradead.org
> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Ryder Lee" <ryder.lee@mediatek.com>, "Bjorn Helgaas" <bhelgaas@google.com>, "Rob Herring" <robh+dt@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Matthias Brugger" <matthias.bgg@gmail.com>, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Rob Herring" <robh@kernel.org>
> Betreff: Re: [PATCH v2 1/2] dt-bindings: PCI: mediatek-gen3: add SoC based clock config
>
> Hi Frank,
>
> After apply this patch, we found some dtbs_check error with the
> following patch which adds the PCIe node for MT8195:
>
> https://lore.kernel.org/linux-pci/20221020111925.30002-3-tinghan.shen@mediatek.com/
>
> arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f0000
> : clock-names:        5: 'top_133m' was expected
>     From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
> gen3.yaml
> arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: pcie@112f8000
> : clock-names:        5: 'top_133m' was expected
>     From schema: Documentation/devicetree/bindings/pci/mediatek-pcie-
> gen3.yaml
>
> Did you get the same error when adding the PCIe node for MT7986?

i'm sure i had tested the yaml and did not get any error, but on my retest i get same error for mt7986 too...

maybe the right way is to remove the contains in the mediatek,mt8192-pcie if condition (to match only if this condition is no fallback),
then it is clean for me. Can you test it with your patches?

allOf:
  - $ref: /schemas/pci/pci-bus.yaml#
  - if:
      properties:
        compatible:
          #contains:
          const: mediatek,mt8192-pcie
    then:
      properties:
        clock-names:
          items:
            - const: pl_250m
            - const: tl_26m
            - const: tl_96m
            - const: tl_32k
            - const: peri_26m
            - const: top_133m


regards Frank

regards Frank
Jianjun Wang (王建军) Nov. 1, 2022, 12:06 p.m. UTC | #4 
Hi Frank,

On Fri, 2022-10-28 at 18:42 +0200, Frank Wunderlich wrote:
> Hi
> > Gesendet: Freitag, 28. Oktober 2022 um 11:24 Uhr
> > Von: "Jianjun Wang" <jianjun.wang@mediatek.com>
> > An: "Frank Wunderlich" <linux@fw-web.de>, 
> > linux-mediatek@lists.infradead.org
> > Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Ryder Lee" <
> > ryder.lee@mediatek.com>, "Bjorn Helgaas" <bhelgaas@google.com>,
> > "Rob Herring" <robh+dt@kernel.org>, "Krzysztof Kozlowski" <
> > krzysztof.kozlowski+dt@linaro.org>, "Matthias Brugger" <
> > matthias.bgg@gmail.com>, linux-pci@vger.kernel.org, 
> > devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
> > linux-arm-kernel@lists.infradead.org, "Rob Herring" <
> > robh@kernel.org>
> > Betreff: Re: [PATCH v2 1/2] dt-bindings: PCI: mediatek-gen3: add
> > SoC based clock config
> > 
> > Hi Frank,
> > 
> > After apply this patch, we found some dtbs_check error with the
> > following patch which adds the PCIe node for MT8195:
> > 
> > 
https://lore.kernel.org/linux-pci/20221020111925.30002-3-tinghan.shen@mediatek.com/
> > 
> > arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: 
> > pcie@112f0000
> > : clock-names:        5: 'top_133m' was expected
> >     From schema: Documentation/devicetree/bindings/pci/mediatek-
> > pcie-
> > gen3.yaml
> > arch/arm64/boot/dts/mediatek/mt8195-cherry-tomato-r2.dtb: 
> > pcie@112f8000
> > : clock-names:        5: 'top_133m' was expected
> >     From schema: Documentation/devicetree/bindings/pci/mediatek-
> > pcie-
> > gen3.yaml
> > 
> > Did you get the same error when adding the PCIe node for MT7986?
> 
> i'm sure i had tested the yaml and did not get any error, but on my
> retest i get same error for mt7986 too...
> 
> maybe the right way is to remove the contains in the mediatek,mt8192-
> pcie if condition (to match only if this condition is no fallback),
> then it is clean for me. Can you test it with your patches?

Sorry for the late response, I have tested with the patch for MT8195,
and it works, thanks!

> 
> allOf:
>   - $ref: /schemas/pci/pci-bus.yaml#
>   - if:
>       properties:
>         compatible:
>           #contains:
>           const: mediatek,mt8192-pcie
>     then:
>       properties:
>         clock-names:
>           items:
>             - const: pl_250m
>             - const: tl_26m
>             - const: tl_96m
>             - const: tl_32k
>             - const: peri_26m
>             - const: top_133m
> 
> 
> regards Frank
> 
> regards Frank
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
index c00be39af64e..98d3f0f1cd76 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
@@ -43,9 +43,6 @@  description: |+
   each set has its own address for MSI message, and supports 32 MSI vectors
   to generate interrupt.
 
-allOf:
-  - $ref: /schemas/pci/pci-bus.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -84,15 +81,7 @@  properties:
     maxItems: 6
 
   clock-names:
-    items:
-      - const: pl_250m
-      - const: tl_26m
-      - const: tl_96m
-      - const: tl_32k
-      - const: peri_26m
-      - enum:
-          - top_133m        # for MT8192
-          - peri_mem        # for MT8188/MT8195
+    maxItems: 6
 
   assigned-clocks:
     maxItems: 1
@@ -138,6 +127,41 @@  required:
   - '#interrupt-cells'
   - interrupt-controller
 
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8192-pcie
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: pl_250m
+            - const: tl_26m
+            - const: tl_96m
+            - const: tl_32k
+            - const: peri_26m
+            - const: top_133m
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8188-pcie
+              - mediatek,mt8195-pcie
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: pl_250m
+            - const: tl_26m
+            - const: tl_96m
+            - const: tl_32k
+            - const: peri_26m
+            - const: peri_mem
+
 unevaluatedProperties: false
 
 examples: