[PATCH] PCI: qcom: Prevent GDSC power down on suspend

[PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Krishna Chaitanya Chundru]

Currently, the driver expects the devices to remain in D0 across system
suspend, but the genpd framework may still power down the associated
GDSC during suspend. When that happens, the PCIe link goes down and
cannot be recovered on resume.

Prevent genpd from turning off the PCIe GDSC by using
dev_pm_genpd_rpm_always_on() so that the power domain stays on while
the controller is suspended. This preserves the link state across
suspend/resume and avoids unrecoverable link failures.

Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5a318487b2b3f6c61d8f5b1fd5cdf2738a1f1dcd..314cf334a313dff35efaf0c023597e6eef483925 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -25,6 +25,7 @@
 #include <linux/pci.h>
 #include <linux/pci-ecam.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/phy/pcie.h>
@@ -2052,6 +2053,11 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
 		pcie->suspended = true;
 	}
 
+	if (pcie->suspended)
+		dev_pm_genpd_rpm_always_on(dev, false);
+	else
+		dev_pm_genpd_rpm_always_on(dev, true);
+
 	/*
 	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
 	 * Because on some platforms, DBI access can happen very late during the

---
base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
change-id: 20260128-genpd_fix-3aa413d9a383

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Konrad Dybcio]

On 1/28/26 1:22 PM, Krishna Chaitanya Chundru wrote:
> Currently, the driver expects the devices to remain in D0 across system
> suspend, but the genpd framework may still power down the associated
> GDSC during suspend. When that happens, the PCIe link goes down and
> cannot be recovered on resume.
> 
> Prevent genpd from turning off the PCIe GDSC by using
> dev_pm_genpd_rpm_always_on() so that the power domain stays on while
> the controller is suspended. This preserves the link state across
> suspend/resume and avoids unrecoverable link failures.
> 
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---

How does this play along with your D3Cold series?

Is this patch supposed to be applied first?

Konrad

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Bjorn Andersson]

On Wed, Jan 28, 2026 at 05:52:42PM +0530, Krishna Chaitanya Chundru wrote:
> Currently, the driver expects the devices to remain in D0 across system
> suspend, but the genpd framework may still power down the associated
> GDSC during suspend. When that happens, the PCIe link goes down and
> cannot be recovered on resume.
> 

The GDSC is a child of CX, so by keeping it always-on, you effectively
put an always-on vote on CX, forever preventing CXPC.

In fact, this is one of the reasons why the PCIe GDSCs on most targets
is marked PWRSTS_RET_ON (in the clock driver) so that the "off state"
doesn't actually turn off the GDSC, but it relinquishes the inherited
vote on CX.

> Prevent genpd from turning off the PCIe GDSC by using
> dev_pm_genpd_rpm_always_on() so that the power domain stays on while
> the controller is suspended. This preserves the link state across
> suspend/resume and avoids unrecoverable link failures.
> 

We are able to suspend/resume a whole bunch of platforms today, which
one are you on?

That said, while we can suspend/resume, we're not allowing CXPC today.
On many systems the main culprit is the icc_set_bw() vote in
qcom_pcie_suspend_noirq().

Regards,
Bjorn

> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5a318487b2b3f6c61d8f5b1fd5cdf2738a1f1dcd..314cf334a313dff35efaf0c023597e6eef483925 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -25,6 +25,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci-ecam.h>
>  #include <linux/pm_opp.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy/pcie.h>
> @@ -2052,6 +2053,11 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  		pcie->suspended = true;
>  	}
>  
> +	if (pcie->suspended)
> +		dev_pm_genpd_rpm_always_on(dev, false);
> +	else
> +		dev_pm_genpd_rpm_always_on(dev, true);
> +
>  	/*
>  	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
>  	 * Because on some platforms, DBI access can happen very late during the
> 
> ---
> base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
> change-id: 20260128-genpd_fix-3aa413d9a383
> 
> Best regards,
> -- 
> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> 
> 

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Manivannan Sadhasivam]

On Wed, Jan 28, 2026 at 08:13:48AM -0600, Bjorn Andersson wrote:
> On Wed, Jan 28, 2026 at 05:52:42PM +0530, Krishna Chaitanya Chundru wrote:
> > Currently, the driver expects the devices to remain in D0 across system
> > suspend, but the genpd framework may still power down the associated
> > GDSC during suspend. When that happens, the PCIe link goes down and
> > cannot be recovered on resume.
> > 
> 
> The GDSC is a child of CX, so by keeping it always-on, you effectively
> put an always-on vote on CX, forever preventing CXPC.
> 
> In fact, this is one of the reasons why the PCIe GDSCs on most targets
> is marked PWRSTS_RET_ON (in the clock driver) so that the "off state"
> doesn't actually turn off the GDSC, but it relinquishes the inherited
> vote on CX.
> 

So this means, you favor the patch that marks the PCIe GDSCs as PWRSTS_RET_ON?

> > Prevent genpd from turning off the PCIe GDSC by using
> > dev_pm_genpd_rpm_always_on() so that the power domain stays on while
> > the controller is suspended. This preserves the link state across
> > suspend/resume and avoids unrecoverable link failures.
> > 
> 
> We are able to suspend/resume a whole bunch of platforms today, which
> one are you on?
> 
> That said, while we can suspend/resume, we're not allowing CXPC today.
> On many systems the main culprit is the icc_set_bw() vote in
> qcom_pcie_suspend_noirq().
> 

Yeah, I still need to look deeply into this part. The stray vote keeps the PCIe
link active as irq core tries to masks the MSIs at the very end of suspend. This
design works fine for firmware controlled suspends, but not for kernel
controlled ones.

- Mani

> Regards,
> Bjorn
> 
> > Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 5a318487b2b3f6c61d8f5b1fd5cdf2738a1f1dcd..314cf334a313dff35efaf0c023597e6eef483925 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/pci.h>
> >  #include <linux/pci-ecam.h>
> >  #include <linux/pm_opp.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/phy/pcie.h>
> > @@ -2052,6 +2053,11 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
> >  		pcie->suspended = true;
> >  	}
> >  
> > +	if (pcie->suspended)
> > +		dev_pm_genpd_rpm_always_on(dev, false);
> > +	else
> > +		dev_pm_genpd_rpm_always_on(dev, true);
> > +
> >  	/*
> >  	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
> >  	 * Because on some platforms, DBI access can happen very late during the
> > 
> > ---
> > base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
> > change-id: 20260128-genpd_fix-3aa413d9a383
> > 
> > Best regards,
> > -- 
> > Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > 
> > 

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

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Jagadeesh Kona]

On 2/18/2026 6:03 PM, Manivannan Sadhasivam wrote:
> On Wed, Jan 28, 2026 at 08:13:48AM -0600, Bjorn Andersson wrote:
>> On Wed, Jan 28, 2026 at 05:52:42PM +0530, Krishna Chaitanya Chundru wrote:
>>> Currently, the driver expects the devices to remain in D0 across system
>>> suspend, but the genpd framework may still power down the associated
>>> GDSC during suspend. When that happens, the PCIe link goes down and
>>> cannot be recovered on resume.
>>>
>>
>> The GDSC is a child of CX, so by keeping it always-on, you effectively
>> put an always-on vote on CX, forever preventing CXPC.
>>
>> In fact, this is one of the reasons why the PCIe GDSCs on most targets
>> is marked PWRSTS_RET_ON (in the clock driver) so that the "off state"
>> doesn't actually turn off the GDSC, but it relinquishes the inherited
>> vote on CX.
>>
> 

Hi Bjorn,

USB host-mode and PCIe non-D3cold use cases require their respective GDSCs
to remain enabled during system suspend. This requirement exists on multiple
targets and is expected to apply to additional targets as well.

The affected GDSCs currently use PWRSTS_RET_ON flag. However, this prevents
the GDSC driver from disabling the GDSC hardware after the first enable, even
when all consumers have become inactive. As a result, the GDSC remains powered
ON unnecessarily.

We propose using the GenPD synced_poweroff flag instead. When synced_poweroff
is set, the GDSC can be disabled during suspend. When it is not set, the GDSC
remains enabled to support consumers that require it across suspend. Consumer
drivers can set this flag using dev_pm_genpd_synced_poweroff() based on their
usecase.

For the affected USB and PCIe GDSCs, this could be implemented using a poweroff
callback as below in gdsc driver:

int gdsc_synced_poweroff_disable(struct generic_pm_domain *domain)
 {
         struct gdsc *sc = domain_to_gdsc(domain);

         /* Disable GDSC when synced_poweroff is set */
         if (domain->synced_poweroff)
                 return gdsc_toggle_logic(sc, GDSC_OFF, false);

         /* Dont disable GDSC in HW when synced_poweroff is not set */
         if (sc->rsupply)
                 return regulator_disable(sc->rsupply);

         return 0;
 }

This would allow the GDSC to remain enabled only when required, while permitting
it to be powered down for other use cases.

Please let us know your comments and suggestions on this approach.

Thanks,
Jagadeesh

Re: [PATCH] PCI: qcom: Prevent GDSC power down on suspend

[Jagadeesh Kona]

On 9/25/2026 10:29 AM, Jagadeesh Kona wrote:
> 
> 
> On 2/18/2026 6:03 PM, Manivannan Sadhasivam wrote:
>> On Wed, Jan 28, 2026 at 08:13:48AM -0600, Bjorn Andersson wrote:
>>> On Wed, Jan 28, 2026 at 05:52:42PM +0530, Krishna Chaitanya Chundru wrote:
>>>> Currently, the driver expects the devices to remain in D0 across system
>>>> suspend, but the genpd framework may still power down the associated
>>>> GDSC during suspend. When that happens, the PCIe link goes down and
>>>> cannot be recovered on resume.
>>>>
>>>
>>> The GDSC is a child of CX, so by keeping it always-on, you effectively
>>> put an always-on vote on CX, forever preventing CXPC.
>>>
>>> In fact, this is one of the reasons why the PCIe GDSCs on most targets
>>> is marked PWRSTS_RET_ON (in the clock driver) so that the "off state"
>>> doesn't actually turn off the GDSC, but it relinquishes the inherited
>>> vote on CX.
>>>
>>
> 
> Hi Bjorn,
> 
> USB host-mode and PCIe non-D3cold use cases require their respective GDSCs
> to remain enabled during system suspend. This requirement exists on multiple
> targets and is expected to apply to additional targets as well.
> 
> The affected GDSCs currently use PWRSTS_RET_ON flag. However, this prevents
> the GDSC driver from disabling the GDSC hardware after the first enable, even
> when all consumers have become inactive. As a result, the GDSC remains powered
> ON unnecessarily.
> 
> We propose using the GenPD synced_poweroff flag instead. When synced_poweroff
> is set, the GDSC can be disabled during suspend. When it is not set, the GDSC
> remains enabled to support consumers that require it across suspend. Consumer
> drivers can set this flag using dev_pm_genpd_synced_poweroff() based on their
> usecase.
> 
> For the affected USB and PCIe GDSCs, this could be implemented using a poweroff
> callback as below in gdsc driver:
> 
> int gdsc_synced_poweroff_disable(struct generic_pm_domain *domain)
>  {
>          struct gdsc *sc = domain_to_gdsc(domain);
> 
>          /* Disable GDSC when synced_poweroff is set */
>          if (domain->synced_poweroff)
>                  return gdsc_toggle_logic(sc, GDSC_OFF, false);
> 
>          /* Dont disable GDSC in HW when synced_poweroff is not set */
>          if (sc->rsupply)
>                  return regulator_disable(sc->rsupply);
> 
>          return 0;
>  }
> 
> This would allow the GDSC to remain enabled only when required, while permitting
> it to be powered down for other use cases.
> 
> Please let us know your comments and suggestions on this approach.
> 

Adding some more details on the GenPD synced_poweroff flag and the corresponding consumer
driver changes with this approach.

The GenPD framework automatically clears GenPD's synced_poweroff flag on every GenPD
power-on operation [1].

Consumer drivers (e.g. PCIe/USB) can invoke dev_pm_genpd_synced_poweroff(dev) in their
suspend path when the GDSC needs to be turned off in hardware. In that case, the GDSC
driver will proceed with disabling the GDSC.

If a consumer driver requires the GDSC to remain on across suspend, it can simply avoid
calling dev_pm_genpd_synced_poweroff() in its suspend path. The GDSC driver will then
keep the GDSC enabled in hardware while still allowing the parent CX rail to enter CXPC.

This approach provides more flexibility to consumer drivers, allowing them to keep the
GDSC enabled only when required and power it off when it is not needed.

Please find the example code in PCIE consumer driver below with this new approach:

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ee63a6ec99de..25ff8651fe91 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -27,6 +27,7 @@
 #include <linux/pci-ecam.h>
 #include <linux/pci-pwrctrl.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/phy/pcie.h>
@@ -2436,6 +2437,8 @@ static int qcom_pcie_suspend_noirq(struct device *dev)


         if (pcie->pci->suspended) {
                 ret = icc_disable(pcie->icc_mem);
                 if (ret)
                         dev_err(dev, "Failed to disable PCIe-MEM interconnect path: %d\n", ret);

                 ret = icc_disable(pcie->icc_cpu);
                 if (ret)
                         dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n", ret);
                 
                 if (pcie->use_pm_opp)
                        dev_pm_opp_set_opp(pcie->pci->dev, NULL);
+
+               dev_pm_genpd_synced_poweroff(dev); /* Invoke GenPD synced poweroff to disable GDSC in HW */
        } else {

Please let us know your feedback or require any additional information.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pmdomain/core.c#n919

Thanks,
Jagadeesh