[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <agtozwcmgwj7kfmzkrglqcba5vaqmd2hv3matzmksbckkrrno3@zdteyblgvpyr>
Date: Mon, 22 Sep 2025 16:56:17 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Hongxing Zhu <hongxing.zhu@....com>
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
On Mon, Sep 22, 2025 at 09:18:20AM +0000, Hongxing Zhu wrote:
> > -----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/
>
I'm not sure though. Could you try to read any DBI register at this point and
see if the hang is observed or not?
LTSSM states are expressed through the DBI registers. So as per my
understanding, unless the whole DBI register space is inaccessible, LTSSM
registers should be accessible.
- Mani
> 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