Refactoring Microchip PCIe driver and add StarFive PCIe

Add PLDA XpressRICH PCIe host common properties dt-binding doc. Microchip PolarFire PCIe host using PLDA IP.
Move common properties from Microchip PolarFire PCIe host to PLDA files.

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

Reviewed-by Hal Feng <[email protected]>

Add PLDA PCIe controller driver codes, Move them from Microchip PolarFire PCIe host driver codes. And move pcie-microchip-host.c to plda directory.

The change includes:
- copy the IP register marcos.
- Add related data structures of PCIe host instance. mc_pcie --> plda_pcie (Get most of data members) mc_msi --> plda_msi
add plda_pcie_ops and plda_evt data structures.
- function rename list:

  mc_pcie_enable_msi       --> plda_pcie_enable_msi
  mc_pcie_setup_window     --> plda_pcie_setup_window
  mc_pcie_setup_windows    --> plda_pcie_setup_iomems

mc_pcie_init_irq_domains --> plda_pcie_init_irq_domains mc_allocate_msi_domains --> plda_allocate_msi_domains
mc_init_interrupts --> plda_pcie_init_irq
msi interrupts related functions and irq domain (primary function is mc_handle_msi):
mc_handle_msi --> plda_handle_msi
intx interrupts related functions and irq domain (primary function is mc_handle_intx):
mc_handle_intx --> plda_handle_intx
event interrupts:
mc_handle_event --> plda_handle_event
- For PolarFire implements non-plda local interrupt events, most of event interrupt process codes can not be re-used. PLDA implements new codes and irq domain ops like PolarFire.
New event functions:
plda_event_handler
plda_pcie_event_map
plda_ack_event_irq

plda_mask_event_irq
plda_unmask_event_irq
plda_hwirq_to_mask
- plda_handle_event adds a new irqnum to event num mapping codes for PLDA local event except DMA engine interrupt events. The DMA engine interrupt events are implemented by vendors. So do not add these events. PolarFire PCIe uses get_events function pointer to get their events num.

Quoted 32 lines · Minda Chen

Add PLDA PCIe controller driver codes, Move them from Microchip PolarFire PCIe host driver codes. And move pcie-microchip-host.c to plda directory.

The change includes:
- copy the IP register marcos.
- Add related data structures of PCIe host instance. mc_pcie --> plda_pcie (Get most of data members) mc_msi --> plda_msi
add plda_pcie_ops and plda_evt data structures.
- function rename list:

  mc_pcie_enable_msi       --> plda_pcie_enable_msi
  mc_pcie_setup_window     --> plda_pcie_setup_window
  mc_pcie_setup_windows    --> plda_pcie_setup_iomems

mc_pcie_init_irq_domains --> plda_pcie_init_irq_domains mc_allocate_msi_domains --> plda_allocate_msi_domains
mc_init_interrupts --> plda_pcie_init_irq
msi interrupts related functions and irq domain (primary function is mc_handle_msi):
mc_handle_msi --> plda_handle_msi
intx interrupts related functions and irq domain (primary function is mc_handle_intx):
mc_handle_intx --> plda_handle_intx
event interrupts:
mc_handle_event --> plda_handle_event
- For PolarFire implements non-plda local interrupt events, most of event interrupt process codes can not be re-used. PLDA implements new codes and irq domain ops like PolarFire.
New event functions:
plda_event_handler
plda_pcie_event_map
plda_ack_event_irq

Quoted 8 lines

plda_mask_event_irq
plda_unmask_event_irq
plda_hwirq_to_mask
- plda_handle_event adds a new irqnum to event num mapping codes for PLDA local event except DMA engine interrupt events. The DMA engine interrupt events are implemented by vendors. So do not add these events. PolarFire PCIe uses get_events function pointer to get their events num.

There's a lot going on here, which makes this hard to review. If possible, I suggest splitting this into multiple patches:

- Move code to plda/ directory with no other changes
- Rename structs/functions from mc_* to plda_* with no other changes
- Add PolarFire-specific code

Other general things:

- Capitalize PLDA, IRQ, INTx, MSI, PCIe in commit logs, comments, dmesg text
- Sort MAINTAINERS entries alphabetically; see

    https://git.kernel.org/linus/c192ac735768

Bjorn

MAINTAINERS +8 -0

+PCI DRIVER FOR PLDA PCIE IP
+M:	Daire McNamara <[email protected]>
+M:	Kevin Xie <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/plda,*
+F:	drivers/pci/controller/plda/*plda*
+
 PCI DRIVER FOR FREESCALE LAYERSCAPE
 M:	Minghuan Lian <[email protected]>
 M:	Mingkai Hu <[email protected]>

Hi--

MAINTAINERS S: Maintained · line 16172

Quoted 8 lines · Minda Chen

MAINTAINERS +0 -0

index 8e4f9d5dca55..ec59c6d00bf9 100644
@@ -16170,6 +16170,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/cdns,*
 F:	drivers/pci/controller/cadence/
 

This new entry is not in the correct location. It should be in alphabetical order, so it goes between

PCI DRIVER FOR NXP LAYERSCAPE GEN4 CONTROLLER
and
PCI DRIVER FOR RENESAS R-CAR

Thanks.

Quoted 11 lines

MAINTAINERS +8 -0

+PCI DRIVER FOR PLDA PCIE IP
+M:	Daire McNamara <[email protected]>
+M:	Kevin Xie <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/plda,*
+F:	drivers/pci/controller/plda/*plda*
+
 PCI DRIVER FOR FREESCALE LAYERSCAPE
 M:	Minghuan Lian <[email protected]>
 M:	Mingkai Hu <[email protected]>

Add StarFive JH7110 SoC PCIe controller dt-bindings. JH7110 using PLDA XpressRICH PCIe host controller IP.

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

Reviewed-by Hal Feng <[email protected]>

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

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

new file mode 100644
index 000000000000..9273e029fb20
@@ -0,0 +1,133 @@
+# 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 host controller
+
+maintainers:
+  - Kevin Xie <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: plda,xpressrich3-axi-common.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+  - $ref: /schemas/gpio/gpio-consumer-common.yaml#
+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  clocks:
+    items:
+      - description: NOC bus clock
+      - description: Transport layer clock
+      - description: AXI MST0 clock
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE0 reset
+      - description: AXI SLAVE 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:

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

+
+  phys:
+    description:
+      Specified PHY is attached to PCIe controller.
+    maxItems: 1
+
+required:
+  - compatible
+  - clocks
+  - resets
+  - starfive,stg-syscon
+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map
+

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

+            compatible = "starfive,jh7110-pcie";
+            reg = <0x9 0x40000000 0x0 0x10000000>,
+                  <0x0 0x2b000000 0x0 0x1000000>;
+            reg-names = "cfg", "apb";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            device_type = "pci";
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
+            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+            bus-range = <0x0 0xff>;
+            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>,
+                     <&stgcrg 16>;
+            reset-gpios = <&gpios 26 GPIO_ACTIVE_LOW>;
+            phys = <&pciephy0>;
+
+            pcie_intc0: interrupt-controller {
+                #address-cells = <0>;
+                #interrupt-cells = <1>;
+                interrupt-controller;
+            };
+        };
+    };

Signature · 1 line

2.17.1

Quoted 78 lines · Minda Chen

Add StarFive JH7110 SoC PCIe controller dt-bindings. JH7110 using PLDA XpressRICH PCIe host controller IP.

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

Reviewed-by Hal Feng <[email protected]>

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

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

new file mode 100644
index 000000000000..9273e029fb20
@@ -0,0 +1,133 @@
+# 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 host controller
+
+maintainers:
+  - Kevin Xie <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: plda,xpressrich3-axi-common.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+  - $ref: /schemas/gpio/gpio-consumer-common.yaml#
+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  clocks:
+    items:
+      - description: NOC bus clock
+      - description: Transport layer clock
+      - description: AXI MST0 clock
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE0 reset
+      - description: AXI SLAVE 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.

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

Quoted 4 lines

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

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

These property names tie them closely with naming on the jh7110, but there's little value in specifying all of these offsets when you have one implementation where they are all fixed.

Do you know what the jh81xx stuff is going to do yet w.r.t. PCI and if so, how could you reuse this property?

Particularly, saying "register 0" seems unlikely to transfer well between SoCs.
I'd be inclined to drop the offsets entirely & rely on match data to provide them if needed in the future.

Quoted 15 lines

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

+
+  phys:
+    description:
+      Specified PHY is attached to PCIe controller.
+    maxItems: 1
+
+required:
+  - compatible
+  - clocks
+  - resets
+  - starfive,stg-syscon
+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map
+

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

Quoted 10 lines

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

+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2b000000 {

nit: you don't need labels in examples if they are not referenced anywhere.

Otherwise, this looks good to me.

Thanks,
Conor.

Quoted 45 lines

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

+            compatible = "starfive,jh7110-pcie";
+            reg = <0x9 0x40000000 0x0 0x10000000>,
+                  <0x0 0x2b000000 0x0 0x1000000>;
+            reg-names = "cfg", "apb";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            device_type = "pci";
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
+            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+            bus-range = <0x0 0xff>;
+            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>,
+                     <&stgcrg 16>;
+            reset-gpios = <&gpios 26 GPIO_ACTIVE_LOW>;
+            phys = <&pciephy0>;
+
+            pcie_intc0: interrupt-controller {
+                #address-cells = <0>;
+                #interrupt-cells = <1>;
+                interrupt-controller;
+            };
+        };
+    };

Signature · 1 line

2.17.1

Quoted 15 lines · Conor Dooley

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

Quoted 13 lines · Minda Chen

Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file +13 -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:
+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
+      for PCIe.

Quoted 3 lines

These property names tie them closely with naming on the jh7110, but there's little value in specifying all of these offsets when you have one implementation where they are all fixed.

Yes, the offset value is tied to SoC.

Quoted 2 lines

Do you know what the jh81xx stuff is going to do yet w.r.t. PCI and if so, how could you reuse this property?

I do not participate in jh8100. But I heard sys-syscon is exist in 81xx. But I think stg-syscon and sys-syscon can be move to a common dt-binding doc. Bot 71x0 and 81x0 driver can use this.

Quoted 4 lines

Particularly, saying "register 0" seems unlikely to transfer well between SoCs.
I'd be inclined to drop the offsets entirely & rely on match data to provide them if needed in the future.

That's ok. The dts can change to starfive,stg-syscon = <&stg_syscon>; I will try to move the offset to driver match data.

Add StarFive JH7110 SoC PCIe controller dt-bindings. JH7110 using PLDA XpressRICH PCIe host controller IP.

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

Reviewed-by Hal Feng <[email protected]>

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

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

new file mode 100644
index 000000000000..9273e029fb20
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---

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

+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  clocks:
+    items:
+      - description: NOC bus clock
+      - description: Transport layer clock
+      - description: AXI MST0 clock
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE0 reset
+      - description: AXI SLAVE 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:

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

+
+  phys:
+    description:
+      Specified PHY is attached to PCIe controller.
+    maxItems: 1

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

+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2b000000 {
+            compatible = "starfive,jh7110-pcie";
+            reg = <0x9 0x40000000 0x0 0x10000000>,
+                  <0x0 0x2b000000 0x0 0x1000000>;
+            reg-names = "cfg", "apb";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            device_type = "pci";
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;

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

+            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>,
+                     <&stgcrg 16>;
+            reset-gpios = <&gpios 26 GPIO_ACTIVE_LOW>;
+            phys = <&pciephy0>;
+
+            pcie_intc0: interrupt-controller {
+                #address-cells = <0>;
+                #interrupt-cells = <1>;
+                interrupt-controller;
+            };
+        };
+    };

Signature · 1 line

2.17.1

Quoted 19 lines · Minda Chen

Add StarFive JH7110 SoC PCIe controller dt-bindings. JH7110 using PLDA XpressRICH PCIe host controller IP.

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

Reviewed-by Hal Feng <[email protected]>

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

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

new file mode 100644
index 000000000000..9273e029fb20
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---

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

Quoted 10 lines

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

+$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PCIe host controller
+
+maintainers:
+  - Kevin Xie <[email protected]>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#

Can go in common schema. Unless endpoint mode is or is going to be supported?

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

Quoted 2 lines

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

+  - $ref: plda,xpressrich3-axi-common.yaml#
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#

This is a bit odd. Do you really need to define the host bridge as an msi-controller? Many host bridges are, but they don't need to be defined as one in DT because PCIe spec defines its own way to do MSIs.

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

Quoted 1 line

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

+  - $ref: /schemas/gpio/gpio-consumer-common.yaml#

No, you don't need to reference this. You need to define what properties from it you use (reset-gpios).

Though if this is for PERST#, then I'd use "perst-gpios" instead.

And why is that not common? It's board specific, not SoC, whether or not there's GPIO control of it.

Quoted 44 lines

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

+
+properties:
+  compatible:
+    const: starfive,jh7110-pcie
+
+  clocks:
+    items:
+      - description: NOC bus clock
+      - description: Transport layer clock
+      - description: AXI MST0 clock
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: noc
+      - const: tl
+      - const: axi_mst0
+      - const: apb
+
+  resets:
+    items:
+      - description: AXI MST0 reset
+      - description: AXI SLAVE0 reset
+      - description: AXI SLAVE 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.

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

Quoted 6 lines

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

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

Perhaps somewhere in here state what these registers are for.

Quoted 11 lines

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

+
+  phys:
+    description:
+      Specified PHY is attached to PCIe controller.
+    maxItems: 1
+
+required:
+  - compatible
+  - clocks
+  - resets
+  - starfive,stg-syscon

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

Quoted 3 lines

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

+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map

These can all be common.

So could 'compatible' though it is implicitly required anyways as without it, the schema is never applied.

Quoted 25 lines

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

+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0: pcie@2b000000 {
+            compatible = "starfive,jh7110-pcie";
+            reg = <0x9 0x40000000 0x0 0x10000000>,
+                  <0x0 0x2b000000 0x0 0x1000000>;
+            reg-names = "cfg", "apb";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            device_type = "pci";
+            ranges = <0x82000000  0x0 0x30000000  0x0 0x30000000 0x0 0x08000000>,
+                     <0xc3000000  0x9 0x00000000  0x9 0x00000000 0x0 0x40000000>;
+            starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+            bus-range = <0x0 0xff>;
+            interrupt-parent = <&plic>;
+            interrupts = <56>;

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

Quoted 6 lines

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

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

Weird!?

Quoted 25 lines

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

+            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>,
+                     <&stgcrg 16>;
+            reset-gpios = <&gpios 26 GPIO_ACTIVE_LOW>;
+            phys = <&pciephy0>;
+
+            pcie_intc0: interrupt-controller {
+                #address-cells = <0>;
+                #interrupt-cells = <1>;
+                interrupt-controller;
+            };
+        };
+    };

Signature · 1 line

2.17.1

This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the same IP and have commit their codes, which are mixed with PLDA controller codes and Microchip platform codes.

For re-use the PLDA controller codes, I request refactoring microchip codes, move PLDA common codes to PLDA files.
Desigware and Cadence is good example for refactoring codes.

So first step is extract the PLDA common codes from microchip, and refactoring the microchip codes.(patch1 - 2)
Then, add Starfive codes. (patch3 - 4)

This patchset is base on v6.5-rc1

patch1 is move PLDA XpressRICH PCIe host common properties dt-binding
docs from microchip,pcie-host.yaml
patch2 is extracting the PLDA common codes from microchip Polarfire PCIe
codes. The change list in the commit message.
patch3 is add StarFive JH7110 PCIe dt-binding doc.
patch4 is add StarFive JH7110 Soc PCIe codes.

Quoted 13 lines · Minda Chen

This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the same IP and have commit their codes, which are mixed with PLDA controller codes and Microchip platform codes.

For re-use the PLDA controller codes, I request refactoring microchip codes, move PLDA common codes to PLDA files.
Desigware and Cadence is good example for refactoring codes.

So first step is extract the PLDA common codes from microchip, and refactoring the microchip codes.(patch1 - 2)
Then, add Starfive codes. (patch3 - 4)

Quoted 9 lines

This patchset is base on v6.5-rc1

patch1 is move PLDA XpressRICH PCIe host common properties dt-binding
docs from microchip,pcie-host.yaml
patch2 is extracting the PLDA common codes from microchip Polarfire PCIe
codes. The change list in the commit message.
patch3 is add StarFive JH7110 PCIe dt-binding doc.
patch4 is add StarFive JH7110 Soc PCIe codes.

Hi Rob, Krzysztof(K.K) and Conor

Do you have any comments for dts-binding doc patch? (patch 1 and patch3) Thanks.

This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the same IP and have commit their codes, which are mixed with PLDA controller codes and Microchip platform codes.

For re-use the PLDA controller codes, I request refactoring microchip codes, move PLDA common codes to PLDA files.
Desigware and Cadence is good example for refactoring codes.

So first step is extract the PLDA common codes from microchip, and refactoring the microchip codes.(patch1 - 2)
Then, add Starfive codes. (patch3 - 4)

This patchset is base on v6.5-rc1

patch1 is move PLDA XpressRICH PCIe host common properties dt-binding
docs from microchip,pcie-host.yaml
patch2 is extracting the PLDA common codes from microchip Polarfire PCIe
codes. The change list in the commit message.
patch3 is add StarFive JH7110 PCIe dt-binding doc.
patch4 is add StarFive JH7110 Soc PCIe codes.

I have noticed that Daire have changed microchip's codes.
https://patchwork.kernel.org/project/linux-pci/cover/[email protected]/
I have changed patch2 base on their commits. StarFive PCIe driver still can work. But their codes is under reviewed and maybe changing. Do not base on their changes first.
I will base on their commit to change patch2 as soon as their commits are accepted.

previous version:
v1:https://patchwork.kernel.org/project/linux-pci/cover/[email protected]/

change:
v2:
patch1:
- squash dt-bindings patches to patch1
- add 'required' list.
- plda doc rename to plda,xpressrich-axi-common.yaml
patch2:
- squash the microchip modification patch to patch 2.
patch3:
- remove the plda common required property.
patch4:
- Sync the hide rc bar ops with config read function.
- Revert the T_PVPERL to 100ms and add comments for the source.
- Replace the link check function by the standard link ops.
- Convert to new pm ops marcos.
- Some formats modification.
- pcie-plda-host modification merge to patch4.
other:
- remove the pcie-plda-plat.c
- remove the starfive dts patch first. for it depends on stg clock and syscon setting.

Minda Chen (4):
dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
PCI: plda: Get common codes from Microchip PolarFire host
dt-bindings: PCI: Add StarFive JH7110 PCIe controller
PCI: starfive: Add JH7110 PCIe controller

 .../bindings/pci/microchip,pcie-host.yaml     |  49 +-
 .../pci/plda,xpressrich3-axi-common.yaml      |  69 ++
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 133 ++++
 MAINTAINERS                                   |  19 +-
 drivers/pci/controller/Kconfig                |   9 +-
 drivers/pci/controller/Makefile               |   2 +-
 drivers/pci/controller/plda/Kconfig           |  31 +
 drivers/pci/controller/plda/Makefile          |   4 +
 .../{ => plda}/pcie-microchip-host.c          | 594 ++--------------
 drivers/pci/controller/plda/pcie-plda-host.c  | 665 ++++++++++++++++++
 drivers/pci/controller/plda/pcie-plda.h       | 242 +++++++
 drivers/pci/controller/plda/pcie-starfive.c   | 438 ++++++++++++
 12 files changed, 1645 insertions(+), 610 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich3-axi-common.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
 create mode 100644 drivers/pci/controller/plda/Kconfig
 create mode 100644 drivers/pci/controller/plda/Makefile
 rename drivers/pci/controller/{ => plda}/pcie-microchip-host.c (50%)
 create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c
 create mode 100644 drivers/pci/controller/plda/pcie-plda.h
 create mode 100644 drivers/pci/controller/plda/pcie-starfive.c

base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 --
2.17.1

Quoted 14 lines · Minda Chen

This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the same IP and have commit their codes, which are mixed with PLDA controller codes and Microchip platform codes.

For re-use the PLDA controller codes, I request refactoring microchip codes, move PLDA common codes to PLDA files.
Desigware and Cadence is good example for refactoring codes.

So first step is extract the PLDA common codes from microchip, and refactoring the microchip codes.(patch1 - 2)
Then, add Starfive codes. (patch3 - 4)

Quoted 8 lines

This patchset is base on v6.5-rc1

patch1 is move PLDA XpressRICH PCIe host common properties dt-binding
docs from microchip,pcie-host.yaml
patch2 is extracting the PLDA common codes from microchip Polarfire PCIe
codes. The change list in the commit message.
patch3 is add StarFive JH7110 PCIe dt-binding doc.
patch4 is add StarFive JH7110 Soc PCIe codes.

Hi Minda,

To test this series properly it needs matching nodes in the VisionFive 2 device trees, but it seems to be missing from this version of the patch series. If I apply the device tree patch from v1 I get errors like this:

pcie-starfive 2b000000.pcie: invalid resource (null) pcie-starfive 2b000000.pcie: error -EINVAL: failed to map reg memory

It would be great if you included the device tree patch in the next series so this can actually be tested.

/Emil

Quoted 66 lines

I have noticed that Daire have changed microchip's codes.
https://patchwork.kernel.org/project/linux-pci/cover/[email protected]/
I have changed patch2 base on their commits. StarFive PCIe driver still can work. But their codes is under reviewed and maybe changing. Do not base on their changes first.
I will base on their commit to change patch2 as soon as their commits are accepted.

previous version:
v1:https://patchwork.kernel.org/project/linux-pci/cover/[email protected]/

change:
v2:
patch1:
- squash dt-bindings patches to patch1
- add 'required' list.
- plda doc rename to plda,xpressrich-axi-common.yaml
patch2:
- squash the microchip modification patch to patch 2.
patch3:
- remove the plda common required property.
patch4:
- Sync the hide rc bar ops with config read function.
- Revert the T_PVPERL to 100ms and add comments for the source.
- Replace the link check function by the standard link ops.
- Convert to new pm ops marcos.
- Some formats modification.
- pcie-plda-host modification merge to patch4.
other:
- remove the pcie-plda-plat.c
- remove the starfive dts patch first. for it depends on stg clock and syscon setting.

Minda Chen (4):
dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
PCI: plda: Get common codes from Microchip PolarFire host
dt-bindings: PCI: Add StarFive JH7110 PCIe controller
PCI: starfive: Add JH7110 PCIe controller

 .../bindings/pci/microchip,pcie-host.yaml     |  49 +-
 .../pci/plda,xpressrich3-axi-common.yaml      |  69 ++
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 133 ++++
 MAINTAINERS                                   |  19 +-
 drivers/pci/controller/Kconfig                |   9 +-
 drivers/pci/controller/Makefile               |   2 +-
 drivers/pci/controller/plda/Kconfig           |  31 +
 drivers/pci/controller/plda/Makefile          |   4 +
 .../{ => plda}/pcie-microchip-host.c          | 594 ++--------------
 drivers/pci/controller/plda/pcie-plda-host.c  | 665 ++++++++++++++++++
 drivers/pci/controller/plda/pcie-plda.h       | 242 +++++++
 drivers/pci/controller/plda/pcie-starfive.c   | 438 ++++++++++++
 12 files changed, 1645 insertions(+), 610 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich3-axi-common.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
 create mode 100644 drivers/pci/controller/plda/Kconfig
 create mode 100644 drivers/pci/controller/plda/Makefile
 rename drivers/pci/controller/{ => plda}/pcie-microchip-host.c (50%)
 create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c
 create mode 100644 drivers/pci/controller/plda/pcie-plda.h
 create mode 100644 drivers/pci/controller/plda/pcie-starfive.c

base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 --
2.17.1