mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] PCI: cadence: skip the link polling when endpoint not connected
@ 2026-06-05  7:19 Aksh Garg
  2026-06-08  6:26 ` Chen Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Aksh Garg @ 2026-06-05  7:19 UTC (permalink / raw)
  To: linux-pci, vigneshr, s-vadapalli, lpieralisi, kwilczynski, mani,
	robh, bhelgaas, mpillai, unicorn_wang, me, 18255117159
  Cc: linux-arm-kernel, linux-kernel, danishanwar, a-garg7

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 skip_link_polling flag to track link state across suspend/resume
cycles. If link was down before suspend, skip the expensive polling
in resume since no endpoint was present.

Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !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..d78c1282a5ee 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->skip_link_polling)
+		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..026414c21ee1 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->skip_link_polling)
+		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..01e49ecccc7b 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
+ * @skip_link_polling: Skip link polling in resume if link was down before
+ *		       suspend, to avoid long delay in resume
  */
 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		skip_link_polling:1;
 };
 
 /**
-- 
2.34.1


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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-05  7:19 [PATCH] PCI: cadence: skip the link polling when endpoint not connected Aksh Garg
@ 2026-06-08  6:26 ` Chen Wang
  2026-06-09  3:42 ` Siddharth Vadapalli
  2026-06-22 14:52 ` Manivannan Sadhasivam
  2 siblings, 0 replies; 10+ messages in thread
From: Chen Wang @ 2026-06-08  6:26 UTC (permalink / raw)
  To: Aksh Garg, linux-pci, vigneshr, s-vadapalli, lpieralisi,
	kwilczynski, mani, robh, bhelgaas, mpillai, me, 18255117159
  Cc: linux-arm-kernel, linux-kernel, danishanwar


On 6/5/2026 3:19 PM, Aksh Garg wrote:
> 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 skip_link_polling flag to track link state across suspend/resume
> cycles. If link was down before suspend, skip the expensive polling
> in resume since no endpoint was present.
>
> Signed-off-by: Aksh Garg <a-garg7@ti.com>

Reviewed-by: Chen Wang <unicorn_wang@outlook.com>

[......]



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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-05  7:19 [PATCH] PCI: cadence: skip the link polling when endpoint not connected Aksh Garg
  2026-06-08  6:26 ` Chen Wang
@ 2026-06-09  3:42 ` Siddharth Vadapalli
  2026-06-09  4:28   ` Aksh Garg
  2026-06-22 14:52 ` Manivannan Sadhasivam
  2 siblings, 1 reply; 10+ messages in thread
From: Siddharth Vadapalli @ 2026-06-09  3:42 UTC (permalink / raw)
  To: Aksh Garg
  Cc: vigneshr, lpieralisi, kwilczynski, mani, robh, bhelgaas, mpillai,
	unicorn_wang, me, 18255117159, linux-pci, linux-arm-kernel,
	linux-kernel, danishanwar, s-vadapalli

On 05/06/26 12:49, Aksh Garg wrote:
> 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 skip_link_polling flag to track link state across suspend/resume
> cycles. If link was down before suspend, skip the expensive polling
> in resume since no endpoint was present.
> 
> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !j721e_pcie_link_up(pcie->cdns_pcie);
> +

Please consider the following scenario:
1. Endpoint device is not connected initially and we suspend with 
'skip_link_polling' set to true.
2. We resume and skip link polling.
3. An Endpoint device is connected to the PCIe Connector a while later.
4. We perform a bus rescan, but with skip_link_polling still set (in 
resume), wouldn't we skip polling for the link to be up?

Shouldn't 'skip_link_polling' be set to false on resume, after we skip the 
polling initially, in order to allow waiting for a link up for a newly 
connected device? Although this may not be done in practice and the 
Endpoint device may always be left connected from the start, I am wondering 
if we should clear 'skip_link_polling' on resume to account for the 
aforementioned scenario. Let me know if I am overlooking something here.

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

[TRIMMED]

