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

[Siddharth 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@xxxxxx>
> ---
>   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.