linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
@ 2025-12-01  6:36 Niklas Cassel
  2025-12-01  6:40 ` Niklas Cassel
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-12-01  6:36 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, Niklas Cassel
  Cc: FUKAUMI Naoki, linux-pci, linux-arm-kernel, linux-rockchip,
	linux-arm-msm

The DWC glue drivers always call pci_host_probe() during probe(), which
will allocate upstream bridge resources and enumerate the bus.

For controllers without Link Up IRQ support, pci_host_probe() is called
after dw_pcie_wait_for_link(), which will also wait the time required by
the PCIe specification before performing PCI Configuration Space reads.

For controllers with Link Up IRQ support, the pci_host_probe() call (which
will perform PCI Configuration Space reads) is done without any of the
delays mandated by the PCIe specification.

For controllers with Link Up IRQ support, since the pci_host_probe() call
is done without any delay (link training might still be ongoing), it is
very unlikely that this scan will find any devices. Once the Link Up IRQ
triggers, the Link Up IRQ handler will call pci_rescan_bus().

This works fine for PCIe endpoints connected to the Root Port, since they
don't extend the bus. However, if the pci_rescan_bus() call detects a PCIe
switch, then there will be a problem when the downstream busses starts
showing up, because the PCIe controller is not hotplug capable, so we are
not allowed to extend the subordinate bus number after the initial scan,
resulting in error messages such as:

pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43
pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them
pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44
pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them
pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45
pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them
pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46
pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41])
pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them
pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46
pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41])
pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them
pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46

While we would like to set the is_hotplug_bridge flag
(quirk_hotplug_bridge()), many embedded SoCs that use the DWC controller
have synthesized the controller without hot-plug support.
Thus, the Link Up IRQ logic is only mimicking hot-plug functionality, i.e.
it is not compliant with the PCI Hot-Plug Specification, so we cannot make
use of the is_hotplug_bridge flag.

In order to let the Link Up IRQ logic handle PCIe switches that are already
powered on (PCIe switches that not powered on already need to implement a
pwrctrl driver), don't perform a pci_host_probe() call during probe()
(which disregards the delays required by the PCIe specification).

Instead let the first Link Up IRQ call pci_host_probe(). Any follow up
Link Up IRQ will call pci_rescan_bus().

The IRQ name in /proc/interrupts for the pcie-qcom driver is renamed in
order to not dereference pp->bridge->bus before it has been assigned.

Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
Fixes: 0e0b45ab5d77 ("PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ")
Reported-by: FUKAUMI Naoki <naoki@radxa.com>
Closes: https://lore.kernel.org/linux-pci/1E8E4DB773970CB5+5a52c9e1-01b8-4872-99b7-021099f04031@radxa.com/
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Changes since v1:
-Rename the IRQ on pcie-qcom to not depend on pp->bridge->bus (Mani)
-Make sure that ret is initialized in dw_pcie_host_init() error path (Dan)

 .../pci/controller/dwc/pcie-designware-host.c | 71 ++++++++++++++++---
 drivers/pci/controller/dwc/pcie-designware.h  |  5 ++
 drivers/pci/controller/dwc/pcie-dw-rockchip.c |  5 +-
 drivers/pci/controller/dwc/pcie-qcom.c        |  9 +--
 4 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda51..bed7b309f6d9e 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -565,6 +565,59 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
 	return 0;
 }
 
+static int dw_pcie_host_initial_scan(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct pci_host_bridge *bridge = pp->bridge;
+	int ret;
+
+	ret = pci_host_probe(bridge);
+	if (ret)
+		return ret;
+
+	if (pp->ops->post_init)
+		pp->ops->post_init(pp);
+
+	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
+
+	return 0;
+}
+
+void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
+{
+	if (!pp->initial_linkup_irq_done) {
+		int ret;
+
+		ret = dw_pcie_host_initial_scan(pp);
+		if (ret) {
+			struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+			struct device *dev = pci->dev;
+
+			dev_err(dev, "Initial scan from IRQ failed: %d\n", ret);
+
+			dw_pcie_stop_link(pci);
+
+			dw_pcie_edma_remove(pci);
+
+			if (pp->has_msi_ctrl)
+				dw_pcie_free_msi(pp);
+
+			if (pp->ops->deinit)
+				pp->ops->deinit(pp);
+
+			if (pp->cfg)
+				pci_ecam_free(pp->cfg);
+		} else {
+			pp->initial_linkup_irq_done = true;
+		}
+	} else {
+		/* Rescan the bus to enumerate endpoint devices */
+		pci_lock_rescan_remove();
+		pci_rescan_bus(pp->bridge->bus);
+		pci_unlock_rescan_remove();
+	}
+}
+
 int dw_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -669,18 +722,18 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	 * If there is no Link Up IRQ, we should not bypass the delay
 	 * because that would require users to manually rescan for devices.
 	 */
-	if (!pp->use_linkup_irq)
+	if (!pp->use_linkup_irq) {
 		/* Ignore errors, the link may come up later */
 		dw_pcie_wait_for_link(pci);
 
-	ret = pci_host_probe(bridge);
-	if (ret)
-		goto err_stop_link;
-
-	if (pp->ops->post_init)
-		pp->ops->post_init(pp);
-
-	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
+		/*
+		 * For platforms with Link Up IRQ, initial scan will be done
+		 * on first Link Up IRQ.
+		 */
+		ret = dw_pcie_host_initial_scan(pp);
+		if (ret)
+			goto err_stop_link;
+	}
 
 	return 0;
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e995f692a1ecd..a31bd93490dcd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -427,6 +427,7 @@ struct dw_pcie_rp {
 	int			msg_atu_index;
 	struct resource		*msg_res;
 	bool			use_linkup_irq;
+	bool			initial_linkup_irq_done;
 	struct pci_eq_presets	presets;
 	struct pci_config_window *cfg;
 	bool			ecam_enabled;
@@ -807,6 +808,7 @@ void dw_pcie_msi_init(struct dw_pcie_rp *pp);
 int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
 void dw_pcie_free_msi(struct dw_pcie_rp *pp);
 int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
+void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp);
 int dw_pcie_host_init(struct dw_pcie_rp *pp);
 void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
 int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
@@ -844,6 +846,9 @@ static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
 	return 0;
 }
 
+static inline void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
+{ }
+
 static inline int dw_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	return 0;
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3e2752c7dd096..8f2cc1ef25e3d 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -466,10 +466,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
 		if (rockchip_pcie_link_up(pci)) {
 			msleep(PCIE_RESET_CONFIG_WAIT_MS);
 			dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
-			/* Rescan the bus to enumerate endpoint devices */
-			pci_lock_rescan_remove();
-			pci_rescan_bus(pp->bridge->bus);
-			pci_unlock_rescan_remove();
+			dw_pcie_handle_link_up_irq(pp);
 		}
 	}
 
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c48a20602d7fa..04f29cd8d8881 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1617,10 +1617,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
 	if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
 		msleep(PCIE_RESET_CONFIG_WAIT_MS);
 		dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
-		/* Rescan the bus to enumerate endpoint devices */
-		pci_lock_rescan_remove();
-		pci_rescan_bus(pp->bridge->bus);
-		pci_unlock_rescan_remove();
+		dw_pcie_handle_link_up_irq(pp);
 
 		qcom_pcie_icc_opp_update(pcie);
 	} else {
@@ -1937,8 +1934,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_phy_exit;
 	}
 
-	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq%d",
-			      pci_domain_nr(pp->bridge->bus));
+	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq_%pOFP",
+			      dev->of_node);
 	if (!name) {
 		ret = -ENOMEM;
 		goto err_host_deinit;
-- 
2.52.0



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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-01  6:36 [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches Niklas Cassel
@ 2025-12-01  6:40 ` Niklas Cassel
  2025-12-08  5:45 ` Niklas Cassel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-12-01  6:40 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner
  Cc: FUKAUMI Naoki, linux-pci, linux-arm-kernel, linux-rockchip,
	linux-arm-msm

Forgot to pick up tags from v1, adding them here so that e.g. b4 will pick
them up:

Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
Tested-by: FUKAUMI Naoki <naoki@radxa.com>


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-01  6:36 [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches Niklas Cassel
  2025-12-01  6:40 ` Niklas Cassel
@ 2025-12-08  5:45 ` Niklas Cassel
  2025-12-08  6:20 ` Krishna Chaitanya Chundru
  2025-12-09  7:11 ` Ilpo Järvinen
  3 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-12-08  5:45 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner
  Cc: FUKAUMI Naoki, linux-pci, linux-arm-kernel, linux-rockchip,
	linux-arm-msm

On Mon, Dec 01, 2025 at 07:36:35AM +0100, Niklas Cassel wrote:

Mani, could you perhaps try this series on Qcom?

I personally prefer this series over a revert because:
It allows to us to still use a PCIe endpoint that comes up after the
main system has booted, e.g. when the PCIe endpoint is a board running
pci-epf-test, without the need for a manual rescan on be bus.

If we go with revert instead, this very nice feature would be gone,
and the user would need to do a manual rescan of the bus.

One could even argue that that is a user visible regression.


Kind regards,
Niklas


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-01  6:36 [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches Niklas Cassel
  2025-12-01  6:40 ` Niklas Cassel
  2025-12-08  5:45 ` Niklas Cassel
@ 2025-12-08  6:20 ` Krishna Chaitanya Chundru
  2025-12-09  5:12   ` Niklas Cassel
  2025-12-09  7:11 ` Ilpo Järvinen
  3 siblings, 1 reply; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-12-08  6:20 UTC (permalink / raw)
  To: Niklas Cassel, Jingoo Han, Manivannan Sadhasivam,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Heiko Stuebner
  Cc: FUKAUMI Naoki, linux-pci, linux-arm-kernel, linux-rockchip,
	linux-arm-msm



