devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] Add driver support for Eswin EIC7700 SoC PCIe controller
@ 2025-11-20 10:10 zhangsenchuan
  2025-11-20 10:11 ` [PATCH v6 1/3] dt-bindings: PCI: eic7700: Add Eswin PCIe host controller zhangsenchuan
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: zhangsenchuan @ 2025-11-20 10:10 UTC (permalink / raw)
  To: bhelgaas, mani, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama
  Cc: ningyu, linmin, pinkesh.vaghela, ouyanghui, Frank.li,
	Senchuan Zhang

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Changes in v6:
- Updates: eswin,eic7700-pcie.yaml
  - Add Reviewed-by: Rob Herring (Arm) <robh@kernel.org>.

- Updates: pcie-eic7700.c
  - Remove pci_root_ports_have_device function judgment during suspend.
  - Remove eic7700_pcie_pme_turn_off and eic7700_pcie_get_ltssm function.
  - Add set no_suspport_L2 flag.

- Updates: pcie-designware.h pcie-designware-host.c
 - The ESWIN EIC7700 soc does not support enter L2 link state. Therefore
   add no_suspport_L2 flag skip PME_Turn_Off broadcast and link state
   check code, other driver can reuse this flag if meet the similar
   situation.
- Link to V5: https://lore.kernel.org/all/20251110090716.1392-1-zhangsenchuan@eswincomputing.com/
- Link to: https://lore.kernel.org/all/e7plmtwtkkd4ymrt2hkztcqdx4ugfjk64oksjyf6lpi2oui53d@vhuo5occyref/

Changes in v5:
- Updates: eswin,eic7700-pcie.yaml
  - Modify reg-names: update mgmt to elbi.
  - Modify clock-names: update pclk to phy_reg.
  - Modify reset-names: update powerup to pwr.
  - Remove powerup modify in "snps,dw-pcie-common.yaml" file.

- Updates: pcie-eic7700.c
  - Update the driver submission comment, mention EIC7700 in the
    "config PCIE_EIC7700" and in the driver title.
  - Update some comments, for examples: "s/PME_TURN_OFF/PME_Turn_Off/",
    "s/INTX/INTx/", "s/PERST/PERST#/", "s/perst/PERST#/", "s/id/ID/".
  - Update "struct *_pcie" name and function name, add the eic7700 prefix.
  - Use PCIEELBI_CTRL0_DEV_TYPE macro and update comment, use FIELD_PREP.
  - Add eic7700_pcie_data pointer in struct eic7700_pcie.
  - Update .deinit callback function name and removed the dw_pcie_link_up
    judgment, add pci_root_ports_have_device function judgment.
  - Remove devm_platform_ioremap_resource_byname function get mgmt, use
    platform_get_resource_byname function get elbi in "pcie-designware.c".
  - Update of_reset_control_get to of_reset_control_get_exclusive, use
    devm_reset_control_bulk_get_exclusive function get resets, update use
    reset_control_bulk_assert/reset_control_bulk_deassert function.
- Link to V4: https://lore.kernel.org/all/20251030082900.1304-1-zhangsenchuan@eswincomputing.com/
- Link to https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/?h=controller/dwc

Changes in v4:
- Updates: eswin,eic7700-pcie.yaml
  - Use snps,dw-pcie.yaml instead pci-host-bridge.yaml.

- Updates: snps,dw-pcie-common.yaml
  - Add powerup reset property, our powerup property is somewhat different
    from the general attributes defined by Synopsys DWC binding.

- Updates: pcie-eic7700.c
  - Update the driver submission comment.
  - Alphabetize so the menuconfig entries remain sorted by vendor.
  - Update use PCI_CAP_LIST_NEXT_MASK macro.
  - Use readl_poll_timeout function.
  - Update eswin_pcie_suspend/eswin_pcie_resume name to
    eswin_pcie_suspend_noirq/eswin_pcie_resume_noirq.
  - PM use dw_pcie_suspend_noirq and dw_pcie_resume_noirq function and add
    eswin_pcie_get_ltssm, eswin_pcie_pme_turn_off, eswin_pcie_host_exit
    function adapt to PM.
- Link to V3: https://lore.kernel.org/linux-pci/20250923120946.1218-1-zhangsenchuan@eswincomputing.com/

Changes in v3:
- Updates: eswin,eic7700-pcie.yaml
  - Based on the last patch yaml file, devicetree separates the root port
    node, changing it significantly. Therefore, "Reviewed-by: Krzysztof
    Kozlowski <krzysztof.kozlowski@linaro.org>" is not added.
  - Clock and reset drivers are under review. In yaml, macro definitions
    used in clock and reset can only be replaced by constant values.
  - Move the num-lanes and perst resets to the PCIe Root Port node, make
    it easier to support multiple Root Ports in future versions of the
    hardware.
  - Update the num-lanes attribute and modify define num-lanes as decimal.
  - Optimize the ranges attribute and clear the relocatable flag (bit 31)
    for any regions.
  - Update comment: inte~inth are actual interrupts and these names align
    with the interrupt names in the hardware IP, inte~inth interrupts
    corresponds to Deassert_INTA~Deassert_INTD.
  - Add Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>.

- Updates: pcie-eic7700.c
  - Update the submission comment and add DWC IP revision, data rate, lane
    information.
  - Optimize the "config PCIE_EIC7700" configuration.
  - Optimize the macro definition, add bitfield definition for the mask,
    and remove redundant comments. optimize comments, make use of 80
    columns for comments.
  - Use the dw_pcie_find_capability function to obtain the offset by
    traversing the function list.
  - Remove the sets MPS code and configure it by PCI core.
  - Alphabetize so the menuconfig entries remain sorted by vendor.
  - Configure ESWIN VID:DID for Root Port as the default values are
	invalid,and remove the redundant lane config.
  - Use reverse Xmas order for all local variables in this driver
  - Hardware doesn't support MSI-X but it advertises MSI-X capability, set
    a flag and clear it conditionally.
  - Resets are all necessary, Update the interface function for resets.
  - Since driver does not depend on any parent to power on any resource,
    the pm runtime related functions are removed.
  - Remove "eswin_pcie_shutdown" function, our comment on the shutdown
    function is incorrect. Moreover, when the host powers reboots,it will
    enter the shutdown function, we are using host reset and do not need
    to assert perst. Therefore, the shutdown function is not necessary.
  - remove "eswin_pcie_remove", because it is not safe to remove it during
    runtime, and this driver has been modified to builtin_platform_driver
    and does not support hot plugging, therefore, the remove function is
    not needed.
  - The Suspend function adds link state judgment, and for controllers
    with active devices, resources cannot be turned off.
  - Add Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>.
- Link to V2: https://lore.kernel.org/linux-pci/20250829082021.49-1-zhangsenchuan@eswincomputing.com/

Changes in v2:
- Updates: eswin,eic7700-pcie.yaml
  - Optimize the naming of "clock-names" and "reset-names".
  - Add a reference to "$ref: /schemas/pci/pci-host-bridge.yaml#".
    (The name of the reset attribute in the "snps,dw-pcie-common.yaml"
    file is different from our reset attribute and "snps,dw-pcie.yaml"
    file cannot be directly referenced)
  - Follow DTS coding style to optimize yaml attributes.
  - Remove status = "disabled" from yaml.

- Updates: pcie-eic7700.c
  - Remove unnecessary imported header files.
  - Use dev_err instead of pr_err and remove the WARN_ON function.
  - The eswin_evb_socket_power_on function is removed and not supported.
  - The eswin_pcie_remove function is placed after the probe function.
  - Optimize function alignment.
  - Manage the clock using the devm_clk_bulk_get_all_enabled function.
  - Handle the release of resources after the dw_pcie_host_init function
    call fails.
  - Remove the dev_dbg function and remove __exit_p.
  - Add support for the system pm function.
- Link to V1: https://lore.kernel.org/all/20250516094057.1300-1-zhangsenchuan@eswincomputing.com/

Senchuan Zhang (3):
  dt-bindings: PCI: eic7700: Add Eswin PCIe host controller
  PCI: eic7700: Add Eswin PCIe host controller driver
  PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast

 .../bindings/pci/eswin,eic7700-pcie.yaml      | 167 ++++++++
 drivers/pci/controller/dwc/Kconfig            |  11 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 .../pci/controller/dwc/pcie-designware-host.c |   4 +
 drivers/pci/controller/dwc/pcie-designware.h  |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c     | 387 ++++++++++++++++++
 6 files changed, 571 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c

--
2.25.1


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

* [PATCH v6 1/3] dt-bindings: PCI: eic7700: Add Eswin PCIe host controller
  2025-11-20 10:10 [PATCH v6 0/3] Add driver support for Eswin EIC7700 SoC PCIe controller zhangsenchuan
@ 2025-11-20 10:11 ` zhangsenchuan
  2025-11-20 10:12 ` [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver zhangsenchuan
  2025-11-20 10:12 ` [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast zhangsenchuan
  2 siblings, 0 replies; 15+ messages in thread
From: zhangsenchuan @ 2025-11-20 10:11 UTC (permalink / raw)
  To: bhelgaas, mani, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama
  Cc: ningyu, linmin, pinkesh.vaghela, ouyanghui, Frank.li,
	Senchuan Zhang

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Add Device Tree binding documentation for the Eswin EIC7700 PCIe
controller module, the PCIe controller enables the core to correctly
initialize and manage the PCIe bus and connected devices.

Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/pci/eswin,eic7700-pcie.yaml      | 167 ++++++++++++++++++
 1 file changed, 167 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
new file mode 100644
index 000000000000..9c0150834e6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/eswin,eic7700-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Eswin EIC7700 PCIe host controller
+
+maintainers:
+  - Yu Ning <ningyu@eswincomputing.com>
+  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+  - Yanghui Ou <ouyanghui@eswincomputing.com>
+
+description:
+  Eswin EIC7700 SoC PCIe root complex controller is based on the Synopsys
+  DesignWare PCIe IP.
+
+properties:
+  compatible:
+    const: eswin,eic7700-pcie
+
+  reg:
+    maxItems: 3
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: elbi
+
+  ranges:
+    maxItems: 3
+
+  '#interrupt-cells':
+    const: 1
+
+  interrupt-names:
+    items:
+      - const: msi
+      - const: inta
+      - const: intb
+      - const: intc
+      - const: intd
+
+  interrupt-map:
+    maxItems: 4
+
+  interrupt-map-mask:
+    items:
+      - const: 0
+      - const: 0
+      - const: 0
+      - const: 7
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: mstr
+      - const: dbi
+      - const: phy_reg
+      - const: aux
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: dbi
+      - const: pwr
+
+patternProperties:
+  "^pcie@":
+    type: object
+    $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+      num-lanes:
+        maximum: 4
+
+      resets:
+        maxItems: 1
+
+      reset-names:
+        items:
+          - const: perst
+
+    required:
+      - reg
+      - ranges
+      - num-lanes
+      - resets
+      - reset-names
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - interrupts
+  - interrupt-names
+  - interrupt-map-mask
+  - interrupt-map
+  - '#interrupt-cells'
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie@54000000 {
+            compatible = "eswin,eic7700-pcie";
+            reg = <0x0 0x54000000 0x0 0x4000000>,
+                  <0x0 0x40000000 0x0 0x800000>,
+                  <0x0 0x50000000 0x0 0x100000>;
+            reg-names = "dbi", "config", "elbi";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            ranges = <0x01000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
+                     <0x02000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
+                     <0x43000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
+            bus-range = <0x00 0xff>;
+            clocks = <&clock 144>,
+                     <&clock 145>,
+                     <&clock 146>,
+                     <&clock 147>;
+            clock-names = "mstr", "dbi", "phy_reg", "aux";
+            resets = <&reset 97>,
+                     <&reset 98>;
+            reset-names = "dbi", "pwr";
+            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
+            interrupt-names = "msi", "inta", "intb", "intc", "intd";
+            interrupt-parent = <&plic>;
+            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
+                            <0x0 0x0 0x0 0x2 &plic 180>,
+                            <0x0 0x0 0x0 0x3 &plic 181>,
+                            <0x0 0x0 0x0 0x4 &plic 182>;
+            device_type = "pci";
+            pcie@0 {
+                reg = <0x0 0x0 0x0 0x0 0x0>;
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges;
+                device_type = "pci";
+                num-lanes = <4>;
+                resets = <&reset 99>;
+                reset-names = "perst";
+            };
+        };
+    };
--
2.25.1


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

* [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-20 10:10 [PATCH v6 0/3] Add driver support for Eswin EIC7700 SoC PCIe controller zhangsenchuan
  2025-11-20 10:11 ` [PATCH v6 1/3] dt-bindings: PCI: eic7700: Add Eswin PCIe host controller zhangsenchuan
@ 2025-11-20 10:12 ` zhangsenchuan
  2025-11-20 12:43   ` Manivannan Sadhasivam
  2025-11-20 13:19   ` Shawn Lin
  2025-11-20 10:12 ` [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast zhangsenchuan
  2 siblings, 2 replies; 15+ messages in thread
From: zhangsenchuan @ 2025-11-20 10:12 UTC (permalink / raw)
  To: bhelgaas, mani, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama
  Cc: ningyu, linmin, pinkesh.vaghela, ouyanghui, Frank.li,
	Senchuan Zhang

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Add driver for the Eswin EIC7700 PCIe host controller, which is based on
the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
interrupts.

Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 drivers/pci/controller/dwc/Kconfig        |  11 +
 drivers/pci/controller/dwc/Makefile       |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
 3 files changed, 399 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 349d4657393c..66568efb324f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -93,6 +93,17 @@ config PCIE_BT1
 	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
 	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
 
+config PCIE_EIC7700
+	bool "Eswin EIC7700 PCIe controller"
+	depends on ARCH_ESWIN || COMPILE_TEST
+	depends on PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
+	  The PCIe controller on EIC7700 is based on DesignWare hardware,
+	  enables support for the PCIe controller in the EIC7700 SoC to work in
+	  host mode.
+
 config PCI_IMX6
 	bool
 
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 7ae28f3b0fb3..04f751c49eba 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
 obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
 obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
+obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
new file mode 100644
index 000000000000..239fdbc501fe
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-eic7700.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN EIC7700 PCIe root complex driver
+ *
+ * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
+ *
+ * Authors: Yu Ning <ningyu@eswincomputing.com>
+ *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+ *          Yanghui Ou <ouyanghui@eswincomputing.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+/* ELBI registers */
+#define PCIEELBI_CTRL0_OFFSET		0x0
+#define PCIEELBI_STATUS0_OFFSET		0x100
+
+/* LTSSM register fields */
+#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
+
+/* APP_HOLD_PHY_RST register fields */
+#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
+
+/* PM_SEL_AUX_CLK register fields */
+#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
+
+/* DEV_TYPE register fields */
+#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
+
+/* Vendor and device ID value */
+#define PCI_VENDOR_ID_ESWIN		0x1fe1
+#define PCI_DEVICE_ID_ESWIN		0x2030
+
+#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
+
+static const char * const eic7700_pcie_rsts[] = {
+	"pwr",
+	"dbi",
+};
+
+struct eic7700_pcie_data {
+	bool msix_cap;
+	bool no_suspport_L2;
+};
+
+struct eic7700_pcie_port {
+	struct list_head list;
+	struct reset_control *perst;
+	int num_lanes;
+};
+
+struct eic7700_pcie {
+	struct dw_pcie pci;
+	struct clk_bulk_data *clks;
+	struct reset_control_bulk_data resets[EIC7700_NUM_RSTS];
+	struct list_head ports;
+	const struct eic7700_pcie_data *data;
+	int num_clks;
+};
+
+#define to_eic7700_pcie(x) dev_get_drvdata((x)->dev)
+
+static int eic7700_pcie_start_link(struct dw_pcie *pci)
+{
+	u32 val;
+
+	/* Enable LTSSM */
+	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+	val |= PCIEELBI_APP_LTSSM_ENABLE;
+	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+
+	return 0;
+}
+
+static bool eic7700_pcie_link_up(struct dw_pcie *pci)
+{
+	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+
+	return val & PCI_EXP_LNKSTA_DLLLA;
+}
+
+static int eic7700_pcie_perst_deassert(struct eic7700_pcie_port *port,
+				       struct eic7700_pcie *pcie)
+{
+	int ret;
+
+	ret = reset_control_assert(port->perst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to assert PERST#\n");
+		return ret;
+	}
+
+	/* Ensure that PERST# has been asserted for at least 100 ms */
+	msleep(PCIE_T_PVPERL_MS);
+
+	ret = reset_control_deassert(port->perst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to deassert PERST#\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int eic7700_pcie_parse_port(struct eic7700_pcie *pcie,
+				   struct device_node *node)
+{
+	struct device *dev = pcie->pci.dev;
+	struct eic7700_pcie_port *port;
+
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	port->perst = of_reset_control_get_exclusive(node, "perst");
+	if (IS_ERR(port->perst)) {
+		dev_err(dev, "Failed to get PERST# reset\n");
+		return PTR_ERR(port->perst);
+	}
+
+	/*
+	 * TODO: Since the Root Port node is separated out by pcie devicetree,
+	 * the DWC core initialization code can't parse the num-lanes attribute
+	 * in the Root Port. Before entering the DWC core initialization code,
+	 * the platform driver code parses the Root Port node. The EIC7700 only
+	 * supports one Root Port node, and the num-lanes attribute is suitable
+	 * for the case of one Root Rort.
+	 */
+	if (!of_property_read_u32(node, "num-lanes", &port->num_lanes))
+		pcie->pci.num_lanes = port->num_lanes;
+
+	INIT_LIST_HEAD(&port->list);
+	list_add_tail(&port->list, &pcie->ports);
+
+	return 0;
+}
+
+static int eic7700_pcie_parse_ports(struct eic7700_pcie *pcie)
+{
+	struct eic7700_pcie_port *port, *tmp;
+	struct device *dev = pcie->pci.dev;
+	int ret;
+
+	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+		ret = eic7700_pcie_parse_port(pcie, of_port);
+		if (ret)
+			goto err_port;
+	}
+
+	return 0;
+
+err_port:
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
+		list_del(&port->list);
+
+	return ret;
+}
+
+static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
+{
+	u16 offset, val;
+
+	/*
+	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
+	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
+	 * linked-list needs to be disabled. Since the PCI Express capability
+	 * structure's next pointer points to the MSI-X capability, and the
+	 * MSI-X capability's next pointer is null (00H), so only the PCI
+	 * Express capability structure's next pointer needs to be set 00H.
+	 */
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	val = dw_pcie_readl_dbi(pci, offset);
+	val &= ~PCI_CAP_LIST_NEXT_MASK;
+	dw_pcie_writel_dbi(pci, offset, val);
+}
+
+static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
+	struct eic7700_pcie_port *port;
+	u8 msi_cap;
+	u32 val;
+	int ret;
+
+	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
+	if (pcie->num_clks < 0)
+		return dev_err_probe(pci->dev, pcie->num_clks,
+				     "Failed to get pcie clocks\n");
+
+	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
+		return ret;
+	}
+
+	/* Configure Root Port type */
+	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
+	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
+	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		ret = eic7700_pcie_perst_deassert(port, pcie);
+		if (ret)
+			goto err_perst;
+	}
+
+	/* Configure app_hold_phy_rst */
+	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
+	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
+
+	/* The maximum waiting time for the clock switch lock is 20ms */
+	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
+				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
+				 20000);
+	if (ret) {
+		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
+		goto err_phy_init;
+	}
+
+	/*
+	 * Configure ESWIN VID:DID for Root Port as the default values are
+	 * invalid.
+	 */
+	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_ESWIN);
+	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_ESWIN);
+
+	/* Configure support 32 MSI vectors */
+	msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+	val = dw_pcie_readw_dbi(pci, msi_cap + PCI_MSI_FLAGS);
+	val &= ~PCI_MSI_FLAGS_QMASK;
+	val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, 5);
+	dw_pcie_writew_dbi(pci, msi_cap + PCI_MSI_FLAGS, val);
+
+	/* Configure disable MSI-X cap */
+	if (!pcie->data->msix_cap)
+		eic7700_pcie_hide_broken_msix_cap(pci);
+
+	return 0;
+
+err_phy_init:
+	list_for_each_entry(port, &pcie->ports, list)
+		reset_control_assert(port->perst);
+err_perst:
+	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
+
+	return ret;
+}
+
+static void eic7700_pcie_host_deinit(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
+	struct eic7700_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		reset_control_assert(port->perst);
+	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
+	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
+}
+
+static const struct dw_pcie_host_ops eic7700_pcie_host_ops = {
+	.init = eic7700_pcie_host_init,
+	.deinit = eic7700_pcie_host_deinit,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = eic7700_pcie_start_link,
+	.link_up = eic7700_pcie_link_up,
+};
+
+static int eic7700_pcie_probe(struct platform_device *pdev)
+{
+	const struct eic7700_pcie_data *data;
+	struct eic7700_pcie_port *port, *tmp;
+	struct device *dev = &pdev->dev;
+	struct eic7700_pcie *pcie;
+	struct dw_pcie *pci;
+	int ret, i;
+
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&pcie->ports);
+
+	pci = &pcie->pci;
+	pci->dev = dev;
+	pci->ops = &dw_pcie_ops;
+	pci->pp.ops = &eic7700_pcie_host_ops;
+	pcie->data = data;
+	pci->no_suspport_L2 = pcie->data->no_suspport_L2;
+
+	for (i = 0; i < EIC7700_NUM_RSTS; i++)
+		pcie->resets[i].id = eic7700_pcie_rsts[i];
+
+	ret = devm_reset_control_bulk_get_exclusive(dev, EIC7700_NUM_RSTS,
+						    pcie->resets);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get resets\n");
+
+	ret = eic7700_pcie_parse_ports(pcie);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to parse Root Port: %d\n", ret);
+
+	platform_set_drvdata(pdev, pcie);
+
+	ret = dw_pcie_host_init(&pci->pp);
+	if (ret) {
+		dev_err(dev, "Failed to initialize host\n");
+		goto err_init;
+	}
+
+	return 0;
+
+err_init:
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+		list_del(&port->list);
+		reset_control_put(port->perst);
+	}
+
+	return ret;
+}
+
+static int eic7700_pcie_suspend_noirq(struct device *dev)
+{
+	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
+
+	return dw_pcie_suspend_noirq(&pcie->pci);
+}
+
+static int eic7700_pcie_resume_noirq(struct device *dev)
+{
+	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
+
+	return dw_pcie_resume_noirq(&pcie->pci);
+}
+
+static const struct dev_pm_ops eic7700_pcie_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(eic7700_pcie_suspend_noirq,
+				  eic7700_pcie_resume_noirq)
+};
+
+static const struct eic7700_pcie_data eic7700_data = {
+	.msix_cap = false,
+	.no_suspport_L2 = true,
+};
+
+static const struct of_device_id eic7700_pcie_of_match[] = {
+	{ .compatible = "eswin,eic7700-pcie", .data = &eic7700_data },
+	{},
+};
+
+static struct platform_driver eic7700_pcie_driver = {
+	.probe = eic7700_pcie_probe,
+	.driver = {
+		.name = "eic7700-pcie",
+		.of_match_table = eic7700_pcie_of_match,
+		.suppress_bind_attrs = true,
+		.pm = &eic7700_pcie_pm_ops,
+	},
+};
+builtin_platform_driver(eic7700_pcie_driver);
+
+MODULE_DESCRIPTION("Eswin EIC7700 PCIe host controller driver");
+MODULE_AUTHOR("Yu Ning <ningyu@eswincomputing.com>");
+MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com>");
+MODULE_AUTHOR("Yanghui Ou <ouyanghui@eswincomputing.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
  2025-11-20 10:10 [PATCH v6 0/3] Add driver support for Eswin EIC7700 SoC PCIe controller zhangsenchuan
  2025-11-20 10:11 ` [PATCH v6 1/3] dt-bindings: PCI: eic7700: Add Eswin PCIe host controller zhangsenchuan
  2025-11-20 10:12 ` [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver zhangsenchuan
@ 2025-11-20 10:12 ` zhangsenchuan
  2025-11-20 12:45   ` Manivannan Sadhasivam
  2 siblings, 1 reply; 15+ messages in thread
From: zhangsenchuan @ 2025-11-20 10:12 UTC (permalink / raw)
  To: bhelgaas, mani, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama
  Cc: ningyu, linmin, pinkesh.vaghela, ouyanghui, Frank.li,
	Senchuan Zhang

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
other driver can reuse this flag if meet the similar situation.

Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda5..a203577606e5 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
 		return 0;

+	if (pci->no_suspport_L2)
+		goto stop_link;
+
 	if (pci->pp.ops->pme_turn_off) {
 		pci->pp.ops->pme_turn_off(&pci->pp);
 	} else {
@@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	 */
 	udelay(1);

+stop_link:
 	dw_pcie_stop_link(pci);
 	if (pci->pp.ops->deinit)
 		pci->pp.ops->deinit(&pci->pp);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e995f692a1ec..170a73299ce5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -539,6 +539,7 @@ struct dw_pcie {
 	 * use_parent_dt_ranges to true to avoid this warning.
 	 */
 	bool			use_parent_dt_ranges;
+	bool			no_suspport_L2;
 };

 #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
--
2.25.1


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

* Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-20 10:12 ` [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver zhangsenchuan
@ 2025-11-20 12:43   ` Manivannan Sadhasivam
  2025-11-26 12:15     ` zhangsenchuan
  2025-11-20 13:19   ` Shawn Lin
  1 sibling, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-20 12:43 UTC (permalink / raw)
  To: zhangsenchuan
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li

On Thu, Nov 20, 2025 at 06:12:06PM +0800, zhangsenchuan@eswincomputing.com wrote:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> interrupts.
> 
> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> ---
>  drivers/pci/controller/dwc/Kconfig        |  11 +
>  drivers/pci/controller/dwc/Makefile       |   1 +
>  drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
>  3 files changed, 399 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c..66568efb324f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -93,6 +93,17 @@ config PCIE_BT1
>  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
>  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
>  
> +config PCIE_EIC7700
> +	bool "Eswin EIC7700 PCIe controller"

You can make it tristate as you've used builtin_platform_driver() which
guarantees that this driver won't be removed once loaded.

> +	depends on ARCH_ESWIN || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> +	  host mode.
> +
>  config PCI_IMX6
>  	bool
>  
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 7ae28f3b0fb3..04f751c49eba 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
>  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> new file mode 100644
> index 000000000000..239fdbc501fe
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ESWIN EIC7700 PCIe root complex driver
> + *
> + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> + *
> + * Authors: Yu Ning <ningyu@eswincomputing.com>
> + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/reset.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +/* ELBI registers */
> +#define PCIEELBI_CTRL0_OFFSET		0x0
> +#define PCIEELBI_STATUS0_OFFSET		0x100
> +
> +/* LTSSM register fields */
> +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> +
> +/* APP_HOLD_PHY_RST register fields */
> +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> +
> +/* PM_SEL_AUX_CLK register fields */
> +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> +
> +/* DEV_TYPE register fields */
> +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> +
> +/* Vendor and device ID value */
> +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> +#define PCI_DEVICE_ID_ESWIN		0x2030
> +
> +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> +
> +static const char * const eic7700_pcie_rsts[] = {
> +	"pwr",
> +	"dbi",
> +};
> +
> +struct eic7700_pcie_data {
> +	bool msix_cap;
> +	bool no_suspport_L2;

support?

> +};
> +
> +struct eic7700_pcie_port {
> +	struct list_head list;
> +	struct reset_control *perst;
> +	int num_lanes;
> +};
> +
> +struct eic7700_pcie {
> +	struct dw_pcie pci;
> +	struct clk_bulk_data *clks;
> +	struct reset_control_bulk_data resets[EIC7700_NUM_RSTS];
> +	struct list_head ports;
> +	const struct eic7700_pcie_data *data;
> +	int num_clks;
> +};
> +
> +#define to_eic7700_pcie(x) dev_get_drvdata((x)->dev)
> +
> +static int eic7700_pcie_start_link(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	/* Enable LTSSM */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val |= PCIEELBI_APP_LTSSM_ENABLE;
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	return 0;
> +}
> +
> +static bool eic7700_pcie_link_up(struct dw_pcie *pci)
> +{
> +	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> +
> +	return val & PCI_EXP_LNKSTA_DLLLA;
> +}
> +
> +static int eic7700_pcie_perst_deassert(struct eic7700_pcie_port *port,
> +				       struct eic7700_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = reset_control_assert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to assert PERST#\n");
> +		return ret;
> +	}

Why assert is part of the deassert helper?

> +
> +	/* Ensure that PERST# has been asserted for at least 100 ms */
> +	msleep(PCIE_T_PVPERL_MS);
> +
> +	ret = reset_control_deassert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert PERST#\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eic7700_pcie_parse_port(struct eic7700_pcie *pcie,
> +				   struct device_node *node)
> +{
> +	struct device *dev = pcie->pci.dev;
> +	struct eic7700_pcie_port *port;
> +
> +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +
> +	port->perst = of_reset_control_get_exclusive(node, "perst");
> +	if (IS_ERR(port->perst)) {
> +		dev_err(dev, "Failed to get PERST# reset\n");
> +		return PTR_ERR(port->perst);
> +	}
> +
> +	/*
> +	 * TODO: Since the Root Port node is separated out by pcie devicetree,
> +	 * the DWC core initialization code can't parse the num-lanes attribute
> +	 * in the Root Port. Before entering the DWC core initialization code,
> +	 * the platform driver code parses the Root Port node. The EIC7700 only
> +	 * supports one Root Port node, and the num-lanes attribute is suitable
> +	 * for the case of one Root Rort.
> +	 */
> +	if (!of_property_read_u32(node, "num-lanes", &port->num_lanes))
> +		pcie->pci.num_lanes = port->num_lanes;
> +
> +	INIT_LIST_HEAD(&port->list);
> +	list_add_tail(&port->list, &pcie->ports);
> +
> +	return 0;
> +}
> +
> +static int eic7700_pcie_parse_ports(struct eic7700_pcie *pcie)
> +{
> +	struct eic7700_pcie_port *port, *tmp;
> +	struct device *dev = pcie->pci.dev;
> +	int ret;
> +
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		ret = eic7700_pcie_parse_port(pcie, of_port);
> +		if (ret)
> +			goto err_port;
> +	}
> +
> +	return 0;
> +
> +err_port:
> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
> +		list_del(&port->list);
> +
> +	return ret;
> +}
> +
> +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> +{
> +	u16 offset, val;
> +
> +	/*
> +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> +	 * linked-list needs to be disabled. Since the PCI Express capability
> +	 * structure's next pointer points to the MSI-X capability, and the
> +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> +	 * Express capability structure's next pointer needs to be set 00H.
> +	 */
> +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	val = dw_pcie_readl_dbi(pci, offset);
> +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> +	dw_pcie_writel_dbi(pci, offset, val);

I hate to enforce dependency for your series, but this is properly handled here:
https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-2-2208f46f4dc2@oss.qualcomm.com

> +}
> +
> +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> +	struct eic7700_pcie_port *port;
> +	u8 msi_cap;
> +	u32 val;
> +	int ret;
> +
> +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> +	if (pcie->num_clks < 0)
> +		return dev_err_probe(pci->dev, pcie->num_clks,
> +				     "Failed to get pcie clocks\n");
> +
> +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);

I think this is being called too early.

> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> +		return ret;
> +	}
> +
> +	/* Configure Root Port type */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	list_for_each_entry(port, &pcie->ports, list) {
> +		ret = eic7700_pcie_perst_deassert(port, pcie);
> +		if (ret)
> +			goto err_perst;
> +	}
> +
> +	/* Configure app_hold_phy_rst */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	/* The maximum waiting time for the clock switch lock is 20ms */
> +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> +				 20000);
> +	if (ret) {
> +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> +		goto err_phy_init;
> +	}

You seem to be configuring the PHY reset and Aux clock, which should come before
deasserting PERST# IMO. PERST# deassertion indicates that the power and clock
are stable and the endpoint can start its operation. I don't know the impact of
these configurations, but it is safe to do them earlier.

> +
> +	/*
> +	 * Configure ESWIN VID:DID for Root Port as the default values are
> +	 * invalid.
> +	 */
> +	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_ESWIN);
> +	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_ESWIN);
> +
> +	/* Configure support 32 MSI vectors */
> +	msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +	val = dw_pcie_readw_dbi(pci, msi_cap + PCI_MSI_FLAGS);
> +	val &= ~PCI_MSI_FLAGS_QMASK;
> +	val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, 5);
> +	dw_pcie_writew_dbi(pci, msi_cap + PCI_MSI_FLAGS, val);

So this configures the MSI for Root Port. But are you sure that the internal MSI
controller could receive MSI from the Root Port?

Take a look: https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-3-2208f46f4dc2@oss.qualcomm.com

> +
> +	/* Configure disable MSI-X cap */
> +	if (!pcie->data->msix_cap)
> +		eic7700_pcie_hide_broken_msix_cap(pci);
> +
> +	return 0;
> +
> +err_phy_init:
> +	list_for_each_entry(port, &pcie->ports, list)
> +		reset_control_assert(port->perst);
> +err_perst:
> +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> +
> +	return ret;
> +}
> +
> +static void eic7700_pcie_host_deinit(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> +	struct eic7700_pcie_port *port;
> +
> +	list_for_each_entry(port, &pcie->ports, list)
> +		reset_control_assert(port->perst);
> +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> +	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> +}
> +
> +static const struct dw_pcie_host_ops eic7700_pcie_host_ops = {
> +	.init = eic7700_pcie_host_init,
> +	.deinit = eic7700_pcie_host_deinit,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = eic7700_pcie_start_link,
> +	.link_up = eic7700_pcie_link_up,
> +};
> +
> +static int eic7700_pcie_probe(struct platform_device *pdev)
> +{
> +	const struct eic7700_pcie_data *data;
> +	struct eic7700_pcie_port *port, *tmp;
> +	struct device *dev = &pdev->dev;
> +	struct eic7700_pcie *pcie;
> +	struct dw_pcie *pci;
> +	int ret, i;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return dev_err_probe(dev, -EINVAL, "OF data missing\n");

-ENODATA

> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&pcie->ports);
> +
> +	pci = &pcie->pci;
> +	pci->dev = dev;
> +	pci->ops = &dw_pcie_ops;
> +	pci->pp.ops = &eic7700_pcie_host_ops;
> +	pcie->data = data;
> +	pci->no_suspport_L2 = pcie->data->no_suspport_L2;
> +
> +	for (i = 0; i < EIC7700_NUM_RSTS; i++)
> +		pcie->resets[i].id = eic7700_pcie_rsts[i];
> +
> +	ret = devm_reset_control_bulk_get_exclusive(dev, EIC7700_NUM_RSTS,
> +						    pcie->resets);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get resets\n");
> +
> +	ret = eic7700_pcie_parse_ports(pcie);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to parse Root Port: %d\n", ret);
> +
> +	platform_set_drvdata(pdev, pcie);
> +

You need to set the runtime PM status for the controller:

	pm_runtime_no_callbacks(dev);
	devm_pm_runtime_enable(dev);
	ret = pm_runtime_get_sync(dev);
	if (ret < 0)
		goto err_pm_runtime_put;

> +	ret = dw_pcie_host_init(&pci->pp);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize host\n");
> +		goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> +		list_del(&port->list);
> +		reset_control_put(port->perst);
> +	}
> +
> +	return ret;
> +}
> +
> +static int eic7700_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> +
> +	return dw_pcie_suspend_noirq(&pcie->pci);
> +}
> +
> +static int eic7700_pcie_resume_noirq(struct device *dev)
> +{
> +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> +
> +	return dw_pcie_resume_noirq(&pcie->pci);
> +}
> +
> +static const struct dev_pm_ops eic7700_pcie_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(eic7700_pcie_suspend_noirq,
> +				  eic7700_pcie_resume_noirq)
> +};
> +
> +static const struct eic7700_pcie_data eic7700_data = {
> +	.msix_cap = false,
> +	.no_suspport_L2 = true,
> +};
> +
> +static const struct of_device_id eic7700_pcie_of_match[] = {
> +	{ .compatible = "eswin,eic7700-pcie", .data = &eic7700_data },
> +	{},
> +};
> +
> +static struct platform_driver eic7700_pcie_driver = {
> +	.probe = eic7700_pcie_probe,
> +	.driver = {
> +		.name = "eic7700-pcie",
> +		.of_match_table = eic7700_pcie_of_match,
> +		.suppress_bind_attrs = true,
> +		.pm = &eic7700_pcie_pm_ops,

Use:
		.probe_type = PROBE_PREFER_ASYNCHRONOUS,

to speed up the controller probe.

- Mani

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

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

* Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
  2025-11-20 10:12 ` [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast zhangsenchuan
@ 2025-11-20 12:45   ` Manivannan Sadhasivam
  2025-11-21  6:48     ` zhangsenchuan
  0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-20 12:45 UTC (permalink / raw)
  To: zhangsenchuan
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li

On Thu, Nov 20, 2025 at 06:12:35PM +0800, zhangsenchuan@eswincomputing.com wrote:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
> no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
> other driver can reuse this flag if meet the similar situation.
> 
> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Does this patch work for you?
https://lore.kernel.org/linux-pci/20251119-pci-dwc-suspend-rework-v1-1-aad104828562@oss.qualcomm.com/

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
>  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda5..a203577606e5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
>  		return 0;
> 
> +	if (pci->no_suspport_L2)
> +		goto stop_link;
> +
>  	if (pci->pp.ops->pme_turn_off) {
>  		pci->pp.ops->pme_turn_off(&pci->pp);
>  	} else {
> @@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	 */
>  	udelay(1);
> 
> +stop_link:
>  	dw_pcie_stop_link(pci);
>  	if (pci->pp.ops->deinit)
>  		pci->pp.ops->deinit(&pci->pp);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ec..170a73299ce5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -539,6 +539,7 @@ struct dw_pcie {
>  	 * use_parent_dt_ranges to true to avoid this warning.
>  	 */
>  	bool			use_parent_dt_ranges;
> +	bool			no_suspport_L2;
>  };
> 
>  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> --
> 2.25.1
> 

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

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

* Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-20 10:12 ` [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver zhangsenchuan
  2025-11-20 12:43   ` Manivannan Sadhasivam
@ 2025-11-20 13:19   ` Shawn Lin
  2025-11-26 12:23     ` zhangsenchuan
  1 sibling, 1 reply; 15+ messages in thread
From: Shawn Lin @ 2025-11-20 13:19 UTC (permalink / raw)
  To: zhangsenchuan, bhelgaas, mani, krzk+dt, conor+dt, lpieralisi,
	kwilczynski, robh, p.zabel, jingoohan1, gustavo.pimentel,
	linux-pci, devicetree, linux-kernel, christian.bruel, mayank.rana,
	shradha.t, krishna.chundru, thippeswamy.havalige, inochiama
  Cc: shawn.lin, ningyu, linmin, pinkesh.vaghela, ouyanghui, Frank.li

在 2025/11/20 星期四 18:12, zhangsenchuan@eswincomputing.com 写道:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> interrupts.
> 

Don't need any stuff regarding to PHY in the driver?

> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> ---
>   drivers/pci/controller/dwc/Kconfig        |  11 +
>   drivers/pci/controller/dwc/Makefile       |   1 +
>   drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
>   3 files changed, 399 insertions(+)
>   create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c..66568efb324f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -93,6 +93,17 @@ config PCIE_BT1
>   	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
>   	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
>   
> +config PCIE_EIC7700
> +	bool "Eswin EIC7700 PCIe controller"
> +	depends on ARCH_ESWIN || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> +	  host mode.
> +
>   config PCI_IMX6
>   	bool
>   
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 7ae28f3b0fb3..04f751c49eba 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>   obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>   obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
>   obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
>   obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>   obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>   obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> new file mode 100644
> index 000000000000..239fdbc501fe
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ESWIN EIC7700 PCIe root complex driver
> + *
> + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> + *
> + * Authors: Yu Ning <ningyu@eswincomputing.com>
> + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/reset.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +/* ELBI registers */
> +#define PCIEELBI_CTRL0_OFFSET		0x0
> +#define PCIEELBI_STATUS0_OFFSET		0x100
> +
> +/* LTSSM register fields */
> +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> +
> +/* APP_HOLD_PHY_RST register fields */
> +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> +
> +/* PM_SEL_AUX_CLK register fields */
> +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> +
> +/* DEV_TYPE register fields */
> +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> +
> +/* Vendor and device ID value */
> +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> +#define PCI_DEVICE_ID_ESWIN		0x2030

It would be better to be moved to pci_ids.h ?

> +
> +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)

If using devm_reset_control_array_get_exclusive, you don't need this
at all.

> +
> +static const char * const eic7700_pcie_rsts[] = {
> +	"pwr",
> +	"dbi",
> +};
> +

Ditto.

> +struct eic7700_pcie_data {
> +	bool msix_cap;
> +	bool no_suspport_L2;
> +};
> +
> +struct eic7700_pcie_port {
> +	struct list_head list;
> +	struct reset_control *perst;
> +	int num_lanes;
> +};
> +
> +struct eic7700_pcie {
> +	struct dw_pcie pci;
> +	struct clk_bulk_data *clks;
> +	struct reset_control_bulk_data resets[EIC7700_NUM_RSTS];
> +	struct list_head ports;
> +	const struct eic7700_pcie_data *data;
> +	int num_clks;
> +};
> +
> +#define to_eic7700_pcie(x) dev_get_drvdata((x)->dev)
> +
> +static int eic7700_pcie_start_link(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	/* Enable LTSSM */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val |= PCIEELBI_APP_LTSSM_ENABLE;
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	return 0;
> +}
> +
> +static bool eic7700_pcie_link_up(struct dw_pcie *pci)
> +{
> +	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);

dw_pcie_readl_dbi()?

> +
> +	return val & PCI_EXP_LNKSTA_DLLLA;
> +}
> +
> +static int eic7700_pcie_perst_deassert(struct eic7700_pcie_port *port,
> +				       struct eic7700_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = reset_control_assert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to assert PERST#\n");
> +		return ret;
> +	}
> +
> +	/* Ensure that PERST# has been asserted for at least 100 ms */
> +	msleep(PCIE_T_PVPERL_MS);
> +
> +	ret = reset_control_deassert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert PERST#\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eic7700_pcie_parse_port(struct eic7700_pcie *pcie,
> +				   struct device_node *node)
> +{
> +	struct device *dev = pcie->pci.dev;
> +	struct eic7700_pcie_port *port;
> +
> +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +
> +	port->perst = of_reset_control_get_exclusive(node, "perst");
> +	if (IS_ERR(port->perst)) {
> +		dev_err(dev, "Failed to get PERST# reset\n");
> +		return PTR_ERR(port->perst);
> +	}
> +
> +	/*
> +	 * TODO: Since the Root Port node is separated out by pcie devicetree,
> +	 * the DWC core initialization code can't parse the num-lanes attribute
> +	 * in the Root Port. Before entering the DWC core initialization code,
> +	 * the platform driver code parses the Root Port node. The EIC7700 only
> +	 * supports one Root Port node, and the num-lanes attribute is suitable
> +	 * for the case of one Root Rort.
> +	 */
> +	if (!of_property_read_u32(node, "num-lanes", &port->num_lanes))
> +		pcie->pci.num_lanes = port->num_lanes;
> +

dw_pcie_get_resources() knows it.

> +	INIT_LIST_HEAD(&port->list);
> +	list_add_tail(&port->list, &pcie->ports);
> +
> +	return 0;
> +}
> +
> +static int eic7700_pcie_parse_ports(struct eic7700_pcie *pcie)
> +{
> +	struct eic7700_pcie_port *port, *tmp;
> +	struct device *dev = pcie->pci.dev;
> +	int ret;
> +
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		ret = eic7700_pcie_parse_port(pcie, of_port);
> +		if (ret)
> +			goto err_port;
> +	}
> +
> +	return 0;
> +
> +err_port:
> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
> +		list_del(&port->list);
> +
> +	return ret;
> +}
> +
> +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> +{
> +	u16 offset, val;
> +
> +	/*
> +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> +	 * linked-list needs to be disabled. Since the PCI Express capability
> +	 * structure's next pointer points to the MSI-X capability, and the
> +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> +	 * Express capability structure's next pointer needs to be set 00H.
> +	 */
> +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	val = dw_pcie_readl_dbi(pci, offset);
> +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> +	dw_pcie_writel_dbi(pci, offset, val);
> +}
> +
> +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> +	struct eic7700_pcie_port *port;
> +	u8 msi_cap;
> +	u32 val;
> +	int ret;
> +
> +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> +	if (pcie->num_clks < 0)
> +		return dev_err_probe(pci->dev, pcie->num_clks,
> +				     "Failed to get pcie clocks\n");
> +
> +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> +		return ret;
> +	}
> +
> +	/* Configure Root Port type */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	list_for_each_entry(port, &pcie->ports, list) {
> +		ret = eic7700_pcie_perst_deassert(port, pcie);
> +		if (ret)
> +			goto err_perst;
> +	}
> +
> +	/* Configure app_hold_phy_rst */
> +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> +
> +	/* The maximum waiting time for the clock switch lock is 20ms */
> +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> +				 20000);
> +	if (ret) {
> +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> +		goto err_phy_init;
> +	}
> +
> +	/*
> +	 * Configure ESWIN VID:DID for Root Port as the default values are
> +	 * invalid.
> +	 */
> +	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_ESWIN);
> +	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_ESWIN);
> +
> +	/* Configure support 32 MSI vectors */
> +	msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +	val = dw_pcie_readw_dbi(pci, msi_cap + PCI_MSI_FLAGS);
> +	val &= ~PCI_MSI_FLAGS_QMASK;
> +	val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, 5);
> +	dw_pcie_writew_dbi(pci, msi_cap + PCI_MSI_FLAGS, val);
> +
> +	/* Configure disable MSI-X cap */
> +	if (!pcie->data->msix_cap)
> +		eic7700_pcie_hide_broken_msix_cap(pci);
> +
> +	return 0;
> +
> +err_phy_init:
> +	list_for_each_entry(port, &pcie->ports, list)
> +		reset_control_assert(port->perst);
> +err_perst:
> +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> +
> +	return ret;
> +}
> +
> +static void eic7700_pcie_host_deinit(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> +	struct eic7700_pcie_port *port;
> +
> +	list_for_each_entry(port, &pcie->ports, list)
> +		reset_control_assert(port->perst);
> +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> +	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> +}
> +
> +static const struct dw_pcie_host_ops eic7700_pcie_host_ops = {
> +	.init = eic7700_pcie_host_init,
> +	.deinit = eic7700_pcie_host_deinit,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = eic7700_pcie_start_link,
> +	.link_up = eic7700_pcie_link_up,
> +};
> +
> +static int eic7700_pcie_probe(struct platform_device *pdev)
> +{
> +	const struct eic7700_pcie_data *data;
> +	struct eic7700_pcie_port *port, *tmp;
> +	struct device *dev = &pdev->dev;
> +	struct eic7700_pcie *pcie;
> +	struct dw_pcie *pci;
> +	int ret, i;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&pcie->ports);
> +
> +	pci = &pcie->pci;
> +	pci->dev = dev;
> +	pci->ops = &dw_pcie_ops;
> +	pci->pp.ops = &eic7700_pcie_host_ops;
> +	pcie->data = data;
> +	pci->no_suspport_L2 = pcie->data->no_suspport_L2;
> +
> +	for (i = 0; i < EIC7700_NUM_RSTS; i++)
> +		pcie->resets[i].id = eic7700_pcie_rsts[i];
> +
> +	ret = devm_reset_control_bulk_get_exclusive(dev, EIC7700_NUM_RSTS,
> +						    pcie->resets);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get resets\n");
> +
> +	ret = eic7700_pcie_parse_ports(pcie);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to parse Root Port: %d\n", ret);
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	ret = dw_pcie_host_init(&pci->pp);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize host\n");
> +		goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> +		list_del(&port->list);
> +		reset_control_put(port->perst);
> +	}
> +
> +	return ret;
> +}
> +
> +static int eic7700_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> +
> +	return dw_pcie_suspend_noirq(&pcie->pci);
> +}
> +
> +static int eic7700_pcie_resume_noirq(struct device *dev)
> +{
> +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> +
> +	return dw_pcie_resume_noirq(&pcie->pci);
> +}
> +
> +static const struct dev_pm_ops eic7700_pcie_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(eic7700_pcie_suspend_noirq,
> +				  eic7700_pcie_resume_noirq)
> +};
> +
> +static const struct eic7700_pcie_data eic7700_data = {
> +	.msix_cap = false,
> +	.no_suspport_L2 = true,
> +};
> +
> +static const struct of_device_id eic7700_pcie_of_match[] = {
> +	{ .compatible = "eswin,eic7700-pcie", .data = &eic7700_data },
> +	{},
> +};
> +
> +static struct platform_driver eic7700_pcie_driver = {
> +	.probe = eic7700_pcie_probe,
> +	.driver = {
> +		.name = "eic7700-pcie",
> +		.of_match_table = eic7700_pcie_of_match,
> +		.suppress_bind_attrs = true,
> +		.pm = &eic7700_pcie_pm_ops,
> +	},
> +};
> +builtin_platform_driver(eic7700_pcie_driver);
> +
> +MODULE_DESCRIPTION("Eswin EIC7700 PCIe host controller driver");
> +MODULE_AUTHOR("Yu Ning <ningyu@eswincomputing.com>");
> +MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com>");
> +MODULE_AUTHOR("Yanghui Ou <ouyanghui@eswincomputing.com>");
> +MODULE_LICENSE("GPL");


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

