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

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Thursday, July 30, 2026 6:30 AM
> To: Hongxing Zhu (OSS) <hongxing.zhu@xxxxxxxxxxx>
> Cc: Frank Li <frank.li@xxxxxxx>; l.stach@xxxxxxxxxxxxxx;
> lpieralisi@xxxxxxxxxx; kwilczynski@xxxxxxxxxx; mani@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 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.
> 
> 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?
> 
> > 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.
Hi Bjorn,

I apologize for not addressing your concerns promptly. Let me clarify the
issue after reviewing this more carefully.

The problem I'm addressing:

L2 entry is being blocked for wakeup-capable devices that fail the D3cold
capability check in `pci_host_common_d3cold_possible()`, even when the device
is already in D3hot state.

Specifically:
- The endpoint device is in D3hot (the driver has done its job correctly)
- However, because it's a wakeup device that doesn't support PME from D3cold,
  it fails the D3cold capability check
- This failure currently prevents L2 entry, even though the device is already
  in the required D3hot state as per the spec
- For platforms like i.MX that power off the RC during suspend, we need L2
  entry to properly notify the endpoint before power loss and complete
  reinitialization successfully on resume.

So, the issue isn't about forcing devices into D3hot or handling devices that
aren't in D3hot - it's about allowing L2 entry for devices that are already
in D3hot but happen to fail the D3cold capability check due to wakeup
requirements.

Regarding your concern about the commit message:

> "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 right. The commit message note about "violating the spec
requirement" is misleading and incorrect. The endpoint device is in D3hot (the
driver has done its job correctly), so it's compliant with the spec
requirement. I should remove or reword that note in the next version.

Would you mind if I resubmitted the patch and used clearer submission
information to accurately describe this situation?

Thank you for your patience and guidance!

Best Regards,
Richard Zhu
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > ---
> > Changes in v2:
> > - Rename the new added flag.
> > - Update commit description and comments.
> >
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c             | 11 +++++++++++
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++--
> >  drivers/pci/controller/dwc/pcie-designware.h      |  8 ++++++++
> >  3 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 98e1db751132a..5a43bf396521b 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1957,6 +1957,17 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> >  		 */
> >  		imx_pcie_add_lut_by_rid(imx_pcie, 0);
> >  	} else {
> > +		/*
> > +		 * i.MX RC is powered off during suspend, requiring L2 entry
> > +		 * to notify endpoints before power loss. Some endpoints fail
> > +		 * the D3cold capability check, which would normally prevent
> > +		 * L2 entry. Force L2 entry anyway since the link will be lost
> > +		 * when RC powers off.
> > +		 *
> > +		 * Note: Per PCIe spec r7.0 sec 5.2, devices should be in
> D3Hot
> > +		 * before PME_Turn_Off. This may not be guaranteed here.
> > +		 */
> > +		pci->pp.suspend_poweroff = true;
> >  		if (imx_check_flag(imx_pcie,
> IMX_PCIE_FLAG_SKIP_L23_READY))
> >  			pci->pp.skip_l23_ready = true;
> >  		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_KEEP_MSI_CAP))
> diff
> > --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 06722259d2e37..95a31d65e1ca9 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1222,14 +1222,20 @@ static int dw_pcie_pme_turn_off(struct
> dw_pcie
> > *pci)
> >
> >  int dw_pcie_suspend_noirq(struct dw_pcie *pci)  {
> > -	bool pme_capable = false;
> > +	bool d3cold, pme_capable = false;
> >  	int ret = 0;
> >  	u32 val;
> >
> >  	if (!dw_pcie_link_up(pci))
> >  		goto stop_link;
> >
> > -	if (!pci_host_common_d3cold_possible(pci->pp.bridge,
> &pme_capable))
> > +	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge,
> &pme_capable);
> > +	/*
> > +	 * Enter L2 if D3cold is possible, or if the platform loses RC power
> > +	 * during suspend (in which case L2 entry is required to notify
> > +	 * endpoints before power loss, regardless of D3cold support).
> > +	 */
> > +	if (!d3cold && !pci->pp.suspend_poweroff)
> >  		return 0;
> >
> >  	if (pci->pp.ops->pme_turn_off) {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index de4b245b1758c..3ce7d30d9a4d5 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -471,6 +471,14 @@ struct dw_pcie_rp {
> >  	bool			native_ecam;
> >  	bool                    skip_l23_ready;
> >  	bool			skip_pwrctrl_off;
> > +	/*
> > +	 * suspend_poweroff: Force L2 entry during suspend even when
> D3cold
> > +	 * capability check fails. Used on platforms where RC loses power
> > +	 * during suspend and must notify endpoints via L2 entry before
> > +	 * power loss. May not meet PCIe spec requirement for devices to be
> > +	 * in D3Hot before PME_Turn_Off.
> > +	 */
> > +	bool                    suspend_poweroff;
> >  };
> >
> >  struct dw_pcie_ep_ops {
> > --
> > 2.34.1
> >