[PATCH 0/2] PCI: dwc: Add multi-port controller support

[PATCH 0/2] PCI: dwc: Add multi-port controller support

[Sumit Kumar]

This series adds support for multi-port PCIe controllers in the DesignWare
driver. Currently, the driver only supports a single Root Port with
controller-level properties, which doesn't work for multi-port controllers
where each port may have different configurations.

This series introduces a per-port structure and parsing API that allows 
each Root Port to be configured independently via pcie@N child nodes in
device tree, while maintaining backward compatibility with existing 
single-port bindings.

Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
---
Sumit Kumar (2):
      PCI: API changes for multi-port controller support
      PCI: dwc: Add multi-port controller support

 drivers/pci/controller/dwc/pci-exynos.c           |   4 +-
 drivers/pci/controller/dwc/pci-imx6.c             |  15 +-
 drivers/pci/controller/dwc/pci-meson.c            |   1 -
 drivers/pci/controller/dwc/pcie-designware-host.c | 168 ++++++++++++++++++----
 drivers/pci/controller/dwc/pcie-designware.c      |  32 ++---
 drivers/pci/controller/dwc/pcie-designware.h      |  17 ++-
 drivers/pci/controller/dwc/pcie-fu740.c           |   6 +-
 drivers/pci/controller/dwc/pcie-intel-gw.c        |  13 +-
 drivers/pci/controller/dwc/pcie-qcom-common.c     |   5 +-
 drivers/pci/controller/dwc/pcie-qcom-ep.c         |   4 +-
 drivers/pci/controller/dwc/pcie-qcom.c            |   4 +-
 drivers/pci/controller/dwc/pcie-rcar-gen4.c       |  13 +-
 drivers/pci/controller/dwc/pcie-spear13xx.c       |   5 +-
 drivers/pci/controller/dwc/pcie-tegra194.c        |   4 +-
 drivers/pci/of.c                                  |   6 +-
 drivers/pci/pci.h                                 |   2 +
 16 files changed, 226 insertions(+), 73 deletions(-)
---
base-commit: 097a6c336d0080725c626fda118ecfec448acd0f
change-id: 20251010-dt-parser-98b50ce18fc1

Best regards,
-- 
Sumit Kumar <sumit.kumar@oss.qualcomm.com>

[PATCH 1/2] PCI: API changes for multi-port controller support

[Sumit Kumar]

For multi-port controllers, equalization preset properties should be
specified in individual root port nodes rather than the controller node,
allowing each port to have its own configuration.

Change of_pci_get_equalization_presets() to accept a device_node pointer.
This allows parsing equalization presets from any device tree node,
which is needed for multi-port PCIe controllers.

Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
---
 drivers/pci/of.c  | 6 ++++--
 drivers/pci/pci.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f119845637e163d9051437c89662762f8..d09eff40b523c920c9ca3eaa64f784765b3c5bf8 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -971,6 +971,7 @@ EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
  * of_pci_get_equalization_presets - Parses the "eq-presets-Ngts" property.
  *
  * @dev: Device containing the properties.
+ * @node: Device tree node containing the properties.
  * @presets: Pointer to store the parsed data.
  * @num_lanes: Maximum number of lanes supported.
  *
@@ -981,6 +982,7 @@ EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
  * errno otherwise.
  */
 int of_pci_get_equalization_presets(struct device *dev,
+				    struct device_node *node,
 				    struct pci_eq_presets *presets,
 				    int num_lanes)
 {
@@ -988,7 +990,7 @@ int of_pci_get_equalization_presets(struct device *dev,
 	int ret;
 
 	presets->eq_presets_8gts[0] = PCI_EQ_RESV;
-	ret = of_property_read_u16_array(dev->of_node, "eq-presets-8gts",
+	ret = of_property_read_u16_array(node, "eq-presets-8gts",
 					 presets->eq_presets_8gts, num_lanes);
 	if (ret && ret != -EINVAL) {
 		dev_err(dev, "Error reading eq-presets-8gts: %d\n", ret);
@@ -998,7 +1000,7 @@ int of_pci_get_equalization_presets(struct device *dev,
 	for (int i = 0; i < EQ_PRESET_TYPE_MAX - 1; i++) {
 		presets->eq_presets_Ngts[i][0] = PCI_EQ_RESV;
 		snprintf(name, sizeof(name), "eq-presets-%dgts", 8 << (i + 1));
-		ret = of_property_read_u8_array(dev->of_node, name,
+		ret = of_property_read_u8_array(node, name,
 						presets->eq_presets_Ngts[i],
 						num_lanes);
 		if (ret && ret != -EINVAL) {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 34f65d69662e9f61f0c489ec58de2ce17d21c0c6..72fa6db95b8a75f6e69b8019d1eb2262b6a46c13 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -965,6 +965,7 @@ void pci_release_bus_of_node(struct pci_bus *bus);
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
 bool of_pci_supply_present(struct device_node *np);
 int of_pci_get_equalization_presets(struct device *dev,
+				    struct device_node *node,
 				    struct pci_eq_presets *presets,
 				    int num_lanes);
 #else
@@ -1013,6 +1014,7 @@ static inline bool of_pci_supply_present(struct device_node *np)
 }
 
 static inline int of_pci_get_equalization_presets(struct device *dev,
+						  struct device_node *node,
 						  struct pci_eq_presets *presets,
 						  int num_lanes)
 {

-- 
2.34.1

[PATCH 2/2] PCI: dwc: Add multi-port controller support

[Sumit Kumar]

The current DesignWare PCIe RC implementation supports only the controller
(Host Bridge) node for specifying the Root Port properties in an assumption
that the underlying platform only supports a single root Port per
controller instance. This limits support for multi-port controllers where
different ports may have different lane configurations and speed limits.

Introduce a separate dw_pcie_port structure to enable multi-port support.
Each Root Port can have independent lane count, speed limit through pcie@N
child nodes in device tree. Add dw_pcie_parse_root_ports()
API to parse these child nodes.

Equalization presets and link width detection currently use common DBI
space for all the root ports. Per-port DBI space assignment for these
features will be added in future.

Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pci-exynos.c           |   4 +-
 drivers/pci/controller/dwc/pci-imx6.c             |  15 +-
 drivers/pci/controller/dwc/pci-meson.c            |   1 -
 drivers/pci/controller/dwc/pcie-designware-host.c | 168 ++++++++++++++++++----
 drivers/pci/controller/dwc/pcie-designware.c      |  32 ++---
 drivers/pci/controller/dwc/pcie-designware.h      |  17 ++-
 drivers/pci/controller/dwc/pcie-fu740.c           |   6 +-
 drivers/pci/controller/dwc/pcie-intel-gw.c        |  13 +-
 drivers/pci/controller/dwc/pcie-qcom-common.c     |   5 +-
 drivers/pci/controller/dwc/pcie-qcom-ep.c         |   4 +-
 drivers/pci/controller/dwc/pcie-qcom.c            |   4 +-
 drivers/pci/controller/dwc/pcie-rcar-gen4.c       |  13 +-
 drivers/pci/controller/dwc/pcie-spear13xx.c       |   5 +-
 drivers/pci/controller/dwc/pcie-tegra194.c        |   4 +-
 14 files changed, 220 insertions(+), 71 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index 1f0e98d07109353e7321667e98ead2695151184c..af991a14b52792d2376fd30875854c46fd65155a 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -358,6 +358,8 @@ static int exynos_pcie_resume_noirq(struct device *dev)
 	struct exynos_pcie *ep = dev_get_drvdata(dev);
 	struct dw_pcie *pci = &ep->pci;
 	struct dw_pcie_rp *pp = &pci->pp;
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(ep->supplies), ep->supplies);
@@ -368,7 +370,7 @@ static int exynos_pcie_resume_noirq(struct device *dev)
 	exynos_pcie_host_init(pp);
 	dw_pcie_setup_rc(pp);
 	exynos_pcie_start_link(pci);
-	return dw_pcie_wait_for_link(pci);
+	return dw_pcie_wait_for_link(pci, port);
 }
 
 static const struct dev_pm_ops exynos_pcie_pm_ops = {
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 80e48746bbaf6815d8b9e879be5fa0bb992d9189..107367b72afc937daf26305dea2b5ad9ce3f86ae 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -939,6 +939,8 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
 	struct device *dev = pci->dev;
 	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
 	u32 tmp;
 	int ret;
 
@@ -963,8 +965,8 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 	/* Start LTSSM. */
 	imx_pcie_ltssm_enable(dev);
 
-	if (pci->max_link_speed > 1) {
-		ret = dw_pcie_wait_for_link(pci);
+	if (port->max_link_speed > 1) {
+		ret = dw_pcie_wait_for_link(pci, port);
 		if (ret)
 			goto err_reset_phy;
 
@@ -972,7 +974,7 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
 		dw_pcie_dbi_ro_wr_en(pci);
 		tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 		tmp &= ~PCI_EXP_LNKCAP_SLS;
-		tmp |= pci->max_link_speed;
+		tmp |= port->max_link_speed;
 		dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
 
 		/*
@@ -1605,6 +1607,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie *pci;
 	struct imx_pcie *imx_pcie;
 	struct device_node *np;
+	struct dw_pcie_port *port;
 	struct device_node *node = dev->of_node;
 	int ret, domain;
 	u16 val;
@@ -1741,9 +1744,9 @@ static int imx_pcie_probe(struct platform_device *pdev)
 				 &imx_pcie->tx_swing_low))
 		imx_pcie->tx_swing_low = 127;
 
-	/* Limit link speed */
-	pci->max_link_speed = 1;
-	of_property_read_u32(node, "fsl,max-link-speed", &pci->max_link_speed);
+	port = list_first_entry(&pci->pp.ports, struct dw_pcie_port, list);
+	port->max_link_speed = 1;
+	of_property_read_u32(node, "fsl,max-link-speed", &port->max_link_speed);
 
 	imx_pcie->vpcie = devm_regulator_get_optional(&pdev->dev, "vpcie");
 	if (IS_ERR(imx_pcie->vpcie)) {
diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 787469d1b396d4c7b3e28edfe276b7b997fb8aee..45d855e72e8479432c66b5c00a9755163ecd2741 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -411,7 +411,6 @@ static int meson_pcie_probe(struct platform_device *pdev)
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
 	pci->pp.ops = &meson_pcie_host_ops;
-	pci->num_lanes = 1;
 
 	mp->phy = devm_phy_get(dev, "pcie");
 	if (IS_ERR(mp->phy)) {
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..a864b90bd51ce475a2fd7de83573cf3ddea196e7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -456,11 +456,86 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
 	return 0;
 }
 
+static int dw_pcie_parse_root_ports(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct dw_pcie_port *port, *tmp;
+	struct device *dev = pci->dev;
+	int max_link_speed;
+	u32 num_lanes;
+	int ret;
+
+	if (!of_get_available_child_count(dev->of_node)) {
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port)
+			return -ENOMEM;
+
+		port->dev_node = dev->of_node;
+		list_add_tail(&port->list, &pp->ports);
+
+		return 0;
+	}
+
+	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+		num_lanes = 0;
+		max_link_speed = 0;
+		of_property_read_u32(of_port, "num-lanes", &num_lanes);
+		max_link_speed = of_pci_get_max_link_speed(of_port);
+
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port) {
+			ret = -ENOMEM;
+			goto err_port_del;
+		}
+
+		port->dev_node = of_port;
+		port->num_lanes = num_lanes;
+		port->max_link_speed = max_link_speed;
+		list_add_tail(&port->list, &pp->ports);
+	}
+
+	return 0;
+
+err_port_del:
+	list_for_each_entry_safe(port, tmp, &pp->ports, list)
+		list_del(&port->list);
+
+	return ret;
+}
+
+static int dw_pcie_parse_legacy_binding(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct dw_pcie_port *port;
+	int max_link_speed;
+	u32 num_lanes;
+	int ret;
+
+	ret = of_property_read_u32(dev->of_node, "num-lanes", &num_lanes);
+	max_link_speed = of_pci_get_max_link_speed(dev->of_node);
+
+	if (ret && max_link_speed <= 0)
+		return -ENOENT;
+
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	port->dev_node = dev->of_node;
+	port->num_lanes = num_lanes;
+	port->max_link_speed = max_link_speed;
+	list_add_tail(&port->list, &pp->ports);
+
+	return 0;
+}
+
 int dw_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct device *dev = pci->dev;
 	struct device_node *np = dev->of_node;
+	struct dw_pcie_port *port, *tmp;
 	struct pci_host_bridge *bridge;
 	int ret;
 
@@ -472,6 +547,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 
 	pp->bridge = bridge;
 
+	INIT_LIST_HEAD(&pp->ports);
+
 	ret = dw_pcie_host_get_resources(pp);
 	if (ret)
 		return ret;
@@ -480,10 +557,25 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	bridge->ops = &dw_pcie_ops;
 	bridge->child_ops = &dw_child_pcie_ops;
 
+	/*
+	 * Try to parse legacy binding first (properties in Host Bridge node).
+	 * If not found, try parsing Root Port child nodes.
+	 */
+	ret = dw_pcie_parse_legacy_binding(pp);
+	if (ret == -ENOENT) {
+		ret = dw_pcie_parse_root_ports(pp);
+		if (ret && ret != -ENOENT) {
+			dev_err(dev, "Failed to parse Root Port: %d\n", ret);
+			return ret;
+		}
+	} else if (ret) {
+		return ret;
+	}
+
 	if (pp->ops->init) {
 		ret = pp->ops->init(pp);
 		if (ret)
-			return ret;
+			goto err_cleanup_ports;
 	}
 
 	if (pci_msi_enabled()) {
@@ -518,12 +610,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 
 	dw_pcie_iatu_detect(pci);
 
-	if (pci->num_lanes < 1)
-		pci->num_lanes = dw_pcie_link_get_max_link_width(pci);
+	list_for_each_entry(port, &pp->ports, list) {
+		if (port->num_lanes < 1)
+			port->num_lanes = dw_pcie_link_get_max_link_width(pci, port);
 
-	ret = of_pci_get_equalization_presets(dev, &pp->presets, pci->num_lanes);
-	if (ret)
-		goto err_free_msi;
+		ret = of_pci_get_equalization_presets(dev, port->dev_node,
+						      &port->presets, port->num_lanes);
+		if (ret)
+			goto err_free_msi;
+	}
 
 	/*
 	 * Allocate the resource for MSG TLP before programming the iATU
@@ -557,8 +652,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	 * because that would require users to manually rescan for devices.
 	 */
 	if (!pp->use_linkup_irq)
-		/* Ignore errors, the link may come up later */
-		dw_pcie_wait_for_link(pci);
+		list_for_each_entry(port, &pp->ports, list)
+			dw_pcie_wait_for_link(pci, port);
 
 	bridge->sysdata = pp;
 
@@ -586,6 +681,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 err_deinit_host:
 	if (pp->ops->deinit)
 		pp->ops->deinit(pp);
+err_cleanup_ports:
+	list_for_each_entry_safe(port, tmp, &pp->ports, list)
+		list_del(&port->list);
 
 	return ret;
 }
@@ -594,6 +692,7 @@ EXPORT_SYMBOL_GPL(dw_pcie_host_init);
 void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct dw_pcie_port *port, *tmp;
 
 	dwc_pcie_debugfs_deinit(pci);
 
@@ -607,6 +706,9 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
 	if (pp->has_msi_ctrl)
 		dw_pcie_free_msi(pp);
 
+	list_for_each_entry_safe(port, tmp, &pp->ports, list)
+		list_del(&port->list);
+
 	if (pp->ops->deinit)
 		pp->ops->deinit(pp);
 }
@@ -830,7 +932,9 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 	return 0;
 }
 
-static void dw_pcie_program_presets(struct dw_pcie_rp *pp, enum pci_bus_speed speed)
+/*TODO: Handling preset values according to dbi space of each port */
+static void dw_pcie_program_presets(struct dw_pcie_rp *pp, struct dw_pcie_port *port,
+				    enum pci_bus_speed speed)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	u8 lane_eq_offset, lane_reg_size, cap_id;
@@ -839,23 +943,23 @@ static void dw_pcie_program_presets(struct dw_pcie_rp *pp, enum pci_bus_speed sp
 	int i;
 
 	if (speed == PCIE_SPEED_8_0GT) {
-		presets = (u8 *)pp->presets.eq_presets_8gts;
+		presets = (u8 *)port->presets.eq_presets_8gts;
 		lane_eq_offset =  PCI_SECPCI_LE_CTRL;
 		cap_id = PCI_EXT_CAP_ID_SECPCI;
 		/* For data rate of 8 GT/S each lane equalization control is 16bits wide*/
 		lane_reg_size = 0x2;
 	} else if (speed == PCIE_SPEED_16_0GT) {
-		presets = pp->presets.eq_presets_Ngts[EQ_PRESET_TYPE_16GTS - 1];
+		presets = port->presets.eq_presets_Ngts[EQ_PRESET_TYPE_16GTS - 1];
 		lane_eq_offset = PCI_PL_16GT_LE_CTRL;
 		cap_id = PCI_EXT_CAP_ID_PL_16GT;
 		lane_reg_size = 0x1;
 	} else if (speed == PCIE_SPEED_32_0GT) {
-		presets =  pp->presets.eq_presets_Ngts[EQ_PRESET_TYPE_32GTS - 1];
+		presets =  port->presets.eq_presets_Ngts[EQ_PRESET_TYPE_32GTS - 1];
 		lane_eq_offset = PCI_PL_32GT_LE_CTRL;
 		cap_id = PCI_EXT_CAP_ID_PL_32GT;
 		lane_reg_size = 0x1;
 	} else if (speed == PCIE_SPEED_64_0GT) {
-		presets =  pp->presets.eq_presets_Ngts[EQ_PRESET_TYPE_64GTS - 1];
+		presets =  port->presets.eq_presets_Ngts[EQ_PRESET_TYPE_64GTS - 1];
 		lane_eq_offset = PCI_PL_64GT_LE_CTRL;
 		cap_id = PCI_EXT_CAP_ID_PL_64GT;
 		lane_reg_size = 0x1;
@@ -874,31 +978,38 @@ static void dw_pcie_program_presets(struct dw_pcie_rp *pp, enum pci_bus_speed sp
 	 * Write preset values to the registers byte-by-byte for the given
 	 * number of lanes and register size.
 	 */
-	for (i = 0; i < pci->num_lanes * lane_reg_size; i++)
+	for (i = 0; i < port->num_lanes * lane_reg_size; i++)
 		dw_pcie_writeb_dbi(pci, cap + lane_eq_offset + i, presets[i]);
 }
 
 static void dw_pcie_config_presets(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-	enum pci_bus_speed speed = pcie_link_speed[pci->max_link_speed];
+	enum pci_bus_speed port_speed;
+	struct dw_pcie_port *port;
 
 	/*
-	 * Lane equalization settings need to be applied for all data rates the
-	 * controller supports and for all supported lanes.
+	 * Lane equalization settings need to be applied for all data rates each
+	 * port supports and for all supported lanes per port.
 	 */
+	list_for_each_entry(port, &pp->ports, list) {
+		if (port->max_link_speed > 0)
+			port_speed = pcie_link_speed[port->max_link_speed];
+		else
+			port_speed = PCIE_SPEED_2_5GT;
 
-	if (speed >= PCIE_SPEED_8_0GT)
-		dw_pcie_program_presets(pp, PCIE_SPEED_8_0GT);
+		if (port_speed >= PCIE_SPEED_8_0GT)
+			dw_pcie_program_presets(pp, port, PCIE_SPEED_8_0GT);
 
-	if (speed >= PCIE_SPEED_16_0GT)
-		dw_pcie_program_presets(pp, PCIE_SPEED_16_0GT);
+		if (port_speed >= PCIE_SPEED_16_0GT)
+			dw_pcie_program_presets(pp, port, PCIE_SPEED_16_0GT);
 
-	if (speed >= PCIE_SPEED_32_0GT)
-		dw_pcie_program_presets(pp, PCIE_SPEED_32_0GT);
+		if (port_speed >= PCIE_SPEED_32_0GT)
+			dw_pcie_program_presets(pp, port, PCIE_SPEED_32_0GT);
 
-	if (speed >= PCIE_SPEED_64_0GT)
-		dw_pcie_program_presets(pp, PCIE_SPEED_64_0GT);
+		if (port_speed >= PCIE_SPEED_64_0GT)
+			dw_pcie_program_presets(pp, port, PCIE_SPEED_64_0GT);
+	}
 }
 
 int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
@@ -1054,6 +1165,7 @@ EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq);
 
 int dw_pcie_resume_noirq(struct dw_pcie *pci)
 {
+	struct dw_pcie_port *port;
 	int ret;
 
 	if (!pci->suspended)
@@ -1075,9 +1187,9 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
 	if (ret)
 		return ret;
 
-	ret = dw_pcie_wait_for_link(pci);
-	if (ret)
-		return ret;
+	list_for_each_entry(port, &pci->pp.ports, list)
+		if (dw_pcie_wait_for_link(pci, port))
+			return -ETIMEDOUT;
 
 	return ret;
 }
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89aad5a08928cc29870ab258d33bee9ff8f83143..804f633874a7a33c7788e90ea45b529a5622060c 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -178,11 +178,6 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
 			return ret;
 	}
 
-	if (pci->max_link_speed < 1)
-		pci->max_link_speed = of_pci_get_max_link_speed(np);
-
-	of_property_read_u32(np, "num-lanes", &pci->num_lanes);
-
 	if (of_property_read_bool(np, "snps,enable-cdm-check"))
 		dw_pcie_cap_set(pci, CDM_CHECK);
 
@@ -696,7 +691,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
 	dw_pcie_writel_atu(pci, dir, index, PCIE_ATU_REGION_CTRL2, 0);
 }
 
-int dw_pcie_wait_for_link(struct dw_pcie *pci)
+int dw_pcie_wait_for_link(struct dw_pcie *pci, struct dw_pcie_port *port)
 {
 	u32 offset, val;
 	int retries;
@@ -719,7 +714,7 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
 	 * speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms
 	 * after Link training completes before sending a Configuration Request.
 	 */
-	if (pci->max_link_speed > 2)
+	if (port && port->max_link_speed > 2)
 		msleep(PCIE_RESET_CONFIG_WAIT_MS);
 
 	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -756,10 +751,11 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
 
-static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
+static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, struct dw_pcie_port *port)
 {
 	u32 cap, ctrl2, link_speed;
 	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	int max_speed = port->max_link_speed;
 
 	cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 
@@ -768,15 +764,16 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
 	 * just cache the hardware default value so that the vendor drivers can
 	 * use it to do any link specific configuration.
 	 */
-	if (pci->max_link_speed < 1) {
-		pci->max_link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
+	if (max_speed < 1) {
+		max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
+		port->max_link_speed = max_speed;
 		return;
 	}
 
 	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
 	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
 
-	switch (pcie_link_speed[pci->max_link_speed]) {
+	switch (pcie_link_speed[max_speed]) {
 	case PCIE_SPEED_2_5GT:
 		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
 		break;
@@ -800,10 +797,10 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
 
 	cap &= ~((u32)PCI_EXP_LNKCAP_SLS);
 	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | link_speed);
-
 }
 
-int dw_pcie_link_get_max_link_width(struct dw_pcie *pci)
+/* TODO: Implement per-port max link width detection using port-specific DBI space */
+int dw_pcie_link_get_max_link_width(struct dw_pcie *pci, struct dw_pcie_port *port)
 {
 	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	u32 lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
@@ -1141,9 +1138,14 @@ void dw_pcie_edma_remove(struct dw_pcie *pci)
 
 void dw_pcie_setup(struct dw_pcie *pci)
 {
+	struct dw_pcie_port *port;
 	u32 val;
 
-	dw_pcie_link_set_max_speed(pci);
+	/* Configure per-port settings */
+	list_for_each_entry(port, &pci->pp.ports, list) {
+		dw_pcie_link_set_max_speed(pci, port);
+		dw_pcie_link_set_max_link_width(pci, port->num_lanes);
+	}
 
 	/* Configure Gen1 N_FTS */
 	if (pci->n_fts[0]) {
@@ -1173,8 +1175,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
 	val &= ~PORT_LINK_FAST_LINK_MODE;
 	val |= PORT_LINK_DLL_LINK_EN;
 	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
-
-	dw_pcie_link_set_max_link_width(pci, pci->num_lanes);
 }
 
 resource_size_t dw_pcie_parent_bus_offset(struct dw_pcie *pci,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 00f52d472dcdd794013a865ad6c4c7cc251edb48..5ead9a40f0e91fb33d65d77cfa3cb61b19dceea7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -18,6 +18,7 @@
 #include <linux/dma/edma.h>
 #include <linux/gpio/consumer.h>
 #include <linux/irq.h>
+#include <linux/list.h>
 #include <linux/msi.h>
 #include <linux/pci.h>
 #include <linux/reset.h>
@@ -392,6 +393,14 @@ struct dw_pcie_ob_atu_cfg {
 	u64 size;
 };
 
+struct dw_pcie_port {
+	struct list_head	list;
+	struct device_node	*dev_node;
+	u32			num_lanes;
+	int			max_link_speed;
+	struct pci_eq_presets	presets;
+};
+
 struct dw_pcie_host_ops {
 	int (*init)(struct dw_pcie_rp *pp);
 	void (*deinit)(struct dw_pcie_rp *pp);
@@ -424,7 +433,7 @@ struct dw_pcie_rp {
 	int			msg_atu_index;
 	struct resource		*msg_res;
 	bool			use_linkup_irq;
-	struct pci_eq_presets	presets;
+	struct list_head	ports;		/* List of dw_pcie_port structures */
 };
 
 struct dw_pcie_ep_ops {
@@ -505,8 +514,6 @@ struct dw_pcie {
 	u32			version;
 	u32			type;
 	unsigned long		caps;
-	int			num_lanes;
-	int			max_link_speed;
 	u8			n_fts[2];
 	struct dw_edma_chip	edma;
 	struct clk_bulk_data	app_clks[DW_PCIE_NUM_APP_CLKS];
@@ -556,8 +563,8 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 bool dw_pcie_link_up(struct dw_pcie *pci);
 void dw_pcie_upconfig_setup(struct dw_pcie *pci);
-int dw_pcie_wait_for_link(struct dw_pcie *pci);
-int dw_pcie_link_get_max_link_width(struct dw_pcie *pci);
+int dw_pcie_wait_for_link(struct dw_pcie *pci, struct dw_pcie_port *port);
+int dw_pcie_link_get_max_link_width(struct dw_pcie *pci, struct dw_pcie_port *port);
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
 			      const struct dw_pcie_ob_atu_cfg *atu);
 int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
index 66367252032b84fd42c719287057471b1fe44e9a..39d9d2c8e1806dd805b669371ae4abc194469e57 100644
--- a/drivers/pci/controller/dwc/pcie-fu740.c
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -179,6 +179,8 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
 {
 	struct device *dev = pci->dev;
 	struct fu740_pcie *afp = dev_get_drvdata(dev);
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
 	u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	int ret;
 	u32 orig, tmp;
@@ -202,7 +204,7 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
 	/* Enable LTSSM */
 	writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
 
-	ret = dw_pcie_wait_for_link(pci);
+	ret = dw_pcie_wait_for_link(pci, port);
 	if (ret) {
 		dev_err(dev, "error: link did not start\n");
 		goto err;
@@ -220,7 +222,7 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
 		tmp |= PORT_LOGIC_SPEED_CHANGE;
 		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
 
-		ret = dw_pcie_wait_for_link(pci);
+		ret = dw_pcie_wait_for_link(pci, port);
 		if (ret) {
 			dev_err(dev, "error: link did not start at new speed\n");
 			goto err;
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index c21906eced61896c8a8307dbd6b72d229f9a5c5f..565aaa3da635749c64693268a8e5c1a59e1f040e 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -131,7 +131,10 @@ static void intel_pcie_link_setup(struct intel_pcie *pcie)
 
 static void intel_pcie_init_n_fts(struct dw_pcie *pci)
 {
-	switch (pci->max_link_speed) {
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+							struct dw_pcie_port, list);
+
+	switch (port->max_link_speed) {
 	case 3:
 		pci->n_fts[1] = PORT_AFR_N_FTS_GEN3;
 		break;
@@ -250,8 +253,10 @@ static int intel_pcie_wait_l2(struct intel_pcie *pcie)
 	u32 value;
 	int ret;
 	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						      struct dw_pcie_port, list);
 
-	if (pci->max_link_speed < 3)
+	if (port->max_link_speed < 3)
 		return 0;
 
 	/* Send PME_TURN_OFF message */
@@ -282,6 +287,8 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 {
 	int ret;
 	struct dw_pcie *pci = &pcie->pci;
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
 
 	intel_pcie_core_rst_assert(pcie);
 	intel_pcie_device_rst_assert(pcie);
@@ -313,7 +320,7 @@ static int intel_pcie_host_setup(struct intel_pcie *pcie)
 	intel_pcie_device_rst_deassert(pcie);
 	intel_pcie_ltssm_enable(pcie);
 
-	ret = dw_pcie_wait_for_link(pci);
+	ret = dw_pcie_wait_for_link(pci, port);
 	if (ret)
 		goto app_init_err;
 
diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.c b/drivers/pci/controller/dwc/pcie-qcom-common.c
index 3aad19b56da8f6ea6525f1aad0102a4d8df0369b..2d0295bd0b377f158017f7dba7a64b88e1dd8f68 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-common.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-common.c
@@ -49,6 +49,8 @@ EXPORT_SYMBOL_GPL(qcom_pcie_common_set_16gt_equalization);
 void qcom_pcie_common_set_16gt_lane_margining(struct dw_pcie *pci)
 {
 	u32 reg;
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+					struct dw_pcie_port, list);
 
 	reg = dw_pcie_readl_dbi(pci, GEN4_LANE_MARGINING_1_OFF);
 	reg &= ~(MARGINING_MAX_VOLTAGE_OFFSET |
@@ -70,7 +72,8 @@ void qcom_pcie_common_set_16gt_lane_margining(struct dw_pcie *pci)
 		MARGINING_MAXLANES |
 		MARGINING_SAMPLE_RATE_TIMING |
 		MARGINING_SAMPLE_RATE_VOLTAGE);
-	reg |= FIELD_PREP(MARGINING_MAXLANES, pci->num_lanes) |
+
+	reg |= FIELD_PREP(MARGINING_MAXLANES, port->num_lanes) |
 		FIELD_PREP(MARGINING_SAMPLE_RATE_TIMING, 0x3f) |
 		FIELD_PREP(MARGINING_SAMPLE_RATE_VOLTAGE, 0x3f);
 	dw_pcie_writel_dbi(pci, GEN4_LANE_MARGINING_2_OFF, reg);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index bf7c6ac0f3e3962de8346ab6e75dfff1d9642aad..6812ab020b3bdb99733dc12a0615ea98a471da63 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -389,6 +389,8 @@ static void qcom_pcie_disable_resources(struct qcom_pcie_ep *pcie_ep)
 
 static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
 {
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+					struct dw_pcie_port, list);
 	struct qcom_pcie_ep *pcie_ep = to_pcie_ep(pci);
 	struct device *dev = pci->dev;
 	u32 val, offset;
@@ -511,7 +513,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
 		goto err_disable_resources;
 	}
 
-	if (pcie_link_speed[pci->max_link_speed] == PCIE_SPEED_16_0GT) {
+	if (pcie_link_speed[port->max_link_speed] == PCIE_SPEED_16_0GT) {
 		qcom_pcie_common_set_16gt_equalization(pci);
 		qcom_pcie_common_set_16gt_lane_margining(pci);
 	}
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..9690ab97cc01160a26034bf8026fe4dbafe48731 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -321,8 +321,10 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
 static int qcom_pcie_start_link(struct dw_pcie *pci)
 {
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						      struct dw_pcie_port, list);
 
-	if (pcie_link_speed[pci->max_link_speed] == PCIE_SPEED_16_0GT) {
+	if (pcie_link_speed[port->max_link_speed] == PCIE_SPEED_16_0GT) {
 		qcom_pcie_common_set_16gt_equalization(pci);
 		qcom_pcie_common_set_16gt_lane_margining(pci);
 	}
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 18055807a4f5f9f1233097e1e47604a3555dccf0..f8c85984025a329b092c788dae270911aec9c51f 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -132,6 +132,8 @@ static int rcar_gen4_pcie_speed_change(struct dw_pcie *dw)
 static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
 {
 	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	struct dw_pcie_port *port = list_first_entry(&dw->pp.ports,
+						struct dw_pcie_port, list);
 	int i, changes, ret;
 
 	if (rcar->drvdata->ltssm_control) {
@@ -144,7 +146,7 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
 	 * Require direct speed change with retrying here if the max_link_speed
 	 * is PCIe Gen2 or higher.
 	 */
-	changes = min_not_zero(dw->max_link_speed, RCAR_MAX_LINK_SPEED) - 1;
+	changes = min_not_zero(port->max_link_speed, RCAR_MAX_LINK_SPEED) - 1;
 
 	/*
 	 * Since dw_pcie_setup_rc() sets it once, PCIe Gen2 will be trained.
@@ -173,6 +175,8 @@ static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
 static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
 {
 	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_port *port = list_first_entry(&dw->pp.ports,
+						struct dw_pcie_port, list);
 	u32 val;
 	int ret;
 
@@ -195,7 +199,7 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar)
 		goto err_unprepare;
 	}
 
-	if (dw->num_lanes < 4)
+	if (port->num_lanes < 4)
 		val |= BIFUR_MOD_SET_ON;
 
 	writel(val, rcar->base + PCIEMSR0);
@@ -563,12 +567,15 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
 static void rcar_gen4_pcie_additional_common_init(struct rcar_gen4_pcie *rcar)
 {
 	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_port *port = list_first_entry(&dw->pp.ports,
+						struct dw_pcie_port, list);
 	u32 val;
 
 	val = dw_pcie_readl_dbi(dw, PCIE_PORT_LANE_SKEW);
 	val &= ~PORT_LANE_SKEW_INSERT_MASK;
-	if (dw->num_lanes < 4)
+	if (port->num_lanes < 4)
 		val |= BIT(6);
+
 	dw_pcie_writel_dbi(dw, PCIE_PORT_LANE_SKEW, val);
 
 	val = readl(rcar->base + PCIEPWRMNGCTRL);
diff --git a/drivers/pci/controller/dwc/pcie-spear13xx.c b/drivers/pci/controller/dwc/pcie-spear13xx.c
index 01794a9d3ad293419c9322ca53fe86036689b287..9221b37cc58ec75986bfc2898ab2879eee794cdc 100644
--- a/drivers/pci/controller/dwc/pcie-spear13xx.c
+++ b/drivers/pci/controller/dwc/pcie-spear13xx.c
@@ -191,6 +191,7 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie *pci;
 	struct spear13xx_pcie *spear13xx_pcie;
 	struct device_node *np = dev->of_node;
+	struct dw_pcie_port *port;
 	int ret;
 
 	spear13xx_pcie = devm_kzalloc(dev, sizeof(*spear13xx_pcie), GFP_KERNEL);
@@ -228,9 +229,9 @@ static int spear13xx_pcie_probe(struct platform_device *pdev)
 		dev_err(dev, "couldn't enable clk for pcie\n");
 		return ret;
 	}
-
+	port = list_first_entry(&pci->pp.ports, struct dw_pcie_port, list);
 	if (of_property_read_bool(np, "st,pcie-is-gen1"))
-		pci->max_link_speed = 1;
+		port->max_link_speed = 1;
 
 	platform_set_drvdata(pdev, spear13xx_pcie);
 
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 4f26086f25daf8e23cafbb2b460e1934734f4103..7b82c2509e966404892a29691f7c21a27279b71f 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -957,6 +957,8 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
 static int tegra_pcie_dw_start_link(struct dw_pcie *pci)
 {
 	struct tegra_pcie_dw *pcie = to_tegra_pcie(pci);
+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
 	struct dw_pcie_rp *pp = &pci->pp;
 	u32 val, offset, tmp;
 	bool retry = true;
@@ -986,7 +988,7 @@ static int tegra_pcie_dw_start_link(struct dw_pcie *pci)
 
 	msleep(100);
 
-	if (dw_pcie_wait_for_link(pci)) {
+	if (dw_pcie_wait_for_link(pci, port)) {
 		if (!retry)
 			return 0;
 		/*

-- 
2.34.1

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Niklas Cassel]

On Mon, Jan 05, 2026 at 05:57:55PM +0530, Sumit Kumar wrote:
> The current DesignWare PCIe RC implementation supports only the controller
> (Host Bridge) node for specifying the Root Port properties in an assumption
> that the underlying platform only supports a single root Port per
> controller instance. This limits support for multi-port controllers where
> different ports may have different lane configurations and speed limits.
> 
> Introduce a separate dw_pcie_port structure to enable multi-port support.
> Each Root Port can have independent lane count, speed limit through pcie@N
> child nodes in device tree. Add dw_pcie_parse_root_ports()
> API to parse these child nodes.
> 
> Equalization presets and link width detection currently use common DBI
> space for all the root ports. Per-port DBI space assignment for these
> features will be added in future.
> 
> Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>

Hello Sumit,

Is there a reason why you represent this as a list of ports rather than a
simple array?

The number of ports is known by parsing the device tree, so it should be
static, no?

At least to me, this seem similar to e.g. how a gpio_device has multiple
gpio_descriptors "struct gpio_desc *descs":
https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib.h#L68C1-L68C26

A list is usually used for something that is dynamic.
I don't think that the number of ports to a PCIe controller will be dynamic.

I can see that struct qcom_pcie in pcie-qcom.c has struct list_head ports,
but that does not necessarily mean that we need to have a list of ports in
pcie-designware-host.c. (pcie-qcom could also be modified to have an array
of ports if there is a desire for similar design pattern.)


Kind regards,
Niklas

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Manivannan Sadhasivam]

On Mon, Jan 05, 2026 at 05:19:38PM +0100, Niklas Cassel wrote:
> On Mon, Jan 05, 2026 at 05:57:55PM +0530, Sumit Kumar wrote:
> > The current DesignWare PCIe RC implementation supports only the controller
> > (Host Bridge) node for specifying the Root Port properties in an assumption
> > that the underlying platform only supports a single root Port per
> > controller instance. This limits support for multi-port controllers where
> > different ports may have different lane configurations and speed limits.
> > 
> > Introduce a separate dw_pcie_port structure to enable multi-port support.
> > Each Root Port can have independent lane count, speed limit through pcie@N
> > child nodes in device tree. Add dw_pcie_parse_root_ports()
> > API to parse these child nodes.
> > 
> > Equalization presets and link width detection currently use common DBI
> > space for all the root ports. Per-port DBI space assignment for these
> > features will be added in future.
> > 
> > Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> 
> Hello Sumit,
> 
> Is there a reason why you represent this as a list of ports rather than a
> simple array?
> 
> The number of ports is known by parsing the device tree, so it should be
> static, no?
> 
> At least to me, this seem similar to e.g. how a gpio_device has multiple
> gpio_descriptors "struct gpio_desc *descs":
> https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib.h#L68C1-L68C26
> 
> A list is usually used for something that is dynamic.
> I don't think that the number of ports to a PCIe controller will be dynamic.
> 
> I can see that struct qcom_pcie in pcie-qcom.c has struct list_head ports,
> but that does not necessarily mean that we need to have a list of ports in
> pcie-designware-host.c. (pcie-qcom could also be modified to have an array
> of ports if there is a desire for similar design pattern.)
> 

Only reason why I went with lists in pcie-qcom is flexibility. There are useful
helpers available for traversing the lists and they seem much more elegant to
use rather than manually traversing the array in C.

But to be frank, I don't really care which one is used as there is going to be
only a handful of ports at max anyway and there is not much overhead.

- Mani

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

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Niklas Cassel]

On Tue, Jan 06, 2026 at 10:49:19AM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jan 05, 2026 at 05:19:38PM +0100, Niklas Cassel wrote:
> > On Mon, Jan 05, 2026 at 05:57:55PM +0530, Sumit Kumar wrote:
> > > The current DesignWare PCIe RC implementation supports only the controller
> > > (Host Bridge) node for specifying the Root Port properties in an assumption
> > > that the underlying platform only supports a single root Port per
> > > controller instance. This limits support for multi-port controllers where
> > > different ports may have different lane configurations and speed limits.
> > > 
> > > Introduce a separate dw_pcie_port structure to enable multi-port support.
> > > Each Root Port can have independent lane count, speed limit through pcie@N
> > > child nodes in device tree. Add dw_pcie_parse_root_ports()
> > > API to parse these child nodes.
> > > 
> > > Equalization presets and link width detection currently use common DBI
> > > space for all the root ports. Per-port DBI space assignment for these
> > > features will be added in future.
> > > 
> > > Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> > 
> > Hello Sumit,
> > 
> > Is there a reason why you represent this as a list of ports rather than a
> > simple array?
> > 
> > The number of ports is known by parsing the device tree, so it should be
> > static, no?
> > 
> > At least to me, this seem similar to e.g. how a gpio_device has multiple
> > gpio_descriptors "struct gpio_desc *descs":
> > https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib.h#L68C1-L68C26
> > 
> > A list is usually used for something that is dynamic.
> > I don't think that the number of ports to a PCIe controller will be dynamic.
> > 
> > I can see that struct qcom_pcie in pcie-qcom.c has struct list_head ports,
> > but that does not necessarily mean that we need to have a list of ports in
> > pcie-designware-host.c. (pcie-qcom could also be modified to have an array
> > of ports if there is a desire for similar design pattern.)
> > 
> 
> Only reason why I went with lists in pcie-qcom is flexibility. There are useful
> helpers available for traversing the lists and they seem much more elegant to
> use rather than manually traversing the array in C.
> 
> But to be frank, I don't really care which one is used as there is going to be
> only a handful of ports at max anyway and there is not much overhead.

Personally, when I see lists, I automatically think of something that is
dynamic, so using lists for something static just looks a little bit out of
place IMHO.

Technically, the difference is speed. If we want a specific element, we
will need to traverse the list. With an array, we can access the element
directly. However, looking at the current patch, it seems like it never
looks for a specific port, it always does an operation for all ports.
So from a speed perspective, it doesn't matter, at least not for now.


One advantage I can see, instead of doing:

+	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+						struct dw_pcie_port, list);
+	return dw_pcie_wait_for_link(pci, port);

for drivers with only one port (most drivers), we could just instead do:

+	return dw_pcie_wait_for_link(pci, pci->pp.port);

To simply get the first element in the array. No need to sprinkle
list_first_entry() everywhere in all the drivers if they just have one port.


For iterating, to avoid manually traversing the array, we could do like
libata and create a simple macro, e.g. ata_qc_for_each():
https://github.com/torvalds/linux/blob/v6.19-rc4/drivers/ata/libata-eh.c#L851-L854
https://github.com/torvalds/linux/blob/v6.19-rc4/include/linux/libata.h#L1657-L1659

And at least personally, I think my brain will parse dw_pcie_port_for_each() { }
faster than it parses list_for_each_entry(port, &pcie->ports, list) { },
since it is more unique, but perhaps I am the weird one here :)


Kind regards,
Niklas

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Niklas Cassel]

On Tue, Jan 06, 2026 at 11:55:46AM +0100, Niklas Cassel wrote:
> For iterating, to avoid manually traversing the array, we could do like
> libata and create a simple macro, e.g. ata_qc_for_each():
> https://github.com/torvalds/linux/blob/v6.19-rc4/drivers/ata/libata-eh.c#L851-L854
> https://github.com/torvalds/linux/blob/v6.19-rc4/include/linux/libata.h#L1657-L1659
> 
> And at least personally, I think my brain will parse dw_pcie_port_for_each() { }
> faster than it parses list_for_each_entry(port, &pcie->ports, list) { },
> since it is more unique, but perhaps I am the weird one here :)

Or... for_each_dw_pcie_port()

We also don't need to supply an integer to the macro, we could do like e.g.
for_each_nhlt_endpoint():
https://github.com/torvalds/linux/blob/v6.19-rc4/include/acpi/nhlt.h#L63-L64

which has does "for (unsigned int __i = 0; " in the macro.


Kind regards,
Niklas

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Manivannan Sadhasivam]

On Tue, Jan 06, 2026 at 11:55:46AM +0100, Niklas Cassel wrote:
> On Tue, Jan 06, 2026 at 10:49:19AM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jan 05, 2026 at 05:19:38PM +0100, Niklas Cassel wrote:
> > > On Mon, Jan 05, 2026 at 05:57:55PM +0530, Sumit Kumar wrote:
> > > > The current DesignWare PCIe RC implementation supports only the controller
> > > > (Host Bridge) node for specifying the Root Port properties in an assumption
> > > > that the underlying platform only supports a single root Port per
> > > > controller instance. This limits support for multi-port controllers where
> > > > different ports may have different lane configurations and speed limits.
> > > > 
> > > > Introduce a separate dw_pcie_port structure to enable multi-port support.
> > > > Each Root Port can have independent lane count, speed limit through pcie@N
> > > > child nodes in device tree. Add dw_pcie_parse_root_ports()
> > > > API to parse these child nodes.
> > > > 
> > > > Equalization presets and link width detection currently use common DBI
> > > > space for all the root ports. Per-port DBI space assignment for these
> > > > features will be added in future.
> > > > 
> > > > Signed-off-by: Sumit Kumar <sumit.kumar@oss.qualcomm.com>
> > > 
> > > Hello Sumit,
> > > 
> > > Is there a reason why you represent this as a list of ports rather than a
> > > simple array?
> > > 
> > > The number of ports is known by parsing the device tree, so it should be
> > > static, no?
> > > 
> > > At least to me, this seem similar to e.g. how a gpio_device has multiple
> > > gpio_descriptors "struct gpio_desc *descs":
> > > https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib.h#L68C1-L68C26
> > > 
> > > A list is usually used for something that is dynamic.
> > > I don't think that the number of ports to a PCIe controller will be dynamic.
> > > 
> > > I can see that struct qcom_pcie in pcie-qcom.c has struct list_head ports,
> > > but that does not necessarily mean that we need to have a list of ports in
> > > pcie-designware-host.c. (pcie-qcom could also be modified to have an array
> > > of ports if there is a desire for similar design pattern.)
> > > 
> > 
> > Only reason why I went with lists in pcie-qcom is flexibility. There are useful
> > helpers available for traversing the lists and they seem much more elegant to
> > use rather than manually traversing the array in C.
> > 
> > But to be frank, I don't really care which one is used as there is going to be
> > only a handful of ports at max anyway and there is not much overhead.
> 
> Personally, when I see lists, I automatically think of something that is
> dynamic, so using lists for something static just looks a little bit out of
> place IMHO.
> 
> Technically, the difference is speed. If we want a specific element, we
> will need to traverse the list. With an array, we can access the element
> directly. However, looking at the current patch, it seems like it never
> looks for a specific port, it always does an operation for all ports.
> So from a speed perspective, it doesn't matter, at least not for now.
> 

Yes. I don't envision the driver doing element based lookup even in the future.

> 
> One advantage I can see, instead of doing:
> 
> +	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
> +						struct dw_pcie_port, list);
> +	return dw_pcie_wait_for_link(pci, port);
> 
> for drivers with only one port (most drivers), we could just instead do:
> 
> +	return dw_pcie_wait_for_link(pci, pci->pp.port);
> 
> To simply get the first element in the array. No need to sprinkle
> list_first_entry() everywhere in all the drivers if they just have one port.
> 
> 
> For iterating, to avoid manually traversing the array, we could do like
> libata and create a simple macro, e.g. ata_qc_for_each():
> https://github.com/torvalds/linux/blob/v6.19-rc4/drivers/ata/libata-eh.c#L851-L854
> https://github.com/torvalds/linux/blob/v6.19-rc4/include/linux/libata.h#L1657-L1659
> 

I specifically do not want to introduce custom helpers. That's one of my primary
motivation for using lists :)

> And at least personally, I think my brain will parse dw_pcie_port_for_each() { }
> faster than it parses list_for_each_entry(port, &pcie->ports, list) { },
> since it is more unique, but perhaps I am the weird one here :)
> 

Arrays for sure will allow us to do O(1) lookups, but considering that we will
only be traversing the ports from the start, I still prefer using lists.

- Mani

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

Re: [PATCH 2/2] PCI: dwc: Add multi-port controller support

[Niklas Cassel]

On Tue, Jan 06, 2026 at 06:41:09PM +0530, Manivannan Sadhasivam wrote:
> > One advantage I can see, instead of doing:
> > 
> > +	struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
> > +						struct dw_pcie_port, list);
> > +	return dw_pcie_wait_for_link(pci, port);
> > 
> > for drivers with only one port (most drivers), we could just instead do:
> > 
> > +	return dw_pcie_wait_for_link(pci, pci->pp.port);
> > 
> > To simply get the first element in the array. No need to sprinkle
> > list_first_entry() everywhere in all the drivers if they just have one port.
> > 
> > 
> > For iterating, to avoid manually traversing the array, we could do like
> > libata and create a simple macro, e.g. ata_qc_for_each():
> > https://github.com/torvalds/linux/blob/v6.19-rc4/drivers/ata/libata-eh.c#L851-L854
> > https://github.com/torvalds/linux/blob/v6.19-rc4/include/linux/libata.h#L1657-L1659
> > 
> 
> I specifically do not want to introduce custom helpers. That's one of my primary
> motivation for using lists :)

You are the maintainer.

IMO it would still be nice if we could avoid sprinkling:

+struct dw_pcie_port *port = list_first_entry(&pci->pp.ports,
+                                             struct dw_pcie_port, list);

all over the glue drivers somehow, especially those that will only have a
single port. Perhaps by introducing a macro?

In case you don't like the idea of introducing a macro to get the first
port...
Since most glue drivers only seem to use the port when calling
dw_pcie_wait_for_link(), perhaps we could introduce an alternate version
of dw_pcie_wait_for_link(), that can be called by glue drivers that only
have a single port.


Kind regards,
Niklas