> -----Original Message----- > From: manivannan.sadhasivam@xxxxxxxxxx > <manivannan.sadhasivam@xxxxxxxxxx> > Sent: 2025年6月1日 1:47 > To: Hongxing Zhu <hongxing.zhu@xxxxxxx> > Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx>; Krishna Chaitanya Chundru > <quic_krichai@xxxxxxxxxxx>; jingoohan1@xxxxxxxxx; Frank Li > <frank.li@xxxxxxx>; l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx; > kw@xxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx; > shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; > festevam@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx > 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: > > > -----Original Message----- > > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx> > > > Sent: 2025年4月8日 22:59 > > > To: Hongxing Zhu <hongxing.zhu@xxxxxxx> > > > Cc: jingoohan1@xxxxxxxxx; Frank Li <frank.li@xxxxxxx>; > > > l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx; > > > manivannan.sadhasivam@xxxxxxxxxx; robh@xxxxxxxxxx; > > > bhelgaas@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > > > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; > > > linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > > imx@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > > > 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: > > > > 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. > > > > > > 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 > > > om%7C59dcbf2632b340e748bb08dda06b34e1%7C686ea1d3bc2b4c6fa92cd99c > 5c3016 > > > 35%7C0%7C0%7C638843104499682928%7CUnknown%7CTWFpbGZsb3d8eyJFb > XB0eU1hcG > > > kiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoy > > > fQ%3D%3D%7C0%7C%7C%7C&sdata=eEXSd14o6XYHC0ww06NaFH2L7Pr%2FgT > P2Dxr77xd5 > > 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. > > > 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 > > > s/ltssm/LTSSM/ > > Okay. > > > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > > @@ -947,7 +947,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 @@ > > > > -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", > > > val); > > > > - 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); > > > > + 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 > > -- > மணிவண்ணன் சதாசிவம்
