[PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support

[PATCH v9 0/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe support

[Yoshihiro Shimoda]

Add R-Car S4-8 (R-Car Gen4) PCIe Host and Endpoint support.
To support them, modify PCIe DesignWare common codes.

Changes from v8:
https://lore.kernel.org/all/20230131095543.1831875-1-yoshihiro.shimoda.uh@renesas.com/
 - Based on next-20230210.
 - Missing dt-bindings patches accidentally.

Changes from v7:
https://lore.kernel.org/all/20221121124400.1282768-1-yoshihiro.shimoda.uh@renesas.com/
 - Based on next-20230131.
 - Update Copyright year of new files.
 - Add a new capability flag (DW_PCIE_CAP_EDMA_UNROLL) for finding eDMA on
   R-Car S4-8.
 - Remove some PCIe configurations like L1 substates from pcie-rcar-gen4-host.c.
 - Change timing of reset_control for suitable this hardware initialization.
 - Add gpio reset handling for host mode.
 - Capitalize the first charactors on each printk message.

Yoshihiro Shimoda (8):
  dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host
  dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
  PCI: Add PCI_EXP_LNKCAP_MLW macros
  PCI: designware-ep: Expose dw_pcie_ep_exit() to module
  PCI: dwc: Add support for triggering legacy IRQs
  PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
  PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support
  MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4

 .../bindings/pci/rcar-gen4-pci-ep.yaml        |  90 +++++++++
 .../bindings/pci/rcar-gen4-pci-host.yaml      |  90 +++++++++
 MAINTAINERS                                   |   1 +
 drivers/pci/controller/dwc/Kconfig            |  18 ++
 drivers/pci/controller/dwc/Makefile           |   4 +
 .../pci/controller/dwc/pcie-designware-ep.c   |  70 ++++++-
 .../pci/controller/dwc/pcie-designware-host.c |   3 +
 drivers/pci/controller/dwc/pcie-designware.c  |  33 +++-
 drivers/pci/controller/dwc/pcie-designware.h  |  19 +-
 .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 185 ++++++++++++++++++
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 ++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 ++++++
 include/uapi/linux/pci_regs.h                 |   6 +
 14 files changed, 894 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h

-- 
2.25.1

[PATCH v9 1/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host

[Yoshihiro Shimoda]

Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe host module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/rcar-gen4-pci-host.yaml      | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml

diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
new file mode 100644
index 000000000000..1bd7612101d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Host
+
+maintainers:
+  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+  - $ref: snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r8a779f0-pcie   # R-Car S4-8
+      - const: renesas,rcar-gen4-pcie  # R-Car Gen4
+
+  interrupts:
+    maxItems: 4
+
+  interrupt-names:
+    items:
+      - const: msi
+      - const: dma
+      - const: sft_ce
+      - const: app
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - power-domains
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a779f0-sysc.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie: pcie@e65d0000 {
+            compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
+            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
+                  <0 0xe65d6200 0 0x0e00>, <0 0xfe000000 0 0x400000>;
+            reg-names = "dbi", "atu", "app", "config";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            bus-range = <0x00 0xff>;
+            device_type = "pci";
+            ranges =  <0x81000000 0 0x00000000 0 0xfe000000 0 0x00010000
+                       0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
+            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
+            interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "msi", "dma", "sft_ce", "app";
+            #interrupt-cells = <1>;
+            interrupt-map-mask = <0 0 0 7>;
+            interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD 624>;
+            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+            resets = <&cpg 624>;
+            num-lanes = <2>;
+            snps,enable-cdm-check;
+            max-link-speed = <2>;
+        };
+    };
-- 
2.25.1

[PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint

[Yoshihiro Shimoda]

Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe endpoint module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/rcar-gen4-pci-ep.yaml        | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
new file mode 100644
index 000000000000..4b10d67e4336
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Endpoint
+
+maintainers:
+  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+  - $ref: snps,dw-pcie-ep.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r8a779f0-pcie-ep   # R-Car S4-8
+      - const: renesas,rcar-gen4-pcie-ep  # R-Car Gen4
+
+  reg:
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: atu
+      - const: appl
+      - const: addr_space
+
+  interrupts:
+    maxItems: 3
+
+  interrupt-names:
+    items:
+      - const: dma
+      - const: sft_ce
+      - const: app
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  max-link-speed: true
+
+  num-lanes: true
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - resets
+  - power-domains
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a779f0-sysc.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0_ep: pcie-ep@e65d0000 {
+            compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
+            reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
+                  <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
+            reg-names = "dbi", "atu", "appl", "addr_space";
+            interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dma", "sft_ce", "app";
+            clocks = <&cpg CPG_MOD 624>;
+            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+            resets = <&cpg 624>;
+            num-lanes = <2>;
+            max-link-speed = <2>;
+        };
+    };
-- 
2.25.1

[PATCH v9 3/8] PCI: Add PCI_EXP_LNKCAP_MLW macros

[Yoshihiro Shimoda]

Add macros defining Maximum Link Width bits in Link Capabilities
Register.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/uapi/linux/pci_regs.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 85ab1278811e..c6fb2420cb17 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -538,6 +538,12 @@
 #define  PCI_EXP_LNKCAP_SLS_16_0GB 0x00000004 /* LNKCAP2 SLS Vector bit 3 */
 #define  PCI_EXP_LNKCAP_SLS_32_0GB 0x00000005 /* LNKCAP2 SLS Vector bit 4 */
 #define  PCI_EXP_LNKCAP_SLS_64_0GB 0x00000006 /* LNKCAP2 SLS Vector bit 5 */
+#define  PCI_EXP_LNKCAP_MLW_X1	0x00000010 /* Maximum Link Width x1 */
+#define  PCI_EXP_LNKCAP_MLW_X2	0x00000020 /* Maximum Link Width x2 */
+#define  PCI_EXP_LNKCAP_MLW_X4	0x00000040 /* Maximum Link Width x4 */
+#define  PCI_EXP_LNKCAP_MLW_X8	0x00000080 /* Maximum Link Width x8 */
+#define  PCI_EXP_LNKCAP_MLW_X12	0x000000c0 /* Maximum Link Width x12 */
+#define  PCI_EXP_LNKCAP_MLW_X16	0x00000100 /* Maximum Link Width x16 */
 #define  PCI_EXP_LNKCAP_MLW	0x000003f0 /* Maximum Link Width */
 #define  PCI_EXP_LNKCAP_ASPMS	0x00000c00 /* ASPM Support */
 #define  PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* ASPM L0s Support */
-- 
2.25.1

[PATCH v9 4/8] PCI: designware-ep: Expose dw_pcie_ep_exit() to module

[Yoshihiro Shimoda]

Expose dw_pcie_ep_exit() to module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8d552f..95efe14f1036 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -622,6 +622,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 
 	pci_epc_mem_exit(epc);
 }
+EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
 
 static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
 {
-- 
2.25.1

[PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs

[Yoshihiro Shimoda]

Add support for triggering legacy IRQs by using outbound ATU.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 66 +++++++++++++++++--
 drivers/pci/controller/dwc/pcie-designware.c  | 25 ++++---
 drivers/pci/controller/dwc/pcie-designware.h  | 12 +++-
 3 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 95efe14f1036..886483bf378b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -6,6 +6,7 @@
  * Author: Kishon Vijay Abraham I <kishon@ti.com>
  */
 
+#include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
@@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
 	return 0;
 }
 
-static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
-				   phys_addr_t phys_addr,
+static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
+				   u8 code, u8 routing, phys_addr_t phys_addr,
 				   u64 pci_addr, size_t size)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
 		return -EINVAL;
 	}
 
-	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
-					   phys_addr, pci_addr, size);
+	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
+					   code, routing, phys_addr, pci_addr,
+					   size);
 	if (ret)
 		return ret;
 
@@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
-	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
+	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
+				      addr, pci_addr, size);
 	if (ret) {
 		dev_err(pci->dev, "Failed to enable address\n");
 		return ret;
@@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
 	.get_features		= dw_pcie_ep_get_features,
 };
 
+static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
+			       u8 routing)
+{
+	struct pci_epc *epc = ep->epc;
+	int ret;
+
+	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
+				      routing, ep->intx_mem_phys, 0,
+				      epc->mem->window.page_size);
+	if (ret)
+		return ret;
+	writel(0, ep->intx_mem);
+	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
+
+	return 0;
+}
+
+static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
+					 int intx)
+{
+	int ret;
+
+	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
+	if (ret)
+		return ret;
+	usleep_range(1000, 2000);
+	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
+}
+
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	struct device *dev = pci->dev;
 
+	if (ep->intx_by_atu)
+		return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
+
 	dev_err(dev, "EP cannot trigger legacy IRQs\n");
 
 	return -EINVAL;
@@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 
 	dw_pcie_edma_remove(pci);
 
+	if (ep->intx_by_atu)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
 
@@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 		goto err_exit_epc_mem;
 	}
 
+	if (ep->intx_by_atu) {
+		ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
+						      epc->mem->window.page_size);
+		if (!ep->intx_mem) {
+			ret = -ENOMEM;
+			dev_err(dev, "Failed to reserve memory for INTx\n");
+			goto err_free_epc_mem;
+		}
+	}
+
 	ret = dw_pcie_edma_detect(pci);
 	if (ret)
-		goto err_free_epc_mem;
+		goto err_free_epc_mem_intx;
 
 	if (ep->ops->get_features) {
 		epc_features = ep->ops->get_features(ep);
@@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 err_remove_edma:
 	dw_pcie_edma_remove(pci);
 
+err_free_epc_mem_intx:
+	if (ep->intx_by_atu)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 err_free_epc_mem:
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..b4315cf84340 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
 }
 
 static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
-				       int index, int type, u64 cpu_addr,
-				       u64 pci_addr, u64 size)
+				       int index, int type, u8 code, u8 routing,
+				       u64 cpu_addr, u64 pci_addr, u64 size)
 {
 	u32 retries, val;
 	u64 limit_addr;
@@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
 			      upper_32_bits(pci_addr));
 
-	val = type | PCIE_ATU_FUNC_NUM(func_no);
+	val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
 	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
 	    dw_pcie_ver_is_ge(pci, 460A))
 		val |= PCIE_ATU_INCREASE_REGION_SIZE;
@@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 		val = dw_pcie_enable_ecrc(val);
 	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
 
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
+	if (code)
+		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
+				      PCIE_ATU_ENABLE |
+				      PCIE_ATU_INHIBIT_PAYLOAD |
+				      PCIE_ATU_HEADER_SUB_ENABLE | code);
+	else
+		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
+				      PCIE_ATU_ENABLE);
 
 	/*
 	 * Make sure ATU enable takes effect before any subsequent config
@@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 			      u64 cpu_addr, u64 pci_addr, u64 size)
 {
-	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
+	return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
 					   cpu_addr, pci_addr, size);
 }
 
 int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-				 int type, u64 cpu_addr, u64 pci_addr,
-				 u64 size)
+				 int type, u8 code, u8 routing, u64 cpu_addr,
+				 u64 pci_addr, u64 size)
 {
-	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
-					   cpu_addr, pci_addr, size);
+	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
+					   routing, cpu_addr, pci_addr, size);
 }
 
 static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 79713ce075cc..1a6e7e9489ee 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -147,11 +147,14 @@
 #define PCIE_ATU_TYPE_IO		0x2
 #define PCIE_ATU_TYPE_CFG0		0x4
 #define PCIE_ATU_TYPE_CFG1		0x5
+#define PCIE_ATU_TYPE_MSG		0x10
 #define PCIE_ATU_TD			BIT(8)
 #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
 #define PCIE_ATU_REGION_CTRL2		0x004
 #define PCIE_ATU_ENABLE			BIT(31)
 #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
+#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
+#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
 #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
 #define PCIE_ATU_LOWER_BASE		0x008
 #define PCIE_ATU_UPPER_BASE		0x00C
@@ -244,6 +247,9 @@
 /* Default eDMA LLP memory size */
 #define DMA_LLP_MEM_SIZE		PAGE_SIZE
 
+#define PCIE_MSG_ASSERT_INTA		0x20
+#define PCIE_MSG_DEASSERT_INTA		0x24
+
 struct dw_pcie;
 struct dw_pcie_rp;
 struct dw_pcie_ep;
@@ -352,7 +358,10 @@ struct dw_pcie_ep {
 	unsigned long		*ob_window_map;
 	void __iomem		*msi_mem;
 	phys_addr_t		msi_mem_phys;
+	void __iomem		*intx_mem;
+	phys_addr_t		intx_mem_phys;
 	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
+	bool			intx_by_atu;
 };
 
 struct dw_pcie_ops {
@@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 			      u64 cpu_addr, u64 pci_addr, u64 size);
 int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
+				 int type, u8 code, u8 routing, u64 cpu_addr,
+				 u64 pci_addr, u64 size);
 int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
 			     u64 cpu_addr, u64 pci_addr, u64 size);
 int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-- 
2.25.1

[PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Add R-Car Gen4 PCIe Host support. This controller is based on
Synopsys DesignWare PCIe.

This controller doesn't support MSI-X interrupt. So, introduce
no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
from dw_pcie_msi_domain_info.

Note that this controller on R-Car S4-8 has an unexpected register
value on the dbi+0x97b register. So, add a new capability flag
which would force the unrolled eDMA mapping for the problematic
device, as suggested by Serge Semin.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/Kconfig            |   9 +
 drivers/pci/controller/dwc/Makefile           |   2 +
 .../pci/controller/dwc/pcie-designware-host.c |   3 +
 drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
 drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
 8 files changed, 419 insertions(+), 3 deletions(-)
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 434f6a4f4041..94805ec31a8f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -414,4 +414,13 @@ config PCIE_FU740
 	  Say Y here if you want PCIe controller support for the SiFive
 	  FU740.
 
+config PCIE_RCAR_GEN4
+	tristate "Renesas R-Car Gen4 PCIe Host controller"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
+	  This uses the DesignWare core.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index bf5c311875a1..486cf706b53d 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
 obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
 obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
 obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
+pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
+obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 9952057c8819..5aefeec15c9b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 
 	irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
 
+	if (pp->no_msix)
+		dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
+
 	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
 						   &dw_pcie_msi_domain_info,
 						   pp->irq_domain);
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index b4315cf84340..7d649ba387f8 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 	 * Indirect eDMA CSRs access has been completely removed since v5.40a
 	 * thus no space is now reserved for the eDMA channels viewport and
 	 * former DMA CTRL register is no longer fixed to FFs.
+	 *
+	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
+	 * register even though the HW-manual explicitly states there must FFs
+	 * if the unrolled mapping is enabled. For such cases the low-level
+	 * drivers are supposed to manually activate the unrolled mapping to
+	 * bypass the auto-detection procedure.
 	 */
-	if (dw_pcie_ver_is_ge(pci, 540A))
+	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
 		val = 0xFFFFFFFF;
 	else
 		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 1a6e7e9489ee..1b1af514b849 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -51,8 +51,9 @@
 
 /* DWC PCIe controller capabilities */
 #define DW_PCIE_CAP_REQ_RES		0
-#define DW_PCIE_CAP_IATU_UNROLL		1
-#define DW_PCIE_CAP_CDM_CHECK		2
+#define DW_PCIE_CAP_EDMA_UNROLL		1
+#define DW_PCIE_CAP_IATU_UNROLL		2
+#define DW_PCIE_CAP_CDM_CHECK		3
 
 #define dw_pcie_cap_is(_pci, _cap) \
 	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
@@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
 struct dw_pcie_rp {
 	bool			has_msi_ctrl:1;
 	bool			cfg0_io_shared:1;
+	bool			no_msix:1;
 	u64			cfg0_base;
 	void __iomem		*va_cfg0_base;
 	u32			cfg0_size;
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
new file mode 100644
index 000000000000..d60f4895ffe9
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	int ret;
+	u32 val;
+
+	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
+	if (ret < 0)
+		return ret;
+
+	dw_pcie_dbi_ro_wr_en(dw);
+
+	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
+	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
+
+	dw_pcie_dbi_ro_wr_dis(dw);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		/* Enable MSI interrupt signal */
+		val = readl(rcar->base + PCIEINTSTS0EN);
+		val |= MSI_CTRL_INT;
+		writel(val, rcar->base + PCIEINTSTS0EN);
+	}
+
+	gpiod_set_value_cansleep(dw->pe_rst, 0);
+
+	dw_pcie_setup_rc(pp);
+
+	dw_pcie_dbi_ro_wr_en(dw);
+	rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
+	dw_pcie_dbi_ro_wr_dis(dw);
+
+	if (!dw_pcie_link_up(dw)) {
+		ret = dw->ops->start_link(dw);
+		if (ret)
+			return ret;
+	}
+
+	/* Ignore errors, the link may come up later */
+	if (dw_pcie_wait_for_link(dw))
+		dev_info(dw->dev, "PCIe link down\n");
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
+	.host_init = rcar_gen4_pcie_host_init,
+};
+
+static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
+				   struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_rp *pp = &dw->pp;
+	int ret;
+
+	pp->ops = &rcar_gen4_pcie_host_ops;
+	pp->no_msix = true;
+	dw_pcie_cap_set(dw, REQ_RES);
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
+{
+	dw_pcie_host_deinit(&rcar->dw.pp);
+	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
+}
+
+static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
+					struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+
+	/* Renesas-specific registers */
+	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
+	if (IS_ERR(rcar->base))
+		return PTR_ERR(rcar->base);
+
+	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
+}
+
+static int rcar_gen4_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen4_pcie *rcar;
+	int err;
+
+	rcar = rcar_gen4_pcie_devm_alloc(dev);
+	if (!rcar)
+		return -ENOMEM;
+
+	err = rcar_gen4_pcie_get_resources(rcar, pdev);
+	if (err < 0) {
+		dev_err(dev, "Failed to request resource: %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, rcar);
+
+	err = rcar_gen4_pcie_prepare(rcar);
+	if (err < 0)
+		return err;
+
+	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
+	if (err < 0)
+		goto err_add;
+
+	return 0;
+
+err_add:
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return err;
+}
+
+static int rcar_gen4_pcie_remove(struct platform_device *pdev)
+{
+	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+	rcar_gen4_remove_dw_pcie_rp(rcar);
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-pcie", },
+	{},
+};
+
+static struct platform_driver rcar_gen4_pcie_driver = {
+	.driver = {
+		.name = "pcie-rcar-gen4",
+		.of_match_table = rcar_gen4_pcie_of_match,
+	},
+	.probe = rcar_gen4_pcie_probe,
+	.remove = rcar_gen4_pcie_remove,
+};
+module_platform_driver(rcar_gen4_pcie_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
new file mode 100644
index 000000000000..a6a29d6125c8
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Renesas-specific */
+#define PCIERSTCTRL1		0x0014
+#define  APP_HOLD_PHY_RST	BIT(16)
+#define  APP_LTSSM_ENABLE	BIT(0)
+
+static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
+					bool enable)
+{
+	u32 val;
+
+	val = readl(rcar->base + PCIERSTCTRL1);
+	if (enable) {
+		val |= APP_LTSSM_ENABLE;
+		val &= ~APP_HOLD_PHY_RST;
+	} else {
+		val &= ~APP_LTSSM_ENABLE;
+		val |= APP_HOLD_PHY_RST;
+	}
+	writel(val, rcar->base + PCIERSTCTRL1);
+}
+
+static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	u32 val, mask;
+
+	val = readl(rcar->base + PCIEINTSTS0);
+	mask = RDLH_LINK_UP | SMLH_LINK_UP;
+
+	return (val & mask) == mask;
+}
+
+static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+	rcar_gen4_pcie_ltssm_enable(rcar, true);
+
+	return 0;
+}
+
+static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+	rcar_gen4_pcie_ltssm_enable(rcar, false);
+}
+
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+				   int num_lanes)
+{
+	u32 val;
+
+	/* Note: Assume the reset is asserted here */
+	val = readl(rcar->base + PCIEMSR0);
+	if (rc)
+		val |= DEVICE_TYPE_RC;
+	else
+		val |= DEVICE_TYPE_EP;
+	if (num_lanes < 4)
+		val |= BIFUR_MOD_SET_ON;
+	writel(val, rcar->base + PCIEMSR0);
+
+	return reset_control_deassert(rcar->rst);
+}
+
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
+{
+	dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
+}
+
+void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
+{
+	u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
+
+	val &= ~PCI_EXP_LNKCAP_MLW;
+	switch (num_lanes) {
+	case 1:
+		val |= PCI_EXP_LNKCAP_MLW_X1;
+		break;
+	case 2:
+		val |= PCI_EXP_LNKCAP_MLW_X2;
+		break;
+	case 4:
+		val |= PCI_EXP_LNKCAP_MLW_X4;
+		break;
+	default:
+		dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
+		val |= PCI_EXP_LNKCAP_MLW_X1;
+		break;
+	}
+	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
+}
+
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
+{
+	struct device *dev = rcar->dw.dev;
+	int err;
+
+	pm_runtime_enable(dev);
+	err = pm_runtime_resume_and_get(dev);
+	if (err < 0) {
+		dev_err(dev, "Failed to resume/get Runtime PM\n");
+		pm_runtime_disable(dev);
+	}
+
+	return err;
+}
+
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
+{
+	struct device *dev = rcar->dw.dev;
+
+	if (!reset_control_status(rcar->rst))
+		reset_control_assert(rcar->rst);
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+}
+
+int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
+				  struct device *dev)
+{
+	rcar->rst = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(rcar->rst)) {
+		dev_err(dev, "Failed to get Cold-reset\n");
+		return PTR_ERR(rcar->rst);
+	}
+
+	return 0;
+}
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = rcar_gen4_pcie_start_link,
+	.stop_link = rcar_gen4_pcie_stop_link,
+	.link_up = rcar_gen4_pcie_link_up,
+};
+
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
+{
+	struct rcar_gen4_pcie *rcar;
+
+	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
+	if (!rcar)
+		return NULL;
+
+	rcar->dw.dev = dev;
+	rcar->dw.ops = &dw_pcie_ops;
+	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
+
+	return rcar;
+}
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
new file mode 100644
index 000000000000..1cdce0cf7dac
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#ifndef _PCIE_RCAR_GEN4_H_
+#define _PCIE_RCAR_GEN4_H_
+
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+/* PCI Express capability */
+#define EXPCAP(x)		(0x0070 + (x))
+/* ASPM L1 PM Substates */
+#define L1PSCAP(x)		(0x01bc + (x))
+/* PCI Shadow offset */
+#define SHADOW_REG(x)		(0x2000 + (x))
+/* BAR Mask registers */
+#define BAR0MASKF		0x0010
+#define BAR1MASKF		0x0014
+#define BAR2MASKF		0x0018
+#define BAR3MASKF		0x001c
+#define BAR4MASKF		0x0020
+#define BAR5MASKF		0x0024
+
+/* Renesas-specific */
+#define PCIEMSR0		0x0000
+#define  BIFUR_MOD_SET_ON	BIT(0)
+#define  DEVICE_TYPE_EP		0
+#define  DEVICE_TYPE_RC		BIT(4)
+
+#define PCIEINTSTS0		0x0084
+#define PCIEINTSTS0EN		0x0310
+#define  MSI_CTRL_INT		BIT(26)
+#define  SMLH_LINK_UP		BIT(7)
+#define  RDLH_LINK_UP		BIT(6)
+#define PCIEDMAINTSTSEN		0x0314
+#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
+
+struct rcar_gen4_pcie {
+	struct dw_pcie		dw;
+	void __iomem		*base;
+	struct reset_control	*rst;
+};
+#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
+
+u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
+void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+				   int num_lanes);
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
+void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
+int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
+				  struct device *dev);
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
+
+#endif /* _PCIE_RCAR_GEN4_H_ */
-- 
2.25.1

[PATCH v9 7/8] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support

[Yoshihiro Shimoda]

Add R-Car Gen4 PCIe Endpoint support. This controller is based on
Synopsys DesignWare PCIe.

This controller requires vender-specific initialization before
.ep_init(). To use dw->dbi and dw->num-lanes in the initialization
code, introduce .ep_pre_init() into struct dw_pcie_ep_ops.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/Kconfig            |   9 +
 drivers/pci/controller/dwc/Makefile           |   2 +
 .../pci/controller/dwc/pcie-designware-ep.c   |   3 +
 drivers/pci/controller/dwc/pcie-designware.h  |   1 +
 .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 185 ++++++++++++++++++
 5 files changed, 200 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 94805ec31a8f..f33e403c7b61 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -423,4 +423,13 @@ config PCIE_RCAR_GEN4
 	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
 	  This uses the DesignWare core.
 
+config PCIE_RCAR_GEN4_EP
+	tristate "Renesas R-Car Gen4 PCIe Endpoint controller"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	help
+	  Say Y here if you want PCIe endpoint controller support on R-Car Gen4
+	  SoCs. This uses the DesignWare core.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 486cf706b53d..0fb0bde26ac4 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
 obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
 pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
 obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
+pcie-rcar-gen4-ep-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-ep.o
+obj-$(CONFIG_PCIE_RCAR_GEN4_EP) += pcie-rcar-gen4-ep-drv.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 886483bf378b..6ce418fd3bfe 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -763,6 +763,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	ep->phys_base = res->start;
 	ep->addr_size = resource_size(res);
 
+	if (ep->ops->ep_pre_init)
+		ep->ops->ep_pre_init(ep);
+
 	dw_pcie_version_detect(pci);
 
 	dw_pcie_iatu_detect(pci);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 1b1af514b849..63b272587eec 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -326,6 +326,7 @@ struct dw_pcie_rp {
 };
 
 struct dw_pcie_ep_ops {
+	void	(*ep_pre_init)(struct dw_pcie_ep *ep);
 	void	(*ep_init)(struct dw_pcie_ep *ep);
 	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
 			     enum pci_epc_irq_type type, u16 interrupt_num);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
new file mode 100644
index 000000000000..ecffbac62f85
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Configuration */
+#define PCICONF3		0x000c
+#define  MULTI_FUNC		BIT(23)
+
+static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	int val;
+
+	rcar_gen4_pcie_set_device_type(rcar, false, dw->num_lanes);
+
+	dw_pcie_dbi_ro_wr_en(dw);
+
+	/* Single function */
+	val = dw_pcie_readl_dbi(dw, PCICONF3);
+	val &= ~MULTI_FUNC;
+	dw_pcie_writel_dbi(dw, PCICONF3, val);
+
+	rcar_gen4_pcie_disable_bar(dw, BAR5MASKF);
+
+	rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
+
+	dw_pcie_dbi_ro_wr_dis(dw);
+}
+
+static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				       enum pci_epc_irq_type type,
+				       u16 interrupt_num)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_EPC_IRQ_LEGACY:
+		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
+	case PCI_EPC_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	default:
+		dev_err(dw->dev, "UNKNOWN IRQ type\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
+	.linkup_notifier = false,
+	.msi_capable = true,
+	.msix_capable = false,
+	.reserved_bar = 1 << BAR_5,
+	.align = SZ_1M,
+};
+
+static const struct pci_epc_features*
+rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
+{
+	return &rcar_gen4_pcie_epc_features;
+}
+
+static const struct dw_pcie_ep_ops pcie_ep_ops = {
+	.ep_pre_init = rcar_gen4_pcie_ep_pre_init,
+	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
+	.get_features = rcar_gen4_pcie_ep_get_features,
+};
+
+static int rcar_gen4_add_pcie_ep(struct rcar_gen4_pcie *rcar,
+				 struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_ep *ep;
+	int ret;
+
+	ep = &dw->ep;
+	ep->ops = &pcie_ep_ops;
+	ep->intx_by_atu = true;
+
+	ret = dw_pcie_ep_init(ep);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize endpoint\n");
+		return ret;
+	}
+
+	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+
+	dw->ops->start_link(dw);
+
+	return 0;
+}
+
+static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+	writel(0, rcar->base + PCIEDMAINTSTSEN);
+	dw_pcie_ep_exit(&rcar->dw.ep);
+}
+
+static int rcar_gen4_pcie_ep_get_resources(struct rcar_gen4_pcie *rcar,
+					   struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+	struct device *dev = dw->dev;
+
+	/* Renesas-specific registers */
+	rcar->base = devm_platform_ioremap_resource_byname(pdev, "appl");
+	if (IS_ERR(rcar->base))
+		return PTR_ERR(rcar->base);
+
+	return rcar_gen4_pcie_devm_reset_get(rcar, dev);
+}
+
+static int rcar_gen4_pcie_ep_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen4_pcie *rcar;
+	int err;
+
+	rcar = rcar_gen4_pcie_devm_alloc(dev);
+	if (!rcar)
+		return -ENOMEM;
+
+	err = rcar_gen4_pcie_ep_get_resources(rcar, pdev);
+	if (err < 0) {
+		dev_err(dev, "failed to request resource: %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, rcar);
+
+	err = rcar_gen4_pcie_prepare(rcar);
+	if (err < 0)
+		return err;
+
+	err = rcar_gen4_add_pcie_ep(rcar, pdev);
+	if (err < 0)
+		goto err_add;
+
+	return 0;
+
+err_add:
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return err;
+}
+
+static int rcar_gen4_pcie_ep_remove(struct platform_device *pdev)
+{
+	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+	rcar_gen4_remove_pcie_ep(rcar);
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-pcie-ep", },
+	{},
+};
+
+static struct platform_driver rcar_gen4_pcie_ep_driver = {
+	.driver = {
+		.name = "pcie-rcar-gen4-ep",
+		.of_match_table = rcar_gen4_pcie_of_match,
+	},
+	.probe = rcar_gen4_pcie_ep_probe,
+	.remove = rcar_gen4_pcie_ep_remove,
+};
+module_platform_driver(rcar_gen4_pcie_ep_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v9 8/8] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4

[Yoshihiro Shimoda]

Update this entry for R-Car Gen4's source code.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 57d1d6ecb33d..d5392527d073 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16001,6 +16001,7 @@ L:	linux-renesas-soc@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/*rcar*
 F:	drivers/pci/controller/*rcar*
+F:	drivers/pci/controller/dwc/*rcar*
 
 PCI DRIVER FOR SAMSUNG EXYNOS
 M:	Jingoo Han <jingoohan1@gmail.com>
-- 
2.25.1

Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Rob Herring]

On Fri, Feb 10, 2023 at 7:49 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
>
> This controller doesn't support MSI-X interrupt. So, introduce
> no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> from dw_pcie_msi_domain_info.
>
> Note that this controller on R-Car S4-8 has an unexpected register
> value on the dbi+0x97b register. So, add a new capability flag
> which would force the unrolled eDMA mapping for the problematic
> device, as suggested by Serge Semin.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |   9 +
>  drivers/pci/controller/dwc/Makefile           |   2 +
>  .../pci/controller/dwc/pcie-designware-host.c |   3 +
>  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
>  drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
>  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
>  8 files changed, 419 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 434f6a4f4041..94805ec31a8f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -414,4 +414,13 @@ config PCIE_FU740
>           Say Y here if you want PCIe controller support for the SiFive
>           FU740.
>
> +config PCIE_RCAR_GEN4
> +       tristate "Renesas R-Car Gen4 PCIe Host controller"
> +       depends on ARCH_RENESAS || COMPILE_TEST
> +       depends on PCI_MSI
> +       select PCIE_DW_HOST
> +       help
> +         Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> +         This uses the DesignWare core.
> +
>  endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bf5c311875a1..486cf706b53d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
>
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..5aefeec15c9b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>
>         irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
>
> +       if (pp->no_msix)
> +               dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> +
>         pp->msi_domain = pci_msi_create_irq_domain(fwnode,
>                                                    &dw_pcie_msi_domain_info,
>                                                    pp->irq_domain);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index b4315cf84340..7d649ba387f8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>          * Indirect eDMA CSRs access has been completely removed since v5.40a
>          * thus no space is now reserved for the eDMA channels viewport and
>          * former DMA CTRL register is no longer fixed to FFs.
> +        *
> +        * Note some devices for unknown reason may have zeros in the eDMA CTRL
> +        * register even though the HW-manual explicitly states there must FFs
> +        * if the unrolled mapping is enabled. For such cases the low-level
> +        * drivers are supposed to manually activate the unrolled mapping to
> +        * bypass the auto-detection procedure.
>          */
> -       if (dw_pcie_ver_is_ge(pci, 540A))
> +       if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
>                 val = 0xFFFFFFFF;
>         else
>                 val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 1a6e7e9489ee..1b1af514b849 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -51,8 +51,9 @@
>
>  /* DWC PCIe controller capabilities */
>  #define DW_PCIE_CAP_REQ_RES            0
> -#define DW_PCIE_CAP_IATU_UNROLL                1
> -#define DW_PCIE_CAP_CDM_CHECK          2
> +#define DW_PCIE_CAP_EDMA_UNROLL                1
> +#define DW_PCIE_CAP_IATU_UNROLL                2
> +#define DW_PCIE_CAP_CDM_CHECK          3
>
>  #define dw_pcie_cap_is(_pci, _cap) \
>         test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
>  struct dw_pcie_rp {
>         bool                    has_msi_ctrl:1;
>         bool                    cfg0_io_shared:1;
> +       bool                    no_msix:1;
>         u64                     cfg0_base;
>         void __iomem            *va_cfg0_base;
>         u32                     cfg0_size;
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> new file mode 100644
> index 000000000000..d60f4895ffe9
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +       struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +       int ret;
> +       u32 val;
> +
> +       ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> +       if (ret < 0)
> +               return ret;
> +
> +       dw_pcie_dbi_ro_wr_en(dw);
> +
> +       rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> +       rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> +       dw_pcie_dbi_ro_wr_dis(dw);
> +
> +       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +               /* Enable MSI interrupt signal */
> +               val = readl(rcar->base + PCIEINTSTS0EN);
> +               val |= MSI_CTRL_INT;
> +               writel(val, rcar->base + PCIEINTSTS0EN);
> +       }
> +
> +       gpiod_set_value_cansleep(dw->pe_rst, 0);
> +
> +       dw_pcie_setup_rc(pp);
> +
> +       dw_pcie_dbi_ro_wr_en(dw);
> +       rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> +       dw_pcie_dbi_ro_wr_dis(dw);
> +
> +       if (!dw_pcie_link_up(dw)) {
> +               ret = dw->ops->start_link(dw);

The DW core code does this for you.

> +               if (ret)
> +                       return ret;
> +       }
> +
> +       /* Ignore errors, the link may come up later */
> +       if (dw_pcie_wait_for_link(dw))
> +               dev_info(dw->dev, "PCIe link down\n");

Same here.

> +
> +       return 0;
> +}
> +
> +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> +       .host_init = rcar_gen4_pcie_host_init,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> +                                  struct platform_device *pdev)
> +{
> +       struct dw_pcie *dw = &rcar->dw;
> +       struct dw_pcie_rp *pp = &dw->pp;
> +       int ret;
> +
> +       pp->ops = &rcar_gen4_pcie_host_ops;
> +       pp->no_msix = true;
> +       dw_pcie_cap_set(dw, REQ_RES);
> +
> +       ret = dw_pcie_host_init(pp);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Failed to initialize host\n");

If this failed, dw_pcie_host_init() should have printed an error. No
reason for every caller to print something.

> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> +{
> +       dw_pcie_host_deinit(&rcar->dw.pp);
> +       gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> +}
> +
> +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> +                                       struct platform_device *pdev)
> +{
> +       struct dw_pcie *dw = &rcar->dw;
> +
> +       /* Renesas-specific registers */
> +       rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> +       if (IS_ERR(rcar->base))
> +               return PTR_ERR(rcar->base);
> +
> +       return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> +}
> +
> +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct rcar_gen4_pcie *rcar;
> +       int err;
> +
> +       rcar = rcar_gen4_pcie_devm_alloc(dev);
> +       if (!rcar)
> +               return -ENOMEM;
> +
> +       err = rcar_gen4_pcie_get_resources(rcar, pdev);
> +       if (err < 0) {
> +               dev_err(dev, "Failed to request resource: %d\n", err);
> +               return err;
> +       }
> +
> +       platform_set_drvdata(pdev, rcar);
> +
> +       err = rcar_gen4_pcie_prepare(rcar);
> +       if (err < 0)
> +               return err;
> +
> +       err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> +       if (err < 0)
> +               goto err_add;

If you didn't add it, then you shouldn't remove it. You are calling
remove on both failed and successful add.

> +
> +       return 0;
> +
> +err_add:
> +       rcar_gen4_pcie_unprepare(rcar);
> +
> +       return err;
> +}
> +
> +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> +{
> +       struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> +       rcar_gen4_remove_dw_pcie_rp(rcar);
> +       rcar_gen4_pcie_unprepare(rcar);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> +       { .compatible = "renesas,rcar-gen4-pcie", },
> +       {},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_driver = {
> +       .driver = {
> +               .name = "pcie-rcar-gen4",
> +               .of_match_table = rcar_gen4_pcie_of_match,
> +       },
> +       .probe = rcar_gen4_pcie_probe,
> +       .remove = rcar_gen4_pcie_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> new file mode 100644
> index 000000000000..a6a29d6125c8
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Renesas-specific */
> +#define PCIERSTCTRL1           0x0014
> +#define  APP_HOLD_PHY_RST      BIT(16)
> +#define  APP_LTSSM_ENABLE      BIT(0)
> +
> +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> +                                       bool enable)
> +{
> +       u32 val;
> +
> +       val = readl(rcar->base + PCIERSTCTRL1);
> +       if (enable) {
> +               val |= APP_LTSSM_ENABLE;
> +               val &= ~APP_HOLD_PHY_RST;
> +       } else {
> +               val &= ~APP_LTSSM_ENABLE;
> +               val |= APP_HOLD_PHY_RST;
> +       }
> +       writel(val, rcar->base + PCIERSTCTRL1);
> +}
> +
> +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> +{
> +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +       u32 val, mask;
> +
> +       val = readl(rcar->base + PCIEINTSTS0);
> +       mask = RDLH_LINK_UP | SMLH_LINK_UP;
> +
> +       return (val & mask) == mask;
> +}
> +
> +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> +{
> +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +       rcar_gen4_pcie_ltssm_enable(rcar, true);
> +
> +       return 0;
> +}
> +
> +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> +{
> +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +       rcar_gen4_pcie_ltssm_enable(rcar, false);
> +}
> +
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +                                  int num_lanes)
> +{
> +       u32 val;
> +
> +       /* Note: Assume the reset is asserted here */
> +       val = readl(rcar->base + PCIEMSR0);
> +       if (rc)
> +               val |= DEVICE_TYPE_RC;
> +       else
> +               val |= DEVICE_TYPE_EP;
> +       if (num_lanes < 4)
> +               val |= BIFUR_MOD_SET_ON;
> +       writel(val, rcar->base + PCIEMSR0);
> +
> +       return reset_control_deassert(rcar->rst);
> +}
> +
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> +{
> +       dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> +}
> +
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> +{
> +       u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> +
> +       val &= ~PCI_EXP_LNKCAP_MLW;
> +       switch (num_lanes) {
> +       case 1:
> +               val |= PCI_EXP_LNKCAP_MLW_X1;
> +               break;
> +       case 2:
> +               val |= PCI_EXP_LNKCAP_MLW_X2;
> +               break;
> +       case 4:
> +               val |= PCI_EXP_LNKCAP_MLW_X4;
> +               break;
> +       default:
> +               dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> +               val |= PCI_EXP_LNKCAP_MLW_X1;
> +               break;

The register field is just the number of lanes, why not do a macro
that just uses directly instead of essentially a lookup table:

val |= PCI_EXP_LNKCAP_MLW(num_lanes);

You shouldn't need to validate 'num_lanes' here except for a max
value. Though dtschema does that (or should). If you want to validate
it further in the kernel, do that somewhere common so everyone
benefits.

Or better yet, refactor this to be common. We already have Tegra194
doing the same thing.


> +       }
> +       dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> +}
> +
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> +{
> +       struct device *dev = rcar->dw.dev;
> +       int err;
> +
> +       pm_runtime_enable(dev);
> +       err = pm_runtime_resume_and_get(dev);
> +       if (err < 0) {
> +               dev_err(dev, "Failed to resume/get Runtime PM\n");
> +               pm_runtime_disable(dev);
> +       }
> +
> +       return err;
> +}
> +
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> +{
> +       struct device *dev = rcar->dw.dev;
> +
> +       if (!reset_control_status(rcar->rst))
> +               reset_control_assert(rcar->rst);
> +       pm_runtime_put(dev);
> +       pm_runtime_disable(dev);
> +}
> +
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> +                                 struct device *dev)
> +{
> +       rcar->rst = devm_reset_control_get(dev, NULL);
> +       if (IS_ERR(rcar->rst)) {
> +               dev_err(dev, "Failed to get Cold-reset\n");
> +               return PTR_ERR(rcar->rst);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +       .start_link = rcar_gen4_pcie_start_link,
> +       .stop_link = rcar_gen4_pcie_stop_link,
> +       .link_up = rcar_gen4_pcie_link_up,
> +};
> +
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> +{
> +       struct rcar_gen4_pcie *rcar;
> +
> +       rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> +       if (!rcar)
> +               return NULL;
> +
> +       rcar->dw.dev = dev;
> +       rcar->dw.ops = &dw_pcie_ops;
> +       dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> +
> +       return rcar;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> new file mode 100644
> index 000000000000..1cdce0cf7dac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#ifndef _PCIE_RCAR_GEN4_H_
> +#define _PCIE_RCAR_GEN4_H_
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* PCI Express capability */
> +#define EXPCAP(x)              (0x0070 + (x))

This offset is discoverable, don't hardcode it.

> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x)             (0x01bc + (x))
> +/* PCI Shadow offset */
> +#define SHADOW_REG(x)          (0x2000 + (x))
> +/* BAR Mask registers */
> +#define BAR0MASKF              0x0010
> +#define BAR1MASKF              0x0014
> +#define BAR2MASKF              0x0018
> +#define BAR3MASKF              0x001c
> +#define BAR4MASKF              0x0020
> +#define BAR5MASKF              0x0024
> +
> +/* Renesas-specific */
> +#define PCIEMSR0               0x0000
> +#define  BIFUR_MOD_SET_ON      BIT(0)
> +#define  DEVICE_TYPE_EP                0
> +#define  DEVICE_TYPE_RC                BIT(4)
> +
> +#define PCIEINTSTS0            0x0084
> +#define PCIEINTSTS0EN          0x0310
> +#define  MSI_CTRL_INT          BIT(26)
> +#define  SMLH_LINK_UP          BIT(7)
> +#define  RDLH_LINK_UP          BIT(6)
> +#define PCIEDMAINTSTSEN                0x0314
> +#define  PCIEDMAINTSTSEN_INIT  GENMASK(15, 0)
> +
> +struct rcar_gen4_pcie {
> +       struct dw_pcie          dw;
> +       void __iomem            *base;
> +       struct reset_control    *rst;
> +};
> +#define to_rcar_gen4_pcie(x)   dev_get_drvdata((x)->dev)
> +
> +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +                                  int num_lanes);
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> +                                 struct device *dev);
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> +
> +#endif /* _PCIE_RCAR_GEN4_H_ */
> --
> 2.25.1
>

Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Bjorn Helgaas]

On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.

> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h

> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x)		(0x01bc + (x))

Doesn't appear to be used; please remove it.

Re: [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint

[Serge Semin]

On Fri, Feb 10, 2023 at 10:49:11PM +0900, Yoshihiro Shimoda wrote:
> Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> PCIe endpoint module.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/pci/rcar-gen4-pci-ep.yaml        | 90 +++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> new file mode 100644
> index 000000000000..4b10d67e4336
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car Gen4 PCIe Endpoint
> +
> +maintainers:
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +allOf:
> +  - $ref: snps,dw-pcie-ep.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: renesas,r8a779f0-pcie-ep   # R-Car S4-8
> +      - const: renesas,rcar-gen4-pcie-ep  # R-Car Gen4
> +
> +  reg:
> +    maxItems: 4
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: atu

> +      - const: appl

Please, use "elbi" or "app" instead. No need in using the
vendor-specific names if there is the generic ones.
(* @Rob, that's why I was insisting in failing the DT-bindings
evaluation for such cases...)
See Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml:98

> +      - const: addr_space
> +
> +  interrupts:
> +    maxItems: 3
> +
> +  interrupt-names:
> +    items:

> +      - const: dma

Are you sure there is a single IRQ line for all eDMA channels?
Judging by the DW PCIe HW manual the eDMA events are signaled by the
wires: edma_int[((CC_NUM_DMA_RD_CHAN+CC_NUM_DMA_WR_CHAN)-1):0]. If you
have a single signal then they must have been combined on the way to
the GIC though... 

> +      - const: sft_ce
> +      - const: app
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +

> +  max-link-speed: true

This prop is determined by the CX_MAX_PCIE_SPEED IP-core synthesize
parameter.

> +
> +  num-lanes: true

This is determined by the CX_NL IP-core synthesize parameter.

Thus you can provide at least the 'maximum' constraint for
the properties above.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - resets
> +  - power-domains
> +  - clocks
> +
> +additionalProperties: false

Are you sure that none of the next properties will be ever used in
the R-Car PCIe End-point DT-nodes?
max-functions
max-virtual-functions
phys
phy-names
reset-gpios
snps,enable-cdm-check
dma-coherent
* etc

I am pretty much sure that the reset-gpios (platform-specific),
max-{virtual-}functions (determined by the CX_NFUNC IP-core synthesize
parameter), phys/phy-names (you had a PHYs CSR space in the
DT-bindings example) and dma-coherent properties are relevant for your
device. At the very least the 'max-functions' prop constraint could be
explicitly added to your DT-bindings file. You must be aware of how
many functions the R-Car PCIe EP support, right? The rest of the
properties could be implicitly evaluated by replacing the
"additionalProperties: false" flag with the "unevaluatedProperties:
false" statement.

-Serge(y)

> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie0_ep: pcie-ep@e65d0000 {
> +            compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
> +            reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
> +                  <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
> +            reg-names = "dbi", "atu", "appl", "addr_space";
> +            interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "dma", "sft_ce", "app";
> +            clocks = <&cpg CPG_MOD 624>;
> +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> +            resets = <&cpg 624>;
> +            num-lanes = <2>;
> +            max-link-speed = <2>;
> +        };
> +    };
> -- 
> 2.25.1
> 
> 

RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Hi Rob,

> From: Rob Herring, Sent: Saturday, February 11, 2023 2:22 AM
> 
> On Fri, Feb 10, 2023 at 7:49 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> >
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller doesn't support MSI-X interrupt. So, introduce
> > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > from dw_pcie_msi_domain_info.
> >
> > Note that this controller on R-Car S4-8 has an unexpected register
> > value on the dbi+0x97b register. So, add a new capability flag
> > which would force the unrolled eDMA mapping for the problematic
> > device, as suggested by Serge Semin.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig            |   9 +
> >  drivers/pci/controller/dwc/Makefile           |   2 +
> >  .../pci/controller/dwc/pcie-designware-host.c |   3 +
> >  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
> >  drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
> >  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
> >  8 files changed, 419 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 434f6a4f4041..94805ec31a8f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -414,4 +414,13 @@ config PCIE_FU740
> >           Say Y here if you want PCIe controller support for the SiFive
> >           FU740.
> >
> > +config PCIE_RCAR_GEN4
> > +       tristate "Renesas R-Car Gen4 PCIe Host controller"
> > +       depends on ARCH_RENESAS || COMPILE_TEST
> > +       depends on PCI_MSI
> > +       select PCIE_DW_HOST
> > +       help
> > +         Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > +         This uses the DesignWare core.
> > +
> >  endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index bf5c311875a1..486cf706b53d 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> >  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> >  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> >  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> >
> >  # The following drivers are for devices that use the generic ACPI
> >  # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9952057c8819..5aefeec15c9b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> >
> >         irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> >
> > +       if (pp->no_msix)
> > +               dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> > +
> >         pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> >                                                    &dw_pcie_msi_domain_info,
> >                                                    pp->irq_domain);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index b4315cf84340..7d649ba387f8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >          * Indirect eDMA CSRs access has been completely removed since v5.40a
> >          * thus no space is now reserved for the eDMA channels viewport and
> >          * former DMA CTRL register is no longer fixed to FFs.
> > +        *
> > +        * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > +        * register even though the HW-manual explicitly states there must FFs
> > +        * if the unrolled mapping is enabled. For such cases the low-level
> > +        * drivers are supposed to manually activate the unrolled mapping to
> > +        * bypass the auto-detection procedure.
> >          */
> > -       if (dw_pcie_ver_is_ge(pci, 540A))
> > +       if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> >                 val = 0xFFFFFFFF;
> >         else
> >                 val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 1a6e7e9489ee..1b1af514b849 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -51,8 +51,9 @@
> >
> >  /* DWC PCIe controller capabilities */
> >  #define DW_PCIE_CAP_REQ_RES            0
> > -#define DW_PCIE_CAP_IATU_UNROLL                1
> > -#define DW_PCIE_CAP_CDM_CHECK          2
> > +#define DW_PCIE_CAP_EDMA_UNROLL                1
> > +#define DW_PCIE_CAP_IATU_UNROLL                2
> > +#define DW_PCIE_CAP_CDM_CHECK          3
> >
> >  #define dw_pcie_cap_is(_pci, _cap) \
> >         test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> >  struct dw_pcie_rp {
> >         bool                    has_msi_ctrl:1;
> >         bool                    cfg0_io_shared:1;
> > +       bool                    no_msix:1;
> >         u64                     cfg0_base;
> >         void __iomem            *va_cfg0_base;
> >         u32                     cfg0_size;
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > new file mode 100644
> > index 000000000000..d60f4895ffe9
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > +       struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +       int ret;
> > +       u32 val;
> > +
> > +       ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       dw_pcie_dbi_ro_wr_en(dw);
> > +
> > +       rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > +       rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > +       dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > +               /* Enable MSI interrupt signal */
> > +               val = readl(rcar->base + PCIEINTSTS0EN);
> > +               val |= MSI_CTRL_INT;
> > +               writel(val, rcar->base + PCIEINTSTS0EN);
> > +       }
> > +
> > +       gpiod_set_value_cansleep(dw->pe_rst, 0);
> > +
> > +       dw_pcie_setup_rc(pp);
> > +
> > +       dw_pcie_dbi_ro_wr_en(dw);
> > +       rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > +       dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +       if (!dw_pcie_link_up(dw)) {
> > +               ret = dw->ops->start_link(dw);
> 
> The DW core code does this for you.

I got it. I'll check whether removing this also works or not.

> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       /* Ignore errors, the link may come up later */
> > +       if (dw_pcie_wait_for_link(dw))
> > +               dev_info(dw->dev, "PCIe link down\n");
> 
> Same here.

Same here.

> > +
> > +       return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > +       .host_init = rcar_gen4_pcie_host_init,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > +                                  struct platform_device *pdev)
> > +{
> > +       struct dw_pcie *dw = &rcar->dw;
> > +       struct dw_pcie_rp *pp = &dw->pp;
> > +       int ret;
> > +
> > +       pp->ops = &rcar_gen4_pcie_host_ops;
> > +       pp->no_msix = true;
> > +       dw_pcie_cap_set(dw, REQ_RES);
> > +
> > +       ret = dw_pcie_host_init(pp);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "Failed to initialize host\n");
> 
> If this failed, dw_pcie_host_init() should have printed an error. No
> reason for every caller to print something.

