The suspend handler checks for the PCIe Link up to decide when to turn off
the controller resources. But this check is racy as the PCIe Link can go
down just after this check.

So use the newly introduced API, pci_root_ports_have_device() that checks
for the presence of a device under any of the Root Ports to replace the
Link up check.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 805edbbfe7eb..b2b89e2e4916 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 static int qcom_pcie_suspend_noirq(struct device *dev)
 {
 	struct qcom_pcie *pcie;
+	struct dw_pcie_rp *pp;
 	int ret = 0;
 
 	pcie = dev_get_drvdata(dev);
@@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
 	 * powerdown state. This will affect the lifetime of the storage devices
 	 * like NVMe.
 	 */
-	if (!dw_pcie_link_up(pcie->pci)) {
-		qcom_pcie_host_deinit(&pcie->pci->pp);
+	pp = &pcie->pci->pp;
+	if (!pci_root_ports_have_device(pp->bridge->bus)) {
+		qcom_pcie_host_deinit(pp);
 		pcie->suspended = true;
 	}
 
-- 
2.48.1

> -----Original Messages-----
> From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> Send time:Thursday, 06/11/2025 14:13:25
> To: lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
> Cc: will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org, zhangsenchuan@eswincomputing.com, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> Subject: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> 
> The suspend handler checks for the PCIe Link up to decide when to turn off
> the controller resources. But this check is racy as the PCIe Link can go
> down just after this check.
> 
> So use the newly introduced API, pci_root_ports_have_device() that checks
> for the presence of a device under any of the Root Ports to replace the
> Link up check.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 805edbbfe7eb..b2b89e2e4916 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  static int qcom_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct qcom_pcie *pcie;
> +	struct dw_pcie_rp *pp;
>  	int ret = 0;
>  
>  	pcie = dev_get_drvdata(dev);
> @@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  	 * powerdown state. This will affect the lifetime of the storage devices
>  	 * like NVMe.
>  	 */
> -	if (!dw_pcie_link_up(pcie->pci)) {
> -		qcom_pcie_host_deinit(&pcie->pci->pp);
> +	pp = &pcie->pci->pp;
> +	if (!pci_root_ports_have_device(pp->bridge->bus)) {

I'm a little confused.
The pci_root_ports_have_device function can help check if there is any device 
available under the Root Ports, if there is a device available, the resource 
cannot be released, is it also necessary to release resources when entering 
the L2/L3 state?

> +		qcom_pcie_host_deinit(pp);
>  		pcie->suspended = true;
>  	}
>  
> -- 
> 2.48.1

Best Regards
Senchuan Zhang

On Thu, Nov 06, 2025 at 06:13:05PM +0800, zhangsenchuan wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > Send time:Thursday, 06/11/2025 14:13:25
> > To: lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
> > Cc: will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org, zhangsenchuan@eswincomputing.com, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > Subject: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > 
> > The suspend handler checks for the PCIe Link up to decide when to turn off
> > the controller resources. But this check is racy as the PCIe Link can go
> > down just after this check.
> > 
> > So use the newly introduced API, pci_root_ports_have_device() that checks
> > for the presence of a device under any of the Root Ports to replace the
> > Link up check.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 805edbbfe7eb..b2b89e2e4916 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> >  static int qcom_pcie_suspend_noirq(struct device *dev)
> >  {
> >  	struct qcom_pcie *pcie;
> > +	struct dw_pcie_rp *pp;
> >  	int ret = 0;
> >  
> >  	pcie = dev_get_drvdata(dev);
> > @@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
> >  	 * powerdown state. This will affect the lifetime of the storage devices
> >  	 * like NVMe.
> >  	 */
> > -	if (!dw_pcie_link_up(pcie->pci)) {
> > -		qcom_pcie_host_deinit(&pcie->pci->pp);
> > +	pp = &pcie->pci->pp;
> > +	if (!pci_root_ports_have_device(pp->bridge->bus)) {
> 
> I'm a little confused.
> The pci_root_ports_have_device function can help check if there is any device 
> available under the Root Ports, if there is a device available, the resource 
> cannot be released, is it also necessary to release resources when entering 
> the L2/L3 state?
> 

It is upto the controller driver to decide. Once the link enters L2/L3, the
device will be in D3Cold state. So the controller can just disable all PCIe
resources to save power.

But it is not possible to transition all PCIe devices to D3Cold during suspend,
for instance NVMe. I'm hoping to fix it too in the coming days.

- Mani

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

> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@kernel.org>
> Send time:Thursday, 06/11/2025 19:57:16
> To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>, lpieralisi@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com, will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org
> Subject: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> 
> On Thu, Nov 06, 2025 at 06:13:05PM +0800, zhangsenchuan wrote:
> > 
> > 
> > 
> > > -----Original Messages-----
> > > From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > Send time:Thursday, 06/11/2025 14:13:25
> > > To: lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
> > > Cc: will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org, zhangsenchuan@eswincomputing.com, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > Subject: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > > 
> > > The suspend handler checks for the PCIe Link up to decide when to turn off
> > > the controller resources. But this check is racy as the PCIe Link can go
> > > down just after this check.
> > > 
> > > So use the newly introduced API, pci_root_ports_have_device() that checks
> > > for the presence of a device under any of the Root Ports to replace the
> > > Link up check.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 805edbbfe7eb..b2b89e2e4916 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > >  static int qcom_pcie_suspend_noirq(struct device *dev)
> > >  {
> > >  	struct qcom_pcie *pcie;
> > > +	struct dw_pcie_rp *pp;
> > >  	int ret = 0;
> > >  
> > >  	pcie = dev_get_drvdata(dev);
> > > @@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
> > >  	 * powerdown state. This will affect the lifetime of the storage devices
> > >  	 * like NVMe.
> > >  	 */
> > > -	if (!dw_pcie_link_up(pcie->pci)) {
> > > -		qcom_pcie_host_deinit(&pcie->pci->pp);
> > > +	pp = &pcie->pci->pp;
> > > +	if (!pci_root_ports_have_device(pp->bridge->bus)) {
> > 
> > I'm a little confused.
> > The pci_root_ports_have_device function can help check if there is any device 
> > available under the Root Ports, if there is a device available, the resource 
> > cannot be released, is it also necessary to release resources when entering 
> > the L2/L3 state?
> > 
> 
> It is upto the controller driver to decide. Once the link enters L2/L3, the
> device will be in D3Cold state. So the controller can just disable all PCIe
> resources to save power.
> 
> But it is not possible to transition all PCIe devices to D3Cold during suspend,
> for instance NVMe. I'm hoping to fix it too in the coming days.
> 
Hi, Manivannan

Thank you for your explanation.

By the way, in v5 patch, I removed the dw_pcie_link_up judgment, and currently
resources are directly released.
At present, i have completed the pcie v5 patch without adding the 
pci_root_ports_have_device function. Do I need to wait for you to merge it 
before sending the V5 patch?

Kind regards,
Senchuan Zhang

On Fri, Nov 07, 2025 at 02:27:15PM +0800, zhangsenchuan wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > Send time:Thursday, 06/11/2025 19:57:16
> > To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> > Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>, lpieralisi@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com, will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org
> > Subject: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > 
> > On Thu, Nov 06, 2025 at 06:13:05PM +0800, zhangsenchuan wrote:
> > > 
> > > 
> > > 
> > > > -----Original Messages-----
> > > > From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > > Send time:Thursday, 06/11/2025 14:13:25
> > > > To: lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
> > > > Cc: will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org, zhangsenchuan@eswincomputing.com, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > > Subject: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > > > 
> > > > The suspend handler checks for the PCIe Link up to decide when to turn off
> > > > the controller resources. But this check is racy as the PCIe Link can go
> > > > down just after this check.
> > > > 
> > > > So use the newly introduced API, pci_root_ports_have_device() that checks
> > > > for the presence of a device under any of the Root Ports to replace the
> > > > Link up check.
> > > > 
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 805edbbfe7eb..b2b89e2e4916 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > >  static int qcom_pcie_suspend_noirq(struct device *dev)
> > > >  {
> > > >  	struct qcom_pcie *pcie;
> > > > +	struct dw_pcie_rp *pp;
> > > >  	int ret = 0;
> > > >  
> > > >  	pcie = dev_get_drvdata(dev);
> > > > @@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
> > > >  	 * powerdown state. This will affect the lifetime of the storage devices
> > > >  	 * like NVMe.
> > > >  	 */
> > > > -	if (!dw_pcie_link_up(pcie->pci)) {
> > > > -		qcom_pcie_host_deinit(&pcie->pci->pp);
> > > > +	pp = &pcie->pci->pp;
> > > > +	if (!pci_root_ports_have_device(pp->bridge->bus)) {
> > > 
> > > I'm a little confused.
> > > The pci_root_ports_have_device function can help check if there is any device 
> > > available under the Root Ports, if there is a device available, the resource 
> > > cannot be released, is it also necessary to release resources when entering 
> > > the L2/L3 state?
> > > 
> > 
> > It is upto the controller driver to decide. Once the link enters L2/L3, the
> > device will be in D3Cold state. So the controller can just disable all PCIe
> > resources to save power.
> > 
> > But it is not possible to transition all PCIe devices to D3Cold during suspend,
> > for instance NVMe. I'm hoping to fix it too in the coming days.
> > 
> Hi, Manivannan
> 
> Thank you for your explanation.
> 
> By the way, in v5 patch, I removed the dw_pcie_link_up judgment, and currently
> resources are directly released.
> At present, i have completed the pcie v5 patch without adding the 
> pci_root_ports_have_device function. Do I need to wait for you to merge it 
> before sending the V5 patch?
> 

I've merged my series to pci/controller/dwc branch. You can use this branch as a
base for your v5.

- Mani

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

> -----Original Messages-----
> From: "Manivannan Sadhasivam" <mani@kernel.org>
> Send time:Saturday, 08/11/2025 18:25:36
> To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>, lpieralisi@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com, will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org
> Subject: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> 
> On Fri, Nov 07, 2025 at 02:27:15PM +0800, zhangsenchuan wrote:
> > 
> > 
> > 
> > > -----Original Messages-----
> > > From: "Manivannan Sadhasivam" <mani@kernel.org>
> > > Send time:Thursday, 06/11/2025 19:57:16
> > > To: zhangsenchuan <zhangsenchuan@eswincomputing.com>
> > > Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>, lpieralisi@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com, will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org
> > > Subject: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > > 
> > > On Thu, Nov 06, 2025 at 06:13:05PM +0800, zhangsenchuan wrote:
> > > > 
> > > > 
> > > > 
> > > > > -----Original Messages-----
> > > > > From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > > > Send time:Thursday, 06/11/2025 14:13:25
> > > > > To: lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
> > > > > Cc: will@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, robh@kernel.org, linux-arm-msm@vger.kernel.org, zhangsenchuan@eswincomputing.com, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > > > > Subject: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend
> > > > > 
> > > > > The suspend handler checks for the PCIe Link up to decide when to turn off
> > > > > the controller resources. But this check is racy as the PCIe Link can go
> > > > > down just after this check.
> > > > > 
> > > > > So use the newly introduced API, pci_root_ports_have_device() that checks
> > > > > for the presence of a device under any of the Root Ports to replace the
> > > > > Link up check.
> > > > > 
> > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++--
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > index 805edbbfe7eb..b2b89e2e4916 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > @@ -2018,6 +2018,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > > >  static int qcom_pcie_suspend_noirq(struct device *dev)
> > > > >  {
> > > > >  	struct qcom_pcie *pcie;
> > > > > +	struct dw_pcie_rp *pp;
> > > > >  	int ret = 0;
> > > > >  
> > > > >  	pcie = dev_get_drvdata(dev);
> > > > > @@ -2053,8 +2054,9 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
> > > > >  	 * powerdown state. This will affect the lifetime of the storage devices
> > > > >  	 * like NVMe.
> > > > >  	 */
> > > > > -	if (!dw_pcie_link_up(pcie->pci)) {
> > > > > -		qcom_pcie_host_deinit(&pcie->pci->pp);
> > > > > +	pp = &pcie->pci->pp;
> > > > > +	if (!pci_root_ports_have_device(pp->bridge->bus)) {
> > > > 
> > > > I'm a little confused.
> > > > The pci_root_ports_have_device function can help check if there is any device 
> > > > available under the Root Ports, if there is a device available, the resource 
> > > > cannot be released, is it also necessary to release resources when entering 
> > > > the L2/L3 state?
> > > > 
> > > 
> > > It is upto the controller driver to decide. Once the link enters L2/L3, the
> > > device will be in D3Cold state. So the controller can just disable all PCIe
> > > resources to save power.
> > > 
> > > But it is not possible to transition all PCIe devices to D3Cold during suspend,
> > > for instance NVMe. I'm hoping to fix it too in the coming days.
> > > 
> > Hi, Manivannan
> > 
> > Thank you for your explanation.
> > 
> > By the way, in v5 patch, I removed the dw_pcie_link_up judgment, and currently
> > resources are directly released.
> > At present, i have completed the pcie v5 patch without adding the 
> > pci_root_ports_have_device function. Do I need to wait for you to merge it 
> > before sending the V5 patch?
> > 
> 
> I've merged my series to pci/controller/dwc branch. You can use this branch as a
> base for your v5.
> 
Hi, Manivannan

Thank you very much for the newly created branch. I have already submitted a v5 
patch based on this branch.

Kind regards,
Senchuan Zhang