linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
@ 2021-08-03  4:38 Mauro Carvalho Chehab
  2021-08-03  4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Binghui Wang,
	Gustavo Pimentel, Jingoo Han, Xiaowei Song, devicetree,
	linux-kernel, linux-pci, linux-phy

Hi Rob,

That's the third version of the DT bindings for Kirin 970 PCIE and its
corresponding PHY. 

It is identical to v2, except by:
	-          pcie@7,0 { // Lane 7: Ethernet
	+          pcie@7,0 { // Lane 6: Ethernet

at Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml

IMO, the best would be to merge this series via your tree, as it
depends on the patch converting the DT bindings for the PCIe DWC
driver.

v3:
  - Fixed a comment on patch 3: The Ethernet controller is at lane 6.

v2:
  - removed the DTS file. I'll submit it in separate, once having
    everything else merged;
  - it now doesn't produce any warnings with:
        make DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/hisilicon,kirin
-pcie.yaml DT_CHECKER_FLAGS=-m dt_binding_check
  - added the upstream node;
  - the clock enable now uses a new property (hisilicon,clken-gpios);
  - the reg for the PCI devices are now properly filled;
  - the pcie@x,y nodes now match the port number from table 4-1 from the
   datasheet.


Mauro Carvalho Chehab (4):
  dt-bindings: PCI: kirin: Fix compatible string
  dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml
  dt-bindings: PCI: kirin: Add support for Kirin970
  dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY

 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 160 ++++++++++++++++++
 .../devicetree/bindings/pci/kirin-pcie.txt    |  50 ------
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
 .../phy/hisilicon,phy-hi3670-pcie.yaml        |  86 ++++++++++
 MAINTAINERS                                   |   2 +-
 5 files changed, 248 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml

-- 
2.31.1



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string
  2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
@ 2021-08-03  4:38 ` Mauro Carvalho Chehab
  2021-08-03 22:22   ` Rob Herring
  2021-08-03  4:38 ` [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Bjorn Helgaas,
	devicetree, linux-kernel, linux-pci, Rob Herring

The pcie-kirin driver doesn't declare a hisilicon,kirin-pcie.
Also, remove the useless comment after the description, as other
compat will be supported by the same driver in the future.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 Documentation/devicetree/bindings/pci/kirin-pcie.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
index 7db30534498f..7adab8999a6a 100644
--- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
@@ -9,7 +9,7 @@ Additional properties are described here:
 
 Required properties
 - compatible:
-	"hisilicon,kirin960-pcie" for PCIe of Kirin960 SoC
+	"hisilicon,kirin960-pcie"
 - reg: Should contain rc_dbi, apb, phy, config registers location and length.
 - reg-names: Must include the following entries:
   "dbi": controller configuration registers;
@@ -23,7 +23,7 @@ Optional properties:
 Example based on kirin960:
 
 	pcie@f4000000 {
-		compatible = "hisilicon,kirin-pcie";
+		compatible = "hisilicon,kirin960-pcie";
 		reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>,
 		      <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>;
 		reg-names = "dbi","apb","phy", "config";
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml
  2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
  2021-08-03  4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
@ 2021-08-03  4:38 ` Mauro Carvalho Chehab
  2021-08-03 22:27   ` Rob Herring
  2021-08-03  4:38 ` [PATCH v3 3/4] dt-bindings: PCI: kirin: Add support for Kirin970 Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Binghui Wang,
	Bjorn Helgaas, Gustavo Pimentel, Jingoo Han, Xiaowei Song,
	devicetree, linux-kernel, linux-pci

Convert the file into a JSON description at the yaml format.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 86 +++++++++++++++++++
 .../devicetree/bindings/pci/kirin-pcie.txt    | 50 -----------
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
 MAINTAINERS                                   |  2 +-
 4 files changed, 88 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
new file mode 100644
index 000000000000..90cab09e8d4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/hisilicon,kirin-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon Kirin SoCs PCIe host DT description
+
+maintainers:
+  - Xiaowei Song <songxiaowei@hisilicon.com>
+  - Binghui Wang <wangbinghui@hisilicon.com>
+
+description: |
+  Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
+  It shares common functions with the PCIe DesignWare core driver and
+  inherits common properties defined in
+  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - hisilicon,kirin960-pcie
+
+  reg:
+    description: |
+      Should contain dbi, apb, config registers location and length.
+      For HiKey960, it should also contain phy.
+    minItems: 3
+    maxItems: 4
+
+  reg-names:
+    minItems: 3
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - reg-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/hi3660-clock.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie@f4000000 {
+        compatible = "hisilicon,kirin960-pcie";
+        reg = <0x0 0xf4000000 0x0 0x1000>,
+              <0x0 0xff3fe000 0x0 0x1000>,
+              <0x0 0xf3f20000 0x0 0x40000>,
+              <0x0 0xf5000000 0x0 0x2000>;
+        reg-names = "dbi", "apb", "phy", "config";
+        bus-range = <0x0  0x1>;
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        ranges = <0x02000000 0x0 0x00000000
+                  0x0 0xf6000000
+                  0x0 0x02000000>;
+        num-lanes = <1>;
+        #interrupt-cells = <1>;
+        interrupts = <0 283 4>;
+        interrupt-names = "msi";
+        interrupt-map-mask = <0xf800 0 0 7>;
+        interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
+                 <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
+                 <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
+                 <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
+                 <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
+        clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
+                      "pcie_apb_sys", "pcie_aclk";
+      };
+    };
diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
deleted file mode 100644
index 7adab8999a6a..000000000000
--- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-HiSilicon Kirin SoCs PCIe host DT description
-
-Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
-It shares common functions with the PCIe DesignWare core driver and
-inherits common properties defined in
-Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
-
-Additional properties are described here:
-
-Required properties
-- compatible:
-	"hisilicon,kirin960-pcie"
-- reg: Should contain rc_dbi, apb, phy, config registers location and length.
-- reg-names: Must include the following entries:
-  "dbi": controller configuration registers;
-  "apb": apb Ctrl register defined by Kirin;
-  "phy": apb PHY register defined by Kirin;
-  "config": PCIe configuration space registers.
-- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
-
-Optional properties:
-
-Example based on kirin960:
-
-	pcie@f4000000 {
-		compatible = "hisilicon,kirin960-pcie";
-		reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>,
-		      <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>;
-		reg-names = "dbi","apb","phy", "config";
-		bus-range = <0x0  0x1>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-		ranges = <0x02000000 0x0 0x00000000 0x0 0xf5000000 0x0 0x2000000>;
-		num-lanes = <1>;
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0xf800 0 0 7>;
-		interrupt-map = <0x0 0 0 1 &gic 0 0 0  282 4>,
-				<0x0 0 0 2 &gic 0 0 0  283 4>,
-				<0x0 0 0 3 &gic 0 0 0  284 4>,
-				<0x0 0 0 4 &gic 0 0 0  285 4>;
-		clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
-			 <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
-			 <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
-			 <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
-			 <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
-		clock-names = "pcie_phy_ref", "pcie_aux",
-			      "pcie_apb_phy", "pcie_apb_sys", "pcie_aclk";
-		reset-gpios = <&gpio11 1 0 >;
-	};
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index a8c1db879fb9..6c7501b8df01 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -34,7 +34,7 @@ properties:
     minItems: 2
     maxItems: 5
     items:
-      enum: [dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link]
+      enum: [dbi, dbi2, config, atu, apb, app, elbi, mgmt, ctrl, parf, cfg, link, phy]
 
   num-lanes:
     description: |
diff --git a/MAINTAINERS b/MAINTAINERS
index b54bd9dd07ec..d5f53b2d3f9c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14420,7 +14420,7 @@ M:	Xiaowei Song <songxiaowei@hisilicon.com>
 M:	Binghui Wang <wangbinghui@hisilicon.com>
 L:	linux-pci@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/pci/kirin-pcie.txt
+F:	Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 F:	drivers/pci/controller/dwc/pcie-kirin.c
 
 PCIE DRIVER FOR HISILICON STB
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3 3/4] dt-bindings: PCI: kirin: Add support for Kirin970
  2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
  2021-08-03  4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
  2021-08-03  4:38 ` [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml Mauro Carvalho Chehab
@ 2021-08-03  4:38 ` Mauro Carvalho Chehab
  2021-08-03  4:38 ` [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY Mauro Carvalho Chehab
  2021-08-03 22:11 ` [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Rob Herring
  4 siblings, 0 replies; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Binghui Wang,
	Bjorn Helgaas, Xiaowei Song, devicetree, linux-kernel, linux-pci

Add a new compatible, plus the new bindings needed by
HiKey970 board.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 76 ++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
index 90cab09e8d4b..c0551d2e606d 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -24,11 +24,12 @@ properties:
     contains:
       enum:
         - hisilicon,kirin960-pcie
+        - hisilicon,kirin970-pcie
 
   reg:
     description: |
       Should contain dbi, apb, config registers location and length.
-      For HiKey960, it should also contain phy.
+      For hisilicon,kirin960-pcie, it should also contain phy.
     minItems: 3
     maxItems: 4
 
@@ -36,6 +37,11 @@ properties:
     minItems: 3
     maxItems: 4
 
+  hisilicon,clken-gpios:
+    description: |
+      Clock input enablement GPIOs from PCI devices like Ethernet, M.2 and
+      mini-PCIe slots.
+
 required:
   - compatible
   - reg
@@ -47,6 +53,7 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/clock/hi3660-clock.h>
+    #include <dt-bindings/clock/hi3670-clock.h>
 
     soc {
       #address-cells = <2>;
@@ -83,4 +90,71 @@ examples:
         clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
                       "pcie_apb_sys", "pcie_aclk";
       };
+
+      pcie@f5000000 {
+        compatible = "hisilicon,kirin970-pcie";
+        reg = <0x0 0xf4000000 0x0 0x1000000>,
+              <0x0 0xfc180000 0x0 0x1000>,
+              <0x0 0xf5000000 0x0 0x2000>;
+        reg-names = "dbi", "apb", "config";
+        bus-range = <0x0  0x1>;
+        msi-parent = <&its_pcie>;
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        phys = <&pcie_phy>;
+        ranges = <0x02000000 0x0 0x00000000
+                  0x0 0xf6000000
+                  0x0 0x02000000>;
+        num-lanes = <1>;
+        #interrupt-cells = <1>;
+        interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "msi";
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+        reset-gpios = <&gpio7 0 0>;
+        hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>, <&gpio20 6 0>;
+
+        pcie@0 { // Lane 0: PCIe switch: Bus 1, Device 0
+          reg = <0 0 0 0 0>;
+          compatible = "pciclass,0604";
+          device_type = "pci";
+          #address-cells = <3>;
+          #size-cells = <2>;
+          ranges;
+          pcie@1,0 { // Lane 4: M.2
+            reg = <0x800 0 0 0 0>;
+            compatible = "pciclass,0604";
+            device_type = "pci";
+            reset-gpios = <&gpio3 1 0>;
+            clkreq-gpios = <&gpio27 3 0 >;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+          };
+          pcie@5,0 { // Lane 5: Mini PCIe
+            reg = <0x2800 0 0 0 0>;
+            compatible = "pciclass,0604";
+            device_type = "pci";
+            reset-gpios = <&gpio27 4 0 >;
+            clkreq-gpios = <&gpio17 0 0 >;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+          };
+          pcie@7,0 { // Lane 6: Ethernet
+            reg = <0x3800 0 0 0 0>;
+            compatible = "pciclass,0604";
+            device_type = "pci";
+            reset-gpios = <&gpio25 2 0 >;
+            clkreq-gpios = <&gpio20 6 0 >;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+          };
+        };
+      };
     };
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
  2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2021-08-03  4:38 ` [PATCH v3 3/4] dt-bindings: PCI: kirin: Add support for Kirin970 Mauro Carvalho Chehab
@ 2021-08-03  4:38 ` Mauro Carvalho Chehab
  2021-08-03 22:29   ` Rob Herring
  2021-08-03 22:11 ` [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Rob Herring
  4 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-03  4:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Kishon Vijay Abraham I, Vinod Koul, devicetree, linux-kernel,
	linux-phy

Document the bindings for HiKey 970 (hi3670) PCIe PHY
interface, supported via the pcie-kirin driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../phy/hisilicon,phy-hi3670-pcie.yaml        | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml

diff --git a/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
new file mode 100644
index 000000000000..1e0153e4f4a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3670-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon Kirin970 PCIe PHY
+
+maintainers:
+  - Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+
+description: |+
+  Bindings for PCIe PHY on HiSilicon Kirin 970.
+
+properties:
+  compatible:
+    const: hisilicon,hi970-pcie-phy
+
+  "#phy-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+    description: PHY Control registers
+
+  phy-supply:
+    description: The PCIe PHY power supply
+
+  clocks:
+    items:
+      - description: PCIe PHY clock
+      - description: PCIe AUX clock
+      - description: PCIe APB PHY clock
+      - description: PCIe APB SYS clock
+      - description: PCIe ACLK clock
+
+  clock-names:
+    items:
+      - const: phy_ref
+      - const: aux
+      - const: apb_phy
+      - const: apb_sys
+      - const: aclk
+
+  clkreq-gpios:
+    description: Clock request GPIOs
+    maxItems: 3
+
+  hisilicon,eye-diagram-param:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: Eye diagram for phy.
+
+required:
+  - "#phy-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - hisilicon,eye-diagram-param
+  - phy-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/hi3670-clock.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      pcie_phy: pcie-phy@fc000000 {
+        compatible = "hisilicon,hi970-pcie-phy";
+        reg = <0x0 0xfc000000 0x0 0x80000>;
+        #phy-cells = <0>;
+        phy-supply = <&ldo33>;
+        clocks = <&crg_ctrl HI3670_CLK_GATE_PCIEPHY_REF>,
+                 <&crg_ctrl HI3670_CLK_GATE_PCIEAUX>,
+                 <&crg_ctrl HI3670_PCLK_GATE_PCIE_PHY>,
+                 <&crg_ctrl HI3670_PCLK_GATE_PCIE_SYS>,
+                 <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
+        clock-names = "phy_ref", "aux",
+                      "apb_phy", "apb_sys", "aclk";
+        hisilicon,eye-diagram-param = <0xffffffff 0xffffffff
+                                       0xffffffff 0xffffffff 0xffffffff>;
+      };
+    };
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2021-08-03  4:38 ` [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY Mauro Carvalho Chehab
@ 2021-08-03 22:11 ` Rob Herring
  2021-08-04  6:50   ` Mauro Carvalho Chehab
  4 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-03 22:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Hi Rob,
>
> That's the third version of the DT bindings for Kirin 970 PCIE and its
> corresponding PHY.
>
> It is identical to v2, except by:
>         -          pcie@7,0 { // Lane 7: Ethernet
>         +          pcie@7,0 { // Lane 6: Ethernet

Can you check whether you have DT node links in sysfs for the PCI
devices? If you don't, then something is wrong still in the topology
or the PCI core is failing to set the DT node pointer in struct
device. Though you don't rely on that currently, we want the topology
to match. It's possible this never worked on arm/arm64 as mainly
powerpc relied on this.

I'd like some way to validate the DT matches the PCI topology. We
could have a tool that generates the DT structure based on the PCI
topology.

> at Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
>
> IMO, the best would be to merge this series via your tree, as it
> depends on the patch converting the DT bindings for the PCIe DWC
> driver.

Yes, agreed.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string
  2021-08-03  4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
@ 2021-08-03 22:22   ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2021-08-03 22:22 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Bjorn Helgaas, linuxarm, mauro.chehab, devicetree, linux-kernel,
	Rob Herring, linux-pci

On Tue, 03 Aug 2021 06:38:55 +0200, Mauro Carvalho Chehab wrote:
> The pcie-kirin driver doesn't declare a hisilicon,kirin-pcie.
> Also, remove the useless comment after the description, as other
> compat will be supported by the same driver in the future.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  Documentation/devicetree/bindings/pci/kirin-pcie.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Applied, thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml
  2021-08-03  4:38 ` [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml Mauro Carvalho Chehab
@ 2021-08-03 22:27   ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2021-08-03 22:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Binghui Wang, Bjorn Helgaas,
	Gustavo Pimentel, Jingoo Han, Xiaowei Song, devicetree,
	linux-kernel, linux-pci

On Tue, Aug 03, 2021 at 06:38:56AM +0200, Mauro Carvalho Chehab wrote:
> Convert the file into a JSON description at the yaml format.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../bindings/pci/hisilicon,kirin-pcie.yaml    | 86 +++++++++++++++++++
>  .../devicetree/bindings/pci/kirin-pcie.txt    | 50 -----------
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
>  MAINTAINERS                                   |  2 +-
>  4 files changed, 88 insertions(+), 52 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
>  delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt

This doesn't apply to my tree. I think the problem is I have some other 
snps,dw-pcie.yaml changes. Can you rebase and resend.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
  2021-08-03  4:38 ` [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY Mauro Carvalho Chehab
@ 2021-08-03 22:29   ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2021-08-03 22:29 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Kishon Vijay Abraham I, Vinod Koul,
	devicetree, linux-kernel, linux-phy

On Tue, Aug 03, 2021 at 06:38:58AM +0200, Mauro Carvalho Chehab wrote:
> Document the bindings for HiKey 970 (hi3670) PCIe PHY
> interface, supported via the pcie-kirin driver.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../phy/hisilicon,phy-hi3670-pcie.yaml        | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
> new file mode 100644
> index 000000000000..1e0153e4f4a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/hisilicon,phy-hi3670-pcie.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/hisilicon,phy-hi3670-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HiSilicon Kirin970 PCIe PHY
> +
> +maintainers:
> +  - Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> +
> +description: |+
> +  Bindings for PCIe PHY on HiSilicon Kirin 970.
> +
> +properties:
> +  compatible:
> +    const: hisilicon,hi970-pcie-phy
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  reg:
> +    maxItems: 1
> +    description: PHY Control registers
> +
> +  phy-supply:
> +    description: The PCIe PHY power supply
> +
> +  clocks:
> +    items:
> +      - description: PCIe PHY clock
> +      - description: PCIe AUX clock
> +      - description: PCIe APB PHY clock
> +      - description: PCIe APB SYS clock
> +      - description: PCIe ACLK clock
> +
> +  clock-names:
> +    items:
> +      - const: phy_ref
> +      - const: aux
> +      - const: apb_phy
> +      - const: apb_sys
> +      - const: aclk
> +
> +  clkreq-gpios:
> +    description: Clock request GPIOs
> +    maxItems: 3

This can be dropped now?

If not, at least use the same property name. (But really, why duplicate 
information in DT).

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-03 22:11 ` [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Rob Herring
@ 2021-08-04  6:50   ` Mauro Carvalho Chehab
  2021-08-04 16:28     ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-04  6:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Tue, 3 Aug 2021 16:11:42 -0600
Rob Herring <robh+dt@kernel.org> escreveu:

> On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Hi Rob,
> >
> > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > corresponding PHY.
> >
> > It is identical to v2, except by:
> >         -          pcie@7,0 { // Lane 7: Ethernet
> >         +          pcie@7,0 { // Lane 6: Ethernet  
> 
> Can you check whether you have DT node links in sysfs for the PCI
> devices? If you don't, then something is wrong still in the topology
> or the PCI core is failing to set the DT node pointer in struct
> device. Though you don't rely on that currently, we want the topology
> to match. It's possible this never worked on arm/arm64 as mainly
> powerpc relied on this.
>
> I'd like some way to validate the DT matches the PCI topology. We
> could have a tool that generates the DT structure based on the PCI
> topology.

The of_node node link is on those places:

	$ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
	/sys/devices/platform/soc/f4000000.pcie/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node

This is the pci stuff under firmware:

	$ tree /sys/firmware/devicetree/base/soc/pcie@f4000000/
	/sys/firmware/devicetree/base/soc/pcie@f4000000/
	├── #address-cells
	├── bus-range
	├── compatible
	├── device_type
	├── hisilicon,clken-gpios
	├── #interrupt-cells
	├── interrupt-map
	├── interrupt-map-mask
	├── interrupt-names
	├── interrupts
	├── msi-parent
	├── name
	├── num-lanes
	├── pcie@0,0
	│   ├── #address-cells
	│   ├── compatible
	│   ├── device_type
	│   ├── name
	│   ├── pcie@1,0
	│   │   ├── #address-cells
	│   │   ├── compatible
	│   │   ├── device_type
	│   │   ├── name
	│   │   ├── ranges
	│   │   ├── reg
	│   │   ├── reset-gpios
	│   │   └── #size-cells
	│   ├── pcie@5,0
	│   │   ├── #address-cells
	│   │   ├── compatible
	│   │   ├── device_type
	│   │   ├── name
	│   │   ├── ranges
	│   │   ├── reg
	│   │   ├── reset-gpios
	│   │   └── #size-cells
	│   ├── pcie@7,0
	│   │   ├── #address-cells
	│   │   ├── compatible
	│   │   ├── device_type
	│   │   ├── name
	│   │   ├── ranges
	│   │   ├── reg
	│   │   ├── reset-gpios
	│   │   └── #size-cells
	│   ├── ranges
	│   ├── reg
	│   └── #size-cells
	├── phys
	├── ranges
	├── reg
	├── reg-names
	├── reset-gpios
	└── #size-cells

And this is what we get from the pcie devnode:

	/sys/devices/platform/soc/f4000000.pcie/
	├── driver -> ../../../../bus/platform/drivers/kirin-pcie
	├── driver_override
	├── modalias
	├── of_node -> ../../../../firmware/devicetree/base/soc/pcie@f4000000
	├── pci0000:00
	│   ├── 0000:00:00.0
	│   │   ├── 0000:00:00.0:pcie001
	│   │   │   ├── driver -> ../../../../../../../bus/pci_express/drivers/pcie_pme
	│   │   │   ├── power
	│   │   │   ├── subsystem -> ../../../../../../../bus/pci_express
	│   │   │   └── uevent
	│   │   ├── 0000:00:00.0:pcie002
	│   │   │   ├── driver -> ../../../../../../../bus/pci_express/drivers/aer
	│   │   │   ├── power
	│   │   │   ├── subsystem -> ../../../../../../../bus/pci_express
	│   │   │   └── uevent
	│   │   ├── 0000:00:00.0:pcie010
	│   │   │   ├── power
	│   │   │   ├── subsystem -> ../../../../../../../bus/pci_express
	│   │   │   └── uevent
	│   │   ├── 0000:01:00.0
	│   │   │   ├── 0000:01:00.0:pcie102
	│   │   │   │   ├── power
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci_express
	│   │   │   │   └── uevent
	│   │   │   ├── 0000:02:01.0
	│   │   │   │   ├── 0000:02:01.0:pcie202
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:02:01.0:pcie210
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:03:00.0
	│   │   │   │   │   ├── aer_dev_correctable
	│   │   │   │   │   ├── aer_dev_fatal
	│   │   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   │   ├── ari_enabled
	│   │   │   │   │   ├── broken_parity_status
	│   │   │   │   │   ├── class
	│   │   │   │   │   ├── config
	│   │   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   │   ├── current_link_speed
	│   │   │   │   │   ├── current_link_width
	│   │   │   │   │   ├── d3cold_allowed
	│   │   │   │   │   ├── device
	│   │   │   │   │   ├── devspec
	│   │   │   │   │   ├── dma_mask_bits
	│   │   │   │   │   ├── driver -> ../../../../../../../../../bus/pci/drivers/nvme
	│   │   │   │   │   ├── driver_override
	│   │   │   │   │   ├── enable
	│   │   │   │   │   ├── irq
	│   │   │   │   │   ├── link
	│   │   │   │   │   │   ├── clkpm
	│   │   │   │   │   │   └── l1_aspm
	│   │   │   │   │   ├── local_cpulist
	│   │   │   │   │   ├── local_cpus
	│   │   │   │   │   ├── max_link_speed
	│   │   │   │   │   ├── max_link_width
	│   │   │   │   │   ├── modalias
	│   │   │   │   │   ├── msi_bus
	│   │   │   │   │   ├── numa_node
	│   │   │   │   │   ├── nvme
	│   │   │   │   │   │   └── nvme0
	│   │   │   │   │   │       ├── address
	│   │   │   │   │   │       ├── cntlid
	│   │   │   │   │   │       ├── dev
	│   │   │   │   │   │       ├── device -> ../../../0000:03:00.0
	│   │   │   │   │   │       ├── firmware_rev
	│   │   │   │   │   │       ├── kato
	│   │   │   │   │   │       ├── model
	│   │   │   │   │   │       ├── ng0n1
	│   │   │   │   │   │       │   ├── dev
	│   │   │   │   │   │       │   ├── device -> ../../nvme0
	│   │   │   │   │   │       │   ├── power
	│   │   │   │   │   │       │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   │   ├── control
	│   │   │   │   │   │       │   │   ├── runtime_active_time
	│   │   │   │   │   │       │   │   ├── runtime_status
	│   │   │   │   │   │       │   │   └── runtime_suspended_time
	│   │   │   │   │   │       │   ├── subsystem -> ../../../../../../../../../../../../class/nvme-generic
	│   │   │   │   │   │       │   └── uevent
	│   │   │   │   │   │       ├── numa_node
	│   │   │   │   │   │       ├── nvme0n1
	│   │   │   │   │   │       │   ├── alignment_offset
	│   │   │   │   │   │       │   ├── bdi -> ../../../../../../../../../../../virtual/bdi/259:0
	│   │   │   │   │   │       │   ├── capability
	│   │   │   │   │   │       │   ├── dev
	│   │   │   │   │   │       │   ├── device -> ../../nvme0
	│   │   │   │   │   │       │   ├── discard_alignment
	│   │   │   │   │   │       │   ├── eui
	│   │   │   │   │   │       │   ├── events
	│   │   │   │   │   │       │   ├── events_async
	│   │   │   │   │   │       │   ├── events_poll_msecs
	│   │   │   │   │   │       │   ├── ext_range
	│   │   │   │   │   │       │   ├── hidden
	│   │   │   │   │   │       │   ├── holders
	│   │   │   │   │   │       │   ├── inflight
	│   │   │   │   │   │       │   ├── integrity
	│   │   │   │   │   │       │   │   ├── device_is_integrity_capable
	│   │   │   │   │   │       │   │   ├── format
	│   │   │   │   │   │       │   │   ├── protection_interval_bytes
	│   │   │   │   │   │       │   │   ├── read_verify
	│   │   │   │   │   │       │   │   ├── tag_size
	│   │   │   │   │   │       │   │   └── write_generate
	│   │   │   │   │   │       │   ├── mq
	│   │   │   │   │   │       │   │   └── 0
	│   │   │   │   │   │       │   │       ├── cpu0
	│   │   │   │   │   │       │   │       ├── cpu1
	│   │   │   │   │   │       │   │       ├── cpu2
	│   │   │   │   │   │       │   │       ├── cpu3
	│   │   │   │   │   │       │   │       ├── cpu4
	│   │   │   │   │   │       │   │       ├── cpu5
	│   │   │   │   │   │       │   │       ├── cpu6
	│   │   │   │   │   │       │   │       ├── cpu7
	│   │   │   │   │   │       │   │       ├── cpu_list
	│   │   │   │   │   │       │   │       ├── nr_reserved_tags
	│   │   │   │   │   │       │   │       └── nr_tags
	│   │   │   │   │   │       │   ├── nsid
	│   │   │   │   │   │       │   ├── nvme0n1p1
	│   │   │   │   │   │       │   │   ├── alignment_offset
	│   │   │   │   │   │       │   │   ├── dev
	│   │   │   │   │   │       │   │   ├── discard_alignment
	│   │   │   │   │   │       │   │   ├── holders
	│   │   │   │   │   │       │   │   ├── inflight
	│   │   │   │   │   │       │   │   ├── partition
	│   │   │   │   │   │       │   │   ├── power
	│   │   │   │   │   │       │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   │   │   ├── control
	│   │   │   │   │   │       │   │   │   ├── runtime_active_time
	│   │   │   │   │   │       │   │   │   ├── runtime_status
	│   │   │   │   │   │       │   │   │   └── runtime_suspended_time
	│   │   │   │   │   │       │   │   ├── ro
	│   │   │   │   │   │       │   │   ├── size
	│   │   │   │   │   │       │   │   ├── start
	│   │   │   │   │   │       │   │   ├── stat
	│   │   │   │   │   │       │   │   ├── subsystem -> ../../../../../../../../../../../../../class/block
	│   │   │   │   │   │       │   │   └── uevent
	│   │   │   │   │   │       │   ├── power
	│   │   │   │   │   │       │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   │   ├── control
	│   │   │   │   │   │       │   │   ├── runtime_active_time
	│   │   │   │   │   │       │   │   ├── runtime_status
	│   │   │   │   │   │       │   │   └── runtime_suspended_time
	│   │   │   │   │   │       │   ├── queue
	│   │   │   │   │   │       │   │   ├── add_random
	│   │   │   │   │   │       │   │   ├── chunk_sectors
	│   │   │   │   │   │       │   │   ├── dax
	│   │   │   │   │   │       │   │   ├── discard_granularity
	│   │   │   │   │   │       │   │   ├── discard_max_bytes
	│   │   │   │   │   │       │   │   ├── discard_max_hw_bytes
	│   │   │   │   │   │       │   │   ├── discard_zeroes_data
	│   │   │   │   │   │       │   │   ├── fua
	│   │   │   │   │   │       │   │   ├── hw_sector_size
	│   │   │   │   │   │       │   │   ├── io_poll
	│   │   │   │   │   │       │   │   ├── io_poll_delay
	│   │   │   │   │   │       │   │   ├── iosched
	│   │   │   │   │   │       │   │   │   ├── aging_expire
	│   │   │   │   │   │       │   │   │   ├── async_depth
	│   │   │   │   │   │       │   │   │   ├── fifo_batch
	│   │   │   │   │   │       │   │   │   ├── front_merges
	│   │   │   │   │   │       │   │   │   ├── read_expire
	│   │   │   │   │   │       │   │   │   ├── write_expire
	│   │   │   │   │   │       │   │   │   └── writes_starved
	│   │   │   │   │   │       │   │   ├── iostats
	│   │   │   │   │   │       │   │   ├── io_timeout
	│   │   │   │   │   │       │   │   ├── logical_block_size
	│   │   │   │   │   │       │   │   ├── max_discard_segments
	│   │   │   │   │   │       │   │   ├── max_hw_sectors_kb
	│   │   │   │   │   │       │   │   ├── max_integrity_segments
	│   │   │   │   │   │       │   │   ├── max_sectors_kb
	│   │   │   │   │   │       │   │   ├── max_segments
	│   │   │   │   │   │       │   │   ├── max_segment_size
	│   │   │   │   │   │       │   │   ├── minimum_io_size
	│   │   │   │   │   │       │   │   ├── nomerges
	│   │   │   │   │   │       │   │   ├── nr_requests
	│   │   │   │   │   │       │   │   ├── nr_zones
	│   │   │   │   │   │       │   │   ├── optimal_io_size
	│   │   │   │   │   │       │   │   ├── physical_block_size
	│   │   │   │   │   │       │   │   ├── read_ahead_kb
	│   │   │   │   │   │       │   │   ├── rotational
	│   │   │   │   │   │       │   │   ├── rq_affinity
	│   │   │   │   │   │       │   │   ├── scheduler
	│   │   │   │   │   │       │   │   ├── stable_writes
	│   │   │   │   │   │       │   │   ├── virt_boundary_mask
	│   │   │   │   │   │       │   │   ├── wbt_lat_usec
	│   │   │   │   │   │       │   │   ├── write_cache
	│   │   │   │   │   │       │   │   ├── write_same_max_bytes
	│   │   │   │   │   │       │   │   ├── write_zeroes_max_bytes
	│   │   │   │   │   │       │   │   ├── zone_append_max_bytes
	│   │   │   │   │   │       │   │   ├── zoned
	│   │   │   │   │   │       │   │   └── zone_write_granularity
	│   │   │   │   │   │       │   ├── range
	│   │   │   │   │   │       │   ├── removable
	│   │   │   │   │   │       │   ├── ro
	│   │   │   │   │   │       │   ├── size
	│   │   │   │   │   │       │   ├── slaves
	│   │   │   │   │   │       │   ├── stat
	│   │   │   │   │   │       │   ├── subsystem -> ../../../../../../../../../../../../class/block
	│   │   │   │   │   │       │   ├── uevent
	│   │   │   │   │   │       │   └── wwid
	│   │   │   │   │   │       ├── power
	│   │   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   ├── control
	│   │   │   │   │   │       │   ├── pm_qos_latency_tolerance_us
	│   │   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │   │       ├── queue_count
	│   │   │   │   │   │       ├── rescan_controller
	│   │   │   │   │   │       ├── reset_controller
	│   │   │   │   │   │       ├── serial
	│   │   │   │   │   │       ├── sqsize
	│   │   │   │   │   │       ├── state
	│   │   │   │   │   │       ├── subsysnqn
	│   │   │   │   │   │       ├── subsystem -> ../../../../../../../../../../../class/nvme
	│   │   │   │   │   │       ├── transport
	│   │   │   │   │   │       └── uevent
	│   │   │   │   │   ├── pools
	│   │   │   │   │   ├── power
	│   │   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │   ├── control
	│   │   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   │   ├── runtime_status
	│   │   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   │   ├── wakeup
	│   │   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   │   ├── power_state
	│   │   │   │   │   ├── remove
	│   │   │   │   │   ├── rescan
	│   │   │   │   │   ├── reset
	│   │   │   │   │   ├── resource
	│   │   │   │   │   ├── resource0
	│   │   │   │   │   ├── revision
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci
	│   │   │   │   │   ├── subsystem_device
	│   │   │   │   │   ├── subsystem_vendor
	│   │   │   │   │   ├── uevent
	│   │   │   │   │   └── vendor
	│   │   │   │   ├── aer_dev_correctable
	│   │   │   │   ├── aer_dev_fatal
	│   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   ├── ari_enabled
	│   │   │   │   ├── broken_parity_status
	│   │   │   │   ├── class
	│   │   │   │   ├── config
	│   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   ├── current_link_speed
	│   │   │   │   ├── current_link_width
	│   │   │   │   ├── d3cold_allowed
	│   │   │   │   ├── device
	│   │   │   │   ├── devspec
	│   │   │   │   ├── dma_mask_bits
	│   │   │   │   ├── driver -> ../../../../../../../../bus/pci/drivers/pcieport
	│   │   │   │   ├── driver_override
	│   │   │   │   ├── enable
	│   │   │   │   ├── irq
	│   │   │   │   ├── link
	│   │   │   │   ├── local_cpulist
	│   │   │   │   ├── local_cpus
	│   │   │   │   ├── max_link_speed
	│   │   │   │   ├── max_link_width
	│   │   │   │   ├── modalias
	│   │   │   │   ├── msi_bus
	│   │   │   │   ├── numa_node
	│   │   │   │   ├── pci_bus
	│   │   │   │   │   └── 0000:03
	│   │   │   │   │       ├── cpuaffinity
	│   │   │   │   │       ├── cpulistaffinity
	│   │   │   │   │       ├── device -> ../../../0000:02:01.0
	│   │   │   │   │       ├── power
	│   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │       │   ├── control
	│   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │       ├── rescan
	│   │   │   │   │       ├── subsystem -> ../../../../../../../../../../class/pci_bus
	│   │   │   │   │       └── uevent
	│   │   │   │   ├── power
	│   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   ├── control
	│   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   ├── runtime_status
	│   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   ├── wakeup
	│   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   ├── power_state
	│   │   │   │   ├── remove
	│   │   │   │   ├── rescan
	│   │   │   │   ├── reset
	│   │   │   │   ├── resource
	│   │   │   │   ├── revision
	│   │   │   │   ├── secondary_bus_number
	│   │   │   │   ├── subordinate_bus_number
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci
	│   │   │   │   ├── subsystem_device
	│   │   │   │   ├── subsystem_vendor
	│   │   │   │   ├── uevent
	│   │   │   │   └── vendor
	│   │   │   ├── 0000:02:04.0
	│   │   │   │   ├── 0000:02:04.0:pcie202
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:02:04.0:pcie210
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── aer_dev_correctable
	│   │   │   │   ├── aer_dev_fatal
	│   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   ├── ari_enabled
	│   │   │   │   ├── broken_parity_status
	│   │   │   │   ├── class
	│   │   │   │   ├── config
	│   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   ├── current_link_speed
	│   │   │   │   ├── current_link_width
	│   │   │   │   ├── d3cold_allowed
	│   │   │   │   ├── device
	│   │   │   │   ├── devspec
	│   │   │   │   ├── dma_mask_bits
	│   │   │   │   ├── driver -> ../../../../../../../../bus/pci/drivers/pcieport
	│   │   │   │   ├── driver_override
	│   │   │   │   ├── enable
	│   │   │   │   ├── irq
	│   │   │   │   ├── link
	│   │   │   │   ├── local_cpulist
	│   │   │   │   ├── local_cpus
	│   │   │   │   ├── max_link_speed
	│   │   │   │   ├── max_link_width
	│   │   │   │   ├── modalias
	│   │   │   │   ├── msi_bus
	│   │   │   │   ├── numa_node
	│   │   │   │   ├── pci_bus
	│   │   │   │   │   └── 0000:04
	│   │   │   │   │       ├── cpuaffinity
	│   │   │   │   │       ├── cpulistaffinity
	│   │   │   │   │       ├── device -> ../../../0000:02:04.0
	│   │   │   │   │       ├── power
	│   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │       │   ├── control
	│   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │       ├── rescan
	│   │   │   │   │       ├── subsystem -> ../../../../../../../../../../class/pci_bus
	│   │   │   │   │       └── uevent
	│   │   │   │   ├── power
	│   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   ├── control
	│   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   ├── runtime_status
	│   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   ├── wakeup
	│   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   ├── power_state
	│   │   │   │   ├── remove
	│   │   │   │   ├── rescan
	│   │   │   │   ├── resource
	│   │   │   │   ├── revision
	│   │   │   │   ├── secondary_bus_number
	│   │   │   │   ├── subordinate_bus_number
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci
	│   │   │   │   ├── subsystem_device
	│   │   │   │   ├── subsystem_vendor
	│   │   │   │   ├── uevent
	│   │   │   │   └── vendor
	│   │   │   ├── 0000:02:05.0
	│   │   │   │   ├── 0000:02:05.0:pcie202
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:02:05.0:pcie210
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:05:00.0
	│   │   │   │   │   ├── aer_dev_correctable
	│   │   │   │   │   ├── aer_dev_fatal
	│   │   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   │   ├── ari_enabled
	│   │   │   │   │   ├── broken_parity_status
	│   │   │   │   │   ├── class
	│   │   │   │   │   ├── config
	│   │   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   │   ├── current_link_speed
	│   │   │   │   │   ├── current_link_width
	│   │   │   │   │   ├── d3cold_allowed
	│   │   │   │   │   ├── device
	│   │   │   │   │   ├── devspec
	│   │   │   │   │   ├── dma_mask_bits
	│   │   │   │   │   ├── driver_override
	│   │   │   │   │   ├── enable
	│   │   │   │   │   ├── irq
	│   │   │   │   │   ├── link
	│   │   │   │   │   │   ├── clkpm
	│   │   │   │   │   │   ├── l0s_aspm
	│   │   │   │   │   │   └── l1_aspm
	│   │   │   │   │   ├── local_cpulist
	│   │   │   │   │   ├── local_cpus
	│   │   │   │   │   ├── max_link_speed
	│   │   │   │   │   ├── max_link_width
	│   │   │   │   │   ├── modalias
	│   │   │   │   │   ├── msi_bus
	│   │   │   │   │   ├── numa_node
	│   │   │   │   │   ├── power
	│   │   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │   ├── control
	│   │   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   │   ├── runtime_status
	│   │   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   │   ├── wakeup
	│   │   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   │   ├── power_state
	│   │   │   │   │   ├── remove
	│   │   │   │   │   ├── rescan
	│   │   │   │   │   ├── reset
	│   │   │   │   │   ├── resource
	│   │   │   │   │   ├── resource0
	│   │   │   │   │   ├── revision
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci
	│   │   │   │   │   ├── subsystem_device
	│   │   │   │   │   ├── subsystem_vendor
	│   │   │   │   │   ├── uevent
	│   │   │   │   │   └── vendor
	│   │   │   │   ├── aer_dev_correctable
	│   │   │   │   ├── aer_dev_fatal
	│   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   ├── ari_enabled
	│   │   │   │   ├── broken_parity_status
	│   │   │   │   ├── class
	│   │   │   │   ├── config
	│   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   ├── current_link_speed
	│   │   │   │   ├── current_link_width
	│   │   │   │   ├── d3cold_allowed
	│   │   │   │   ├── device
	│   │   │   │   ├── devspec
	│   │   │   │   ├── dma_mask_bits
	│   │   │   │   ├── driver -> ../../../../../../../../bus/pci/drivers/pcieport
	│   │   │   │   ├── driver_override
	│   │   │   │   ├── enable
	│   │   │   │   ├── irq
	│   │   │   │   ├── link
	│   │   │   │   ├── local_cpulist
	│   │   │   │   ├── local_cpus
	│   │   │   │   ├── max_link_speed
	│   │   │   │   ├── max_link_width
	│   │   │   │   ├── modalias
	│   │   │   │   ├── msi_bus
	│   │   │   │   ├── numa_node
	│   │   │   │   ├── pci_bus
	│   │   │   │   │   └── 0000:05
	│   │   │   │   │       ├── cpuaffinity
	│   │   │   │   │       ├── cpulistaffinity
	│   │   │   │   │       ├── device -> ../../../0000:02:05.0
	│   │   │   │   │       ├── power
	│   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │       │   ├── control
	│   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │       ├── rescan
	│   │   │   │   │       ├── subsystem -> ../../../../../../../../../../class/pci_bus
	│   │   │   │   │       └── uevent
	│   │   │   │   ├── power
	│   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   ├── control
	│   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   ├── runtime_status
	│   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   ├── wakeup
	│   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   ├── power_state
	│   │   │   │   ├── remove
	│   │   │   │   ├── rescan
	│   │   │   │   ├── resource
	│   │   │   │   ├── revision
	│   │   │   │   ├── secondary_bus_number
	│   │   │   │   ├── subordinate_bus_number
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci
	│   │   │   │   ├── subsystem_device
	│   │   │   │   ├── subsystem_vendor
	│   │   │   │   ├── uevent
	│   │   │   │   └── vendor
	│   │   │   ├── 0000:02:07.0
	│   │   │   │   ├── 0000:02:07.0:pcie202
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:02:07.0:pcie210
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:06:00.0
	│   │   │   │   │   ├── aer_dev_correctable
	│   │   │   │   │   ├── aer_dev_fatal
	│   │   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   │   ├── ari_enabled
	│   │   │   │   │   ├── broken_parity_status
	│   │   │   │   │   ├── class
	│   │   │   │   │   ├── config
	│   │   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   │   ├── current_link_speed
	│   │   │   │   │   ├── current_link_width
	│   │   │   │   │   ├── d3cold_allowed
	│   │   │   │   │   ├── device
	│   │   │   │   │   ├── devspec
	│   │   │   │   │   ├── dma_mask_bits
	│   │   │   │   │   ├── driver -> ../../../../../../../../../bus/pci/drivers/r8169
	│   │   │   │   │   ├── driver_override
	│   │   │   │   │   ├── enable
	│   │   │   │   │   ├── irq
	│   │   │   │   │   ├── link
	│   │   │   │   │   │   ├── clkpm
	│   │   │   │   │   │   └── l1_aspm
	│   │   │   │   │   ├── local_cpulist
	│   │   │   │   │   ├── local_cpus
	│   │   │   │   │   ├── max_link_speed
	│   │   │   │   │   ├── max_link_width
	│   │   │   │   │   ├── mdio_bus
	│   │   │   │   │   │   └── r8169-600
	│   │   │   │   │   │       ├── device -> ../../../0000:06:00.0
	│   │   │   │   │   │       ├── power
	│   │   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   ├── control
	│   │   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │   │       ├── r8169-600:00
	│   │   │   │   │   │       │   ├── attached_dev -> ../../../net/enp6s0
	│   │   │   │   │   │       │   ├── driver -> ../../../../../../../../../../../../bus/mdio_bus/drivers/RTL8211E Gigabit Ethernet
	│   │   │   │   │   │       │   ├── phy_dev_flags
	│   │   │   │   │   │       │   ├── phy_has_fixups
	│   │   │   │   │   │       │   ├── phy_id
	│   │   │   │   │   │       │   ├── phy_interface
	│   │   │   │   │   │       │   ├── power
	│   │   │   │   │   │       │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   │   ├── control
	│   │   │   │   │   │       │   │   ├── runtime_active_time
	│   │   │   │   │   │       │   │   ├── runtime_status
	│   │   │   │   │   │       │   │   └── runtime_suspended_time
	│   │   │   │   │   │       │   ├── statistics
	│   │   │   │   │   │       │   │   ├── errors
	│   │   │   │   │   │       │   │   ├── reads
	│   │   │   │   │   │       │   │   ├── transfers
	│   │   │   │   │   │       │   │   └── writes
	│   │   │   │   │   │       │   ├── subsystem -> ../../../../../../../../../../../../bus/mdio_bus
	│   │   │   │   │   │       │   └── uevent
	│   │   │   │   │   │       ├── statistics
	│   │   │   │   │   │       │   ├── errors
	│   │   │   │   │   │       │   ├── errors_0
	│   │   │   │   │   │       │   ├── errors_1
	│   │   │   │   │   │       │   ├── errors_10
	│   │   │   │   │   │       │   ├── errors_11
	│   │   │   │   │   │       │   ├── errors_12
	│   │   │   │   │   │       │   ├── errors_13
	│   │   │   │   │   │       │   ├── errors_14
	│   │   │   │   │   │       │   ├── errors_15
	│   │   │   │   │   │       │   ├── errors_16
	│   │   │   │   │   │       │   ├── errors_17
	│   │   │   │   │   │       │   ├── errors_18
	│   │   │   │   │   │       │   ├── errors_19
	│   │   │   │   │   │       │   ├── errors_2
	│   │   │   │   │   │       │   ├── errors_20
	│   │   │   │   │   │       │   ├── errors_21
	│   │   │   │   │   │       │   ├── errors_22
	│   │   │   │   │   │       │   ├── errors_23
	│   │   │   │   │   │       │   ├── errors_24
	│   │   │   │   │   │       │   ├── errors_25
	│   │   │   │   │   │       │   ├── errors_26
	│   │   │   │   │   │       │   ├── errors_27
	│   │   │   │   │   │       │   ├── errors_28
	│   │   │   │   │   │       │   ├── errors_29
	│   │   │   │   │   │       │   ├── errors_3
	│   │   │   │   │   │       │   ├── errors_30
	│   │   │   │   │   │       │   ├── errors_31
	│   │   │   │   │   │       │   ├── errors_4
	│   │   │   │   │   │       │   ├── errors_5
	│   │   │   │   │   │       │   ├── errors_6
	│   │   │   │   │   │       │   ├── errors_7
	│   │   │   │   │   │       │   ├── errors_8
	│   │   │   │   │   │       │   ├── errors_9
	│   │   │   │   │   │       │   ├── reads
	│   │   │   │   │   │       │   ├── reads_0
	│   │   │   │   │   │       │   ├── reads_1
	│   │   │   │   │   │       │   ├── reads_10
	│   │   │   │   │   │       │   ├── reads_11
	│   │   │   │   │   │       │   ├── reads_12
	│   │   │   │   │   │       │   ├── reads_13
	│   │   │   │   │   │       │   ├── reads_14
	│   │   │   │   │   │       │   ├── reads_15
	│   │   │   │   │   │       │   ├── reads_16
	│   │   │   │   │   │       │   ├── reads_17
	│   │   │   │   │   │       │   ├── reads_18
	│   │   │   │   │   │       │   ├── reads_19
	│   │   │   │   │   │       │   ├── reads_2
	│   │   │   │   │   │       │   ├── reads_20
	│   │   │   │   │   │       │   ├── reads_21
	│   │   │   │   │   │       │   ├── reads_22
	│   │   │   │   │   │       │   ├── reads_23
	│   │   │   │   │   │       │   ├── reads_24
	│   │   │   │   │   │       │   ├── reads_25
	│   │   │   │   │   │       │   ├── reads_26
	│   │   │   │   │   │       │   ├── reads_27
	│   │   │   │   │   │       │   ├── reads_28
	│   │   │   │   │   │       │   ├── reads_29
	│   │   │   │   │   │       │   ├── reads_3
	│   │   │   │   │   │       │   ├── reads_30
	│   │   │   │   │   │       │   ├── reads_31
	│   │   │   │   │   │       │   ├── reads_4
	│   │   │   │   │   │       │   ├── reads_5
	│   │   │   │   │   │       │   ├── reads_6
	│   │   │   │   │   │       │   ├── reads_7
	│   │   │   │   │   │       │   ├── reads_8
	│   │   │   │   │   │       │   ├── reads_9
	│   │   │   │   │   │       │   ├── transfers
	│   │   │   │   │   │       │   ├── transfers_0
	│   │   │   │   │   │       │   ├── transfers_1
	│   │   │   │   │   │       │   ├── transfers_10
	│   │   │   │   │   │       │   ├── transfers_11
	│   │   │   │   │   │       │   ├── transfers_12
	│   │   │   │   │   │       │   ├── transfers_13
	│   │   │   │   │   │       │   ├── transfers_14
	│   │   │   │   │   │       │   ├── transfers_15
	│   │   │   │   │   │       │   ├── transfers_16
	│   │   │   │   │   │       │   ├── transfers_17
	│   │   │   │   │   │       │   ├── transfers_18
	│   │   │   │   │   │       │   ├── transfers_19
	│   │   │   │   │   │       │   ├── transfers_2
	│   │   │   │   │   │       │   ├── transfers_20
	│   │   │   │   │   │       │   ├── transfers_21
	│   │   │   │   │   │       │   ├── transfers_22
	│   │   │   │   │   │       │   ├── transfers_23
	│   │   │   │   │   │       │   ├── transfers_24
	│   │   │   │   │   │       │   ├── transfers_25
	│   │   │   │   │   │       │   ├── transfers_26
	│   │   │   │   │   │       │   ├── transfers_27
	│   │   │   │   │   │       │   ├── transfers_28
	│   │   │   │   │   │       │   ├── transfers_29
	│   │   │   │   │   │       │   ├── transfers_3
	│   │   │   │   │   │       │   ├── transfers_30
	│   │   │   │   │   │       │   ├── transfers_31
	│   │   │   │   │   │       │   ├── transfers_4
	│   │   │   │   │   │       │   ├── transfers_5
	│   │   │   │   │   │       │   ├── transfers_6
	│   │   │   │   │   │       │   ├── transfers_7
	│   │   │   │   │   │       │   ├── transfers_8
	│   │   │   │   │   │       │   ├── transfers_9
	│   │   │   │   │   │       │   ├── writes
	│   │   │   │   │   │       │   ├── writes_0
	│   │   │   │   │   │       │   ├── writes_1
	│   │   │   │   │   │       │   ├── writes_10
	│   │   │   │   │   │       │   ├── writes_11
	│   │   │   │   │   │       │   ├── writes_12
	│   │   │   │   │   │       │   ├── writes_13
	│   │   │   │   │   │       │   ├── writes_14
	│   │   │   │   │   │       │   ├── writes_15
	│   │   │   │   │   │       │   ├── writes_16
	│   │   │   │   │   │       │   ├── writes_17
	│   │   │   │   │   │       │   ├── writes_18
	│   │   │   │   │   │       │   ├── writes_19
	│   │   │   │   │   │       │   ├── writes_2
	│   │   │   │   │   │       │   ├── writes_20
	│   │   │   │   │   │       │   ├── writes_21
	│   │   │   │   │   │       │   ├── writes_22
	│   │   │   │   │   │       │   ├── writes_23
	│   │   │   │   │   │       │   ├── writes_24
	│   │   │   │   │   │       │   ├── writes_25
	│   │   │   │   │   │       │   ├── writes_26
	│   │   │   │   │   │       │   ├── writes_27
	│   │   │   │   │   │       │   ├── writes_28
	│   │   │   │   │   │       │   ├── writes_29
	│   │   │   │   │   │       │   ├── writes_3
	│   │   │   │   │   │       │   ├── writes_30
	│   │   │   │   │   │       │   ├── writes_31
	│   │   │   │   │   │       │   ├── writes_4
	│   │   │   │   │   │       │   ├── writes_5
	│   │   │   │   │   │       │   ├── writes_6
	│   │   │   │   │   │       │   ├── writes_7
	│   │   │   │   │   │       │   ├── writes_8
	│   │   │   │   │   │       │   └── writes_9
	│   │   │   │   │   │       ├── subsystem -> ../../../../../../../../../../../class/mdio_bus
	│   │   │   │   │   │       └── uevent
	│   │   │   │   │   ├── modalias
	│   │   │   │   │   ├── msi_bus
	│   │   │   │   │   ├── net
	│   │   │   │   │   │   └── enp6s0
	│   │   │   │   │   │       ├── addr_assign_type
	│   │   │   │   │   │       ├── address
	│   │   │   │   │   │       ├── addr_len
	│   │   │   │   │   │       ├── broadcast
	│   │   │   │   │   │       ├── carrier
	│   │   │   │   │   │       ├── carrier_changes
	│   │   │   │   │   │       ├── carrier_down_count
	│   │   │   │   │   │       ├── carrier_up_count
	│   │   │   │   │   │       ├── device -> ../../../0000:06:00.0
	│   │   │   │   │   │       ├── dev_id
	│   │   │   │   │   │       ├── dev_port
	│   │   │   │   │   │       ├── dormant
	│   │   │   │   │   │       ├── duplex
	│   │   │   │   │   │       ├── flags
	│   │   │   │   │   │       ├── gro_flush_timeout
	│   │   │   │   │   │       ├── ifalias
	│   │   │   │   │   │       ├── ifindex
	│   │   │   │   │   │       ├── iflink
	│   │   │   │   │   │       ├── link_mode
	│   │   │   │   │   │       ├── mtu
	│   │   │   │   │   │       ├── name_assign_type
	│   │   │   │   │   │       ├── napi_defer_hard_irqs
	│   │   │   │   │   │       ├── netdev_group
	│   │   │   │   │   │       ├── operstate
	│   │   │   │   │   │       ├── phydev -> ../../mdio_bus/r8169-600/r8169-600:00
	│   │   │   │   │   │       ├── phys_port_id
	│   │   │   │   │   │       ├── phys_port_name
	│   │   │   │   │   │       ├── phys_switch_id
	│   │   │   │   │   │       ├── power
	│   │   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │   │       │   ├── control
	│   │   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │   │       ├── proto_down
	│   │   │   │   │   │       ├── queues
	│   │   │   │   │   │       │   ├── rx-0
	│   │   │   │   │   │       │   │   ├── rps_cpus
	│   │   │   │   │   │       │   │   └── rps_flow_cnt
	│   │   │   │   │   │       │   └── tx-0
	│   │   │   │   │   │       │       ├── byte_queue_limits
	│   │   │   │   │   │       │       │   ├── hold_time
	│   │   │   │   │   │       │       │   ├── inflight
	│   │   │   │   │   │       │       │   ├── limit
	│   │   │   │   │   │       │       │   ├── limit_max
	│   │   │   │   │   │       │       │   └── limit_min
	│   │   │   │   │   │       │       ├── traffic_class
	│   │   │   │   │   │       │       ├── tx_maxrate
	│   │   │   │   │   │       │       ├── tx_timeout
	│   │   │   │   │   │       │       ├── xps_cpus
	│   │   │   │   │   │       │       └── xps_rxqs
	│   │   │   │   │   │       ├── speed
	│   │   │   │   │   │       ├── statistics
	│   │   │   │   │   │       │   ├── collisions
	│   │   │   │   │   │       │   ├── multicast
	│   │   │   │   │   │       │   ├── rx_bytes
	│   │   │   │   │   │       │   ├── rx_compressed
	│   │   │   │   │   │       │   ├── rx_crc_errors
	│   │   │   │   │   │       │   ├── rx_dropped
	│   │   │   │   │   │       │   ├── rx_errors
	│   │   │   │   │   │       │   ├── rx_fifo_errors
	│   │   │   │   │   │       │   ├── rx_frame_errors
	│   │   │   │   │   │       │   ├── rx_length_errors
	│   │   │   │   │   │       │   ├── rx_missed_errors
	│   │   │   │   │   │       │   ├── rx_nohandler
	│   │   │   │   │   │       │   ├── rx_over_errors
	│   │   │   │   │   │       │   ├── rx_packets
	│   │   │   │   │   │       │   ├── tx_aborted_errors
	│   │   │   │   │   │       │   ├── tx_bytes
	│   │   │   │   │   │       │   ├── tx_carrier_errors
	│   │   │   │   │   │       │   ├── tx_compressed
	│   │   │   │   │   │       │   ├── tx_dropped
	│   │   │   │   │   │       │   ├── tx_errors
	│   │   │   │   │   │       │   ├── tx_fifo_errors
	│   │   │   │   │   │       │   ├── tx_heartbeat_errors
	│   │   │   │   │   │       │   ├── tx_packets
	│   │   │   │   │   │       │   └── tx_window_errors
	│   │   │   │   │   │       ├── subsystem -> ../../../../../../../../../../../class/net
	│   │   │   │   │   │       ├── testing
	│   │   │   │   │   │       ├── threaded
	│   │   │   │   │   │       ├── tx_queue_len
	│   │   │   │   │   │       ├── type
	│   │   │   │   │   │       └── uevent
	│   │   │   │   │   ├── numa_node
	│   │   │   │   │   ├── power
	│   │   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   │   ├── control
	│   │   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   │   ├── runtime_status
	│   │   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   │   ├── wakeup
	│   │   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   │   ├── power_state
	│   │   │   │   │   ├── remove
	│   │   │   │   │   ├── rescan
	│   │   │   │   │   ├── reset
	│   │   │   │   │   ├── resource
	│   │   │   │   │   ├── resource2
	│   │   │   │   │   ├── resource4
	│   │   │   │   │   ├── resource4_wc
	│   │   │   │   │   ├── revision
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci
	│   │   │   │   │   ├── subsystem_device
	│   │   │   │   │   ├── subsystem_vendor
	│   │   │   │   │   ├── uevent
	│   │   │   │   │   ├── vendor
	│   │   │   │   │   └── vpd
	│   │   │   │   ├── aer_dev_correctable
	│   │   │   │   ├── aer_dev_fatal
	│   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   ├── ari_enabled
	│   │   │   │   ├── broken_parity_status
	│   │   │   │   ├── class
	│   │   │   │   ├── config
	│   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   ├── current_link_speed
	│   │   │   │   ├── current_link_width
	│   │   │   │   ├── d3cold_allowed
	│   │   │   │   ├── device
	│   │   │   │   ├── devspec
	│   │   │   │   ├── dma_mask_bits
	│   │   │   │   ├── driver -> ../../../../../../../../bus/pci/drivers/pcieport
	│   │   │   │   ├── driver_override
	│   │   │   │   ├── enable
	│   │   │   │   ├── irq
	│   │   │   │   ├── link
	│   │   │   │   ├── local_cpulist
	│   │   │   │   ├── local_cpus
	│   │   │   │   ├── max_link_speed
	│   │   │   │   ├── max_link_width
	│   │   │   │   ├── modalias
	│   │   │   │   ├── msi_bus
	│   │   │   │   ├── numa_node
	│   │   │   │   ├── pci_bus
	│   │   │   │   │   └── 0000:06
	│   │   │   │   │       ├── cpuaffinity
	│   │   │   │   │       ├── cpulistaffinity
	│   │   │   │   │       ├── device -> ../../../0000:02:07.0
	│   │   │   │   │       ├── power
	│   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │       │   ├── control
	│   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │       ├── rescan
	│   │   │   │   │       ├── subsystem -> ../../../../../../../../../../class/pci_bus
	│   │   │   │   │       └── uevent
	│   │   │   │   ├── power
	│   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   ├── control
	│   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   ├── runtime_status
	│   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   ├── wakeup
	│   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   ├── power_state
	│   │   │   │   ├── remove
	│   │   │   │   ├── rescan
	│   │   │   │   ├── resource
	│   │   │   │   ├── revision
	│   │   │   │   ├── secondary_bus_number
	│   │   │   │   ├── subordinate_bus_number
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci
	│   │   │   │   ├── subsystem_device
	│   │   │   │   ├── subsystem_vendor
	│   │   │   │   ├── uevent
	│   │   │   │   └── vendor
	│   │   │   ├── 0000:02:09.0
	│   │   │   │   ├── 0000:02:09.0:pcie202
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── 0000:02:09.0:pcie210
	│   │   │   │   │   ├── power
	│   │   │   │   │   ├── subsystem -> ../../../../../../../../../bus/pci_express
	│   │   │   │   │   └── uevent
	│   │   │   │   ├── aer_dev_correctable
	│   │   │   │   ├── aer_dev_fatal
	│   │   │   │   ├── aer_dev_nonfatal
	│   │   │   │   ├── ari_enabled
	│   │   │   │   ├── broken_parity_status
	│   │   │   │   ├── class
	│   │   │   │   ├── config
	│   │   │   │   ├── consistent_dma_mask_bits
	│   │   │   │   ├── current_link_speed
	│   │   │   │   ├── current_link_width
	│   │   │   │   ├── d3cold_allowed
	│   │   │   │   ├── device
	│   │   │   │   ├── devspec
	│   │   │   │   ├── dma_mask_bits
	│   │   │   │   ├── driver -> ../../../../../../../../bus/pci/drivers/pcieport
	│   │   │   │   ├── driver_override
	│   │   │   │   ├── enable
	│   │   │   │   ├── irq
	│   │   │   │   ├── link
	│   │   │   │   ├── local_cpulist
	│   │   │   │   ├── local_cpus
	│   │   │   │   ├── max_link_speed
	│   │   │   │   ├── max_link_width
	│   │   │   │   ├── modalias
	│   │   │   │   ├── msi_bus
	│   │   │   │   ├── numa_node
	│   │   │   │   ├── pci_bus
	│   │   │   │   │   └── 0000:07
	│   │   │   │   │       ├── cpuaffinity
	│   │   │   │   │       ├── cpulistaffinity
	│   │   │   │   │       ├── device -> ../../../0000:02:09.0
	│   │   │   │   │       ├── power
	│   │   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │   │       │   ├── control
	│   │   │   │   │       │   ├── runtime_active_time
	│   │   │   │   │       │   ├── runtime_status
	│   │   │   │   │       │   └── runtime_suspended_time
	│   │   │   │   │       ├── rescan
	│   │   │   │   │       ├── subsystem -> ../../../../../../../../../../class/pci_bus
	│   │   │   │   │       └── uevent
	│   │   │   │   ├── power
	│   │   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   │   ├── control
	│   │   │   │   │   ├── runtime_active_time
	│   │   │   │   │   ├── runtime_status
	│   │   │   │   │   ├── runtime_suspended_time
	│   │   │   │   │   ├── wakeup
	│   │   │   │   │   ├── wakeup_abort_count
	│   │   │   │   │   ├── wakeup_active
	│   │   │   │   │   ├── wakeup_active_count
	│   │   │   │   │   ├── wakeup_count
	│   │   │   │   │   ├── wakeup_expire_count
	│   │   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   │   └── wakeup_total_time_ms
	│   │   │   │   ├── power_state
	│   │   │   │   ├── remove
	│   │   │   │   ├── rescan
	│   │   │   │   ├── resource
	│   │   │   │   ├── revision
	│   │   │   │   ├── secondary_bus_number
	│   │   │   │   ├── subordinate_bus_number
	│   │   │   │   ├── subsystem -> ../../../../../../../../bus/pci
	│   │   │   │   ├── subsystem_device
	│   │   │   │   ├── subsystem_vendor
	│   │   │   │   ├── uevent
	│   │   │   │   └── vendor
	│   │   │   ├── aer_dev_correctable
	│   │   │   ├── aer_dev_fatal
	│   │   │   ├── aer_dev_nonfatal
	│   │   │   ├── ari_enabled
	│   │   │   ├── broken_parity_status
	│   │   │   ├── class
	│   │   │   ├── config
	│   │   │   ├── consistent_dma_mask_bits
	│   │   │   ├── current_link_speed
	│   │   │   ├── current_link_width
	│   │   │   ├── d3cold_allowed
	│   │   │   ├── device
	│   │   │   ├── devspec
	│   │   │   ├── dma_mask_bits
	│   │   │   ├── driver -> ../../../../../../../bus/pci/drivers/pcieport
	│   │   │   ├── driver_override
	│   │   │   ├── enable
	│   │   │   ├── irq
	│   │   │   ├── link
	│   │   │   │   ├── l0s_aspm
	│   │   │   │   └── l1_aspm
	│   │   │   ├── local_cpulist
	│   │   │   ├── local_cpus
	│   │   │   ├── max_link_speed
	│   │   │   ├── max_link_width
	│   │   │   ├── modalias
	│   │   │   ├── msi_bus
	│   │   │   ├── numa_node
	│   │   │   ├── pci_bus
	│   │   │   │   └── 0000:02
	│   │   │   │       ├── cpuaffinity
	│   │   │   │       ├── cpulistaffinity
	│   │   │   │       ├── device -> ../../../0000:01:00.0
	│   │   │   │       ├── power
	│   │   │   │       │   ├── autosuspend_delay_ms
	│   │   │   │       │   ├── control
	│   │   │   │       │   ├── runtime_active_time
	│   │   │   │       │   ├── runtime_status
	│   │   │   │       │   └── runtime_suspended_time
	│   │   │   │       ├── rescan
	│   │   │   │       ├── subsystem -> ../../../../../../../../../class/pci_bus
	│   │   │   │       └── uevent
	│   │   │   ├── power
	│   │   │   │   ├── autosuspend_delay_ms
	│   │   │   │   ├── control
	│   │   │   │   ├── runtime_active_time
	│   │   │   │   ├── runtime_status
	│   │   │   │   ├── runtime_suspended_time
	│   │   │   │   ├── wakeup
	│   │   │   │   ├── wakeup_abort_count
	│   │   │   │   ├── wakeup_active
	│   │   │   │   ├── wakeup_active_count
	│   │   │   │   ├── wakeup_count
	│   │   │   │   ├── wakeup_expire_count
	│   │   │   │   ├── wakeup_last_time_ms
	│   │   │   │   ├── wakeup_max_time_ms
	│   │   │   │   └── wakeup_total_time_ms
	│   │   │   ├── power_state
	│   │   │   ├── remove
	│   │   │   ├── rescan
	│   │   │   ├── reset
	│   │   │   ├── resource
	│   │   │   ├── resource0
	│   │   │   ├── revision
	│   │   │   ├── secondary_bus_number
	│   │   │   ├── subordinate_bus_number
	│   │   │   ├── subsystem -> ../../../../../../../bus/pci
	│   │   │   ├── subsystem_device
	│   │   │   ├── subsystem_vendor
	│   │   │   ├── uevent
	│   │   │   └── vendor
	│   │   ├── aer_dev_correctable
	│   │   ├── aer_dev_fatal
	│   │   ├── aer_dev_nonfatal
	│   │   ├── aer_rootport_total_err_cor
	│   │   ├── aer_rootport_total_err_fatal
	│   │   ├── aer_rootport_total_err_nonfatal
	│   │   ├── ari_enabled
	│   │   ├── broken_parity_status
	│   │   ├── class
	│   │   ├── config
	│   │   ├── consistent_dma_mask_bits
	│   │   ├── current_link_speed
	│   │   ├── current_link_width
	│   │   ├── d3cold_allowed
	│   │   ├── device
	│   │   ├── devspec
	│   │   ├── dma_mask_bits
	│   │   ├── driver -> ../../../../../../bus/pci/drivers/pcieport
	│   │   ├── driver_override
	│   │   ├── enable
	│   │   ├── irq
	│   │   ├── link
	│   │   ├── local_cpulist
	│   │   ├── local_cpus
	│   │   ├── max_link_speed
	│   │   ├── max_link_width
	│   │   ├── modalias
	│   │   ├── msi_bus
	│   │   ├── numa_node
	│   │   ├── of_node -> ../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
	│   │   ├── pci_bus
	│   │   │   └── 0000:01
	│   │   │       ├── cpuaffinity
	│   │   │       ├── cpulistaffinity
	│   │   │       ├── device -> ../../../0000:00:00.0
	│   │   │       ├── of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
	│   │   │       ├── power
	│   │   │       │   ├── autosuspend_delay_ms
	│   │   │       │   ├── control
	│   │   │       │   ├── runtime_active_time
	│   │   │       │   ├── runtime_status
	│   │   │       │   └── runtime_suspended_time
	│   │   │       ├── rescan
	│   │   │       ├── subsystem -> ../../../../../../../../class/pci_bus
	│   │   │       ├── uevent
	│   │   │       └── waiting_for_supplier
	│   │   ├── power
	│   │   │   ├── autosuspend_delay_ms
	│   │   │   ├── control
	│   │   │   ├── runtime_active_time
	│   │   │   ├── runtime_status
	│   │   │   ├── runtime_suspended_time
	│   │   │   ├── wakeup
	│   │   │   ├── wakeup_abort_count
	│   │   │   ├── wakeup_active
	│   │   │   ├── wakeup_active_count
	│   │   │   ├── wakeup_count
	│   │   │   ├── wakeup_expire_count
	│   │   │   ├── wakeup_last_time_ms
	│   │   │   ├── wakeup_max_time_ms
	│   │   │   └── wakeup_total_time_ms
	│   │   ├── power_state
	│   │   ├── remove
	│   │   ├── rescan
	│   │   ├── reset
	│   │   ├── resource
	│   │   ├── resource0
	│   │   ├── revision
	│   │   ├── secondary_bus_number
	│   │   ├── subordinate_bus_number
	│   │   ├── subsystem -> ../../../../../../bus/pci
	│   │   ├── subsystem_device
	│   │   ├── subsystem_vendor
	│   │   ├── uevent
	│   │   └── vendor
	│   ├── pci_bus
	│   │   └── 0000:00
	│   │       ├── cpuaffinity
	│   │       ├── cpulistaffinity
	│   │       ├── device -> ../../../pci0000:00
	│   │       ├── of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000
	│   │       ├── power
	│   │       │   ├── autosuspend_delay_ms
	│   │       │   ├── control
	│   │       │   ├── runtime_active_time
	│   │       │   ├── runtime_status
	│   │       │   └── runtime_suspended_time
	│   │       ├── rescan
	│   │       ├── subsystem -> ../../../../../../../class/pci_bus
	│   │       ├── uevent
	│   │       └── waiting_for_supplier
	│   ├── power
	│   │   ├── autosuspend_delay_ms
	│   │   ├── control
	│   │   ├── runtime_active_time
	│   │   ├── runtime_status
	│   │   └── runtime_suspended_time
	│   └── uevent
	├── power
	│   ├── autosuspend_delay_ms
	│   ├── control
	│   ├── runtime_active_time
	│   ├── runtime_status
	│   └── runtime_suspended_time
	├── subsystem -> ../../../../bus/platform
	├── supplier:amba:e8a12000.gpio -> ../../../virtual/devlink/amba:e8a12000.gpio--platform:f4000000.pcie
	├── supplier:amba:e8a1c000.gpio -> ../../../virtual/devlink/amba:e8a1c000.gpio--platform:f4000000.pcie
	├── supplier:amba:e8a1f000.gpio -> ../../../virtual/devlink/amba:e8a1f000.gpio--platform:f4000000.pcie
	├── supplier:amba:fff10000.gpio -> ../../../virtual/devlink/amba:fff10000.gpio--platform:f4000000.pcie
	├── supplier:phy:phy-fc000000.pcie-phy.1 -> ../../../virtual/devlink/phy:phy-fc000000.pcie-phy.1--platform:f4000000.pcie
	├── supplier:platform:fc000000.pcie-phy -> ../../../virtual/devlink/platform:fc000000.pcie-phy--platform:f4000000.pcie
	└── uevent

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-04  6:50   ` Mauro Carvalho Chehab
@ 2021-08-04 16:28     ` Rob Herring
  2021-08-05  7:46       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-04 16:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 3 Aug 2021 16:11:42 -0600
> Rob Herring <robh+dt@kernel.org> escreveu:
> 
> > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > Hi Rob,
> > >
> > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > corresponding PHY.
> > >
> > > It is identical to v2, except by:
> > >         -          pcie@7,0 { // Lane 7: Ethernet
> > >         +          pcie@7,0 { // Lane 6: Ethernet  
> > 
> > Can you check whether you have DT node links in sysfs for the PCI
> > devices? If you don't, then something is wrong still in the topology
> > or the PCI core is failing to set the DT node pointer in struct
> > device. Though you don't rely on that currently, we want the topology
> > to match. It's possible this never worked on arm/arm64 as mainly
> > powerpc relied on this.
> >
> > I'd like some way to validate the DT matches the PCI topology. We
> > could have a tool that generates the DT structure based on the PCI
> > topology.
> 
> The of_node node link is on those places:
> 
> 	$ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> 	/sys/devices/platform/soc/f4000000.pcie/of_node
> 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node

Looks like we're missing some... 

It's not immediately obvious to me what's wrong here. Only the root 
bus is getting it's DT node set. The relevant code is pci_scan_device(), 
pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try 
to reproduce and debug it.

In the mean time, I applied the series but haven't pushed it out.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-04 16:28     ` Rob Herring
@ 2021-08-05  7:46       ` Mauro Carvalho Chehab
  2021-08-05  7:58         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-05  7:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Wed, 4 Aug 2021 10:28:53 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > Em Tue, 3 Aug 2021 16:11:42 -0600
> > Rob Herring <robh+dt@kernel.org> escreveu:
> >   
> > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:  
> > > >
> > > > Hi Rob,
> > > >
> > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > corresponding PHY.
> > > >
> > > > It is identical to v2, except by:
> > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > >         +          pcie@7,0 { // Lane 6: Ethernet    
> > > 
> > > Can you check whether you have DT node links in sysfs for the PCI
> > > devices? If you don't, then something is wrong still in the topology
> > > or the PCI core is failing to set the DT node pointer in struct
> > > device. Though you don't rely on that currently, we want the topology
> > > to match. It's possible this never worked on arm/arm64 as mainly
> > > powerpc relied on this.
> > >
> > > I'd like some way to validate the DT matches the PCI topology. We
> > > could have a tool that generates the DT structure based on the PCI
> > > topology.  
> > 
> > The of_node node link is on those places:
> > 
> > 	$ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > 	/sys/devices/platform/soc/f4000000.pcie/of_node
> > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node  
> 
> Looks like we're missing some... 
> 
> It's not immediately obvious to me what's wrong here. Only the root 
> bus is getting it's DT node set. The relevant code is pci_scan_device(), 
> pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try 
> to reproduce and debug it.

I added a printk on both pci_set_*of_node() functions:

	[    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	[    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
	[    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
	[    5.059263]  (null): pci_set_of_node: of_node: (null)
	[    5.085552]  (null): pci_set_of_node: of_node: (null)
	[    5.112073]  (null): pci_set_of_node: of_node: (null)
	[    5.138320]  (null): pci_set_of_node: of_node: (null)
	[    5.164673]  (null): pci_set_of_node: of_node: (null)
	[    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
	[    5.240539]  (null): pci_set_of_node: of_node: (null)
	[    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
	[    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
	[    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
	[    5.345516]  (null): pci_set_of_node: of_node: (null)
	[    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

It sounds that the parent is missing when pci_set_bus_of_node() is
called on some places. I'll try to identify why.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-05  7:46       ` Mauro Carvalho Chehab
@ 2021-08-05  7:58         ` Mauro Carvalho Chehab
  2021-08-06 16:23           ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-05  7:58 UTC (permalink / raw)
  To: Rob Herring, Linuxarm, mauro.chehab
  Cc: Binghui Wang, Gustavo Pimentel, Jingoo Han, Xiaowei Song,
	devicetree, linux-kernel, PCI, linux-phy

Em Thu, 5 Aug 2021 09:46:12 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Wed, 4 Aug 2021 10:28:53 -0600
> Rob Herring <robh@kernel.org> escreveu:
> 
> > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:  
> > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > Rob Herring <robh+dt@kernel.org> escreveu:
> > >     
> > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > <mchehab+huawei@kernel.org> wrote:    
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > corresponding PHY.
> > > > >
> > > > > It is identical to v2, except by:
> > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > >         +          pcie@7,0 { // Lane 6: Ethernet      
> > > > 
> > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > devices? If you don't, then something is wrong still in the topology
> > > > or the PCI core is failing to set the DT node pointer in struct
> > > > device. Though you don't rely on that currently, we want the topology
> > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > powerpc relied on this.
> > > >
> > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > could have a tool that generates the DT structure based on the PCI
> > > > topology.    
> > > 
> > > The of_node node link is on those places:
> > > 
> > > 	$ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > 	/sys/devices/platform/soc/f4000000.pcie/of_node
> > > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > 	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node    
> > 
> > Looks like we're missing some... 
> > 
> > It's not immediately obvious to me what's wrong here. Only the root 
> > bus is getting it's DT node set. The relevant code is pci_scan_device(), 
> > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try 
> > to reproduce and debug it.  
> 
> I added a printk on both pci_set_*of_node() functions:
> 
> 	[    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> 	[    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> 	[    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> 	[    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> 	[    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> 	[    5.059263]  (null): pci_set_of_node: of_node: (null)
> 	[    5.085552]  (null): pci_set_of_node: of_node: (null)
> 	[    5.112073]  (null): pci_set_of_node: of_node: (null)
> 	[    5.138320]  (null): pci_set_of_node: of_node: (null)
> 	[    5.164673]  (null): pci_set_of_node: of_node: (null)
> 	[    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> 	[    5.240539]  (null): pci_set_of_node: of_node: (null)
> 	[    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> 	[    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> 	[    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> 	[    5.345516]  (null): pci_set_of_node: of_node: (null)
> 	[    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

The enclosed patch makes the above a clearer:

	[    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	[    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
	[    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
	[    4.968821] pci 0000:02:01.0: pci_set_of_node: of_node: (null)
	[    5.003538] pci 0000:02:04.0: pci_set_of_node: of_node: (null)
	[    5.041348] pci 0000:02:05.0: pci_set_of_node: of_node: (null)
	[    5.092770] pci 0000:02:07.0: pci_set_of_node: of_node: (null)
	[    5.118298] pci 0000:02:09.0: pci_set_of_node: of_node: (null)
	[    5.178215] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
	[    5.198433] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
	[    5.233330] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
	[    5.247071] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
	[    5.260898] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
	[    5.293764] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
	[    5.332808] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

> 
> It sounds that the parent is missing when pci_set_bus_of_node() is
> called on some places. I'll try to identify why.
> 
> Thanks,
> Mauro

Thanks,
Mauro

[PATCH] pci: setup PCI before setting the OF node

With this change, it is easier to add a debug printk at
pci_set_of_node() in order to address possible issues.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 79177ac37880..c5dfc1afb1d3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2374,15 +2374,14 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
-	pci_set_of_node(dev);
-
 	if (pci_setup_device(dev)) {
-		pci_release_of_node(dev);
 		pci_bus_put(dev->bus);
 		kfree(dev);
 		return NULL;
 	}
 
+	pci_set_of_node(dev);
+
 	return dev;
 }
 



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-05  7:58         ` Mauro Carvalho Chehab
@ 2021-08-06 16:23           ` Rob Herring
  2021-08-10  9:42             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-06 16:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Thu, 5 Aug 2021 09:46:12 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
>
> > Em Wed, 4 Aug 2021 10:28:53 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >
> > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > >
> > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > corresponding PHY.
> > > > > >
> > > > > > It is identical to v2, except by:
> > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > >         +          pcie@7,0 { // Lane 6: Ethernet
> > > > >
> > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > devices? If you don't, then something is wrong still in the topology
> > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > device. Though you don't rely on that currently, we want the topology
> > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > powerpc relied on this.
> > > > >
> > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > could have a tool that generates the DT structure based on the PCI
> > > > > topology.
> > > >
> > > > The of_node node link is on those places:
> > > >
> > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > >
> > > Looks like we're missing some...
> > >
> > > It's not immediately obvious to me what's wrong here. Only the root
> > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > to reproduce and debug it.
> >
> > I added a printk on both pci_set_*of_node() functions:
> >
> >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
>
> The enclosed patch makes the above a clearer:
>
>         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
>         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
>         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
>         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
>         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)

I believe the issue is we need another bridge node in the DT
hierarchy. What we have is:

Bus 0 is node /soc/pcie@f4000000
Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
under /soc/pcie@f4000000/pcie@0,0

So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-06 16:23           ` Rob Herring
@ 2021-08-10  9:42             ` Mauro Carvalho Chehab
  2021-08-10 13:44               ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-10  9:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Fri, 6 Aug 2021 10:23:35 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Thu, 5 Aug 2021 09:46:12 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> >  
> > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > Rob Herring <robh@kernel.org> escreveu:
> > >  
> > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:  
> > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > >  
> > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > <mchehab+huawei@kernel.org> wrote:  
> > > > > > >
> > > > > > > Hi Rob,
> > > > > > >
> > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > corresponding PHY.
> > > > > > >
> > > > > > > It is identical to v2, except by:
> > > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > > >         +          pcie@7,0 { // Lane 6: Ethernet  
> > > > > >
> > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > powerpc relied on this.
> > > > > >
> > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > topology.  
> > > > >
> > > > > The of_node node link is on those places:
> > > > >
> > > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node  
> > > >
> > > > Looks like we're missing some...
> > > >
> > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > to reproduce and debug it.  
> > >
> > > I added a printk on both pci_set_*of_node() functions:
> > >
> > >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> > >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)  
> >
> > The enclosed patch makes the above a clearer:
> >
> >         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> >         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> >         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)  
> 
> I believe the issue is we need another bridge node in the DT
> hierarchy. What we have is:
> 
> Bus 0 is node /soc/pcie@f4000000
> Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> under /soc/pcie@f4000000/pcie@0,0
> 
> So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}

Adding a child pcie@0 produces the following output from my debug
patches:

[    4.984278]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
[    5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
[    5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
[    5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
[    5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
[    5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
[    5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
[    5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
[    5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
[    5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
[    5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

It produces the following sysfs nodes:

	$ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
	/sys/devices/platform/soc/f4000000.pcie/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
	/sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node


I'm enclosing the DT schema I'm using.



Thanks,
Mauro

---

		pcie@f4000000 {
			compatible = "hisilicon,kirin970-pcie";
			reg = <0x0 0xf4000000 0x0 0x1000000>,
			      <0x0 0xfc180000 0x0 0x1000>,
			      <0x0 0xf5000000 0x0 0x2000>;
			reg-names = "dbi", "apb", "config";
			bus-range = <0x00 0xff>;
			#address-cells = <3>;
			#size-cells = <2>;
			device_type = "pci";
			phys = <&pcie_phy>;
			ranges = <0x02000000 0x0 0x00000000
				  0x0 0xf6000000
				  0x0 0x02000000>;
			num-lanes = <1>;
			#interrupt-cells = <1>;
			interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
			interrupt-names = "msi";
			interrupt-map-mask = <0 0 0 7>;
			interrupt-map = <0x0 0 0 1
					 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 2
					 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 3
					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 4
					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
			reset-gpios = <&gpio7 0 0>;
			hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
						<&gpio20 6 0>;
			pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
				reg = <0x80 0 0 0 0>;
				compatible = "pciclass,0604";
				device_type = "pci";
				#address-cells = <3>;
				#size-cells = <2>;
				ranges;
				bus-range = <0x01 0xff>;
				msi-parent = <&its_pcie>;

				pcie@0,0 { // Lane 0: upstream
					reg = <0x010000 0 0 0 0>;
					compatible = "pciclass,0604";
					device_type = "pci";
					#address-cells = <3>;
					#size-cells = <2>;
					ranges;
				};
				pcie@1,0 { // Lane 4: M.2
					reg = <0x010800 0 0 0 0>;
					compatible = "pciclass,0604";
					device_type = "pci";
					reset-gpios = <&gpio3 1 0>;
					#address-cells = <3>;
					#size-cells = <2>;
					ranges;
				};

				pcie@5,0 { // Lane 5: Mini PCIe
					reg = <0x012800 0 0 0 0>;
					compatible = "pciclass,0604";
					device_type = "pci";
					reset-gpios = <&gpio27 4 0 >;
					#address-cells = <3>;
					#size-cells = <2>;
					ranges;
				};

				pcie@7,0 { // Lane 6: Ethernet
					reg = <0x013800 0 0 0 0>;
					compatible = "pciclass,0604";
					device_type = "pci";
					reset-gpios = <&gpio25 2 0 >;
					#address-cells = <3>;
					#size-cells = <2>;
					ranges;
				};
			};
		};




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10  9:42             ` Mauro Carvalho Chehab
@ 2021-08-10 13:44               ` Rob Herring
  2021-08-10 14:20                 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-10 13:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Fri, 6 Aug 2021 10:23:35 -0600
> Rob Herring <robh@kernel.org> escreveu:
>
> > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > >
> > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > Rob Herring <robh@kernel.org> escreveu:
> > > >
> > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > >
> > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > > > >
> > > > > > > > Hi Rob,
> > > > > > > >
> > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > corresponding PHY.
> > > > > > > >
> > > > > > > > It is identical to v2, except by:
> > > > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > > > >         +          pcie@7,0 { // Lane 6: Ethernet
> > > > > > >
> > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > powerpc relied on this.
> > > > > > >
> > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > topology.
> > > > > >
> > > > > > The of_node node link is on those places:
> > > > > >
> > > > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > > >
> > > > > Looks like we're missing some...
> > > > >
> > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > to reproduce and debug it.
> > > >
> > > > I added a printk on both pci_set_*of_node() functions:
> > > >
> > > >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> > > >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > >
> > > The enclosed patch makes the above a clearer:
> > >
> > >         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > >         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > >         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > >         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > >         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> >
> > I believe the issue is we need another bridge node in the DT
> > hierarchy. What we have is:
> >
> > Bus 0 is node /soc/pcie@f4000000
> > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > under /soc/pcie@f4000000/pcie@0,0
> >
> > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}
>
> Adding a child pcie@0 produces the following output from my debug
> patches:

You removed your changes to the PCI code other than the debug print?
>
> [    4.984278]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> [    5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> [    5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> [    5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> [    5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> [    5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0

This should not happen. The devfn doesn't match.

> [    5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> [    5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> [    5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> [    5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> [    5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> [    5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> [    5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> [    5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> [    5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> [    5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> [    5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
>
> It produces the following sysfs nodes:
>
>         $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
>         /sys/devices/platform/soc/f4000000.pcie/of_node
>         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
>         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
>         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
>         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
>         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
>
>
> I'm enclosing the DT schema I'm using.
>
>
>
> Thanks,
> Mauro
>
> ---
>
>                 pcie@f4000000 {
>                         compatible = "hisilicon,kirin970-pcie";
>                         reg = <0x0 0xf4000000 0x0 0x1000000>,
>                               <0x0 0xfc180000 0x0 0x1000>,
>                               <0x0 0xf5000000 0x0 0x2000>;
>                         reg-names = "dbi", "apb", "config";
>                         bus-range = <0x00 0xff>;
>                         #address-cells = <3>;
>                         #size-cells = <2>;
>                         device_type = "pci";
>                         phys = <&pcie_phy>;
>                         ranges = <0x02000000 0x0 0x00000000
>                                   0x0 0xf6000000
>                                   0x0 0x02000000>;
>                         num-lanes = <1>;
>                         #interrupt-cells = <1>;
>                         interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
>                         interrupt-names = "msi";
>                         interrupt-map-mask = <0 0 0 7>;
>                         interrupt-map = <0x0 0 0 1
>                                          &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0x0 0 0 2
>                                          &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0x0 0 0 3
>                                          &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0x0 0 0 4
>                                          &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
>                         reset-gpios = <&gpio7 0 0>;
>                         hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
>                                                 <&gpio20 6 0>;
>                         pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
>                                 reg = <0x80 0 0 0 0>;

s/0x80/0/

>                                 compatible = "pciclass,0604";
>                                 device_type = "pci";
>                                 #address-cells = <3>;
>                                 #size-cells = <2>;
>                                 ranges;
>                                 bus-range = <0x01 0xff>;
>                                 msi-parent = <&its_pcie>;
>
>                                 pcie@0,0 { // Lane 0: upstream
>                                         reg = <0x010000 0 0 0 0>;

While technically correct having the bus# in the address, that doesn't
work for FDT since we don't know the bus assignment. So we should just
use 0.

>                                         compatible = "pciclass,0604";
>                                         device_type = "pci";
>                                         #address-cells = <3>;
>                                         #size-cells = <2>;
>                                         ranges;
>                                 };
>                                 pcie@1,0 { // Lane 4: M.2

These 3 nodes (1, 5, 7) need to be child nodes of the above node.

>                                         reg = <0x010800 0 0 0 0>;

Just 0x800

>                                         compatible = "pciclass,0604";
>                                         device_type = "pci";
>                                         reset-gpios = <&gpio3 1 0>;
>                                         #address-cells = <3>;
>                                         #size-cells = <2>;
>                                         ranges;
>                                 };
>
>                                 pcie@5,0 { // Lane 5: Mini PCIe
>                                         reg = <0x012800 0 0 0 0>;

0x2800

>                                         compatible = "pciclass,0604";
>                                         device_type = "pci";
>                                         reset-gpios = <&gpio27 4 0 >;
>                                         #address-cells = <3>;
>                                         #size-cells = <2>;
>                                         ranges;
>                                 };
>
>                                 pcie@7,0 { // Lane 6: Ethernet
>                                         reg = <0x013800 0 0 0 0>;

0x3800

>                                         compatible = "pciclass,0604";
>                                         device_type = "pci";
>                                         reset-gpios = <&gpio25 2 0 >;
>                                         #address-cells = <3>;
>                                         #size-cells = <2>;
>                                         ranges;
>                                 };
>                         };
>                 };
>
>
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10 13:44               ` Rob Herring
@ 2021-08-10 14:20                 ` Mauro Carvalho Chehab
  2021-08-10 17:13                   ` Rob Herring
  0 siblings, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-10 14:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Tue, 10 Aug 2021 07:44:50 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Fri, 6 Aug 2021 10:23:35 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >  
> > > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:  
> > > >
> > > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > > >  
> > > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > >  
> > > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:  
> > > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > > >  
> > > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > > <mchehab+huawei@kernel.org> wrote:  
> > > > > > > > >
> > > > > > > > > Hi Rob,
> > > > > > > > >
> > > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > > corresponding PHY.
> > > > > > > > >
> > > > > > > > > It is identical to v2, except by:
> > > > > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > > > > >         +          pcie@7,0 { // Lane 6: Ethernet  
> > > > > > > >
> > > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > > powerpc relied on this.
> > > > > > > >
> > > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > > topology.  
> > > > > > >
> > > > > > > The of_node node link is on those places:
> > > > > > >
> > > > > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node  
> > > > > >
> > > > > > Looks like we're missing some...
> > > > > >
> > > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > > to reproduce and debug it.  
> > > > >
> > > > > I added a printk on both pci_set_*of_node() functions:
> > > > >
> > > > >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> > > > >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)  
> > > >
> > > > The enclosed patch makes the above a clearer:
> > > >
> > > >         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > >         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > >         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > >         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > >         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)  
> > >
> > > I believe the issue is we need another bridge node in the DT
> > > hierarchy. What we have is:
> > >
> > > Bus 0 is node /soc/pcie@f4000000
> > > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > > under /soc/pcie@f4000000/pcie@0,0
> > >
> > > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}  
> >
> > Adding a child pcie@0 produces the following output from my debug
> > patches:  
> 
> You removed your changes to the PCI code other than the debug print?

Yes.

> >
> > [    4.984278]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > [    5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > [    5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > [    5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > [    5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > [    5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0  
> 
> This should not happen. The devfn doesn't match.
> 
> > [    5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > [    5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > [    5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > [    5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > [    5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > [    5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> > [    5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > [    5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > [    5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > [    5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> > [    5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> >
> > It produces the following sysfs nodes:
> >
> >         $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> >         /sys/devices/platform/soc/f4000000.pcie/of_node
> >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
> >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
> >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> >
> >
> > I'm enclosing the DT schema I'm using.
> >
> >
> >
> > Thanks,
> > Mauro
> >
> > ---
> >
> >                 pcie@f4000000 {
> >                         compatible = "hisilicon,kirin970-pcie";
> >                         reg = <0x0 0xf4000000 0x0 0x1000000>,
> >                               <0x0 0xfc180000 0x0 0x1000>,
> >                               <0x0 0xf5000000 0x0 0x2000>;
> >                         reg-names = "dbi", "apb", "config";
> >                         bus-range = <0x00 0xff>;
> >                         #address-cells = <3>;
> >                         #size-cells = <2>;
> >                         device_type = "pci";
> >                         phys = <&pcie_phy>;
> >                         ranges = <0x02000000 0x0 0x00000000
> >                                   0x0 0xf6000000
> >                                   0x0 0x02000000>;
> >                         num-lanes = <1>;
> >                         #interrupt-cells = <1>;
> >                         interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> >                         interrupt-names = "msi";
> >                         interrupt-map-mask = <0 0 0 7>;
> >                         interrupt-map = <0x0 0 0 1
> >                                          &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0x0 0 0 2
> >                                          &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0x0 0 0 3
> >                                          &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0x0 0 0 4
> >                                          &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> >                         reset-gpios = <&gpio7 0 0>;
> >                         hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
> >                                                 <&gpio20 6 0>;
> >                         pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> >                                 reg = <0x80 0 0 0 0>;  
> 
> s/0x80/0/
> 
> >                                 compatible = "pciclass,0604";
> >                                 device_type = "pci";
> >                                 #address-cells = <3>;
> >                                 #size-cells = <2>;
> >                                 ranges;
> >                                 bus-range = <0x01 0xff>;
> >                                 msi-parent = <&its_pcie>;
> >
> >                                 pcie@0,0 { // Lane 0: upstream
> >                                         reg = <0x010000 0 0 0 0>;  
> 
> While technically correct having the bus# in the address, that doesn't
> work for FDT since we don't know the bus assignment. So we should just
> use 0.

Using 0 causes DTB compilation to produce a warning, due to the
bus-range. Without the bus-range, there will be runtime warnings,
as this will be assigned as bus 1.

> 
> >                                         compatible = "pciclass,0604";
> >                                         device_type = "pci";
> >                                         #address-cells = <3>;
> >                                         #size-cells = <2>;
> >                                         ranges;
> >                                 };
> >                                 pcie@1,0 { // Lane 4: M.2  
> 
> These 3 nodes (1, 5, 7) need to be child nodes of the above node.
> 
> >                                         reg = <0x010800 0 0 0 0>;  
> 
> Just 0x800

The same applies here and to all the other nodes: they all need to have
the bus number on it, as otherwise either DTB compilation warnings
are generated, or runtime ones are produced, like:


            [    4.986196] kirin-pcie f4000000.pcie: PCI host bridge to bus 0000:00
            [    4.992572] pci_bus 0000:00: root bus resource [bus 00-01]
    ...
            [    5.065566] pci_bus 0000:01: busn_res: can not insert [bus 01-ff] under [bus 00-01] (conflicts with (null) [bus 00-01])
    
> 
> >                                         compatible = "pciclass,0604";
> >                                         device_type = "pci";
> >                                         reset-gpios = <&gpio3 1 0>;
> >                                         #address-cells = <3>;
> >                                         #size-cells = <2>;
> >                                         ranges;
> >                                 };
> >
> >                                 pcie@5,0 { // Lane 5: Mini PCIe
> >                                         reg = <0x012800 0 0 0 0>;  
> 
> 0x2800
> 
> >                                         compatible = "pciclass,0604";
> >                                         device_type = "pci";
> >                                         reset-gpios = <&gpio27 4 0 >;
> >                                         #address-cells = <3>;
> >                                         #size-cells = <2>;
> >                                         ranges;
> >                                 };
> >
> >                                 pcie@7,0 { // Lane 6: Ethernet
> >                                         reg = <0x013800 0 0 0 0>;  
> 
> 0x3800
> 
> >                                         compatible = "pciclass,0604";
> >                                         device_type = "pci";
> >                                         reset-gpios = <&gpio25 2 0 >;
> >                                         #address-cells = <3>;
> >                                         #size-cells = <2>;
> >                                         ranges;
> >                                 };
> >                         };
> >                 };
> >
> >
> >  



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10 14:20                 ` Mauro Carvalho Chehab
@ 2021-08-10 17:13                   ` Rob Herring
  2021-08-10 17:52                     ` Rob Herring
  2021-08-11  6:46                     ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 23+ messages in thread
From: Rob Herring @ 2021-08-10 17:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Tue, Aug 10, 2021 at 8:21 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Tue, 10 Aug 2021 07:44:50 -0600
> Rob Herring <robh@kernel.org> escreveu:
>
> > On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > Em Fri, 6 Aug 2021 10:23:35 -0600
> > > Rob Herring <robh@kernel.org> escreveu:
> > >
> > > > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > > > <mchehab+huawei@kernel.org> wrote:
> > > > >
> > > > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > > > >
> > > > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > > >
> > > > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > > > >
> > > > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Rob,
> > > > > > > > > >
> > > > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > > > corresponding PHY.
> > > > > > > > > >
> > > > > > > > > > It is identical to v2, except by:
> > > > > > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > > > > > >         +          pcie@7,0 { // Lane 6: Ethernet
> > > > > > > > >
> > > > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > > > powerpc relied on this.
> > > > > > > > >
> > > > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > > > topology.
> > > > > > > >
> > > > > > > > The of_node node link is on those places:
> > > > > > > >
> > > > > > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > > > > >
> > > > > > > Looks like we're missing some...
> > > > > > >
> > > > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > > > to reproduce and debug it.
> > > > > >
> > > > > > I added a printk on both pci_set_*of_node() functions:
> > > > > >
> > > > > >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > > >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > > >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > > >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > > >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > > >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> > > > > >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > > >
> > > > > The enclosed patch makes the above a clearer:
> > > > >
> > > > >         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > >         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > >         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > >         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > >         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > >
> > > > I believe the issue is we need another bridge node in the DT
> > > > hierarchy. What we have is:
> > > >
> > > > Bus 0 is node /soc/pcie@f4000000
> > > > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > > > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > > > under /soc/pcie@f4000000/pcie@0,0
> > > >
> > > > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}
> > >
> > > Adding a child pcie@0 produces the following output from my debug
> > > patches:
> >
> > You removed your changes to the PCI code other than the debug print?
>
> Yes.
>
> > >
> > > [    4.984278]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > [    5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > [    5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > [    5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > [    5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > [    5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >
> > This should not happen. The devfn doesn't match.
> >
> > > [    5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > [    5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > [    5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > [    5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > [    5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > [    5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> > > [    5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > [    5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > [    5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > [    5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> > > [    5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > >
> > > It produces the following sysfs nodes:
> > >
> > >         $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > >
> > >
> > > I'm enclosing the DT schema I'm using.
> > >
> > >
> > >
> > > Thanks,
> > > Mauro
> > >
> > > ---
> > >
> > >                 pcie@f4000000 {
> > >                         compatible = "hisilicon,kirin970-pcie";
> > >                         reg = <0x0 0xf4000000 0x0 0x1000000>,
> > >                               <0x0 0xfc180000 0x0 0x1000>,
> > >                               <0x0 0xf5000000 0x0 0x2000>;
> > >                         reg-names = "dbi", "apb", "config";
> > >                         bus-range = <0x00 0xff>;
> > >                         #address-cells = <3>;
> > >                         #size-cells = <2>;
> > >                         device_type = "pci";
> > >                         phys = <&pcie_phy>;
> > >                         ranges = <0x02000000 0x0 0x00000000
> > >                                   0x0 0xf6000000
> > >                                   0x0 0x02000000>;
> > >                         num-lanes = <1>;
> > >                         #interrupt-cells = <1>;
> > >                         interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > >                         interrupt-names = "msi";
> > >                         interrupt-map-mask = <0 0 0 7>;
> > >                         interrupt-map = <0x0 0 0 1
> > >                                          &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0x0 0 0 2
> > >                                          &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0x0 0 0 3
> > >                                          &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0x0 0 0 4
> > >                                          &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > >                         reset-gpios = <&gpio7 0 0>;
> > >                         hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
> > >                                                 <&gpio20 6 0>;
> > >                         pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> > >                                 reg = <0x80 0 0 0 0>;
> >
> > s/0x80/0/
> >
> > >                                 compatible = "pciclass,0604";
> > >                                 device_type = "pci";
> > >                                 #address-cells = <3>;
> > >                                 #size-cells = <2>;
> > >                                 ranges;
> > >                                 bus-range = <0x01 0xff>;
> > >                                 msi-parent = <&its_pcie>;
> > >
> > >                                 pcie@0,0 { // Lane 0: upstream
> > >                                         reg = <0x010000 0 0 0 0>;
> >
> > While technically correct having the bus# in the address, that doesn't
> > work for FDT since we don't know the bus assignment. So we should just
> > use 0.
>
> Using 0 causes DTB compilation to produce a warning, due to the
> bus-range. Without the bus-range, there will be runtime warnings,
> as this will be assigned as bus 1.

Okay, that might be something we need to fix.


> > >                                         compatible = "pciclass,0604";
> > >                                         device_type = "pci";
> > >                                         #address-cells = <3>;
> > >                                         #size-cells = <2>;
> > >                                         ranges;
> > >                                 };
> > >                                 pcie@1,0 { // Lane 4: M.2
> >
> > These 3 nodes (1, 5, 7) need to be child nodes of the above node.

This was the main issue.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10 17:13                   ` Rob Herring
@ 2021-08-10 17:52                     ` Rob Herring
  2021-08-11  7:11                       ` Mauro Carvalho Chehab
  2021-08-11  6:46                     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-10 17:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Tue, Aug 10, 2021 at 11:13 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Aug 10, 2021 at 8:21 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Tue, 10 Aug 2021 07:44:50 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >
> > > On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:
> > > >
> > > > Em Fri, 6 Aug 2021 10:23:35 -0600
> > > > Rob Herring <robh@kernel.org> escreveu:
> > > >
> > > > > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > >
> > > > > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > > > > >
> > > > > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > > > >
> > > > > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > > > > >
> > > > > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Rob,
> > > > > > > > > > >
> > > > > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > > > > corresponding PHY.
> > > > > > > > > > >
> > > > > > > > > > > It is identical to v2, except by:
> > > > > > > > > > >         -          pcie@7,0 { // Lane 7: Ethernet
> > > > > > > > > > >         +          pcie@7,0 { // Lane 6: Ethernet
> > > > > > > > > >
> > > > > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > > > > powerpc relied on this.
> > > > > > > > > >
> > > > > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > > > > topology.
> > > > > > > > >
> > > > > > > > > The of_node node link is on those places:
> > > > > > > > >
> > > > > > > > >   $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > > > > >   /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > > > > > >
> > > > > > > > Looks like we're missing some...
> > > > > > > >
> > > > > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > > > > to reproduce and debug it.
> > > > > > >
> > > > > > > I added a printk on both pci_set_*of_node() functions:
> > > > > > >
> > > > > > >       [    4.872991]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > > >       [    4.913806]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > > >       [    4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > >       [    4.990622]  (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > >       [    5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > > > >       [    5.059263]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.085552]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.112073]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.138320]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.164673]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > > > >       [    5.240539]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > > > >       [    5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > > > >       [    5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > > > >       [    5.345516]  (null): pci_set_of_node: of_node: (null)
> > > > > > >       [    5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > > > >
> > > > > > The enclosed patch makes the above a clearer:
> > > > > >
> > > > > >         [    4.800975]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > >         [    4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > >         [    4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > >         [    4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > >         [    4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > >
> > > > > I believe the issue is we need another bridge node in the DT
> > > > > hierarchy. What we have is:
> > > > >
> > > > > Bus 0 is node /soc/pcie@f4000000
> > > > > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > > > > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > > > > under /soc/pcie@f4000000/pcie@0,0
> > > > >
> > > > > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}
> > > >
> > > > Adding a child pcie@0 produces the following output from my debug
> > > > patches:
> > >
> > > You removed your changes to the PCI code other than the debug print?
> >
> > Yes.
> >
> > > >
> > > > [    4.984278]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > [    5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > [    5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [    5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [    5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [    5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > >
> > > This should not happen. The devfn doesn't match.
> > >
> > > > [    5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [    5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [    5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [    5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [    5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > [    5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> > > > [    5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > [    5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > [    5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > [    5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> > > > [    5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > >
> > > > It produces the following sysfs nodes:
> > > >
> > > >         $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > >         /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > >
> > > >
> > > > I'm enclosing the DT schema I'm using.
> > > >
> > > >
> > > >
> > > > Thanks,
> > > > Mauro
> > > >
> > > > ---
> > > >
> > > >                 pcie@f4000000 {
> > > >                         compatible = "hisilicon,kirin970-pcie";
> > > >                         reg = <0x0 0xf4000000 0x0 0x1000000>,
> > > >                               <0x0 0xfc180000 0x0 0x1000>,
> > > >                               <0x0 0xf5000000 0x0 0x2000>;
> > > >                         reg-names = "dbi", "apb", "config";
> > > >                         bus-range = <0x00 0xff>;
> > > >                         #address-cells = <3>;
> > > >                         #size-cells = <2>;
> > > >                         device_type = "pci";
> > > >                         phys = <&pcie_phy>;
> > > >                         ranges = <0x02000000 0x0 0x00000000
> > > >                                   0x0 0xf6000000
> > > >                                   0x0 0x02000000>;
> > > >                         num-lanes = <1>;
> > > >                         #interrupt-cells = <1>;
> > > >                         interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > > >                         interrupt-names = "msi";
> > > >                         interrupt-map-mask = <0 0 0 7>;
> > > >                         interrupt-map = <0x0 0 0 1
> > > >                                          &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > > >                                         <0x0 0 0 2
> > > >                                          &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > > >                                         <0x0 0 0 3
> > > >                                          &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > > >                                         <0x0 0 0 4
> > > >                                          &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > > >                         reset-gpios = <&gpio7 0 0>;
> > > >                         hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
> > > >                                                 <&gpio20 6 0>;
> > > >                         pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> > > >                                 reg = <0x80 0 0 0 0>;
> > >
> > > s/0x80/0/
> > >
> > > >                                 compatible = "pciclass,0604";
> > > >                                 device_type = "pci";
> > > >                                 #address-cells = <3>;
> > > >                                 #size-cells = <2>;
> > > >                                 ranges;
> > > >                                 bus-range = <0x01 0xff>;
> > > >                                 msi-parent = <&its_pcie>;
> > > >
> > > >                                 pcie@0,0 { // Lane 0: upstream
> > > >                                         reg = <0x010000 0 0 0 0>;
> > >
> > > While technically correct having the bus# in the address, that doesn't
> > > work for FDT since we don't know the bus assignment. So we should just
> > > use 0.
> >
> > Using 0 causes DTB compilation to produce a warning, due to the
> > bus-range.

What's the warning? 'bus-range' should be optional.

> > Without the bus-range, there will be runtime warnings,
> > as this will be assigned as bus 1.
>
> Okay, that might be something we need to fix.

Actually, I don't see how there's a problem. First, the only place we
read 'bus-range' is of_pci_parse_bus_range() and that's only called
for the host bridge. Any other occurrence of 'bus-range' should be
ignored. The only place we read 'reg' is of_pci_get_devfn() and we
mask just the devfn bits.

>            [    4.992572] pci_bus 0000:00: root bus resource [bus 00-01]

I don't see any way this can happen other than pcie@f4000000 node
containing 'bus-range = <0 1>;'. It comes from
pci_host_bridge.windows.

Rob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10 17:13                   ` Rob Herring
  2021-08-10 17:52                     ` Rob Herring
@ 2021-08-11  6:46                     ` Mauro Carvalho Chehab
  2021-08-12  3:13                       ` Rob Herring
  1 sibling, 1 reply; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-11  6:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Tue, 10 Aug 2021 11:13:48 -0600
Rob Herring <robh@kernel.org> escreveu:

> > > >                                         compatible = "pciclass,0604";
> > > >                                         device_type = "pci";
> > > >                                         #address-cells = <3>;
> > > >                                         #size-cells = <2>;
> > > >                                         ranges;
> > > >                                 };
> > > >                                 pcie@1,0 { // Lane 4: M.2  
> > >
> > > These 3 nodes (1, 5, 7) need to be child nodes of the above node.  
> 
> This was the main issue.

Ok, placing 1, 5, 7 as child nodes of 0 worked, with the attached
DT schema:


	$ ls -l $(find /sys/devices/platform/soc/f4000000.pcie/ -name of_node)
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/of_node -> ../../../../firmware/devicetree/base/soc/pcie@f4000000
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/pci_bus/0000:03/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/pci_bus/0000:05/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/pci_bus/0000:06/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node -> ../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node -> ../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
	lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000

The logs also seem OK on my eyes:

	[    3.911082]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	[    4.001104] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
	[    4.043609] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.076756] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.120652] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.150766] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.196413] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.238896] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.280116] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.309821] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.370830] pci_bus 0000:03: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
	[    4.382345] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
	[    4.411966] pci_bus 0000:05: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
	[    4.439898] pci_bus 0000:06: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	[    4.491616] pci 0000:06:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	[    4.519907] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

Thanks,
Mauro


		pcie@f4000000 {
			compatible = "hisilicon,kirin970-pcie";
			reg = <0x0 0xf4000000 0x0 0x1000000>,
			      <0x0 0xfc180000 0x0 0x1000>,
			      <0x0 0xf5000000 0x0 0x2000>;
			reg-names = "dbi", "apb", "config";
			bus-range = <0x00 0xff>;
			#address-cells = <3>;
			#size-cells = <2>;
			device_type = "pci";
			phys = <&pcie_phy>;
			ranges = <0x02000000 0x0 0x00000000
				  0x0 0xf6000000
				  0x0 0x02000000>;
			num-lanes = <1>;
			#interrupt-cells = <1>;
			interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
			interrupt-names = "msi";
			interrupt-map-mask = <0 0 0 7>;
			interrupt-map = <0x0 0 0 1
					 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 2
					 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 3
					 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
					<0x0 0 0 4
					 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
			reset-gpios = <&gpio7 0 0>;
			hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
						<&gpio20 6 0>;
			pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
				reg = <0x80 0 0 0 0>;
				compatible = "pciclass,0604";
				device_type = "pci";
				#address-cells = <3>;
				#size-cells = <2>;
				ranges;
				bus-range = <0x01 0xff>;
				msi-parent = <&its_pcie>;

				pcie@0,0 { // Lane 0: upstream
					reg = <0x010000 0 0 0 0>;
					compatible = "pciclass,0604";
					device_type = "pci";
					#address-cells = <3>;
					#size-cells = <2>;
					ranges;

					pcie@1,0 { // Lane 4: M.2
						reg = <0x010800 0 0 0 0>;
						compatible = "pciclass,0604";
						device_type = "pci";
						reset-gpios = <&gpio3 1 0>;
						#address-cells = <3>;
						#size-cells = <2>;
						ranges;
					};

					pcie@5,0 { // Lane 5: Mini PCIe
						reg = <0x012800 0 0 0 0>;
						compatible = "pciclass,0604";
						device_type = "pci";
						reset-gpios = <&gpio27 4 0 >;
						#address-cells = <3>;
						#size-cells = <2>;
						ranges;
					};

					pcie@7,0 { // Lane 6: Ethernet
						reg = <0x013800 0 0 0 0>;
						compatible = "pciclass,0604";
						device_type = "pci";
						reset-gpios = <&gpio25 2 0 >;
						#address-cells = <3>;
						#size-cells = <2>;
						ranges;
					};
				};
			};
		};

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-10 17:52                     ` Rob Herring
@ 2021-08-11  7:11                       ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-11  7:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Tue, 10 Aug 2021 11:52:34 -0600
Rob Herring <robh@kernel.org> escreveu:

> > > > >                         pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> > > > >                                 reg = <0x80 0 0 0 0>;  
> > > >
> > > > s/0x80/0/
> > > >  
> > > > >                                 compatible = "pciclass,0604";
> > > > >                                 device_type = "pci";
> > > > >                                 #address-cells = <3>;
> > > > >                                 #size-cells = <2>;
> > > > >                                 ranges;
> > > > >                                 bus-range = <0x01 0xff>;
> > > > >                                 msi-parent = <&its_pcie>;
> > > > >
> > > > >                                 pcie@0,0 { // Lane 0: upstream
> > > > >                                         reg = <0x010000 0 0 0 0>;  
> > > >
> > > > While technically correct having the bus# in the address, that doesn't
> > > > work for FDT since we don't know the bus assignment. So we should just
> > > > use 0.  
> > >
> > > Using 0 causes DTB compilation to produce a warning, due to the
> > > bus-range.  
> 
> What's the warning? 'bus-range' should be optional.

With this DT schema (simplified to show only relevant bits):

		pcie@f4000000 {
			bus-range = <0x00 0xff>;
			pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
				bus-range = <0x01 0xff>;
				pcie@0,0 { // Lane 0: upstream
					reg = <0x0000 0 0 0 0>;
					pcie@1,0 { // Lane 4: M.2
						reg = <0x0800 0 0 0 0>;
					};

					pcie@5,0 { // Lane 5: Mini PCIe
						reg = <0x2800 0 0 0 0>;
					};

					pcie@7,0 { // Lane 6: Ethernet
						reg = <0x3800 0 0 0 0>;
					};
				};
			};
		};


This is the compilation warning:
	$ make dtbs
	arch/arm64/boot/dts/hisilicon/hi3670.dtsi:735.5-29: Warning (pci_device_bus_num): /soc/pcie@f4000000/pcie@0,0/pcie@0,0:bus-range: PCI bus number 0 out of range, expected (1 - 1)

This is solved by changing:
				pcie@0,0 { // Lane 0: upstream
					reg = <0x0000 0 0 0 0>;
to:
				pcie@0,0 { // Lane 0: upstream
					reg = <0x010000 0 0 0 0>;



> 
> > > Without the bus-range, there will be runtime warnings,
> > > as this will be assigned as bus 1.  
> >
> > Okay, that might be something we need to fix.  
> 
> Actually, I don't see how there's a problem. First, the only place we
> read 'bus-range' is of_pci_parse_bus_range() and that's only called
> for the host bridge. Any other occurrence of 'bus-range' should be
> ignored. The only place we read 'reg' is of_pci_get_devfn() and we
> mask just the devfn bits.
> 
> >            [    4.992572] pci_bus 0000:00: root bus resource [bus 00-01]  
> 
> I don't see any way this can happen other than pcie@f4000000 node
> containing 'bus-range = <0 1>;'. It comes from
> pci_host_bridge.windows.

Yeah, you're right. On the first versions, 'bus-range = <0 1>;' was used,
just like:
	arch/arm64/boot/dts/hisilicon/hi3660.dtsi

(I have a fixup patch fixing the warning on Kirin 960 due to that)

Ok, I did another test here: if I drop both dma-range, it will output:

	[    3.778028] kirin-pcie f4000000.pcie:   No bus range found for /soc/pcie@f4000000, using [bus 00-ff]

But no conflict warnings. So, I guess dropping bus-range and the explicit
bus 1 setting at "reg" is a clean approach.

As a reference, this is the dmesg log here (with OF debug turned on):

# dmesg|grep -i pci
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-rc1+ root=UUID=646147e1-d186-44cc-b0d2-60c42f8dcc6d ro drm.debug=0x6 earlycon=pl011,0xfff32000,115200 console=tty0 console=ttyAMA6,115200n8 root=/dev/sdd12 rootwait rw quiet efi=noruntime drm.debug=0x06 "dyndbg=file drivers/pci/of.c +p;  drivers/gpu/* +p; file drivers/pci/of.c +p;  drivers/spmi/* +p; file drivers/pci/of.c +p;  drivers/mfd/* +p; file drivers/pci/of.c +p;  drivers/regulator/* +p; file drivers/pci/of.c +p;  drivers/usb/core/hub.c +p" no_console_suspend loglevel=9 kernel.panic=5
[    0.000000] Unknown command line parameters: BOOT_IMAGE=/boot/vmlinuz-5.14.0-rc1+ dyndbg=file drivers/pci/of.c +p;  drivers/gpu/* +p; file drivers/pci/of.c +p;  drivers/spmi/* +p; file drivers/pci/of.c +p;  drivers/mfd/* +p; file drivers/pci/of.c +p;  drivers/regulator/* +p; file drivers/pci/of.c +p;  drivers/usb/core/hub.c +p
[    0.725101] PCI: CLS 0 bytes, default 64
[    1.520828] ehci-pci: EHCI PCI platform driver
[    1.521022] ohci-pci: OHCI PCI platform driver
[    2.204793]     dyndbg=file drivers/pci/of.c +p;  drivers/gpu/* +p; file drivers/pci/of.c +p;  drivers/spmi/* +p; file drivers/pci/of.c +p;  drivers/mfd/* +p; file drivers/pci/of.c +p;  drivers/regulator/* +p; file drivers/pci/of.c +p;  drivers/usb/core/hub.c +p
[    3.767252] kirin-pcie f4000000.pcie: host bridge /soc/pcie@f4000000 ranges:
[    3.778028] kirin-pcie f4000000.pcie:   No bus range found for /soc/pcie@f4000000, using [bus 00-ff]
[    3.792466] kirin-pcie f4000000.pcie: Parsing ranges property...
[    3.801919] kirin-pcie f4000000.pcie:      MEM 0x00f6000000..0x00f7ffffff -> 0x0000000000
[    3.813680] kirin-pcie f4000000.pcie: invalid resource
[    3.822189] kirin-pcie f4000000.pcie: iATU unroll: enabled
[    3.831159] kirin-pcie f4000000.pcie: Detected iATU regions: 8 outbound, 8 inbound
[    3.943155] kirin-pcie f4000000.pcie: Link up
[    3.956821]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
[    3.979143] kirin-pcie f4000000.pcie: PCI host bridge to bus 0000:00
[    3.989441] pci_bus 0000:00: root bus resource [bus 00-ff]
[    3.998050] pci_bus 0000:00: root bus resource [mem 0xf6000000-0xf7ffffff] (bus address [0x00000000-0x01ffffff])
[    4.011965] pci 0000:00:00.0: [19e5:3670] type 01 class 0x060400
[    4.021177] pci 0000:00:00.0: reg 0x10: [mem 0xf6000000-0xf6ffffff]
[    4.031041] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
[    4.041362] pci 0000:00:00.0: supports D1 D2
[    4.049191] pci 0000:00:00.0: PME# supported from D0 D1 D2 D3hot
[    4.059554] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
[    4.071133] pci 0000:01:00.0: [10b5:8606] type 01 class 0x060400
[    4.080831] pci 0000:01:00.0: reg 0x10: [mem 0xf6000000-0xf601ffff]
[    4.103511] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
[    4.115403] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    4.139816] pci 0000:01:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.157297] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.172380] pci 0000:02:01.0: [10b5:8606] type 01 class 0x060400
[    4.182279] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.197047] pci 0000:02:01.0: PME# supported from D0 D3hot D3cold
[    4.207583] pci 0000:02:04.0: [10b5:8606] type 01 class 0x060400
[    4.217659] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.234551] pci 0000:02:04.0: PME# supported from D0 D3hot D3cold
[    4.254284] pci 0000:02:05.0: [10b5:8606] type 01 class 0x060400
[    4.267668] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.282530] pci 0000:02:05.0: PME# supported from D0 D3hot D3cold
[    4.295077] pci 0000:02:07.0: [10b5:8606] type 01 class 0x060400
[    4.306032] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.320927] pci 0000:02:07.0: PME# supported from D0 D3hot D3cold
[    4.333414] pci 0000:02:09.0: [10b5:8606] type 01 class 0x060400
[    4.340439] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
[    4.350084] pci 0000:02:09.0: PME# supported from D0 D3hot D3cold
[    4.358150] pci 0000:02:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.366379] pci 0000:02:04.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.374515] pci 0000:02:05.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.382682] pci 0000:02:07.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.390834] pci 0000:02:09.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    4.399079] pci_bus 0000:03: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
[    4.409309] pci 0000:03:00.0: [144d:a809] type 00 class 0x010802
[    4.415564] pci 0000:03:00.0: reg 0x10: [mem 0xf6000000-0xf6003fff 64bit]
[    4.422769] pci 0000:03:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
[    4.433782] pci 0000:03:00.0: 4.000 Gb/s available PCIe bandwidth, limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link)
[    4.449954] pci_bus 0000:03: busn_res: [bus 03-ff] end is updated to 03
[    4.456836] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
[    4.457918] pci_bus 0000:04: busn_res: [bus 04-ff] end is updated to 04
[    4.478895] pci_bus 0000:05: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
[    4.491772] pci_bus 0000:05: busn_res: [bus 05-ff] end is updated to 05
[    4.503438] pci_bus 0000:06: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
[    4.529140] pci 0000:06:00.0: [10ec:8168] type 00 class 0x020000
[    4.535374] pci 0000:06:00.0: reg 0x10: [io  0x0000-0x00ff]
[    4.541229] pci 0000:06:00.0: reg 0x18: [mem 0xf7000000-0xf7000fff 64bit]
[    4.548194] pci 0000:06:00.0: reg 0x20: [mem 0xf7200000-0xf7203fff 64bit pref]
[    4.561063] pci 0000:06:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
[    4.582075] pci 0000:06:00.0: supports D1 D2
[    4.586357] pci 0000:06:00.0: PME# supported from D0 D1 D2 D3hot D3cold
[    4.594727] pci_bus 0000:06: busn_res: [bus 06-ff] end is updated to 06
[    4.601538] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
[    4.608473] pci_bus 0000:07: busn_res: [bus 07-ff] end is updated to 07
[    4.615124] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 07
[    4.621810] pci 0000:00:00.0: BAR 0: assigned [mem 0xf6000000-0xf6ffffff]
[    4.628606] pci 0000:00:00.0: BAR 14: assigned [mem 0xf7000000-0xf72fffff]
[    4.628610] pci 0000:00:00.0: BAR 15: assigned [mem 0xf7300000-0xf73fffff pref]
[    4.642813] pci 0000:00:00.0: BAR 13: no space for [io  size 0x1000]
[    4.649174] pci 0000:00:00.0: BAR 13: failed to assign [io  size 0x1000]
[    4.661518] pci 0000:01:00.0: BAR 14: assigned [mem 0xf7000000-0xf71fffff]
[    4.669074] pci 0000:01:00.0: BAR 15: assigned [mem 0xf7300000-0xf73fffff 64bit pref]
[    4.677066] pci 0000:01:00.0: BAR 0: assigned [mem 0xf7200000-0xf721ffff]
[    4.683891] pci 0000:01:00.0: BAR 13: no space for [io  size 0x1000]
[    4.690252] pci 0000:01:00.0: BAR 13: failed to assign [io  size 0x1000]
[    4.690266] pci 0000:02:01.0: BAR 14: assigned [mem 0xf7000000-0xf70fffff]
[    4.690270] pci 0000:02:07.0: BAR 14: assigned [mem 0xf7100000-0xf71fffff]
[    4.710728] pci 0000:02:07.0: BAR 15: assigned [mem 0xf7300000-0xf73fffff 64bit pref]
[    4.727780] pci 0000:02:07.0: BAR 13: no space for [io  size 0x1000]
[    4.727783] pci 0000:02:07.0: BAR 13: failed to assign [io  size 0x1000]
[    4.727790] pci 0000:03:00.0: BAR 0: assigned [mem 0xf7000000-0xf7003fff 64bit]
[    4.727904] pci 0000:02:01.0: PCI bridge to [bus 03]
[    4.753165] pci 0000:02:01.0:   bridge window [mem 0xf7000000-0xf70fffff]
[    4.769738] pci 0000:02:04.0: PCI bridge to [bus 04]
[    4.774843] pci 0000:02:05.0: PCI bridge to [bus 05]
[    4.779959] pci 0000:06:00.0: BAR 4: assigned [mem 0xf7300000-0xf7303fff 64bit pref]
[    4.787839] pci 0000:06:00.0: BAR 2: assigned [mem 0xf7100000-0xf7100fff 64bit]
[    4.795273] pci 0000:06:00.0: BAR 0: no space for [io  size 0x0100]
[    4.801543] pci 0000:06:00.0: BAR 0: failed to assign [io  size 0x0100]
[    4.808159] pci 0000:02:07.0: PCI bridge to [bus 06]
[    4.813166] pci 0000:02:07.0:   bridge window [mem 0xf7100000-0xf71fffff]
[    4.819981] pci 0000:02:07.0:   bridge window [mem 0xf7300000-0xf73fffff 64bit pref]
[    4.827782] pci 0000:02:09.0: PCI bridge to [bus 07]
[    4.832871] pci 0000:01:00.0: PCI bridge to [bus 02-07]
[    4.838138] pci 0000:01:00.0:   bridge window [mem 0xf7000000-0xf71fffff]
[    4.844952] pci 0000:01:00.0:   bridge window [mem 0xf7300000-0xf73fffff 64bit pref]
[    4.852751] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    4.857980] pci 0000:00:00.0:   bridge window [mem 0xf7000000-0xf72fffff]
[    4.864770] pci 0000:00:00.0:   bridge window [mem 0xf7300000-0xf73fffff pref]
[    4.881547] pcieport 0000:00:00.0: PME: Signaling with IRQ 58
[    4.888905] pcieport 0000:00:00.0: AER: enabled with IRQ 58
[    4.895013] pcieport 0000:01:00.0: enabling device (0000 -> 0002)
[    4.903260] pcieport 0000:02:01.0: enabling device (0000 -> 0002)
[    4.914761] pcieport 0000:02:07.0: enabling device (0000 -> 0002)
[    4.928984] nvme nvme0: pci function 0000:03:00.0

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-11  6:46                     ` Mauro Carvalho Chehab
@ 2021-08-12  3:13                       ` Rob Herring
  2021-08-12  7:48                         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2021-08-12  3:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

On Wed, Aug 11, 2021 at 1:46 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Tue, 10 Aug 2021 11:13:48 -0600
> Rob Herring <robh@kernel.org> escreveu:
>
> > > > >                                         compatible = "pciclass,0604";
> > > > >                                         device_type = "pci";
> > > > >                                         #address-cells = <3>;
> > > > >                                         #size-cells = <2>;
> > > > >                                         ranges;
> > > > >                                 };
> > > > >                                 pcie@1,0 { // Lane 4: M.2
> > > >
> > > > These 3 nodes (1, 5, 7) need to be child nodes of the above node.
> >
> > This was the main issue.
>
> Ok, placing 1, 5, 7 as child nodes of 0 worked, with the attached
> DT schema:
>
>
>         $ ls -l $(find /sys/devices/platform/soc/f4000000.pcie/ -name of_node)
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/of_node -> ../../../../firmware/devicetree/base/soc/pcie@f4000000
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/pci_bus/0000:03/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/pci_bus/0000:05/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/pci_bus/0000:06/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node -> ../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node -> ../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
>         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000

This all looks right to me, but...

> The logs also seem OK on my eyes:
>
>         [    3.911082]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
>         [    4.001104] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
>         [    4.043609] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
>         [    4.076756] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
>         [    4.120652] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         [    4.150766] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         [    4.196413] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         [    4.238896] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         [    4.280116] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
>         [    4.309821] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0

...these do not.

>         [    4.370830] pci_bus 0000:03: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
>         [    4.382345] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
>         [    4.411966] pci_bus 0000:05: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
>         [    4.439898] pci_bus 0000:06: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
>         [    4.491616] pci 0000:06:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
>         [    4.519907] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
  2021-08-12  3:13                       ` Rob Herring
@ 2021-08-12  7:48                         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 23+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-12  7:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, Binghui Wang, Gustavo Pimentel,
	Jingoo Han, Xiaowei Song, devicetree, linux-kernel, PCI,
	linux-phy

Em Wed, 11 Aug 2021 22:13:32 -0500
Rob Herring <robh@kernel.org> escreveu:

> On Wed, Aug 11, 2021 at 1:46 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Tue, 10 Aug 2021 11:13:48 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >  
> > > > > >                                         compatible = "pciclass,0604";
> > > > > >                                         device_type = "pci";
> > > > > >                                         #address-cells = <3>;
> > > > > >                                         #size-cells = <2>;
> > > > > >                                         ranges;
> > > > > >                                 };
> > > > > >                                 pcie@1,0 { // Lane 4: M.2  
> > > > >
> > > > > These 3 nodes (1, 5, 7) need to be child nodes of the above node.  
> > >
> > > This was the main issue.  
> >
> > Ok, placing 1, 5, 7 as child nodes of 0 worked, with the attached
> > DT schema:
> >
> >
> >         $ ls -l $(find /sys/devices/platform/soc/f4000000.pcie/ -name of_node)
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/of_node -> ../../../../firmware/devicetree/base/soc/pcie@f4000000
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:01.0/pci_bus/0000:03/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:05.0/pci_bus/0000:05/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:07.0/pci_bus/0000:06/of_node -> ../../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node -> ../../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node -> ../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node -> ../../../../../../../../firmware/devicetree/base/soc/pcie@f4000000/pcie@0,0
> >         lrwxrwxrwx 1 root root 0 ago 11 08:43 /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node -> ../../../../../../../firmware/devicetree/base/soc/pcie@f4000000  
> 
> This all looks right to me, but...
> 
> > The logs also seem OK on my eyes:
> >
> >         [    3.911082]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> >         [    4.001104] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> >         [    4.043609] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >         [    4.076756] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> >         [    4.120652] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         [    4.150766] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         [    4.196413] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         [    4.238896] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         [    4.280116] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> >         [    4.309821] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0  
> 
> ...these do not.

For the above:

	s/of_node:/BUS of_node:/

The debug printk is misleading. It is actually printing the BUS of_node:

	void pci_set_of_node(struct pci_dev *dev)
	{
		dev_dbg(&dev->dev, "%s: of_node: %pOF\n",
			__func__, dev->bus->dev.of_node);

		if (!dev->bus->dev.of_node)
			return;
	...

If I move it to the right place, e. g.:

	void pci_set_of_node(struct pci_dev *dev)
	{
		if (!dev->bus->dev.of_node) {
			dev_dbg(&dev->dev, "%s: BUS of_node is null\n",
				__func__, dev->bus->dev.of_node);
			return;
		}
		dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
							    dev->devfn);
		if (dev->dev.of_node)
			dev->dev.fwnode = &dev->dev.of_node->fwnode;

		dev_dbg(&dev->dev, "%s: of_node: %pOF\n",
			__func__, dev->dev.of_node);
	}

It will produce:

	[    4.155771]  (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
	[    4.208740] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.236759] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
	[    4.257899] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.310350] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
	[    4.337784] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
	[    4.370998] pci 0000:02:04.0: pci_set_of_node: of_node: (null)
	[    4.391459] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
	[    4.415378] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	[    4.439420] pci 0000:02:09.0: pci_set_of_node: of_node: (null)
	[    4.494288] pci_bus 0000:03: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@1,0
	[    4.511394] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
	[    4.525084] pci_bus 0000:05: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@5,0
	[    4.542173] pci_bus 0000:06: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0/pcie@7,0
	[    4.578575] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
	[    4.612159] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)

Which reflects the PCIe topology.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-08-12  7:48 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03  4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
2021-08-03  4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
2021-08-03 22:22   ` Rob Herring
2021-08-03  4:38 ` [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml Mauro Carvalho Chehab
2021-08-03 22:27   ` Rob Herring
2021-08-03  4:38 ` [PATCH v3 3/4] dt-bindings: PCI: kirin: Add support for Kirin970 Mauro Carvalho Chehab
2021-08-03  4:38 ` [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY Mauro Carvalho Chehab
2021-08-03 22:29   ` Rob Herring
2021-08-03 22:11 ` [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Rob Herring
2021-08-04  6:50   ` Mauro Carvalho Chehab
2021-08-04 16:28     ` Rob Herring
2021-08-05  7:46       ` Mauro Carvalho Chehab
2021-08-05  7:58         ` Mauro Carvalho Chehab
2021-08-06 16:23           ` Rob Herring
2021-08-10  9:42             ` Mauro Carvalho Chehab
2021-08-10 13:44               ` Rob Herring
2021-08-10 14:20                 ` Mauro Carvalho Chehab
2021-08-10 17:13                   ` Rob Herring
2021-08-10 17:52                     ` Rob Herring
2021-08-11  7:11                       ` Mauro Carvalho Chehab
2021-08-11  6:46                     ` Mauro Carvalho Chehab
2021-08-12  3:13                       ` Rob Herring
2021-08-12  7:48                         ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).