2024-04-10 18:15:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 1/4] dt-bindings: PCI: cdns,cdns-pcie-host: drop redundant msi-parent and pci-bus.yaml

The binding reference common cdns-pcie-host.yaml, which already defines
msi-parent and has a reference to pci-bus.yaml schema. Drop redundant
pieces here to make it a bit smaller.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
1. Add tags.
---
Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml | 3 ---
1 file changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
index bc3c48f60fff..a8190d9b100f 100644
--- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
@@ -10,7 +10,6 @@ maintainers:
- Tom Joseph <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
- $ref: cdns-pcie-host.yaml#

properties:
@@ -25,8 +24,6 @@ properties:
- const: reg
- const: cfg

- msi-parent: true
-
required:
- reg
- reg-names
--
2.34.1



2024-04-10 18:16:03

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

MT7621 PCI host bridge has children which apparently are also PCI host
bridges, at least that's what the binding suggest. The children have
"reg" property, but do not explicitly define it. Instead they rely on
pci-bus.yaml schema, but that one has "reg" without any constraints.

Define the "reg" for the children, so the binding will be more specific
and later will allow dropping reference to deprecated pci-bus.yaml
schema.

Acked-by: Sergio Paracuellos <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
1. Add tags.
---
.../devicetree/bindings/pci/mediatek,mt7621-pcie.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
index e63e6458cea8..61d027239910 100644
--- a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
@@ -36,6 +36,9 @@ patternProperties:
$ref: /schemas/pci/pci-bus.yaml#

properties:
+ reg:
+ maxItems: 1
+
resets:
maxItems: 1

--
2.34.1


2024-04-10 18:16:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 3/4] dt-bindings: PCI: host-bridges: switch from deprecated pci-bus.yaml

dtschema package with core schemas deprecated pci-bus.yaml schema in
favor of pci-host-bridge.yaml. Update all bindings to use the latter
one.

The difference between pci-bus.yaml and pci-host-bridge.yaml is only in
lack of "reg" property defined by the latter, which should not have any
effect here, because all these bindings define the "reg".

The change is therefore quite trivial, however it requires dtschema
package v2024.02 or newer.

Reviewed-by: Geert Uytterhoeven <[email protected]> # Renesas
Acked-by: Sergio Paracuellos <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
1. Add tags.
2. Split mediatek,mt7621-pcie to separate patch as it uses
pci-pci-bridge schema.
---
Documentation/devicetree/bindings/pci/amlogic,axg-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/apple,pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml | 2 +-
Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml | 2 +-
Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 2 +-
Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml | 2 +-
Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/loongson.yaml | 2 +-
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml | 2 +-
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml | 2 +-
Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml | 2 +-
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/rcar-pci-host.yaml | 2 +-
.../devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 2 +-
Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml | 2 +-
Documentation/devicetree/bindings/pci/versatile.yaml | 2 +-
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 2 +-
Documentation/devicetree/bindings/pci/xlnx,axi-pcie-host.yaml | 2 +-
Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +-
Documentation/devicetree/bindings/pci/xlnx,xdma-host.yaml | 2 +-
24 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/amlogic,axg-pcie.yaml b/Documentation/devicetree/bindings/pci/amlogic,axg-pcie.yaml
index a5bd90bc0712..79a21ba0f9fd 100644
--- a/Documentation/devicetree/bindings/pci/amlogic,axg-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/amlogic,axg-pcie.yaml
@@ -13,7 +13,7 @@ description:
Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core.

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/pci/snps,dw-pcie-common.yaml#

# We need a select here so we don't match all nodes with 'snps,dw-pcie'
diff --git a/Documentation/devicetree/bindings/pci/apple,pcie.yaml b/Documentation/devicetree/bindings/pci/apple,pcie.yaml
index 215ff9a9c835..c8775f9cb071 100644
--- a/Documentation/devicetree/bindings/pci/apple,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/apple,pcie.yaml
@@ -85,7 +85,7 @@ required:
unevaluatedProperties: false

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/interrupt-controller/msi-controller.yaml#
- if:
properties:
diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
index 0e07ab61a48d..5434c144d2ec 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
@@ -11,7 +11,7 @@ maintainers:
- Scott Branden <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 22491f7f8852..11f8ea33240c 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -108,7 +108,7 @@ required:
- msi-controller

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/interrupt-controller/msi-controller.yaml#
- if:
properties:
diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
index a6b494401ebb..f4eb82e684bd 100644
--- a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
@@ -10,7 +10,7 @@ maintainers:
- Tom Joseph <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: cdns-pcie.yaml#

properties:
diff --git a/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
index 92efbf0f1297..378dd1c8e2ee 100644
--- a/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
+++ b/Documentation/devicetree/bindings/pci/faraday,ftpci100.yaml
@@ -51,7 +51,7 @@ description: |
<0x6000 0 0 4 &pci_intc 2>;

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
index d25423aa7167..3484e0b4b412 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
@@ -116,7 +116,7 @@ required:
- ranges

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- if:
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
index debfb54a8042..3cae2e0f7f5e 100644
--- a/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/intel,ixp4xx-pci.yaml
@@ -12,7 +12,7 @@ maintainers:
description: PCI host controller found in the Intel IXP4xx SoC series.

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml
index 505acc4f3efc..1fd557504b10 100644
--- a/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/intel,keembay-pcie.yaml
@@ -11,7 +11,7 @@ maintainers:
- Srikanth Thokala <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/loongson.yaml b/Documentation/devicetree/bindings/pci/loongson.yaml
index a8324a9bd002..1988465e73a1 100644
--- a/Documentation/devicetree/bindings/pci/loongson.yaml
+++ b/Documentation/devicetree/bindings/pci/loongson.yaml
@@ -13,7 +13,7 @@ description: |+
PCI host controller found on Loongson PCHs and SoCs.

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
index 7e8c7a2a5f9b..76d742051f73 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
@@ -140,7 +140,7 @@ required:
- interrupt-controller

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- if:
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
index f7a3c2636355..a3c4ddc094aa 100644
--- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
@@ -10,7 +10,7 @@ maintainers:
- Daire McNamara <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/interrupt-controller/msi-controller.yaml#

properties:
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 0d1b23523f62..0a39bbfcb28b 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -95,6 +95,6 @@ anyOf:
- msi-map

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

additionalProperties: true
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index cf9a6910b542..f867746b1ae5 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -130,7 +130,7 @@ anyOf:
- msi-map

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- if:
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/rcar-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-pci-host.yaml
index b6a7cb32f61e..210c3f2bf94c 100644
--- a/Documentation/devicetree/bindings/pci/rcar-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/rcar-pci-host.yaml
@@ -12,7 +12,7 @@ maintainers:
- Yoshihiro Shimoda <[email protected]>

allOf:
- - $ref: pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml
index 5a0d64d3ae6b..b288cdb1ec70 100644
--- a/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml
+++ b/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml
@@ -110,7 +110,7 @@ required:
- "#interrupt-cells"

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

- if:
properties:
diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie.yaml
index 002b728cbc71..720a5f945a4e 100644
--- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie.yaml
@@ -10,7 +10,7 @@ maintainers:
- Shawn Lin <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: rockchip,rk3399-pcie-common.yaml#

properties:
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 022055edbf9e..548f59d76ef2 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -23,7 +23,7 @@ select:
- compatible

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/pci/snps,dw-pcie-common.yaml#
- if:
not:
diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
index a20dccbafd94..695e491b7b3b 100644
--- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
@@ -11,7 +11,7 @@ maintainers:
- Kishon Vijay Abraham I <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/versatile.yaml b/Documentation/devicetree/bindings/pci/versatile.yaml
index 09748ef6b94f..294c7cd84b37 100644
--- a/Documentation/devicetree/bindings/pci/versatile.yaml
+++ b/Documentation/devicetree/bindings/pci/versatile.yaml
@@ -13,7 +13,7 @@ description: |+
PCI host controller found on the ARM Versatile PB board's FPGA.

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
index 4734be456bde..b75ceefa6f93 100644
--- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
@@ -10,7 +10,7 @@ maintainers:
- Bharat Kumar Gogada <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/xlnx,axi-pcie-host.yaml b/Documentation/devicetree/bindings/pci/xlnx,axi-pcie-host.yaml
index 69b7decabd45..fb87b960a250 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,axi-pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,axi-pcie-host.yaml
@@ -10,7 +10,7 @@ maintainers:
- Thippeswamy Havalige <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
index 426f90a47f35..b0d07c71c1c0 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
@@ -10,7 +10,7 @@ maintainers:
- Thippeswamy Havalige <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
- $ref: /schemas/interrupt-controller/msi-controller.yaml#

properties:
diff --git a/Documentation/devicetree/bindings/pci/xlnx,xdma-host.yaml b/Documentation/devicetree/bindings/pci/xlnx,xdma-host.yaml
index 0aa00b8e49b3..2f59b3a73dd2 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,xdma-host.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,xdma-host.yaml
@@ -10,7 +10,7 @@ maintainers:
- Thippeswamy Havalige <[email protected]>

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
--
2.34.1


2024-04-10 18:16:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 4/4] dt-bindings: PCI: mediatek,mt7621-pcie: switch from deprecated pci-bus.yaml

dtschema package with core schemas deprecated pci-bus.yaml schema in
favor of individual schemas per host, device and pci-pci.

Switch Mediatek MT7621 PCIe host bridge binding to this new schema.

This requires dtschema package newer than v2024.02 to work fully.
v2024.02 will partially work: with a warning.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Important: v2024.03 (said dtschema newer than v2024.02) was not yet
released, therefore this patch probably should wait a bit. Previous
patches do not depend anyhow on future release, so they can be taken as
is.

Changes in v2:
1. New patch
2. Split mediatek,mt7621-pcie to separate patch as it uses
pci-pci-bridge schema.
---
.../devicetree/bindings/pci/mediatek,mt7621-pcie.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
index 61d027239910..6fba42156db6 100644
--- a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
@@ -14,7 +14,7 @@ description: |+
with 3 Root Ports. Each Root Port supports a Gen1 1-lane Link

allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
+ - $ref: /schemas/pci/pci-host-bridge.yaml#

properties:
compatible:
@@ -33,7 +33,7 @@ properties:
patternProperties:
'^pcie@[0-2],0$':
type: object
- $ref: /schemas/pci/pci-bus.yaml#
+ $ref: /schemas/pci/pci-pci-bridge.yaml#

properties:
reg:
--
2.34.1


2024-04-10 19:32:48

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] dt-bindings: PCI: mediatek,mt7621-pcie: switch from deprecated pci-bus.yaml


On Wed, 10 Apr 2024 20:15:21 +0200, Krzysztof Kozlowski wrote:
> dtschema package with core schemas deprecated pci-bus.yaml schema in
> favor of individual schemas per host, device and pci-pci.
>
> Switch Mediatek MT7621 PCIe host bridge binding to this new schema.
>
> This requires dtschema package newer than v2024.02 to work fully.
> v2024.02 will partially work: with a warning.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> Important: v2024.03 (said dtschema newer than v2024.02) was not yet
> released, therefore this patch probably should wait a bit. Previous
> patches do not depend anyhow on future release, so they can be taken as
> is.
>
> Changes in v2:
> 1. New patch
> 2. Split mediatek,mt7621-pcie to separate patch as it uses
> pci-pci-bridge schema.
> ---
> .../devicetree/bindings/pci/mediatek,mt7621-pcie.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.example.dtb: pcie@1e140000: pcie@0,0: Unevaluated properties are not allowed ('clocks', 'phy-names', 'phys', 'resets' were unexpected)
from schema $id: http://devicetree.org/schemas/pci/mediatek,mt7621-pcie.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.example.dtb: pcie@1e140000: pcie@1,0: Unevaluated properties are not allowed ('clocks', 'phy-names', 'phys', 'resets' were unexpected)
from schema $id: http://devicetree.org/schemas/pci/mediatek,mt7621-pcie.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.example.dtb: pcie@1e140000: pcie@2,0: Unevaluated properties are not allowed ('clocks', 'phy-names', 'phys', 'resets' were unexpected)
from schema $id: http://devicetree.org/schemas/pci/mediatek,mt7621-pcie.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-04-10 21:44:16

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> MT7621 PCI host bridge has children which apparently are also PCI host
> bridges, at least that's what the binding suggest.

What does it even mean for a PCI host bridge to have a child that is
also a PCI host bridge?

Does this mean a driver binds to the "parent" host bridge, enumerates
the PCI devices below it, and finds a "child" host bridge?

> The children have
> "reg" property, but do not explicitly define it. Instead they rely on
> pci-bus.yaml schema, but that one has "reg" without any constraints.
>
> Define the "reg" for the children, so the binding will be more specific
> and later will allow dropping reference to deprecated pci-bus.yaml
> schema.
>
> Acked-by: Sergio Paracuellos <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> Changes in v2:
> 1. Add tags.
> ---
> .../devicetree/bindings/pci/mediatek,mt7621-pcie.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> index e63e6458cea8..61d027239910 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> @@ -36,6 +36,9 @@ patternProperties:
> $ref: /schemas/pci/pci-bus.yaml#
>
> properties:
> + reg:
> + maxItems: 1
> +
> resets:
> maxItems: 1
>
> --
> 2.34.1
>

2024-04-11 06:01:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On 10/04/2024 23:26, Bjorn Helgaas wrote:
> On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
>> MT7621 PCI host bridge has children which apparently are also PCI host
>> bridges, at least that's what the binding suggest.
>
> What does it even mean for a PCI host bridge to have a child that is
> also a PCI host bridge?
>
> Does this mean a driver binds to the "parent" host bridge, enumerates
> the PCI devices below it, and finds a "child" host bridge?

I think the question should be towards Mediatek folks. I don't know what
this hardware is exactly, just looks like pci-pci-bridge. The driver
calls the children host bridges as "ports".

Best regards,
Krzysztof


2024-04-11 06:13:48

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

Hi,

On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> >> MT7621 PCI host bridge has children which apparently are also PCI host
> >> bridges, at least that's what the binding suggest.
> >
> > What does it even mean for a PCI host bridge to have a child that is
> > also a PCI host bridge?
> >
> > Does this mean a driver binds to the "parent" host bridge, enumerates
> > the PCI devices below it, and finds a "child" host bridge?

Yes, that is exactly what you can see on enumeration.

The following is a typical boot trace where all bridges has a device also below:

[ 20.927280] mt7621-pci 1e140000.pcie: host bridge /pcie@1e140000 ranges:
[ 20.940675] mt7621-pci 1e140000.pcie: No bus range found for
/pcie@1e140000, using [bus 00-ff]
[ 20.958228] mt7621-pci 1e140000.pcie: MEM
0x0060000000..0x006fffffff -> 0x0060000000
[ 20.974566] mt7621-pci 1e140000.pcie: IO
0x001e160000..0x001e16ffff -> 0x0000000000
[ 21.369711] mt7621-pci 1e140000.pcie: PCIE0 enabled
[ 21.379316] mt7621-pci 1e140000.pcie: PCIE1 enabled
[ 21.389140] mt7621-pci 1e140000.pcie: PCIE2 enabled
[ 21.399014] PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
[ 21.413343] mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
[ 21.425952] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 21.437023] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[ 21.450657] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[ 21.462960] pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
[ 21.474832] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
[ 21.487255] pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
[ 21.499807] pci 0000:00:00.0: supports D1
[ 21.507625] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
[ 21.519923] pci 0000:00:01.0: [0e8d:0801] type 01 class 0x060400
[ 21.531827] pci 0000:00:01.0: reg 0x10: [mem 0x00000000-0x7fffffff]
[ 21.544225] pci 0000:00:01.0: reg 0x14: [mem 0x00000000-0x0000ffff]
[ 21.556773] pci 0000:00:01.0: supports D1
[ 21.564621] pci 0000:00:01.0: PME# supported from D0 D1 D3hot
[ 21.576823] pci 0000:00:02.0: [0e8d:0801] type 01 class 0x060400
[ 21.588726] pci 0000:00:02.0: reg 0x10: [mem 0x00000000-0x7fffffff]
[ 21.601128] pci 0000:00:02.0: reg 0x14: [mem 0x00000000-0x0000ffff]
[ 21.613668] pci 0000:00:02.0: supports D1
[ 21.621520] pci 0000:00:02.0: PME# supported from D0 D1 D3hot
[ 21.634850] pci 0000:00:00.0: bridge configuration invalid ([bus
00-00]), reconfiguring
[ 21.650699] pci 0000:00:01.0: bridge configuration invalid ([bus
00-00]), reconfiguring
[ 21.666571] pci 0000:00:02.0: bridge configuration invalid ([bus
00-00]), reconfiguring
[ 21.682825] pci 0000:01:00.0: [1b21:0611] type 00 class 0x010185
[ 21.694707] pci 0000:01:00.0: reg 0x10: [io 0x0000-0x0007]
[ 21.705725] pci 0000:01:00.0: reg 0x14: [io 0x0000-0x0003]
[ 21.716789] pci 0000:01:00.0: reg 0x18: [io 0x0000-0x0007]
[ 21.727843] pci 0000:01:00.0: reg 0x1c: [io 0x0000-0x0003]
[ 21.738907] pci 0000:01:00.0: reg 0x20: [io 0x0000-0x000f]
[ 21.749979] pci 0000:01:00.0: reg 0x24: [mem 0x00000000-0x000001ff]
[ 21.762568] pci 0000:01:00.0: 2.000 Gb/s available PCIe bandwidth,
limited by 2.5 GT/s PCIe x1 link at 0000:00:00.0 (capable of 4.000
Gb/s with 5.0 GT/s PCIe x1 link)
[ 21.819657] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[ 21.829966] pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
[ 21.842054] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
[ 21.855532] pci 0000:00:00.0: bridge window [mem
0x00000000-0x000fffff pref]
[ 21.869874] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[ 21.883352] pci 0000:02:00.0: [1b21:0611] type 00 class 0x010185
[ 21.895230] pci 0000:02:00.0: reg 0x10: [io 0x0000-0x0007]
[ 21.906256] pci 0000:02:00.0: reg 0x14: [io 0x0000-0x0003]
[ 21.917309] pci 0000:02:00.0: reg 0x18: [io 0x0000-0x0007]
[ 21.928373] pci 0000:02:00.0: reg 0x1c: [io 0x0000-0x0003]
[ 21.939428] pci 0000:02:00.0: reg 0x20: [io 0x0000-0x000f]
[ 21.950500] pci 0000:02:00.0: reg 0x24: [mem 0x00000000-0x000001ff]
[ 21.963094] pci 0000:02:00.0: 2.000 Gb/s available PCIe bandwidth,
limited by 2.5 GT/s PCIe x1 link at 0000:00:01.0 (capable of 4.000
Gb/s with 5.0 GT/s PCIe x1 link)
[ 22.029662] pci 0000:00:01.0: PCI bridge to [bus 02-ff]
[ 22.039993] pci 0000:00:01.0: bridge window [io 0x0000-0x0fff]
[ 22.052063] pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff]
[ 22.065538] pci 0000:00:01.0: bridge window [mem
0x00000000-0x000fffff pref]
[ 22.079886] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 02
[ 22.093352] pci 0000:03:00.0: [1b21:0611] type 00 class 0x010185
[ 22.105233] pci 0000:03:00.0: reg 0x10: [io 0x0000-0x0007]
[ 22.116249] pci 0000:03:00.0: reg 0x14: [io 0x0000-0x0003]
[ 22.127313] pci 0000:03:00.0: reg 0x18: [io 0x0000-0x0007]
[ 22.138367] pci 0000:03:00.0: reg 0x1c: [io 0x0000-0x0003]
[ 22.149451] pci 0000:03:00.0: reg 0x20: [io 0x0000-0x000f]
[ 22.160503] pci 0000:03:00.0: reg 0x24: [mem 0x00000000-0x000001ff]
[ 22.173091] pci 0000:03:00.0: 2.000 Gb/s available PCIe bandwidth,
limited by 2.5 GT/s PCIe x1 link at 0000:00:02.0 (capable of 4.000
Gb/s with 5.0 GT/s PCIe x1 link)
[ 22.239653] pci 0000:00:02.0: PCI bridge to [bus 03-ff]
[ 22.249973] pci 0000:00:02.0: bridge window [io 0x0000-0x0fff]
[ 22.262045] pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff]
[ 22.275524] pci 0000:00:02.0: bridge window [mem
0x00000000-0x000fffff pref]
[ 22.289870] pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
[ 22.303080] pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
[ 22.316129] pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
[ 22.329956] pci 0000:00:01.0: BAR 0: no space for [mem size 0x80000000]
[ 22.343081] pci 0000:00:01.0: BAR 0: failed to assign [mem size 0x80000000]
[ 22.356912] pci 0000:00:02.0: BAR 0: no space for [mem size 0x80000000]
[ 22.370053] pci 0000:00:02.0: BAR 0: failed to assign [mem size 0x80000000]
[ 22.383870] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[ 22.397349] pci 0000:00:00.0: BAR 9: assigned [mem
0x60100000-0x601fffff pref]
[ 22.411692] pci 0000:00:01.0: BAR 8: assigned [mem 0x60200000-0x602fffff]
[ 22.425165] pci 0000:00:01.0: BAR 9: assigned [mem
0x60300000-0x603fffff pref]
[ 22.439522] pci 0000:00:02.0: BAR 8: assigned [mem 0x60400000-0x604fffff]
[ 22.452991] pci 0000:00:02.0: BAR 9: assigned [mem
0x60500000-0x605fffff pref]
[ 22.467328] pci 0000:00:00.0: BAR 1: assigned [mem 0x60600000-0x6060ffff]
[ 22.480814] pci 0000:00:01.0: BAR 1: assigned [mem 0x60610000-0x6061ffff]
[ 22.494303] pci 0000:00:02.0: BAR 1: assigned [mem 0x60620000-0x6062ffff]
[ 22.507766] pci 0000:00:00.0: BAR 7: assigned [io 0x0000-0x0fff]
[ 22.519861] pci 0000:00:01.0: BAR 7: assigned [io 0x1000-0x1fff]
[ 22.531960] pci 0000:00:02.0: BAR 7: assigned [io 0x2000-0x2fff]
[ 22.544068] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[ 22.557548] pci 0000:01:00.0: BAR 4: assigned [io 0x0000-0x000f]
[ 22.569635] pci 0000:01:00.0: BAR 0: assigned [io 0x0010-0x0017]
[ 22.581726] pci 0000:01:00.0: BAR 2: assigned [io 0x0018-0x001f]
[ 22.593827] pci 0000:01:00.0: BAR 1: assigned [io 0x0020-0x0023]
[ 22.605917] pci 0000:01:00.0: BAR 3: assigned [io 0x0024-0x0027]
[ 22.618030] pci 0000:00:00.0: PCI bridge to [bus 01]
[ 22.627859] pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
[ 22.639956] pci 0000:00:00.0: bridge window [mem 0x60000000-0x600fffff]
[ 22.653437] pci 0000:00:00.0: bridge window [mem
0x60100000-0x601fffff pref]
[ 22.667785] pci 0000:02:00.0: BAR 5: assigned [mem 0x60200000-0x602001ff]
[ 22.681268] pci 0000:02:00.0: BAR 4: assigned [io 0x1000-0x100f]
[ 22.693359] pci 0000:02:00.0: BAR 0: assigned [io 0x1010-0x1017]
[ 22.705450] pci 0000:02:00.0: BAR 2: assigned [io 0x1018-0x101f]
[ 22.717552] pci 0000:02:00.0: BAR 1: assigned [io 0x1020-0x1023]
[ 22.729654] pci 0000:02:00.0: BAR 3: assigned [io 0x1024-0x1027]
[ 22.741746] pci 0000:00:01.0: PCI bridge to [bus 02]
[ 22.751587] pci 0000:00:01.0: bridge window [io 0x1000-0x1fff]
[ 22.763678] pci 0000:00:01.0: bridge window [mem 0x60200000-0x602fffff]
[ 22.777161] pci 0000:00:01.0: bridge window [mem
0x60300000-0x603fffff pref]
[ 22.791515] pci 0000:03:00.0: BAR 5: assigned [mem 0x60400000-0x604001ff]
[ 22.804991] pci 0000:03:00.0: BAR 4: assigned [io 0x2000-0x200f]
[ 22.817084] pci 0000:03:00.0: BAR 0: assigned [io 0x2010-0x2017]
[ 22.829175] pci 0000:03:00.0: BAR 2: assigned [io 0x2018-0x201f]
[ 22.841274] pci 0000:03:00.0: BAR 1: assigned [io 0x2020-0x2023]
[ 22.853379] pci 0000:03:00.0: BAR 3: assigned [io 0x2024-0x2027]
[ 22.865472] pci 0000:00:02.0: PCI bridge to [bus 03]
[ 22.875312] pci 0000:00:02.0: bridge window [io 0x2000-0x2fff]
[ 22.887403] pci 0000:00:02.0: bridge window [mem 0x60400000-0x604fffff]

>
> I think the question should be towards Mediatek folks. I don't know what
> this hardware is exactly, just looks like pci-pci-bridge. The driver
> calls the children host bridges as "ports".

You can see the topology here in my first driver submit cover letter
message [0].

Thanks,
Sergio Paracuellos

[0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/

>
> Best regards,
> Krzysztof
>

2024-04-11 06:20:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On 11/04/2024 08:13, Sergio Paracuellos wrote:
>
>>
>> I think the question should be towards Mediatek folks. I don't know what
>> this hardware is exactly, just looks like pci-pci-bridge. The driver
>> calls the children host bridges as "ports".
>
> You can see the topology here in my first driver submit cover letter
> message [0].
>

Useful diagram, thanks. It would be great if you could add it to the
binding description.

Best regards,
Krzysztof


2024-04-11 06:48:11

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 8:20 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 11/04/2024 08:13, Sergio Paracuellos wrote:
> >
> >>
> >> I think the question should be towards Mediatek folks. I don't know what
> >> this hardware is exactly, just looks like pci-pci-bridge. The driver
> >> calls the children host bridges as "ports".
> >
> > You can see the topology here in my first driver submit cover letter
> > message [0].
> >
>
> Useful diagram, thanks. It would be great if you could add it to the
> binding description.

Sure, I will do it depending on time hopefully sooner than later :-).

Best regards,
Sergio Paracuellos

>
> Best regards,
> Krzysztof
>

2024-04-11 12:39:29

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote:
> On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
> <[email protected]> wrote:
> > On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> > >> MT7621 PCI host bridge has children which apparently are also PCI host
> > >> bridges, at least that's what the binding suggest.
> > >
> > > What does it even mean for a PCI host bridge to have a child that is
> > > also a PCI host bridge?
> > >
> > > Does this mean a driver binds to the "parent" host bridge, enumerates
> > > the PCI devices below it, and finds a "child" host bridge?
>
> Yes, that is exactly what you can see on enumeration.
>
> The following is a typical boot trace where all bridges has a device also below:
>
> mt7621-pci 1e140000.pcie: host bridge /pcie@1e140000 ranges:
> mt7621-pci 1e140000.pcie: No bus range found for /pcie@1e140000, using [bus 00-ff]
> mt7621-pci 1e140000.pcie: MEM 0x0060000000..0x006fffffff -> 0x0060000000
> mt7621-pci 1e140000.pcie: IO 0x001e160000..0x001e16ffff -> 0x0000000000
> mt7621-pci 1e140000.pcie: PCIE0 enabled
> mt7621-pci 1e140000.pcie: PCIE1 enabled
> mt7621-pci 1e140000.pcie: PCIE2 enabled
> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00

1e140000.pcie is a host bridge. It has some CPU-specific bus on the
upstream side, standard PCI (domain 0000, buses 00-ff) on the
downstream side.

> pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> pci 0000:00:01.0: [0e8d:0801] type 01 class 0x060400
> pci 0000:00:02.0: [0e8d:0801] type 01 class 0x060400

> pci 0000:01:00.0: [1b21:0611] type 00 class 0x010185

> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff pref]

00:00.0 looks like a PCIe Root Port to bus 01. This is not a host
bridge; it's just a standard PCI-to-PCI bridge with PCI on both the
upstream and downstream sides.

> pci 0000:02:00.0: [1b21:0611] type 00 class 0x010185

> pci 0000:00:01.0: PCI bridge to [bus 02-ff]
> pci 0000:00:01.0: bridge window [io 0x0000-0x0fff]
> pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff pref]

00:01.0 is another Root Port to bus 02.

> pci 0000:03:00.0: [1b21:0611] type 00 class 0x010185

> pci 0000:00:02.0: PCI bridge to [bus 03-ff]
> pci 0000:00:02.0: bridge window [io 0x0000-0x0fff]
> pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff pref]
> pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03

And 00:02.0 is a third Root Port to bus 03.

> pci 0000:00:00.0: PCI bridge to [bus 01]
> pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> pci 0000:00:00.0: bridge window [mem 0x60000000-0x600fffff]
> pci 0000:00:00.0: bridge window [mem 0x60100000-0x601fffff pref]
> pci 0000:00:01.0: PCI bridge to [bus 02]
> pci 0000:00:01.0: bridge window [io 0x1000-0x1fff]
> pci 0000:00:01.0: bridge window [mem 0x60200000-0x602fffff]
> pci 0000:00:01.0: bridge window [mem 0x60300000-0x603fffff pref]
> pci 0000:00:02.0: PCI bridge to [bus 03]
> pci 0000:00:02.0: bridge window [io 0x2000-0x2fff]
> pci 0000:00:02.0: bridge window [mem 0x60400000-0x604fffff]
>
> > I think the question should be towards Mediatek folks. I don't know what
> > this hardware is exactly, just looks like pci-pci-bridge. The driver
> > calls the children host bridges as "ports".
>
> You can see the topology here in my first driver submit cover letter
> message [0].
>
> [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/

Nothing unusual here, this looks like the standard PCIe topology.

What *might* be unusual is describing the Root Ports in DT. Since
they are standard PCI devices, they shouldn't need DT description
unless there's some unusual power/clock/reset control or something
that is not discoverable via PCI enumeration.

Bjorn

2024-04-11 13:34:34

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 2:39 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote:
> > On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> > > On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> > > >> MT7621 PCI host bridge has children which apparently are also PCI host
> > > >> bridges, at least that's what the binding suggest.
> > > >
> > > > What does it even mean for a PCI host bridge to have a child that is
> > > > also a PCI host bridge?
> > > >
> > > > Does this mean a driver binds to the "parent" host bridge, enumerates
> > > > the PCI devices below it, and finds a "child" host bridge?
> >
> > Yes, that is exactly what you can see on enumeration.
> >
> > The following is a typical boot trace where all bridges has a device also below:
> >
> > mt7621-pci 1e140000.pcie: host bridge /pcie@1e140000 ranges:
> > mt7621-pci 1e140000.pcie: No bus range found for /pcie@1e140000, using [bus 00-ff]
> > mt7621-pci 1e140000.pcie: MEM 0x0060000000..0x006fffffff -> 0x0060000000
> > mt7621-pci 1e140000.pcie: IO 0x001e160000..0x001e16ffff -> 0x0000000000
> > mt7621-pci 1e140000.pcie: PCIE0 enabled
> > mt7621-pci 1e140000.pcie: PCIE1 enabled
> > mt7621-pci 1e140000.pcie: PCIE2 enabled
> > mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
>
> 1e140000.pcie is a host bridge. It has some CPU-specific bus on the
> upstream side, standard PCI (domain 0000, buses 00-ff) on the
> downstream side.
>
> > pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> > pci 0000:00:01.0: [0e8d:0801] type 01 class 0x060400
> > pci 0000:00:02.0: [0e8d:0801] type 01 class 0x060400
>
> > pci 0000:01:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> > pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff pref]
>
> 00:00.0 looks like a PCIe Root Port to bus 01. This is not a host
> bridge; it's just a standard PCI-to-PCI bridge with PCI on both the
> upstream and downstream sides.
>
> > pci 0000:02:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:01.0: PCI bridge to [bus 02-ff]
> > pci 0000:00:01.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff pref]
>
> 00:01.0 is another Root Port to bus 02.
>
> > pci 0000:03:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:02.0: PCI bridge to [bus 03-ff]
> > pci 0000:00:02.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff pref]
> > pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
>
> And 00:02.0 is a third Root Port to bus 03.
>
> > pci 0000:00:00.0: PCI bridge to [bus 01]
> > pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:00.0: bridge window [mem 0x60000000-0x600fffff]
> > pci 0000:00:00.0: bridge window [mem 0x60100000-0x601fffff pref]
> > pci 0000:00:01.0: PCI bridge to [bus 02]
> > pci 0000:00:01.0: bridge window [io 0x1000-0x1fff]
> > pci 0000:00:01.0: bridge window [mem 0x60200000-0x602fffff]
> > pci 0000:00:01.0: bridge window [mem 0x60300000-0x603fffff pref]
> > pci 0000:00:02.0: PCI bridge to [bus 03]
> > pci 0000:00:02.0: bridge window [io 0x2000-0x2fff]
> > pci 0000:00:02.0: bridge window [mem 0x60400000-0x604fffff]
> >
> > > I think the question should be towards Mediatek folks. I don't know what
> > > this hardware is exactly, just looks like pci-pci-bridge. The driver
> > > calls the children host bridges as "ports".
> >
> > You can see the topology here in my first driver submit cover letter
> > message [0].
> >
> > [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/
>
> Nothing unusual here, this looks like the standard PCIe topology.
>
> What *might* be unusual is describing the Root Ports in DT. Since
> they are standard PCI devices, they shouldn't need DT description
> unless there's some unusual power/clock/reset control or something
> that is not discoverable via PCI enumeration.

It looks like it is necessary since every port has its own
configuration registers, phy, clock, reset and interrupt.

Thanks,
Sergio Paracuellos

> Bjorn

2024-04-11 14:24:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 07:39:17AM -0500, Bjorn Helgaas wrote:
> On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote:
> > On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
> > <[email protected]> wrote:
> > > On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> > > >> MT7621 PCI host bridge has children which apparently are also PCI host
> > > >> bridges, at least that's what the binding suggest.
> > > >
> > > > What does it even mean for a PCI host bridge to have a child that is
> > > > also a PCI host bridge?

It should say 'root port' instead as the binding description correctly
says.

> > > >
> > > > Does this mean a driver binds to the "parent" host bridge, enumerates
> > > > the PCI devices below it, and finds a "child" host bridge?
> >
> > Yes, that is exactly what you can see on enumeration.
> >
> > The following is a typical boot trace where all bridges has a device also below:
> >
> > mt7621-pci 1e140000.pcie: host bridge /pcie@1e140000 ranges:
> > mt7621-pci 1e140000.pcie: No bus range found for /pcie@1e140000, using [bus 00-ff]
> > mt7621-pci 1e140000.pcie: MEM 0x0060000000..0x006fffffff -> 0x0060000000
> > mt7621-pci 1e140000.pcie: IO 0x001e160000..0x001e16ffff -> 0x0000000000
> > mt7621-pci 1e140000.pcie: PCIE0 enabled
> > mt7621-pci 1e140000.pcie: PCIE1 enabled
> > mt7621-pci 1e140000.pcie: PCIE2 enabled
> > mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
>
> 1e140000.pcie is a host bridge. It has some CPU-specific bus on the
> upstream side, standard PCI (domain 0000, buses 00-ff) on the
> downstream side.
>
> > pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> > pci 0000:00:01.0: [0e8d:0801] type 01 class 0x060400
> > pci 0000:00:02.0: [0e8d:0801] type 01 class 0x060400
>
> > pci 0000:01:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> > pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff pref]
>
> 00:00.0 looks like a PCIe Root Port to bus 01. This is not a host
> bridge; it's just a standard PCI-to-PCI bridge with PCI on both the
> upstream and downstream sides.
>
> > pci 0000:02:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:01.0: PCI bridge to [bus 02-ff]
> > pci 0000:00:01.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:01.0: bridge window [mem 0x00000000-0x000fffff pref]
>
> 00:01.0 is another Root Port to bus 02.
>
> > pci 0000:03:00.0: [1b21:0611] type 00 class 0x010185
>
> > pci 0000:00:02.0: PCI bridge to [bus 03-ff]
> > pci 0000:00:02.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:02.0: bridge window [mem 0x00000000-0x000fffff pref]
> > pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
>
> And 00:02.0 is a third Root Port to bus 03.
>
> > pci 0000:00:00.0: PCI bridge to [bus 01]
> > pci 0000:00:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0000:00:00.0: bridge window [mem 0x60000000-0x600fffff]
> > pci 0000:00:00.0: bridge window [mem 0x60100000-0x601fffff pref]
> > pci 0000:00:01.0: PCI bridge to [bus 02]
> > pci 0000:00:01.0: bridge window [io 0x1000-0x1fff]
> > pci 0000:00:01.0: bridge window [mem 0x60200000-0x602fffff]
> > pci 0000:00:01.0: bridge window [mem 0x60300000-0x603fffff pref]
> > pci 0000:00:02.0: PCI bridge to [bus 03]
> > pci 0000:00:02.0: bridge window [io 0x2000-0x2fff]
> > pci 0000:00:02.0: bridge window [mem 0x60400000-0x604fffff]
> >
> > > I think the question should be towards Mediatek folks. I don't know what
> > > this hardware is exactly, just looks like pci-pci-bridge. The driver
> > > calls the children host bridges as "ports".
> >
> > You can see the topology here in my first driver submit cover letter
> > message [0].
> >
> > [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/
>
> Nothing unusual here, this looks like the standard PCIe topology.
>
> What *might* be unusual is describing the Root Ports in DT. Since
> they are standard PCI devices, they shouldn't need DT description
> unless there's some unusual power/clock/reset control or something
> that is not discoverable via PCI enumeration.

It's only unusual because typically there's only 1 RP per host bridge
and properties which really apply to the RP get stuck in the host bridge
node because we don't have a RP node. An example is perst-gpios. That's
not a property of the RP either, but the RP is the upstream side of a
slot and we often don't have a node for the device either.

Rob

2024-04-11 17:41:25

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 09:21:07AM -0500, Rob Herring wrote:
> On Thu, Apr 11, 2024 at 07:39:17AM -0500, Bjorn Helgaas wrote:
> > On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote:
> > > On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
> > > <[email protected]> wrote:
> > > > On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > > > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> > > > >> MT7621 PCI host bridge has children which apparently are also PCI host
> > > > >> bridges, at least that's what the binding suggest.
> > > > >
> > > > > What does it even mean for a PCI host bridge to have a child that is
> > > > > also a PCI host bridge?
>
> It should say 'root port' instead as the binding description correctly
> says.

OK, that makes a lot more sense, and we should fix the commit log.

> > > > I think the question should be towards Mediatek folks. I don't know what
> > > > this hardware is exactly, just looks like pci-pci-bridge. The driver
> > > > calls the children host bridges as "ports".
> > >
> > > You can see the topology here in my first driver submit cover letter
> > > message [0].
> > >
> > > [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/
> >
> > Nothing unusual here, this looks like the standard PCIe topology.
> >
> > What *might* be unusual is describing the Root Ports in DT. Since
> > they are standard PCI devices, they shouldn't need DT description
> > unless there's some unusual power/clock/reset control or something
> > that is not discoverable via PCI enumeration.
>
> It's only unusual because typically there's only 1 RP per host bridge
> and properties which really apply to the RP get stuck in the host bridge
> node because we don't have a RP node. An example is perst-gpios. That's
> not a property of the RP either, but the RP is the upstream side of a
> slot and we often don't have a node for the device either.

Makes sense.

I'm still confused about one thing, maybe just because I don't really
know how to read these bindings. The binding now looks like this:

properties:
compatible:
const: mediatek,mt7621-pci

reg:
items:
- description: host-pci bridge registers
- description: pcie port 0 RC control registers # A
- description: pcie port 1 RC control registers # A
- description: pcie port 2 RC control registers # A

patternProperties:
'^pcie@[0-2],0$':
type: object
$ref: /schemas/pci/pci-pci-bridge.yaml#

properties:
reg: # B
maxItems: 1

It looks like the "A" items are separate things from the "B" items?

But I think the relevant code is here:

mt7621_pcie_probe
mt7621_pcie_parse_dt
pcie->base = devm_platform_ioremap_resource(pdev, 0) # 1
for_each_available_child_of_node(node, child)
mt7621_pcie_parse_port
port->base = devm_platform_ioremap_resource(pdev, slot + 1) # 2

where it looks like both "1" and "2" use the items in the "A" list,
i.e., resources 0, 1, 2, 3, all from the same platform device. Is
there code that uses the "B" item that this patch adds?

Bjorn

2024-04-11 18:25:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: PCI: mediatek,mt7621: add missing child node reg

On Thu, Apr 11, 2024 at 11:11 AM Bjorn Helgaas <[email protected]> wrote:
>
> On Thu, Apr 11, 2024 at 09:21:07AM -0500, Rob Herring wrote:
> > On Thu, Apr 11, 2024 at 07:39:17AM -0500, Bjorn Helgaas wrote:
> > > On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote:
> > > > On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski
> > > > <[email protected]> wrote:
> > > > > On 10/04/2024 23:26, Bjorn Helgaas wrote:
> > > > > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote:
> > > > > >> MT7621 PCI host bridge has children which apparently are also PCI host
> > > > > >> bridges, at least that's what the binding suggest.
> > > > > >
> > > > > > What does it even mean for a PCI host bridge to have a child that is
> > > > > > also a PCI host bridge?
> >
> > It should say 'root port' instead as the binding description correctly
> > says.
>
> OK, that makes a lot more sense, and we should fix the commit log.
>
> > > > > I think the question should be towards Mediatek folks. I don't know what
> > > > > this hardware is exactly, just looks like pci-pci-bridge. The driver
> > > > > calls the children host bridges as "ports".
> > > >
> > > > You can see the topology here in my first driver submit cover letter
> > > > message [0].
> > > >
> > > > [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@mail.gmail.com/t/
> > >
> > > Nothing unusual here, this looks like the standard PCIe topology.
> > >
> > > What *might* be unusual is describing the Root Ports in DT. Since
> > > they are standard PCI devices, they shouldn't need DT description
> > > unless there's some unusual power/clock/reset control or something
> > > that is not discoverable via PCI enumeration.
> >
> > It's only unusual because typically there's only 1 RP per host bridge
> > and properties which really apply to the RP get stuck in the host bridge
> > node because we don't have a RP node. An example is perst-gpios. That's
> > not a property of the RP either, but the RP is the upstream side of a
> > slot and we often don't have a node for the device either.
>
> Makes sense.
>
> I'm still confused about one thing, maybe just because I don't really
> know how to read these bindings. The binding now looks like this:
>
> properties:
> compatible:
> const: mediatek,mt7621-pci
>
> reg:
> items:
> - description: host-pci bridge registers
> - description: pcie port 0 RC control registers # A
> - description: pcie port 1 RC control registers # A
> - description: pcie port 2 RC control registers # A
>
> patternProperties:
> '^pcie@[0-2],0$':
> type: object
> $ref: /schemas/pci/pci-pci-bridge.yaml#
>
> properties:
> reg: # B
> maxItems: 1
>
> It looks like the "A" items are separate things from the "B" items?
>
> But I think the relevant code is here:
>
> mt7621_pcie_probe
> mt7621_pcie_parse_dt
> pcie->base = devm_platform_ioremap_resource(pdev, 0) # 1
> for_each_available_child_of_node(node, child)
> mt7621_pcie_parse_port
> port->base = devm_platform_ioremap_resource(pdev, slot + 1) # 2
>
> where it looks like both "1" and "2" use the items in the "A" list,
> i.e., resources 0, 1, 2, 3, all from the same platform device. Is
> there code that uses the "B" item that this patch adds?

The A items are in the host address space. The B item is a PCI
address. Specifically, for PCI devices, the first entry is config
space with just the device and function (devfn). The format is defined
in the OpenFirmware PCI bus supplement.

Rob