I got it. I will remove it.

> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > +{
> > +       dw_pcie_host_deinit(&rcar->dw.pp);
> > +       gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > +}
> > +
> > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > +                                       struct platform_device *pdev)
> > +{
> > +       struct dw_pcie *dw = &rcar->dw;
> > +
> > +       /* Renesas-specific registers */
> > +       rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > +       if (IS_ERR(rcar->base))
> > +               return PTR_ERR(rcar->base);
> > +
> > +       return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > +}
> > +
> > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct rcar_gen4_pcie *rcar;
> > +       int err;
> > +
> > +       rcar = rcar_gen4_pcie_devm_alloc(dev);
> > +       if (!rcar)
> > +               return -ENOMEM;
> > +
> > +       err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > +       if (err < 0) {
> > +               dev_err(dev, "Failed to request resource: %d\n", err);
> > +               return err;
> > +       }
> > +
> > +       platform_set_drvdata(pdev, rcar);
> > +
> > +       err = rcar_gen4_pcie_prepare(rcar);
> > +       if (err < 0)
> > +               return err;
> > +
> > +       err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > +       if (err < 0)
> > +               goto err_add;
> 
> If you didn't add it, then you shouldn't remove it. You are calling
> remove on both failed and successful add.

I'm afraid but, I could not understand why/where I'm calling remove on
both failed and successful add. rcar_gen4_remove_dw_pcie_rp(rcar) is
called from rcar_gen4_pcie_remove() only, and IIUC, the .remove() function
is only called after .probe() was successful.

> > +
> > +       return 0;
> > +
> > +err_add:
> > +       rcar_gen4_pcie_unprepare(rcar);
> > +
> > +       return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > +{
> > +       struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > +       rcar_gen4_remove_dw_pcie_rp(rcar);
> > +       rcar_gen4_pcie_unprepare(rcar);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > +       { .compatible = "renesas,rcar-gen4-pcie", },
> > +       {},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_driver = {
> > +       .driver = {
> > +               .name = "pcie-rcar-gen4",
> > +               .of_match_table = rcar_gen4_pcie_of_match,
> > +       },
> > +       .probe = rcar_gen4_pcie_probe,
> > +       .remove = rcar_gen4_pcie_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > new file mode 100644
> > index 000000000000..a6a29d6125c8
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Renesas-specific */
> > +#define PCIERSTCTRL1           0x0014
> > +#define  APP_HOLD_PHY_RST      BIT(16)
> > +#define  APP_LTSSM_ENABLE      BIT(0)
> > +
> > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > +                                       bool enable)
> > +{
> > +       u32 val;
> > +
> > +       val = readl(rcar->base + PCIERSTCTRL1);
> > +       if (enable) {
> > +               val |= APP_LTSSM_ENABLE;
> > +               val &= ~APP_HOLD_PHY_RST;
> > +       } else {
> > +               val &= ~APP_LTSSM_ENABLE;
> > +               val |= APP_HOLD_PHY_RST;
> > +       }
> > +       writel(val, rcar->base + PCIERSTCTRL1);
> > +}
> > +
> > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > +{
> > +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +       u32 val, mask;
> > +
> > +       val = readl(rcar->base + PCIEINTSTS0);
> > +       mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > +
> > +       return (val & mask) == mask;
> > +}
> > +
> > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > +{
> > +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +       rcar_gen4_pcie_ltssm_enable(rcar, true);
> > +
> > +       return 0;
> > +}
> > +
> > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > +{
> > +       struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +       rcar_gen4_pcie_ltssm_enable(rcar, false);
> > +}
> > +
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +                                  int num_lanes)
> > +{
> > +       u32 val;
> > +
> > +       /* Note: Assume the reset is asserted here */
> > +       val = readl(rcar->base + PCIEMSR0);
> > +       if (rc)
> > +               val |= DEVICE_TYPE_RC;
> > +       else
> > +               val |= DEVICE_TYPE_EP;
> > +       if (num_lanes < 4)
> > +               val |= BIFUR_MOD_SET_ON;
> > +       writel(val, rcar->base + PCIEMSR0);
> > +
> > +       return reset_control_deassert(rcar->rst);
> > +}
> > +
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > +{
> > +       dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > +}
> > +
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > +{
> > +       u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > +
> > +       val &= ~PCI_EXP_LNKCAP_MLW;
> > +       switch (num_lanes) {
> > +       case 1:
> > +               val |= PCI_EXP_LNKCAP_MLW_X1;
> > +               break;
> > +       case 2:
> > +               val |= PCI_EXP_LNKCAP_MLW_X2;
> > +               break;
> > +       case 4:
> > +               val |= PCI_EXP_LNKCAP_MLW_X4;
> > +               break;
> > +       default:
> > +               dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > +               val |= PCI_EXP_LNKCAP_MLW_X1;
> > +               break;
> 
> The register field is just the number of lanes, why not do a macro
> that just uses directly instead of essentially a lookup table:
> 
> val |= PCI_EXP_LNKCAP_MLW(num_lanes);

I see. I should have assumed I should use PCI_EXP_LNKCAP_MLW_Xxx macros.

> You shouldn't need to validate 'num_lanes' here except for a max
> value. Though dtschema does that (or should). If you want to validate
> it further in the kernel, do that somewhere common so everyone
> benefits.

I got it. I'll revise dt-bindings doc for it.

> Or better yet, refactor this to be common. We already have Tegra194
> doing the same thing.

Thank you for your suggestion. I'll check it.


> > +       }
> > +       dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > +}
> > +
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +       struct device *dev = rcar->dw.dev;
> > +       int err;
> > +
> > +       pm_runtime_enable(dev);
> > +       err = pm_runtime_resume_and_get(dev);
> > +       if (err < 0) {
> > +               dev_err(dev, "Failed to resume/get Runtime PM\n");
> > +               pm_runtime_disable(dev);
> > +       }
> > +
> > +       return err;
> > +}
> > +
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +       struct device *dev = rcar->dw.dev;
> > +
> > +       if (!reset_control_status(rcar->rst))
> > +               reset_control_assert(rcar->rst);
> > +       pm_runtime_put(dev);
> > +       pm_runtime_disable(dev);
> > +}
> > +
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > +                                 struct device *dev)
> > +{
> > +       rcar->rst = devm_reset_control_get(dev, NULL);
> > +       if (IS_ERR(rcar->rst)) {
> > +               dev_err(dev, "Failed to get Cold-reset\n");
> > +               return PTR_ERR(rcar->rst);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > +       .start_link = rcar_gen4_pcie_start_link,
> > +       .stop_link = rcar_gen4_pcie_stop_link,
> > +       .link_up = rcar_gen4_pcie_link_up,
> > +};
> > +
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > +{
> > +       struct rcar_gen4_pcie *rcar;
> > +
> > +       rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > +       if (!rcar)
> > +               return NULL;
> > +
> > +       rcar->dw.dev = dev;
> > +       rcar->dw.ops = &dw_pcie_ops;
> > +       dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > +
> > +       return rcar;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > new file mode 100644
> > index 000000000000..1cdce0cf7dac
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef _PCIE_RCAR_GEN4_H_
> > +#define _PCIE_RCAR_GEN4_H_
> > +
> > +#include <linux/io.h>
> > +#include <linux/pci.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* PCI Express capability */
> > +#define EXPCAP(x)              (0x0070 + (x))
> 
> This offset is discoverable, don't hardcode it.

I got it. I'll fix it.

Best regards,
Yoshihiro Shimoda

> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x)             (0x01bc + (x))
> > +/* PCI Shadow offset */
> > +#define SHADOW_REG(x)          (0x2000 + (x))
> > +/* BAR Mask registers */
> > +#define BAR0MASKF              0x0010
> > +#define BAR1MASKF              0x0014
> > +#define BAR2MASKF              0x0018
> > +#define BAR3MASKF              0x001c
> > +#define BAR4MASKF              0x0020
> > +#define BAR5MASKF              0x0024
> > +
> > +/* Renesas-specific */
> > +#define PCIEMSR0               0x0000
> > +#define  BIFUR_MOD_SET_ON      BIT(0)
> > +#define  DEVICE_TYPE_EP                0
> > +#define  DEVICE_TYPE_RC                BIT(4)
> > +
> > +#define PCIEINTSTS0            0x0084
> > +#define PCIEINTSTS0EN          0x0310
> > +#define  MSI_CTRL_INT          BIT(26)
> > +#define  SMLH_LINK_UP          BIT(7)
> > +#define  RDLH_LINK_UP          BIT(6)
> > +#define PCIEDMAINTSTSEN                0x0314
> > +#define  PCIEDMAINTSTSEN_INIT  GENMASK(15, 0)
> > +
> > +struct rcar_gen4_pcie {
> > +       struct dw_pcie          dw;
> > +       void __iomem            *base;
> > +       struct reset_control    *rst;
> > +};
> > +#define to_rcar_gen4_pcie(x)   dev_get_drvdata((x)->dev)
> > +
> > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +                                  int num_lanes);
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > +                                 struct device *dev);
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > +
> > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > --
> > 2.25.1
> >

RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Hi Bjorn,

> From: Bjorn Helgaas, Sent: Saturday, February 11, 2023 7:31 AM
> 
> On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> 
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> 
> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x)		(0x01bc + (x))
> 
> Doesn't appear to be used; please remove it.

Oops. I'll remove it.

Best regards,
Yoshihiro Shimoda

RE: [PATCH v9 2/8] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Monday, February 13, 2023 6:11 AM
> 
> On Fri, Feb 10, 2023 at 10:49:11PM +0900, Yoshihiro Shimoda wrote:
> > Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> > PCIe endpoint module.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../bindings/pci/rcar-gen4-pci-ep.yaml        | 90 +++++++++++++++++++
> >  1 file changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> > new file mode 100644
> > index 000000000000..4b10d67e4336
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> > @@ -0,0 +1,90 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id:
<snip URL>
> > +$schema:
<snip URL>
> > +
> > +title: Renesas R-Car Gen4 PCIe Endpoint
> > +
> > +maintainers:
> > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +allOf:
> > +  - $ref: snps,dw-pcie-ep.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: renesas,r8a779f0-pcie-ep   # R-Car S4-8
> > +      - const: renesas,rcar-gen4-pcie-ep  # R-Car Gen4
> > +
> > +  reg:
> > +    maxItems: 4
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: atu
> 
> > +      - const: appl
> 
> Please, use "elbi" or "app" instead. No need in using the
> vendor-specific names if there is the generic ones.
> (* @Rob, that's why I was insisting in failing the DT-bindings
> evaluation for such cases...)
> See Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml:98

I got it. I'll fix it.

> > +      - const: addr_space
> > +
> > +  interrupts:
> > +    maxItems: 3
> > +
> > +  interrupt-names:
> > +    items:
> 
> > +      - const: dma
> 
> Are you sure there is a single IRQ line for all eDMA channels?

Yes.

> Judging by the DW PCIe HW manual the eDMA events are signaled by the
> wires: edma_int[((CC_NUM_DMA_RD_CHAN+CC_NUM_DMA_WR_CHAN)-1):0]. If you
> have a single signal then they must have been combined on the way to
> the GIC though...

I think so.
The drivers/dma/dw-edma/dw-edma-core.c seems to support nr_irqs == 1.

> > +      - const: sft_ce
> > +      - const: app
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> 
> > +  max-link-speed: true
> 
> This prop is determined by the CX_MAX_PCIE_SPEED IP-core synthesize
> parameter.
> 
> > +
> > +  num-lanes: true
> 
> This is determined by the CX_NL IP-core synthesize parameter.
> 
> Thus you can provide at least the 'maximum' constraint for
> the properties above.

I'll add 'maximum' to each property.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - resets
> > +  - power-domains
> > +  - clocks
> > +
> > +additionalProperties: false
> 
> Are you sure that none of the next properties will be ever used in
> the R-Car PCIe End-point DT-nodes?
> max-functions
> max-virtual-functions
> phys
> phy-names
> reset-gpios
> snps,enable-cdm-check
> dma-coherent
> * etc
> 
> I am pretty much sure that the reset-gpios (platform-specific),
> max-{virtual-}functions (determined by the CX_NFUNC IP-core synthesize
> parameter), phys/phy-names (you had a PHYs CSR space in the
> DT-bindings example) and dma-coherent properties are relevant for your
> device. At the very least the 'max-functions' prop constraint could be
> explicitly added to your DT-bindings file. You must be aware of how
> many functions the R-Car PCIe EP support, right?

The R-Car PCIe EP supports two functions. So, I'll add 'max-functions'
property with 'maximum: 2'.

> The rest of the
> properties could be implicitly evaluated by replacing the
> "additionalProperties: false" flag with the "unevaluatedProperties:
> false" statement.

I got it. I'll change "additionalProperties: false" to 
"unevaluatedProperties: false".

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/power/r8a779f0-sysc.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pcie0_ep: pcie-ep@e65d0000 {
> > +            compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
> > +            reg = <0 0xe65d0000 0 0x1000>, <0 0xe65d1000 0 0x1000>,
> > +                  <0 0xe65d3000 0 0x2000>, <0 0xfe000000 0 0x400000>;
> > +            reg-names = "dbi", "atu", "appl", "addr_space";
> > +            interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "dma", "sft_ce", "app";
> > +            clocks = <&cpg CPG_MOD 624>;
> > +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > +            resets = <&cpg 624>;
> > +            num-lanes = <2>;
> > +            max-link-speed = <2>;
> > +        };
> > +    };
> > --
> > 2.25.1
> >
> >

Re: [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs

[Serge Semin]

On Fri, Feb 10, 2023 at 10:49:14PM +0900, Yoshihiro Shimoda wrote:
> Add support for triggering legacy IRQs by using outbound ATU.

I supposed a little more details would nice, like outbound iATU is
utilized to send assert and de-assert INTx TLPs. The message is
generated based on the payloadless Msg TLP with type 0x14, where 0x4
is the routing code implying the terminated at Receiver message. The
message code is specified as b1000xx for the INTx assertion and
b1001xx for the INTx de-assertion. Etc...

> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../pci/controller/dwc/pcie-designware-ep.c   | 66 +++++++++++++++++--
>  drivers/pci/controller/dwc/pcie-designware.c  | 25 ++++---
>  drivers/pci/controller/dwc/pcie-designware.h  | 12 +++-
>  3 files changed, 87 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 95efe14f1036..886483bf378b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -6,6 +6,7 @@
>   * Author: Kishon Vijay Abraham I <kishon@ti.com>
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  
> @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
>  	return 0;
>  }
>  
> -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> -				   phys_addr_t phys_addr,
> +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> +				   u8 code, u8 routing, phys_addr_t phys_addr,
>  				   u64 pci_addr, size_t size)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
>  		return -EINVAL;
>  	}
>  
> -	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> -					   phys_addr, pci_addr, size);
> +	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> +					   code, routing, phys_addr, pci_addr,
> +					   size);
>  	if (ret)
>  		return ret;
>  
> @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> -	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> +				      addr, pci_addr, size);
>  	if (ret) {
>  		dev_err(pci->dev, "Failed to enable address\n");
>  		return ret;
> @@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
>  	.get_features		= dw_pcie_ep_get_features,
>  };
>  
> +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> +			       u8 routing)
> +{
> +	struct pci_epc *epc = ep->epc;
> +	int ret;
> +
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> +				      routing, ep->intx_mem_phys, 0,
> +				      epc->mem->window.page_size);
> +	if (ret)
> +		return ret;
> +	writel(0, ep->intx_mem);
> +	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> +
> +	return 0;
> +}
> +
> +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> +					 int intx)
> +{
> +	int ret;
> +
> +	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
> +	if (ret)
> +		return ret;
> +	usleep_range(1000, 2000);
> +	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> +}
> +
>  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct device *dev = pci->dev;
>  

> +	if (ep->intx_by_atu)