On 12/1/2025 12:06 PM, Niklas Cassel wrote:
> The DWC glue drivers always call pci_host_probe() during probe(), which
> will allocate upstream bridge resources and enumerate the bus.
>
> For controllers without Link Up IRQ support, pci_host_probe() is called
> after dw_pcie_wait_for_link(), which will also wait the time required by
> the PCIe specification before performing PCI Configuration Space reads.
>
> For controllers with Link Up IRQ support, the pci_host_probe() call (which
> will perform PCI Configuration Space reads) is done without any of the
> delays mandated by the PCIe specification.
>
> For controllers with Link Up IRQ support, since the pci_host_probe() call
> is done without any delay (link training might still be ongoing), it is
> very unlikely that this scan will find any devices. Once the Link Up IRQ
> triggers, the Link Up IRQ handler will call pci_rescan_bus().
>
> This works fine for PCIe endpoints connected to the Root Port, since they
> don't extend the bus. However, if the pci_rescan_bus() call detects a PCIe
> switch, then there will be a problem when the downstream busses starts
> showing up, because the PCIe controller is not hotplug capable, so we are
> not allowed to extend the subordinate bus number after the initial scan,
> resulting in error messages such as:
>
> pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43
> pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them
> pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44
> pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them
> pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45
> pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them
> pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46
> pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them
> pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46
> pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41])
> pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them
> pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
>
> While we would like to set the is_hotplug_bridge flag
> (quirk_hotplug_bridge()), many embedded SoCs that use the DWC controller
> have synthesized the controller without hot-plug support.
> Thus, the Link Up IRQ logic is only mimicking hot-plug functionality, i.e.
> it is not compliant with the PCI Hot-Plug Specification, so we cannot make
> use of the is_hotplug_bridge flag.
>
> In order to let the Link Up IRQ logic handle PCIe switches that are already
> powered on (PCIe switches that not powered on already need to implement a
> pwrctrl driver), don't perform a pci_host_probe() call during probe()
> (which disregards the delays required by the PCIe specification).
>
> Instead let the first Link Up IRQ call pci_host_probe(). Any follow up
> Link Up IRQ will call pci_rescan_bus().
>
> The IRQ name in /proc/interrupts for the pcie-qcom driver is renamed in
> order to not dereference pp->bridge->bus before it has been assigned.
>
> Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
> Fixes: 0e0b45ab5d77 ("PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ")
> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Closes: https://lore.kernel.org/linux-pci/1E8E4DB773970CB5+5a52c9e1-01b8-4872-99b7-021099f04031@radxa.com/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> Changes since v1:
> -Rename the IRQ on pcie-qcom to not depend on pp->bridge->bus (Mani)
> -Make sure that ret is initialized in dw_pcie_host_init() error path (Dan)
>
>   .../pci/controller/dwc/pcie-designware-host.c | 71 ++++++++++++++++---
>   drivers/pci/controller/dwc/pcie-designware.h  |  5 ++
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c |  5 +-
>   drivers/pci/controller/dwc/pcie-qcom.c        |  9 +--
>   4 files changed, 71 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda51..bed7b309f6d9e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -565,6 +565,59 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
>   	return 0;
>   }
>   
> +static int dw_pcie_host_initial_scan(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct pci_host_bridge *bridge = pp->bridge;
> +	int ret;
> +
> +	ret = pci_host_probe(bridge);
> +	if (ret)
> +		return ret;
> +
> +	if (pp->ops->post_init)
> +		pp->ops->post_init(pp);
> +
> +	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
> +
> +	return 0;
> +}
> +
> +void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
> +{
> +	if (!pp->initial_linkup_irq_done) {
> +		int ret;
> +
> +		ret = dw_pcie_host_initial_scan(pp);
> +		if (ret) {
> +			struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +			struct device *dev = pci->dev;
> +
> +			dev_err(dev, "Initial scan from IRQ failed: %d\n", ret);
> +
> +			dw_pcie_stop_link(pci);
> +
> +			dw_pcie_edma_remove(pci);
> +
> +			if (pp->has_msi_ctrl)
> +				dw_pcie_free_msi(pp);
> +
> +			if (pp->ops->deinit)
> +				pp->ops->deinit(pp);
> +
> +			if (pp->cfg)
> +				pci_ecam_free(pp->cfg);
> +		} else {
> +			pp->initial_linkup_irq_done = true;
> +		}
> +	} else {
> +		/* Rescan the bus to enumerate endpoint devices */
> +		pci_lock_rescan_remove();
> +		pci_rescan_bus(pp->bridge->bus);
> +		pci_unlock_rescan_remove();
> +	}
> +}
> +
>   int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   {
>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -669,18 +722,18 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	 * If there is no Link Up IRQ, we should not bypass the delay
>   	 * because that would require users to manually rescan for devices.
>   	 */
> -	if (!pp->use_linkup_irq)
> +	if (!pp->use_linkup_irq) {
>   		/* Ignore errors, the link may come up later */
>   		dw_pcie_wait_for_link(pci);
>   
> -	ret = pci_host_probe(bridge);
> -	if (ret)
> -		goto err_stop_link;
> -
> -	if (pp->ops->post_init)
> -		pp->ops->post_init(pp);
> -
> -	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
> +		/*
> +		 * For platforms with Link Up IRQ, initial scan will be done
> +		 * on first Link Up IRQ.
> +		 */
> +		ret = dw_pcie_host_initial_scan(pp);
> +		if (ret)
> +			goto err_stop_link;
This will cause breakages in qcom platforms, qcom platforms use pwrctrl 
framework for
enabling power to the endpoints. And the pwrctrl drivers will be created 
as part of the initial scan.
As we skipping the initial scan here, pwrctrl driver will be never 
created and the power to the
endpoint will be never enabled. This will cause link to be never up.

- Krishna Chaitanya.
> +	}
>   
>   	return 0;
>   
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ecd..a31bd93490dcd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -427,6 +427,7 @@ struct dw_pcie_rp {
>   	int			msg_atu_index;
>   	struct resource		*msg_res;
>   	bool			use_linkup_irq;
> +	bool			initial_linkup_irq_done;
>   	struct pci_eq_presets	presets;
>   	struct pci_config_window *cfg;
>   	bool			ecam_enabled;
> @@ -807,6 +808,7 @@ void dw_pcie_msi_init(struct dw_pcie_rp *pp);
>   int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
>   void dw_pcie_free_msi(struct dw_pcie_rp *pp);
>   int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> +void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp);
>   int dw_pcie_host_init(struct dw_pcie_rp *pp);
>   void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
>   int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
> @@ -844,6 +846,9 @@ static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>   	return 0;
>   }
>   
> +static inline void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
> +{ }
> +
>   static inline int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   {
>   	return 0;
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 3e2752c7dd096..8f2cc1ef25e3d 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -466,10 +466,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
>   		if (rockchip_pcie_link_up(pci)) {
>   			msleep(PCIE_RESET_CONFIG_WAIT_MS);
>   			dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
> -			/* Rescan the bus to enumerate endpoint devices */
> -			pci_lock_rescan_remove();
> -			pci_rescan_bus(pp->bridge->bus);
> -			pci_unlock_rescan_remove();
> +			dw_pcie_handle_link_up_irq(pp);
>   		}
>   	}
>   
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c48a20602d7fa..04f29cd8d8881 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1617,10 +1617,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>   	if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
>   		msleep(PCIE_RESET_CONFIG_WAIT_MS);
>   		dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
> -		/* Rescan the bus to enumerate endpoint devices */
> -		pci_lock_rescan_remove();
> -		pci_rescan_bus(pp->bridge->bus);
> -		pci_unlock_rescan_remove();
> +		dw_pcie_handle_link_up_irq(pp);
>   
>   		qcom_pcie_icc_opp_update(pcie);
>   	} else {
> @@ -1937,8 +1934,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>   		goto err_phy_exit;
>   	}
>   
> -	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq%d",
> -			      pci_domain_nr(pp->bridge->bus));
> +	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq_%pOFP",
> +			      dev->of_node);
>   	if (!name) {
>   		ret = -ENOMEM;
>   		goto err_host_deinit;



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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-08  6:20 ` Krishna Chaitanya Chundru
@ 2025-12-09  5:12   ` Niklas Cassel
  2025-12-09  5:27     ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 12+ messages in thread
From: Niklas Cassel @ 2025-12-09  5:12 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

Hello Krishna,

Currently:
For controllers with Link Up IRQ support, the pci_host_probe() call (which
will perform PCI Configuration Space reads) is done without any of the
delays mandated by the PCIe specification.

This seems quite bad.

A device might not be fully initialized during during the time of these
PCI Configuration Space reads, but might still return some bogus values
that are actually different from the Configuration Space reads if done
after respecting the delays mandated by the PCIe specification.

I think the options are:
1) Keep the pci_host_probe() call in dw_pcie_host_init() for controllers
   with Link Up IRQ support, but make sure that we respect the delays also
   in this case.
or
2) Remove the pci_host_probe() call from dw_pcie_host_init(), and make sure
   that pci_host_probe() is done by the first Link Up IRQ
   (i.e. what this patch does).


One big thing with using the Link Up IRQ is to not do any delay during PCIe
controller driver's probe(), which reduces startup time, exactly as your
commit message in commit 36971d6c5a9a ("PCI: qcom: Don't wait for link if
we can detect Link Up") explains.
Therefore, I don't think that 1) is a good solution, so that leaves us with
2).


If pwrctrl drivers are created as part of the pci_host_probe() call,
I think that perhaps an alternative would be to create an explict
pwrctrl_init() function, and let the PCI controller drivers that actually
use pwrctrl call that from their probe().
(And just remove the same from pci_host_probe() ?)

In fact, looking at your suggested patches (that hasn't landed yet):
[PATCH 3/5] PCI/pwrctrl: Add APIs for explicitly creating and destroying pwrctrl devices
[PATCH 5/5] PCI/pwrctrl: Switch to the new pwrctrl APIs

https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-5-78a72627683d@oss.qualcomm.com/
https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-3-78a72627683d@oss.qualcomm.com/

Seem to do exactly that:
Call pci_pwrctrl_create_devices() explicitly from the PCIe controller drivers
directly, and removes the pci_pwrctrl_create_device() call from pci_host_probe().

So I don't really understand your concern with this series, at least not if
it goes on top of your series:
https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com/


Kind regards,
Niklas


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-09  5:12   ` Niklas Cassel
@ 2025-12-09  5:27     ` Krishna Chaitanya Chundru
  2025-12-09  5:36       ` Niklas Cassel
  0 siblings, 1 reply; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-12-09  5:27 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm



On 12/9/2025 10:42 AM, Niklas Cassel wrote:
> Hello Krishna,
>
> Currently:
> For controllers with Link Up IRQ support, the pci_host_probe() call (which
> will perform PCI Configuration Space reads) is done without any of the
> delays mandated by the PCIe specification.
>
> This seems quite bad.
>
> A device might not be fully initialized during during the time of these
> PCI Configuration Space reads, but might still return some bogus values
> that are actually different from the Configuration Space reads if done
> after respecting the delays mandated by the PCIe specification.
>
> I think the options are:
> 1) Keep the pci_host_probe() call in dw_pcie_host_init() for controllers
>     with Link Up IRQ support, but make sure that we respect the delays also
>     in this case.
> or
> 2) Remove the pci_host_probe() call from dw_pcie_host_init(), and make sure
>     that pci_host_probe() is done by the first Link Up IRQ
>     (i.e. what this patch does).
>
>
> One big thing with using the Link Up IRQ is to not do any delay during PCIe
> controller driver's probe(), which reduces startup time, exactly as your
> commit message in commit 36971d6c5a9a ("PCI: qcom: Don't wait for link if
> we can detect Link Up") explains.
> Therefore, I don't think that 1) is a good solution, so that leaves us with
> 2).
>
>
> If pwrctrl drivers are created as part of the pci_host_probe() call,
> I think that perhaps an alternative would be to create an explict
> pwrctrl_init() function, and let the PCI controller drivers that actually
> use pwrctrl call that from their probe().
> (And just remove the same from pci_host_probe() ?)
>
> In fact, looking at your suggested patches (that hasn't landed yet):
> [PATCH 3/5] PCI/pwrctrl: Add APIs for explicitly creating and destroying pwrctrl devices
> [PATCH 5/5] PCI/pwrctrl: Switch to the new pwrctrl APIs
>
> https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-5-78a72627683d@oss.qualcomm.com/
> https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-3-78a72627683d@oss.qualcomm.com/
>
> Seem to do exactly that:
> Call pci_pwrctrl_create_devices() explicitly from the PCIe controller drivers
> directly, and removes the pci_pwrctrl_create_device() call from pci_host_probe().
>
> So I don't really understand your concern with this series, at least not if
> it goes on top of your series:
> https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com/
Hi Niklas,

If this series goes on top of the our series i.e pwrctrl rework series, 
I don't have any concerns.
My only concern is link up IRQ never fires if this patch goes before series.

- Krishna Chaitanya.
>
>
> Kind regards,
> Niklas



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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-09  5:27     ` Krishna Chaitanya Chundru
@ 2025-12-09  5:36       ` Niklas Cassel
  2025-12-12  3:52         ` Manivannan Sadhasivam
  0 siblings, 1 reply; 12+ messages in thread
From: Niklas Cassel @ 2025-12-09  5:36 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

On Tue, Dec 09, 2025 at 10:57:18AM +0530, Krishna Chaitanya Chundru wrote:
> On 12/9/2025 10:42 AM, Niklas Cassel wrote:
> > 
> > So I don't really understand your concern with this series, at least not if
> > it goes on top of your series:
> > https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com/
> Hi Niklas,
> 
> If this series goes on top of the our series i.e pwrctrl rework series, I
> don't have any concerns.
> My only concern is link up IRQ never fires if this patch goes before series.
> 
> - Krishna Chaitanya.

This patch missed the v6.19 merge window (and so did the pwrctrl rework
series), so as long as Mani queues up both for v6.20 (with the pwrctrl
rework series getting applied first), I think we are good.


Kind regards,
Niklas


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-01  6:36 [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches Niklas Cassel
                   ` (2 preceding siblings ...)
  2025-12-08  6:20 ` Krishna Chaitanya Chundru
@ 2025-12-09  7:11 ` Ilpo Järvinen
  2025-12-09  7:45   ` Niklas Cassel
  3 siblings, 1 reply; 12+ messages in thread
From: Ilpo Järvinen @ 2025-12-09  7:11 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

On Mon, 1 Dec 2025, Niklas Cassel wrote:

> The DWC glue drivers always call pci_host_probe() during probe(), which
> will allocate upstream bridge resources and enumerate the bus.
> 
> For controllers without Link Up IRQ support, pci_host_probe() is called
> after dw_pcie_wait_for_link(), which will also wait the time required by
> the PCIe specification before performing PCI Configuration Space reads.
> 
> For controllers with Link Up IRQ support, the pci_host_probe() call (which
> will perform PCI Configuration Space reads) is done without any of the
> delays mandated by the PCIe specification.
> 
> For controllers with Link Up IRQ support, since the pci_host_probe() call
> is done without any delay (link training might still be ongoing), it is
> very unlikely that this scan will find any devices. Once the Link Up IRQ
> triggers, the Link Up IRQ handler will call pci_rescan_bus().
> 
> This works fine for PCIe endpoints connected to the Root Port, since they
> don't extend the bus. However, if the pci_rescan_bus() call detects a PCIe
> switch, then there will be a problem when the downstream busses starts
> showing up, because the PCIe controller is not hotplug capable, so we are
> not allowed to extend the subordinate bus number after the initial scan,
> resulting in error messages such as:
> 
> pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43
> pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them
> pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44
> pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them
> pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45
> pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them
> pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46
> pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41])
> pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them
> pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46
> pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41])
> pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them
> pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
> 
> While we would like to set the is_hotplug_bridge flag
> (quirk_hotplug_bridge()), many embedded SoCs that use the DWC controller
> have synthesized the controller without hot-plug support.
> Thus, the Link Up IRQ logic is only mimicking hot-plug functionality, i.e.
> it is not compliant with the PCI Hot-Plug Specification, so we cannot make
> use of the is_hotplug_bridge flag.
> 
> In order to let the Link Up IRQ logic handle PCIe switches that are already
> powered on (PCIe switches that not powered on already need to implement a
> pwrctrl driver), don't perform a pci_host_probe() call during probe()
> (which disregards the delays required by the PCIe specification).
> 
> Instead let the first Link Up IRQ call pci_host_probe(). Any follow up
> Link Up IRQ will call pci_rescan_bus().
> 
> The IRQ name in /proc/interrupts for the pcie-qcom driver is renamed in
> order to not dereference pp->bridge->bus before it has been assigned.
> 
> Fixes: ec9fd499b9c6 ("PCI: dw-rockchip: Don't wait for link since we can detect Link Up")
> Fixes: 0e0b45ab5d77 ("PCI: dw-rockchip: Enumerate endpoints based on dll_link_up IRQ")
> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Closes: https://lore.kernel.org/linux-pci/1E8E4DB773970CB5+5a52c9e1-01b8-4872-99b7-021099f04031@radxa.com/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Hi Niklas,

