Re: [PATCH v2 3/3] PCI: dwc: Check for the device presence during suspend and resume

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Frank]

On Fri, Nov 07, 2025 at 10:13:19AM +0530, Manivannan Sadhasivam wrote:
> If there is no device available under the Root Ports, there is no point in
> sending PME_Turn_Off and waiting for L2/L3 transition during suspend, it
> will result in a timeout. Hence, skip those steps if no device is available
> during suspend.
> 
> During resume, do not wait for the link up if there was no device connected
> before suspend. It is very unlikely that a device will get connected while
> the host system was suspended.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 20c9333bcb1c..5a39e7139ec9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -20,6 +20,7 @@
>  #include <linux/platform_device.h>
>  
>  #include "../../pci.h"
> +#include "../pci-host-common.h"
>  #include "pcie-designware.h"
>  
>  static struct pci_ops dw_pcie_ops;
> @@ -1129,6 +1130,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	u32 val;
>  	int ret;
>  
> +	if (!pci_root_ports_have_device(pci->pp.bridge->bus))
> +		goto stop_link;

This looks racy.  Maybe it's still OK, but I think it would be good to
include a comment to acknowledge that and explain why either outcome
is acceptable, e.g., if a user removes a device during suspend, it
results in a timeout but nothing more terrible.

>  	/*
>  	 * If L1SS is supported, then do not put the link into L2 as some
>  	 * devices such as NVMe expect low resume latency.
> @@ -1162,6 +1166,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	 */
>  	udelay(1);
>  
> +stop_link:
>  	dw_pcie_stop_link(pci);
>  	if (pci->pp.ops->deinit)
>  		pci->pp.ops->deinit(&pci->pp);
> @@ -1195,6 +1200,14 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * If there was no device before suspend, skip waiting for link up as
> +	 * it is bound to fail. It is very unlikely that a device will get
> +	 * connected *during* suspend.

I'm not convinced.  Unlike the suspend side, where the race window is
tiny, here the window is the entire time the system is suspended, and
at least in laptop usage, there's no reason I would hesitate to plug
something in while suspended.

Regardless, the overall behavior needs to be acceptable whether or not
a device was connected during suspend.

This is probably the same thing you said, Frank, sorry if I'm just
repeating it.

> +	if (!pci_root_ports_have_device(pci->pp.bridge->bus))
> +		return 0;
> +
>  	ret = dw_pcie_wait_for_link(pci);
>  	if (ret)
>  		return ret;
> -- 
> 2.48.1
>