Regards,
Siddharth.

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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-09  3:42 ` Siddharth Vadapalli
@ 2026-06-09  4:28   ` Aksh Garg
  2026-06-09  5:11     ` Siddharth Vadapalli
  0 siblings, 1 reply; 10+ messages in thread
From: Aksh Garg @ 2026-06-09  4:28 UTC (permalink / raw)
  To: Siddharth Vadapalli
  Cc: vigneshr, lpieralisi, kwilczynski, mani, robh, bhelgaas, mpillai,
	unicorn_wang, me, 18255117159, linux-pci, linux-arm-kernel,
	linux-kernel, danishanwar



On 09/06/26 09:12, Siddharth Vadapalli wrote:
> On 05/06/26 12:49, Aksh Garg wrote:
>> 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 skip_link_polling flag to track link state across suspend/resume
>> cycles. If link was down before suspend, skip the expensive polling
>> in resume since no endpoint was present.
>>
>> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !j721e_pcie_link_up(pcie->cdns_pcie);
>> +
> 
> Please consider the following scenario:
> 1. Endpoint device is not connected initially and we suspend with 
> 'skip_link_polling' set to true.
> 2. We resume and skip link polling.
> 3. An Endpoint device is connected to the PCIe Connector a while later.
> 4. We perform a bus rescan, but with skip_link_polling still set (in 
> resume), wouldn't we skip polling for the link to be up?
> 
> Shouldn't 'skip_link_polling' be set to false on resume, after we skip 
> the polling initially, in order to allow waiting for a link up for a 
> newly connected device? Although this may not be done in practice and 
> the Endpoint device may always be left connected from the start, I am 
> wondering if we should clear 'skip_link_polling' on resume to account 
> for the aforementioned scenario. Let me know if I am overlooking 
> something here.

This path is not used during bus rescan. This implies that during the 
rescan, we do not call cdns_pcie_host_link_setup() or 
cdns_pcie_hpa_host_link_setup().

> 
>>           gpiod_set_value_cansleep(pcie->reset_gpio, 0);
>>           clk_disable_unprepare(pcie->refclk);
>>       }
> 
> [TRIMMED]
> 
> Regards,
> Siddharth.


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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-09  4:28   ` Aksh Garg
@ 2026-06-09  5:11     ` Siddharth Vadapalli
  2026-06-09  5:36       ` Manikandan Karunakaran Pillai
  0 siblings, 1 reply; 10+ messages in thread
From: Siddharth Vadapalli @ 2026-06-09  5:11 UTC (permalink / raw)
  To: Aksh Garg
  Cc: s-vadapalli, vigneshr, lpieralisi, kwilczynski, mani, robh,
	bhelgaas, mpillai, unicorn_wang, me, 18255117159, linux-pci,
	linux-arm-kernel, linux-kernel, danishanwar

On 09/06/26 09:58, Aksh Garg wrote:
> 
> 
> On 09/06/26 09:12, Siddharth Vadapalli wrote:
>> On 05/06/26 12:49, Aksh Garg wrote:
>>> 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 skip_link_polling flag to track link state across suspend/resume
>>> cycles. If link was down before suspend, skip the expensive polling
>>> in resume since no endpoint was present.
>>>
>>> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !j721e_pcie_link_up(pcie->cdns_pcie);
>>> +
>>
>> Please consider the following scenario:
>> 1. Endpoint device is not connected initially and we suspend with 
>> 'skip_link_polling' set to true.
>> 2. We resume and skip link polling.
>> 3. An Endpoint device is connected to the PCIe Connector a while later.
>> 4. We perform a bus rescan, but with skip_link_polling still set (in 
>> resume), wouldn't we skip polling for the link to be up?
>>
>> Shouldn't 'skip_link_polling' be set to false on resume, after we skip 
>> the polling initially, in order to allow waiting for a link up for a 
>> newly connected device? Although this may not be done in practice and the 
>> Endpoint device may always be left connected from the start, I am 
>> wondering if we should clear 'skip_link_polling' on resume to account for 
>> the aforementioned scenario. Let me know if I am overlooking something here.
> 
> This path is not used during bus rescan. This implies that during the 
> rescan, we do not call cdns_pcie_host_link_setup() or 
> cdns_pcie_hpa_host_link_setup().
Thank you for clarifying.

Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

Regards,
Siddharth.

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

* RE: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-09  5:11     ` Siddharth Vadapalli
@ 2026-06-09  5:36       ` Manikandan Karunakaran Pillai
  0 siblings, 0 replies; 10+ messages in thread
From: Manikandan Karunakaran Pillai @ 2026-06-09  5:36 UTC (permalink / raw)
  To: Siddharth Vadapalli, Aksh Garg
  Cc: vigneshr, lpieralisi, kwilczynski, mani, robh, bhelgaas,
	unicorn_wang, me, 18255117159, linux-pci, linux-arm-kernel,
	linux-kernel, danishanwar



>-----Original Message-----
>From: Siddharth Vadapalli <s-vadapalli@ti.com>
>Sent: Tuesday, June 9, 2026 10:42 AM
>To: Aksh Garg <a-garg7@ti.com>
>Cc: s-vadapalli@ti.com; vigneshr@ti.com; lpieralisi@kernel.org;
>kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
>bhelgaas@google.com; Manikandan Karunakaran Pillai
><mpillai@cadence.com>; unicorn_wang@outlook.com; me@ziyao.cc;
>18255117159@163.com; linux-pci@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org; danishanwar@ti.com
>Subject: Re: [PATCH] PCI: cadence: skip the link polling when endpoint not
>connected
>
>EXTERNAL MAIL
>
>
>On 09/06/26 09:58, Aksh Garg wrote:
>>
>>
>> On 09/06/26 09:12, Siddharth Vadapalli wrote:
>>> On 05/06/26 12:49, Aksh Garg wrote:
>>>> 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 skip_link_polling flag to track link state across suspend/resume
>>>> cycles. If link was down before suspend, skip the expensive polling
>>>> in resume since no endpoint was present.
>>>>
>>>> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !j721e_pcie_link_up(pcie->cdns_pcie);
>>>> +
>>>
>>> Please consider the following scenario:
>>> 1. Endpoint device is not connected initially and we suspend with
>>> 'skip_link_polling' set to true.
>>> 2. We resume and skip link polling.
>>> 3. An Endpoint device is connected to the PCIe Connector a while later.
>>> 4. We perform a bus rescan, but with skip_link_polling still set (in
>>> resume), wouldn't we skip polling for the link to be up?
>>>
>>> Shouldn't 'skip_link_polling' be set to false on resume, after we skip
>>> the polling initially, in order to allow waiting for a link up for a
>>> newly connected device? Although this may not be done in practice and the
>>> Endpoint device may always be left connected from the start, I am
>>> wondering if we should clear 'skip_link_polling' on resume to account for
>>> the aforementioned scenario. Let me know if I am overlooking something
>here.
>>
>> This path is not used during bus rescan. This implies that during the
>> rescan, we do not call cdns_pcie_host_link_setup() or
>> cdns_pcie_hpa_host_link_setup().
>Thank you for clarifying.
>
>Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>

Looks good to me


>Regards,
>Siddharth.

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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-05  7:19 [PATCH] PCI: cadence: skip the link polling when endpoint not connected Aksh Garg
  2026-06-08  6:26 ` Chen Wang
  2026-06-09  3:42 ` Siddharth Vadapalli
@ 2026-06-22 14:52 ` Manivannan Sadhasivam
  2026-06-23  5:23   ` Aksh Garg
  2 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-06-22 14:52 UTC (permalink / raw)
  To: Aksh Garg
  Cc: linux-pci, vigneshr, s-vadapalli, lpieralisi, kwilczynski, robh,
	bhelgaas, mpillai, unicorn_wang, me, 18255117159,
	linux-arm-kernel, linux-kernel, danishanwar

On Fri, Jun 05, 2026 at 12:49:22PM +0530, Aksh Garg wrote:
> 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 skip_link_polling flag to track link state across suspend/resume
> cycles. If link was down before suspend, skip the expensive polling
> in resume since no endpoint was present.
> 

Won't you need the delay if a device gets plugged while the host was suspended?
We had this same concern with the DWC drivers and we left the delay as-is as
nothing prevents an user to connect a device when the host was suspended.

- Mani

> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !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..d78c1282a5ee 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->skip_link_polling)
> +		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..026414c21ee1 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->skip_link_polling)
> +		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..01e49ecccc7b 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
> + * @skip_link_polling: Skip link polling in resume if link was down before
> + *		       suspend, to avoid long delay in resume
>   */
>  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		skip_link_polling:1;
>  };
>  
>  /**
> -- 
> 2.34.1
> 

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

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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-22 14:52 ` Manivannan Sadhasivam
@ 2026-06-23  5:23   ` Aksh Garg
  2026-06-23  5:45     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 10+ messages in thread
From: Aksh Garg @ 2026-06-23  5:23 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: linux-pci, vigneshr, s-vadapalli, lpieralisi, kwilczynski, robh,
	bhelgaas, mpillai, unicorn_wang, me, 18255117159,
	linux-arm-kernel, linux-kernel, danishanwar



On 22/06/26 20:22, Manivannan Sadhasivam wrote:
> On Fri, Jun 05, 2026 at 12:49:22PM +0530, Aksh Garg wrote:
>> 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 skip_link_polling flag to track link state across suspend/resume
>> cycles. If link was down before suspend, skip the expensive polling
>> in resume since no endpoint was present.
>>
> 
> Won't you need the delay if a device gets plugged while the host was suspended?
> We had this same concern with the DWC drivers and we left the delay as-is as
> nothing prevents an user to connect a device when the host was suspended.

Yes, that is true. However, the platforms that do not support hot-plug 
can skip the delay during resume since no device could have been 
connected while suspended (only those platform's driver would be 
expected to set 'rc->skip_link_polling) just like what Tegra264 PCIe 
driver tries to do at [1], where the driver sets "pcie->link_up = false" 
if the link is down during the probe and if the controller doesn't 
support hot-plug.

Even if a user connects a device (when the host was suspended) to a 
platform which doesn't support hotplug, the user is expected to run a 
bus rescan, which doesn't call cdns_pcie_host_link_setup() anyway, hence 
the flag 'skip_link_polling' is not taken into account in that case.

[1]: 
https://lore.kernel.org/all/20260617-tegra264-pcie-v7-3-eae7ae964629@nvidia.com/

> 
> - Mani
> 
>> Signed-off-by: Aksh Garg <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..849eb8bb9e45 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->skip_link_polling = !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..d78c1282a5ee 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->skip_link_polling)
>> +		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..026414c21ee1 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->skip_link_polling)
>> +		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..01e49ecccc7b 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
>> + * @skip_link_polling: Skip link polling in resume if link was down before
>> + *		       suspend, to avoid long delay in resume
>>    */
>>   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		skip_link_polling:1;
>>   };
>>   
>>   /**
>> -- 
>> 2.34.1
>>
> 


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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-23  5:23   ` Aksh Garg
@ 2026-06-23  5:45     ` Manivannan Sadhasivam
  2026-06-23  6:37       ` Aksh Garg
  0 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-06-23  5:45 UTC (permalink / raw)
  To: Aksh Garg
  Cc: linux-pci, vigneshr, s-vadapalli, lpieralisi, kwilczynski, robh,
	bhelgaas, mpillai, unicorn_wang, me, 18255117159,
	linux-arm-kernel, linux-kernel, danishanwar

On Tue, Jun 23, 2026 at 10:53:48AM +0530, Aksh Garg wrote:
> 
> 
> On 22/06/26 20:22, Manivannan Sadhasivam wrote:
> > On Fri, Jun 05, 2026 at 12:49:22PM +0530, Aksh Garg wrote:
> > > 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 skip_link_polling flag to track link state across suspend/resume
> > > cycles. If link was down before suspend, skip the expensive polling
> > > in resume since no endpoint was present.
> > > 
> > 
> > Won't you need the delay if a device gets plugged while the host was suspended?
> > We had this same concern with the DWC drivers and we left the delay as-is as
> > nothing prevents an user to connect a device when the host was suspended.
> 
> Yes, that is true. However, the platforms that do not support hot-plug can
> skip the delay during resume since no device could have been connected while
> suspended (only those platform's driver would be expected to set
> 'rc->skip_link_polling) just like what Tegra264 PCIe driver tries to do at
> [1], where the driver sets "pcie->link_up = false" if the link is down
> during the probe and if the controller doesn't support hot-plug.
> 

If the platform doesn't support hotplug, then it is perfectly fine to skip the
delay. But you need to name the flag as like 'no_hotplug' or something
relevant, not 'skip_link_polling'.

> Even if a user connects a device (when the host was suspended) to a platform
> which doesn't support hotplug, the user is expected to run a bus rescan,
> which doesn't call cdns_pcie_host_link_setup() anyway, hence the flag
> 'skip_link_polling' is not taken into account in that case.
> 

This is not about running the rescan, but about initializing the device. The 1s
delay in cdns_pcie_host_wait_for_link() is mandated by the PCIe spec after
starting LTSSM and before issuing the config read to the device. If this delay
is skipped and if a config read is issued to the device, then the device may not
respond and the failure may lead to PCI stack assuming that the device is dead.

- Mani

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

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

* Re: [PATCH] PCI: cadence: skip the link polling when endpoint not connected
  2026-06-23  5:45     ` Manivannan Sadhasivam
@ 2026-06-23  6:37       ` Aksh Garg
  0 siblings, 0 replies; 10+ messages in thread
From: Aksh Garg @ 2026-06-23  6:37 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: linux-pci, vigneshr, s-vadapalli, lpieralisi, kwilczynski, robh,
	bhelgaas, mpillai, unicorn_wang, me, 18255117159,
	linux-arm-kernel, linux-kernel, danishanwar



On 23/06/26 11:15, Manivannan Sadhasivam wrote:
> On Tue, Jun 23, 2026 at 10:53:48AM +0530, Aksh Garg wrote:
>>
>>
>> On 22/06/26 20:22, Manivannan Sadhasivam wrote:
>>> On Fri, Jun 05, 2026 at 12:49:22PM +0530, Aksh Garg wrote:
>>>> 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 skip_link_polling flag to track link state across suspend/resume
>>>> cycles. If link was down before suspend, skip the expensive polling
>>>> in resume since no endpoint was present.
>>>>
>>>
>>> Won't you need the delay if a device gets plugged while the host was suspended?
>>> We had this same concern with the DWC drivers and we left the delay as-is as
>>> nothing prevents an user to connect a device when the host was suspended.
>>
>> Yes, that is true. However, the platforms that do not support hot-plug can
>> skip the delay during resume since no device could have been connected while
>> suspended (only those platform's driver would be expected to set
>> 'rc->skip_link_polling) just like what Tegra264 PCIe driver tries to do at
>> [1], where the driver sets "pcie->link_up = false" if the link is down
>> during the probe and if the controller doesn't support hot-plug.
>>
> 
> If the platform doesn't support hotplug, then it is perfectly fine to skip the
> delay. But you need to name the flag as like 'no_hotplug' or something
> relevant, not 'skip_link_polling'.

Sure, I will rename the flag to 'link_down_no_hotplug' to clarify that 
it tracks the link state when there's no hotplug support.

> 
>> Even if a user connects a device (when the host was suspended) to a platform
>> which doesn't support hotplug, the user is expected to run a bus rescan,
>> which doesn't call cdns_pcie_host_link_setup() anyway, hence the flag
>> 'skip_link_polling' is not taken into account in that case.
>>
> 
> This is not about running the rescan, but about initializing the device. The 1s
> delay in cdns_pcie_host_wait_for_link() is mandated by the PCIe spec after
> starting LTSSM and before issuing the config read to the device. If this delay
> is skipped and if a config read is issued to the device, then the device may not
> respond and the failure may lead to PCI stack assuming that the device is dead.

Understood, thanks for the explanation.

> 
> - Mani
> 


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

end of thread, other threads:[~2026-06-23  6:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-05  7:19 [PATCH] PCI: cadence: skip the link polling when endpoint not connected Aksh Garg
2026-06-08  6:26 ` Chen Wang
2026-06-09  3:42 ` Siddharth Vadapalli
2026-06-09  4:28   ` Aksh Garg
2026-06-09  5:11     ` Siddharth Vadapalli
2026-06-09  5:36       ` Manikandan Karunakaran Pillai
2026-06-22 14:52 ` Manivannan Sadhasivam
2026-06-23  5:23   ` Aksh Garg
2026-06-23  5:45     ` Manivannan Sadhasivam
2026-06-23  6:37       ` Aksh Garg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome