All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support
@ 2022-11-21 12:43 Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
                   ` (8 more replies)
  0 siblings, 9 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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 v6:
 https://lore.kernel.org/all/20220922080647.3489791-1-yoshihiro.shimoda.uh@renesas.com/
 - Based on next-20221116.
 -- And based on the following patches:
    [PATCH v7 00/20] PCI: dwc: Add generic resources and Baikal-T1 support
    https://lore.kernel.org/linux-pci/20221113191301.5526-1-Sergey.Semin@baikalelectronics.ru/
    [PATCH v6 00/24] dmaengine: dw-edma: Add RP/EP local DMA controllers support
    https://lore.kernel.org/linux-pci/20221107210438.1515-1-Sergey.Semin@baikalelectronics.ru/
 - Update dt-bindings docs for the latest based code.
 - Add support for triggering legacy IRQs in the patch [06/10] (new).
 - Add .no_msix flag into the patch [07/10].
 - Merge .ep_pre_init() support into the patch [08/10].
 - Add .reserved_bar for BAR5 instead in the patch [08/10].
 - Change SPDX-License-Identifier from "GPL-2.0" to "GPL-2.0-only".

Changes from v5:
 https://lore.kernel.org/all/20220905071257.1059436-1-yoshihiro.shimoda.uh@renesas.com/
 - No treewide patches.
 - Drop PCI_EXP_LNKCAP_MLW_X32 in patch [03/10].
 - Add Acked-by in patch [03/10].
 - Fix subject prefix in patch [04/10], [05/10], [08/10] and [09/10].
 - Fix typo in patch [05/10] and [07/10].

Yoshihiro Shimoda (9):
  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: Avoid reading a register to detect whether eDMA exists
  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  |  29 ++-
 drivers/pci/controller/dwc/pcie-designware.h  |  14 +-
 .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 182 +++++++++++++++++
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 190 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 181 +++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 ++++++
 include/uapi/linux/pci_regs.h                 |   6 +
 14 files changed, 923 insertions(+), 18 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


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

* [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	Yoshihiro Shimoda, Rob Herring

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


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

* [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	Yoshihiro Shimoda, Rob Herring

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


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

* [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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 1c3591c8e09e..31216c6a07ad 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


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

* [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (2 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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


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

* [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (3 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-22 13:55   ` Manivannan Sadhasivam
  2022-11-21 12:43 ` [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	Yoshihiro Shimoda

Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
So, directly read the eDMA register if edma.reg_base is not zero.

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

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 637d01807c67..2cc8584da6f4 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 {
 	u32 val;
 
-	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
-	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+	if (pci->edma.reg_base) {
 		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
 
 		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
@@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
 
 		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
+		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
 	} else {
 		return -ENODEV;
 	}
-- 
2.25.1


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

* [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (4 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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 2cc8584da6f4..79ac389b794a 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 028155c03acd..ab50c4577495 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -146,11 +146,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
@@ -243,6 +246,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;
@@ -351,7 +357,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 {
@@ -418,7 +427,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


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

* [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (5 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-22 15:04   ` Bjorn Helgaas
  2022-11-21 12:43 ` [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
  2022-11-21 12:44 ` [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
  8 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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.

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.h  |   1 +
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 190 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 181 +++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  63 ++++++
 7 files changed, 449 insertions(+)
 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 771b8b146623..99717b0f4e50 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -393,4 +393,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_IRQ_DOMAIN
+	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 d18ff0519a62..122e4bbb98f9 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.h b/drivers/pci/controller/dwc/pcie-designware.h
index ab50c4577495..ca0f9678517f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -302,6 +302,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..646e8efaaed8
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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;
+
+	rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
+
+	dw_pcie_dbi_ro_wr_en(dw);
+
+	/* Enable L1 Substates */
+	val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
+	val &= ~PCI_L1SS_CTL1_L1SS_MASK;
+	val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
+	       PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
+	dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);
+
+	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
+	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
+
+	/* Set Root Control */
+	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
+	val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
+	       PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
+	       PCI_EXP_RTCTL_CRSSVE;
+	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
+
+	/* Set Interrupt Disable, SERR# Enable, Parity Error Response */
+	val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
+	val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
+	       PCI_COMMAND_INTX_DISABLE;
+	dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
+
+	/* Enable SERR */
+	val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
+	val |= PCI_BRIDGE_CTL_SERR;
+	dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
+
+	/* Device control */
+	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
+	val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
+	       PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
+	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
+
+	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);
+	}
+
+	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;
+
+	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);
+}
+
+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..9900d1353998
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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)
+
+#define DWC_VERSION		0x520a
+
+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);
+}
+
+void rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+				    int num_lanes)
+{
+	u32 val;
+
+	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);
+}
+
+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, "%s: failed to resume/get Runtime PM\n", __func__);
+		goto err_resume_and_get;
+	}
+
+	err = reset_control_deassert(rcar->rst);
+	if (err < 0) {
+		dev_err(dev, "%s: failed to deassert reset_control\n", __func__);
+		goto err_deassert;
+	}
+
+	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+
+	return 0;
+
+err_deassert:
+	pm_runtime_put(dev);
+
+err_resume_and_get:
+	pm_runtime_disable(dev);
+
+	return err;
+}
+
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
+{
+	struct device *dev = rcar->dw.dev;
+
+	writel(0, rcar->base + PCIEDMAINTSTSEN);
+	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;
+	rcar->dw.version = DWC_VERSION;
+
+	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..db401f884fc8
--- /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 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);
+void 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


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

* [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (6 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2022-11-21 12:43 ` Yoshihiro Shimoda
  2022-11-21 12:44 ` [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:43 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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    | 182 ++++++++++++++++++
 5 files changed, 197 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 99717b0f4e50..4889bde3471b 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -402,4 +402,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..be2b31856c69 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -765,6 +765,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 
 	dw_pcie_version_detect(pci);
 
+	if (ep->ops->ep_pre_init)
+		ep->ops->ep_pre_init(ep);
+
 	dw_pcie_iatu_detect(pci);
 
 	ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ca0f9678517f..25c11a591ae2 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -324,6 +324,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..c043844a2acd
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022 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;
+	}
+
+	dw->ops->start_link(dw);
+
+	return 0;
+}
+
+static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+	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


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

* [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4
  2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
                   ` (7 preceding siblings ...)
  2022-11-21 12:43 ` [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
@ 2022-11-21 12:44 ` Yoshihiro Shimoda
  8 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-21 12:44 UTC (permalink / raw)
  To: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt
  Cc: marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc,
	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 08b67532e374..4d62fe45032c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15972,6 +15972,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


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

* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
@ 2022-11-22 13:55   ` Manivannan Sadhasivam
  2022-11-27 23:55     ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Manivannan Sadhasivam @ 2022-11-22 13:55 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, fancer.lancer,
	Sergey.Semin

+ Serge (who authored EDMA support)

Thanks,
Mani

On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> So, directly read the eDMA register if edma.reg_base is not zero.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 637d01807c67..2cc8584da6f4 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  {
>  	u32 val;
>  
> -	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> +	if (pci->edma.reg_base) {
>  		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
>  
>  		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
>  
>  		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> +		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
>  	} else {
>  		return -ENODEV;
>  	}
> -- 
> 2.25.1
> 

-- 
மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
  2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
@ 2022-11-22 15:04   ` Bjorn Helgaas
  2022-11-25 11:37     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Bjorn Helgaas @ 2022-11-22 15:04 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc

On Mon, Nov 21, 2022 at 09:43:58PM +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.

> +	/* Enable L1 Substates */
> +	val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
> +	val &= ~PCI_L1SS_CTL1_L1SS_MASK;
> +	val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
> +	       PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
> +	dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);

This seems like something that ought to be done by the PCI core in
pcie/aspm.c.  L1.2 also depends on LTR being supported and configured.

If it needs to be enabled here, can you expand the comment to say why
and how LTR is being configured?

> +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> +
> +	/* Set Root Control */
> +	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
> +	val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> +	       PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> +	       PCI_EXP_RTCTL_CRSSVE;
> +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> +
> +	/* Set Interrupt Disable, SERR# Enable, Parity Error Response */
> +	val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
> +	val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> +	       PCI_COMMAND_INTX_DISABLE;
> +	dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> +
> +	/* Enable SERR */
> +	val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
> +	val |= PCI_BRIDGE_CTL_SERR;
> +	dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> +
> +	/* Device control */
> +	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
> +	val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> +	       PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
> +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);

The above also looks like things that should be configured by the PCI
core.

> +		dev_err(&pdev->dev, "Failed to initialize host\n");
> +		dev_err(dev, "failed to request resource: %d\n", err);

Pick a capitalization style.

> +		dev_err(dev, "%s: failed to resume/get Runtime PM\n", __func__);

The driver name + device ID + message text printed by dev_err() should
be enough that __func__ isn't needed.

Bjorn

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

* RE: [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
  2022-11-22 15:04   ` Bjorn Helgaas
@ 2022-11-25 11:37     ` Yoshihiro Shimoda
  0 siblings, 0 replies; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-25 11:37 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc

Hi Bjorn,

> From: Bjorn Helgaas, Sent: Wednesday, November 23, 2022 12:05 AM
> 
> On Mon, Nov 21, 2022 at 09:43:58PM +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.
> 
> > +	/* Enable L1 Substates */
> > +	val = dw_pcie_readl_dbi(dw, L1PSCAP(PCI_L1SS_CTL1));
> > +	val &= ~PCI_L1SS_CTL1_L1SS_MASK;
> > +	val |= PCI_L1SS_CTL1_PCIPM_L1_2 | PCI_L1SS_CTL1_PCIPM_L1_1 |
> > +	       PCI_L1SS_CTL1_ASPM_L1_2 | PCI_L1SS_CTL1_ASPM_L1_1;
> > +	dw_pcie_writel_dbi(dw, L1PSCAP(PCI_L1SS_CTL1), val);
> 
> This seems like something that ought to be done by the PCI core in
> pcie/aspm.c.  L1.2 also depends on LTR being supported and configured.
> 
> If it needs to be enabled here, can you expand the comment to say why
> and how LTR is being configured?

Thank you for your review! I realized that this driver should not enable
it here, as you mentioned. However, I don't know why but it needs to be
enabled here. Otherwise, this driver cannot work. So, I'm investigating
the issue now.

> > +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> > +
> > +	/* Set Root Control */
> > +	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_RTCTL));
> > +	val |= PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> > +	       PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> > +	       PCI_EXP_RTCTL_CRSSVE;
> > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> > +
> > +	/* Set Interrupt Disable, SERR# Enable, Parity Error Response */
> > +	val = dw_pcie_readl_dbi(dw, PCI_COMMAND);
> > +	val |= PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> > +	       PCI_COMMAND_INTX_DISABLE;
> > +	dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> > +
> > +	/* Enable SERR */
> > +	val = dw_pcie_readb_dbi(dw, PCI_BRIDGE_CONTROL);
> > +	val |= PCI_BRIDGE_CTL_SERR;
> > +	dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> > +
> > +	/* Device control */
> > +	val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_DEVCTL));
> > +	val |= PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> > +	       PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE;
> > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);
> 
> The above also looks like things that should be configured by the PCI
> core.

I think so. I realized that the following settings are not needed here.
So, I'll drop them.

> > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_RTCTL), val);
> > +	dw_pcie_writel_dbi(dw, PCI_COMMAND, val);
> > +	dw_pcie_writeb_dbi(dw, PCI_BRIDGE_CONTROL, val);
> > +	dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_DEVCTL), val);


> > +		dev_err(&pdev->dev, "Failed to initialize host\n");
> > +		dev_err(dev, "failed to request resource: %d\n", err);
> 
> Pick a capitalization style.

I'll fix the style.

> > +		dev_err(dev, "%s: failed to resume/get Runtime PM\n", __func__);
> 
> The driver name + device ID + message text printed by dev_err() should
> be enough that __func__ isn't needed.

I got it. I'll fix this on v8.

Best regards,
Yoshihiro Shimoda

> Bjorn

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

* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-22 13:55   ` Manivannan Sadhasivam
@ 2022-11-27 23:55     ` Serge Semin
  2022-11-28  2:52       ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-27 23:55 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Yoshihiro Shimoda
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> + Serge (who authored EDMA support)

Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
merged in yet and miss the patch author in the Cc list.)

@Yoshihiro, on the next patchset revisions please don't forget to add
my email address to the copy list.

> 
> Thanks,
> Mani
> 
> On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > So, directly read the eDMA register if edma.reg_base is not zero.
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 637d01807c67..2cc8584da6f4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  {
> >  	u32 val;
> >  

> > -	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > +	if (pci->edma.reg_base) {
> >  		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >  
> >  		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> >  
> >  		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > +		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);

Look what you suggest here:
< u32 val;
< ...
< if (pci->edma.reg_base) {
< 	...
< } else if (val != 0xFFFFFFFF) {
< 	...
< } else {
< ...

It would be strange if your compiler didn't warn about 'val' being used
uninitialized here, which in its turn would introduce a regression for
the platforms with the indirectly accessible eDMA registers.

Anyway you can't just drop something what didn't work for you
hardware. The method you suggest to fix here works fine for multiple
DW PCIe IP-cores. Judging by the HW manuals it should work at least up
to v5.30a. Are you sure that your controller is of v5.20a? I see you
overwrite the IP-core version for the PCIe host driver only. Why is
that necessary? Does the version auto-detection procedure work
incorrectly for you? What does the dbi+0x8f8 CSR contain in the host
and EP registers space? Similarly could you also provide a content of
the +0x978 register?

-Sergey

> >  	} else {
> >  		return -ENODEV;
> >  	}
> > -- 
> > 2.25.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-27 23:55     ` Serge Semin
@ 2022-11-28  2:52       ` Yoshihiro Shimoda
  2022-11-28 11:59         ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-28  2:52 UTC (permalink / raw)
  To: Serge Semin, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

Hi Serge,

> From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> 
> On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> > + Serge (who authored EDMA support)
> 
> Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
> merged in yet and miss the patch author in the Cc list.)
> 
> @Yoshihiro, on the next patchset revisions please don't forget to add
> my email address to the copy list.

Sure! I'll add your email address on the next patchset revisions.

> > Thanks,
> > Mani
> >
> > On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > > So, directly read the eDMA register if edma.reg_base is not zero.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 637d01807c67..2cc8584da6f4 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  {
> > >  	u32 val;
> > >
> 
> > > -	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > +	if (pci->edma.reg_base) {
> > >  		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > >
> > >  		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > >
> > >  		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > > +		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> 
> Look what you suggest here:
> < u32 val;
> < ...
> < if (pci->edma.reg_base) {
> < 	...
> < } else if (val != 0xFFFFFFFF) {
> < 	...
> < } else {
> < ...
> 
> It would be strange if your compiler didn't warn about 'val' being used
> uninitialized here, which in its turn would introduce a regression for
> the platforms with the indirectly accessible eDMA registers.

You're correct. It's strange. I don't know why my using compiler [1] didn't
warn about the 'val' though...

[1]
https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-aarch64-linux.tar.xz

> Anyway you can't just drop something what didn't work for you
> hardware. The method you suggest to fix here works fine for multiple
> DW PCIe IP-cores. Judging by the HW manuals it should work at least up
> to v5.30a. Are you sure that your controller is of v5.20a? I see you
> overwrite the IP-core version for the PCIe host driver only. Why is
> that necessary? Does the version auto-detection procedure work
> incorrectly for you?

Thank you for pointed it out! I realized that overwriting the IP-core
Version was not needed. So, I'll drop it on v8.
---
#define DWC_VERSION             0x520a
...
struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
{
...
	rcar->dw.version = DWC_VERSION;
---

> What does the dbi+0x8f8 CSR contain in the host
> and EP registers space? Similarly could you also provide a content of
> the +0x978 register?

The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
--------------- (sorry, replace tabs with spaces...)---------------
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 {
        u32 val;

+       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
+               dw_pcie_readl_dma(pci, 0x8f8),
+               dw_pcie_readl_dma(pci, 0x978));
+
        if (pci->edma.reg_base) {
                pci->edma.mf = EDMA_MF_EDMA_UNROLL;
-------------------------------------------------------------------

----- Output -----
# dmesg | grep edma
[    1.108709] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 00000000, +0x978 = 00000000
------------------

So, should I change the condition like below?

---
-	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
...
-	} else if (val != 0xFFFFFFFF) {
-	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
---

Best regards,
Yoshihiro Shimoda

> -Sergey
> 
> > >  	} else {
> > >  		return -ENODEV;
> > >  	}
> > > --
> > > 2.25.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-28  2:52       ` Yoshihiro Shimoda
@ 2022-11-28 11:59         ` Serge Semin
  2022-11-28 12:41           ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-28 11:59 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: Manivannan Sadhasivam, lpieralisi, robh+dt, kw, bhelgaas,
	krzk+dt, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc, Sergey.Semin

On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > 
> > On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> > > + Serge (who authored EDMA support)
> > 
> > Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
> > merged in yet and miss the patch author in the Cc list.)
> > 
> > @Yoshihiro, on the next patchset revisions please don't forget to add
> > my email address to the copy list.
> 
> Sure! I'll add your email address on the next patchset revisions.
> 
> > > Thanks,
> > > Mani
> > >
> > > On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > > > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > > > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > > > So, directly read the eDMA register if edma.reg_base is not zero.
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 637d01807c67..2cc8584da6f4 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > >  {
> > > >  	u32 val;
> > > >
> > 
> > > > -	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > +	if (pci->edma.reg_base) {
> > > >  		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > > >
> > > >  		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > > > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > > >  		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > > >
> > > >  		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > > > +		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > 
> > Look what you suggest here:
> > < u32 val;
> > < ...
> > < if (pci->edma.reg_base) {
> > < 	...
> > < } else if (val != 0xFFFFFFFF) {
> > < 	...
> > < } else {
> > < ...
> > 
> > It would be strange if your compiler didn't warn about 'val' being used
> > uninitialized here, which in its turn would introduce a regression for
> > the platforms with the indirectly accessible eDMA registers.
> 
> You're correct. It's strange. I don't know why my using compiler [1] didn't
> warn about the 'val' though...
> 
> [1]
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-aarch64-linux.tar.xz
> 
> > Anyway you can't just drop something what didn't work for you
> > hardware. The method you suggest to fix here works fine for multiple
> > DW PCIe IP-cores. Judging by the HW manuals it should work at least up
> > to v5.30a. Are you sure that your controller is of v5.20a? I see you
> > overwrite the IP-core version for the PCIe host driver only. Why is
> > that necessary? Does the version auto-detection procedure work
> > incorrectly for you?
> 
> Thank you for pointed it out! I realized that overwriting the IP-core
> Version was not needed. So, I'll drop it on v8.
> ---
> #define DWC_VERSION             0x520a
> ...
> struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> {
> ...
> 	rcar->dw.version = DWC_VERSION;
> ---
> 
> > What does the dbi+0x8f8 CSR contain in the host
> > and EP registers space? Similarly could you also provide a content of
> > the +0x978 register?
> 
> The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> --------------- (sorry, replace tabs with spaces...)---------------
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  {
>         u32 val;
> 

> +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> +               dw_pcie_readl_dma(pci, 0x8f8),
> +               dw_pcie_readl_dma(pci, 0x978));
> +

No, this should have been the dw_pcie_readl_dbi() calls instead of
dw_pcie_readl_!dma!(). What I try to understand from these values is
the real version of your controller (dbi+0x8f8) and whether the legacy
eDMA viewport registers range follows the documented convention of
having FFs in the dbi+0x978 register. My current assumption that
either your IP-core is newer than v5.30a or has some vendor-specific
modification. But let's see the value first.

>         if (pci->edma.reg_base) {
>                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> -------------------------------------------------------------------
> 
> ----- Output -----
> # dmesg | grep edma
> [    1.108709] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 00000000, +0x978 = 00000000
> ------------------
> 

> So, should I change the condition like below?
> 
> ---
> -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> ...
> -	} else if (val != 0xFFFFFFFF) {
> -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> ---

Definitely no. Even though it's impossible to have the eDMA controller
configured with zero number of read and write channels we shouldn't
assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
means having the unrolled eDMA CSRs mapping. Let's have a look at the
content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
values we'll come up with what to do next.

-Serge(y)

> 
> Best regards,
> Yoshihiro Shimoda
> 
> > -Sergey
> > 
> > > >  	} else {
> > > >  		return -ENODEV;
> > > >  	}
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்

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

* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-28 11:59         ` Serge Semin
@ 2022-11-28 12:41           ` Yoshihiro Shimoda
  2022-11-28 16:11             ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-28 12:41 UTC (permalink / raw)
  To: Serge Semin
  Cc: Manivannan Sadhasivam, lpieralisi, robh+dt, kw, bhelgaas,
	krzk+dt, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc, Sergey.Semin

Hi Serge,

> From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> 
> On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge,
> >
> > > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > >
<snip>
> > > What does the dbi+0x8f8 CSR contain in the host
> > > and EP registers space? Similarly could you also provide a content of
> > > the +0x978 register?
> >
> > The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> > --------------- (sorry, replace tabs with spaces...)---------------
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  {
> >         u32 val;
> >
> 
> > +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > +               dw_pcie_readl_dma(pci, 0x8f8),
> > +               dw_pcie_readl_dma(pci, 0x978));
> > +
> 
> No, this should have been the dw_pcie_readl_dbi() calls instead of
> dw_pcie_readl_!dma!(). What I try to understand from these values is
> the real version of your controller (dbi+0x8f8) and whether the legacy
> eDMA viewport registers range follows the documented convention of
> having FFs in the dbi+0x978 register. My current assumption that
> either your IP-core is newer than v5.30a or has some vendor-specific
> modification. But let's see the value first.

Oops! I'm sorry for my bad code. After fixed the code, the values are:
---
[    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
---

<snip>
> > So, should I change the condition like below?
> >
> > ---
> > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > ...
> > -	} else if (val != 0xFFFFFFFF) {
> > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > ---
> 
> Definitely no. Even though it's impossible to have the eDMA controller
> configured with zero number of read and write channels we shouldn't
> assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> means having the unrolled eDMA CSRs mapping. Let's have a look at the
> content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> values we'll come up with what to do next.

I got it.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > -Sergey
> > >
> > > > >  	} else {
> > > > >  		return -ENODEV;
> > > > >  	}
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > > --
> > > > மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-28 12:41           ` Yoshihiro Shimoda
@ 2022-11-28 16:11             ` Serge Semin
  2022-11-29  0:21               ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-11-28 16:11 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > 
> > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > Hi Serge,
> > >
> > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > > >
> <snip>
> > > > What does the dbi+0x8f8 CSR contain in the host
> > > > and EP registers space? Similarly could you also provide a content of
> > > > the +0x978 register?
> > >
> > > The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> > > --------------- (sorry, replace tabs with spaces...)---------------
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  {
> > >         u32 val;
> > >
> > 
> > > +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > +               dw_pcie_readl_dma(pci, 0x8f8),
> > > +               dw_pcie_readl_dma(pci, 0x978));
> > > +
> > 
> > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > the real version of your controller (dbi+0x8f8) and whether the legacy
> > eDMA viewport registers range follows the documented convention of
> > having FFs in the dbi+0x978 register. My current assumption that
> > either your IP-core is newer than v5.30a or has some vendor-specific
> > modification. But let's see the value first.
> 

> Oops! I'm sorry for my bad code. After fixed the code, the values are:
> ---
> [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> ---

@Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
that what the content of the +0x978 CSR was really read by means of the
dw_pcie_readl_dbi() method?

@Mani, could you please have a look at the content of the +0x8f8 and
+0x978 CSRs in the CDM space of the available to you controllers?

I've reproduced the behavior what discovered by @Yoshihiro, but on
v5.40a IP-core only. It was expected for that controller version, but
v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
space. It's strange to see it didn't.

-Sergey

> 
> <snip>
> > > So, should I change the condition like below?
> > >
> > > ---
> > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > ...
> > > -	} else if (val != 0xFFFFFFFF) {
> > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > ---
> > 
> > Definitely no. Even though it's impossible to have the eDMA controller
> > configured with zero number of read and write channels we shouldn't
> > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > values we'll come up with what to do next.
> 
> I got it.
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > -Serge(y)
> > 
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > > -Sergey
> > > >
> > > > > >  	} else {
> > > > > >  		return -ENODEV;
> > > > > >  	}
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > >
> > > > > --
> > > > > மணிவண்ணன் சதாசிவம்

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

* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-28 16:11             ` Serge Semin
@ 2022-11-29  0:21               ` Yoshihiro Shimoda
  2022-12-08 12:26                 ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-11-29  0:21 UTC (permalink / raw)
  To: Serge Semin, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

Hi Serge,

> From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> 
> On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge,
> >
> > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > >
> > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > Hi Serge,
<snip>
> > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > eDMA viewport registers range follows the documented convention of
> > > having FFs in the dbi+0x978 register. My current assumption that
> > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > modification. But let's see the value first.
> >
> 
> > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > ---
> > [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > ---
> 
> @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> that what the content of the +0x978 CSR was really read by means of the
> dw_pcie_readl_dbi() method?

Yes, I used dw_pcie_readl_dbi() like below.

--------------- (sorry, tabs replaced with spaces) ---------------
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 {
        u32 val;

+       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
+               dw_pcie_readl_dbi(pci, 0x8f8),
+               dw_pcie_readl_dbi(pci, 0x978));
+
        if (pci->edma.reg_base) {
                pci->edma.mf = EDMA_MF_EDMA_UNROLL;
------------------------------------------------------------------

Best regards,
Yoshihiro Shimoda

> @Mani, could you please have a look at the content of the +0x8f8 and
> +0x978 CSRs in the CDM space of the available to you controllers?
> 
> I've reproduced the behavior what discovered by @Yoshihiro, but on
> v5.40a IP-core only. It was expected for that controller version, but
> v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> space. It's strange to see it didn't.
> 
> -Sergey
> 
> >
> > <snip>
> > > > So, should I change the condition like below?
> > > >
> > > > ---
> > > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > ...
> > > > -	} else if (val != 0xFFFFFFFF) {
> > > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > ---
> > >
> > > Definitely no. Even though it's impossible to have the eDMA controller
> > > configured with zero number of read and write channels we shouldn't
> > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > values we'll come up with what to do next.
> >
> > I got it.
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > -Serge(y)
> > >
> > > >
> > > > Best regards,
> > > > Yoshihiro Shimoda
> > > >
> > > > > -Sergey
> > > > >
> > > > > > >  	} else {
> > > > > > >  		return -ENODEV;
> > > > > > >  	}
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > மணிவண்ணன் சதாசிவம்

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

* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-11-29  0:21               ` Yoshihiro Shimoda
@ 2022-12-08 12:26                 ` Yoshihiro Shimoda
  2022-12-08 14:01                   ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-12-08 12:26 UTC (permalink / raw)
  To: Serge Semin, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

Hi Serge, Manivannan,

> From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
> 
> > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> >
> > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > Hi Serge,
> > >
> > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > >
> > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > Hi Serge,
> <snip>
> > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > eDMA viewport registers range follows the documented convention of
> > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > modification. But let's see the value first.
> > >
> >
> > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > ---
> > > [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > ---
> >
> > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > that what the content of the +0x978 CSR was really read by means of the
> > dw_pcie_readl_dbi() method?
> 
> Yes, I used dw_pcie_readl_dbi() like below.
> 
> --------------- (sorry, tabs replaced with spaces) ---------------
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  {
>         u32 val;
> 
> +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> +               dw_pcie_readl_dbi(pci, 0x8f8),
> +               dw_pcie_readl_dbi(pci, 0x978));
> +
>         if (pci->edma.reg_base) {
>                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> ------------------------------------------------------------------
> 
> > @Mani, could you please have a look at the content of the +0x8f8 and
> > +0x978 CSRs in the CDM space of the available to you controllers?
> >
> > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > v5.40a IP-core only. It was expected for that controller version, but
> > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > space. It's strange to see it didn't.

So, should I add a quirk flag for my controller like below?
---
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 69665a8a002e..384bd2c0ea75 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
        u32 val;

        val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
-       if (val == 0xFFFFFFFF && pci->edma.reg_base) {
+       if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
                pci->edma.mf = EDMA_MF_EDMA_UNROLL;

                val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
---

Best regards,
Yoshihiro Shimoda

> > -Sergey
> >
> > >
> > > <snip>
> > > > > So, should I change the condition like below?
> > > > >
> > > > > ---
> > > > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > ...
> > > > > -	} else if (val != 0xFFFFFFFF) {
> > > > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > ---
> > > >
> > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > configured with zero number of read and write channels we shouldn't
> > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > values we'll come up with what to do next.
> > >
> > > I got it.
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > > -Serge(y)
> > > >
> > > > >
> > > > > Best regards,
> > > > > Yoshihiro Shimoda
> > > > >
> > > > > > -Sergey
> > > > > >
> > > > > > > >  	} else {
> > > > > > > >  		return -ENODEV;
> > > > > > > >  	}
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-08 12:26                 ` Yoshihiro Shimoda
@ 2022-12-08 14:01                   ` Serge Semin
  2022-12-09  7:45                     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-12-08 14:01 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Frank Li, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

Cc += Frank Li

@Frank could you have a look at the thread and check the content of
the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
devices?

On Thu, Dec 08, 2022 at 12:26:18PM +0000, Yoshihiro Shimoda wrote:
> Hi Serge, Manivannan,
> 
> > From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
> > 
> > > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> > >
> > > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > > Hi Serge,
> > > >
> > > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > > >
> > > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > > Hi Serge,
> > <snip>
> > > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > > eDMA viewport registers range follows the documented convention of
> > > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > > modification. But let's see the value first.
> > > >
> > >
> > > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > > ---
> > > > [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > > ---
> > >
> > > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > > that what the content of the +0x978 CSR was really read by means of the
> > > dw_pcie_readl_dbi() method?
> > 
> > Yes, I used dw_pcie_readl_dbi() like below.
> > 
> > --------------- (sorry, tabs replaced with spaces) ---------------
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  {
> >         u32 val;
> > 
> > +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > +               dw_pcie_readl_dbi(pci, 0x8f8),
> > +               dw_pcie_readl_dbi(pci, 0x978));
> > +
> >         if (pci->edma.reg_base) {
> >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > ------------------------------------------------------------------
> > 
> > > @Mani, could you please have a look at the content of the +0x8f8 and
> > > +0x978 CSRs in the CDM space of the available to you controllers?
> > >
> > > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > > v5.40a IP-core only. It was expected for that controller version, but
> > > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > > space. It's strange to see it didn't.
> 

> So, should I add a quirk flag for my controller like below?
> ---
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 69665a8a002e..384bd2c0ea75 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>         u32 val;
> 
>         val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);

> -       if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> +       if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
>                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> 
>                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);

Very much no. I have just got rid from such boolean flags from the DW
PCI private data.

Please be patient. Maintainers not always respond as soon as we would
want. Please note my patchset also stalls due to your discovery (DW
eDMA patches still under review).

What we have discovered here is the undocumented behavior. The
HW-manuals from 4.80 up to 5.30 say that the register dbi+0x978 must
have FFs if the register doesn't exist. A similar rule is defined for
the iATU CSRs space and it's working well at least up to IP-core
5.40a. The viewport-based eDMA CSRs space has been removed from the
HW-manuals since IP-core 5.40a and I can assure that IP-core 5.40a has
zeros in dbi+0x978 on the real HW. I do have another devices with eDMA
but all of them have been synthesized with the legacy viewport-based
eDMA access, so I can't check whether the FFs statement is correct for
the devices older than 5.40. You detected the problem on the IP-core
5.20a, but @Mani didn't see it on his devices. Neither does Frank
AFAIU. That's why I asked him and @Frank to check what value the
dbi+0x8f8 and dbi+0x978 registers have in their devices at hand. After
that we'll either add the IP-core version based test here:
< -       val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
< +       if (dw_pcie_ver_is_ge(pci, 5?0A)) {
< +               val = 0xFFFFFFFF;
< +       else
< +               val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
or add a new capability flag if @Mani has IP-core 5.20a with FFs in
the CSRs dbi+0x978. Like this:
< -       val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
< +       if (dw_pcie_cap_is(pci, EDMA_UNROLL)) {
< +               val = 0xFFFFFFFF;
< +       else
< +               val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);

So the main goal of this activity is to check whether your discovery
is a bug on your particular device or is a bug in the HW-manuals. If
the later is correct then the eDMA CSRs space auto-detection procedure
should be fixed to be executed up to the corresponding IP-core
version. If the former is correct then we'll add a new capability
flag. In anyway having the new boolean field in the device private
data wouldn't be a good option since there is the capabilities API
available in the driver.

-Serge(y)

> ---
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > > -Sergey
> > >
> > > >
> > > > <snip>
> > > > > > So, should I change the condition like below?
> > > > > >
> > > > > > ---
> > > > > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > > ...
> > > > > > -	} else if (val != 0xFFFFFFFF) {
> > > > > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > > ---
> > > > >
> > > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > > configured with zero number of read and write channels we shouldn't
> > > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > > values we'll come up with what to do next.
> > > >
> > > > I got it.
> > > >
> > > > Best regards,
> > > > Yoshihiro Shimoda
> > > >
> > > > > -Serge(y)
> > > > >
> > > > > >
> > > > > > Best regards,
> > > > > > Yoshihiro Shimoda
> > > > > >
> > > > > > > -Sergey
> > > > > > >
> > > > > > > > >  	} else {
> > > > > > > > >  		return -ENODEV;
> > > > > > > > >  	}
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > மணிவண்ணன் சதாசிவம்

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

* RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-08 14:01                   ` Serge Semin
@ 2022-12-09  7:45                     ` Yoshihiro Shimoda
       [not found]                       ` <HE1PR0401MB23319A9F4AF7630A82249D65881C9@HE1PR0401MB2331.eurprd04.prod.outlook.com>
  0 siblings, 1 reply; 27+ messages in thread
From: Yoshihiro Shimoda @ 2022-12-09  7:45 UTC (permalink / raw)
  To: Serge Semin, Frank Li, Manivannan Sadhasivam
  Cc: lpieralisi, robh+dt, kw, bhelgaas, krzk+dt, marek.vasut+renesas,
	linux-pci, devicetree, linux-renesas-soc, Sergey.Semin

Hi Serge,

> From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> 
> Cc += Frank Li
> 
> @Frank could you have a look at the thread and check the content of
> the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> devices?
> 
> On Thu, Dec 08, 2022 at 12:26:18PM +0000, Yoshihiro Shimoda wrote:
> > Hi Serge, Manivannan,
> >
> > > From: Yoshihiro Shimoda, Sent: Tuesday, November 29, 2022 9:22 AM
> > >
> > > > From: Serge Semin, Sent: Tuesday, November 29, 2022 1:11 AM
> > > >
> > > > On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> > > > > Hi Serge,
> > > > >
> > > > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > > > > >
> > > > > > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > > > > > Hi Serge,
> > > <snip>
> > > > > > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > > > > > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > > > > > the real version of your controller (dbi+0x8f8) and whether the legacy
> > > > > > eDMA viewport registers range follows the documented convention of
> > > > > > having FFs in the dbi+0x978 register. My current assumption that
> > > > > > either your IP-core is newer than v5.30a or has some vendor-specific
> > > > > > modification. But let's see the value first.
> > > > >
> > > >
> > > > > Oops! I'm sorry for my bad code. After fixed the code, the values are:
> > > > > ---
> > > > > [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> > > > > ---
> > > >
> > > > @Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
> > > > that what the content of the +0x978 CSR was really read by means of the
> > > > dw_pcie_readl_dbi() method?
> > >
> > > Yes, I used dw_pcie_readl_dbi() like below.
> > >
> > > --------------- (sorry, tabs replaced with spaces) ---------------
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  {
> > >         u32 val;
> > >
> > > +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > +               dw_pcie_readl_dbi(pci, 0x8f8),
> > > +               dw_pcie_readl_dbi(pci, 0x978));
> > > +
> > >         if (pci->edma.reg_base) {
> > >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > > ------------------------------------------------------------------
> > >
> > > > @Mani, could you please have a look at the content of the +0x8f8 and
> > > > +0x978 CSRs in the CDM space of the available to you controllers?
> > > >
> > > > I've reproduced the behavior what discovered by @Yoshihiro, but on
> > > > v5.40a IP-core only. It was expected for that controller version, but
> > > > v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
> > > > space. It's strange to see it didn't.
> >
> 
> > So, should I add a quirk flag for my controller like below?
> > ---
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 69665a8a002e..384bd2c0ea75 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -844,7 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >         u32 val;
> >
> >         val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> 
> > -       if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > +       if ((pci->no_dma_ctrl_reg || val == 0xFFFFFFFF) && pci->edma.reg_base) {
> >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >
> >                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> 
> Very much no. I have just got rid from such boolean flags from the DW
> PCI private data.
> 
> Please be patient. Maintainers not always respond as soon as we would
> want. Please note my patchset also stalls due to your discovery (DW
> eDMA patches still under review).
> 
> What we have discovered here is the undocumented behavior. The
> HW-manuals from 4.80 up to 5.30 say that the register dbi+0x978 must
> have FFs if the register doesn't exist. A similar rule is defined for
> the iATU CSRs space and it's working well at least up to IP-core
> 5.40a. The viewport-based eDMA CSRs space has been removed from the
> HW-manuals since IP-core 5.40a and I can assure that IP-core 5.40a has
> zeros in dbi+0x978 on the real HW. I do have another devices with eDMA
> but all of them have been synthesized with the legacy viewport-based
> eDMA access, so I can't check whether the FFs statement is correct for
> the devices older than 5.40. You detected the problem on the IP-core
> 5.20a, but @Mani didn't see it on his devices. Neither does Frank
> AFAIU. That's why I asked him and @Frank to check what value the
> dbi+0x8f8 and dbi+0x978 registers have in their devices at hand. After
> that we'll either add the IP-core version based test here:
> < -       val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> < +       if (dw_pcie_ver_is_ge(pci, 5?0A)) {
> < +               val = 0xFFFFFFFF;
> < +       else
> < +               val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> or add a new capability flag if @Mani has IP-core 5.20a with FFs in
> the CSRs dbi+0x978. Like this:
> < -       val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> < +       if (dw_pcie_cap_is(pci, EDMA_UNROLL)) {
> < +               val = 0xFFFFFFFF;
> < +       else
> < +               val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> 
> So the main goal of this activity is to check whether your discovery
> is a bug on your particular device or is a bug in the HW-manuals. If
> the later is correct then the eDMA CSRs space auto-detection procedure
> should be fixed to be executed up to the corresponding IP-core
> version. If the former is correct then we'll add a new capability
> flag. In anyway having the new boolean field in the device private
> data wouldn't be a good option since there is the capabilities API
> available in the driver.

Thank you very much for your detailed explanation! I understood.
# At least, I should have realized that the PCI dwc driver has
# the capabilities API before I send an email...

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > ---
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > > -Sergey
> > > >
> > > > >
> > > > > <snip>
> > > > > > > So, should I change the condition like below?
> > > > > > >
> > > > > > > ---
> > > > > > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > > > > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > > > > > ...
> > > > > > > -	} else if (val != 0xFFFFFFFF) {
> > > > > > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > > > > > ---
> > > > > >
> > > > > > Definitely no. Even though it's impossible to have the eDMA controller
> > > > > > configured with zero number of read and write channels we shouldn't
> > > > > > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > > > > > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > > > > > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > > > > > values we'll come up with what to do next.
> > > > >
> > > > > I got it.
> > > > >
> > > > > Best regards,
> > > > > Yoshihiro Shimoda
> > > > >
> > > > > > -Serge(y)
> > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Yoshihiro Shimoda
> > > > > > >
> > > > > > > > -Sergey
> > > > > > > >
> > > > > > > > > >  	} else {
> > > > > > > > > >  		return -ENODEV;
> > > > > > > > > >  	}
> > > > > > > > > > --
> > > > > > > > > > 2.25.1
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > மணிவண்ணன் சதாசிவம்

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

* Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
       [not found]                       ` <HE1PR0401MB23319A9F4AF7630A82249D65881C9@HE1PR0401MB2331.eurprd04.prod.outlook.com>
@ 2022-12-11 15:28                         ` Serge Semin
  2022-12-12 12:56                           ` Manivannan Sadhasivam
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-12-11 15:28 UTC (permalink / raw)
  To: Frank Li
  Cc: Yoshihiro Shimoda, Manivannan Sadhasivam, lpieralisi, robh+dt,
	kw, bhelgaas, krzk+dt, marek.vasut+renesas, linux-pci,
	devicetree, linux-renesas-soc, Sergey.Semin

Hi Frank

On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> >
> > Cc += Frank Li
> >
> > @Frank could you have a look at the thread and check the content of
> > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > devices?
> 

> [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001

Thanks for the reply. So it's 4.80a with the legacy viewport-based
access. Alas it isn't what we need in this thread. We'll need
@Mani's respond in order to decide how to fix the auto-detection
procedure.

-Serge(y)

> 
> Frank Li
> 
> 

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

* Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-11 15:28                         ` [EXT] " Serge Semin
@ 2022-12-12 12:56                           ` Manivannan Sadhasivam
  2022-12-12 16:56                             ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Manivannan Sadhasivam @ 2022-12-12 12:56 UTC (permalink / raw)
  To: Serge Semin
  Cc: Frank Li, Yoshihiro Shimoda, lpieralisi, robh+dt, kw, bhelgaas,
	krzk+dt, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc, Sergey.Semin

Hi Serge,

On Sun, Dec 11, 2022 at 06:28:49PM +0300, Serge Semin wrote:
> Hi Frank
> 
> On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> > Hi Serge,
> > 
> > > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> > >
> > > Cc += Frank Li
> > >
> > > @Frank could you have a look at the thread and check the content of
> > > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > > devices?
> > 
> 
> > [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001
> 
> Thanks for the reply. So it's 4.80a with the legacy viewport-based
> access. Alas it isn't what we need in this thread. We'll need
> @Mani's respond in order to decide how to fix the auto-detection
> procedure.
> 

Sorry for the late reply!

With below diff on the EP:

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 6f3805228a18..0eb4d3218738 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -665,6 +665,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
        if (val == 0xFFFFFFFF && pci->edma.reg_base) {
                pci->edma.mf = EDMA_MF_EDMA_UNROLL;
 
+               dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
+                       dw_pcie_readl_dbi(pci, 0x8f8),
+                       dw_pcie_readl_dbi(pci, 0x978));
+
                val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
        } else if (val != 0xFFFFFFFF) {
                pci->edma.mf = EDMA_MF_EDMA_LEGACY;


The output was:

qcom-pcie-ep 1c08000.pcie-ep: dw_pcie_edma_find_chip: +0x8f8 = 3533302a, +0x978 = ffffffff

Hope this helps!

Thanks,
Mani

> -Serge(y)
> 
> > 
> > Frank Li
> > 
> > 

-- 
மணிவண்ணன் சதாசிவம்

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

* Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-12 12:56                           ` Manivannan Sadhasivam
@ 2022-12-12 16:56                             ` Serge Semin
  2022-12-12 17:11                               ` Manivannan Sadhasivam
  0 siblings, 1 reply; 27+ messages in thread
From: Serge Semin @ 2022-12-12 16:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Frank Li, Yoshihiro Shimoda, lpieralisi, robh+dt, kw, bhelgaas,
	krzk+dt, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc, Sergey.Semin

On Mon, Dec 12, 2022 at 06:26:58PM +0530, Manivannan Sadhasivam wrote:
> Hi Serge,
> 
> On Sun, Dec 11, 2022 at 06:28:49PM +0300, Serge Semin wrote:
> > Hi Frank
> > 
> > On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> > > Hi Serge,
> > > 
> > > > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> > > >
> > > > Cc += Frank Li
> > > >
> > > > @Frank could you have a look at the thread and check the content of
> > > > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > > > devices?
> > > 
> > 
> > > [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001
> > 
> > Thanks for the reply. So it's 4.80a with the legacy viewport-based
> > access. Alas it isn't what we need in this thread. We'll need
> > @Mani's respond in order to decide how to fix the auto-detection
> > procedure.
> > 
> 

> Sorry for the late reply!
> 
> With below diff on the EP:
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 6f3805228a18..0eb4d3218738 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -665,6 +665,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>         if (val == 0xFFFFFFFF && pci->edma.reg_base) {
>                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
>  
> +               dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> +                       dw_pcie_readl_dbi(pci, 0x8f8),
> +                       dw_pcie_readl_dbi(pci, 0x978));
> +
>                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
>         } else if (val != 0xFFFFFFFF) {
>                 pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> 
> 
> The output was:
> 
> qcom-pcie-ep 1c08000.pcie-ep: dw_pcie_edma_find_chip: +0x8f8 = 3533302a, +0x978 = ffffffff
> 
> Hope this helps!

Great! Thanks. This indeed helps. So it's 5.30a IP-core. Just one
quick question. Does that device have eDMA embedded into the DW PCIe
controller?

-Serge(y)

> 
> Thanks,
> Mani
> 
> > -Serge(y)
> > 
> > > 
> > > Frank Li
> > > 
> > > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

* Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-12 16:56                             ` Serge Semin
@ 2022-12-12 17:11                               ` Manivannan Sadhasivam
  2022-12-13 23:11                                 ` Serge Semin
  0 siblings, 1 reply; 27+ messages in thread
From: Manivannan Sadhasivam @ 2022-12-12 17:11 UTC (permalink / raw)
  To: Serge Semin
  Cc: Frank Li, Yoshihiro Shimoda, lpieralisi, robh+dt, kw, bhelgaas,
	krzk+dt, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc, Sergey.Semin

On Mon, Dec 12, 2022 at 07:56:00PM +0300, Serge Semin wrote:
> On Mon, Dec 12, 2022 at 06:26:58PM +0530, Manivannan Sadhasivam wrote:
> > Hi Serge,
> > 
> > On Sun, Dec 11, 2022 at 06:28:49PM +0300, Serge Semin wrote:
> > > Hi Frank
> > > 
> > > On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> > > > Hi Serge,
> > > > 
> > > > > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> > > > >
> > > > > Cc += Frank Li
> > > > >
> > > > > @Frank could you have a look at the thread and check the content of
> > > > > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > > > > devices?
> > > > 
> > > 
> > > > [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001
> > > 
> > > Thanks for the reply. So it's 4.80a with the legacy viewport-based
> > > access. Alas it isn't what we need in this thread. We'll need
> > > @Mani's respond in order to decide how to fix the auto-detection
> > > procedure.
> > > 
> > 
> 
> > Sorry for the late reply!
> > 
> > With below diff on the EP:
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 6f3805228a18..0eb4d3218738 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -665,6 +665,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >         if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >  
> > +               dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > +                       dw_pcie_readl_dbi(pci, 0x8f8),
> > +                       dw_pcie_readl_dbi(pci, 0x978));
> > +
> >                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> >         } else if (val != 0xFFFFFFFF) {
> >                 pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > 
> > 
> > The output was:
> > 
> > qcom-pcie-ep 1c08000.pcie-ep: dw_pcie_edma_find_chip: +0x8f8 = 3533302a, +0x978 = ffffffff
> > 
> > Hope this helps!
> 
> Great! Thanks. This indeed helps. So it's 5.30a IP-core. Just one
> quick question. Does that device have eDMA embedded into the DW PCIe
> controller?
> 

Yes it is and it is the test platform I use for eDMA/PCI_EP work.

Thanks,
Mani

> -Serge(y)
> 
> > 
> > Thanks,
> > Mani
> > 
> > > -Serge(y)
> > > 
> > > > 
> > > > Frank Li
> > > > 
> > > > 
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்

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

* Re: [EXT] RE: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists
  2022-12-12 17:11                               ` Manivannan Sadhasivam
@ 2022-12-13 23:11                                 ` Serge Semin
  0 siblings, 0 replies; 27+ messages in thread
From: Serge Semin @ 2022-12-13 23:11 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Yoshihiro Shimoda
  Cc: Frank Li, lpieralisi, robh+dt, kw, bhelgaas, krzk+dt,
	marek.vasut+renesas, linux-pci, devicetree, linux-renesas-soc

On Mon, Dec 12, 2022 at 10:41:02PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Dec 12, 2022 at 07:56:00PM +0300, Serge Semin wrote:
> > On Mon, Dec 12, 2022 at 06:26:58PM +0530, Manivannan Sadhasivam wrote:
> > > Hi Serge,
> > > 
> > > On Sun, Dec 11, 2022 at 06:28:49PM +0300, Serge Semin wrote:
> > > > Hi Frank
> > > > 
> > > > On Fri, Dec 09, 2022 at 03:52:42PM +0000, Frank Li wrote:
> > > > > Hi Serge,
> > > > > 
> > > > > > From: Serge Semin, Sent: Thursday, December 8, 2022 11:01 PM
> > > > > >
> > > > > > Cc += Frank Li
> > > > > >
> > > > > > @Frank could you have a look at the thread and check the content of
> > > > > > the CSRs dbi+0x8f8 and dbi+0x978 on available to you DW PCIe +EDMA
> > > > > > devices?
> > > > > 
> > > > 
> > > > > [    2.598038] imx6q-pcie 5f010000.pcie_ep: imx_add_pcie_ep: +0x8f8 = 3438302a, +0x978 = 00010001
> > > > 
> > > > Thanks for the reply. So it's 4.80a with the legacy viewport-based
> > > > access. Alas it isn't what we need in this thread. We'll need
> > > > @Mani's respond in order to decide how to fix the auto-detection
> > > > procedure.
> > > > 
> > > 
> > 
> > > Sorry for the late reply!
> > > 
> > > With below diff on the EP:
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 6f3805228a18..0eb4d3218738 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -665,6 +665,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >         if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > >                 pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> > >  
> > > +               dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > +                       dw_pcie_readl_dbi(pci, 0x8f8),
> > > +                       dw_pcie_readl_dbi(pci, 0x978));
> > > +
> > >                 val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > >         } else if (val != 0xFFFFFFFF) {
> > >                 pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> > > 
> > > 
> > > The output was:
> > > 
> > > qcom-pcie-ep 1c08000.pcie-ep: dw_pcie_edma_find_chip: +0x8f8 = 3533302a, +0x978 = ffffffff
> > > 
> > > Hope this helps!
> > 
> > Great! Thanks. This indeed helps. So it's 5.30a IP-core. Just one
> > quick question. Does that device have eDMA embedded into the DW PCIe
> > controller?
> > 
> 

> Yes it is and it is the test platform I use for eDMA/PCI_EP work.

So the procedure works well for IP-core 5.30a and AFAICS it doesn't
for 5.40a (eDMA viewport-based CSRs are missing in the HW-manual) and
for an unexpected reason in IP-core 5.20a synthesized for Renesas
R-Car Gen4 PCIe. Thus this seems more like a vendor-specific problem,
than a version-specific one since the HW-manual in both 5.20a and
5.30a cases state that the dbi+0x978 register must have FFs if the CSR
doesn't exist. It doesn't exist if the next statement is false:
!CX_PL_REG_DISABLE && CC_DMA_ENABLE && !CC_UNROLL_ENABLE && CC_DEVICE_TYPE!=3
So seeing the R-Car Gen4 PCIe has the unrolled eDMA mapping the
dbi+0x978 registers must contain FFs.

The best solution in this case would be to have a special
capability flag which would force the unrolled eDMA mapping for the
problematic devices. Like this:
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -840,8 +840,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);
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -52,7 +52,8 @@
 /* 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		2
+#define DW_PCIE_CAP_CDM_CHECK		3
 
 #define dw_pcie_cap_is(_pci, _cap) \
 	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)

The patch above is based on the updated version of my patchset, which
I'll resubmit for review tomorrow. I'll add @Yoshihiro in Cc-list of
the series.

-Serge(y)

> 
> Thanks,
> Mani
> 
> > -Serge(y)
> > 
> > > 
> > > Thanks,
> > > Mani
> > > 
> > > > -Serge(y)
> > > > 
> > > > > 
> > > > > Frank Li
> > > > > 
> > > > > 
> > > 
> > > -- 
> > > மணிவண்ணன் சதாசிவம்
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

end of thread, other threads:[~2022-12-13 23:11 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-21 12:43 [PATCH v7 0/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe support Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 1/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 2/9] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 3/9] PCI: Add PCI_EXP_LNKCAP_MLW macros Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 4/9] PCI: designware-ep: Expose dw_pcie_ep_exit() to module Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists Yoshihiro Shimoda
2022-11-22 13:55   ` Manivannan Sadhasivam
2022-11-27 23:55     ` Serge Semin
2022-11-28  2:52       ` Yoshihiro Shimoda
2022-11-28 11:59         ` Serge Semin
2022-11-28 12:41           ` Yoshihiro Shimoda
2022-11-28 16:11             ` Serge Semin
2022-11-29  0:21               ` Yoshihiro Shimoda
2022-12-08 12:26                 ` Yoshihiro Shimoda
2022-12-08 14:01                   ` Serge Semin
2022-12-09  7:45                     ` Yoshihiro Shimoda
     [not found]                       ` <HE1PR0401MB23319A9F4AF7630A82249D65881C9@HE1PR0401MB2331.eurprd04.prod.outlook.com>
2022-12-11 15:28                         ` [EXT] " Serge Semin
2022-12-12 12:56                           ` Manivannan Sadhasivam
2022-12-12 16:56                             ` Serge Semin
2022-12-12 17:11                               ` Manivannan Sadhasivam
2022-12-13 23:11                                 ` Serge Semin
2022-11-21 12:43 ` [PATCH v7 6/9] PCI: dwc: Add support for triggering legacy IRQs Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 7/9] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support Yoshihiro Shimoda
2022-11-22 15:04   ` Bjorn Helgaas
2022-11-25 11:37     ` Yoshihiro Shimoda
2022-11-21 12:43 ` [PATCH v7 8/9] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support Yoshihiro Shimoda
2022-11-21 12:44 ` [PATCH v7 9/9] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 Yoshihiro Shimoda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.