[PATCH 0/3] PCI: dwc: Replace Link up check with device presence in suspend path

[PATCH 0/3] PCI: dwc: Replace Link up check with device presence in suspend path

[Manivannan Sadhasivam]

Hi,

This series aims to fix the usage of dw_pcie_link_up() API to check for Link up
during system suspend. The motivation for this series comes from recent
discussions [1] [2], where developers wanted to skip PME_Turn_Off broadcast in
dw_pcie_suspend_noirq() API when devices are not attached to the bus. They ended
up using dw_pcie_link_up() to check for the device presence due to the bad
example in the pcie-qcom driver which does the same. The usage of
dw_pcie_link_up() API here would be racy as the link can go down at any time
after the check.

So to properly check for the device presence, this series introduces an API, 
pci_root_ports_have_device(), that accepts the Root bus pointer and checks for
the presence of a device under any of the Root Ports. This API is used to
replace the dw_pcie_link_up() check in suspend path of pcie-qcom driver and also
used to skip the PME_Turn_Off brodcast message in dwc_pcie_suspend_noirq() API.

Testing
=======

This series is tested on Qualcomm Lenovo Thinkpad T14s and observed no
functional change during the system suspend path.

- Mani

[1] https://lore.kernel.org/linux-pci/CAKfTPtCtHquxtK=Zx2WSNm15MmqeUXO8XXi8FkS4EpuP80PP7g@mail.gmail.com/
[2] https://lore.kernel.org/linux-pci/27516921.17f2.1997bb2a498.Coremail.zhangsenchuan@eswincomputing.com/

Manivannan Sadhasivam (3):
  PCI: host-common: Add an API to check for any device under the Root
    Ports
  PCI: qcom: Check for the presence of a device instead of Link up
    during suspend
  PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is
    available

 .../pci/controller/dwc/pcie-designware-host.c |  5 +++++
 drivers/pci/controller/dwc/pcie-qcom.c        |  6 ++++--
 drivers/pci/controller/pci-host-common.c      | 21 +++++++++++++++++++
 drivers/pci/controller/pci-host-common.h      |  2 ++
 4 files changed, 32 insertions(+), 2 deletions(-)

-- 
2.48.1

[PATCH 1/3] PCI: host-common: Add an API to check for any device under the Root Ports

[Manivannan Sadhasivam]

Some controller drivers need to check if there is any device available
under the Root Ports. So add an API that returns 'true' if a device is
found under any of the Root Ports, 'false' otherwise.

Controller drivers can use this API for usecases like turning off the
controller resources only if there are no devices under the Root Ports,
skipping PME_Turn_Off broadcast etc...

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/pci-host-common.c | 21 +++++++++++++++++++++
 drivers/pci/controller/pci-host-common.h |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 810d1c8de24e..6b4f90903dc6 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -17,6 +17,27 @@
 
 #include "pci-host-common.h"
 
+/**
+ * pci_root_ports_have_device - Check if the Root Ports under the Root bus have
+ *				any device underneath
+ * @dev: Root bus
+ *
+ * Return: true if a device is found, false otherwise
+ */
+bool pci_root_ports_have_device(struct pci_bus *root_bus)
+{
+	struct pci_bus *child;
+
+	/* Iterate over the Root Port busses and look for any device */
+	list_for_each_entry(child, &root_bus->children, node) {
+		if (list_count_nodes(&child->devices))
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(pci_root_ports_have_device);
+
 static void gen_pci_unmap_cfg(void *ptr)
 {
 	pci_ecam_free((struct pci_config_window *)ptr);
diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index 51c35ec0cf37..ff1c2ff98043 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -19,4 +19,6 @@ void pci_host_common_remove(struct platform_device *pdev);
 
 struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
 	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
+
+bool pci_root_ports_have_device(struct pci_bus *root_bus);
 #endif
-- 
2.48.1

Re: [PATCH 1/3] PCI: host-common: Add an API to check for any device under the Root Ports

[Konrad Dybcio]

On 11/6/25 7:13 AM, Manivannan Sadhasivam wrote:
> Some controller drivers need to check if there is any device available
> under the Root Ports. So add an API that returns 'true' if a device is
> found under any of the Root Ports, 'false' otherwise.
> 
> Controller drivers can use this API for usecases like turning off the
> controller resources only if there are no devices under the Root Ports,
> skipping PME_Turn_Off broadcast etc...
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/pci/controller/pci-host-common.c | 21 +++++++++++++++++++++
>  drivers/pci/controller/pci-host-common.h |  2 ++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index 810d1c8de24e..6b4f90903dc6 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -17,6 +17,27 @@
>  
>  #include "pci-host-common.h"
>  
> +/**
> + * pci_root_ports_have_device - Check if the Root Ports under the Root bus have
> + *				any device underneath
> + * @dev: Root bus
> + *
> + * Return: true if a device is found, false otherwise
> + */
> +bool pci_root_ports_have_device(struct pci_bus *root_bus)
> +{
> +	struct pci_bus *child;
> +
> +	/* Iterate over the Root Port busses and look for any device */
> +	list_for_each_entry(child, &root_bus->children, node) {
> +		if (list_count_nodes(&child->devices))

Is this list ever shrunk? I grepped around and couldn't find where
that happens

Konrad

Re: [PATCH 1/3] PCI: host-common: Add an API to check for any device under the Root Ports

[Manivannan Sadhasivam]

On Thu, Nov 06, 2025 at 10:47:48AM +0100, Konrad Dybcio wrote:
> On 11/6/25 7:13 AM, Manivannan Sadhasivam wrote:
> > Some controller drivers need to check if there is any device available
> > under the Root Ports. So add an API that returns 'true' if a device is
> > found under any of the Root Ports, 'false' otherwise.
> > 
> > Controller drivers can use this API for usecases like turning off the
> > controller resources only if there are no devices under the Root Ports,
> > skipping PME_Turn_Off broadcast etc...
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/pci/controller/pci-host-common.c | 21 +++++++++++++++++++++
> >  drivers/pci/controller/pci-host-common.h |  2 ++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > index 810d1c8de24e..6b4f90903dc6 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -17,6 +17,27 @@
> >  
> >  #include "pci-host-common.h"
> >  
> > +/**
> > + * pci_root_ports_have_device - Check if the Root Ports under the Root bus have
> > + *				any device underneath
> > + * @dev: Root bus
> > + *
> > + * Return: true if a device is found, false otherwise
> > + */
> > +bool pci_root_ports_have_device(struct pci_bus *root_bus)
> > +{
> > +	struct pci_bus *child;
> > +
> > +	/* Iterate over the Root Port busses and look for any device */
> > +	list_for_each_entry(child, &root_bus->children, node) {
> > +		if (list_count_nodes(&child->devices))
> 
> Is this list ever shrunk? I grepped around and couldn't find where
> that happens
> 

So I digged into this and other 'pci_bus' lists, to my shock, none of list
entries were getting dropped. I'll send out fixes for all of them.

Thanks for catching this historical issue that no one noticed before :)

- Mani

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

Re: [PATCH 1/3] PCI: host-common: Add an API to check for any device under the Root Ports

[kernel test robot]

Hi Manivannan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.18-rc4 next-20251106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam/PCI-host-common-Add-an-API-to-check-for-any-device-under-the-Root-Ports/20251106-141822
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20251106061326.8241-2-manivannan.sadhasivam%40oss.qualcomm.com
patch subject: [PATCH 1/3] PCI: host-common: Add an API to check for any device under the Root Ports
config: arm-randconfig-001-20251107 (https://download.01.org/0day-ci/archive/20251107/202511070717.PZBzRyQO-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251107/202511070717.PZBzRyQO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511070717.PZBzRyQO-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/pci/controller/pci-host-common.c:27 function parameter 'root_bus' not described in 'pci_root_ports_have_device'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[Manivannan Sadhasivam]

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

Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[zhangsenchuan]

> -----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

Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[Manivannan Sadhasivam]

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

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

Re: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[zhangsenchuan]

> -----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

Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[Manivannan Sadhasivam]

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

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

Re: Re: [PATCH 2/3] PCI: qcom: Check for the presence of a device instead of Link up during suspend

[zhangsenchuan]

> -----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

[PATCH 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available

[Manivannan Sadhasivam]

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, it will result in a
timeout.

Hence, skip those steps if no device is available during suspend. The
resume flow remains unchanged.

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

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 20c9333bcb1c..b6b8139e91e3 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;
+
 	/*
 	 * 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);
-- 
2.48.1

Re: [PATCH 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available

[zhangsenchuan]

> -----Original Messages-----
> From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> Send time:Thursday, 06/11/2025 14:13:26
> 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 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available
> 
> 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, it will result in a
> timeout.
> 
> Hence, skip those steps if no device is available during suspend. The
> resume flow remains unchanged.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 20c9333bcb1c..b6b8139e91e3 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;
> +
>  	/*
>  	 * 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);
> -- 
> 2.48.1

hi, Manivannan

I'd like your advice on a few things.

If there is no device available under the Root Ports, the dw_pcie_wait_for_link
function in dw_pcie_resume_noirq still need to wait for the link_up? Otherwise
linkup will TIMEDOUT. At this time, when the resume function return, -ETIMEDOUT 
returned which will raise "PM: failed to resume noirq: error -110".

Currently, in the pci-imx6.c/pci-layerscape.c/pcie-stm32.c file, the 
dw_pcie_resume_noirq function is directly returned after use.
Does the pci_root_ports_have_device function help vendor avoid 
this problem?

Best Regards,
Senchuan Zhang

Re: [PATCH 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available

[Manivannan Sadhasivam]

On Thu, Nov 06, 2025 at 06:02:55PM +0800, zhangsenchuan wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>
> > Send time:Thursday, 06/11/2025 14:13:26
> > 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 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available
> > 
> > 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, it will result in a
> > timeout.
> > 
> > Hence, skip those steps if no device is available during suspend. The
> > resume flow remains unchanged.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 20c9333bcb1c..b6b8139e91e3 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;
> > +
> >  	/*
> >  	 * 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);
> > -- 
> > 2.48.1
> 
> hi, Manivannan
> 
> I'd like your advice on a few things.
> 
> If there is no device available under the Root Ports, the dw_pcie_wait_for_link
> function in dw_pcie_resume_noirq still need to wait for the link_up? Otherwise
> linkup will TIMEDOUT. At this time, when the resume function return, -ETIMEDOUT 
> returned which will raise "PM: failed to resume noirq: error -110".
> 

I thought about it, but was worried that if there was no device inserted before
suspend, but if one gets inserted during suspend (before resume), then the link
up failure will get un-noticed if the link doesn't come up for the new device
and we skip the timeout.

But thinking more, it occurs to me that it will be a very rare case. So I'll
skip returning timeout from dw_pcie_wait_for_link() if there were no devices
before suspend. However, I do think that the dw_pcie_wait_for_link() should not
be skipped even if there were no devices earlier.

- Mani

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

Re: [PATCH 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available

[kernel test robot]

Hi Manivannan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.18-rc4 next-20251106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam/PCI-host-common-Add-an-API-to-check-for-any-device-under-the-Root-Ports/20251106-141822
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20251106061326.8241-4-manivannan.sadhasivam%40oss.qualcomm.com
patch subject: [PATCH 3/3] PCI: dwc: Skip PME_Turn_Off and L2/L3 transition if no device is available
config: arm-randconfig-001-20251107 (https://download.01.org/0day-ci/archive/20251107/202511071025.JM5nTGnO-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251107/202511071025.JM5nTGnO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511071025.JM5nTGnO-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pcie-designware-host.c:1133:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    1133 |         if (!pci_root_ports_have_device(pci->pp.bridge->bus))
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/controller/dwc/pcie-designware-host.c:1176:9: note: uninitialized use occurs here
    1176 |         return ret;
         |                ^~~
   drivers/pci/controller/dwc/pcie-designware-host.c:1133:2: note: remove the 'if' if its condition is always false
    1133 |         if (!pci_root_ports_have_device(pci->pp.bridge->bus))
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1134 |                 goto stop_link;
         |                 ~~~~~~~~~~~~~~
   drivers/pci/controller/dwc/pcie-designware-host.c:1131:9: note: initialize the variable 'ret' to silence this warning
    1131 |         int ret;
         |                ^
         |                 = 0
   1 warning generated.


vim +1133 drivers/pci/controller/dwc/pcie-designware-host.c

  1126	
  1127	int dw_pcie_suspend_noirq(struct dw_pcie *pci)
  1128	{
  1129		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
  1130		u32 val;
  1131		int ret;
  1132	
> 1133		if (!pci_root_ports_have_device(pci->pp.bridge->bus))
  1134			goto stop_link;
  1135	
  1136		/*
  1137		 * If L1SS is supported, then do not put the link into L2 as some
  1138		 * devices such as NVMe expect low resume latency.
  1139		 */
  1140		if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
  1141			return 0;
  1142	
  1143		if (pci->pp.ops->pme_turn_off) {
  1144			pci->pp.ops->pme_turn_off(&pci->pp);
  1145		} else {
  1146			ret = dw_pcie_pme_turn_off(pci);
  1147			if (ret)
  1148				return ret;
  1149		}
  1150	
  1151		ret = read_poll_timeout(dw_pcie_get_ltssm, val,
  1152					val == DW_PCIE_LTSSM_L2_IDLE ||
  1153					val <= DW_PCIE_LTSSM_DETECT_WAIT,
  1154					PCIE_PME_TO_L2_TIMEOUT_US/10,
  1155					PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
  1156		if (ret) {
  1157			/* Only log message when LTSSM isn't in DETECT or POLL */
  1158			dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM: 0x%x\n", val);
  1159			return ret;
  1160		}
  1161	
  1162		/*
  1163		 * Per PCIe r6.0, sec 5.3.3.2.1, software should wait at least
  1164		 * 100ns after L2/L3 Ready before turning off refclock and
  1165		 * main power. This is harmless when no endpoint is connected.
  1166		 */
  1167		udelay(1);
  1168	
  1169	stop_link:
  1170		dw_pcie_stop_link(pci);
  1171		if (pci->pp.ops->deinit)
  1172			pci->pp.ops->deinit(&pci->pp);
  1173	
  1174		pci->suspended = true;
  1175	
  1176		return ret;
  1177	}
  1178	EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq);
  1179	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki