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

[Jagadeesh Kona <jagadeesh.kona@oss.qualcomm.com>]

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