RE: [PATCH v2] PCI: dwc: Add suspend_poweroff flag for platforms with RC power loss

[Hongxing Zhu (OSS)]

> -----Original Message-----
> From: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> Sent: Monday, August 3, 2026 11:33 PM
> To: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Cc: Hongxing Zhu (OSS) <hongxing.zhu@xxxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx;
> kwilczynski@xxxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-
> pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Hongxing Zhu <hongxing.zhu@xxxxxxx>
> Subject: Re: [PATCH v2] PCI: dwc: Add suspend_poweroff flag for platforms with
> RC power loss
> 
> On Thu, Jul 30, 2026 at 07:09:13AM -0500, Bjorn Helgaas wrote:
> > On Thu, Jul 30, 2026 at 08:16:36AM +0000, Hongxing Zhu (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > > > Sent: Thursday, July 30, 2026 6:30 AM
> > > ...
> >
> > > > On Fri, Jul 17, 2026 at 03:41:21PM +0800, hongxing.zhu@xxxxxxxxxxx
> wrote:
> > > > > From: Richard Zhu <hongxing.zhu@xxxxxxx>
> > > > >
> > > > > Some platforms like i.MX power off their PCIe RC controllers
> > > > > during system suspend, requiring full re-initialization on
> > > > > resume. These platforms need to enter L2 state to properly
> > > > > notify endpoints before power loss.
> > > > >
> > > > > According to PCIe base spec r7.0, sec 5.2, the system software
> > > > > should transition the device into D3Hot before broadcasting the
> > > > > PME_Turn_Off message to initiate L2 entry. However, some
> > > > > endpoint devices fail the D3cold capability check in
> > > > > pci_host_common_d3cold_possible(), which would normally prevent L2
> entry.
> > > >
> > > > Wakeup devices that don't support PME from D3cold will fail the
> > > > D3cold capability check, but I don't think those are the problem
> > > > you're solving.
> > > >
> > > You're right.
> > >
> > > > This appears to handle devices that are not in D3hot, and that's
> > > > not a property of the endpoint; it's a property of its driver.  Is
> > > > the problem here that some driver didn't put its device in D3hot?
> > > >
> > > I observed some endpoint devices failing the D3cold capability
> > > check, but I haven't identified the root cause.
> >
> > Seems like some instrumentation in pci_host_common_d3cold_possible()
> > could tell us which devices/drivers are relevant.
> >
> > > > > For platforms where the RC loses power during suspend, L2 entry
> > > > > is essential regardless of D3cold support, as the link will be
> > > > > lost anyway.  Add a suspend_poweroff flag to force L2 entry in
> > > > > such cases, and enable it for i.MX PCIe controllers.
> > > > >
> > > > > Note: This violates the spec requirement that devices be in
> > > > > D3Hot before PME_Turn_Off, but is necessary for proper operation
> > > > > on platforms with RC power loss during suspend.
> > > >
> > > > If the device isn't in D3hot, it may still be active, and I think
> > > > the PME_Turn_Off will abort any DMAs in progress, which doesn't
> > > > sound like proper operation of the endpoint.
> > >
> > > You're correct - this isn't ideal for endpoint operation. However,
> > > on i.MX platforms, the RC controller will lose power during system
> > > suspend regardless. Without sending PME_Turn_Off, endpoints would
> > > have no notification before the link abruptly goes down.
> >
> > The PME_Turn_Off is some notification, but I don't think the endpoint
> > can do DMA or MSI, so its driver won't be involved.  It may be able to
> > do things internally, e.g., write caches to an SSD or transmit packets
> > from its internal buffers.  But I think it might still lose power in
> > the middle of operations like that, so it doesn't sound reliable.
> >
> > > This patch adds a flag to force L2 entry for such platforms,
> > > choosing to notify endpoints (even if not ideal) rather than having
> > > the link drop unexpectedly. I'm appreciated for suggestions if
> > > there's a better approach to handle this scenario.
> >
> > Mani recently added pci_suspend_retains_context(), and I think it
> > returns "true" on i.MX.  But if the RC is powered off and all the
> > devices need full re-initialization on resume, it sounds like it
> > *should* return "false".
> 
> Right. pci_suspend_retains_context() was added just for that purpose of telling
> client drivers to prepare for context loss. So if this API returns 'false', then they
> should not hold the device in D0.
Thanks for your comments.

If I update dw_pcie_suspend_noirq() to use pci_suspend_retains_context() to
determine whether to proceed with L2 entry and RC power down. The new logic
would be changed as below:

-       if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
+       list_for_each_entry(pdev, &bridge->bus->devices, bus_list) {
+               context_retain &= pci_suspend_retains_context(pdev);
+               if (!context_retain)
+                       break;
+       }
+
+       d3cold = pci_host_common_d3cold_possible(bridge, &pme_capable);
+       /*
+        * Enter L2 if:
+        * 1. D3cold is supported (normal case), OR
+        * 2. Any device doesn't retain context during suspend
+        *    (indicates RC power loss, requiring L2 entry to notify
+        *    endpoints via PME_Turn_Off before power is lost)
+        */
+       if (!d3cold && context_retain)
                return 0;

This means:

If context is NOT retained (pci_suspend_retains_context() returns false), the
function will proceed with L2 entry.
If context IS retained (returns true) AND D3cold is not possible, it will skip
suspend operations.
For i.MX: pci_suspend_retains_context() returns false (since i.MX PCIe RC
loses power), the RC will properly enter L2 and power down.

So do Qcom PCIe driver(pci_suspend_retains_context() returns false as well).

Is this approach acceptable for Qcom?

Best Regards
Richard Zhu
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்