IMO the flag is redundant. Your implementation is generic enough to be
useful for all the DW PCIe EPs. If you are afraid to break things,
then just make it optional. If no outbound physical memory could be
allocated then initialize intx_mem with NULL and move on with further
initializations. As before the Legacy IRQ raise method shall return
EINVAL in that case.

> +		return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
> +
>  	dev_err(dev, "EP cannot trigger legacy IRQs\n");
>  
>  	return -EINVAL;
> @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  
>  	dw_pcie_edma_remove(pci);
>  
> +	if (ep->intx_by_atu)
> +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> +				      epc->mem->window.page_size);
> +
>  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
>  			      epc->mem->window.page_size);
>  
> @@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  		goto err_exit_epc_mem;
>  	}
>  

> +	if (ep->intx_by_atu) {
> +		ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> +						      epc->mem->window.page_size);
> +		if (!ep->intx_mem) {
> +			ret = -ENOMEM;
> +			dev_err(dev, "Failed to reserve memory for INTx\n");

As I suggested above you can just dev_warn() here and move on with
EP initialization.

> +			goto err_free_epc_mem;
> +		}
> +	}
> +
>  	ret = dw_pcie_edma_detect(pci);
>  	if (ret)
> -		goto err_free_epc_mem;
> +		goto err_free_epc_mem_intx;
>  
>  	if (ep->ops->get_features) {
>  		epc_features = ep->ops->get_features(ep);
> @@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  err_remove_edma:
>  	dw_pcie_edma_remove(pci);
>  
> +err_free_epc_mem_intx:
> +	if (ep->intx_by_atu)
> +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> +				      epc->mem->window.page_size);
> +
>  err_free_epc_mem:
>  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
>  			      epc->mem->window.page_size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..b4315cf84340 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
>  }
>  

>  static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> -				       int index, int type, u64 cpu_addr,
> -				       u64 pci_addr, u64 size)
> +				       int index, int type, u8 code, u8 routing,
> +				       u64 cpu_addr, u64 pci_addr, u64 size)

The implementation gets to be too complicated especially with having
nine arguments now. Seven args have been not that good either, but at
the very least it was more-or-less coherent with respect to the Mem/IO
outbound TLPs generations. Now in addition to that it will be also
responsible for the MSG TLPs mapping. What we could simplify here is:

< either 1. Drop separate routing arg. It can be merged into the type
argument seeing it's a part of one by specification. Thus we'll have
only eight arguments left. That will simplify the prototype, but not
the implementation.

< or 2. Split up the outbound mem/IO and MSG windows setups.
In the later case you'll need only the next data for the MSG TLPs
mapping: function #, MW index, message code, CPU base address, MW size.

< or 3. Convert __dw_pcie_prog_outbound_atu() to accepting a
dw_pcie_outbound_atu(-ish) structure with all the arguments listed as
fields: MW index, function #, message type, routing type, message
code (valid if MSG type specified), CPU base address, PCIe base
address, MW size.

What do you think? @Rob, @Bjorn?

(Please don't forget to define the macros for the routing and message
code values.)

>  {
>  	u32 retries, val;
>  	u64 limit_addr;
> @@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
>  			      upper_32_bits(pci_addr));
>  
> -	val = type | PCIE_ATU_FUNC_NUM(func_no);
> +	val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
>  	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
>  	    dw_pcie_ver_is_ge(pci, 460A))
>  		val |= PCIE_ATU_INCREASE_REGION_SIZE;
> @@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  		val = dw_pcie_enable_ecrc(val);
>  	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
>  
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);

> +	if (code)
> +		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> +				      PCIE_ATU_ENABLE |
> +				      PCIE_ATU_INHIBIT_PAYLOAD |
> +				      PCIE_ATU_HEADER_SUB_ENABLE | code);

AFAICS the setup above is only specific to the Msg TLPs. If so then it
would have been more generic to use if(type == ?) conditional
statement here. What do you think?

> +	else
> +		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> +				      PCIE_ATU_ENABLE);

The next modification seems to be looking better in this case:
+ val = PCIE_ATU_ENABLE;
+ if (type == PCIE_ATU_TYPE_MSG)
+ 	val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | code;
+ 
+ dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, val);

-Serge(y)

>  
>  	/*
>  	 * Make sure ATU enable takes effect before any subsequent config
> @@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			      u64 cpu_addr, u64 pci_addr, u64 size)
>  {
> -	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> +	return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
>  					   cpu_addr, pci_addr, size);
>  }
>  
>  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				 int type, u64 cpu_addr, u64 pci_addr,
> -				 u64 size)
> +				 int type, u8 code, u8 routing, u64 cpu_addr,
> +				 u64 pci_addr, u64 size)
>  {
> -	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> -					   cpu_addr, pci_addr, size);
> +	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
> +					   routing, cpu_addr, pci_addr, size);
>  }
>  
>  static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 79713ce075cc..1a6e7e9489ee 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -147,11 +147,14 @@
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
>  #define PCIE_ATU_TYPE_CFG1		0x5
> +#define PCIE_ATU_TYPE_MSG		0x10
>  #define PCIE_ATU_TD			BIT(8)
>  #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
>  #define PCIE_ATU_REGION_CTRL2		0x004
>  #define PCIE_ATU_ENABLE			BIT(31)
>  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> +#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
> +#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
>  #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
>  #define PCIE_ATU_LOWER_BASE		0x008
>  #define PCIE_ATU_UPPER_BASE		0x00C
> @@ -244,6 +247,9 @@
>  /* Default eDMA LLP memory size */
>  #define DMA_LLP_MEM_SIZE		PAGE_SIZE
>  
> +#define PCIE_MSG_ASSERT_INTA		0x20
> +#define PCIE_MSG_DEASSERT_INTA		0x24
> +
>  struct dw_pcie;
>  struct dw_pcie_rp;
>  struct dw_pcie_ep;
> @@ -352,7 +358,10 @@ struct dw_pcie_ep {
>  	unsigned long		*ob_window_map;
>  	void __iomem		*msi_mem;
>  	phys_addr_t		msi_mem_phys;
> +	void __iomem		*intx_mem;
> +	phys_addr_t		intx_mem_phys;
>  	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
> +	bool			intx_by_atu;
>  };
>  
>  struct dw_pcie_ops {
> @@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			      u64 cpu_addr, u64 pci_addr, u64 size);
>  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
> +				 int type, u8 code, u8 routing, u64 cpu_addr,
> +				 u64 pci_addr, u64 size);
>  int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
>  			     u64 cpu_addr, u64 pci_addr, u64 size);
>  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -- 
> 2.25.1
> 
> 

Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Serge Semin]

On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
> 
> This controller doesn't support MSI-X interrupt. So, introduce
> no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> from dw_pcie_msi_domain_info.
> 

> Note that this controller on R-Car S4-8 has an unexpected register
> value on the dbi+0x97b register. So, add a new capability flag
> which would force the unrolled eDMA mapping for the problematic
> device, as suggested by Serge Semin.

Please move the noted fix to a separate pre-requisite patch seeing your
update depends on it. Thus this patch will be a bit simpler to review
with no harm to the changes atomicity.

> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |   9 +
>  drivers/pci/controller/dwc/Makefile           |   2 +
>  .../pci/controller/dwc/pcie-designware-host.c |   3 +
>  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
>  drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
>  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
>  8 files changed, 419 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 434f6a4f4041..94805ec31a8f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -414,4 +414,13 @@ config PCIE_FU740
>  	  Say Y here if you want PCIe controller support for the SiFive
>  	  FU740.
>  
> +config PCIE_RCAR_GEN4
> +	tristate "Renesas R-Car Gen4 PCIe Host controller"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> +	  This uses the DesignWare core.
> +
>  endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bf5c311875a1..486cf706b53d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 9952057c8819..5aefeec15c9b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>  
>  	irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
>  

> +	if (pp->no_msix)
> +		dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;

Changing static data field in the probe path doesn't seem like
correct. Is it really that necessary to clear out that flag? The rest
of the LLDDs don't seem to be bothered with that (mine included). On
the other hand it seems to me that the iMSI-RX engine doesn't really
support true MSI-X messages. It always stick to a single base address
with up to 256 IRQs in total and up to 32 IRQs per function. Do you
suggest to drop that flag then for all DW PCIe hosts or just for ones
with iMSI-RX engine in-use or just forget about it?

@Rob, @Bjorn?

> +
>  	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
>  						   &dw_pcie_msi_domain_info,
>  						   pp->irq_domain);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index b4315cf84340..7d649ba387f8 100644

> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  	 * Indirect eDMA CSRs access has been completely removed since v5.40a
>  	 * thus no space is now reserved for the eDMA channels viewport and
>  	 * former DMA CTRL register is no longer fixed to FFs.
> +	 *
> +	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
> +	 * register even though the HW-manual explicitly states there must FFs
> +	 * if the unrolled mapping is enabled. For such cases the low-level
> +	 * drivers are supposed to manually activate the unrolled mapping to
> +	 * bypass the auto-detection procedure.
>  	 */
> -	if (dw_pcie_ver_is_ge(pci, 540A))
> +	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
>  		val = 0xFFFFFFFF;
>  	else
>  		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);

As I suggested in the head of this message please move this to a
separate pre-requisite patch.

> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 1a6e7e9489ee..1b1af514b849 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -51,8 +51,9 @@
>  
>  /* DWC PCIe controller capabilities */
>  #define DW_PCIE_CAP_REQ_RES		0

> -#define DW_PCIE_CAP_IATU_UNROLL		1
> -#define DW_PCIE_CAP_CDM_CHECK		2
> +#define DW_PCIE_CAP_EDMA_UNROLL		1
> +#define DW_PCIE_CAP_IATU_UNROLL		2
> +#define DW_PCIE_CAP_CDM_CHECK		3

ditto

>  
>  #define dw_pcie_cap_is(_pci, _cap) \
>  	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
>  struct dw_pcie_rp {
>  	bool			has_msi_ctrl:1;
>  	bool			cfg0_io_shared:1;
> +	bool			no_msix:1;
>  	u64			cfg0_base;
>  	void __iomem		*va_cfg0_base;
>  	u32			cfg0_size;
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> new file mode 100644
> index 000000000000..d60f4895ffe9
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	int ret;
> +	u32 val;
> +
> +	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> +	if (ret < 0)
> +		return ret;
> +
> +	dw_pcie_dbi_ro_wr_en(dw);
> +
> +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> +	dw_pcie_dbi_ro_wr_dis(dw);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		/* Enable MSI interrupt signal */
> +		val = readl(rcar->base + PCIEINTSTS0EN);
> +		val |= MSI_CTRL_INT;
> +		writel(val, rcar->base + PCIEINTSTS0EN);
> +	}
> +
> +	gpiod_set_value_cansleep(dw->pe_rst, 0);
> +

> +	dw_pcie_setup_rc(pp);
> +
> +	dw_pcie_dbi_ro_wr_en(dw);
> +	rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> +	dw_pcie_dbi_ro_wr_dis(dw);
> +
> +	if (!dw_pcie_link_up(dw)) {
> +		ret = dw->ops->start_link(dw);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Ignore errors, the link may come up later */
> +	if (dw_pcie_wait_for_link(dw))
> +		dev_info(dw->dev, "PCIe link down\n");

This whole procedure is done in the dw_pcie_host_init() method (@Rob
already noted that a bit earlier). The only exception is the
rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
the generic code (see further for more details).

> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> +	.host_init = rcar_gen4_pcie_host_init,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> +				   struct platform_device *pdev)
> +{
> +	struct dw_pcie *dw = &rcar->dw;
> +	struct dw_pcie_rp *pp = &dw->pp;
> +	int ret;
> +
> +	pp->ops = &rcar_gen4_pcie_host_ops;
> +	pp->no_msix = true;
> +	dw_pcie_cap_set(dw, REQ_RES);
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> +{
> +	dw_pcie_host_deinit(&rcar->dw.pp);
> +	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> +}
> +

> +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> +					struct platform_device *pdev)
> +{
> +	struct dw_pcie *dw = &rcar->dw;
> +
> +	/* Renesas-specific registers */
> +	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> +	if (IS_ERR(rcar->base))
> +		return PTR_ERR(rcar->base);
> +
> +	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> +}

Please note that after fixing the RCAR PCIe EP DT-bindings to permit
the "app" instead of "appl" named reg property (see my note the RCAR
PCie EP DT-bindings patch) the method above will be identical in the
RCAR RP and EP driver implementations. So you'll be able to move it
to the pcie-rcar-gen4.c driver.

> +
> +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen4_pcie *rcar;
> +	int err;
> +
> +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> +	if (!rcar)
> +		return -ENOMEM;
> +
> +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> +	if (err < 0) {
> +		dev_err(dev, "Failed to request resource: %d\n", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, rcar);
> +
> +	err = rcar_gen4_pcie_prepare(rcar);
> +	if (err < 0)
> +		return err;
> +
> +	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> +	if (err < 0)
> +		goto err_add;
> +
> +	return 0;
> +
> +err_add:
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return err;
> +}
> +
> +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> +{
> +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> +	rcar_gen4_remove_dw_pcie_rp(rcar);
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> +	{ .compatible = "renesas,rcar-gen4-pcie", },
> +	{},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_driver = {
> +	.driver = {
> +		.name = "pcie-rcar-gen4",
> +		.of_match_table = rcar_gen4_pcie_of_match,
> +	},
> +	.probe = rcar_gen4_pcie_probe,
> +	.remove = rcar_gen4_pcie_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> new file mode 100644
> index 000000000000..a6a29d6125c8
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Renesas-specific */
> +#define PCIERSTCTRL1		0x0014
> +#define  APP_HOLD_PHY_RST	BIT(16)
> +#define  APP_LTSSM_ENABLE	BIT(0)
> +
> +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> +					bool enable)
> +{
> +	u32 val;
> +
> +	val = readl(rcar->base + PCIERSTCTRL1);
> +	if (enable) {
> +		val |= APP_LTSSM_ENABLE;
> +		val &= ~APP_HOLD_PHY_RST;
> +	} else {
> +		val &= ~APP_LTSSM_ENABLE;
> +		val |= APP_HOLD_PHY_RST;
> +	}
> +	writel(val, rcar->base + PCIERSTCTRL1);
> +}
> +
> +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	u32 val, mask;
> +
> +	val = readl(rcar->base + PCIEINTSTS0);
> +	mask = RDLH_LINK_UP | SMLH_LINK_UP;
> +
> +	return (val & mask) == mask;
> +}
> +
> +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	rcar_gen4_pcie_ltssm_enable(rcar, true);
> +
> +	return 0;
> +}
> +
> +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	rcar_gen4_pcie_ltssm_enable(rcar, false);
> +}
> +
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +				   int num_lanes)
> +{
> +	u32 val;
> +
> +	/* Note: Assume the reset is asserted here */
> +	val = readl(rcar->base + PCIEMSR0);
> +	if (rc)
> +		val |= DEVICE_TYPE_RC;
> +	else
> +		val |= DEVICE_TYPE_EP;
> +	if (num_lanes < 4)
> +		val |= BIFUR_MOD_SET_ON;
> +	writel(val, rcar->base + PCIEMSR0);
> +
> +	return reset_control_deassert(rcar->rst);
> +}
> +

> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> +{
> +	dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> +}
> +

Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
driver assumes that the DBI2 is defined within the 0x1000 offset with
respect to the DBI space (see dw_pcie_get_resources()). In your case
it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
What about either defining the "dbi2" reg-named DT-property in the
DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
the dw_pcie.dbi_base2 field in your LLDD?

Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
only in the framework of the BARs mapping setup procedure. Are you
sure it's working well in your case seeing the DBI2 base address is
most likely incorrectly initialized? Anyway if you get to define the
dbi_base2 field properly you'll be able to use the
dw_pcie_ep_reset_bar() method in the EP driver instead of the function
above (please see the way it's utilized in the rest of the DW PCIe EP
low-level drivers).

Another question. Are you sure the method above is relevant to the DW
PCIe Root Port controller? I don't see any shadow registers defined
for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
5.30, 5.40). What are they used for anyway?

> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> +{
> +	u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> +
> +	val &= ~PCI_EXP_LNKCAP_MLW;
> +	switch (num_lanes) {
> +	case 1:
> +		val |= PCI_EXP_LNKCAP_MLW_X1;
> +		break;
> +	case 2:
> +		val |= PCI_EXP_LNKCAP_MLW_X2;
> +		break;
> +	case 4:
> +		val |= PCI_EXP_LNKCAP_MLW_X4;
> +		break;
> +	default:
> +		dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> +		val |= PCI_EXP_LNKCAP_MLW_X1;
> +		break;
> +	}
> +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> +}

This seems to be a generic action. What about moving it to 
pcie-designware.c implementing a function similar to
dw_pcie_link_set_max_speed(), which would be called in
dw_pcie_setup()?
It could be named as dw_pcie_link_set_max_(link_)?width() and besides
of the code above would consist of the PCIE_PORT_LINK_CONTROL and
PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
dw_pcie_setup().

* Please do that in a separate pre-requisite patch.

> +
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> +{
> +	struct device *dev = rcar->dw.dev;
> +	int err;
> +
> +	pm_runtime_enable(dev);
> +	err = pm_runtime_resume_and_get(dev);
> +	if (err < 0) {
> +		dev_err(dev, "Failed to resume/get Runtime PM\n");
> +		pm_runtime_disable(dev);
> +	}
> +
> +	return err;
> +}
> +
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> +{
> +	struct device *dev = rcar->dw.dev;
> +
> +	if (!reset_control_status(rcar->rst))
> +		reset_control_assert(rcar->rst);
> +	pm_runtime_put(dev);
> +	pm_runtime_disable(dev);
> +}
> +
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> +				  struct device *dev)
> +{
> +	rcar->rst = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(rcar->rst)) {
> +		dev_err(dev, "Failed to get Cold-reset\n");
> +		return PTR_ERR(rcar->rst);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = rcar_gen4_pcie_start_link,
> +	.stop_link = rcar_gen4_pcie_stop_link,
> +	.link_up = rcar_gen4_pcie_link_up,
> +};
> +
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar;
> +
> +	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> +	if (!rcar)
> +		return NULL;
> +
> +	rcar->dw.dev = dev;
> +	rcar->dw.ops = &dw_pcie_ops;
> +	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> +
> +	return rcar;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> new file mode 100644
> index 000000000000..1cdce0cf7dac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#ifndef _PCIE_RCAR_GEN4_H_
> +#define _PCIE_RCAR_GEN4_H_
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +

> +/* PCI Express capability */
> +#define EXPCAP(x)		(0x0070 + (x))
> +/* ASPM L1 PM Substates */
> +#define L1PSCAP(x)		(0x01bc + (x))
> +/* PCI Shadow offset */
> +#define SHADOW_REG(x)		(0x2000 + (x))
> +/* BAR Mask registers */
> +#define BAR0MASKF		0x0010
> +#define BAR1MASKF		0x0014
> +#define BAR2MASKF		0x0018
> +#define BAR3MASKF		0x001c
> +#define BAR4MASKF		0x0020
> +#define BAR5MASKF		0x0024
> +

* If you get to take some of my notes above into account you'll be able to
drop these macros...

> +/* Renesas-specific */
> +#define PCIEMSR0		0x0000
> +#define  BIFUR_MOD_SET_ON	BIT(0)
> +#define  DEVICE_TYPE_EP		0
> +#define  DEVICE_TYPE_RC		BIT(4)
> +
> +#define PCIEINTSTS0		0x0084
> +#define PCIEINTSTS0EN		0x0310
> +#define  MSI_CTRL_INT		BIT(26)
> +#define  SMLH_LINK_UP		BIT(7)
> +#define  RDLH_LINK_UP		BIT(6)
> +#define PCIEDMAINTSTSEN		0x0314
> +#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)

Vendor-specific prefix would make the code using these macros much
more readable.

> +
> +struct rcar_gen4_pcie {
> +	struct dw_pcie		dw;
> +	void __iomem		*base;
> +	struct reset_control	*rst;
> +};
> +#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
> +

> +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);

I don't see these two methods being defined in your driver. Are these
artefacts from the previous patch revisions?

-Serge(y)

> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +				   int num_lanes);

> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> +				  struct device *dev);
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> +
> +#endif /* _PCIE_RCAR_GEN4_H_ */
> -- 
> 2.25.1
> 
> 

RE: [PATCH v9 5/8] PCI: dwc: Add support for triggering legacy IRQs

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Thursday, February 16, 2023 12:19 AM
> 
> On Fri, Feb 10, 2023 at 10:49:14PM +0900, Yoshihiro Shimoda wrote:
> > Add support for triggering legacy IRQs by using outbound ATU.
> 
> I supposed a little more details would nice, like outbound iATU is
> utilized to send assert and de-assert INTx TLPs. The message is
> generated based on the payloadless Msg TLP with type 0x14, where 0x4
> is the routing code implying the terminated at Receiver message. The
> message code is specified as b1000xx for the INTx assertion and
> b1001xx for the INTx de-assertion. Etc...

Thank you for your comments. The descriptions are very nice to me.
So, I'll add the descriptions.

> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../pci/controller/dwc/pcie-designware-ep.c   | 66 +++++++++++++++++--
> >  drivers/pci/controller/dwc/pcie-designware.c  | 25 ++++---
> >  drivers/pci/controller/dwc/pcie-designware.h  | 12 +++-
> >  3 files changed, 87 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 95efe14f1036..886483bf378b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -6,6 +6,7 @@
> >   * Author: Kishon Vijay Abraham I <kishon@ti.com>
> >   */
> >
> > +#include <linux/delay.h>
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> >
> > @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> >  	return 0;
> >  }
> >
> > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > -				   phys_addr_t phys_addr,
> > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> > +				   u8 code, u8 routing, phys_addr_t phys_addr,
> >  				   u64 pci_addr, size_t size)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> >  		return -EINVAL;
> >  	}
> >
> > -	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> > -					   phys_addr, pci_addr, size);
> > +	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> > +					   code, routing, phys_addr, pci_addr,
> > +					   size);
> >  	if (ret)
> >  		return ret;
> >
> > @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > -	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> > +				      addr, pci_addr, size);
> >  	if (ret) {
> >  		dev_err(pci->dev, "Failed to enable address\n");
> >  		return ret;
> > @@ -479,11 +482,43 @@ static const struct pci_epc_ops epc_ops = {
> >  	.get_features		= dw_pcie_ep_get_features,
> >  };
> >
> > +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> > +			       u8 routing)
> > +{
> > +	struct pci_epc *epc = ep->epc;
> > +	int ret;
> > +
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> > +				      routing, ep->intx_mem_phys, 0,
> > +				      epc->mem->window.page_size);
> > +	if (ret)
> > +		return ret;
> > +	writel(0, ep->intx_mem);
> > +	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> > +					 int intx)
> > +{
> > +	int ret;
> > +
> > +	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
> > +	if (ret)
> > +		return ret;
> > +	usleep_range(1000, 2000);
> > +	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> > +}
> > +
> >  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	struct device *dev = pci->dev;
> >
> 
> > +	if (ep->intx_by_atu)
> 
> IMO the flag is redundant. Your implementation is generic enough to be
> useful for all the DW PCIe EPs. If you are afraid to break things,

Yes, I'm afraid to break things.

> then just make it optional. If no outbound physical memory could be
> allocated then initialize intx_mem with NULL and move on with further
> initializations. As before the Legacy IRQ raise method shall return
> EINVAL in that case.

I got it. I'll modify this patch like that.

> > +		return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
> > +
> >  	dev_err(dev, "EP cannot trigger legacy IRQs\n");
> >
> >  	return -EINVAL;
> > @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> >
> >  	dw_pcie_edma_remove(pci);
> >
> > +	if (ep->intx_by_atu)
> > +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > +				      epc->mem->window.page_size);
> > +
> >  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> >  			      epc->mem->window.page_size);
> >
> > @@ -789,9 +828,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  		goto err_exit_epc_mem;
> >  	}
> >
> 
> > +	if (ep->intx_by_atu) {
> > +		ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> > +						      epc->mem->window.page_size);
> > +		if (!ep->intx_mem) {
> > +			ret = -ENOMEM;
> > +			dev_err(dev, "Failed to reserve memory for INTx\n");
> 
> As I suggested above you can just dev_warn() here and move on with
> EP initialization.

I got it.

> > +			goto err_free_epc_mem;
> > +		}
> > +	}
> > +
> >  	ret = dw_pcie_edma_detect(pci);
> >  	if (ret)
> > -		goto err_free_epc_mem;
> > +		goto err_free_epc_mem_intx;
> >
> >  	if (ep->ops->get_features) {
> >  		epc_features = ep->ops->get_features(ep);
> > @@ -808,6 +857,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  err_remove_edma:
> >  	dw_pcie_edma_remove(pci);
> >
> > +err_free_epc_mem_intx:
> > +	if (ep->intx_by_atu)
> > +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > +				      epc->mem->window.page_size);
> > +
> >  err_free_epc_mem:
> >  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> >  			      epc->mem->window.page_size);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 53a16b8b6ac2..b4315cf84340 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -465,8 +465,8 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
> >  }
> >
> 
> >  static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > -				       int index, int type, u64 cpu_addr,
> > -				       u64 pci_addr, u64 size)
> > +				       int index, int type, u8 code, u8 routing,
> > +				       u64 cpu_addr, u64 pci_addr, u64 size)
> 
> The implementation gets to be too complicated especially with having
> nine arguments now. Seven args have been not that good either, but at
> the very least it was more-or-less coherent with respect to the Mem/IO
> outbound TLPs generations. Now in addition to that it will be also
> responsible for the MSG TLPs mapping. What we could simplify here is:
> 
> < either 1. Drop separate routing arg. It can be merged into the type
> argument seeing it's a part of one by specification. Thus we'll have
> only eight arguments left. That will simplify the prototype, but not
> the implementation.
> 
> < or 2. Split up the outbound mem/IO and MSG windows setups.
> In the later case you'll need only the next data for the MSG TLPs
> mapping: function #, MW index, message code, CPU base address, MW size.
> 
> < or 3. Convert __dw_pcie_prog_outbound_atu() to accepting a
> dw_pcie_outbound_atu(-ish) structure with all the arguments listed as
> fields: MW index, function #, message type, routing type, message
> code (valid if MSG type specified), CPU base address, PCIe base
> address, MW size.
> 
> What do you think? @Rob, @Bjorn?

I don't have a strong opinion, but I prefer the 3 above.

> (Please don't forget to define the macros for the routing and message
> code values.)
> 
> >  {
> >  	u32 retries, val;
> >  	u64 limit_addr;
> > @@ -498,7 +498,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >  	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
> >  			      upper_32_bits(pci_addr));
> >
> > -	val = type | PCIE_ATU_FUNC_NUM(func_no);
> > +	val = type | routing | PCIE_ATU_FUNC_NUM(func_no);
> >  	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
> >  	    dw_pcie_ver_is_ge(pci, 460A))
> >  		val |= PCIE_ATU_INCREASE_REGION_SIZE;
> > @@ -506,7 +506,14 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >  		val = dw_pcie_enable_ecrc(val);
> >  	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
> >
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
> 
> > +	if (code)
> > +		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> > +				      PCIE_ATU_ENABLE |
> > +				      PCIE_ATU_INHIBIT_PAYLOAD |
> > +				      PCIE_ATU_HEADER_SUB_ENABLE | code);
> 
> AFAICS the setup above is only specific to the Msg TLPs. If so then it
> would have been more generic to use if(type == ?) conditional
> statement here. What do you think?
> 
> > +	else
> > +		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2,
> > +				      PCIE_ATU_ENABLE);
> 
> The next modification seems to be looking better in this case:
> + val = PCIE_ATU_ENABLE;
> + if (type == PCIE_ATU_TYPE_MSG)
> + 	val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | code;
> +
> + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, val);

Thank you for the suggestion. It looks better than my patch.
So, I'll modify it.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> >
> >  	/*
> >  	 * Make sure ATU enable takes effect before any subsequent config
> > @@ -528,16 +535,16 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			      u64 cpu_addr, u64 pci_addr, u64 size)
> >  {
> > -	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> > +	return __dw_pcie_prog_outbound_atu(pci, 0, index, type, 0, 0,
> >  					   cpu_addr, pci_addr, size);
> >  }
> >
> >  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > -				 int type, u64 cpu_addr, u64 pci_addr,
> > -				 u64 size)
> > +				 int type, u8 code, u8 routing, u64 cpu_addr,
> > +				 u64 pci_addr, u64 size)
> >  {
> > -	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> > -					   cpu_addr, pci_addr, size);
> > +	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, code,
> > +					   routing, cpu_addr, pci_addr, size);
> >  }
> >
> >  static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 79713ce075cc..1a6e7e9489ee 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -147,11 +147,14 @@
> >  #define PCIE_ATU_TYPE_IO		0x2
> >  #define PCIE_ATU_TYPE_CFG0		0x4
> >  #define PCIE_ATU_TYPE_CFG1		0x5
> > +#define PCIE_ATU_TYPE_MSG		0x10
> >  #define PCIE_ATU_TD			BIT(8)
> >  #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
> >  #define PCIE_ATU_REGION_CTRL2		0x004
> >  #define PCIE_ATU_ENABLE			BIT(31)
> >  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> > +#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
> > +#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
> >  #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
> >  #define PCIE_ATU_LOWER_BASE		0x008
> >  #define PCIE_ATU_UPPER_BASE		0x00C
> > @@ -244,6 +247,9 @@
> >  /* Default eDMA LLP memory size */
> >  #define DMA_LLP_MEM_SIZE		PAGE_SIZE
> >
> > +#define PCIE_MSG_ASSERT_INTA		0x20
> > +#define PCIE_MSG_DEASSERT_INTA		0x24
> > +
> >  struct dw_pcie;
> >  struct dw_pcie_rp;
> >  struct dw_pcie_ep;
> > @@ -352,7 +358,10 @@ struct dw_pcie_ep {
> >  	unsigned long		*ob_window_map;
> >  	void __iomem		*msi_mem;
> >  	phys_addr_t		msi_mem_phys;
> > +	void __iomem		*intx_mem;
> > +	phys_addr_t		intx_mem_phys;
> >  	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
> > +	bool			intx_by_atu;
> >  };
> >
> >  struct dw_pcie_ops {
> > @@ -419,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> >  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			      u64 cpu_addr, u64 pci_addr, u64 size);
> >  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > -				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
> > +				 int type, u8 code, u8 routing, u64 cpu_addr,
> > +				 u64 pci_addr, u64 size);
> >  int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
> >  			     u64 cpu_addr, u64 pci_addr, u64 size);
> >  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > --
> > 2.25.1
> >
> >

RE: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Thursday, February 16, 2023 3:33 AM
> 
> On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller doesn't support MSI-X interrupt. So, introduce
> > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > from dw_pcie_msi_domain_info.
> >
> 
> > Note that this controller on R-Car S4-8 has an unexpected register
> > value on the dbi+0x97b register. So, add a new capability flag
> > which would force the unrolled eDMA mapping for the problematic
> > device, as suggested by Serge Semin.
> 
> Please move the noted fix to a separate pre-requisite patch seeing your
> update depends on it. Thus this patch will be a bit simpler to review
> with no harm to the changes atomicity.

There is other patch though, I think Lorenzo prefers non separated patch:
https://lore.kernel.org/all/Y245ZtqqqelXif+Y@lpieralisi/

But, Lorenzo, what do you think?

> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig            |   9 +
> >  drivers/pci/controller/dwc/Makefile           |   2 +
> >  .../pci/controller/dwc/pcie-designware-host.c |   3 +
> >  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
> >  drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
> >  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
> >  8 files changed, 419 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 434f6a4f4041..94805ec31a8f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -414,4 +414,13 @@ config PCIE_FU740
> >  	  Say Y here if you want PCIe controller support for the SiFive
> >  	  FU740.
> >
> > +config PCIE_RCAR_GEN4
> > +	tristate "Renesas R-Car Gen4 PCIe Host controller"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	depends on PCI_MSI
> > +	select PCIE_DW_HOST
> > +	help
> > +	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > +	  This uses the DesignWare core.
> > +
> >  endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index bf5c311875a1..486cf706b53d 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> >  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> >  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> >  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> >
> >  # The following drivers are for devices that use the generic ACPI
> >  # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9952057c8819..5aefeec15c9b 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> >
> >  	irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> >
> 
> > +	if (pp->no_msix)
> > +		dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> 
> Changing static data field in the probe path doesn't seem like
> correct. Is it really that necessary to clear out that flag?

I didn't investigate the detail, but even if clearing this flag
disappeared, it seemed to work.

> The rest
> of the LLDDs don't seem to be bothered with that (mine included). On
> the other hand it seems to me that the iMSI-RX engine doesn't really
> support true MSI-X messages. It always stick to a single base address
> with up to 256 IRQs in total and up to 32 IRQs per function. Do you
> suggest to drop that flag then for all DW PCIe hosts or just for ones
> with iMSI-RX engine in-use or just forget about it?

I assumed almost all controller supported MSI-X, so that I suggested
to drop the MSI_FLAG_PCI_MSIX flag if needed.

> @Rob, @Bjorn?
> 
> > +
> >  	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> >  						   &dw_pcie_msi_domain_info,
> >  						   pp->irq_domain);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index b4315cf84340..7d649ba387f8 100644
> 
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  	 * Indirect eDMA CSRs access has been completely removed since v5.40a
> >  	 * thus no space is now reserved for the eDMA channels viewport and
> >  	 * former DMA CTRL register is no longer fixed to FFs.
> > +	 *
> > +	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > +	 * register even though the HW-manual explicitly states there must FFs
> > +	 * if the unrolled mapping is enabled. For such cases the low-level
> > +	 * drivers are supposed to manually activate the unrolled mapping to
> > +	 * bypass the auto-detection procedure.
> >  	 */
> > -	if (dw_pcie_ver_is_ge(pci, 540A))
> > +	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> >  		val = 0xFFFFFFFF;
> >  	else
> >  		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> 
> As I suggested in the head of this message please move this to a
> separate pre-requisite patch.

I would like to know Lorenzo's preference.

> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 1a6e7e9489ee..1b1af514b849 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -51,8 +51,9 @@
> >
> >  /* DWC PCIe controller capabilities */
> >  #define DW_PCIE_CAP_REQ_RES		0
> 
> > -#define DW_PCIE_CAP_IATU_UNROLL		1
> > -#define DW_PCIE_CAP_CDM_CHECK		2
> > +#define DW_PCIE_CAP_EDMA_UNROLL		1
> > +#define DW_PCIE_CAP_IATU_UNROLL		2
> > +#define DW_PCIE_CAP_CDM_CHECK		3
> 
> ditto
> 
> >
> >  #define dw_pcie_cap_is(_pci, _cap) \
> >  	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> >  struct dw_pcie_rp {
> >  	bool			has_msi_ctrl:1;
> >  	bool			cfg0_io_shared:1;
> > +	bool			no_msix:1;
> >  	u64			cfg0_base;
> >  	void __iomem		*va_cfg0_base;
> >  	u32			cfg0_size;
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > new file mode 100644
> > index 000000000000..d60f4895ffe9
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +	int ret;
> > +	u32 val;
> > +
> > +	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	dw_pcie_dbi_ro_wr_en(dw);
> > +
> > +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > +	dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > +		/* Enable MSI interrupt signal */
> > +		val = readl(rcar->base + PCIEINTSTS0EN);
> > +		val |= MSI_CTRL_INT;
> > +		writel(val, rcar->base + PCIEINTSTS0EN);
> > +	}
> > +
> > +	gpiod_set_value_cansleep(dw->pe_rst, 0);
> > +
> 
> > +	dw_pcie_setup_rc(pp);
> > +
> > +	dw_pcie_dbi_ro_wr_en(dw);
> > +	rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > +	dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +	if (!dw_pcie_link_up(dw)) {
> > +		ret = dw->ops->start_link(dw);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	/* Ignore errors, the link may come up later */
> > +	if (dw_pcie_wait_for_link(dw))
> > +		dev_info(dw->dev, "PCIe link down\n");
> 
> This whole procedure is done in the dw_pcie_host_init() method (@Rob
> already noted that a bit earlier). The only exception is the
> rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
> the generic code (see further for more details).

I investigated this and then I found they can be just removed.
# This is related to the following patch. After I applied the following,
# I can remove the above codes.
# https://lore.kernel.org/linux-pci/20230216092012.3256440-1-yoshihiro.shimoda.uh@renesas.com/

> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > +	.host_init = rcar_gen4_pcie_host_init,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > +				   struct platform_device *pdev)
> > +{
> > +	struct dw_pcie *dw = &rcar->dw;
> > +	struct dw_pcie_rp *pp = &dw->pp;
> > +	int ret;
> > +
> > +	pp->ops = &rcar_gen4_pcie_host_ops;
> > +	pp->no_msix = true;
> > +	dw_pcie_cap_set(dw, REQ_RES);
> > +
> > +	ret = dw_pcie_host_init(pp);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to initialize host\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > +{
> > +	dw_pcie_host_deinit(&rcar->dw.pp);
> > +	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > +}
> > +
> 
> > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > +					struct platform_device *pdev)
> > +{
> > +	struct dw_pcie *dw = &rcar->dw;
> > +
> > +	/* Renesas-specific registers */
> > +	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > +	if (IS_ERR(rcar->base))
> > +		return PTR_ERR(rcar->base);
> > +
> > +	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > +}
> 
> Please note that after fixing the RCAR PCIe EP DT-bindings to permit
> the "app" instead of "appl" named reg property (see my note the RCAR
> PCie EP DT-bindings patch) the method above will be identical in the
> RCAR RP and EP driver implementations. So you'll be able to move it
> to the pcie-rcar-gen4.c driver.

I think so. I'll modify it.

> > +
> > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct rcar_gen4_pcie *rcar;
> > +	int err;
> > +
> > +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> > +	if (!rcar)
> > +		return -ENOMEM;
> > +
> > +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > +	if (err < 0) {
> > +		dev_err(dev, "Failed to request resource: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, rcar);
> > +
> > +	err = rcar_gen4_pcie_prepare(rcar);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > +	if (err < 0)
> > +		goto err_add;
> > +
> > +	return 0;
> > +
> > +err_add:
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > +{
> > +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > +	rcar_gen4_remove_dw_pcie_rp(rcar);
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > +	{ .compatible = "renesas,rcar-gen4-pcie", },
> > +	{},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_driver = {
> > +	.driver = {
> > +		.name = "pcie-rcar-gen4",
> > +		.of_match_table = rcar_gen4_pcie_of_match,
> > +	},
> > +	.probe = rcar_gen4_pcie_probe,
> > +	.remove = rcar_gen4_pcie_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > new file mode 100644
> > index 000000000000..a6a29d6125c8
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Renesas-specific */
> > +#define PCIERSTCTRL1		0x0014
> > +#define  APP_HOLD_PHY_RST	BIT(16)
> > +#define  APP_LTSSM_ENABLE	BIT(0)
> > +
> > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > +					bool enable)
> > +{
> > +	u32 val;
> > +
> > +	val = readl(rcar->base + PCIERSTCTRL1);
> > +	if (enable) {
> > +		val |= APP_LTSSM_ENABLE;
> > +		val &= ~APP_HOLD_PHY_RST;
> > +	} else {
> > +		val &= ~APP_LTSSM_ENABLE;
> > +		val |= APP_HOLD_PHY_RST;
> > +	}
> > +	writel(val, rcar->base + PCIERSTCTRL1);
> > +}
> > +
> > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +	u32 val, mask;
> > +
> > +	val = readl(rcar->base + PCIEINTSTS0);
> > +	mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > +
> > +	return (val & mask) == mask;
> > +}
> > +
> > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +	rcar_gen4_pcie_ltssm_enable(rcar, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +	rcar_gen4_pcie_ltssm_enable(rcar, false);
> > +}
> > +
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +				   int num_lanes)
> > +{
> > +	u32 val;
> > +
> > +	/* Note: Assume the reset is asserted here */
> > +	val = readl(rcar->base + PCIEMSR0);
> > +	if (rc)
> > +		val |= DEVICE_TYPE_RC;
> > +	else
> > +		val |= DEVICE_TYPE_EP;
> > +	if (num_lanes < 4)
> > +		val |= BIFUR_MOD_SET_ON;
> > +	writel(val, rcar->base + PCIEMSR0);
> > +
> > +	return reset_control_deassert(rcar->rst);
> > +}
> > +
> 
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > +{
> > +	dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > +}
> > +
> 
> Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
> driver assumes that the DBI2 is defined within the 0x1000 offset with
> respect to the DBI space (see dw_pcie_get_resources()). In your case
> it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
> What about either defining the "dbi2" reg-named DT-property in the
> DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
> the dw_pcie.dbi_base2 field in your LLDD?
> 
> Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
> only in the framework of the BARs mapping setup procedure. Are you
> sure it's working well in your case seeing the DBI2 base address is
> most likely incorrectly initialized? Anyway if you get to define the
> dbi_base2 field properly you'll be able to use the
> dw_pcie_ep_reset_bar() method in the EP driver instead of the function
> above (please see the way it's utilized in the rest of the DW PCIe EP
> low-level drivers).

Thank you for your comments. I'll investigate it.

> Another question. Are you sure the method above is relevant to the DW
> PCIe Root Port controller? I don't see any shadow registers defined
> for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
> 5.30, 5.40). What are they used for anyway?

To be honest, I reuse this code from our BSP which other guy made.
So, I'll investigate it.

> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > +{
> > +	u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > +
> > +	val &= ~PCI_EXP_LNKCAP_MLW;
> > +	switch (num_lanes) {
> > +	case 1:
> > +		val |= PCI_EXP_LNKCAP_MLW_X1;
> > +		break;
> > +	case 2:
> > +		val |= PCI_EXP_LNKCAP_MLW_X2;
> > +		break;
> > +	case 4:
> > +		val |= PCI_EXP_LNKCAP_MLW_X4;
> > +		break;
> > +	default:
> > +		dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > +		val |= PCI_EXP_LNKCAP_MLW_X1;
> > +		break;
> > +	}
> > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > +}
> 
> This seems to be a generic action. What about moving it to
> pcie-designware.c implementing a function similar to
> dw_pcie_link_set_max_speed(), which would be called in
> dw_pcie_setup()?

Rob also suggests it. So, I'll investigate it.

> It could be named as dw_pcie_link_set_max_(link_)?width() and besides
> of the code above would consist of the PCIE_PORT_LINK_CONTROL and
> PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
> dw_pcie_setup().

Thank you for your comments.

> * Please do that in a separate pre-requisite patch.
> 
> > +
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct device *dev = rcar->dw.dev;
> > +	int err;
> > +
> > +	pm_runtime_enable(dev);
> > +	err = pm_runtime_resume_and_get(dev);
> > +	if (err < 0) {
> > +		dev_err(dev, "Failed to resume/get Runtime PM\n");
> > +		pm_runtime_disable(dev);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct device *dev = rcar->dw.dev;
> > +
> > +	if (!reset_control_status(rcar->rst))
> > +		reset_control_assert(rcar->rst);
> > +	pm_runtime_put(dev);
> > +	pm_runtime_disable(dev);
> > +}
> > +
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > +				  struct device *dev)
> > +{
> > +	rcar->rst = devm_reset_control_get(dev, NULL);
> > +	if (IS_ERR(rcar->rst)) {
> > +		dev_err(dev, "Failed to get Cold-reset\n");
> > +		return PTR_ERR(rcar->rst);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > +	.start_link = rcar_gen4_pcie_start_link,
> > +	.stop_link = rcar_gen4_pcie_stop_link,
> > +	.link_up = rcar_gen4_pcie_link_up,
> > +};
> > +
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > +{
> > +	struct rcar_gen4_pcie *rcar;
> > +
> > +	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > +	if (!rcar)
> > +		return NULL;
> > +
> > +	rcar->dw.dev = dev;
> > +	rcar->dw.ops = &dw_pcie_ops;
> > +	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > +
> > +	return rcar;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > new file mode 100644
> > index 000000000000..1cdce0cf7dac
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef _PCIE_RCAR_GEN4_H_
> > +#define _PCIE_RCAR_GEN4_H_
> > +
> > +#include <linux/io.h>
> > +#include <linux/pci.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
> 
> > +/* PCI Express capability */
> > +#define EXPCAP(x)		(0x0070 + (x))
> > +/* ASPM L1 PM Substates */
> > +#define L1PSCAP(x)		(0x01bc + (x))
> > +/* PCI Shadow offset */
> > +#define SHADOW_REG(x)		(0x2000 + (x))
> > +/* BAR Mask registers */
> > +#define BAR0MASKF		0x0010
> > +#define BAR1MASKF		0x0014
> > +#define BAR2MASKF		0x0018
> > +#define BAR3MASKF		0x001c
> > +#define BAR4MASKF		0x0020
> > +#define BAR5MASKF		0x0024
> > +
> 
> * If you get to take some of my notes above into account you'll be able to
> drop these macros...

I got it.

> > +/* Renesas-specific */
> > +#define PCIEMSR0		0x0000
> > +#define  BIFUR_MOD_SET_ON	BIT(0)
> > +#define  DEVICE_TYPE_EP		0
> > +#define  DEVICE_TYPE_RC		BIT(4)
> > +
> > +#define PCIEINTSTS0		0x0084
> > +#define PCIEINTSTS0EN		0x0310
> > +#define  MSI_CTRL_INT		BIT(26)
> > +#define  SMLH_LINK_UP		BIT(7)
> > +#define  RDLH_LINK_UP		BIT(6)
> > +#define PCIEDMAINTSTSEN		0x0314
> > +#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
> 
> Vendor-specific prefix would make the code using these macros much
> more readable.

I got it. I'll rename them.

> > +
> > +struct rcar_gen4_pcie {
> > +	struct dw_pcie		dw;
> > +	void __iomem		*base;
> > +	struct reset_control	*rst;
> > +};
> > +#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
> > +
> 
> > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> 
> I don't see these two methods being defined in your driver. Are these
> artefacts from the previous patch revisions?

Oops. I'll remove them.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +				   int num_lanes);
> 
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > +				  struct device *dev);
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > +
> > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > --
> > 2.25.1
> >
> >

Re: [PATCH v9 6/8] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Serge Semin]

On Thu, Feb 16, 2023 at 11:33:16AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Thursday, February 16, 2023 3:33 AM
> > 
> > On Fri, Feb 10, 2023 at 10:49:15PM +0900, Yoshihiro Shimoda wrote:
> > > Add R-Car Gen4 PCIe Host support. This controller is based on
> > > Synopsys DesignWare PCIe.
> > >
> > > This controller doesn't support MSI-X interrupt. So, introduce
> > > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > > from dw_pcie_msi_domain_info.
> > >
> > 
> > > Note that this controller on R-Car S4-8 has an unexpected register
> > > value on the dbi+0x97b register. So, add a new capability flag
> > > which would force the unrolled eDMA mapping for the problematic
> > > device, as suggested by Serge Semin.
> > 
> > Please move the noted fix to a separate pre-requisite patch seeing your
> > update depends on it. Thus this patch will be a bit simpler to review
> > with no harm to the changes atomicity.
> 
> There is other patch though, I think Lorenzo prefers non separated patch:
> https://lore.kernel.org/all/Y245ZtqqqelXif+Y@lpieralisi/
> 
> But, Lorenzo, what do you think?
> 
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >  drivers/pci/controller/dwc/Kconfig            |   9 +
> > >  drivers/pci/controller/dwc/Makefile           |   2 +
> > >  .../pci/controller/dwc/pcie-designware-host.c |   3 +
> > >  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
> > >  drivers/pci/controller/dwc/pcie-designware.h  |   6 +-
> > >  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 165 +++++++++++++++++
> > >  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 166 ++++++++++++++++++
> > >  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 +++++++
> > >  8 files changed, 419 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 434f6a4f4041..94805ec31a8f 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -414,4 +414,13 @@ config PCIE_FU740
> > >  	  Say Y here if you want PCIe controller support for the SiFive
> > >  	  FU740.
> > >
> > > +config PCIE_RCAR_GEN4
> > > +	tristate "Renesas R-Car Gen4 PCIe Host controller"
> > > +	depends on ARCH_RENESAS || COMPILE_TEST
> > > +	depends on PCI_MSI
> > > +	select PCIE_DW_HOST
> > > +	help
> > > +	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > > +	  This uses the DesignWare core.
> > > +
> > >  endmenu
> > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > index bf5c311875a1..486cf706b53d 100644
> > > --- a/drivers/pci/controller/dwc/Makefile
> > > +++ b/drivers/pci/controller/dwc/Makefile
> > > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> > >  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> > >  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> > >  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> > >
> > >  # The following drivers are for devices that use the generic ACPI
> > >  # pci_root.c driver but don't support standard ECAM config access.
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 9952057c8819..5aefeec15c9b 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -242,6 +242,9 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> > >
> > >  	irq_domain_update_bus_token(pp->irq_domain, DOMAIN_BUS_NEXUS);
> > >
> > 
> > > +	if (pp->no_msix)
> > > +		dw_pcie_msi_domain_info.flags &= ~MSI_FLAG_PCI_MSIX;
> > 
> > Changing static data field in the probe path doesn't seem like
> > correct. Is it really that necessary to clear out that flag?
> 

> I didn't investigate the detail, but even if clearing this flag
> disappeared, it seemed to work.

Yeah, until we need to allocate more than 32 IRQ vectors per
hierarchy.) That can be tuned by either increasing the
MSI_DEF_NUM_VECTORS macros value to 256 or re-initializing the
num_vectors field in the LLDD with MAX_MSI_IRQS. That's what iMSI-RX
engine is capable of. But anyways it isn't near to what the
MSI-X-capable controller is supposed to support.

> 
> > The rest
> > of the LLDDs don't seem to be bothered with that (mine included). On
> > the other hand it seems to me that the iMSI-RX engine doesn't really
> > support true MSI-X messages. It always stick to a single base address
> > with up to 256 IRQs in total and up to 32 IRQs per function. Do you
> > suggest to drop that flag then for all DW PCIe hosts or just for ones
> > with iMSI-RX engine in-use or just forget about it?
> 

> I assumed almost all controller supported MSI-X, so that I suggested
> to drop the MSI_FLAG_PCI_MSIX flag if needed.

AFAICS iMSI-RX engine supports just MSI only at very least due to
having a single CSR for the MSI target address meanwhile MSI-X is
supposed to support multiple target addresses.

