2022-10-25 08:02:06

by Frank Wunderlich

[permalink] [raw]
Subject: [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(-)

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:
--
2.34.1



2022-10-28 09:58:51

by Jianjun Wang (王建军)

[permalink] [raw]
Subject: 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.

On Tue, 2022-10-25 at 09:28 +0200, Frank Wunderlich wrote:
> 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(-)
>
> 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:


2022-10-28 11:43:48

by Frank Wunderlich

[permalink] [raw]
Subject: 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]>:
>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.
>
>On Tue, 2022-10-25 at 09:28 +0200, Frank Wunderlich wrote:
>> 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.

>> 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

2022-10-28 17:30:44

by Frank Wunderlich

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

Hi
> 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
>
> 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

2022-11-01 12:20:06

by Jianjun Wang (王建军)

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

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" <[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
> >
> > 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