-----Original Message-----
From: manivannan.sadhasivam@linaro.org
[off-list ref]
Sent: 2025年6月1日 1:47
To: Hongxing Zhu <hongxing.zhu@nxp.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>; Krishna Chaitanya Chundru
[off-list ref]; jingoohan1@gmail.com; Frank Li
[off-list ref]; l.stach@pengutronix.de; lpieralisi@kernel.org;
kw@linux.com; 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: [PATCH v1 1/4] PCI: dwc: Add quirk to fix hang issue in L2 poll of
suspend

On Wed, Apr 09, 2025 at 02:31:22AM +0000, Hongxing Zhu wrote:

quoted

quoted

-----Original Message-----
From: Bjorn Helgaas <helgaas@kernel.org>
Sent: 2025年4月8日 22:59
To: Hongxing Zhu <hongxing.zhu@nxp.com>
Cc: jingoohan1@gmail.com; Frank Li <frank.li@nxp.com>;
l.stach@pengutronix.de; lpieralisi@kernel.org; kw@linux.com;
manivannan.sadhasivam@linaro.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: [PATCH v1 1/4] PCI: dwc: Add quirk to fix hang issue in
L2 poll of suspend

On Tue, Apr 08, 2025 at 02:52:18PM +0800, Richard Zhu wrote:

quoted

i.MX6QP PCIe is hang in L2 poll during suspend when one endpoint
device is connected, for example the Intel e1000e network card.

Refer to Figure5-1 Link Power Management State Flow Diagram of PCI
Express Base Spec Rev6.0. L0 can be transferred to LDn directly.

Please include the section number.  Section numbers are easy to find
because they're in the spec PDF contents, but figures are not.
E.g., "PCIe r6.0, sec 5.2, fig 5-1"

Okay, would add them later.

quoted

quoted

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 on some PME_Turn_Off handshake broken platforms.

Maybe we don't need to poll for these LTSSM states on *any*
platform, and we could just remove the poll and timeout completely?

Yes, I used to suggest remove the L2 poll and timeout in the following
discussion.
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml
.org%2Flkml%2F2024%2F11%2F18%2F200&data=05%7C02%7Chongxing.zhu

%40nxp.c

quoted

om%7C59dcbf2632b340e748bb08dda06b34e1%7C686ea1d3bc2b4c6fa92cd99c
5c3016

quoted

35%7C0%7C0%7C638843104499682928%7CUnknown%7CTWFpbGZsb3d8eyJFb
XB0eU1hcG

quoted

kiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoy

quoted

fQ%3D%3D%7C0%7C%7C%7C&sdata=eEXSd14o6XYHC0ww06NaFH2L7Pr%2FgT
P2Dxr77xd5

quoted

Zp0%3D&reserved=0
Hi Krishna:
Is it feasible to eliminate the L2 poll and timeout in this context?

Spec r6.0, sec 5.2 mandates L2/L3 Ready state:

"L2/L3 Ready transition protocol support is required."

Also in many places, it suggests waiting for L2/L3 Ready state before powering
down the device. So I don't think we should just remove the poll for all
platforms.

Hi Mani:
Thanks for your review. 
For some L2 POLL broken platforms (e.x i.MX6QP PCIe). Can we just wait
 for a enough long time here without the L2/L3 state polling?
Because that a recommended 10ms max wait refer to PCIe r6.0, sec 5.3.3.2.1
 PME Synchronization.

quoted

quoted

If not, we need to explain why it is safe to skip the poll on some platforms.
"Skipping the poll avoids a hang" is not a sufficient explanation.

So the issue is that the device doesn't transition to L2/L3 Ready state and the
host platform just 'hangs'? Do we know why the hang happens?

I can’t find the exact root cause of hand after I looked through the
 trace log captured by the protocol analyzer. Without the L2 poll by SW,
 the PM_Enter_L23 DLLP(R<-) and PM_Request_Ack  DLLP (R->) can be
 captured after PME_Turn_Off is kicked off, and PME_TO_Ack is received.

Since the designer of the i.MX6Q PCIe can't be contacted anymore. I don't
konw what's going on when do the L2 POLL. I suspect that the LTSSM_STATE 
registers are not accessible anymore after the PM_Enter_L23/PM_Request_Ack
 handshake on i.MX6Q PCIe.

Best Regards
Richard Zhu

quoted

quoted

s/ltssm/LTSSM/

Okay.

quoted

quoted

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

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

{

quoted

quoted

quoted

 	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 @@
-964,15 +964,17 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 			return ret;
 	}

-	ret = read_poll_timeout(dw_pcie_get_ltssm, val,
-				val == DW_PCIE_LTSSM_L2_IDLE ||
-				val <= DW_PCIE_LTSSM_DETECT_WAIT,
-				PCIE_PME_TO_L2_TIMEOUT_US/10,
-				PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
-	if (ret) {
-		/* Only log message when LTSSM isn't in DETECT or POLL */
-		dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM:

0x%x\n",

quoted

quoted

val);

quoted

-		return ret;
+	if (!dwc_check_quirk(pci, QUIRK_NOL2POLL_IN_PM)) {
+		ret = read_poll_timeout(dw_pcie_get_ltssm, val,
+					val == DW_PCIE_LTSSM_L2_IDLE ||
+					val <= DW_PCIE_LTSSM_DETECT_WAIT,
+					PCIE_PME_TO_L2_TIMEOUT_US/10,
+					PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
+		if (ret) {
+			/* Only log message when LTSSM isn't in DETECT or POLL */
+			dev_err(pci->dev, "Timeout waiting for L2 entry! LTSSM:

0x%x\n", val);

quoted

+			return ret;
+		}
 	}

 	/*

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

b/drivers/pci/controller/dwc/pcie-designware.h
index 56aafdbcdaca..05fe654d7761 100644

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

@@ -282,6 +282,9 @@
 /* Default eDMA LLP memory size */
 #define DMA_LLP_MEM_SIZE		PAGE_SIZE

+#define QUIRK_NOL2POLL_IN_PM		BIT(0)
+#define dwc_check_quirk(pci, val)	(pci->quirk_flag & val)

Maybe just my personal preference, but I don't like things named "check"
because that just means "look at"; it doesn't give any hint about
how to interpret the result of looking at it.

How about dwc_match_quirk(pci, val) (pci->quirk_flag & val)?

Maybe just dwc_quirk()?

- Mani

--
மணிவண்ணன் சதாசிவம்