> 
> > @Rob, @Bjorn?
> > 
> > > +
> > >  	pp->msi_domain = pci_msi_create_irq_domain(fwnode,
> > >  						   &dw_pcie_msi_domain_info,
> > >  						   pp->irq_domain);
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index b4315cf84340..7d649ba387f8 100644
> > 
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -847,8 +847,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  	 * Indirect eDMA CSRs access has been completely removed since v5.40a
> > >  	 * thus no space is now reserved for the eDMA channels viewport and
> > >  	 * former DMA CTRL register is no longer fixed to FFs.
> > > +	 *
> > > +	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > > +	 * register even though the HW-manual explicitly states there must FFs
> > > +	 * if the unrolled mapping is enabled. For such cases the low-level
> > > +	 * drivers are supposed to manually activate the unrolled mapping to
> > > +	 * bypass the auto-detection procedure.
> > >  	 */
> > > -	if (dw_pcie_ver_is_ge(pci, 540A))
> > > +	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> > >  		val = 0xFFFFFFFF;
> > >  	else
> > >  		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > 
> > As I suggested in the head of this message please move this to a
> > separate pre-requisite patch.
> 
> I would like to know Lorenzo's preference.
> 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 1a6e7e9489ee..1b1af514b849 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -51,8 +51,9 @@
> > >
> > >  /* DWC PCIe controller capabilities */
> > >  #define DW_PCIE_CAP_REQ_RES		0
> > 
> > > -#define DW_PCIE_CAP_IATU_UNROLL		1
> > > -#define DW_PCIE_CAP_CDM_CHECK		2
> > > +#define DW_PCIE_CAP_EDMA_UNROLL		1
> > > +#define DW_PCIE_CAP_IATU_UNROLL		2
> > > +#define DW_PCIE_CAP_CDM_CHECK		3
> > 
> > ditto
> > 
> > >
> > >  #define dw_pcie_cap_is(_pci, _cap) \
> > >  	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > > @@ -303,6 +304,7 @@ struct dw_pcie_host_ops {
> > >  struct dw_pcie_rp {
> > >  	bool			has_msi_ctrl:1;
> > >  	bool			cfg0_io_shared:1;
> > > +	bool			no_msix:1;
> > >  	u64			cfg0_base;
> > >  	void __iomem		*va_cfg0_base;
> > >  	u32			cfg0_size;
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > > new file mode 100644
> > > index 000000000000..d60f4895ffe9
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > > @@ -0,0 +1,165 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include "pcie-rcar-gen4.h"
> > > +#include "pcie-designware.h"
> > > +
> > > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > > +{
> > > +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +	int ret;
> > > +	u32 val;
> > > +
> > > +	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	dw_pcie_dbi_ro_wr_en(dw);
> > > +
> > > +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > > +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > > +
> > > +	dw_pcie_dbi_ro_wr_dis(dw);
> > > +
> > > +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > > +		/* Enable MSI interrupt signal */
> > > +		val = readl(rcar->base + PCIEINTSTS0EN);
> > > +		val |= MSI_CTRL_INT;
> > > +		writel(val, rcar->base + PCIEINTSTS0EN);
> > > +	}
> > > +
> > > +	gpiod_set_value_cansleep(dw->pe_rst, 0);
> > > +
> > 
> > > +	dw_pcie_setup_rc(pp);
> > > +
> > > +	dw_pcie_dbi_ro_wr_en(dw);
> > > +	rcar_gen4_pcie_set_max_link_width(dw, dw->num_lanes);
> > > +	dw_pcie_dbi_ro_wr_dis(dw);
> > > +
> > > +	if (!dw_pcie_link_up(dw)) {
> > > +		ret = dw->ops->start_link(dw);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > > +	/* Ignore errors, the link may come up later */
> > > +	if (dw_pcie_wait_for_link(dw))
> > > +		dev_info(dw->dev, "PCIe link down\n");
> > 
> > This whole procedure is done in the dw_pcie_host_init() method (@Rob
> > already noted that a bit earlier). The only exception is the
> > rcar_gen4_pcie_set_max_link_width() method, which I suggest to move
> > the generic code (see further for more details).
> 

> I investigated this and then I found they can be just removed.
> # This is related to the following patch. After I applied the following,
> # I can remove the above codes.
> # https://lore.kernel.org/linux-pci/20230216092012.3256440-1-yoshihiro.shimoda.uh@renesas.com/

Yeah, my mistake.( I should have been more attentive. Sorry about
that.

-Serge(y)

> 
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > > +	.host_init = rcar_gen4_pcie_host_init,
> > > +};
> > > +
> > > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > > +				   struct platform_device *pdev)
> > > +{
> > > +	struct dw_pcie *dw = &rcar->dw;
> > > +	struct dw_pcie_rp *pp = &dw->pp;
> > > +	int ret;
> > > +
> > > +	pp->ops = &rcar_gen4_pcie_host_ops;
> > > +	pp->no_msix = true;
> > > +	dw_pcie_cap_set(dw, REQ_RES);
> > > +
> > > +	ret = dw_pcie_host_init(pp);
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "Failed to initialize host\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > > +{
> > > +	dw_pcie_host_deinit(&rcar->dw.pp);
> > > +	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > > +}
> > > +
> > 
> > > +static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > > +					struct platform_device *pdev)
> > > +{
> > > +	struct dw_pcie *dw = &rcar->dw;
> > > +
> > > +	/* Renesas-specific registers */
> > > +	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > > +	if (IS_ERR(rcar->base))
> > > +		return PTR_ERR(rcar->base);
> > > +
> > > +	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > > +}
> > 
> > Please note that after fixing the RCAR PCIe EP DT-bindings to permit
> > the "app" instead of "appl" named reg property (see my note the RCAR
> > PCie EP DT-bindings patch) the method above will be identical in the
> > RCAR RP and EP driver implementations. So you'll be able to move it
> > to the pcie-rcar-gen4.c driver.
> 
> I think so. I'll modify it.
> 
> > > +
> > > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *dev = &pdev->dev;
> > > +	struct rcar_gen4_pcie *rcar;
> > > +	int err;
> > > +
> > > +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> > > +	if (!rcar)
> > > +		return -ENOMEM;
> > > +
> > > +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > > +	if (err < 0) {
> > > +		dev_err(dev, "Failed to request resource: %d\n", err);
> > > +		return err;
> > > +	}
> > > +
> > > +	platform_set_drvdata(pdev, rcar);
> > > +
> > > +	err = rcar_gen4_pcie_prepare(rcar);
> > > +	if (err < 0)
> > > +		return err;
> > > +
> > > +	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > > +	if (err < 0)
> > > +		goto err_add;
> > > +
> > > +	return 0;
> > > +
> > > +err_add:
> > > +	rcar_gen4_pcie_unprepare(rcar);
> > > +
> > > +	return err;
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > > +{
> > > +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > > +
> > > +	rcar_gen4_remove_dw_pcie_rp(rcar);
> > > +	rcar_gen4_pcie_unprepare(rcar);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > > +	{ .compatible = "renesas,rcar-gen4-pcie", },
> > > +	{},
> > > +};
> > > +
> > > +static struct platform_driver rcar_gen4_pcie_driver = {
> > > +	.driver = {
> > > +		.name = "pcie-rcar-gen4",
> > > +		.of_match_table = rcar_gen4_pcie_of_match,
> > > +	},
> > > +	.probe = rcar_gen4_pcie_probe,
> > > +	.remove = rcar_gen4_pcie_remove,
> > > +};
> > > +module_platform_driver(rcar_gen4_pcie_driver);
> > > +
> > > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > new file mode 100644
> > > index 000000000000..a6a29d6125c8
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > @@ -0,0 +1,166 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include "pcie-rcar-gen4.h"
> > > +#include "pcie-designware.h"
> > > +
> > > +/* Renesas-specific */
> > > +#define PCIERSTCTRL1		0x0014
> > > +#define  APP_HOLD_PHY_RST	BIT(16)
> > > +#define  APP_LTSSM_ENABLE	BIT(0)
> > > +
> > > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > > +					bool enable)
> > > +{
> > > +	u32 val;
> > > +
> > > +	val = readl(rcar->base + PCIERSTCTRL1);
> > > +	if (enable) {
> > > +		val |= APP_LTSSM_ENABLE;
> > > +		val &= ~APP_HOLD_PHY_RST;
> > > +	} else {
> > > +		val &= ~APP_LTSSM_ENABLE;
> > > +		val |= APP_HOLD_PHY_RST;
> > > +	}
> > > +	writel(val, rcar->base + PCIERSTCTRL1);
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > > +{
> > > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +	u32 val, mask;
> > > +
> > > +	val = readl(rcar->base + PCIEINTSTS0);
> > > +	mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > > +
> > > +	return (val & mask) == mask;
> > > +}
> > > +
> > > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > > +{
> > > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +
> > > +	rcar_gen4_pcie_ltssm_enable(rcar, true);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > > +{
> > > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > > +
> > > +	rcar_gen4_pcie_ltssm_enable(rcar, false);
> > > +}
> > > +
> > > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > > +				   int num_lanes)
> > > +{
> > > +	u32 val;
> > > +
> > > +	/* Note: Assume the reset is asserted here */
> > > +	val = readl(rcar->base + PCIEMSR0);
> > > +	if (rc)
> > > +		val |= DEVICE_TYPE_RC;
> > > +	else
> > > +		val |= DEVICE_TYPE_EP;
> > > +	if (num_lanes < 4)
> > > +		val |= BIFUR_MOD_SET_ON;
> > > +	writel(val, rcar->base + PCIEMSR0);
> > > +
> > > +	return reset_control_deassert(rcar->rst);
> > > +}
> > > +
> > 
> > > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > > +{
> > > +	dw_pcie_writel_dbi(dw, SHADOW_REG(bar_mask_reg), 0x0);
> > > +}
> > > +
> > 
> > Hm, this seems like a DBI2/DBI_CS2 CSRs. By default the DW PCIe core
> > driver assumes that the DBI2 is defined within the 0x1000 offset with
> > respect to the DBI space (see dw_pcie_get_resources()). In your case
> > it must be within 0x2000 offset (judging by the SHADOW_REG() macro).
> > What about either defining the "dbi2" reg-named DT-property in the
> > DT-bindings (see DW PCIe RP/EP generic DT-bindings) or re-defining
> > the dw_pcie.dbi_base2 field in your LLDD?
> > 
> > Note the dbi_cs2 space is currently utilized in the DW PCIe EP driver
> > only in the framework of the BARs mapping setup procedure. Are you
> > sure it's working well in your case seeing the DBI2 base address is
> > most likely incorrectly initialized? Anyway if you get to define the
> > dbi_base2 field properly you'll be able to use the
> > dw_pcie_ep_reset_bar() method in the EP driver instead of the function
> > above (please see the way it's utilized in the rest of the DW PCIe EP
> > low-level drivers).
> 
> Thank you for your comments. I'll investigate it.
> 
> > Another question. Are you sure the method above is relevant to the DW
> > PCIe Root Port controller? I don't see any shadow registers defined
> > for the host BARs in any HW-manual I've got (4.60, 4.70, 4.90, 5.20,
> > 5.30, 5.40). What are they used for anyway?
> 
> To be honest, I reuse this code from our BSP which other guy made.
> So, I'll investigate it.
> 
> > > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *dw, int num_lanes)
> > > +{
> > > +	u32 val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> > > +
> > > +	val &= ~PCI_EXP_LNKCAP_MLW;
> > > +	switch (num_lanes) {
> > > +	case 1:
> > > +		val |= PCI_EXP_LNKCAP_MLW_X1;
> > > +		break;
> > > +	case 2:
> > > +		val |= PCI_EXP_LNKCAP_MLW_X2;
> > > +		break;
> > > +	case 4:
> > > +		val |= PCI_EXP_LNKCAP_MLW_X4;
> > > +		break;
> > > +	default:
> > > +		dev_info(dw->dev, "Invalid num-lanes %d\n", num_lanes);
> > > +		val |= PCI_EXP_LNKCAP_MLW_X1;
> > > +		break;
> > > +	}
> > > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCAP), val);
> > > +}
> > 
> > This seems to be a generic action. What about moving it to
> > pcie-designware.c implementing a function similar to
> > dw_pcie_link_set_max_speed(), which would be called in
> > dw_pcie_setup()?
> 
> Rob also suggests it. So, I'll investigate it.
> 
> > It could be named as dw_pcie_link_set_max_(link_)?width() and besides
> > of the code above would consist of the PCIE_PORT_LINK_CONTROL and
> > PCIE_LINK_WIDTH_SPEED_CONTROL CSR initializations performed in
> > dw_pcie_setup().
> 
> Thank you for your comments.
> 
> > * Please do that in a separate pre-requisite patch.
> > 
> > > +
> > > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > > +{
> > > +	struct device *dev = rcar->dw.dev;
> > > +	int err;
> > > +
> > > +	pm_runtime_enable(dev);
> > > +	err = pm_runtime_resume_and_get(dev);
> > > +	if (err < 0) {
> > > +		dev_err(dev, "Failed to resume/get Runtime PM\n");
> > > +		pm_runtime_disable(dev);
> > > +	}
> > > +
> > > +	return err;
> > > +}
> > > +
> > > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > > +{
> > > +	struct device *dev = rcar->dw.dev;
> > > +
> > > +	if (!reset_control_status(rcar->rst))
> > > +		reset_control_assert(rcar->rst);
> > > +	pm_runtime_put(dev);
> > > +	pm_runtime_disable(dev);
> > > +}
> > > +
> > > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > > +				  struct device *dev)
> > > +{
> > > +	rcar->rst = devm_reset_control_get(dev, NULL);
> > > +	if (IS_ERR(rcar->rst)) {
> > > +		dev_err(dev, "Failed to get Cold-reset\n");
> > > +		return PTR_ERR(rcar->rst);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct dw_pcie_ops dw_pcie_ops = {
> > > +	.start_link = rcar_gen4_pcie_start_link,
> > > +	.stop_link = rcar_gen4_pcie_stop_link,
> > > +	.link_up = rcar_gen4_pcie_link_up,
> > > +};
> > > +
> > > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > > +{
> > > +	struct rcar_gen4_pcie *rcar;
> > > +
> > > +	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > > +	if (!rcar)
> > > +		return NULL;
> > > +
> > > +	rcar->dw.dev = dev;
> > > +	rcar->dw.ops = &dw_pcie_ops;
> > > +	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > > +
> > > +	return rcar;
> > > +}
> > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > > new file mode 100644
> > > index 000000000000..1cdce0cf7dac
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > > @@ -0,0 +1,63 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#ifndef _PCIE_RCAR_GEN4_H_
> > > +#define _PCIE_RCAR_GEN4_H_
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include "pcie-designware.h"
> > > +
> > 
> > > +/* PCI Express capability */
> > > +#define EXPCAP(x)		(0x0070 + (x))
> > > +/* ASPM L1 PM Substates */
> > > +#define L1PSCAP(x)		(0x01bc + (x))
> > > +/* PCI Shadow offset */
> > > +#define SHADOW_REG(x)		(0x2000 + (x))
> > > +/* BAR Mask registers */
> > > +#define BAR0MASKF		0x0010
> > > +#define BAR1MASKF		0x0014
> > > +#define BAR2MASKF		0x0018
> > > +#define BAR3MASKF		0x001c
> > > +#define BAR4MASKF		0x0020
> > > +#define BAR5MASKF		0x0024
> > > +
> > 
> > * If you get to take some of my notes above into account you'll be able to
> > drop these macros...
> 
> I got it.
> 
> > > +/* Renesas-specific */
> > > +#define PCIEMSR0		0x0000
> > > +#define  BIFUR_MOD_SET_ON	BIT(0)
> > > +#define  DEVICE_TYPE_EP		0
> > > +#define  DEVICE_TYPE_RC		BIT(4)
> > > +
> > > +#define PCIEINTSTS0		0x0084
> > > +#define PCIEINTSTS0EN		0x0310
> > > +#define  MSI_CTRL_INT		BIT(26)
> > > +#define  SMLH_LINK_UP		BIT(7)
> > > +#define  RDLH_LINK_UP		BIT(6)
> > > +#define PCIEDMAINTSTSEN		0x0314
> > > +#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
> > 
> > Vendor-specific prefix would make the code using these macros much
> > more readable.
> 
> I got it. I'll rename them.
> 
> > > +
> > > +struct rcar_gen4_pcie {
> > > +	struct dw_pcie		dw;
> > > +	void __iomem		*base;
> > > +	struct reset_control	*rst;
> > > +};
> > > +#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
> > > +
> > 
> > > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> > 
> > I don't see these two methods being defined in your driver. Are these
> > artefacts from the previous patch revisions?
> 
> Oops. I'll remove them.
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > -Serge(y)
> > 
> > > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > > +				   int num_lanes);
> > 
> > > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > > +void rcar_gen4_pcie_set_max_link_width(struct dw_pcie *pci, int num_lanes);
> > > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > > +int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *pcie,
> > > +				  struct device *dev);
> > > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > > +
> > > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > > --
> > > 2.25.1
> > >
> > >
>