Add JH7110 PCIe driver support

Add PCIe controller driver dt-binding documents
for StarFive JH7110 SoC platform.

Signed-off-by Minda Chen <[email protected]>

---
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +66 -0

new file mode 100644
index 000000000000..fa4829766195
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PCIe 2.0 host controller
+
+maintainers:
+  - Minda Chen <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: reg
+      - const: config
+
+  msi-parent: true
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE reset
+      - description: AXI SLAVE0 reset
+      - description: PCIE BRIDGE reset
+      - description: PCIE CORE reset
+      - description: PCIE APB reset
+
+  reset-names:
+    items:
+      - const: mst0
+      - const: slv0
+      - const: slv
+      - const: brg
+      - const: core
+      - const: apb
+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +20 -0

+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  phys:
+    maxItems: 1
+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +7 -0

+  - interrupts
+  - interrupt-map-mask
+  - interrupt-map
+  - clocks
+  - clock-names
+  - resets
+  - msi-controller

Quoted 13 lines · Minda Chen

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -0

+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 106

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Are you sure this is correct? You have interrupt controller as child node.

Quoted 27 lines · Krzysztof Kozlowski

Quoted 13 lines · Minda Chen

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -0

+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 106

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Quoted 1 line

Are you sure this is correct? You have interrupt controller as child node.

I know existing stuff in-tree is far from a guarantee that it'll be right, but this does at least follow what we've got for PolarFire SoC:
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml

Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in similar waters.
This note existed in the original text form binding of the Microchip PCI controller:

| +NOTE:
| +The core provides a single interrupt for both INTx/MSI messages. So,
| +create an interrupt controller node to support 'interrupt-map' DT
| +functionality.  The driver will create an IRQ domain for this map, decode
| +the four INTx interrupts in ISR and route them to this domain.

Given the similarities, I figure the same requirement applies here too. Minda?

Quoted 44 lines · Conor Dooley

Quoted 27 lines · Krzysztof Kozlowski

Quoted 13 lines · Minda Chen

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -0

+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 106

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Quoted 1 line

Are you sure this is correct? You have interrupt controller as child node.

I know existing stuff in-tree is far from a guarantee that it'll be right, but this does at least follow what we've got for PolarFire SoC:
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml

Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in similar waters.
This note existed in the original text form binding of the Microchip PCI controller:

| +NOTE:
| +The core provides a single interrupt for both INTx/MSI messages. So,
| +create an interrupt controller node to support 'interrupt-map' DT
| +functionality.  The driver will create an IRQ domain for this map, decode
| +the four INTx interrupts in ISR and route them to this domain.

Quoted 2 lines

Given the similarities, I figure the same requirement applies here too. Minda?

Further, if, as I currently suspect, there's a lot of commonality here, should the binding as well as the driver be split into common pdla bits and microchip/starfive specific ones?

Suppose that's more one for you Krzysztof.

Quoted 27 lines · Krzysztof Kozlowski

Quoted 13 lines · Minda Chen

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -0

+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 106

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Quoted 1 line

Are you sure this is correct? You have interrupt controller as child node.

I know existing stuff in-tree is far from a guarantee that it'll be right, but this does at least follow what we've got for PolarFire SoC:
Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml

Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in similar waters.
This note existed in the original text form binding of the Microchip PCI controller:

| +NOTE:
| +The core provides a single interrupt for both INTx/MSI messages. So,
| +create an interrupt controller node to support 'interrupt-map' DT
| +functionality.  The driver will create an IRQ domain for this map, decode
| +the four INTx interrupts in ISR and route them to this domain.

Given the similarities, I figure the same requirement applies here too. Minda?

Cheers,
Conor.

Quoted 2 lines · Minda Chen

Add PCIe controller driver dt-binding documents
for StarFive JH7110 SoC platform.

Use subject prefixes matching the subsystem (which you can get for example with git log --oneline -- DIRECTORY_OR_FILE on the directory your patch is touching). Missing: 's'

Subject: drop second/last, redundant "dt-binding documents". The "dt-bindings" prefix is already stating that these are bindings and documentation.

Drop also full stop.

Quoted 83 lines

Signed-off-by Minda Chen <[email protected]>

---
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +70 -0

