-----Original Message-----
From: Bjorn Helgaas <helgaas@kernel.org>
Sent: 2025年8月18日 23:49
To: Hongxing Zhu <hongxing.zhu@nxp.com>
Cc: Frank Li <frank.li@nxp.com>; jingoohan1@gmail.com;
l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
mani@kernel.org; robh@kernel.org; bhelgaas@google.com;
shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
festevam@gmail.com; linux-pci@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev;
linux-kernel@vger.kernel.org
Subject: Re: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if
QUIRK_NOL2POLL_IN_PM is existing in suspend

On Mon, Aug 18, 2025 at 03:32:01PM +0800, Richard Zhu wrote:

quoted

Refer to PCIe r6.0, sec 5.2, fig 5-1 Link Power Management State Flow
Diagram. Both L0 and L2/L3 Ready can be transferred to LDn directly.

It's harmless to let dw_pcie_suspend_noirq() proceed suspend after the
PME_Turn_Off is sent out, whatever the LTSSM state is in L2 or L3
after a recommended 10ms max wait refer to PCIe r6.0, sec 5.3.3.2.1
PME Synchronization.

The LTSSM states are inaccessible on i.MX6QP and i.MX7D after the
PME_Turn_Off is sent out.

To support this case, don't poll L2 state and apply a simple delay of
PCIE_PME_TO_L2_TIMEOUT_US(10ms) if the QUIRK_NOL2POLL_IN_PM flag

is

quoted

set in suspend.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 31
+++++++++++++------  drivers/pci/controller/dwc/pcie-designware.h  |
4 +++
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c

b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b5012..20a7f827babbf 100644

--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c

@@ -1007,7 +1007,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)

{
 	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	u32 val;
-	int ret;
+	int ret = 0;

      /*
       * If L1SS is supported, then do not put the link into L2 as
some

         * devices such as NVMe expect low resume latency.
         */
         if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) &
PCI_EXP_LNKCTL_ASPM_L1)
                return 0;

You didn't change it in this patch (the L1SS test was added by
4774faf854f5 ("PCI: dwc: Implement generic suspend/resume functionality")),
but this L1SS check is an encapsulation problem.
The ASPM configuration shouldn't leak out here in such an ad hoc way.

Hi Bjorn:
Thanks for your comments.
Should I created another commit to get ride of the L1SS check codes?

*All* drivers, not just NVMe, would prefer low resume latency.

How do we deal with this in other host controller drivers?  If any other
driver puts links in L2, I suppose they would have the same issue?  Maybe
DWC is the only one that puts the link in L2?

What happens when we add a new driver that puts links in L2?  I guess
we'll be debugging some NVMe issue again?

Up to now, this is the first one routine to do the L1SS check in L2 entry.

Best Regards
Richard Zhu

Bjorn