[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<AS8PR04MB88335CEA526C3B5E957EF0C88C12A@AS8PR04MB8833.eurprd04.prod.outlook.com>
Date: Mon, 22 Sep 2025 09:18:20 +0000
From: Hongxing Zhu <hongxing.zhu@....com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: Frank Li <frank.li@....com>, "jingoohan1@...il.com"
<jingoohan1@...il.com>, "l.stach@...gutronix.de" <l.stach@...gutronix.de>,
"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "kwilczynski@...nel.org"
<kwilczynski@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "shawnguo@...nel.org"
<shawnguo@...nel.org>, "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>, "festevam@...il.com"
<festevam@...il.com>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "imx@...ts.linux.dev"
<imx@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v5 2/6] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is
existing in suspend
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@...nel.org>
> Sent: 2025年9月20日 14:29
> To: Hongxing Zhu <hongxing.zhu@....com>
> Cc: Frank Li <frank.li@....com>; jingoohan1@...il.com;
> l.stach@...gutronix.de; lpieralisi@...nel.org; kwilczynski@...nel.org;
> robh@...nel.org; bhelgaas@...gle.com; shawnguo@...nel.org;
> s.hauer@...gutronix.de; kernel@...gutronix.de; festevam@...il.com;
> linux-pci@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> imx@...ts.linux.dev; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v5 2/6] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM
> is existing in suspend
>
> On Tue, Sep 02, 2025 at 04:01:47PM +0800, Richard Zhu wrote:
> > 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.
> >
>
> This statement is not accurate. A single register read cannot cause hang AFAIK.
> I'm guessing that the link down (LDn) happens after initiating PME_Turn_Off
> and the access to CSR register (LTSSM) causes hang.
>
> Is my understanding correct?
The access of LTSSM is not relied on the link is up or not. For example,
the LTSSM stats can be accessed when the link is in the training
and not up yet.
Per to the discussion with Bjorn, the most possible reason is that the
LTSSM isn't powered anymore on i.MX6/7 when PME_Turn_Off is kicked off.
https://lore.kernel.org/imx/20250819192838.GA526045@bhelgaas/
Best Regards
Richard Zhu
>
> > 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
> > set in suspend.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> > Reviewed-by: Frank Li <Frank.Li@....com>
>
> We need Fixes tag also and you do need to set the flag in relevant glue driver
> in this patch itself so that it can cleanly be backported.
>
> - Mani
>
> > ---
> > .../pci/controller/dwc/pcie-designware-host.c | 34
> > +++++++++++++------ drivers/pci/controller/dwc/pcie-designware.h |
> > 4 +++
> > 2 files changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 9d46d1f0334b..57a1ba08c427 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1016,15 +1016,29 @@ 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_quirk(pci, QUIRK_NOL2POLL_IN_PM)) {
> > + /*
> > + * Add the QUIRK_NOL2_POLL_IN_PM case to avoid the read hang,
> > + * when LTSSM is not powered in L2/L3/LDn properly.
> > + *
> > + * 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. On the LTSSM states poll broken
> > + * platforms, add a max 10ms delay refer to PCIe r6.0,
> > + * sec 5.3.3.2.1 PME Synchronization.
> > + */
> > + mdelay(PCIE_PME_TO_L2_TIMEOUT_US/1000);
> > + } else {
> > + 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;
> > + }
> > }
> >
> > /*
> > @@ -1040,7 +1054,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >
> > pci->suspended = true;
> >
> > - return ret;
> > + return 0;
> > }
> > EXPORT_SYMBOL_GPL(dw_pcie_suspend_noirq);
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 00f52d472dcd..4e5bf6cb6ce8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -295,6 +295,9 @@
> > /* Default eDMA LLP memory size */
> > #define DMA_LLP_MEM_SIZE PAGE_SIZE
> >
> > +#define QUIRK_NOL2POLL_IN_PM BIT(0)
> > +#define dwc_quirk(pci, val) (pci->quirk_flag & val)
> > +
> > struct dw_pcie;
> > struct dw_pcie_rp;
> > struct dw_pcie_ep;
> > @@ -504,6 +507,7 @@ struct dw_pcie {
> > const struct dw_pcie_ops *ops;
> > u32 version;
> > u32 type;
> > + u32 quirk_flag;
> > unsigned long caps;
> > int num_lanes;
> > int max_link_speed;
> > --
> > 2.37.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists