linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller
@ 2025-08-20 11:18 Randolph Lin
  2025-08-20 11:18 ` [PATCH 1/6] riscv: dts: andes: Define dma-ranges for coherent port Randolph Lin
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Add support for Andes Qilai SoC PCIe controller

These patches introduce driver support for the PCIe controller on the
Andes Qilai SoC.

Randolph Lin (6):
  riscv: dts: andes: Define dma-ranges for coherent port
  PCI: dwc: Add outbound ATU range check callback
  dt-bindings: Add Andes QiLai PCIe support
  riscv: dts: andes: Add PCIe node into the QiLai SoC
  PCI: andes: Add Andes QiLai SoC PCIe host driver support
  MAINTAINERS: Add maintainers for Andes QiLai PCIe driver

 .../bindings/pci/andestech,qilai-pcie.yaml    | 146 +++++++++++
 MAINTAINERS                                   |   7 +
 arch/riscv/boot/dts/andes/qilai.dtsi          |  77 ++++++
 drivers/pci/controller/dwc/Kconfig            |  13 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-andes-qilai.c | 227 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.c  |  18 +-
 drivers/pci/controller/dwc/pcie-designware.h  |   3 +
 8 files changed, 487 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-andes-qilai.c

-- 
2.34.1


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

* [PATCH 1/6] riscv: dts: andes: Define dma-ranges for coherent port
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  2025-08-20 11:18 ` [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback Randolph Lin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Specify the dma-ranges property to map the coherent port
address space from 0x400000000 to 0x4400000000, ensuring
proper DMA address translation for devices under this port.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 arch/riscv/boot/dts/andes/qilai.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/andes/qilai.dtsi b/arch/riscv/boot/dts/andes/qilai.dtsi
index de3de32f8c39..d78d57b3bc52 100644
--- a/arch/riscv/boot/dts/andes/qilai.dtsi
+++ b/arch/riscv/boot/dts/andes/qilai.dtsi
@@ -126,6 +126,7 @@ soc {
 		interrupt-parent = <&plic>;
 		#address-cells = <2>;
 		#size-cells = <2>;
+		dma-ranges = <0x44 0x00000000 0x4 0x00000000 0x4 0x00000000>;
 
 		plmt: timer@100000 {
 			compatible = "andestech,qilai-plmt", "andestech,plmt0";
-- 
2.34.1


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

* [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
  2025-08-20 11:18 ` [PATCH 1/6] riscv: dts: andes: Define dma-ranges for coherent port Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  2025-08-20 15:52   ` Bjorn Helgaas
  2025-08-20 11:18 ` [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support Randolph Lin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Introduce a callback for outbound ATU range checking to support
range validations specific to cases that deviate from the generic
check.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 18 +++++++++++++-----
 drivers/pci/controller/dwc/pcie-designware.h |  3 +++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89aad5a08928..f410aefaeb5e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -535,12 +535,20 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
 	u32 retries, val;
 	u64 limit_addr;
 
-	limit_addr = parent_bus_addr + atu->size - 1;
+	if (pci->ops && pci->ops->outbound_atu_check) {
+		val = pci->ops->outbound_atu_check(pci, atu, &limit_addr);
+		if (val)
+			return val;
+	} else {
+		limit_addr = parent_bus_addr + atu->size - 1;
 
-	if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
-	    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
-	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
-		return -EINVAL;
+		if ((limit_addr & ~pci->region_limit) !=
+		    (parent_bus_addr & ~pci->region_limit) ||
+		    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
+		    !IS_ALIGNED(atu->pci_addr, pci->region_align) ||
+		    !atu->size) {
+			return -EINVAL;
+		}
 	}
 
 	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 00f52d472dcd..40dd2c83b1c7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -469,6 +469,9 @@ struct dw_pcie_ep {
 
 struct dw_pcie_ops {
 	u64	(*cpu_addr_fixup)(struct dw_pcie *pcie, u64 cpu_addr);
+	u32	(*outbound_atu_check)(struct dw_pcie *pcie,
+				      const struct dw_pcie_ob_atu_cfg *atu,
+				      u64 *limit_addr);
 	u32	(*read_dbi)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
 			    size_t size);
 	void	(*write_dbi)(struct dw_pcie *pcie, void __iomem *base, u32 reg,
-- 
2.34.1


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

* [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
  2025-08-20 11:18 ` [PATCH 1/6] riscv: dts: andes: Define dma-ranges for coherent port Randolph Lin
  2025-08-20 11:18 ` [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  2025-08-21  6:11   ` Krzysztof Kozlowski
  2025-08-20 11:18 ` [PATCH 4/6] riscv: dts: andes: Add PCIe node into the QiLai SoC Randolph Lin
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Add the Andes QiLai PCIe node, which includes 3 Root Complexes.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 .../bindings/pci/andestech,qilai-pcie.yaml    | 146 ++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml b/Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml
new file mode 100644
index 000000000000..2c67b3a6828e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/andestech,qilai-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Andes QiLai PCIe host controller
+
+description: |+
+  Andes QiLai PCIe host controller is based on the Synopsys DesignWare
+  PCI core. It shares common features with the PCIe DesignWare core and
+  inherits common properties defined in
+  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
+
+maintainers:
+  - Randolph Lin <randolph@andestech.com>
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: andestech,qilai-pcie
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers.
+      - description: PCIe configuration space region.
+      - description: APB registers.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: apb
+
+  ranges:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 1
+
+  interrupt-controller: true
+
+required:
+  - reg
+  - reg-names
+  - interrupts
+  - "#interrupt-cells"
+  - interrupt-controller
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pci@80000000 {
+        compatible = "andestech,qilai-pcie";
+        device_type = "pci";
+        reg = <0x00 0x80000000 0x00 0x20000000>,
+              <0x20 0x00000000 0x00 0x00010000>,
+              <0x00 0x04000000 0x00 0x00001000>;
+        reg-names = "dbi", "config", "apb";
+        bus-range = <0x0 0xff>;
+        num-viewport = <4>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        ranges = <0x02000000 0x00 0x10000000 0x20 0x10000000 0x0 0xF0000000>,
+                 <0x43000000 0x01 0x00000000 0x21 0x0000000 0x1F 0x00000000>;
+
+        #interrupt-cells = <1>;
+        interrupts = <0xF>;
+        interrupt-names = "msi";
+        interrupt-parent = <&plic0>;
+        interrupt-controller;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0 0 0 1 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 2 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 3 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 4 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>;
+      };
+
+      pci@a0000000 {
+        compatible = "andestech,qilai-pcie";
+        device_type = "pci";
+        reg = <0x00 0xA0000000 0x00 0x20000000>,
+              <0x10 0x00000000 0x00 0x00010000>,
+              <0x00 0x04001000 0x00 0x00001000>;
+        reg-names = "dbi", "config", "apb";
+        bus-range = <0x0 0xff>;
+        num-viewport = <4>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        ranges = <0x02000000 0x00 0x10000000 0x10 0x10000000 0x0 0xF0000000>,
+                 <0x43000000 0x01 0x00000000 0x11 0x00000000 0x7 0x00000000>;
+
+        #interrupt-cells = <1>;
+        interrupts = <0xE>;
+        interrupt-names = "msi";
+        interrupt-parent = <&plic0>;
+        interrupt-controller;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0 0 0 1 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 2 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 3 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 4 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>;
+      };
+
+      pci@c0000000 {
+        compatible = "andestech,qilai-pcie";
+        device_type = "pci";
+        reg = <0x00 0xC0000000 0x00 0x20000000>,
+              <0x18 0x00000000 0x00 0x00010000>,
+              <0x00 0x04002000 0x00 0x00001000>;
+        reg-names = "dbi", "config", "apb";
+        bus-range = <0x0 0xff>;
+        num-viewport = <4>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        ranges = <0x02000000 0x00 0x10000000 0x18 0x10000000 0x0 0xF0000000>,
+                 <0x43000000 0x01 0x00000000 0x19 0x00000000 0x7 0x00000000>;
+
+        #interrupt-cells = <1>;
+        interrupts = <0xD>;
+        interrupt-names = "msi";
+        interrupt-parent = <&plic0>;
+        interrupt-controller;
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0 0 0 1 &plic0 0xD IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 2 &plic0 0xD IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 3 &plic0 0xD IRQ_TYPE_LEVEL_HIGH>,
+                        <0 0 0 4 &plic0 0xD IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
-- 
2.34.1


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

* [PATCH 4/6] riscv: dts: andes: Add PCIe node into the QiLai SoC
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
                   ` (2 preceding siblings ...)
  2025-08-20 11:18 ` [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  2025-08-20 11:18 ` [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support Randolph Lin
  2025-08-20 11:18 ` [PATCH 6/6] MAINTAINERS: Add maintainers for Andes QiLai PCIe driver Randolph Lin
  5 siblings, 0 replies; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Add the Andes QiLai PCIe node, which includes 3 Root Complexes.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 arch/riscv/boot/dts/andes/qilai.dtsi | 76 ++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/riscv/boot/dts/andes/qilai.dtsi b/arch/riscv/boot/dts/andes/qilai.dtsi
index d78d57b3bc52..9a0e32499621 100644
--- a/arch/riscv/boot/dts/andes/qilai.dtsi
+++ b/arch/riscv/boot/dts/andes/qilai.dtsi
@@ -183,5 +183,81 @@ uart0: serial@30300000 {
 			reg-io-width = <4>;
 			no-loopback-test;
 		};
+
+		/* DM0 */
+		pci@80000000 {
+			compatible = "andestech,qilai-pcie";
+			device_type = "pci";
+			reg = <0x00 0x80000000 0x00 0x20000000>, /* DBI registers */
+			      <0x20 0x00000000 0x00 0x00010000>, /* Configuration registers */
+			      <0x00 0x04000000 0x00 0x00001000>; /* APB registers */
+			reg-names = "dbi", "config", "apb";
+			bus-range = <0x0 0xff>;
+			num-viewport = <4>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges = <0x02000000 0x00 0x10000000 0x20 0x10000000 0x0 0xF0000000>,
+				 <0x43000000 0x01 0x00000000 0x21 0x0000000 0x1F 0x00000000>;
+			#interrupt-cells = <1>;
+			interrupts = <0xF IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			interrupt-parent = <&plic>;
+			interrupt-map-mask = <0 0 0 0>;
+			interrupt-map = <0 0 0 1 &plic 0xF IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &plic 0xF IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &plic 0xF IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &plic 0xF IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		/* DM1 */
+		pci@a0000000 {
+			compatible = "andestech,qilai-pcie";
+			device_type = "pci";
+			reg = <0x00 0xA0000000 0x00 0x20000000>, /* DBI registers */
+			      <0x10 0x00000000 0x00 0x00010000>, /* Configuration registers */
+			      <0x00 0x04001000 0x00 0x00001000>; /* APB registers */
+			reg-names = "dbi", "config", "apb";
+			bus-range = <0x0 0xff>;
+			num-viewport = <4>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges = <0x02000000 0x00 0x10000000 0x10 0x10000000 0x0 0xF0000000>,
+				 <0x43000000 0x01 0x00000000 0x11 0x00000000 0x7 0x00000000>;
+			#interrupt-cells = <1>;
+			interrupts = <0xE IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			interrupt-parent = <&plic>;
+			interrupt-controller;
+			interrupt-map-mask = <0 0 0 0>;
+			interrupt-map = <0 0 0 1 &plic 0xE IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &plic 0xE IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &plic 0xE IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &plic 0xE IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		/* DM2 */
+		pci@c0000000 {
+			compatible = "andestech,qilai-pcie";
+			device_type = "pci";
+			reg = <0x00 0xC0000000 0x00 0x20000000>, /* DBI registers */
+			      <0x18 0x00000000 0x00 0x00010000>, /* Configuration registers */
+			      <0x00 0x04002000 0x00 0x00001000>; /* APB registers */
+			reg-names = "dbi", "config", "apb";
+			bus-range = <0x0 0xff>;
+			num-viewport = <4>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges = <0x02000000 0x00 0x10000000 0x18 0x10000000 0x0 0xF0000000>,
+				 <0x43000000 0x01 0x00000000 0x19 0x00000000 0x7 0x00000000>;
+			#interrupt-cells = <1>;
+			interrupts = <0xD IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			interrupt-parent = <&plic>;
+			interrupt-map-mask = <0 0 0 0>;
+			interrupt-map = <0 0 0 1 &plic 0xD IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &plic 0xD IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &plic 0xD IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &plic 0xD IRQ_TYPE_LEVEL_HIGH>;
+		};
 	};
 };
-- 
2.34.1


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

* [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
                   ` (3 preceding siblings ...)
  2025-08-20 11:18 ` [PATCH 4/6] riscv: dts: andes: Add PCIe node into the QiLai SoC Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  2025-08-20 15:54   ` Bjorn Helgaas
  2025-08-20 11:18 ` [PATCH 6/6] MAINTAINERS: Add maintainers for Andes QiLai PCIe driver Randolph Lin
  5 siblings, 1 reply; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Add driver support for DesignWare based PCIe controller in Andes
QiLai SoC. The driver only supports the Root Complex mode.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 drivers/pci/controller/dwc/Kconfig            |  13 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-andes-qilai.c | 227 ++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-andes-qilai.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index ff6b6d9e18ec..a9c5a43f648b 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -49,6 +49,19 @@ config PCIE_AMD_MDB
 	  DesignWare IP and therefore the driver re-uses the DesignWare
 	  core functions to implement the driver.
 
+config PCIE_ANDES_QILAI
+	bool "ANDES QiLai PCIe controller"
+	depends on OF && (RISCV || COMPILE_TEST)
+	depends on PCI_MSI
+	depends on ARCH_ANDES
+	select PCIE_DW_HOST
+	default y if ARCH_ANDES
+	help
+	  Say Y here if you want to enable PCIe controller support on Andes
+	  QiLai SoCs. The Andes QiLai SoCs PCIe controller is based on
+	  DesignWare IP and therefore the driver re-uses the DesignWare
+	  core functions to implement the driver.
+
 config PCI_MESON
 	tristate "Amlogic Meson PCIe controller"
 	default m if ARCH_MESON
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 6919d27798d1..de9583cbd675 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
 obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
 obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
+obj-$(CONFIG_PCIE_ANDES_QILAI) += pcie-andes-qilai.o
 obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
diff --git a/drivers/pci/controller/dwc/pcie-andes-qilai.c b/drivers/pci/controller/dwc/pcie-andes-qilai.c
new file mode 100644
index 000000000000..dd06eee82cac
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the PCIe Controller in QiLai from Andes
+ *
+ * Copyright (C) 2025 Andes Technology Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define PCIE_INTR_CONTROL1			0x15c
+#define PCIE_MSI_CTRL_INT_EN			BIT(28)
+
+#define PCIE_LOGIC_COHERENCY_CONTROL3		0x8e8
+/* Write-Back, Read and Write Allocate */
+#define IOCP_ARCACHE				0xf
+/* Write-Back, Read and Write Allocate */
+#define IOCP_AWCACHE				0xf
+#define PCIE_CFG_MSTR_ARCACHE_MODE		GENMASK(6, 3)
+#define PCIE_CFG_MSTR_AWCACHE_MODE		GENMASK(14, 11)
+#define PCIE_CFG_MSTR_ARCACHE_VALUE		GENMASK(22, 19)
+#define PCIE_CFG_MSTR_AWCACHE_VALUE		GENMASK(30, 27)
+
+#define PCIE_GEN_CONTROL2			0x54
+#define PCIE_CFG_LTSSM_EN			BIT(0)
+
+#define PCIE_REGS_PCIE_SII_PM_STATE		0xc0
+#define SMLH_LINK_UP				BIT(6)
+#define RDLH_LINK_UP				BIT(7)
+#define PCIE_REGS_PCIE_SII_LINK_UP		(SMLH_LINK_UP | RDLH_LINK_UP)
+
+struct qilai_pcie {
+	struct dw_pcie dw;
+	struct platform_device *pdev;
+	void __iomem *apb_base;
+};
+
+#define to_qilai_pcie(_dw) container_of(_dw, struct qilai_pcie, dw)
+
+static u64 qilai_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
+{
+	struct dw_pcie_rp *pp = &pci->pp;
+
+	return cpu_addr - pp->cfg0_base;
+}
+
+static u32 qilai_pcie_outbound_atu_check(struct dw_pcie *pci,
+					 const struct dw_pcie_ob_atu_cfg *atu,
+					 u64 *limit_addr)
+{
+	u64 parent_bus_addr = atu->parent_bus_addr;
+
+	*limit_addr = parent_bus_addr + atu->size - 1;
+
+	/*
+	 * Addresses below 4 GB are not 1:1 mapped; therefore, range checks
+	 * only need to ensure addresses below 4 GB match pci->region_limit.
+	 */
+	if (lower_32_bits(*limit_addr & ~pci->region_limit) !=
+	    lower_32_bits(parent_bus_addr & ~pci->region_limit) ||
+	    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
+	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size)
+		return -EINVAL;
+	else
+		return 0;
+}
+
+static bool qilai_pcie_link_up(struct dw_pcie *pci)
+{
+	struct qilai_pcie *qlpci = to_qilai_pcie(pci);
+	u32 val;
+
+	/* Read smlh & rdlh link up by checking debug port */
+	dw_pcie_read(qlpci->apb_base + PCIE_REGS_PCIE_SII_PM_STATE, 0x4, &val);
+
+	return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;
+}
+
+static int qilai_pcie_start_link(struct dw_pcie *pci)
+{
+	struct qilai_pcie *qlpci = to_qilai_pcie(pci);
+	u32 val;
+
+	/* Do phy link up */
+	dw_pcie_read(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, &val);
+	val |= PCIE_CFG_LTSSM_EN;
+	dw_pcie_write(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, val);
+
+	return 0;
+}
+
+static const struct dw_pcie_ops qilai_pcie_ops = {
+	.cpu_addr_fixup = qilai_pcie_cpu_addr_fixup,
+	.outbound_atu_check = qilai_pcie_outbound_atu_check,
+	.link_up = qilai_pcie_link_up,
+	.start_link = qilai_pcie_start_link,
+};
+
+static struct qilai_pcie *qilai_pcie_create_data(struct platform_device *pdev)
+{
+	struct qilai_pcie *qlpci;
+
+	qlpci = devm_kzalloc(&pdev->dev, sizeof(*qlpci), GFP_KERNEL);
+	if (!qlpci)
+		return ERR_PTR(-ENOMEM);
+
+	qlpci->pdev = pdev;
+	platform_set_drvdata(pdev, qlpci);
+
+	return qlpci;
+}
+
+/*
+ * Setup the Qilai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
+ * Write-Back, Read and Write Allocate mode.
+ */
+static void qilai_pcie_iocp_cache_setup(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	u32 val;
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	dw_pcie_read(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
+		     sizeof(val), &val);
+	val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_MODE, IOCP_ARCACHE);
+	val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_MODE, IOCP_AWCACHE);
+	val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_VALUE, IOCP_ARCACHE);
+	val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_VALUE, IOCP_AWCACHE);
+	dw_pcie_write(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
+		      sizeof(val), val);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+}
+
+static void qilai_pcie_enable_msi(struct qilai_pcie *qlpci)
+{
+	u32 val;
+
+	dw_pcie_read(qlpci->apb_base + PCIE_INTR_CONTROL1,
+		     sizeof(val), &val);
+	val |= PCIE_MSI_CTRL_INT_EN;
+	dw_pcie_write(qlpci->apb_base + PCIE_INTR_CONTROL1,
+		      sizeof(val), val);
+}
+
+static int qilai_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct qilai_pcie *qlpci = to_qilai_pcie(pci);
+
+	qilai_pcie_enable_msi(qlpci);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops qilai_pcie_host_ops = {
+	.init = qilai_pcie_host_init,
+};
+
+static int qilai_pcie_add_port(struct qilai_pcie *qlpci)
+{
+	struct device *dev = &qlpci->pdev->dev;
+	struct platform_device *pdev = qlpci->pdev;
+	int ret;
+
+	qlpci->dw.dev = dev;
+	qlpci->dw.ops = &qilai_pcie_ops;
+	qlpci->dw.pp.num_vectors = MAX_MSI_IRQS;
+	qlpci->dw.pp.ops = &qilai_pcie_host_ops;
+
+	dw_pcie_cap_set(&qlpci->dw, REQ_RES);
+
+	qlpci->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
+	if (IS_ERR(qlpci->apb_base))
+		return PTR_ERR(qlpci->apb_base);
+
+	ret = dw_pcie_host_init(&qlpci->dw.pp);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
+		return ret;
+	}
+
+	qilai_pcie_iocp_cache_setup(&qlpci->dw.pp);
+
+	return 0;
+}
+
+static int qilai_pcie_probe(struct platform_device *pdev)
+{
+	struct qilai_pcie *qlpci;
+
+	qlpci = qilai_pcie_create_data(pdev);
+	if (IS_ERR(qlpci))
+		return PTR_ERR(qlpci);
+
+	platform_set_drvdata(pdev, qlpci);
+
+	return qilai_pcie_add_port(qlpci);
+}
+
+static const struct of_device_id qilai_pcie_of_match[] = {
+	{ .compatible = "andestech,qilai-pcie" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, qilai_pcie_of_match);
+
+static struct platform_driver qilai_pcie_driver = {
+	.probe = qilai_pcie_probe,
+	.driver = {
+		.name	= "qilai-pcie",
+		.of_match_table = qilai_pcie_of_match,
+	},
+};
+
+module_platform_driver(qilai_pcie_driver);
+
+MODULE_AUTHOR("Randolph Lin <randolph@andestech.com>");
+MODULE_DESCRIPTION("Andes Qilai PCIe driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


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

* [PATCH 6/6] MAINTAINERS: Add maintainers for Andes QiLai PCIe driver
  2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
                   ` (4 preceding siblings ...)
  2025-08-20 11:18 ` [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support Randolph Lin
@ 2025-08-20 11:18 ` Randolph Lin
  5 siblings, 0 replies; 11+ messages in thread
From: Randolph Lin @ 2025-08-20 11:18 UTC (permalink / raw)
  To: linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609, Randolph Lin

Here add maintainer information for Andes QiLai PCIe driver.

Signed-off-by: Randolph Lin <randolph@andestech.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index daf520a13bdf..7caede190f73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19221,6 +19221,13 @@ S:	Supported
 F:	Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml
 F:	drivers/pci/controller/pcie-altera.c
 
+PCI DRIVER FOR ANDES QILAI PCIE
+M:	Randolph Lin <randolph@andestech.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/andestech,qilai-pcie.yaml
+F:	drivers/pci/controller/dwc/pcie-andes-qilai.c
+
 PCI DRIVER FOR APPLIEDMICRO XGENE
 M:	Toan Le <toan@os.amperecomputing.com>
 L:	linux-pci@vger.kernel.org
-- 
2.34.1


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

* Re: [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback
  2025-08-20 11:18 ` [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback Randolph Lin
@ 2025-08-20 15:52   ` Bjorn Helgaas
  0 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 15:52 UTC (permalink / raw)
  To: Randolph Lin
  Cc: linux-pci, ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer,
	aou, alex, jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609

On Wed, Aug 20, 2025 at 07:18:39PM +0800, Randolph Lin wrote:
> Introduce a callback for outbound ATU range checking to support
> range validations specific to cases that deviate from the generic
> check.
> 
> Signed-off-by: Randolph Lin <randolph@andestech.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 18 +++++++++++++-----
>  drivers/pci/controller/dwc/pcie-designware.h |  3 +++
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 89aad5a08928..f410aefaeb5e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -535,12 +535,20 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
>  	u32 retries, val;
>  	u64 limit_addr;
>  
> -	limit_addr = parent_bus_addr + atu->size - 1;
> +	if (pci->ops && pci->ops->outbound_atu_check) {
> +		val = pci->ops->outbound_atu_check(pci, atu, &limit_addr);

The return is not a "val" and not a "u32".  It should be named "ret"
or similar.  Should be "int" since the callback and
dw_pcie_prog_outbound_atu() return "int".  But see below for possible
signature change.

Also not 100% convinced this is needed, see other patch where this is
implemented.

> +		if (val)
> +			return val;
> +	} else {
> +		limit_addr = parent_bus_addr + atu->size - 1;
>  
> -	if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
> -	    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> -	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
> -		return -EINVAL;
> +		if ((limit_addr & ~pci->region_limit) !=
> +		    (parent_bus_addr & ~pci->region_limit) ||
> +		    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> +		    !IS_ALIGNED(atu->pci_addr, pci->region_align) ||
> +		    !atu->size) {
> +			return -EINVAL;
> +		}
>  	}
>  
>  	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 00f52d472dcd..40dd2c83b1c7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -469,6 +469,9 @@ struct dw_pcie_ep {
>  
>  struct dw_pcie_ops {
>  	u64	(*cpu_addr_fixup)(struct dw_pcie *pcie, u64 cpu_addr);
> +	u32	(*outbound_atu_check)(struct dw_pcie *pcie,
> +				      const struct dw_pcie_ob_atu_cfg *atu,
> +				      u64 *limit_addr);

I have kind of an allergic reaction to things named "check" because
the name doesn't suggest anything about what the function does or what
it will return.  For bool functions, I prefer a name that's a
predicate that can be either true or false, e.g., "valid".

This isn't a bool, but possibly *could* be, e.g.,
"outbound_atu_addr_valid()".  Then the caller would be something like:

  if (!pci->ops->outbound_atu_addr_valid(...))
    return -EINVAL;

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

* Re: [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support
  2025-08-20 11:18 ` [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support Randolph Lin
@ 2025-08-20 15:54   ` Bjorn Helgaas
  2025-08-29  9:41     ` Randolph Lin
  0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2025-08-20 15:54 UTC (permalink / raw)
  To: Randolph Lin
  Cc: linux-pci, ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer,
	aou, alex, jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609

On Wed, Aug 20, 2025 at 07:18:42PM +0800, Randolph Lin wrote:
> Add driver support for DesignWare based PCIe controller in Andes
> QiLai SoC. The driver only supports the Root Complex mode.
> 
> Signed-off-by: Randolph Lin <randolph@andestech.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |  13 +
>  drivers/pci/controller/dwc/Makefile           |   1 +
>  drivers/pci/controller/dwc/pcie-andes-qilai.c | 227 ++++++++++++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-andes-qilai.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index ff6b6d9e18ec..a9c5a43f648b 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -49,6 +49,19 @@ config PCIE_AMD_MDB
>  	  DesignWare IP and therefore the driver re-uses the DesignWare
>  	  core functions to implement the driver.
>  
> +config PCIE_ANDES_QILAI
> +	bool "ANDES QiLai PCIe controller"
> +	depends on OF && (RISCV || COMPILE_TEST)
> +	depends on PCI_MSI
> +	depends on ARCH_ANDES
> +	select PCIE_DW_HOST
> +	default y if ARCH_ANDES
> +	help
> +	  Say Y here if you want to enable PCIe controller support on Andes
> +	  QiLai SoCs. The Andes QiLai SoCs PCIe controller is based on
> +	  DesignWare IP and therefore the driver re-uses the DesignWare
> +	  core functions to implement the driver.
> +
>  config PCI_MESON
>  	tristate "Amlogic Meson PCIe controller"
>  	default m if ARCH_MESON
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 6919d27798d1..de9583cbd675 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> +obj-$(CONFIG_PCIE_ANDES_QILAI) += pcie-andes-qilai.o
>  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> diff --git a/drivers/pci/controller/dwc/pcie-andes-qilai.c b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> new file mode 100644
> index 000000000000..dd06eee82cac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the PCIe Controller in QiLai from Andes
> + *
> + * Copyright (C) 2025 Andes Technology Corporation
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_INTR_CONTROL1			0x15c
> +#define PCIE_MSI_CTRL_INT_EN			BIT(28)
> +
> +#define PCIE_LOGIC_COHERENCY_CONTROL3		0x8e8
> +/* Write-Back, Read and Write Allocate */
> +#define IOCP_ARCACHE				0xf
> +/* Write-Back, Read and Write Allocate */
> +#define IOCP_AWCACHE				0xf

Are IOCP_ARCACHE and IOCP_AWCACHE supposed to be identical values with
identical comments?

> +#define PCIE_CFG_MSTR_ARCACHE_MODE		GENMASK(6, 3)
> +#define PCIE_CFG_MSTR_AWCACHE_MODE		GENMASK(14, 11)
> +#define PCIE_CFG_MSTR_ARCACHE_VALUE		GENMASK(22, 19)
> +#define PCIE_CFG_MSTR_AWCACHE_VALUE		GENMASK(30, 27)
> +
> +#define PCIE_GEN_CONTROL2			0x54
> +#define PCIE_CFG_LTSSM_EN			BIT(0)
> +
> +#define PCIE_REGS_PCIE_SII_PM_STATE		0xc0
> +#define SMLH_LINK_UP				BIT(6)
> +#define RDLH_LINK_UP				BIT(7)
> +#define PCIE_REGS_PCIE_SII_LINK_UP		(SMLH_LINK_UP | RDLH_LINK_UP)
> +
> +struct qilai_pcie {
> +	struct dw_pcie dw;
> +	struct platform_device *pdev;
> +	void __iomem *apb_base;
> +};
> +
> +#define to_qilai_pcie(_dw) container_of(_dw, struct qilai_pcie, dw)
> +
> +static u64 qilai_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
> +{
> +	struct dw_pcie_rp *pp = &pci->pp;
> +
> +	return cpu_addr - pp->cfg0_base;

Sorry, we can't do this.  We're removing .cpu_addr_fixup() because
it's a workaround for defects in the DT description.  See these
commits, for example:

    befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
    b9812179f601 ("PCI: imx6: Remove imx_pcie_cpu_addr_fixup()")
    07ae413e169d ("PCI: intel-gw: Remove intel_pcie_cpu_addr()")

> +}
> +
> +static u32 qilai_pcie_outbound_atu_check(struct dw_pcie *pci,
> +					 const struct dw_pcie_ob_atu_cfg *atu,
> +					 u64 *limit_addr)
> +{
> +	u64 parent_bus_addr = atu->parent_bus_addr;
> +
> +	*limit_addr = parent_bus_addr + atu->size - 1;
> +
> +	/*
> +	 * Addresses below 4 GB are not 1:1 mapped; therefore, range checks
> +	 * only need to ensure addresses below 4 GB match pci->region_limit.
> +	 */
> +	if (lower_32_bits(*limit_addr & ~pci->region_limit) !=
> +	    lower_32_bits(parent_bus_addr & ~pci->region_limit) ||
> +	    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> +	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size)
> +		return -EINVAL;
> +	else
> +		return 0;

I'm a little dubious about this because the knowledge about 1:1
mapping feels like something we should know from devicetree.  But I'm
not an expert in this area.

If you do need this, write it like this, which I think is slightly
simpler than the if/else:

  if (...)
    return -EINVAL;

  return 0;

> +}
> +
> +static bool qilai_pcie_link_up(struct dw_pcie *pci)
> +{
> +	struct qilai_pcie *qlpci = to_qilai_pcie(pci);

Suggest using "pcie" as the name for "struct qilai_pcie *" pointers
because that's a common pattern in other drivers.

> +	u32 val;
> +
> +	/* Read smlh & rdlh link up by checking debug port */
> +	dw_pcie_read(qlpci->apb_base + PCIE_REGS_PCIE_SII_PM_STATE, 0x4, &val);
> +
> +	return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;
> +}
> +
> +static int qilai_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> +	u32 val;
> +
> +	/* Do phy link up */
> +	dw_pcie_read(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, &val);
> +	val |= PCIE_CFG_LTSSM_EN;
> +	dw_pcie_write(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, val);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_ops qilai_pcie_ops = {
> +	.cpu_addr_fixup = qilai_pcie_cpu_addr_fixup,
> +	.outbound_atu_check = qilai_pcie_outbound_atu_check,
> +	.link_up = qilai_pcie_link_up,
> +	.start_link = qilai_pcie_start_link,
> +};
> +
> +static struct qilai_pcie *qilai_pcie_create_data(struct platform_device *pdev)
> +{
> +	struct qilai_pcie *qlpci;
> +
> +	qlpci = devm_kzalloc(&pdev->dev, sizeof(*qlpci), GFP_KERNEL);
> +	if (!qlpci)
> +		return ERR_PTR(-ENOMEM);
> +
> +	qlpci->pdev = pdev;
> +	platform_set_drvdata(pdev, qlpci);
> +
> +	return qlpci;

Doesn't seem worth a separate function to me.  I see this is copied
from bt1_pcie_create_data(), and kudos for paying attention to how
other drivers do things.  But I don't think it's really worth it
there, either, and it's the only driver that does that.

Most drivers do it directly in *_pcie_probe(), which also makes it
possible to delay the platform_set_drvdata() until after qlpci is
initialized (and also makes it obvious that you do the
platform_set_drvdata() *twice* :)).

> +}
> +
> +/*
> + * Setup the Qilai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
> + * Write-Back, Read and Write Allocate mode.
> + */
> +static void qilai_pcie_iocp_cache_setup(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	u32 val;
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +
> +	dw_pcie_read(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> +		     sizeof(val), &val);
> +	val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_MODE, IOCP_ARCACHE);
> +	val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_MODE, IOCP_AWCACHE);
> +	val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_VALUE, IOCP_ARCACHE);
> +	val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_VALUE, IOCP_AWCACHE);

Since you never clear anything in "val", it looks like this assumes
the value you read from PCIE_LOGIC_COHERENCY_CONTROL3 contains zeroes
where you insert the new values.  Possibly use FIELD_MODIFY() instead?

> +	dw_pcie_write(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> +		      sizeof(val), val);
> +
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
> +static void qilai_pcie_enable_msi(struct qilai_pcie *qlpci)
> +{
> +	u32 val;
> +
> +	dw_pcie_read(qlpci->apb_base + PCIE_INTR_CONTROL1,
> +		     sizeof(val), &val);
> +	val |= PCIE_MSI_CTRL_INT_EN;
> +	dw_pcie_write(qlpci->apb_base + PCIE_INTR_CONTROL1,
> +		      sizeof(val), val);

Apparently you don't need dw_pcie_dbi_ro_wr_en() and
dw_pcie_dbi_ro_wr_dis() around this?  Or maybe this relies on the DWC
core taking care of this, I dunno.

> +}
> +
> +static int qilai_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> +
> +	qilai_pcie_enable_msi(qlpci);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops qilai_pcie_host_ops = {
> +	.init = qilai_pcie_host_init,
> +};
> +
> +static int qilai_pcie_add_port(struct qilai_pcie *qlpci)
> +{
> +	struct device *dev = &qlpci->pdev->dev;
> +	struct platform_device *pdev = qlpci->pdev;
> +	int ret;
> +
> +	qlpci->dw.dev = dev;
> +	qlpci->dw.ops = &qilai_pcie_ops;
> +	qlpci->dw.pp.num_vectors = MAX_MSI_IRQS;
> +	qlpci->dw.pp.ops = &qilai_pcie_host_ops;
> +
> +	dw_pcie_cap_set(&qlpci->dw, REQ_RES);
> +
> +	qlpci->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
> +	if (IS_ERR(qlpci->apb_base))
> +		return PTR_ERR(qlpci->apb_base);
> +
> +	ret = dw_pcie_host_init(&qlpci->dw.pp);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
> +		return ret;
> +	}
> +
> +	qilai_pcie_iocp_cache_setup(&qlpci->dw.pp);
> +
> +	return 0;
> +}
> +
> +static int qilai_pcie_probe(struct platform_device *pdev)
> +{
> +	struct qilai_pcie *qlpci;
> +
> +	qlpci = qilai_pcie_create_data(pdev);
> +	if (IS_ERR(qlpci))
> +		return PTR_ERR(qlpci);
> +
> +	platform_set_drvdata(pdev, qlpci);
> +
> +	return qilai_pcie_add_port(qlpci);
> +}
> +
> +static const struct of_device_id qilai_pcie_of_match[] = {
> +	{ .compatible = "andestech,qilai-pcie" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, qilai_pcie_of_match);
> +
> +static struct platform_driver qilai_pcie_driver = {
> +	.probe = qilai_pcie_probe,
> +	.driver = {
> +		.name	= "qilai-pcie",
> +		.of_match_table = qilai_pcie_of_match,
> +	},
> +};
> +
> +module_platform_driver(qilai_pcie_driver);
> +
> +MODULE_AUTHOR("Randolph Lin <randolph@andestech.com>");
> +MODULE_DESCRIPTION("Andes Qilai PCIe driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.34.1
> 

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

* Re: [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support
  2025-08-20 11:18 ` [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support Randolph Lin
@ 2025-08-21  6:11   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-21  6:11 UTC (permalink / raw)
  To: Randolph Lin, linux-pci
  Cc: ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer, aou, alex,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609

On 20/08/2025 13:18, Randolph Lin wrote:
> Add the Andes QiLai PCIe node, which includes 3 Root Complexes.
> 
> Signed-off-by: Randolph Lin <randolph@andestech.com>

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>

...

> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pci@80000000 {
> +        compatible = "andestech,qilai-pcie";
> +        device_type = "pci";
> +        reg = <0x00 0x80000000 0x00 0x20000000>,
> +              <0x20 0x00000000 0x00 0x00010000>,
> +              <0x00 0x04000000 0x00 0x00001000>;
> +        reg-names = "dbi", "config", "apb";
> +        bus-range = <0x0 0xff>;
> +        num-viewport = <4>;
> +
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        ranges = <0x02000000 0x00 0x10000000 0x20 0x10000000 0x0 0xF0000000>,
> +                 <0x43000000 0x01 0x00000000 0x21 0x0000000 0x1F 0x00000000>;
> +
> +        #interrupt-cells = <1>;
> +        interrupts = <0xF>;
> +        interrupt-names = "msi";
> +        interrupt-parent = <&plic0>;
> +        interrupt-controller;
> +        interrupt-map-mask = <0 0 0 7>;
> +        interrupt-map = <0 0 0 1 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 2 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 3 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 4 &plic0 0xF IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +      pci@a0000000 {
> +        compatible = "andestech,qilai-pcie";
> +        device_type = "pci";
> +        reg = <0x00 0xA0000000 0x00 0x20000000>,
> +              <0x10 0x00000000 0x00 0x00010000>,
> +              <0x00 0x04001000 0x00 0x00001000>;
> +        reg-names = "dbi", "config", "apb";
> +        bus-range = <0x0 0xff>;
> +        num-viewport = <4>;
> +
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        ranges = <0x02000000 0x00 0x10000000 0x10 0x10000000 0x0 0xF0000000>,
> +                 <0x43000000 0x01 0x00000000 0x11 0x00000000 0x7 0x00000000>;
> +
> +        #interrupt-cells = <1>;
> +        interrupts = <0xE>;
> +        interrupt-names = "msi";
> +        interrupt-parent = <&plic0>;
> +        interrupt-controller;
> +        interrupt-map-mask = <0 0 0 7>;
> +        interrupt-map = <0 0 0 1 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 2 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 3 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>,
> +                        <0 0 0 4 &plic0 0xE IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +      pci@c0000000 {

One example is enough.

> +        compatible = "andestech,qilai-pcie";
> +        device_type = "pci";
> +        reg = <0x00 0xC0000000 0x00 0x20000000>,
> +              <0x18 0x00000000 0x00 0x00010000>,
> +              <0x00 0x04002000 0x00 0x00001000>;
> +        reg-names = "dbi", "config", "apb";
> +        bus-range = <0x0 0xff>;
> +        num-viewport = <4>;
Best regards,
Krzysztof

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

* Re: [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support
  2025-08-20 15:54   ` Bjorn Helgaas
@ 2025-08-29  9:41     ` Randolph Lin
  0 siblings, 0 replies; 11+ messages in thread
From: Randolph Lin @ 2025-08-29  9:41 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, ben717, robh, krzk+dt, conor+dt, paul.walmsley, palmer,
	aou, alex, jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas,
	randolph.sklin, tim609

Hi Bjorn Helgaas,

Thank you for the feedback. I will update the changes accordingly.

Rn Wed, Aug 20, 2025 at 10:54:44AM -0500, Bjorn Helgaas wrote:
> [EXTERNAL MAIL]
> 
> On Wed, Aug 20, 2025 at 07:18:42PM +0800, Randolph Lin wrote:
> > Add driver support for DesignWare based PCIe controller in Andes
> > QiLai SoC. The driver only supports the Root Complex mode.
> >
> > Signed-off-by: Randolph Lin <randolph@andestech.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig            |  13 +
> >  drivers/pci/controller/dwc/Makefile           |   1 +
> >  drivers/pci/controller/dwc/pcie-andes-qilai.c | 227 ++++++++++++++++++
> >  3 files changed, 241 insertions(+)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-andes-qilai.c
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index ff6b6d9e18ec..a9c5a43f648b 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -49,6 +49,19 @@ config PCIE_AMD_MDB
> >         DesignWare IP and therefore the driver re-uses the DesignWare
> >         core functions to implement the driver.
> >
> > +config PCIE_ANDES_QILAI
> > +     bool "ANDES QiLai PCIe controller"
> > +     depends on OF && (RISCV || COMPILE_TEST)
> > +     depends on PCI_MSI
> > +     depends on ARCH_ANDES
> > +     select PCIE_DW_HOST
> > +     default y if ARCH_ANDES
> > +     help
> > +       Say Y here if you want to enable PCIe controller support on Andes
> > +       QiLai SoCs. The Andes QiLai SoCs PCIe controller is based on
> > +       DesignWare IP and therefore the driver re-uses the DesignWare
> > +       core functions to implement the driver.
> > +
> >  config PCI_MESON
> >       tristate "Amlogic Meson PCIe controller"
> >       default m if ARCH_MESON
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index 6919d27798d1..de9583cbd675 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> >  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> >  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> > +obj-$(CONFIG_PCIE_ANDES_QILAI) += pcie-andes-qilai.o
> >  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> >  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> >  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > diff --git a/drivers/pci/controller/dwc/pcie-andes-qilai.c b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> > new file mode 100644
> > index 000000000000..dd06eee82cac
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-andes-qilai.c
> > @@ -0,0 +1,227 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for the PCIe Controller in QiLai from Andes
> > + *
> > + * Copyright (C) 2025 Andes Technology Corporation
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/types.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define PCIE_INTR_CONTROL1                   0x15c
> > +#define PCIE_MSI_CTRL_INT_EN                 BIT(28)
> > +
> > +#define PCIE_LOGIC_COHERENCY_CONTROL3                0x8e8
> > +/* Write-Back, Read and Write Allocate */
> > +#define IOCP_ARCACHE                         0xf
> > +/* Write-Back, Read and Write Allocate */
> > +#define IOCP_AWCACHE                         0xf
> 
> Are IOCP_ARCACHE and IOCP_AWCACHE supposed to be identical values with
> identical comments?
> 
> > +#define PCIE_CFG_MSTR_ARCACHE_MODE           GENMASK(6, 3)
> > +#define PCIE_CFG_MSTR_AWCACHE_MODE           GENMASK(14, 11)
> > +#define PCIE_CFG_MSTR_ARCACHE_VALUE          GENMASK(22, 19)
> > +#define PCIE_CFG_MSTR_AWCACHE_VALUE          GENMASK(30, 27)
> > +
> > +#define PCIE_GEN_CONTROL2                    0x54
> > +#define PCIE_CFG_LTSSM_EN                    BIT(0)
> > +
> > +#define PCIE_REGS_PCIE_SII_PM_STATE          0xc0
> > +#define SMLH_LINK_UP                         BIT(6)
> > +#define RDLH_LINK_UP                         BIT(7)
> > +#define PCIE_REGS_PCIE_SII_LINK_UP           (SMLH_LINK_UP | RDLH_LINK_UP)
> > +
> > +struct qilai_pcie {
> > +     struct dw_pcie dw;
> > +     struct platform_device *pdev;
> > +     void __iomem *apb_base;
> > +};
> > +
> > +#define to_qilai_pcie(_dw) container_of(_dw, struct qilai_pcie, dw)
> > +
> > +static u64 qilai_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 cpu_addr)
> > +{
> > +     struct dw_pcie_rp *pp = &pci->pp;
> > +
> > +     return cpu_addr - pp->cfg0_base;
> 
> Sorry, we can't do this.  We're removing .cpu_addr_fixup() because
> it's a workaround for defects in the DT description.  See these
> commits, for example:
> 
>     befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
>     b9812179f601 ("PCI: imx6: Remove imx_pcie_cpu_addr_fixup()")
>     07ae413e169d ("PCI: intel-gw: Remove intel_pcie_cpu_addr()")
> 

I’m a bit confused about the following question:
After removing cpu_addr_fixup, should we use pci->parent_bus_offset to store the
offset value, or should pci->parent_bus_offset remain 0?

In the commit message:
befc86a0b354 ("PCI: dwc: Use parent_bus_offset to remove need for .cpu_addr_fixup()")
    We know the parent_bus_offset, either computed from a DT reg property (the
    offset is the CPU physical addr - the 'config'/'addr_space' address on the
    parent bus) or from a .cpu_addr_fixup() (which may have used a host bridge
    window offset).

We know that "the offset is the CPU physical addr - the 'config'/'addr_space'
address on the parent bus".

However, in dw_pcie_host_get_resources(), it passes pp->cfg0_base, which is
parsed from the device tree using "config", as the cpu_phys_addr parameter to
dw_pcie_parent_bus_offset(). It also passes "config" as the 2nd parameter to
dw_pcie_parent_bus_offset().

In dw_pcie_parent_bus_offset(), the 2nd parameter is used to get the index
from the devicetree "reg-names" field, and the result is used as the
'config'/'addr_space' address. 

It seems that the same value is being obtained through a different method,
and the return value appears to be 0.
Could I be misunderstanding something?

> > +}
> > +
> > +static u32 qilai_pcie_outbound_atu_check(struct dw_pcie *pci,
> > +                                      const struct dw_pcie_ob_atu_cfg *atu,
> > +                                      u64 *limit_addr)
> > +{
> > +     u64 parent_bus_addr = atu->parent_bus_addr;
> > +
> > +     *limit_addr = parent_bus_addr + atu->size - 1;
> > +
> > +     /*
> > +      * Addresses below 4 GB are not 1:1 mapped; therefore, range checks
> > +      * only need to ensure addresses below 4 GB match pci->region_limit.
> > +      */
> > +     if (lower_32_bits(*limit_addr & ~pci->region_limit) !=
> > +         lower_32_bits(parent_bus_addr & ~pci->region_limit) ||
> > +         !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
> > +         !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size)
> > +             return -EINVAL;
> > +     else
> > +             return 0;
> 
> I'm a little dubious about this because the knowledge about 1:1
> mapping feels like something we should know from devicetree.  But I'm
> not an expert in this area.
> 
> If you do need this, write it like this, which I think is slightly
> simpler than the if/else:
> 
>   if (...)
>     return -EINVAL;
> 
>   return 0;
> 
> > +}
> > +
> > +static bool qilai_pcie_link_up(struct dw_pcie *pci)
> > +{
> > +     struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> 
> Suggest using "pcie" as the name for "struct qilai_pcie *" pointers
> because that's a common pattern in other drivers.
> 
> > +     u32 val;
> > +
> > +     /* Read smlh & rdlh link up by checking debug port */
> > +     dw_pcie_read(qlpci->apb_base + PCIE_REGS_PCIE_SII_PM_STATE, 0x4, &val);
> > +
> > +     return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;
> > +}
> > +
> > +static int qilai_pcie_start_link(struct dw_pcie *pci)
> > +{
> > +     struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> > +     u32 val;
> > +
> > +     /* Do phy link up */
> > +     dw_pcie_read(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, &val);
> > +     val |= PCIE_CFG_LTSSM_EN;
> > +     dw_pcie_write(qlpci->apb_base + PCIE_GEN_CONTROL2, 0x4, val);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops qilai_pcie_ops = {
> > +     .cpu_addr_fixup = qilai_pcie_cpu_addr_fixup,
> > +     .outbound_atu_check = qilai_pcie_outbound_atu_check,
> > +     .link_up = qilai_pcie_link_up,
> > +     .start_link = qilai_pcie_start_link,
> > +};
> > +
> > +static struct qilai_pcie *qilai_pcie_create_data(struct platform_device *pdev)
> > +{
> > +     struct qilai_pcie *qlpci;
> > +
> > +     qlpci = devm_kzalloc(&pdev->dev, sizeof(*qlpci), GFP_KERNEL);
> > +     if (!qlpci)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     qlpci->pdev = pdev;
> > +     platform_set_drvdata(pdev, qlpci);
> > +
> > +     return qlpci;
> 
> Doesn't seem worth a separate function to me.  I see this is copied
> from bt1_pcie_create_data(), and kudos for paying attention to how
> other drivers do things.  But I don't think it's really worth it
> there, either, and it's the only driver that does that.
> 
> Most drivers do it directly in *_pcie_probe(), which also makes it
> possible to delay the platform_set_drvdata() until after qlpci is
> initialized (and also makes it obvious that you do the
> platform_set_drvdata() *twice* :)).
> 
> > +}
> > +
> > +/*
> > + * Setup the Qilai PCIe IOCP (IO Coherence Port) Read/Write Behaviors to the
> > + * Write-Back, Read and Write Allocate mode.
> > + */
> > +static void qilai_pcie_iocp_cache_setup(struct dw_pcie_rp *pp)
> > +{
> > +     struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +     u32 val;
> > +
> > +     dw_pcie_dbi_ro_wr_en(pci);
> > +
> > +     dw_pcie_read(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> > +                  sizeof(val), &val);
> > +     val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_MODE, IOCP_ARCACHE);
> > +     val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_MODE, IOCP_AWCACHE);
> > +     val |= FIELD_PREP(PCIE_CFG_MSTR_ARCACHE_VALUE, IOCP_ARCACHE);
> > +     val |= FIELD_PREP(PCIE_CFG_MSTR_AWCACHE_VALUE, IOCP_AWCACHE);
> 
> Since you never clear anything in "val", it looks like this assumes
> the value you read from PCIE_LOGIC_COHERENCY_CONTROL3 contains zeroes
> where you insert the new values.  Possibly use FIELD_MODIFY() instead?
> 
> > +     dw_pcie_write(pci->dbi_base + PCIE_LOGIC_COHERENCY_CONTROL3,
> > +                   sizeof(val), val);
> > +
> > +     dw_pcie_dbi_ro_wr_dis(pci);
> > +}
> > +
> > +static void qilai_pcie_enable_msi(struct qilai_pcie *qlpci)
> > +{
> > +     u32 val;
> > +
> > +     dw_pcie_read(qlpci->apb_base + PCIE_INTR_CONTROL1,
> > +                  sizeof(val), &val);
> > +     val |= PCIE_MSI_CTRL_INT_EN;
> > +     dw_pcie_write(qlpci->apb_base + PCIE_INTR_CONTROL1,
> > +                   sizeof(val), val);
> 
> Apparently you don't need dw_pcie_dbi_ro_wr_en() and
> dw_pcie_dbi_ro_wr_dis() around this?  Or maybe this relies on the DWC
> core taking care of this, I dunno.
> 

Maybe these are not the DBI registers. 
I think the API is wrong, that's why you confused.
I will use readl() and writel() to instead of it. 

> > +}
> > +
> > +static int qilai_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > +     struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +     struct qilai_pcie *qlpci = to_qilai_pcie(pci);
> > +
> > +     qilai_pcie_enable_msi(qlpci);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops qilai_pcie_host_ops = {
> > +     .init = qilai_pcie_host_init,
> > +};
> > +
> > +static int qilai_pcie_add_port(struct qilai_pcie *qlpci)
> > +{
> > +     struct device *dev = &qlpci->pdev->dev;
> > +     struct platform_device *pdev = qlpci->pdev;
> > +     int ret;
> > +
> > +     qlpci->dw.dev = dev;
> > +     qlpci->dw.ops = &qilai_pcie_ops;
> > +     qlpci->dw.pp.num_vectors = MAX_MSI_IRQS;
> > +     qlpci->dw.pp.ops = &qilai_pcie_host_ops;
> > +
> > +     dw_pcie_cap_set(&qlpci->dw, REQ_RES);
> > +
> > +     qlpci->apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
> > +     if (IS_ERR(qlpci->apb_base))
> > +             return PTR_ERR(qlpci->apb_base);
> > +
> > +     ret = dw_pcie_host_init(&qlpci->dw.pp);
> > +     if (ret) {
> > +             dev_err_probe(dev, ret, "Failed to initialize PCIe host\n");
> > +             return ret;
> > +     }
> > +
> > +     qilai_pcie_iocp_cache_setup(&qlpci->dw.pp);
> > +
> > +     return 0;
> > +}
> > +
> > +static int qilai_pcie_probe(struct platform_device *pdev)
> > +{
> > +     struct qilai_pcie *qlpci;
> > +
> > +     qlpci = qilai_pcie_create_data(pdev);
> > +     if (IS_ERR(qlpci))
> > +             return PTR_ERR(qlpci);
> > +
> > +     platform_set_drvdata(pdev, qlpci);
> > +
> > +     return qilai_pcie_add_port(qlpci);
> > +}
> > +
> > +static const struct of_device_id qilai_pcie_of_match[] = {
> > +     { .compatible = "andestech,qilai-pcie" },
> > +     {},
> > +};
> > +MODULE_DEVICE_TABLE(of, qilai_pcie_of_match);
> > +
> > +static struct platform_driver qilai_pcie_driver = {
> > +     .probe = qilai_pcie_probe,
> > +     .driver = {
> > +             .name   = "qilai-pcie",
> > +             .of_match_table = qilai_pcie_of_match,
> > +     },
> > +};
> > +
> > +module_platform_driver(qilai_pcie_driver);
> > +
> > +MODULE_AUTHOR("Randolph Lin <randolph@andestech.com>");
> > +MODULE_DESCRIPTION("Andes Qilai PCIe driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.34.1
> >

Sincerely,
Randolph Lin

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

end of thread, other threads:[~2025-08-29  9:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 11:18 [PATCH 0/6] Add support for Andes Qilai SoC PCIe controller Randolph Lin
2025-08-20 11:18 ` [PATCH 1/6] riscv: dts: andes: Define dma-ranges for coherent port Randolph Lin
2025-08-20 11:18 ` [PATCH 2/6] PCI: dwc: Add outbound ATU range check callback Randolph Lin
2025-08-20 15:52   ` Bjorn Helgaas
2025-08-20 11:18 ` [PATCH 3/6] dt-bindings: Add Andes QiLai PCIe support Randolph Lin
2025-08-21  6:11   ` Krzysztof Kozlowski
2025-08-20 11:18 ` [PATCH 4/6] riscv: dts: andes: Add PCIe node into the QiLai SoC Randolph Lin
2025-08-20 11:18 ` [PATCH 5/6] PCI: andes: Add Andes QiLai SoC PCIe host driver support Randolph Lin
2025-08-20 15:54   ` Bjorn Helgaas
2025-08-29  9:41     ` Randolph Lin
2025-08-20 11:18 ` [PATCH 6/6] MAINTAINERS: Add maintainers for Andes QiLai PCIe driver Randolph Lin

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