* Re: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
  2025-11-20 12:45   ` Manivannan Sadhasivam
@ 2025-11-21  6:48     ` zhangsenchuan
  2025-11-21  7:14       ` Shawn Lin
  0 siblings, 1 reply; 15+ messages in thread
From: zhangsenchuan @ 2025-11-21  6:48 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li




> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@kernel.org>
> Send time:Thursday, 20/11/2025 20:45:38
> To: zhangsenchuan@eswincomputing.com
> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> Subject: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
> 
> On Thu, Nov 20, 2025 at 06:12:35PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > 
> > The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
> > no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
> > other driver can reuse this flag if meet the similar situation.
> > 
> > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Does this patch work for you?
> https://lore.kernel.org/linux-pci/20251119-pci-dwc-suspend-rework-v1-1-aad104828562@oss.qualcomm.com/

if the PCIe link is not up, this suits me too, but if the PCIe link up, 
our hardware does not support entering the L2 link state. At this point, 
it is also necessary to skip the broadcast PME_Turn_Off message and wait
for L2 transition.

Kind regards,
Senchuan Zhang

> 
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
> >  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index e92513c5bda5..a203577606e5 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >  	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
> >  		return 0;
> > 
> > +	if (pci->no_suspport_L2)
> > +		goto stop_link;
> > +
> >  	if (pci->pp.ops->pme_turn_off) {
> >  		pci->pp.ops->pme_turn_off(&pci->pp);
> >  	} else {
> > @@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >  	 */
> >  	udelay(1);
> > 
> > +stop_link:
> >  	dw_pcie_stop_link(pci);
> >  	if (pci->pp.ops->deinit)
> >  		pci->pp.ops->deinit(&pci->pp);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index e995f692a1ec..170a73299ce5 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -539,6 +539,7 @@ struct dw_pcie {
> >  	 * use_parent_dt_ranges to true to avoid this warning.
> >  	 */
> >  	bool			use_parent_dt_ranges;
> > +	bool			no_suspport_L2;
> >  };
> > 
> >  #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> > --
> > 2.25.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

* Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
  2025-11-21  6:48     ` zhangsenchuan
@ 2025-11-21  7:14       ` Shawn Lin
  2025-11-24  9:53         ` zhangsenchuan
  0 siblings, 1 reply; 15+ messages in thread
From: Shawn Lin @ 2025-11-21  7:14 UTC (permalink / raw)
  To: zhangsenchuan
  Cc: shawn.lin, bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski,
	robh, p.zabel, jingoohan1, gustavo.pimentel, linux-pci,
	devicetree, linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li, Manivannan Sadhasivam

在 2025/11/21 星期五 14:48, zhangsenchuan 写道:
> 
> 
> 
>> -----Original Messages-----
>> From: "Manivannan Sadhasivam" <mani@kernel.org>
>> Send time:Thursday, 20/11/2025 20:45:38
>> To: zhangsenchuan@eswincomputing.com
>> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
>> Subject: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
>>
>> On Thu, Nov 20, 2025 at 06:12:35PM +0800, zhangsenchuan@eswincomputing.com wrote:
>>> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
>>>
>>> The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
>>> no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
>>> other driver can reuse this flag if meet the similar situation.
>>>
>>> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
>>> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
>>> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
>>
>> Does this patch work for you?
>> https://lore.kernel.org/linux-pci/20251119-pci-dwc-suspend-rework-v1-1-aad104828562@oss.qualcomm.com/
> 
> if the PCIe link is not up, this suits me too, but if the PCIe link up,
> our hardware does not support entering the L2 link state. At this point,

Per PCIe spec, 5.2 Link State Power Management:
"L2/L3 Ready transition protocol support is required.
The L2/L3 Ready state entry transition process must begin as soon as
possible following the acknowledgment of a PME_Turn_Off Message, (i.e.,
the injection of a PME_TO_Ack TLP). The Downstream component initiates
L2/L3 Ready entry by sending a PM_Enter_L23 DLLP. "

"L2 support is optional, and dependent upon the presence of auxiliary power"

I interpret it as it is mandatory support for broadcast PME_Turn_Off and
ack PME_TO_Ack, which is what the original code does. Then your IP can't
really go into L2 for whatever reasons? Will your controller be broken 
if acking PME_Turn_Off?  Otherwise we could still let L2/L3 Ready
transstion happen and leave the device in L3 to save power.


> it is also necessary to skip the broadcast PME_Turn_Off message and wait
> for L2 transition.
> 
> Kind regards,
> Senchuan Zhang
> 
>>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
>>>   drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> index e92513c5bda5..a203577606e5 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> @@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>>>   	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
>>>   		return 0;
>>>
>>> +	if (pci->no_suspport_L2)
>>> +		goto stop_link;
>>> +
>>>   	if (pci->pp.ops->pme_turn_off) {
>>>   		pci->pp.ops->pme_turn_off(&pci->pp);
>>>   	} else {
>>> @@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>>>   	 */
>>>   	udelay(1);
>>>
>>> +stop_link:
>>>   	dw_pcie_stop_link(pci);
>>>   	if (pci->pp.ops->deinit)
>>>   		pci->pp.ops->deinit(&pci->pp);
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>>> index e995f692a1ec..170a73299ce5 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>>> @@ -539,6 +539,7 @@ struct dw_pcie {
>>>   	 * use_parent_dt_ranges to true to avoid this warning.
>>>   	 */
>>>   	bool			use_parent_dt_ranges;
>>> +	bool			no_suspport_L2;
>>>   };
>>>
>>>   #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
>>> --
>>> 2.25.1
>>>
>>
>> -- 
>> மணிவண்ணன் சதாசிவம்


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

* Re: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
  2025-11-21  7:14       ` Shawn Lin
@ 2025-11-24  9:53         ` zhangsenchuan
  0 siblings, 0 replies; 15+ messages in thread
From: zhangsenchuan @ 2025-11-24  9:53 UTC (permalink / raw)
  To: Shawn Lin
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li, Manivannan Sadhasivam




> -----Original Messages-----
> From: "Shawn Lin" <shawn.lin@rock-chips.com>
> Send time:Friday, 21/11/2025 15:14:51
> To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> Cc: shawn.lin@rock-chips.com, bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com, "Manivannan Sadhasivam" <mani@kernel.org>
> Subject: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
> 
> 在 2025/11/21 星期五 14:48, zhangsenchuan 写道:
> > 
> > 
> > 
> >> -----Original Messages-----
> >> From: "Manivannan Sadhasivam" <mani@kernel.org>
> >> Send time:Thursday, 20/11/2025 20:45:38
> >> To: zhangsenchuan@eswincomputing.com
> >> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> >> Subject: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
> >>
> >> On Thu, Nov 20, 2025 at 06:12:35PM +0800, zhangsenchuan@eswincomputing.com wrote:
> >>> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> >>>
> >>> The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
> >>> no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
> >>> other driver can reuse this flag if meet the similar situation.
> >>>
> >>> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> >>> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> >>> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> >>
> >> Does this patch work for you?
> >> https://lore.kernel.org/linux-pci/20251119-pci-dwc-suspend-rework-v1-1-aad104828562@oss.qualcomm.com/
> > 
> > if the PCIe link is not up, this suits me too, but if the PCIe link up,
> > our hardware does not support entering the L2 link state. At this point,
> 
> Per PCIe spec, 5.2 Link State Power Management:
> "L2/L3 Ready transition protocol support is required.
> The L2/L3 Ready state entry transition process must begin as soon as
> possible following the acknowledgment of a PME_Turn_Off Message, (i.e.,
> the injection of a PME_TO_Ack TLP). The Downstream component initiates
> L2/L3 Ready entry by sending a PM_Enter_L23 DLLP. "
> 
> "L2 support is optional, and dependent upon the presence of auxiliary power"
> 
> I interpret it as it is mandatory support for broadcast PME_Turn_Off and
> ack PME_TO_Ack, which is what the original code does. Then your IP can't
> really go into L2 for whatever reasons? Will your controller be broken 
> if acking PME_Turn_Off?  Otherwise we could still let L2/L3 Ready
> transstion happen and leave the device in L3 to save power.

Hi, Shawn

Thank you for your comment and explanation.
Our hardware cannot broadcast PME_Turn_Off. Otherwise, the controller will
have problems, the dbi register shows that it cannot be accesseded. The link
state cannot be read in the read_poll_timeout function.

Kind regards,
Senchuan Zhang

> 
> 
> > it is also necessary to skip the broadcast PME_Turn_Off message and wait
> > for L2 transition.
> > 
> > 
> >>
> >>> ---
> >>>   drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
> >>>   drivers/pci/controller/dwc/pcie-designware.h      | 1 +
> >>>   2 files changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> >>> index e92513c5bda5..a203577606e5 100644
> >>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> >>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> >>> @@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >>>   	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
> >>>   		return 0;
> >>>
> >>> +	if (pci->no_suspport_L2)
> >>> +		goto stop_link;
> >>> +
> >>>   	if (pci->pp.ops->pme_turn_off) {
> >>>   		pci->pp.ops->pme_turn_off(&pci->pp);
> >>>   	} else {
> >>> @@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >>>   	 */
> >>>   	udelay(1);
> >>>
> >>> +stop_link:
> >>>   	dw_pcie_stop_link(pci);
> >>>   	if (pci->pp.ops->deinit)
> >>>   		pci->pp.ops->deinit(&pci->pp);
> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> >>> index e995f692a1ec..170a73299ce5 100644
> >>> --- a/drivers/pci/controller/dwc/pcie-designware.h
> >>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> >>> @@ -539,6 +539,7 @@ struct dw_pcie {
> >>>   	 * use_parent_dt_ranges to true to avoid this warning.
> >>>   	 */
> >>>   	bool			use_parent_dt_ranges;
> >>> +	bool			no_suspport_L2;
> >>>   };
> >>>
> >>>   #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> >>> --
> >>> 2.25.1
> >>>
> >>
> >> -- 
> >> மணிவண்ணன் சதாசிவம்

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

* Re: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-20 12:43   ` Manivannan Sadhasivam
@ 2025-11-26 12:15     ` zhangsenchuan
  2025-11-26 13:14       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 15+ messages in thread
From: zhangsenchuan @ 2025-11-26 12:15 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li




> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@kernel.org>
> Send time:Thursday, 20/11/2025 20:43:47
> To: zhangsenchuan@eswincomputing.com
> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> 
> On Thu, Nov 20, 2025 at 06:12:06PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > 
> > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > interrupts.
> > 
> > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig        |  11 +
> >  drivers/pci/controller/dwc/Makefile       |   1 +
> >  drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> >  3 files changed, 399 insertions(+)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> > 
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 349d4657393c..66568efb324f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -93,6 +93,17 @@ config PCIE_BT1
> >  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
> >  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> >  
> > +config PCIE_EIC7700
> > +	bool "Eswin EIC7700 PCIe controller"
> 
> You can make it tristate as you've used builtin_platform_driver() which
> guarantees that this driver won't be removed once loaded.

Okey, thanks

> 
> > +	depends on ARCH_ESWIN || COMPILE_TEST
> > +	depends on PCI_MSI
> > +	select PCIE_DW_HOST
> > +	help
> > +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> > +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> > +	  host mode.
> > +
> >  config PCI_IMX6
> >  	bool
> >  
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index 7ae28f3b0fb3..04f751c49eba 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> >  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> >  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> >  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> >  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> >  obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > new file mode 100644
> > index 000000000000..239fdbc501fe
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > @@ -0,0 +1,387 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ESWIN EIC7700 PCIe root complex driver
> > + *
> > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > + *
> > + * Authors: Yu Ning <ningyu@eswincomputing.com>
> > + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/resource.h>
> > +#include <linux/reset.h>
> > +#include <linux/types.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* ELBI registers */
> > +#define PCIEELBI_CTRL0_OFFSET		0x0
> > +#define PCIEELBI_STATUS0_OFFSET		0x100
> > +
> > +/* LTSSM register fields */
> > +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> > +
> > +/* APP_HOLD_PHY_RST register fields */
> > +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> > +
> > +/* PM_SEL_AUX_CLK register fields */
> > +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> > +
> > +/* DEV_TYPE register fields */
> > +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> > +
> > +/* Vendor and device ID value */
> > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > +#define PCI_DEVICE_ID_ESWIN		0x2030
> > +
> > +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> > +
> > +static const char * const eic7700_pcie_rsts[] = {
> > +	"pwr",
> > +	"dbi",
> > +};
> > +
> > +struct eic7700_pcie_data {
> > +	bool msix_cap;
> > +	bool no_suspport_L2;
> 
> support?

Okey, spelling mistake:)

mani, by the way, our controller cannot broadcast PME_Turn_Off. Previously,
i skip broadcast PME_Turn_Off in our controller code. Frank suggested that 
set the flag bit to skip in the public code. At present, do you think it's 
okay for me to add no_support_L2?

> > +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> > +{
> > +	u16 offset, val;
> > +
> > +	/*
> > +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> > +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> > +	 * linked-list needs to be disabled. Since the PCI Express capability
> > +	 * structure's next pointer points to the MSI-X capability, and the
> > +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> > +	 * Express capability structure's next pointer needs to be set 00H.
> > +	 */
> > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > +	val = dw_pcie_readl_dbi(pci, offset);
> > +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> > +	dw_pcie_writel_dbi(pci, offset, val);
> 
> I hate to enforce dependency for your series, but this is properly handled here:
> https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-2-2208f46f4dc2@oss.qualcomm.com

Okey, then I can rely on this patch to solve the problem of the missing MSI-X. 
Thank you for the reminder.

> 
> > +}
> > +
> > +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> > +	struct eic7700_pcie_port *port;
> > +	u8 msi_cap;
> > +	u32 val;
> > +	int ret;
> > +
> > +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> > +	if (pcie->num_clks < 0)
> > +		return dev_err_probe(pci->dev, pcie->num_clks,
> > +				     "Failed to get pcie clocks\n");
> > +
> > +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
> 
> I think this is being called too early.

Before accessing the register. I need to deassert, otherwise, there will
be problems with register access.

> 
> > +	if (ret) {
> > +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Configure Root Port type */
> > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> > +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +
> > +	list_for_each_entry(port, &pcie->ports, list) {
> > +		ret = eic7700_pcie_perst_deassert(port, pcie);
> > +		if (ret)
> > +			goto err_perst;
> > +	}
> > +
> > +	/* Configure app_hold_phy_rst */
> > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +
> > +	/* The maximum waiting time for the clock switch lock is 20ms */
> > +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> > +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> > +				 20000);
> > +	if (ret) {
> > +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> > +		goto err_phy_init;
> > +	}
> 
> You seem to be configuring the PHY reset and Aux clock, which should come before
> deasserting PERST# IMO. PERST# deassertion indicates that the power and clock
> are stable and the endpoint can start its operation. I don't know the impact of
> these configurations, but it is safe to do them earlier.

I think your understanding is correct. Unfortunately, in our hardware design, we 
need to deassert perst first before we can operate the configuration of the phy.

> 
> > +
> > +	/*
> > +	 * Configure ESWIN VID:DID for Root Port as the default values are
> > +	 * invalid.
> > +	 */
> > +	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_ESWIN);
> > +	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_ESWIN);
> > +
> > +	/* Configure support 32 MSI vectors */
> > +	msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > +	val = dw_pcie_readw_dbi(pci, msi_cap + PCI_MSI_FLAGS);
> > +	val &= ~PCI_MSI_FLAGS_QMASK;
> > +	val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, 5);
> > +	dw_pcie_writew_dbi(pci, msi_cap + PCI_MSI_FLAGS, val);
> 
> So this configures the MSI for Root Port. But are you sure that the internal MSI
> controller could receive MSI from the Root Port?
> 
> Take a look: https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-3-2208f46f4dc2@oss.qualcomm.com

Currently, our driver defaults to using the iMSI-RX module as MSI controller. 
If that's the case, i might need to rely on this patch.

> 
> > +
> > +	/* Configure disable MSI-X cap */
> > +	if (!pcie->data->msix_cap)
> > +		eic7700_pcie_hide_broken_msix_cap(pci);
> > +
> > +	return 0;
> > +
> > +err_phy_init:
> > +	list_for_each_entry(port, &pcie->ports, list)
> > +		reset_control_assert(port->perst);
> > +err_perst:
> > +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> > +
> > +	return ret;
> > +}
> > +
> > +static void eic7700_pcie_host_deinit(struct dw_pcie_rp *pp)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> > +	struct eic7700_pcie_port *port;
> > +
> > +	list_for_each_entry(port, &pcie->ports, list)
> > +		reset_control_assert(port->perst);
> > +	reset_control_bulk_assert(EIC7700_NUM_RSTS, pcie->resets);
> > +	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> > +}
> > +
> > +static const struct dw_pcie_host_ops eic7700_pcie_host_ops = {
> > +	.init = eic7700_pcie_host_init,
> > +	.deinit = eic7700_pcie_host_deinit,
> > +};
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > +	.start_link = eic7700_pcie_start_link,
> > +	.link_up = eic7700_pcie_link_up,
> > +};
> > +
> > +static int eic7700_pcie_probe(struct platform_device *pdev)
> > +{
> > +	const struct eic7700_pcie_data *data;
> > +	struct eic7700_pcie_port *port, *tmp;
> > +	struct device *dev = &pdev->dev;
> > +	struct eic7700_pcie *pcie;
> > +	struct dw_pcie *pci;
> > +	int ret, i;
> > +
> > +	data = of_device_get_match_data(dev);
> > +	if (!data)
> > +		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
> 
> -ENODATA

Okey, thanks

> 
> > +
> > +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> > +	if (!pcie)
> > +		return -ENOMEM;
> > +
> > +	INIT_LIST_HEAD(&pcie->ports);
> > +
> > +	pci = &pcie->pci;
> > +	pci->dev = dev;
> > +	pci->ops = &dw_pcie_ops;
> > +	pci->pp.ops = &eic7700_pcie_host_ops;
> > +	pcie->data = data;
> > +	pci->no_suspport_L2 = pcie->data->no_suspport_L2;
> > +
> > +	for (i = 0; i < EIC7700_NUM_RSTS; i++)
> > +		pcie->resets[i].id = eic7700_pcie_rsts[i];
> > +
> > +	ret = devm_reset_control_bulk_get_exclusive(dev, EIC7700_NUM_RSTS,
> > +						    pcie->resets);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to get resets\n");
> > +
> > +	ret = eic7700_pcie_parse_ports(pcie);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret,
> > +				     "Failed to parse Root Port: %d\n", ret);
> > +
> > +	platform_set_drvdata(pdev, pcie);
> > +
> 
> You need to set the runtime PM status for the controller:
> 
> 	pm_runtime_no_callbacks(dev);
> 	devm_pm_runtime_enable(dev);
> 	ret = pm_runtime_get_sync(dev);
> 	if (ret < 0)
> 		goto err_pm_runtime_put;

Okey, thanks

