[PATCH v1 0/2] Add driver support for Eswin eic7700 SoC PCIe controller

[PATCH v1 0/2] Add driver support for Eswin eic7700 SoC PCIe controller

[zhangsenchuan]

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

     Support for the Eswin eic7700 PCIe driver control program has been
    added to the Linux kernel, which is part of the Eswin SoC family.

    Features:
     Implements support for the Eswin eic7700 SoC PCIe controller,
    It is a high-speed data transmission interface, which can
    enhance the speed and performance of computers,It can be used
    to connect different types of devices.

    Supported chips:
     Eswin eic7700 series SoC.

    Test:
     Tested this patch on the Sifive HiFive Premier P550 (which uses the
    EIC7700 SoC),The pcie driver controller operates normally through
    the nvme peripheral test.

Senchuan Zhang (2):
  dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe host controller
  PCI: eic7700: Add Eswin eic7700 PCIe host controller driver

 .../bindings/pci/eswin,eic7700-pcie.yaml      | 171 +++++++
 drivers/pci/controller/dwc/Kconfig            |  12 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c     | 437 ++++++++++++++++++
 4 files changed, 621 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

[PATCH v1 1/2] dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe host controller

[zhangsenchuan]

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.

Co-developed-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 .../bindings/pci/eswin,eic7700-pcie.yaml      | 171 ++++++++++++++++++
 1 file changed, 171 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..e1d150c7c81a
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
@@ -0,0 +1,171 @@
+# 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>
+
+description: |
+  The PCIe controller on EIC7700 SoC.
+
+properties:
+  compatible:
+    const: eswin,eic7700-pcie
+
+  reg:
+    maxItems: 3
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: mgmt
+
+  "#address-cells":
+    const: 3
+  "#size-cells":
+    const: 2
+  '#interrupt-cells':
+    const: 1
+
+  interrupts:
+    maxItems: 9
+
+  interrupt-names:
+    items:
+      - const: msi
+      - const: inta
+      - const: intb
+      - const: intc
+      - const: intd
+
+  interrupt-controller:
+    type: object
+
+  interrupt-map:
+    maxItems: 4
+
+  interrupt-map-mask:
+    items:
+      - const: 0
+      - const: 0
+      - const: 0
+      - const: 7
+
+  clocks:
+    maxItems: 4
+    description: handles to clock for the pcie controller.
+
+  clock-names:
+    items:
+      - const: pcie_aclk
+      - const: pcie_cfg_clk
+      - const: pcie_cr_clk
+      - const: pcie_aux_clk
+    description: the name of each clock.
+
+  resets:
+    description: resets to be used by the controller.
+
+  reset-names:
+    items:
+      - const: pcie_cfg
+      - const: pcie_powerup
+      - const: pcie_pwren
+    description: names of the resets listed in resets property in the same order.
+
+  bus-range:
+    items:
+      - const: 0
+      - const: 0xff
+
+  device_type:
+    const: pci
+
+  ranges: true
+
+  dma-noncoherent: true
+
+  num-lanes:
+    maximum: 4
+
+  numa-node-id:
+    maximum: 0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - interrupt-parent
+  - interrupt-map-mask
+  - interrupt-map
+  - '#address-cells'
+  - '#size-cells'
+  - '#interrupt-cells'
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - bus-range
+  - dma-noncoherent
+  - num-lanes
+  - ranges
+  - numa-node-id
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie: pcie@54000000 {
+          compatible = "eswin,eic7700-pcie";
+          clocks = <&clock 562>,
+                   <&clock 563>,
+                   <&clock 564>,
+                   <&clock 565>;
+          clock-names = "pcie_aclk", "pcie_cfg_clk", "pcie_cr_clk", "pcie_aux_clk";
+
+          reset-names = "pcie_cfg", "pcie_powerup", "pcie_pwren";
+          resets = <&reset 8 (1 << 0)>,
+                   <&reset 8 (1 << 1)>,
+                   <&reset 8 (1 << 2)>;
+
+          #address-cells = <3>;
+          #size-cells = <2>;
+          #interrupt-cells = <1>;
+
+          reg = <0x0 0x54000000 0x0 0x4000000>,
+                <0x0 0x40000000 0x0 0x800000>,
+                <0x0 0x50000000 0x0 0x100000>;
+          reg-names = "dbi", "config", "mgmt";
+          device_type = "pci";
+
+          bus-range = <0x0 0xff>;
+          ranges = <0x81000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
+                  <0x82000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
+                  <0xc3000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
+
+          num-lanes = <0x4>;
+          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>;
+          status = "disabled";
+          numa-node-id = <0>;
+          dma-noncoherent;
+        };
+    };
--
2.25.1

[PATCH v1 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver

[zhangsenchuan]

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Add driver for the Eswin EIC7700 PCIe host controller.
The controller is based on the DesignWare PCIe core.

Co-developed-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 drivers/pci/controller/dwc/Kconfig        |  12 +
 drivers/pci/controller/dwc/Makefile       |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c | 437 ++++++++++++++++++++++
 3 files changed, 450 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 d9f0386396ed..21258f7dada9 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -480,4 +480,16 @@ config PCIE_VISCONTI_HOST
 	  Say Y here if you want PCIe controller support on Toshiba Visconti SoC.
 	  This driver supports TMPV7708 SoC.

+config PCIE_EIC7700
+	tristate "ESWIN PCIe host controller"
+	depends on PCI_MSI
+	depends on ARCH_ESWIN || COMPILE_TEST
+	select PCIE_DW_HOST
+	help
+      Enables support for the PCIe controller in the Eswin SoC
+      The PCI controller on Eswin is based on DesignWare hardware
+      It is a high-speed hardware bus standard used to connect
+      processors with external devices. Say Y here if you want
+      PCIe controller support for the ESWIN.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 908cb7f345db..2906477d92a5 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
 obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
 obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
 obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
+obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o

 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
new file mode 100644
index 000000000000..0413d1f61cb4
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-eic7700.c
@@ -0,0 +1,437 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN PCIe root complex driver
+ *
+ * Copyright 2024, Beijing ESWIN Computing Technology Co., Ltd.. All rights reserved.
+ *
+ * Authors: Yu Ning <ningyu@eswincomputing.com>
+ *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/resource.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/reset.h>
+#include <linux/gpio/consumer.h>
+#include <linux/property.h>
+#include "pcie-designware.h"
+#include <linux/pm_runtime.h>
+struct eswin_pcie {
+	struct dw_pcie pci;
+	void __iomem *mgmt_base;
+	struct gpio_desc *reset;
+	struct clk *pcie_aux;
+	struct clk *pcie_cfg;
+	struct clk *pcie_cr;
+	struct clk *pcie_aclk;
+	struct reset_control *powerup_rst;
+	struct reset_control *cfg_rst;
+	struct reset_control *perst;
+};
+
+#define PCIE_PM_SEL_AUX_CLK BIT(16)
+#define PCIEMGMT_APP_HOLD_PHY_RST BIT(6)
+#define PCIEMGMT_APP_LTSSM_ENABLE BIT(5)
+#define PCIEMGMT_DEVICE_TYPE_MASK 0xf
+
+#define PCIEMGMT_CTRL0_OFFSET 0x0
+#define PCIEMGMT_STATUS0_OFFSET 0x100
+
+#define PCIE_TYPE_DEV_VEND_ID 0x0
+#define PCIE_DSP_PF0_MSI_CAP 0x50
+#define PCIE_NEXT_CAP_PTR 0x70
+#define DEVICE_CONTROL_DEVICE_STATUS 0x78
+
+#define PCIE_MSI_MULTIPLE_MSG_32 (0x5 << 17)
+#define PCIE_MSI_MULTIPLE_MSG_MASK (0x7 << 17)
+
+#define PCIEMGMT_LINKUP_STATE_VALIDATE ((0x11 << 2) | 0x3)
+#define PCIEMGMT_LINKUP_STATE_MASK 0xff
+
+static void eswin_pcie_shutdown(struct platform_device *pdev)
+{
+	struct eswin_pcie *pcie = platform_get_drvdata(pdev);
+
+	/* Bring down link, so bootloader gets clean state in case of reboot */
+	reset_control_assert(pcie->perst);
+}
+
+static int eswin_pcie_start_link(struct dw_pcie *pci)
+{
+	struct device *dev = pci->dev;
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+	u32 val;
+
+	/* Enable LTSSM */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val |= PCIEMGMT_APP_LTSSM_ENABLE;
+	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	return 0;
+}
+
+static int eswin_pcie_link_up(struct dw_pcie *pci)
+{
+	struct device *dev = pci->dev;
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+	u32 val;
+
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_STATUS0_OFFSET);
+	if ((val & PCIEMGMT_LINKUP_STATE_MASK) ==
+	    PCIEMGMT_LINKUP_STATE_VALIDATE)
+		return 1;
+	else
+		return 0;
+}
+
+static int eswin_pcie_clk_enable(struct eswin_pcie *pcie)
+{
+	int ret;
+
+	ret = clk_prepare_enable(pcie->pcie_cr);
+	if (ret) {
+		pr_err("PCIe: failed to enable cr clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(pcie->pcie_aclk);
+	if (ret) {
+		pr_err("PCIe: failed to enable aclk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(pcie->pcie_cfg);
+	if (ret) {
+		pr_err("PCIe: failed to enable cfg_clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(pcie->pcie_aux);
+	if (ret) {
+		pr_err("PCIe: failed to enable aux_clk: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int eswin_pcie_clk_disable(struct eswin_pcie *eswin_pcie)
+{
+	clk_disable_unprepare(eswin_pcie->pcie_aux);
+	clk_disable_unprepare(eswin_pcie->pcie_cfg);
+	clk_disable_unprepare(eswin_pcie->pcie_cr);
+	clk_disable_unprepare(eswin_pcie->pcie_aclk);
+
+	return 0;
+}
+
+static int eswin_pcie_power_on(struct eswin_pcie *pcie)
+{
+	int ret = 0;
+
+	/* pciet_cfg_rstn */
+	ret = reset_control_reset(pcie->cfg_rst);
+	WARN_ON(ret != 0);
+
+	/* pciet_powerup_rstn */
+	ret = reset_control_reset(pcie->powerup_rst);
+	WARN_ON(ret != 0);
+
+	return ret;
+}
+
+static int eswin_pcie_power_off(struct eswin_pcie *eswin_pcie)
+{
+	reset_control_assert(eswin_pcie->perst);
+
+	reset_control_assert(eswin_pcie->powerup_rst);
+
+	reset_control_assert(eswin_pcie->cfg_rst);
+
+	return 0;
+}
+
+static int eswin_evb_socket_power_on(struct device *dev)
+{
+	int err_desc = 0;
+	struct gpio_desc *gpio;
+
+	gpio = devm_gpiod_get(dev, "pci-socket", GPIOD_OUT_LOW);
+	err_desc = IS_ERR(gpio);
+
+	if (err_desc) {
+		pr_debug("No power control gpio found, maybe not needed\n");
+		return 0;
+	}
+
+	gpiod_set_value(gpio, 1);
+
+	return err_desc;
+}
+
+static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct eswin_pcie *pcie = dev_get_drvdata(pci->dev);
+	int ret;
+	u32 val;
+
+	/* pciet_aux_clken, pcie_cfg_clken */
+	ret = eswin_pcie_clk_enable(pcie);
+	if (ret)
+		return ret;
+
+	ret = eswin_pcie_power_on(pcie);
+	if (ret)
+		return ret;
+
+	/* set device type : rc */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val &= 0xfffffff0;
+	writel_relaxed(val | 0x4, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+
+	ret = reset_control_assert(pcie->perst);
+	WARN_ON(ret != 0);
+
+	eswin_evb_socket_power_on(pcie->pci.dev);
+	msleep(100);
+	ret = reset_control_deassert(pcie->perst);
+	WARN_ON(ret != 0);
+
+	/* app_hold_phy_rst */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val &= ~(0x40);
+	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+
+	/* wait pm_sel_aux_clk to 0 */
+	for (ret = 50; ret > 0; ret--) {
+		val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_STATUS0_OFFSET);
+		if (!(val & PCIE_PM_SEL_AUX_CLK))
+			break;
+		msleep(2);
+	}
+
+	if (!ret) {
+		dev_info(pcie->pci.dev, "No clock exist.\n");
+		eswin_pcie_power_off(pcie);
+		eswin_pcie_clk_disable(pcie);
+		return -ENODEV;
+	}
+
+	/* config eswin vendor id and win2030 device id */
+	dw_pcie_writel_dbi(pci, PCIE_TYPE_DEV_VEND_ID, 0x20301fe1);
+
+	/* lane fix config, real driver NOT need, default x4 */
+	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
+	val &= 0xffffff80;
+	val |= 0x44;
+	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
+
+	val = dw_pcie_readl_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS);
+	val &= ~(0x7 << 5);
+	val |= (0x2 << 5);
+	dw_pcie_writel_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS, val);
+
+	/*  config support 32 msi vectors */
+	val = dw_pcie_readl_dbi(pci, PCIE_DSP_PF0_MSI_CAP);
+	val &= ~PCIE_MSI_MULTIPLE_MSG_MASK;
+	val |= PCIE_MSI_MULTIPLE_MSG_32;
+	dw_pcie_writel_dbi(pci, PCIE_DSP_PF0_MSI_CAP, val);
+
+	/* disable msix cap */
+	val = dw_pcie_readl_dbi(pci, PCIE_NEXT_CAP_PTR);
+	val &= 0xffff00ff;
+	dw_pcie_writel_dbi(pci, PCIE_NEXT_CAP_PTR, val);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
+	.init = eswin_pcie_host_init,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = eswin_pcie_start_link,
+	.link_up = eswin_pcie_link_up,
+};
+
+static int __exit eswin_pcie_remove(struct platform_device *pdev)
+{
+	struct eswin_pcie *pcie = platform_get_drvdata(pdev);
+
+	dw_pcie_host_deinit(&pcie->pci.pp);
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	eswin_pcie_power_off(pcie);
+	eswin_pcie_clk_disable(pcie);
+
+	return 0;
+}
+
+static int eswin_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct dw_pcie *pci;
+	struct eswin_pcie *pcie;
+	int err;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+	pci = &pcie->pci;
+	pci->dev = dev;
+	pci->ops = &dw_pcie_ops;
+	pci->pp.ops = &eswin_pcie_host_ops;
+
+	/* SiFive specific region: mgmt */
+	pcie->mgmt_base = devm_platform_ioremap_resource_byname(pdev, "mgmt");
+	if (IS_ERR(pcie->mgmt_base))
+		return PTR_ERR(pcie->mgmt_base);
+
+	/* Fetch clocks */
+	pcie->pcie_aux = devm_clk_get(dev, "pcie_aux_clk");
+	if (IS_ERR(pcie->pcie_aux)) {
+		return dev_err_probe(
+			dev, PTR_ERR(pcie->pcie_aux),
+			"pcie_aux clock source missing or invalid\n");
+	}
+
+	pcie->pcie_cfg = devm_clk_get(dev, "pcie_cfg_clk");
+	if (IS_ERR(pcie->pcie_cfg)) {
+		return dev_err_probe(
+			dev, PTR_ERR(pcie->pcie_cfg),
+			"pcie_cfg_clk clock source missing or invalid\n");
+	}
+
+	pcie->pcie_cr = devm_clk_get(dev, "pcie_cr_clk");
+	if (IS_ERR(pcie->pcie_cr)) {
+		return dev_err_probe(
+			dev, PTR_ERR(pcie->pcie_cr),
+			"pcie_cr_clk clock source missing or invalid\n");
+	}
+
+	pcie->pcie_aclk = devm_clk_get(dev, "pcie_aclk");
+
+	if (IS_ERR(pcie->pcie_aclk)) {
+		return dev_err_probe(
+			dev, PTR_ERR(pcie->pcie_aclk),
+			"pcie_aclk clock source missing or invalid\n");
+	}
+
+	/* Fetch reset */
+	pcie->powerup_rst =
+		devm_reset_control_get_optional(&pdev->dev, "pcie_powerup");
+	if (IS_ERR_OR_NULL(pcie->powerup_rst))
+		dev_err_probe(dev, PTR_ERR(pcie->powerup_rst),
+			      "unable to get powerup reset\n");
+
+	pcie->cfg_rst = devm_reset_control_get_optional(&pdev->dev, "pcie_cfg");
+	if (IS_ERR_OR_NULL(pcie->cfg_rst))
+		dev_err_probe(dev, PTR_ERR(pcie->cfg_rst),
+			      "unable to get cfg reset\n");
+
+	pcie->perst = devm_reset_control_get_optional(&pdev->dev, "pcie_pwren");
+	if (IS_ERR_OR_NULL(pcie->perst))
+		dev_err_probe(dev, PTR_ERR(pcie->perst),
+			      "unable to get perst\n");
+
+	platform_set_drvdata(pdev, pcie);
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	err = pm_runtime_get_sync(dev);
+	if (err < 0) {
+		dev_err(dev, "pm_runtime_get_sync failed: %d\n", err);
+		goto pm_runtime_put;
+	}
+
+	return dw_pcie_host_init(&pci->pp);
+
+pm_runtime_put:
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+	return err;
+}
+
+static const struct of_device_id eswin_pcie_of_match[] = {
+	{
+		.compatible = "eswin,eic7700-pcie",
+	},
+	{},
+};
+
+static int eswin_pcie_suspend(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "suspend %s\n", __func__);
+	if (!pm_runtime_status_suspended(dev))
+		eswin_pcie_clk_disable(pcie);
+
+	return 0;
+}
+
+static int eswin_pcie_resume(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "resume %s\n", __func__);
+	if (!pm_runtime_status_suspended(dev))
+		eswin_pcie_clk_enable(pcie);
+
+	return 0;
+}
+
+static int eswin_pcie_runtime_suspend(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "runtime suspend %s\n", __func__);
+	return eswin_pcie_clk_disable(pcie);
+}
+
+static int eswin_pcie_runtime_resume(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "runtime resume %s\n", __func__);
+	return eswin_pcie_clk_enable(pcie);
+}
+
+static const struct dev_pm_ops eswin_pcie_pm_ops = {
+	RUNTIME_PM_OPS(eswin_pcie_runtime_suspend, eswin_pcie_runtime_resume,
+		       NULL)
+		NOIRQ_SYSTEM_SLEEP_PM_OPS(eswin_pcie_suspend, eswin_pcie_resume)
+};
+
+static struct platform_driver eswin_pcie_driver = {
+	.driver = {
+			.name = "eic7700-pcie",
+			.of_match_table = eswin_pcie_of_match,
+			.suppress_bind_attrs = true,
+			.pm = &eswin_pcie_pm_ops,
+	},
+	.probe = eswin_pcie_probe,
+	.remove = __exit_p(eswin_pcie_remove),
+	.shutdown = eswin_pcie_shutdown,
+};
+
+module_platform_driver(eswin_pcie_driver);
+
+MODULE_DEVICE_TABLE(of, eswin_pcie_of_match);
+MODULE_DESCRIPTION("PCIe host controller driver for eic7700 SoCs");
+MODULE_AUTHOR("Yu Ning <ningyu@eswincomputing.com>");
+MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com>");
+MODULE_LICENSE("GPL");
--
2.25.1

Re: [PATCH v1 1/2] dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe host controller

[Krzysztof Kozlowski]

On 16/05/2025 11:42, zhangsenchuan@eswincomputing.com wrote:
> 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.
> 
> Co-developed-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> ---
>  .../bindings/pci/eswin,eic7700-pcie.yaml      | 171 ++++++++++++++++++

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

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

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

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

Limited review follows.

>  1 file changed, 171 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..e1d150c7c81a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> @@ -0,0 +1,171 @@
> +# 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>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The PCIe controller on EIC7700 SoC.
> +
> +properties:
> +  compatible:
> +    const: eswin,eic7700-pcie
> +
> +  reg:
> +    maxItems: 3
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: config
> +      - const: mgmt
> +
> +  "#address-cells":
> +    const: 3
> +  "#size-cells":
> +    const: 2
> +  '#interrupt-cells':
> +    const: 1
> +
> +  interrupts:
> +    maxItems: 9
> +
> +  interrupt-names:
> +    items:
> +      - const: msi
> +      - const: inta
> +      - const: intb
> +      - const: intc
> +      - const: intd

Does not match interrupts.

> +
> +  interrupt-controller:
> +    type: object
> +
> +  interrupt-map:
> +    maxItems: 4
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0
> +      - const: 0
> +      - const: 0
> +      - const: 7
> +
> +  clocks:
> +    maxItems: 4
> +    description: handles to clock for the pcie controller.

Drop description, obvious.

> +
> +  clock-names:
> +    items:
> +      - const: pcie_aclk
> +      - const: pcie_cfg_clk
> +      - const: pcie_cr_clk
> +      - const: pcie_aux_clk

Drop all _clk
Drop all pcie_

> +    description: the name of each clock.

Drop description

> +
> +  resets:
> +    description: resets to be used by the controller.
> +
> +  reset-names:
> +    items:
> +      - const: pcie_cfg
> +      - const: pcie_powerup
> +      - const: pcie_pwren

Drop all pcie_

> +    description: names of the resets listed in resets property in the same order.

What did you use as starting point / example?

> +
> +  bus-range:
> +    items:
> +      - const: 0
> +      - const: 0xff
> +
> +  device_type:
> +    const: pci
> +
> +  ranges: true
> +
> +  dma-noncoherent: true
> +
> +  num-lanes:
> +    maximum: 4
> +
> +  numa-node-id:
> +    maximum: 0

This is confusing.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-names
> +  - interrupt-parent
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - '#address-cells'
> +  - '#size-cells'
> +  - '#interrupt-cells'
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - bus-range
> +  - dma-noncoherent
> +  - num-lanes
> +  - ranges
> +  - numa-node-id

Missing ref to pci schemas. Look at other bindings.

> +
> +additionalProperties: false

And then this will be changed as well.

> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie: pcie@54000000 {
> +          compatible = "eswin,eic7700-pcie";

Messed indentation. Use 4 spaces for example indentation.

> +          clocks = <&clock 562>,

Wrong property order. Follow DTS coding style.

> +                   <&clock 563>,
> +                   <&clock 564>,
> +                   <&clock 565>;
> +          clock-names = "pcie_aclk", "pcie_cfg_clk", "pcie_cr_clk", "pcie_aux_clk";
> +
> +          reset-names = "pcie_cfg", "pcie_powerup", "pcie_pwren";
> +          resets = <&reset 8 (1 << 0)>,
> +                   <&reset 8 (1 << 1)>,
> +                   <&reset 8 (1 << 2)>;
> +
> +          #address-cells = <3>;
> +          #size-cells = <2>;
> +          #interrupt-cells = <1>;
> +
> +          reg = <0x0 0x54000000 0x0 0x4000000>,
> +                <0x0 0x40000000 0x0 0x800000>,
> +                <0x0 0x50000000 0x0 0x100000>;
> +          reg-names = "dbi", "config", "mgmt";
> +          device_type = "pci";
> +
> +          bus-range = <0x0 0xff>;
> +          ranges = <0x81000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
> +                  <0x82000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
> +                  <0xc3000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
> +
> +          num-lanes = <0x4>;
> +          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>;
> +          status = "disabled";

Drop. Look at other bindings how this is written. There is no binding
with status disabled. It makes just no sense.

> +          numa-node-id = <0>;
> +          dma-noncoherent;
> +        };
> +    };
> --
> 2.25.1
> 