new file mode 100644
index 000000000000..fa4829766195
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PCIe 2.0 host controller
+
+maintainers:
+  - Minda Chen <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: reg
+      - const: config
+
+  msi-parent: true
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE reset
+      - description: AXI SLAVE0 reset
+      - description: PCIE BRIDGE reset
+      - description: PCIE CORE reset
+      - description: PCIE APB reset
+
+  reset-names:
+    items:
+      - const: mst0
+      - const: slv0
+      - const: slv
+      - const: brg
+      - const: core
+      - const: apb
+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+    description:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 76

Quoted 6 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +6 -0

+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
+      for PCIe.
+
+  pwren-gpios:
+    description: Should specify the GPIO for controlling the PCI bus device power on.

What are these? Different than defined in gpio-consumer-common?

Quoted 20 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +20 -0

+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  phys:
+    maxItems: 1
+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 106

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"

Keep consistent quotes - either ' or "

Are you sure this is correct? You have interrupt controller as child node.

Quoted 11 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +11 -0

+  - interrupts
+  - interrupt-map-mask
+  - interrupt-map
+  - clocks
+  - clock-names
+  - resets
+  - msi-controller
+
+unevaluatedProperties: false
+
+examples:

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 123

Quoted 6 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +6 -0

+  - |
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2B000000 {

Lowercase hex. Everywhere.

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 129

Quoted 6 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +6 -0

+            compatible = "starfive,jh7110-pcie";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            reg = <0x0 0x2B000000 0x0 0x1000000>,
+                  <0x9 0x40000000 0x0 0x10000000>;

reg (and reg-names and ranges) is always second property.

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 142

Quoted 13 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -0

+            reg-names = "reg", "config";
+            device_type = "pci";
+            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+            bus-range = <0x0 0xff>;
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
+            interrupt-parent = <&plic>;
+            interrupts = <56>;
+            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+            interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
+                            <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
+                            <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
+                            <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;

Best regards,
Krzysztof

Quoted 11 lines

Quoted 11 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +11 -0

+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+    description:

Quoted 9 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 76

Quoted 6 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +6 -0

+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
+      for PCIe.
+
+  pwren-gpios:
+    description: Should specify the GPIO for controlling the PCI bus device power on.

What are these? Different than defined in gpio-consumer-common?

power gpio board level configuration. It it not a requried property

Quoted 20 lines · Minda Chen

Quoted 20 lines

Quoted 7 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +7 -0

+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml line 76

Quoted 10 lines

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +10 -0

+        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+    description:
+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
+      for PCIe.
+
+  pwren-gpios:
+    description: Should specify the GPIO for controlling the PCI bus device power on.

What are these? Different than defined in gpio-consumer-common?

Quoted 1 line

power gpio board level configuration. It it not a requried property

What is "board level configuration"? Again - is it different than powerdown-gpios from gpio-consumer-common.yaml?

Best regards,
Krzysztof

Add PCIe controller driver dt-binding documents
for StarFive JH7110 SoC platform.

Signed-off-by Minda Chen <[email protected]>

---
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +8 -0

new file mode 100644
index 000000000000..fa4829766195
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PCIe 2.0 host controller
+

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +136 -0

+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: reg
+      - const: config
+
+  msi-parent: true
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE reset
+      - description: AXI SLAVE0 reset
+      - description: PCIE BRIDGE reset
+      - description: PCIE CORE reset
+      - description: PCIE APB reset
+
+  reset-names:
+    items:
+      - const: mst0
+      - const: slv0
+      - const: slv
+      - const: brg
+      - const: core
+      - const: apb
+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
+    description:
+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
+      for PCIe.
+
+  pwren-gpios:
+    description: Should specify the GPIO for controlling the PCI bus device power on.
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  phys:
+    maxItems: 1
+
+  interrupt-controller:
+    type: object
+    properties:
+      '#address-cells':
+        const: 0
+
+      '#interrupt-cells':
+        const: 1
+
+      interrupt-controller: true
+
+    required:
+      - '#address-cells'
+      - '#interrupt-cells'
+      - interrupt-controller
+
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"
+  - interrupts
+  - interrupt-map-mask
+  - interrupt-map
+  - clocks
+  - clock-names
+  - resets
+  - msi-controller
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2B000000 {
+            compatible = "starfive,jh7110-pcie";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            reg = <0x0 0x2B000000 0x0 0x1000000>,
+                  <0x9 0x40000000 0x0 0x10000000>;
+            reg-names = "reg", "config";
+            device_type = "pci";
+            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+            bus-range = <0x0 0xff>;
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
+            interrupt-parent = <&plic>;
+            interrupts = <56>;
+            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+            interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
+                            <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
+                            <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
+                            <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;
+            msi-parent = <&pcie0>;
+            msi-controller;
+            clocks = <&syscrg 86>,
+                     <&stgcrg 10>,
+                     <&stgcrg 8>,
+                     <&stgcrg 9>;
+            clock-names = "noc", "tl", "axi_mst0", "apb";
+            resets = <&stgcrg 11>,
+                     <&stgcrg 12>,
+                     <&stgcrg 13>,
+                     <&stgcrg 14>,
+                     <&stgcrg 15>,

The PCIe is a PCIe2, single lane PCIe compliant controller.

Signed-off-by Minda Chen <[email protected]>

---
 .../jh7110-starfive-visionfive-2.dtsi         | 58 ++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 88 +++++++++++++++++++
 2 files changed, 146 insertions(+)

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +0 -0

index cf0a66faf5d3..4552919e69b0 100644

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +14 -0

 	uart0_pins: uart0-0 {
 		tx-pins {
 			pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX,

@@ -228,3 +272,17 @@
 	dr_mode = "peripheral";
 	status = "okay";
 };
+
+&pcie0 {
+	pinctrl-names = "default";
+	reset-gpios = <&sysgpio 26 GPIO_ACTIVE_LOW>;
+	phys = <&pciephy0>;
+	status = "okay";
+};
+
+&pcie1 {
+	pinctrl-names = "default";
+	reset-gpios = <&sysgpio 28 GPIO_ACTIVE_LOW>;
+	phys = <&pciephy1>;
+	status = "okay";
+};

arch/riscv/boot/dts/starfive/jh7110.dtsi +0 -0

index 2f67196ffac0..c309ec550ba7 100644

This patchset adds PCIe driver for the StarFive JH7110 SoC. The patch has been tested on the VisionFive 2 board. The test devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter.

This patchset should be applied after the patchset [1], [2], [3] and[4]:
[1] https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/
[4] https://patchwork.kernel.org/project/linux-usb/cover/[email protected]/

+CC Daire

Hey Minda,

Quoted 3 lines · Minda Chen

This patchset adds PCIe driver for the StarFive JH7110 SoC. The patch has been tested on the VisionFive 2 board. The test devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter.

I was talking with Daire last week about some changes he's working on for the microchip driver, and we seemed to recall an off-list email sent to Daire & Bjorn about extracting the common PLDA bits from the pcie-microchip-host driver to be used with an (at that point) unreleased SoC. Perhaps Bjorn has this in his mailbox somewhere still, our corporate mail policy scrubs things from over a year ago & I could not find it.

I realised that that may actually have been StarFive, and the driver on your GitHub [1] certainly felt very familiar to Daire (he said it was very similar to his earlier revisions of his driver).

I've not looked at a diff between this and the version you ship on GitHub, but first a quick inspection it mostly just looks like you did s/plda/sifive/ on the file.

I'm obviously not a PCI maintainer, but if there are common bits between the two drivers, extracting common bits seems like a good idea to me...

https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_devel/drivers/pci/controller/pcie-plda.c

Quoted 6 lines

This patchset should be applied after the patchset [1], [2], [3] and[4]:
[1] https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/
[4] https://patchwork.kernel.org/project/linux-usb/cover/[email protected]/

How many of the dependencies here are compiletime for the driver & how many of them are just for the dts patch?

Cheers,
Conor.

Gah, I never actually CCed Daire. Apologies for the additional email.

Quoted 21 lines · Conor Dooley

+CC Daire

Hey Minda,

Quoted 3 lines · Minda Chen

This patchset adds PCIe driver for the StarFive JH7110 SoC. The patch has been tested on the VisionFive 2 board. The test devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter.

I was talking with Daire last week about some changes he's working on for the microchip driver, and we seemed to recall an off-list email sent to Daire & Bjorn about extracting the common PLDA bits from the pcie-microchip-host driver to be used with an (at that point) unreleased SoC. Perhaps Bjorn has this in his mailbox somewhere still, our corporate mail policy scrubs things from over a year ago & I could not find it.

I realised that that may actually have been StarFive, and the driver on your GitHub [1] certainly felt very familiar to Daire (he said it was very similar to his earlier revisions of his driver).

Quoted 6 lines

I've not looked at a diff between this and the version you ship on GitHub, but first a quick inspection it mostly just looks like you did s/plda/sifive/ on the file.

I'm obviously not a PCI maintainer, but if there are common bits between the two drivers, extracting common bits seems like a good idea to me...

Thanks. It is pleasure to using same common codes. Does common bits changes

Quoted 11 lines

https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_devel/drivers/pci/controller/pcie-plda.c

Quoted 6 lines

This patchset should be applied after the patchset [1], [2], [3] and[4]:
[1] https://patchwork.kernel.org/project/linux-riscv/cover/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://patchwork.kernel.org/project/linux-phy/cover/[email protected]/
[4] https://patchwork.kernel.org/project/linux-usb/cover/[email protected]/

How many of the dependencies here are compiletime for the driver & how many of them are just for the dts patch?

Quoted 2 lines

Cheers,
Conor.

Quoted 22 lines · Conor Dooley

Quoted 21 lines · Conor Dooley

Quoted 3 lines · Minda Chen

This patchset adds PCIe driver for the StarFive JH7110 SoC. The patch has been tested on the VisionFive 2 board. The test devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter.

I was talking with Daire last week about some changes he's working on for the microchip driver, and we seemed to recall an off-list email sent to Daire & Bjorn about extracting the common PLDA bits from the pcie-microchip-host driver to be used with an (at that point) unreleased SoC. Perhaps Bjorn has this in his mailbox somewhere still, our corporate mail policy scrubs things from over a year ago & I could not find it.

I realised that that may actually have been StarFive, and the driver on your GitHub [1] certainly felt very familiar to Daire (he said it was very similar to his earlier revisions of his driver).

I've not looked at a diff between this and the version you ship on GitHub, but first a quick inspection it mostly just looks like you did s/plda/sifive/ on the file.

Quoted 2 lines

Quoted 2 lines

I'm obviously not a PCI maintainer, but if there are common bits between the two drivers, extracting common bits seems like a good idea to me...

Thanks. It is pleasure to using same common codes. Does common bits changes will upstream soon?

And I see there are many difference between pcie-microchip-host and our codes.

Quoted 1 line

Quoted 1 line

https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_devel/drivers/pci/controller/pcie-plda.c

Hey Minda,

Quoted 23 lines · Minda Chen

Quoted 22 lines · Conor Dooley

Quoted 21 lines · Conor Dooley

Quoted 3 lines · Minda Chen

This patchset adds PCIe driver for the StarFive JH7110 SoC. The patch has been tested on the VisionFive 2 board. The test devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter.

I was talking with Daire last week about some changes he's working on for the microchip driver, and we seemed to recall an off-list email sent to Daire & Bjorn about extracting the common PLDA bits from the pcie-microchip-host driver to be used with an (at that point) unreleased SoC. Perhaps Bjorn has this in his mailbox somewhere still, our corporate mail policy scrubs things from over a year ago & I could not find it.

I realised that that may actually have been StarFive, and the driver on your GitHub [1] certainly felt very familiar to Daire (he said it was very similar to his earlier revisions of his driver).

I've not looked at a diff between this and the version you ship on GitHub, but first a quick inspection it mostly just looks like you did s/plda/sifive/ on the file.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

I'm obviously not a PCI maintainer, but if there are common bits between the two drivers, extracting common bits seems like a good idea to me...

Thanks. It is pleasure to using same common codes. Does common bits changes will upstream soon?

I don't quite get what you mean. We've got some changes that are in progress here:
https://lore.kernel.org/linux-pci/[email protected]/
We've been quiet there for a while, but Daire's back looking into Robin's comments in there about the range parsing/window setup at the moment.

I'm not sure if that's what you mean though, since you said "common bits" & Daire was doing that work in a world where there was no jh7110 driver in the mix.
Extracting common bits would be part of the process of adding a new driver, as I don't think there's any real reason to do so without another in-tree user.

Quoted 1 line

And I see there are many difference between pcie-microchip-host and our codes.

Right. I'd expect there to be a fair difference between our integrations of the IP, and therefore there'll be a bunch of non-shareable bits.

You need the stg,syscon & phy bits, and the clock/reset handling is clearly different too.

Quoted 1 line

Quoted 1 line

Quoted 1 line

https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_devel/drivers/pci/controller/pcie-plda.c

I had a bit of a read through this again today with Daire to check what the differences actually are and it *looked* like the main, non-implementation related, differences were the extra "event" domain that was created to simplify the driver & the bottom half interrupt handling.
That all came out of the review process, so it's likely that some of the same requests would be made of you by the PCI maintainers anyway.

As an aside, you should probably run checkpatch --strict on this submission, there's a rake of coding style "issues" in the new code you've added.

Cheers,
Conor.