[PATCH v2] PCI: cadence: skip the link polling when endpoint not connected

[PATCH v2] PCI: cadence: skip the link polling when endpoint not connected

[Aksh Garg]

cdns_pcie_host_wait_for_link() polls on link-up for 10 retries with a
delay of 90-100ms each (~1 second). A call to cdns_pcie_host_link_setup()
during the resume operation blocks the resume operation unnecessarily for
~1s even when no endpoint device is connected.

Add link_down_no_hotplug flag to track link state across suspend/resume
cycles (for the platforms that do not support hotplug). If link was
down before suspend in such platforms, skip the expensive polling in
resume since no endpoint was present.

Reviewed-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Signed-off-by: Aksh Garg <a-garg7@ti.com>
---

Changes from v1 to v2:
- Updated the flag name from 'skip_link_polling' to 'link_down_no_hotplug'

v1: https://lore.kernel.org/all/20260605071922.1724499-1-a-garg7@ti.com/

 drivers/pci/controller/cadence/pci-j721e.c             | 5 +++++
 drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 3 +++
 drivers/pci/controller/cadence/pcie-cadence-host.c     | 3 +++
 drivers/pci/controller/cadence/pcie-cadence.h          | 3 +++
 4 files changed, 14 insertions(+)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index bfdfe98d5aba..48db7a6cf754 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -686,6 +686,11 @@ static int j721e_pcie_suspend_noirq(struct device *dev)
 	struct j721e_pcie *pcie = dev_get_drvdata(dev);
 
 	if (pcie->mode == PCI_MODE_RC) {
+		struct cdns_pcie_rc *rc = cdns_pcie_to_rc(pcie->cdns_pcie);
+
+		/* If link is down before suspend, skip polling in resume */
+		rc->link_down_no_hotplug = !j721e_pcie_link_up(pcie->cdns_pcie);
+
 		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
 		clk_disable_unprepare(pcie->refclk);
 	}
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
index 0f540bed58e8..31cf50cff8f8 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
@@ -301,6 +301,9 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc *rc)
 		return ret;
 	}
 
+	if (rc->link_down_no_hotplug)
+		return 0;
+
 	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
 	if (ret)
 		dev_dbg(dev, "PCIe link never came up\n");
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 0bc9e6e90e0e..50abc657a871 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -352,6 +352,9 @@ int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc)
 		return ret;
 	}
 
+	if (rc->link_down_no_hotplug)
+		return 0;
+
 	ret = cdns_pcie_host_start_link(rc, cdns_pcie_link_up);
 	if (ret)
 		dev_dbg(dev, "PCIe link never came up\n");
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 574e9cf4d003..1561022c1a8b 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -117,6 +117,8 @@ struct cdns_pcie {
  * @no_inbound_map: Whether inbound mapping is supported
  * @quirk_broken_aspm_l0s: Disable ASPM L0s support as quirk
  * @quirk_broken_aspm_l1: Disable ASPM L1 support as quirk
+ * @link_down_no_hotplug: Skip link polling during resume on no-hotplug
+ *			  platforms when link was down before suspend
  */
 struct cdns_pcie_rc {
 	struct cdns_pcie	pcie;
@@ -131,6 +133,7 @@ struct cdns_pcie_rc {
 	unsigned int            no_inbound_map:1;
 	unsigned int            quirk_broken_aspm_l0s:1;
 	unsigned int            quirk_broken_aspm_l1:1;
+	unsigned int		link_down_no_hotplug:1;
 };
 
 /**
-- 
2.34.1

Re: [PATCH v2] PCI: cadence: skip the link polling when endpoint not connected

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Checking PCIe link state during the host controller's `suspend_noirq` incorrectly infers a disconnected link because downstream devices have already been suspended, breaking system resume for all connected endpoints.
--

commit 3a9bc5bfaa54c1ff36dc42ca33244027b719cd03
Author: Aksh Garg <a-garg7@ti.com>

PCI: cadence: skip the link polling when endpoint not connected

This commit introduces a flag to skip PCIe link polling during system resume
on platforms without hotplug support if the link was down before suspend.
By avoiding the polling delay when no endpoint is connected, it speeds up
the resume operation.

> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -686,6 +686,11 @@ static int j721e_pcie_suspend_noirq(struct device *dev)
>  	struct j721e_pcie *pcie = dev_get_drvdata(dev);
>  
>  	if (pcie->mode == PCI_MODE_RC) {
> +		struct cdns_pcie_rc *rc = cdns_pcie_to_rc(pcie->cdns_pcie);
> +
> +		/* If link is down before suspend, skip polling in resume */
> +		rc->link_down_no_hotplug = !j721e_pcie_link_up(pcie->cdns_pcie);

[Severity: High]
Will this correctly identify connected devices during suspend? 

Because the PM core suspends devices in reverse-probe order, child endpoints
will have already been suspended and placed in a low-power state (like D3cold)
before j721e_pcie_suspend_noirq() runs. 

Does this mean the link will likely drop, causing this code to capture a
"down" state even for properly connected, valid endpoints?

> +
>  		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
>  		clk_disable_unprepare(pcie->refclk);
>  	}

[ ... ]

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -352,6 +352,9 @@ int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc)
>  		return ret;
>  	}
>  
> +	if (rc->link_down_no_hotplug)
> +		return 0;
> +

[Severity: High]
Could skipping cdns_pcie_host_start_link() here cause subsequent endpoint
resume operations to fail?

If the flag was incorrectly set to true during suspend, the Root Complex will
skip waiting for the link to train. When the PM core later attempts to resume
the endpoints, couldn't accesses to configuration space over the untrained
link time out or trigger an asynchronous external abort?

>  	ret = cdns_pcie_host_start_link(rc, cdns_pcie_link_up);
>  	if (ret)
>  		dev_dbg(dev, "PCIe link never came up\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623095148.721206-1-a-garg7@ti.com?part=1