> 
> > +	ret = dw_pcie_host_init(&pci->pp);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to initialize host\n");
> > +		goto err_init;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_init:
> > +	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > +		list_del(&port->list);
> > +		reset_control_put(port->perst);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int eic7700_pcie_suspend_noirq(struct device *dev)
> > +{
> > +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> > +
> > +	return dw_pcie_suspend_noirq(&pcie->pci);
> > +}
> > +
> > +static int eic7700_pcie_resume_noirq(struct device *dev)
> > +{
> > +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> > +
> > +	return dw_pcie_resume_noirq(&pcie->pci);
> > +}
> > +
> > +static const struct dev_pm_ops eic7700_pcie_pm_ops = {
> > +	NOIRQ_SYSTEM_SLEEP_PM_OPS(eic7700_pcie_suspend_noirq,
> > +				  eic7700_pcie_resume_noirq)
> > +};
> > +
> > +static const struct eic7700_pcie_data eic7700_data = {
> > +	.msix_cap = false,
> > +	.no_suspport_L2 = true,
> > +};
> > +
> > +static const struct of_device_id eic7700_pcie_of_match[] = {
> > +	{ .compatible = "eswin,eic7700-pcie", .data = &eic7700_data },
> > +	{},
> > +};
> > +
> > +static struct platform_driver eic7700_pcie_driver = {
> > +	.probe = eic7700_pcie_probe,
> > +	.driver = {
> > +		.name = "eic7700-pcie",
> > +		.of_match_table = eic7700_pcie_of_match,
> > +		.suppress_bind_attrs = true,
> > +		.pm = &eic7700_pcie_pm_ops,
> 
> Use:
> 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,

Okey, thanks

> 
> to speed up the controller probe.
> 
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்

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

* Re: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-20 13:19   ` Shawn Lin
@ 2025-11-26 12:23     ` zhangsenchuan
  0 siblings, 0 replies; 15+ messages in thread
From: zhangsenchuan @ 2025-11-26 12:23 UTC (permalink / raw)
  To: Shawn Lin
  Cc: bhelgaas, mani, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li




> -----Original Messages-----
> From: "Shawn Lin" <shawn.lin@rock-chips.com>
> Send time:Thursday, 20/11/2025 21:19:40
> To: zhangsenchuan@eswincomputing.com, bhelgaas@google.com, mani@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com
> Cc: shawn.lin@rock-chips.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> 
> 在 2025/11/20 星期四 18:12, zhangsenchuan@eswincomputing.com 写道:
> > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > 
> > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > interrupts.
> > 
> 
> Don't need any stuff regarding to PHY in the driver?

Thank you for your comment, Shawn

Clarification
The phy is automatically configured by the hardware, and there are processes 
in the code waiting for the phy to be initialized.

> 
> > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > ---
> >   drivers/pci/controller/dwc/Kconfig        |  11 +
> >   drivers/pci/controller/dwc/Makefile       |   1 +
> >   drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> >   3 files changed, 399 insertions(+)
> >   create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> > 
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 349d4657393c..66568efb324f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -93,6 +93,17 @@ config PCIE_BT1
> >   	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
> >   	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> >   
> > +config PCIE_EIC7700
> > +	bool "Eswin EIC7700 PCIe controller"
> > +	depends on ARCH_ESWIN || COMPILE_TEST
> > +	depends on PCI_MSI
> > +	select PCIE_DW_HOST
> > +	help
> > +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> > +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> > +	  host mode.
> > +
> >   config PCI_IMX6
> >   	bool
> >   
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index 7ae28f3b0fb3..04f751c49eba 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >   obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> >   obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> >   obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> >   obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> >   obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> >   obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > new file mode 100644
> > index 000000000000..239fdbc501fe
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > @@ -0,0 +1,387 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ESWIN EIC7700 PCIe root complex driver
> > + *
> > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > + *
> > + * Authors: Yu Ning <ningyu@eswincomputing.com>
> > + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/resource.h>
> > +#include <linux/reset.h>
> > +#include <linux/types.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* ELBI registers */
> > +#define PCIEELBI_CTRL0_OFFSET		0x0
> > +#define PCIEELBI_STATUS0_OFFSET		0x100
> > +
> > +/* LTSSM register fields */
> > +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> > +
> > +/* APP_HOLD_PHY_RST register fields */
> > +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> > +
> > +/* PM_SEL_AUX_CLK register fields */
> > +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> > +
> > +/* DEV_TYPE register fields */
> > +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> > +
> > +/* Vendor and device ID value */
> > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > +#define PCI_DEVICE_ID_ESWIN		0x2030
> 
> It would be better to be moved to pci_ids.h ?

No vendor using Synopsys IP was found in the pci_ids.h file to have 
their VENDOR_ID and DEVICE_ID. Mani and bjorn may have agreed to keep 
the VENDOR_ID and DEVICE_ID in our own file.

> 
> > +
> > +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> 
> If using devm_reset_control_array_get_exclusive, you don't need this
> at all.

The maintainer of the reset module, Philipp, recommended 
devm_reset_control_bulk_get_exclusive() to me in the v4 patch. Maybe 
he had other considerations.

> 
> > +
> > +static const char * const eic7700_pcie_rsts[] = {
> > +	"pwr",
> > +	"dbi",
> > +};
> > +
> 
> Ditto.
> 
> > +struct eic7700_pcie_data {
> > +	bool msix_cap;
> > +	bool no_suspport_L2;
> > +};
> > +
> > +struct eic7700_pcie_port {
> > +	struct list_head list;
> > +	struct reset_control *perst;
> > +	int num_lanes;
> > +};
> > +
> > +struct eic7700_pcie {
> > +	struct dw_pcie pci;
> > +	struct clk_bulk_data *clks;
> > +	struct reset_control_bulk_data resets[EIC7700_NUM_RSTS];
> > +	struct list_head ports;
> > +	const struct eic7700_pcie_data *data;
> > +	int num_clks;
> > +};
> > +
> > +#define to_eic7700_pcie(x) dev_get_drvdata((x)->dev)
> > +
> > +static int eic7700_pcie_start_link(struct dw_pcie *pci)
> > +{
> > +	u32 val;
> > +
> > +	/* Enable LTSSM */
> > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +	val |= PCIEELBI_APP_LTSSM_ENABLE;
> > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > +
> > +	return 0;
> > +}
> > +
> > +static bool eic7700_pcie_link_up(struct dw_pcie *pci)
> > +{
> > +	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > +	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> 
> dw_pcie_readl_dbi()?

Okey, thanks:)

> 
> > +
> > +	return val & PCI_EXP_LNKSTA_DLLLA;
> > +}
> > +
> > +static int eic7700_pcie_perst_deassert(struct eic7700_pcie_port *port,
> > +				       struct eic7700_pcie *pcie)
> > +{
> > +	int ret;
> > +
> > +	ret = reset_control_assert(port->perst);
> > +	if (ret) {
> > +		dev_err(pcie->pci.dev, "Failed to assert PERST#\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Ensure that PERST# has been asserted for at least 100 ms */
> > +	msleep(PCIE_T_PVPERL_MS);
> > +
> > +	ret = reset_control_deassert(port->perst);
> > +	if (ret) {
> > +		dev_err(pcie->pci.dev, "Failed to deassert PERST#\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int eic7700_pcie_parse_port(struct eic7700_pcie *pcie,
> > +				   struct device_node *node)
> > +{
> > +	struct device *dev = pcie->pci.dev;
> > +	struct eic7700_pcie_port *port;
> > +
> > +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> > +	if (!port)
> > +		return -ENOMEM;
> > +
> > +	port->perst = of_reset_control_get_exclusive(node, "perst");
> > +	if (IS_ERR(port->perst)) {
> > +		dev_err(dev, "Failed to get PERST# reset\n");
> > +		return PTR_ERR(port->perst);
> > +	}
> > +
> > +	/*
> > +	 * TODO: Since the Root Port node is separated out by pcie devicetree,
> > +	 * the DWC core initialization code can't parse the num-lanes attribute
> > +	 * in the Root Port. Before entering the DWC core initialization code,
> > +	 * the platform driver code parses the Root Port node. The EIC7700 only
> > +	 * supports one Root Port node, and the num-lanes attribute is suitable
> > +	 * for the case of one Root Rort.
> > +	 */
> > +	if (!of_property_read_u32(node, "num-lanes", &port->num_lanes))
> > +		pcie->pci.num_lanes = port->num_lanes;
> > +
> 
> dw_pcie_get_resources() knows it.

The num-lanes of the root port node cannot be parsed in dw_pcie_get_resources.
Our device tree has separated the root port node.

Kind regards,
Senchuan Zhang


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

* Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-26 12:15     ` zhangsenchuan
@ 2025-11-26 13:14       ` Manivannan Sadhasivam
  2025-11-28  8:26         ` zhangsenchuan
  0 siblings, 1 reply; 15+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-26 13:14 UTC (permalink / raw)
  To: zhangsenchuan
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li

On Wed, Nov 26, 2025 at 08:15:11PM +0800, zhangsenchuan wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > Send time:Thursday, 20/11/2025 20:43:47
> > To: zhangsenchuan@eswincomputing.com
> > Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> > Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> > 
> > On Thu, Nov 20, 2025 at 06:12:06PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > 
> > > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > > interrupts.
> > > 
> > > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > ---
> > >  drivers/pci/controller/dwc/Kconfig        |  11 +
> > >  drivers/pci/controller/dwc/Makefile       |   1 +
> > >  drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> > >  3 files changed, 399 insertions(+)
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> > > 
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 349d4657393c..66568efb324f 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -93,6 +93,17 @@ config PCIE_BT1
> > >  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
> > >  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> > >  
> > > +config PCIE_EIC7700
> > > +	bool "Eswin EIC7700 PCIe controller"
> > 
> > You can make it tristate as you've used builtin_platform_driver() which
> > guarantees that this driver won't be removed once loaded.
> 
> Okey, thanks
> 
> > 
> > > +	depends on ARCH_ESWIN || COMPILE_TEST
> > > +	depends on PCI_MSI
> > > +	select PCIE_DW_HOST
> > > +	help
> > > +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > > +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> > > +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> > > +	  host mode.
> > > +
> > >  config PCI_IMX6
> > >  	bool
> > >  
> > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > index 7ae28f3b0fb3..04f751c49eba 100644
> > > --- a/drivers/pci/controller/dwc/Makefile
> > > +++ b/drivers/pci/controller/dwc/Makefile
> > > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> > >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> > >  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> > >  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> > >  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> > >  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > >  obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > new file mode 100644
> > > index 000000000000..239fdbc501fe
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > @@ -0,0 +1,387 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * ESWIN EIC7700 PCIe root complex driver
> > > + *
> > > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > > + *
> > > + * Authors: Yu Ning <ningyu@eswincomputing.com>
> > > + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> > > + */
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/resource.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/types.h>
> > > +
> > > +#include "pcie-designware.h"
> > > +
> > > +/* ELBI registers */
> > > +#define PCIEELBI_CTRL0_OFFSET		0x0
> > > +#define PCIEELBI_STATUS0_OFFSET		0x100
> > > +
> > > +/* LTSSM register fields */
> > > +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> > > +
> > > +/* APP_HOLD_PHY_RST register fields */
> > > +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> > > +
> > > +/* PM_SEL_AUX_CLK register fields */
> > > +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> > > +
> > > +/* DEV_TYPE register fields */
> > > +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> > > +
> > > +/* Vendor and device ID value */
> > > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > > +#define PCI_DEVICE_ID_ESWIN		0x2030
> > > +
> > > +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> > > +
> > > +static const char * const eic7700_pcie_rsts[] = {
> > > +	"pwr",
> > > +	"dbi",
> > > +};
> > > +
> > > +struct eic7700_pcie_data {
> > > +	bool msix_cap;
> > > +	bool no_suspport_L2;
> > 
> > support?
> 
> Okey, spelling mistake:)
> 
> mani, by the way, our controller cannot broadcast PME_Turn_Off. Previously,
> i skip broadcast PME_Turn_Off in our controller code. Frank suggested that 
> set the flag bit to skip in the public code. At present, do you think it's 
> okay for me to add no_support_L2?
> 

This sounds weird. As per the spec r6.0, sec 5.2, "L2/L3 Ready transition
protocol support is required." So this means the controller has to support
PME_Turn_Off broadcast. Are you saying that your controller is not spec
compliant? and the link can only enter L3 (power off) abruptly?

> > > +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> > > +{
> > > +	u16 offset, val;
> > > +
> > > +	/*
> > > +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> > > +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> > > +	 * linked-list needs to be disabled. Since the PCI Express capability
> > > +	 * structure's next pointer points to the MSI-X capability, and the
> > > +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> > > +	 * Express capability structure's next pointer needs to be set 00H.
> > > +	 */
> > > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > > +	val = dw_pcie_readl_dbi(pci, offset);
> > > +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> > > +	dw_pcie_writel_dbi(pci, offset, val);
> > 
> > I hate to enforce dependency for your series, but this is properly handled here:
> > https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-2-2208f46f4dc2@oss.qualcomm.com
> 
> Okey, then I can rely on this patch to solve the problem of the missing MSI-X. 
> Thank you for the reminder.
> 
> > 
> > > +}
> > > +
> > > +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> > > +{
> > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> > > +	struct eic7700_pcie_port *port;
> > > +	u8 msi_cap;
> > > +	u32 val;
> > > +	int ret;
> > > +
> > > +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> > > +	if (pcie->num_clks < 0)
> > > +		return dev_err_probe(pci->dev, pcie->num_clks,
> > > +				     "Failed to get pcie clocks\n");
> > > +
> > > +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
> > 
> > I think this is being called too early.
> 
> Before accessing the register. I need to deassert, otherwise, there will
> be problems with register access.
> 
> > 
> > > +	if (ret) {
> > > +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Configure Root Port type */
> > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> > > +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > +
> > > +	list_for_each_entry(port, &pcie->ports, list) {
> > > +		ret = eic7700_pcie_perst_deassert(port, pcie);
> > > +		if (ret)
> > > +			goto err_perst;
> > > +	}
> > > +
> > > +	/* Configure app_hold_phy_rst */
> > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > +
> > > +	/* The maximum waiting time for the clock switch lock is 20ms */
> > > +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> > > +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> > > +				 20000);
> > > +	if (ret) {
> > > +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> > > +		goto err_phy_init;
> > > +	}
> > 
> > You seem to be configuring the PHY reset and Aux clock, which should come before
> > deasserting PERST# IMO. PERST# deassertion indicates that the power and clock
> > are stable and the endpoint can start its operation. I don't know the impact of
> > these configurations, but it is safe to do them earlier.
> 
> I think your understanding is correct. Unfortunately, in our hardware design, we 
> need to deassert perst first before we can operate the configuration of the phy.
> 

Sorry, I don't understand how it is possible, unless this reset is not PERST#.
PERST# is just an indication to the component and has no relation with the
controller.

- Mani

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

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

* Re: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-26 13:14       ` Manivannan Sadhasivam
@ 2025-11-28  8:26         ` zhangsenchuan
  2025-11-28 15:07           ` Manivannan Sadhasivam
  0 siblings, 1 reply; 15+ messages in thread
From: zhangsenchuan @ 2025-11-28  8:26 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li




> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@kernel.org>
> Send time:Wednesday, 26/11/2025 21:14:54
> To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> 
> On Wed, Nov 26, 2025 at 08:15:11PM +0800, zhangsenchuan wrote:
> > 
> > 
> > 
> > > -----Original Messages-----
> > > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > > Send time:Thursday, 20/11/2025 20:43:47
> > > To: zhangsenchuan@eswincomputing.com
> > > Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> > > Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> > > 
> > > On Thu, Nov 20, 2025 at 06:12:06PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > > > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > 
> > > > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > > > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > > > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > > > interrupts.
> > > > 
> > > > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > > > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > > > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/Kconfig        |  11 +
> > > >  drivers/pci/controller/dwc/Makefile       |   1 +
> > > >  drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> > > >  3 files changed, 399 insertions(+)
> > > >  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > > index 349d4657393c..66568efb324f 100644
> > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > @@ -93,6 +93,17 @@ config PCIE_BT1
> > > >  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
> > > >  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> > > >  
> > > > +config PCIE_EIC7700
> > > > +	bool "Eswin EIC7700 PCIe controller"
> > > 
> > > You can make it tristate as you've used builtin_platform_driver() which
> > > guarantees that this driver won't be removed once loaded.
> > 
> > Okey, thanks
> > 
> > > 
> > > > +	depends on ARCH_ESWIN || COMPILE_TEST
> > > > +	depends on PCI_MSI
> > > > +	select PCIE_DW_HOST
> > > > +	help
> > > > +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > > > +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> > > > +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> > > > +	  host mode.
> > > > +
> > > >  config PCI_IMX6
> > > >  	bool
> > > >  
> > > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > > index 7ae28f3b0fb3..04f751c49eba 100644
> > > > --- a/drivers/pci/controller/dwc/Makefile
> > > > +++ b/drivers/pci/controller/dwc/Makefile
> > > > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> > > >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> > > >  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> > > >  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > > > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> > > >  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> > > >  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > > >  obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > > > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > > new file mode 100644
> > > > index 000000000000..239fdbc501fe
> > > > --- /dev/null
> > > > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > > @@ -0,0 +1,387 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * ESWIN EIC7700 PCIe root complex driver
> > > > + *
> > > > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > > > + *
> > > > + * Authors: Yu Ning <ningyu@eswincomputing.com>
> > > > + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> > > > + */
> > > > +
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/iopoll.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/of.h>
> > > > +#include <linux/pci.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/resource.h>
> > > > +#include <linux/reset.h>
> > > > +#include <linux/types.h>
> > > > +
> > > > +#include "pcie-designware.h"
> > > > +
> > > > +/* ELBI registers */
> > > > +#define PCIEELBI_CTRL0_OFFSET		0x0
> > > > +#define PCIEELBI_STATUS0_OFFSET		0x100
> > > > +
> > > > +/* LTSSM register fields */
> > > > +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> > > > +
> > > > +/* APP_HOLD_PHY_RST register fields */
> > > > +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> > > > +
> > > > +/* PM_SEL_AUX_CLK register fields */
> > > > +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> > > > +
> > > > +/* DEV_TYPE register fields */
> > > > +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> > > > +
> > > > +/* Vendor and device ID value */
> > > > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > > > +#define PCI_DEVICE_ID_ESWIN		0x2030
> > > > +
> > > > +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> > > > +
> > > > +static const char * const eic7700_pcie_rsts[] = {
> > > > +	"pwr",
> > > > +	"dbi",
> > > > +};
> > > > +
> > > > +struct eic7700_pcie_data {
> > > > +	bool msix_cap;
> > > > +	bool no_suspport_L2;
> > > 
> > > support?
> > 
> > Okey, spelling mistake:)
> > 
> > mani, by the way, our controller cannot broadcast PME_Turn_Off. Previously,
> > i skip broadcast PME_Turn_Off in our controller code. Frank suggested that 
> > set the flag bit to skip in the public code. At present, do you think it's 
> > okay for me to add no_support_L2?
> > 
> 
> This sounds weird. As per the spec r6.0, sec 5.2, "L2/L3 Ready transition
> protocol support is required." So this means the controller has to support
> PME_Turn_Off broadcast. Are you saying that your controller is not spec
> compliant? and the link can only enter L3 (power off) abruptly?

Hi, Mani

Clarification
After confirmation with the hardware team, L2/L3 low-power link state is not 
supported, and cannot be controlled to enter L2/L3 through trigger 
PME_turn_off/PME_To-Ack handshake, because this function is not implemented in 
hardware, nor is the Vmain/Vaux power domain implemented to implement L2/L3 
low-power wake-up mechanism.

Our PCIe only implements L0s/L1/L1.1 low-power mode. if want to put PCIe into 
a more energy-efficient state, only power down PCIe. Although the PCIe Spec 
requires the implementation of L2/L3 ready state, it is regrettable that we
have not been able to match the protocol requirements at this point. We will
gradually improve more features required by the Spec in future products.

Regarding the handling of skipping PME_Turn_Off broadcast, should I add a 
flag bit to skip it in Synopsys public code or register a callback function 
to skip it in our platform driver? Which approach is better? I'd like to hear
your suggestions. Thanks!

Kind regards,
Senchuan Zhang

> 
> > > > +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> > > > +{
> > > > +	u16 offset, val;
> > > > +
> > > > +	/*
> > > > +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> > > > +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> > > > +	 * linked-list needs to be disabled. Since the PCI Express capability
> > > > +	 * structure's next pointer points to the MSI-X capability, and the
> > > > +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> > > > +	 * Express capability structure's next pointer needs to be set 00H.
> > > > +	 */
> > > > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > > > +	val = dw_pcie_readl_dbi(pci, offset);
> > > > +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> > > > +	dw_pcie_writel_dbi(pci, offset, val);
> > > 
> > > I hate to enforce dependency for your series, but this is properly handled here:
> > > https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-2-2208f46f4dc2@oss.qualcomm.com
> > 
> > Okey, then I can rely on this patch to solve the problem of the missing MSI-X. 
> > Thank you for the reminder.
> > 
> > > 
> > > > +}
> > > > +
> > > > +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> > > > +{
> > > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> > > > +	struct eic7700_pcie_port *port;
> > > > +	u8 msi_cap;
> > > > +	u32 val;
> > > > +	int ret;
> > > > +
> > > > +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> > > > +	if (pcie->num_clks < 0)
> > > > +		return dev_err_probe(pci->dev, pcie->num_clks,
> > > > +				     "Failed to get pcie clocks\n");
> > > > +
> > > > +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
> > > 
> > > I think this is being called too early.
> > 
> > Before accessing the register. I need to deassert, otherwise, there will
> > be problems with register access.
> > 
> > > 
> > > > +	if (ret) {
> > > > +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Configure Root Port type */
> > > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> > > > +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> > > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > +
> > > > +	list_for_each_entry(port, &pcie->ports, list) {
> > > > +		ret = eic7700_pcie_perst_deassert(port, pcie);
> > > > +		if (ret)
> > > > +			goto err_perst;
> > > > +	}
> > > > +
> > > > +	/* Configure app_hold_phy_rst */
> > > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> > > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > +
> > > > +	/* The maximum waiting time for the clock switch lock is 20ms */
> > > > +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> > > > +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> > > > +				 20000);
> > > > +	if (ret) {
> > > > +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> > > > +		goto err_phy_init;
> > > > +	}
> > > 
> > > You seem to be configuring the PHY reset and Aux clock, which should come before
> > > deasserting PERST# IMO. PERST# deassertion indicates that the power and clock
> > > are stable and the endpoint can start its operation. I don't know the impact of
> > > these configurations, but it is safe to do them earlier.
> > 
> > I think your understanding is correct. Unfortunately, in our hardware design, we 
> > need to deassert perst first before we can operate the configuration of the phy.
> > 
> 
> Sorry, I don't understand how it is possible, unless this reset is not PERST#.
> PERST# is just an indication to the component and has no relation with the
> controller.

After confirmation with the hardware team, Clarification as follows:

The power and reference clk of phy for our product PCIe are provided by the power
module and reference clock crystal oscillator on the board, and the power and 
reference clk of phy for the remote EP are also provided by the power and reference
clock crystal oscillator on the board through slots, using PCIe common reference 
clock mode. 
   
PERST# is sent to the local PCIe through the configuration register inside our 
chip, and this PERST# is also sent to the remote EP through the slot. Therefore,
in order to deassert PERST#, we first need our system to boot and configure it
through software, hen app_hold_phy_rst can be configured through software, the 
entire chip is already in a stable state, including the stability of power and 
reference clock.

Both our PCIe controller and phy require PERST# as the primary reset source, 
the configuration of app_hold_phy_rst is controlled by the controller to control
the state of phy through the pipe_raneX_powerdown signal in the pipe interface. 
Therefore, it is necessary to first deassert PERST# release controller and phy's 
Perst_n before configuring app_hold_phy_rst to allow the controller to control 
the pipe_raneX_powerdown signal.

PERST# is sent to both local PCIe and remote EP simultaneously for RC and EP 
reset synchronization, as well as to meet PCIe Spec requirements, which require 
entering the training process after stable power on and 20ms after deassert PERST#.
The signal control diagram is as follows:
                                           |
                                           | PERST#
                                           |
                    |----------------------|------------------------|                       
                    |                      |                        |
                    V                      V                        V
             |--------------|       |--------------|        |--------------|
             |  Controler   |       |      PHY     |        |      EP      |               
             |--------------|       |--------------|        |--------------| 
                    Λ                      Λ    Λ                      Λ
                    |        pci—>elbi_base|    |                      |
                    |                      |    |----------------------|
                    |pcie->resets          |                Λ
                    |(pwr/dbi resets)      |                |External ref clk
                                                            |

Kind regards,
Senchuan Zhang


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

* Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
  2025-11-28  8:26         ` zhangsenchuan
@ 2025-11-28 15:07           ` Manivannan Sadhasivam
  0 siblings, 0 replies; 15+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-28 15:07 UTC (permalink / raw)
  To: zhangsenchuan
  Cc: bhelgaas, krzk+dt, conor+dt, lpieralisi, kwilczynski, robh,
	p.zabel, jingoohan1, gustavo.pimentel, linux-pci, devicetree,
	linux-kernel, christian.bruel, mayank.rana, shradha.t,
	krishna.chundru, thippeswamy.havalige, inochiama, ningyu, linmin,
	pinkesh.vaghela, ouyanghui, Frank.li

On Fri, Nov 28, 2025 at 04:26:14PM +0800, zhangsenchuan wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > Send time:Wednesday, 26/11/2025 21:14:54
> > To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> > Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> > Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> > 
> > On Wed, Nov 26, 2025 at 08:15:11PM +0800, zhangsenchuan wrote:
> > > 
> > > 
> > > 
> > > > -----Original Messages-----
> > > > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > > > Send time:Thursday, 20/11/2025 20:43:47
> > > > To: zhangsenchuan@eswincomputing.com
> > > > Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
> > > > Subject: Re: [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver
> > > > 
> > > > On Thu, Nov 20, 2025 at 06:12:06PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > > > > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > > 
> > > > > Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> > > > > the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> > > > > supports a data rate of 8 GT/s and 4 channels, support INTx and MSI
> > > > > interrupts.
> > > > > 
> > > > > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > > > > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > > > > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > > ---
> > > > >  drivers/pci/controller/dwc/Kconfig        |  11 +
> > > > >  drivers/pci/controller/dwc/Makefile       |   1 +
> > > > >  drivers/pci/controller/dwc/pcie-eic7700.c | 387 ++++++++++++++++++++++
> > > > >  3 files changed, 399 insertions(+)
> > > > >  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > > > index 349d4657393c..66568efb324f 100644
> > > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > > @@ -93,6 +93,17 @@ config PCIE_BT1
> > > > >  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
> > > > >  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
> > > > >  
> > > > > +config PCIE_EIC7700
> > > > > +	bool "Eswin EIC7700 PCIe controller"
> > > > 
> > > > You can make it tristate as you've used builtin_platform_driver() which
> > > > guarantees that this driver won't be removed once loaded.
> > > 
> > > Okey, thanks
> > > 
> > > > 
> > > > > +	depends on ARCH_ESWIN || COMPILE_TEST
> > > > > +	depends on PCI_MSI
> > > > > +	select PCIE_DW_HOST
> > > > > +	help
> > > > > +	  Say Y here if you want PCIe controller support for the Eswin EIC7700.
> > > > > +	  The PCIe controller on EIC7700 is based on DesignWare hardware,
> > > > > +	  enables support for the PCIe controller in the EIC7700 SoC to work in
> > > > > +	  host mode.
> > > > > +
> > > > >  config PCI_IMX6
> > > > >  	bool
> > > > >  
> > > > > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > > > > index 7ae28f3b0fb3..04f751c49eba 100644
> > > > > --- a/drivers/pci/controller/dwc/Makefile
> > > > > +++ b/drivers/pci/controller/dwc/Makefile
> > > > > @@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> > > > >  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> > > > >  obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
> > > > >  obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
> > > > > +obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
> > > > >  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> > > > >  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> > > > >  obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > > > new file mode 100644
> > > > > index 000000000000..239fdbc501fe
> > > > > --- /dev/null
> > > > > +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> > > > > @@ -0,0 +1,387 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * ESWIN EIC7700 PCIe root complex driver
> > > > > + *
> > > > > + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> > > > > + *
> > > > > + * Authors: Yu Ning <ningyu@eswincomputing.com>
> > > > > + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > > > + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/interrupt.h>
> > > > > +#include <linux/iopoll.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/of.h>
> > > > > +#include <linux/pci.h>
> > > > > +#include <linux/platform_device.h>
> > > > > +#include <linux/resource.h>
> > > > > +#include <linux/reset.h>
> > > > > +#include <linux/types.h>
> > > > > +
> > > > > +#include "pcie-designware.h"
> > > > > +
> > > > > +/* ELBI registers */
> > > > > +#define PCIEELBI_CTRL0_OFFSET		0x0
> > > > > +#define PCIEELBI_STATUS0_OFFSET		0x100
> > > > > +
> > > > > +/* LTSSM register fields */
> > > > > +#define PCIEELBI_APP_LTSSM_ENABLE	BIT(5)
> > > > > +
> > > > > +/* APP_HOLD_PHY_RST register fields */
> > > > > +#define PCIEELBI_APP_HOLD_PHY_RST	BIT(6)
> > > > > +
> > > > > +/* PM_SEL_AUX_CLK register fields */
> > > > > +#define PCIEELBI_PM_SEL_AUX_CLK		BIT(16)
> > > > > +
> > > > > +/* DEV_TYPE register fields */
> > > > > +#define PCIEELBI_CTRL0_DEV_TYPE		GENMASK(3, 0)
> > > > > +
> > > > > +/* Vendor and device ID value */
> > > > > +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> > > > > +#define PCI_DEVICE_ID_ESWIN		0x2030
> > > > > +
> > > > > +#define EIC7700_NUM_RSTS		ARRAY_SIZE(eic7700_pcie_rsts)
> > > > > +
> > > > > +static const char * const eic7700_pcie_rsts[] = {
> > > > > +	"pwr",
> > > > > +	"dbi",
> > > > > +};
> > > > > +
> > > > > +struct eic7700_pcie_data {
> > > > > +	bool msix_cap;
> > > > > +	bool no_suspport_L2;
> > > > 
> > > > support?
> > > 
> > > Okey, spelling mistake:)
> > > 
> > > mani, by the way, our controller cannot broadcast PME_Turn_Off. Previously,
> > > i skip broadcast PME_Turn_Off in our controller code. Frank suggested that 
> > > set the flag bit to skip in the public code. At present, do you think it's 
> > > okay for me to add no_support_L2?
> > > 
> > 
> > This sounds weird. As per the spec r6.0, sec 5.2, "L2/L3 Ready transition
> > protocol support is required." So this means the controller has to support
> > PME_Turn_Off broadcast. Are you saying that your controller is not spec
> > compliant? and the link can only enter L3 (power off) abruptly?
> 
> Hi, Mani
> 
> Clarification
> After confirmation with the hardware team, L2/L3 low-power link state is not 
> supported, and cannot be controlled to enter L2/L3 through trigger 
> PME_turn_off/PME_To-Ack handshake, because this function is not implemented in 
> hardware, nor is the Vmain/Vaux power domain implemented to implement L2/L3 
> low-power wake-up mechanism.
> 

Ok, thanks for checking.

> Our PCIe only implements L0s/L1/L1.1 low-power mode. if want to put PCIe into 
> a more energy-efficient state, only power down PCIe. Although the PCIe Spec 
> requires the implementation of L2/L3 ready state, it is regrettable that we
> have not been able to match the protocol requirements at this point. We will
> gradually improve more features required by the Spec in future products.
> 
> Regarding the handling of skipping PME_Turn_Off broadcast, should I add a 
> flag bit to skip it in Synopsys public code or register a callback function 
> to skip it in our platform driver? Which approach is better? I'd like to hear
> your suggestions. Thanks!
> 

I'd prefer a flag, like, 'struct dw_pcie::no_pme_handshake'.

> Kind regards,
> Senchuan Zhang
> 
> > 
> > > > > +static void eic7700_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
> > > > > +{
> > > > > +	u16 offset, val;
> > > > > +
> > > > > +	/*
> > > > > +	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
> > > > > +	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
> > > > > +	 * linked-list needs to be disabled. Since the PCI Express capability
> > > > > +	 * structure's next pointer points to the MSI-X capability, and the
> > > > > +	 * MSI-X capability's next pointer is null (00H), so only the PCI
> > > > > +	 * Express capability structure's next pointer needs to be set 00H.
> > > > > +	 */
> > > > > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > > > > +	val = dw_pcie_readl_dbi(pci, offset);
> > > > > +	val &= ~PCI_CAP_LIST_NEXT_MASK;
> > > > > +	dw_pcie_writel_dbi(pci, offset, val);
> > > > 
> > > > I hate to enforce dependency for your series, but this is properly handled here:
> > > > https://lore.kernel.org/linux-pci/20251109-remove_cap-v1-2-2208f46f4dc2@oss.qualcomm.com
> > > 
> > > Okey, then I can rely on this patch to solve the problem of the missing MSI-X. 
> > > Thank you for the reminder.
> > > 
> > > > 
> > > > > +}
> > > > > +
> > > > > +static int eic7700_pcie_host_init(struct dw_pcie_rp *pp)
> > > > > +{
> > > > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > > +	struct eic7700_pcie *pcie = to_eic7700_pcie(pci);
> > > > > +	struct eic7700_pcie_port *port;
> > > > > +	u8 msi_cap;
> > > > > +	u32 val;
> > > > > +	int ret;
> > > > > +
> > > > > +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> > > > > +	if (pcie->num_clks < 0)
> > > > > +		return dev_err_probe(pci->dev, pcie->num_clks,
> > > > > +				     "Failed to get pcie clocks\n");
> > > > > +
> > > > > +	ret = reset_control_bulk_deassert(EIC7700_NUM_RSTS, pcie->resets);
> > > > 
> > > > I think this is being called too early.
> > > 
> > > Before accessing the register. I need to deassert, otherwise, there will
> > > be problems with register access.
> > > 
> > > > 
> > > > > +	if (ret) {
> > > > > +		dev_err(pcie->pci.dev, "Failed to deassert resets\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Configure Root Port type */
> > > > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > > +	val &= ~PCIEELBI_CTRL0_DEV_TYPE;
> > > > > +	val |= FIELD_PREP(PCIEELBI_CTRL0_DEV_TYPE, PCI_EXP_TYPE_ROOT_PORT);
> > > > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > > +
> > > > > +	list_for_each_entry(port, &pcie->ports, list) {
> > > > > +		ret = eic7700_pcie_perst_deassert(port, pcie);
> > > > > +		if (ret)
> > > > > +			goto err_perst;
> > > > > +	}
> > > > > +
> > > > > +	/* Configure app_hold_phy_rst */
> > > > > +	val = readl_relaxed(pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > > +	val &= ~PCIEELBI_APP_HOLD_PHY_RST;
> > > > > +	writel_relaxed(val, pci->elbi_base + PCIEELBI_CTRL0_OFFSET);
> > > > > +
> > > > > +	/* The maximum waiting time for the clock switch lock is 20ms */
> > > > > +	ret = readl_poll_timeout(pci->elbi_base + PCIEELBI_STATUS0_OFFSET,
> > > > > +				 val, !(val & PCIEELBI_PM_SEL_AUX_CLK), 1000,
> > > > > +				 20000);
> > > > > +	if (ret) {
> > > > > +		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
> > > > > +		goto err_phy_init;
> > > > > +	}
> > > > 
> > > > You seem to be configuring the PHY reset and Aux clock, which should come before
> > > > deasserting PERST# IMO. PERST# deassertion indicates that the power and clock
> > > > are stable and the endpoint can start its operation. I don't know the impact of
> > > > these configurations, but it is safe to do them earlier.
> > > 
> > > I think your understanding is correct. Unfortunately, in our hardware design, we 
> > > need to deassert perst first before we can operate the configuration of the phy.
> > > 
> > 
> > Sorry, I don't understand how it is possible, unless this reset is not PERST#.
> > PERST# is just an indication to the component and has no relation with the
> > controller.
> 
> After confirmation with the hardware team, Clarification as follows:
> 
> The power and reference clk of phy for our product PCIe are provided by the power
> module and reference clock crystal oscillator on the board, and the power and 
> reference clk of phy for the remote EP are also provided by the power and reference
> clock crystal oscillator on the board through slots, using PCIe common reference 
> clock mode. 
>    
> PERST# is sent to the local PCIe through the configuration register inside our 
> chip, and this PERST# is also sent to the remote EP through the slot. Therefore,
> in order to deassert PERST#, we first need our system to boot and configure it
> through software, hen app_hold_phy_rst can be configured through software, the 
> entire chip is already in a stable state, including the stability of power and 
> reference clock.
> 
> Both our PCIe controller and phy require PERST# as the primary reset source, 
> the configuration of app_hold_phy_rst is controlled by the controller to control
> the state of phy through the pipe_raneX_powerdown signal in the pipe interface. 
> Therefore, it is necessary to first deassert PERST# release controller and phy's 
> Perst_n before configuring app_hold_phy_rst to allow the controller to control 
> the pipe_raneX_powerdown signal.
> 

It looks crazy (using PERST# for both controller and the endpoint). Anyway, you
should include this information in the comment above
reset_control_bulk_deassert().

- Mani

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

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

end of thread, other threads:[~2025-11-28 15:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-20 10:10 [PATCH v6 0/3] Add driver support for Eswin EIC7700 SoC PCIe controller zhangsenchuan
2025-11-20 10:11 ` [PATCH v6 1/3] dt-bindings: PCI: eic7700: Add Eswin PCIe host controller zhangsenchuan
2025-11-20 10:12 ` [PATCH v6 2/3] PCI: eic7700: Add Eswin PCIe host controller driver zhangsenchuan
2025-11-20 12:43   ` Manivannan Sadhasivam
2025-11-26 12:15     ` zhangsenchuan
2025-11-26 13:14       ` Manivannan Sadhasivam
2025-11-28  8:26         ` zhangsenchuan
2025-11-28 15:07           ` Manivannan Sadhasivam
2025-11-20 13:19   ` Shawn Lin
2025-11-26 12:23     ` zhangsenchuan
2025-11-20 10:12 ` [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast zhangsenchuan
2025-11-20 12:45   ` Manivannan Sadhasivam
2025-11-21  6:48     ` zhangsenchuan
2025-11-21  7:14       ` Shawn Lin
2025-11-24  9:53         ` zhangsenchuan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).