Now this patch looks interesting (and managed to catch my attention 
despite being a controllers/ patch).

You're only talking about bus number allocations here but we did hit a 
similar problem with bridge window allocations where not enough 
information is available at the time of the initial scan + resource 
allocation (currently it is one of the issues that prevents fixing one 
resource overlap bug):

https://lore.kernel.org/linux-pci/8f9c9950-1612-6e2d-388a-ce69cf3aae1a@linux.intel.com/

(Please check also Mani's reply that relates to pwrctrl.)

So I definitely like the general direction this patch goes (though I'm a 
bit worried though if the downstream busses trickle in, the resource 
allocation problems will not be fully solved but maybe that fear is not 
warranted, I don't know).

-- 
 i.

> ---
> Changes since v1:
> -Rename the IRQ on pcie-qcom to not depend on pp->bridge->bus (Mani)
> -Make sure that ret is initialized in dw_pcie_host_init() error path (Dan)
> 
>  .../pci/controller/dwc/pcie-designware-host.c | 71 ++++++++++++++++---
>  drivers/pci/controller/dwc/pcie-designware.h  |  5 ++
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c |  5 +-
>  drivers/pci/controller/dwc/pcie-qcom.c        |  9 +--
>  4 files changed, 71 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda51..bed7b309f6d9e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -565,6 +565,59 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
>  	return 0;
>  }
>  
> +static int dw_pcie_host_initial_scan(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct pci_host_bridge *bridge = pp->bridge;
> +	int ret;
> +
> +	ret = pci_host_probe(bridge);
> +	if (ret)
> +		return ret;
> +
> +	if (pp->ops->post_init)
> +		pp->ops->post_init(pp);
> +
> +	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
> +
> +	return 0;
> +}
> +
> +void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
> +{
> +	if (!pp->initial_linkup_irq_done) {
> +		int ret;
> +
> +		ret = dw_pcie_host_initial_scan(pp);
> +		if (ret) {
> +			struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +			struct device *dev = pci->dev;
> +
> +			dev_err(dev, "Initial scan from IRQ failed: %d\n", ret);
> +
> +			dw_pcie_stop_link(pci);
> +
> +			dw_pcie_edma_remove(pci);
> +
> +			if (pp->has_msi_ctrl)
> +				dw_pcie_free_msi(pp);
> +
> +			if (pp->ops->deinit)
> +				pp->ops->deinit(pp);
> +
> +			if (pp->cfg)
> +				pci_ecam_free(pp->cfg);
> +		} else {
> +			pp->initial_linkup_irq_done = true;
> +		}
> +	} else {
> +		/* Rescan the bus to enumerate endpoint devices */
> +		pci_lock_rescan_remove();
> +		pci_rescan_bus(pp->bridge->bus);
> +		pci_unlock_rescan_remove();
> +	}
> +}
> +
>  int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -669,18 +722,18 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  	 * If there is no Link Up IRQ, we should not bypass the delay
>  	 * because that would require users to manually rescan for devices.
>  	 */
> -	if (!pp->use_linkup_irq)
> +	if (!pp->use_linkup_irq) {
>  		/* Ignore errors, the link may come up later */
>  		dw_pcie_wait_for_link(pci);
>  
> -	ret = pci_host_probe(bridge);
> -	if (ret)
> -		goto err_stop_link;
> -
> -	if (pp->ops->post_init)
> -		pp->ops->post_init(pp);
> -
> -	dwc_pcie_debugfs_init(pci, DW_PCIE_RC_TYPE);
> +		/*
> +		 * For platforms with Link Up IRQ, initial scan will be done
> +		 * on first Link Up IRQ.
> +		 */
> +		ret = dw_pcie_host_initial_scan(pp);
> +		if (ret)
> +			goto err_stop_link;
> +	}
>  
>  	return 0;
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ecd..a31bd93490dcd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -427,6 +427,7 @@ struct dw_pcie_rp {
>  	int			msg_atu_index;
>  	struct resource		*msg_res;
>  	bool			use_linkup_irq;
> +	bool			initial_linkup_irq_done;
>  	struct pci_eq_presets	presets;
>  	struct pci_config_window *cfg;
>  	bool			ecam_enabled;
> @@ -807,6 +808,7 @@ void dw_pcie_msi_init(struct dw_pcie_rp *pp);
>  int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
>  void dw_pcie_free_msi(struct dw_pcie_rp *pp);
>  int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> +void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp);
>  int dw_pcie_host_init(struct dw_pcie_rp *pp);
>  void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
>  int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
> @@ -844,6 +846,9 @@ static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>  	return 0;
>  }
>  
> +static inline void dw_pcie_handle_link_up_irq(struct dw_pcie_rp *pp)
> +{ }
> +
>  static inline int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	return 0;
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 3e2752c7dd096..8f2cc1ef25e3d 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -466,10 +466,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
>  		if (rockchip_pcie_link_up(pci)) {
>  			msleep(PCIE_RESET_CONFIG_WAIT_MS);
>  			dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
> -			/* Rescan the bus to enumerate endpoint devices */
> -			pci_lock_rescan_remove();
> -			pci_rescan_bus(pp->bridge->bus);
> -			pci_unlock_rescan_remove();
> +			dw_pcie_handle_link_up_irq(pp);
>  		}
>  	}
>  
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c48a20602d7fa..04f29cd8d8881 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1617,10 +1617,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>  	if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
>  		msleep(PCIE_RESET_CONFIG_WAIT_MS);
>  		dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
> -		/* Rescan the bus to enumerate endpoint devices */
> -		pci_lock_rescan_remove();
> -		pci_rescan_bus(pp->bridge->bus);
> -		pci_unlock_rescan_remove();
> +		dw_pcie_handle_link_up_irq(pp);
>  
>  		qcom_pcie_icc_opp_update(pcie);
>  	} else {
> @@ -1937,8 +1934,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  		goto err_phy_exit;
>  	}
>  
> -	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq%d",
> -			      pci_domain_nr(pp->bridge->bus));
> +	name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq_%pOFP",
> +			      dev->of_node);
>  	if (!name) {
>  		ret = -ENOMEM;
>  		goto err_host_deinit;
> 


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-09  7:11 ` Ilpo Järvinen
@ 2025-12-09  7:45   ` Niklas Cassel
  0 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-12-09  7:45 UTC (permalink / raw)
  To: Ilpo Järvinen, Val Packett
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

Hello Ilpo,

On Tue, Dec 09, 2025 at 09:11:37AM +0200, Ilpo Järvinen wrote:
> 
> Now this patch looks interesting (and managed to catch my attention 
> despite being a controllers/ patch).
> 
> You're only talking about bus number allocations here but we did hit a 
> similar problem with bridge window allocations where not enough 
> information is available at the time of the initial scan + resource 
> allocation (currently it is one of the issues that prevents fixing one 
> resource overlap bug):
> 
> https://lore.kernel.org/linux-pci/8f9c9950-1612-6e2d-388a-ce69cf3aae1a@linux.intel.com/

I'm quite sure that Val's problem will be solved by applying:
https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com/

If not, he could try applying the above series, and then this patch on top.


Kind regards,
Niklas



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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-09  5:36       ` Niklas Cassel
@ 2025-12-12  3:52         ` Manivannan Sadhasivam
  2025-12-12  6:37           ` Niklas Cassel
  0 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2025-12-12  3:52 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Krishna Chaitanya Chundru, Jingoo Han, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

On Tue, Dec 09, 2025 at 02:36:44PM +0900, Niklas Cassel wrote:
> On Tue, Dec 09, 2025 at 10:57:18AM +0530, Krishna Chaitanya Chundru wrote:
> > On 12/9/2025 10:42 AM, Niklas Cassel wrote:
> > > 
> > > So I don't really understand your concern with this series, at least not if
> > > it goes on top of your series:
> > > https://lore.kernel.org/linux-pci/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com/
> > Hi Niklas,
> > 
> > If this series goes on top of the our series i.e pwrctrl rework series, I
> > don't have any concerns.
> > My only concern is link up IRQ never fires if this patch goes before series.
> > 
> > - Krishna Chaitanya.
> 
> This patch missed the v6.19 merge window (and so did the pwrctrl rework
> series), so as long as Mani queues up both for v6.20 (with the pwrctrl
> rework series getting applied first), I think we are good.
> 

The plan is to merge pwrctrl series to v6.20 (unless we get strong objections),
but once that happens, Qcom doesn't need this patch.

So it'd be good if you can just limit this patch to just Rockchip. Then once the
Rockchip also moves to pwrctrl, we can revert this patch (also the whole IRQ
based link up since the Root Port is not hotplug capable).

- Mani

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


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-12  3:52         ` Manivannan Sadhasivam
@ 2025-12-12  6:37           ` Niklas Cassel
  2025-12-22  6:15             ` Manivannan Sadhasivam
  0 siblings, 1 reply; 12+ messages in thread
From: Niklas Cassel @ 2025-12-12  6:37 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Krishna Chaitanya Chundru, Jingoo Han, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

Hello Mani,

On Fri, Dec 12, 2025 at 12:52:35PM +0900, Manivannan Sadhasivam wrote:
> > This patch missed the v6.19 merge window (and so did the pwrctrl rework
> > series), so as long as Mani queues up both for v6.20 (with the pwrctrl
> > rework series getting applied first), I think we are good.
> > 
> 
> The plan is to merge pwrctrl series to v6.20 (unless we get strong objections),
> but once that happens, Qcom doesn't need this patch.
> 
> So it'd be good if you can just limit this patch to just Rockchip. Then once the
> Rockchip also moves to pwrctrl, we can revert this patch (also the whole IRQ
> based link up since the Root Port is not hotplug capable).

The main reason why I did not like a revert, was because it would remove
the nice feature where the bus is enumerated automatically on link up.

I assumed that the plan was to add Link Up IRQ support in pwrctrl in the
future.

If that is not the case, and the plan is instead to eventually remove the
existing Link Up IRQ support, then perhaps you could simply apply patches
from:
https://lore.kernel.org/linux-pci/20251111105100.869997-8-cassel@kernel.org/

Either the whole series or just patch 1 and 2, maintainer's choice :)


Kind regards,
Niklas


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

* Re: [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches
  2025-12-12  6:37           ` Niklas Cassel
@ 2025-12-22  6:15             ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2025-12-22  6:15 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Krishna Chaitanya Chundru, Jingoo Han, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner, FUKAUMI Naoki, linux-pci, linux-arm-kernel,
	linux-rockchip, linux-arm-msm

On Fri, Dec 12, 2025 at 03:37:26PM +0900, Niklas Cassel wrote:
> Hello Mani,
> 
> On Fri, Dec 12, 2025 at 12:52:35PM +0900, Manivannan Sadhasivam wrote:
> > > This patch missed the v6.19 merge window (and so did the pwrctrl rework
> > > series), so as long as Mani queues up both for v6.20 (with the pwrctrl
> > > rework series getting applied first), I think we are good.
> > > 
> > 
> > The plan is to merge pwrctrl series to v6.20 (unless we get strong objections),
> > but once that happens, Qcom doesn't need this patch.
> > 
> > So it'd be good if you can just limit this patch to just Rockchip. Then once the
> > Rockchip also moves to pwrctrl, we can revert this patch (also the whole IRQ
> > based link up since the Root Port is not hotplug capable).
> 
> The main reason why I did not like a revert, was because it would remove
> the nice feature where the bus is enumerated automatically on link up.
> 
> I assumed that the plan was to add Link Up IRQ support in pwrctrl in the
> future.
> 

As I dig more and more into pwrctrl, it becomes clear to me that emulating
hotplug on non-hotplug capable hw is a bad idea. Though it gives a nice user
experience in majority of the cases, it also gives issue in some cases like
switch. So pwrctrl will just power on all devices/slots before initial bus scan
and will not do any link up based enumeration later. And neither the controller
driver.

> If that is not the case, and the plan is instead to eventually remove the
> existing Link Up IRQ support, then perhaps you could simply apply patches
> from:
> https://lore.kernel.org/linux-pci/20251111105100.869997-8-cassel@kernel.org/
> 
> Either the whole series or just patch 1 and 2, maintainer's choice :)
> 

I tried applying the whole series, but it fails at patch 2. Could you please
rebase against, pci/controller/dwc and repost?

- Mani

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


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

end of thread, other threads:[~2025-12-22  6:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01  6:36 [PATCH v2] PCI: dwc: Make Link Up IRQ logic handle already powered on PCIe switches Niklas Cassel
2025-12-01  6:40 ` Niklas Cassel
2025-12-08  5:45 ` Niklas Cassel
2025-12-08  6:20 ` Krishna Chaitanya Chundru
2025-12-09  5:12   ` Niklas Cassel
2025-12-09  5:27     ` Krishna Chaitanya Chundru
2025-12-09  5:36       ` Niklas Cassel
2025-12-12  3:52         ` Manivannan Sadhasivam
2025-12-12  6:37           ` Niklas Cassel
2025-12-22  6:15             ` Manivannan Sadhasivam
2025-12-09  7:11 ` Ilpo Järvinen
2025-12-09  7:45   ` Niklas Cassel

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