Best regards,
Krzysztof

Re: [PATCH v1 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver

[Krzysztof Kozlowski]

On 16/05/2025 11:43, zhangsenchuan@eswincomputing.com wrote:

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>

Where do you use it?

> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/reset.h>
> +#include <linux/gpio/consumer.h>

That's mess. Clean up your patches before sending from such trivial
mistakes.

> +#include <linux/property.h>

Most of these headers look like unused.

> +#include "pcie-designware.h"
> +#include <linux/pm_runtime.h>
> +struct eswin_pcie {
> +	struct dw_pcie pci;
> +	void __iomem *mgmt_base;
> +	struct gpio_desc *reset;
> +	struct clk *pcie_aux;
> +	struct clk *pcie_cfg;
> +	struct clk *pcie_cr;
> +	struct clk *pcie_aclk;
> +	struct reset_control *powerup_rst;
> +	struct reset_control *cfg_rst;
> +	struct reset_control *perst;
> +};
> +
> +#define PCIE_PM_SEL_AUX_CLK BIT(16)
> +#define PCIEMGMT_APP_HOLD_PHY_RST BIT(6)
> +#define PCIEMGMT_APP_LTSSM_ENABLE BIT(5)
> +#define PCIEMGMT_DEVICE_TYPE_MASK 0xf
> +
> +#define PCIEMGMT_CTRL0_OFFSET 0x0
> +#define PCIEMGMT_STATUS0_OFFSET 0x100
> +
> +#define PCIE_TYPE_DEV_VEND_ID 0x0
> +#define PCIE_DSP_PF0_MSI_CAP 0x50
> +#define PCIE_NEXT_CAP_PTR 0x70
> +#define DEVICE_CONTROL_DEVICE_STATUS 0x78
> +
> +#define PCIE_MSI_MULTIPLE_MSG_32 (0x5 << 17)
> +#define PCIE_MSI_MULTIPLE_MSG_MASK (0x7 << 17)
> +
> +#define PCIEMGMT_LINKUP_STATE_VALIDATE ((0x11 << 2) | 0x3)
> +#define PCIEMGMT_LINKUP_STATE_MASK 0xff
> +
> +static void eswin_pcie_shutdown(struct platform_device *pdev)
> +{
> +	struct eswin_pcie *pcie = platform_get_drvdata(pdev);
> +
> +	/* Bring down link, so bootloader gets clean state in case of reboot */
> +	reset_control_assert(pcie->perst);
> +}
> +
> +static int eswin_pcie_start_link(struct dw_pcie *pci)
> +{
> +	struct device *dev = pci->dev;
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	/* Enable LTSSM */
> +	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +	val |= PCIEMGMT_APP_LTSSM_ENABLE;
> +	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +	return 0;
> +}
> +
> +static int eswin_pcie_link_up(struct dw_pcie *pci)
> +{
> +	struct device *dev = pci->dev;
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_STATUS0_OFFSET);
> +	if ((val & PCIEMGMT_LINKUP_STATE_MASK) ==
> +	    PCIEMGMT_LINKUP_STATE_VALIDATE)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +static int eswin_pcie_clk_enable(struct eswin_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(pcie->pcie_cr);
> +	if (ret) {
> +		pr_err("PCIe: failed to enable cr clk: %d\n", ret);

No, your driver must use everywhere dev_, not pr_.

> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(pcie->pcie_aclk);
> +	if (ret) {
> +		pr_err("PCIe: failed to enable aclk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(pcie->pcie_cfg);
> +	if (ret) {
> +		pr_err("PCIe: failed to enable cfg_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(pcie->pcie_aux);
> +	if (ret) {
> +		pr_err("PCIe: failed to enable aux_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_clk_disable(struct eswin_pcie *eswin_pcie)
> +{
> +	clk_disable_unprepare(eswin_pcie->pcie_aux);
> +	clk_disable_unprepare(eswin_pcie->pcie_cfg);
> +	clk_disable_unprepare(eswin_pcie->pcie_cr);
> +	clk_disable_unprepare(eswin_pcie->pcie_aclk);
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_power_on(struct eswin_pcie *pcie)
> +{
> +	int ret = 0;
> +
> +	/* pciet_cfg_rstn */
> +	ret = reset_control_reset(pcie->cfg_rst);
> +	WARN_ON(ret != 0);

No, handle the error instead.

> +
> +	/* pciet_powerup_rstn */
> +	ret = reset_control_reset(pcie->powerup_rst);
> +	WARN_ON(ret != 0);

No, handle the error instead.

> +
> +	return ret;
> +}
> +
> +static int eswin_pcie_power_off(struct eswin_pcie *eswin_pcie)
> +{
> +	reset_control_assert(eswin_pcie->perst);
> +
> +	reset_control_assert(eswin_pcie->powerup_rst);
> +
> +	reset_control_assert(eswin_pcie->cfg_rst);
> +
> +	return 0;
> +}
> +
> +static int eswin_evb_socket_power_on(struct device *dev)
> +{
> +	int err_desc = 0;
> +	struct gpio_desc *gpio;
> +
> +	gpio = devm_gpiod_get(dev, "pci-socket", GPIOD_OUT_LOW);

Undocumented ABI. Looks not correct either - see PCI controller bindings.


> +	err_desc = IS_ERR(gpio);
> +
> +	if (err_desc) {
> +		pr_debug("No power control gpio found, maybe not needed\n");
> +		return 0;
> +	}
> +
> +	gpiod_set_value(gpio, 1);
> +
> +	return err_desc;
> +}
> +
> +static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eswin_pcie *pcie = dev_get_drvdata(pci->dev);
> +	int ret;
> +	u32 val;
> +
> +	/* pciet_aux_clken, pcie_cfg_clken */
> +	ret = eswin_pcie_clk_enable(pcie);
> +	if (ret)
> +		return ret;
> +
> +	ret = eswin_pcie_power_on(pcie);
> +	if (ret)
> +		return ret;
> +
> +	/* set device type : rc */
> +	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +	val &= 0xfffffff0;
> +	writel_relaxed(val | 0x4, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +
> +	ret = reset_control_assert(pcie->perst);
> +	WARN_ON(ret != 0);
> +
> +	eswin_evb_socket_power_on(pcie->pci.dev);
> +	msleep(100);
> +	ret = reset_control_deassert(pcie->perst);
> +	WARN_ON(ret != 0);
> +
> +	/* app_hold_phy_rst */
> +	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +	val &= ~(0x40);
> +	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +
> +	/* wait pm_sel_aux_clk to 0 */
> +	for (ret = 50; ret > 0; ret--) {
> +		val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_STATUS0_OFFSET);
> +		if (!(val & PCIE_PM_SEL_AUX_CLK))
> +			break;
> +		msleep(2);
> +	}
> +
> +	if (!ret) {
> +		dev_info(pcie->pci.dev, "No clock exist.\n");
> +		eswin_pcie_power_off(pcie);
> +		eswin_pcie_clk_disable(pcie);
> +		return -ENODEV;
> +	}
> +
> +	/* config eswin vendor id and win2030 device id */
> +	dw_pcie_writel_dbi(pci, PCIE_TYPE_DEV_VEND_ID, 0x20301fe1);
> +
> +	/* lane fix config, real driver NOT need, default x4 */
> +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
> +	val &= 0xffffff80;
> +	val |= 0x44;
> +	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
> +
> +	val = dw_pcie_readl_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS);
> +	val &= ~(0x7 << 5);
> +	val |= (0x2 << 5);
> +	dw_pcie_writel_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS, val);
> +
> +	/*  config support 32 msi vectors */
> +	val = dw_pcie_readl_dbi(pci, PCIE_DSP_PF0_MSI_CAP);
> +	val &= ~PCIE_MSI_MULTIPLE_MSG_MASK;
> +	val |= PCIE_MSI_MULTIPLE_MSG_32;
> +	dw_pcie_writel_dbi(pci, PCIE_DSP_PF0_MSI_CAP, val);
> +
> +	/* disable msix cap */
> +	val = dw_pcie_readl_dbi(pci, PCIE_NEXT_CAP_PTR);
> +	val &= 0xffff00ff;
> +	dw_pcie_writel_dbi(pci, PCIE_NEXT_CAP_PTR, val);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
> +	.init = eswin_pcie_host_init,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = eswin_pcie_start_link,
> +	.link_up = eswin_pcie_link_up,
> +};
> +
> +static int __exit eswin_pcie_remove(struct platform_device *pdev)

Remove goes after probe

> +{
> +	struct eswin_pcie *pcie = platform_get_drvdata(pdev);
> +
> +	dw_pcie_host_deinit(&pcie->pci.pp);
> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	eswin_pcie_power_off(pcie);
> +	eswin_pcie_clk_disable(pcie);
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct dw_pcie *pci;
> +	struct eswin_pcie *pcie;
> +	int err;
> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +	pci = &pcie->pci;
> +	pci->dev = dev;
> +	pci->ops = &dw_pcie_ops;
> +	pci->pp.ops = &eswin_pcie_host_ops;
> +
> +	/* SiFive specific region: mgmt */
> +	pcie->mgmt_base = devm_platform_ioremap_resource_byname(pdev, "mgmt");
> +	if (IS_ERR(pcie->mgmt_base))
> +		return PTR_ERR(pcie->mgmt_base);
> +
> +	/* Fetch clocks */
> +	pcie->pcie_aux = devm_clk_get(dev, "pcie_aux_clk");
> +	if (IS_ERR(pcie->pcie_aux)) {
> +		return dev_err_probe(
> +			dev, PTR_ERR(pcie->pcie_aux),
> +			"pcie_aux clock source missing or invalid\n");

Messed alignment. This applies eveywhere.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.

> +	}
> +
> +	pcie->pcie_cfg = devm_clk_get(dev, "pcie_cfg_clk");
> +	if (IS_ERR(pcie->pcie_cfg)) {
> +		return dev_err_probe(
> +			dev, PTR_ERR(pcie->pcie_cfg),
> +			"pcie_cfg_clk clock source missing or invalid\n");
> +	}
> +
> +	pcie->pcie_cr = devm_clk_get(dev, "pcie_cr_clk");
> +	if (IS_ERR(pcie->pcie_cr)) {
> +		return dev_err_probe(
> +			dev, PTR_ERR(pcie->pcie_cr),
> +			"pcie_cr_clk clock source missing or invalid\n");
> +	}
> +
> +	pcie->pcie_aclk = devm_clk_get(dev, "pcie_aclk");
> +
> +	if (IS_ERR(pcie->pcie_aclk)) {
> +		return dev_err_probe(
> +			dev, PTR_ERR(pcie->pcie_aclk),
> +			"pcie_aclk clock source missing or invalid\n");
> +	}
> +
> +	/* Fetch reset */
> +	pcie->powerup_rst =
> +		devm_reset_control_get_optional(&pdev->dev, "pcie_powerup");
> +	if (IS_ERR_OR_NULL(pcie->powerup_rst))
> +		dev_err_probe(dev, PTR_ERR(pcie->powerup_rst),
> +			      "unable to get powerup reset\n");
> +
> +	pcie->cfg_rst = devm_reset_control_get_optional(&pdev->dev, "pcie_cfg");
> +	if (IS_ERR_OR_NULL(pcie->cfg_rst))
> +		dev_err_probe(dev, PTR_ERR(pcie->cfg_rst),
> +			      "unable to get cfg reset\n");
> +
> +	pcie->perst = devm_reset_control_get_optional(&pdev->dev, "pcie_pwren");
> +	if (IS_ERR_OR_NULL(pcie->perst))
> +		dev_err_probe(dev, PTR_ERR(pcie->perst),
> +			      "unable to get perst\n");
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +	err = pm_runtime_get_sync(dev);
> +	if (err < 0) {
> +		dev_err(dev, "pm_runtime_get_sync failed: %d\n", err);
> +		goto pm_runtime_put;
> +	}
> +
> +	return dw_pcie_host_init(&pci->pp);

Missing cleanup of runtime PM

> +
> +pm_runtime_put:
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +	return err;
> +}
> +
> +static const struct of_device_id eswin_pcie_of_match[] = {
> +	{
> +		.compatible = "eswin,eic7700-pcie",
> +	},
> +	{},
> +};
> +
> +static int eswin_pcie_suspend(struct device *dev)
> +{
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "suspend %s\n", __func__);

Drop

> +	if (!pm_runtime_status_suspended(dev))
> +		eswin_pcie_clk_disable(pcie);
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_resume(struct device *dev)
> +{
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "resume %s\n", __func__);

Drop

> +	if (!pm_runtime_status_suspended(dev))
> +		eswin_pcie_clk_enable(pcie);
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_runtime_suspend(struct device *dev)
> +{
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "runtime suspend %s\n", __func__);

Drop

> +	return eswin_pcie_clk_disable(pcie);
> +}
> +
> +static int eswin_pcie_runtime_resume(struct device *dev)
> +{
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "runtime resume %s\n", __func__);

Drop

> +	return eswin_pcie_clk_enable(pcie);
> +}
> +
> +static const struct dev_pm_ops eswin_pcie_pm_ops = {
> +	RUNTIME_PM_OPS(eswin_pcie_runtime_suspend, eswin_pcie_runtime_resume,
> +		       NULL)
> +		NOIRQ_SYSTEM_SLEEP_PM_OPS(eswin_pcie_suspend, eswin_pcie_resume)
> +};
> +
> +static struct platform_driver eswin_pcie_driver = {
> +	.driver = {
> +			.name = "eic7700-pcie",
> +			.of_match_table = eswin_pcie_of_match,
> +			.suppress_bind_attrs = true,
> +			.pm = &eswin_pcie_pm_ops,
> +	},
> +	.probe = eswin_pcie_probe,
> +	.remove = __exit_p(eswin_pcie_remove),

exit and suppress bind attrs feels wrong. Unnecessary and odd, although
maybe this is something common in PCI?

> +	.shutdown = eswin_pcie_shutdown,
> +};
> +
> +module_platform_driver(eswin_pcie_driver);
> +
> +MODULE_DEVICE_TABLE(of, eswin_pcie_of_match);
> +MODULE_DESCRIPTION("PCIe host controller driver for eic7700 SoCs");
> +MODULE_AUTHOR("Yu Ning <ningyu@eswincomputing.com>");
> +MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
> 


Best regards,
Krzysztof

Re: [PATCH v1 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus linus/master v6.15-rc6 next-20250516]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/zhangsenchuan-eswincomputing-com/dt-bindings-PCI-eic7700-Add-Eswin-eic7700-PCIe-host-controller/20250516-174445
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250516094315.179-1-zhangsenchuan%40eswincomputing.com
patch subject: [PATCH v1 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20250517/202505170506.s78iuxFL-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250517/202505170506.s78iuxFL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505170506.s78iuxFL-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/printk.h:6,
                    from include/linux/kernel.h:31,
                    from include/linux/clk.h:13,
                    from drivers/pci/controller/dwc/pcie-eic7700.c:11:
>> drivers/pci/controller/dwc/pcie-eic7700.c:427:28: error: initialization of 'void (*)(struct platform_device *)' from incompatible pointer type 'int (*)(struct platform_device *)' [-Wincompatible-pointer-types]
     427 |         .remove = __exit_p(eswin_pcie_remove),
         |                            ^~~~~~~~~~~~~~~~~
   include/linux/init.h:395:21: note: in definition of macro '__exit_p'
     395 | #define __exit_p(x) x
         |                     ^
   drivers/pci/controller/dwc/pcie-eic7700.c:427:28: note: (near initialization for 'eswin_pcie_driver.remove')
     427 |         .remove = __exit_p(eswin_pcie_remove),
         |                            ^~~~~~~~~~~~~~~~~
   include/linux/init.h:395:21: note: in definition of macro '__exit_p'
     395 | #define __exit_p(x) x
         |                     ^


vim +427 drivers/pci/controller/dwc/pcie-eic7700.c

   418	
   419	static struct platform_driver eswin_pcie_driver = {
   420		.driver = {
   421				.name = "eic7700-pcie",
   422				.of_match_table = eswin_pcie_of_match,
   423				.suppress_bind_attrs = true,
   424				.pm = &eswin_pcie_pm_ops,
   425		},
   426		.probe = eswin_pcie_probe,
 > 427		.remove = __exit_p(eswin_pcie_remove),
   428		.shutdown = eswin_pcie_shutdown,
   429	};
   430	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki