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

> -----Original Message-----
> From: Bjorn Helgaas <[email protected]>
> Sent: Wednesday, August 5, 2026 2:19 AM
> To: Hongxing Zhu (OSS) <[email protected]>
> Cc: Manivannan Sadhasivam <[email protected]>; Frank Li <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; Hongxing Zhu <[email protected]>
> Subject: Re: [PATCH v2] PCI: dwc: Add suspend_poweroff flag for platforms with
> RC power loss
> 
> On Tue, Aug 04, 2026 at 06:32:26AM +0000, Hongxing Zhu (OSS) wrote:
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <[email protected]>
> > > Sent: Monday, August 3, 2026 11:33 PM
> > ...
> 
> > > 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 <[email protected]>
> > > > > > Sent: Thursday, July 30, 2026 6:30 AM
> > > > > ...
> > > >
> > > > > > On Fri, Jul 17, 2026 at 03:41:21PM +0800,
> > > > > > [email protected]
> > > wrote:
> > > > > > > From: Richard Zhu <[email protected]>
> > > > > > >
> > > > > > > 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:
> 
> I'm not suggesting using pci_suspend_retains_context() in
> dw_pcie_suspend_noirq().  pci_suspend_retains_context() is for endpoint
> drivers.
> 
> I think pci_suspend_retains_context() should return 'false' on any platform
> where dw_pcie_suspend_noirq() is going to send PME_Turn_Off.
> Sending PME_Turn_Off means we intend to remove main power.  When main
> power is removed, devices will be in D3cold and they will not retain their
> internal context.
> 
> If pci_suspend_retains_context() returns 'false' and
> pci_host_common_d3cold_possible() returns also returns 'false', it means some
> driver didn't put its device in D3hot when it should have.
> 
> In that case, I think you should either fix that driver so it *does* put the device
> in D3hot (based on pci_suspend_retains_context()), or just accept that you can't
> power off the RC.
Understood, thanks for your explanation.

Best Regards
Richard Zhu