rework mtk pcie-gen3 bindings and support mt7986

PATCH Superseded Frank Wunderlich 5 participants 10 messages Oct 25, 2022 – Nov 1, 2022

Subsystems (5)

Device tree bindings PCI Arm MediaTek PCI native host bridge and endpoint drivers +1 more subsystems

lore

Share & tools

lore

Frank Wunderlich

author

2022-10-25 07:28 UTC #0 lore raw

v2 cover rework mtk pcie-gen3 bindings and support mt7986

From: Frank Wunderlich <[email protected]>

This Series prepares support for mt7986 PCIe which is basicly gen3 PCIe but with slightly differnt clock configuration.

To make differences better to read i split the exiting bindings which has already different settings with a compatible switch and then add a new one for mt7986.

v2:
- fixed typo in part 1 (SoC based config)
- squashed part2+3 (compatible and clock config for mt7986)

Frank Wunderlich (2):
dt-bindings: PCI: mediatek-gen3: add SoC based clock config
dt-bindings: PCI: mediatek-gen3: add support for mt7986

 .../bindings/pci/mediatek-pcie-gen3.yaml      | 65 +++++++++++++++----
 1 file changed, 53 insertions(+), 12 deletions(-)

Signature · 1 line

2.34.1

Frank Wunderlich

author

2022-10-25 07:29 UTC #1 lore raw

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

From: Frank Wunderlich <[email protected]>

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

Signed-off-by Frank Wunderlich <[email protected]>

Reviewed-by Rob Herring <[email protected]>

---
v2:
- fix typo in mediatek,mt8192-pcie

---
 .../bindings/pci/mediatek-pcie-gen3.yaml      | 48 ++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +36 ~~-12~~

index c00be39af64e..98d3f0f1cd76 100644
@@ -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:

Signature · 1 line

2.34.1

Jianjun Wang

2022-10-28 09:25 UTC #2 lore raw

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/[email protected]/

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.

Quoted 91 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>

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

Signed-off-by Frank Wunderlich <[email protected]>

Reviewed-by Rob Herring <[email protected]>

---
v2:
- fix typo in mediatek,mt8192-pcie

---
 .../bindings/pci/mediatek-pcie-gen3.yaml      | 48 ++++++++++++++---
--
 1 file changed, 36 insertions(+), 12 deletions(-)

(quoted diff) +0 -0

gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie- gen3.yaml
index c00be39af64e..98d3f0f1cd76 100644

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +0 -0

@@ -43,9 +43,6 @@ description: |+
   each set has its own address for MSI message, and supports 32 MSI

vectors
to generate interrupt.

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +36 ~~-12~~

-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

author

2022-10-28 10:31 UTC #3 lore raw

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

Am 28. Oktober 2022 11:24:36 MESZ schrieb Jianjun Wang <[email protected]>:

Quoted 16 lines

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/[email protected]/
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

Quoted 7 lines

Did you get the same error when adding the PCIe node for MT7986?
Thanks.
Quoted 2 lines · Frank Wunderlich
From: Frank Wunderlich <[email protected]>

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.

Quoted 10 lines

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml properties: · line 95
Quoted 10 lines
clock-names:
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +0 -9
-    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.

Quoted 37 lines

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +35 -0

+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

Quoted 2 lines

Quoted 1 line
examples:

regards Frank

Frank Wunderlich

2022-10-28 16:42 UTC #4 lore raw

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

In reply to Jianjun Wang

Quoted 22 lines

Gesendet: Freitag, 28. Oktober 2022 um 11:24 Uhr
Von: "Jianjun Wang" <[email protected]>
An: "Frank Wunderlich" <[email protected]>, [email protected]
Cc: "Frank Wunderlich" <[email protected]>, "Ryder Lee" <[email protected]>, "Bjorn Helgaas" <[email protected]>, "Rob Herring" <[email protected]>, "Krzysztof Kozlowski" <[email protected]>, "Matthias Brugger" <[email protected]>, [email protected], [email protected], [email protected], [email protected], "Rob Herring" <[email protected]>
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/[email protected]/
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

Quoted 1 line

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

Jianjun Wang (王建军)

2022-11-01 12:06 UTC #5 lore raw

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

Hi Frank,

Quoted 22 lines · Frank Wunderlich

Hi
Quoted 21 lines
Gesendet: Freitag, 28. Oktober 2022 um 11:24 Uhr
Von: "Jianjun Wang" <[email protected]>
An: "Frank Wunderlich" <[email protected]>, [email protected]
Cc: "Frank Wunderlich" <[email protected]>, "Ryder Lee" < [email protected]>, "Bjorn Helgaas" <[email protected]>, "Rob Herring" <[email protected]>, "Krzysztof Kozlowski" < [email protected]>, "Matthias Brugger" < [email protected]>, [email protected], [email protected], [email protected], [email protected], "Rob Herring" < [email protected]>
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/[email protected]/

Quoted 14 lines

Quoted 14 lines
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

Quoted 8 lines

Quoted 1 line
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!

Quoted 23 lines

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

Frank Wunderlich

author

2022-10-25 07:29 UTC #6 lore raw

PATCH v2 2/2 dt-bindings: PCI: mediatek-gen3: add support for mt7986

In reply to Frank Wunderlich

From: Frank Wunderlich <[email protected]>

Add compatible string and clock-definition for mt7986. It needs 4 clocks for PCIe, define them in binding.

Signed-off-by Frank Wunderlich <[email protected]>

---
v2:
- squashed patch 2+3 (compatible and clock definition)

---
 .../bindings/pci/mediatek-pcie-gen3.yaml        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +17 -0

index 98d3f0f1cd76..57d0e84253e9 100644
@@ -48,6 +48,7 @@ properties:
     oneOf:
       - items:
           - enum:
+              - mediatek,mt7986-pcie
               - mediatek,mt8188-pcie
               - mediatek,mt8195-pcie
           - const: mediatek,mt8192-pcie

@@ -78,9 +79,11 @@ properties:
       - const: mac
 
   clocks:
+    minItems: 4
     maxItems: 6
 
   clock-names:
+    minItems: 4
     maxItems: 6
 
   assigned-clocks:

@@ -161,6 +164,20 @@ allOf:
             - const: tl_32k
             - const: peri_26m
             - const: peri_mem
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt7986-pcie
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: pl_250m
+            - const: tl_26m
+            - const: peri_26m
+            - const: top_133m
 
 unevaluatedProperties: false

Signature · 1 line

2.34.1

AngeloGioacchino Del Regno

maintainer

2022-10-25 11:11 UTC #7 lore raw

Re: [PATCH v2 2/2] dt-bindings: PCI: mediatek-gen3: add support for mt7986

Il 25/10/22 09:28, Frank Wunderlich ha scritto:

Quoted 26 lines

From: Frank Wunderlich <[email protected]>
Add compatible string and clock-definition for mt7986. It needs 4 clocks for PCIe, define them in binding.
Signed-off-by Frank Wunderlich <[email protected]>
---
v2:
- squashed patch 2+3 (compatible and clock definition)
---
  .../bindings/pci/mediatek-pcie-gen3.yaml        | 17 +++++++++++++++++
  1 file changed, 17 insertions(+)
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +1 -0
index 98d3f0f1cd76..57d0e84253e9 100644
@@ -48,6 +48,7 @@ properties:
      oneOf:
        - items:
            - enum:
+              - mediatek,mt7986-pcie
                - mediatek,mt8188-pcie
                - mediatek,mt8195-pcie
            - const: mediatek,mt8192-pcie

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml properties: · line 83

Quoted 5 lines

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +1 -0

@@ -78,9 +79,11 @@ properties:
        - const: mac
 
    clocks:
+    minItems: 4
      maxItems: 6

I'm not sure that this is really correct.
If you have a list of items (const or description, doesn't matter), you don't have to specify maxItems, as it is implicitly equal to the number of items. Also, specifying minItems means that you're "setting" one or more items as optional.

It looks like you're specifying a specific and definite list of items for both clocks and clock-names... and for all of the supported SoCs, so, I don't think that having {min,max}Items globally specified on clocks, clock-names make any kind of sense.

.....but I'd like for someone else to give an opinion on this as well.

Krzysztof, please?

Cheers,
Angelo

Quoted 28 lines

Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +15 -0

    clock-names:
+    minItems: 4
      maxItems: 6
 
    assigned-clocks:

@@ -161,6 +164,20 @@ allOf:
              - const: tl_32k
              - const: peri_26m
              - const: peri_mem
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt7986-pcie
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: pl_250m
+            - const: tl_26m
+            - const: peri_26m
+            - const: top_133m
 
  unevaluatedProperties: false

Rob Herring

maintainer

2022-10-26 19:46 UTC #8 lore raw

Re: [PATCH v2 2/2] dt-bindings: PCI: mediatek-gen3: add support for mt7986

Quoted 38 lines · AngeloGioacchino Del Regno

Il 25/10/22 09:28, Frank Wunderlich ha scritto:
Quoted 26 lines
From: Frank Wunderlich <[email protected]>
Add compatible string and clock-definition for mt7986. It needs 4 clocks for PCIe, define them in binding.
Signed-off-by Frank Wunderlich <[email protected]>
---
v2:
- squashed patch 2+3 (compatible and clock definition)
---
  .../bindings/pci/mediatek-pcie-gen3.yaml        | 17 +++++++++++++++++
  1 file changed, 17 insertions(+)
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +1 -0
index 98d3f0f1cd76..57d0e84253e9 100644
@@ -48,6 +48,7 @@ properties:
      oneOf:
        - items:
            - enum:
+              - mediatek,mt7986-pcie
                - mediatek,mt8188-pcie
                - mediatek,mt8195-pcie
            - const: mediatek,mt8192-pcie
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml properties: · line 82
Quoted 4 lines
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml +1 -0
@@ -78,9 +79,11 @@ properties:
        - const: mac
    clocks:
+    minItems: 4
      maxItems: 6
I'm not sure that this is really correct.
If you have a list of items (const or description, doesn't matter), you don't have to specify maxItems, as it is implicitly equal to the number of items. Also, specifying minItems means that you're "setting" one or more items as optional.

Quoted 6 lines

It looks like you're specifying a specific and definite list of items for both clocks and clock-names... and for all of the supported SoCs, so, I don't think that having {min,max}Items globally specified on clocks, clock-names make any kind of sense.
.....but I'd like for someone else to give an opinion on this as well.

It does make sense. Globally, we have the full range of possible clocks and then the if/then schemas further restrict it.

Rob

Rob Herring

maintainer Reviewed-by on v2 2/2

2022-10-26 19:47 UTC #9 lore raw

Re: [PATCH v2 2/2] dt-bindings: PCI: mediatek-gen3: add support for mt7986

In reply to Frank Wunderlich

Quoted 5 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>
Add compatible string and clock-definition for mt7986. It needs 4 clocks for PCIe, define them in binding.

Quoted 8 lines

Signed-off-by Frank Wunderlich <[email protected]>
---
v2:
- squashed patch 2+3 (compatible and clock definition)
---
 .../bindings/pci/mediatek-pcie-gen3.yaml        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Reviewed-by Rob Herring <[email protected]>