[RESEND PATCH v3 0/5] Add quirks to proceed PME handshake in DWC PM

[RESEND PATCH v3 0/5] Add quirks to proceed PME handshake in DWC PM

[Richard Zhu]

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 of i.MX6QP PCIe is inaccessible after the PME_Turn_Off
is kicked off. To handle this case, don't poll L2 state and add one max
10ms delay if QUIRK_NOL2POLL_IN_PM flag is existing in suspend.

Main changes in v3:
- Adjust the patch sequence to avoid the build break.
- Update the commit message.
v2:https://patchwork.kernel.org/project/linux-pci/cover/20250618024116.3704579-1-hongxing.zhu@nxp.com/

Main changes in v2:
Add the following two patches.
- Skip PME_Turn_Off message if there is no endpoint connected.
- Don't return error when wait for link up.
v1:https://patchwork.kernel.org/project/linux-pci/cover/20250408065221.1941928-1-hongxing.zhu@nxp.com/

[RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is
[RESEND v3 2/5] PCI: imx6: Don't poll LTSSM state of i.MX6QP PCIe in
[RESEND v3 3/5] PCI: imx6: Don't poll LTSSM state of i.MX7D PCIe in
[RESEND v3 4/5] PCI: dwc: Skip PME_Turn_Off message if there is no
[RESEND v3 5/5] PCI: dwc: Don't return error when wait for link up

drivers/pci/controller/dwc/pci-imx6.c             |  4 ++++
drivers/pci/controller/dwc/pcie-designware-host.c | 50 +++++++++++++++++++++++++++++++-------------------
drivers/pci/controller/dwc/pcie-designware.h      |  4 ++++
3 files changed, 39 insertions(+), 19 deletions(-)

[RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Richard Zhu]

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 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
@@ -1024,15 +1024,26 @@ 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)) {
+		/*
+		 * 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;
+		}
 	}
 
 	/*
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 00f52d472dcdd..4e5bf6cb6ce80 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

Re: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Bjorn Helgaas]

On Mon, Aug 18, 2025 at 03:32:01PM +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.
> 
> 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@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.

*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?

Bjorn

RE: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Hongxing Zhu]

> -----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:
> > 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
> > 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

Re: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Bjorn Helgaas]

On Tue, Aug 19, 2025 at 05:51:41AM +0000, Hongxing Zhu wrote:
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > On Mon, Aug 18, 2025 at 03:32:01PM +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.
> ...

> > > +++ 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.

> Should I created another commit to get ride of the L1SS check codes?

If we remove that check, I guess we would put the link into L2 when
ASPM L1 is enabled (not when "L1SS is supported" as the comment
claims).

Obviously this check was added for a reason, so I assume something bad
would happen if we removed it.  But at the same time, AFAICS this
check only applies to imx6 and layerscape because none of the other
drivers use dw_pcie_suspend_noirq().

So yes, I do think it should be removed because it's only a partial
band-aid for whatever the problem is.  It would probably break
something, but it looks to me like it's already broken on most
platforms, and we need to figure out a real solution that fixes
everybody.

> > *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.

Re: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Bjorn Helgaas]

On Mon, Aug 18, 2025 at 03:32:01PM +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.
> 
> 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@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;

I think it's pointless to initialize "ret" because in every case where
ret is set, we return it immediately if it is non-zero.  We should
just return 0 explicitly at the end of the function and skip this
initialization.

>       /*
>        * If L1SS is supported, then do not put the link into L2 as some
> @@ -1024,15 +1024,26 @@ 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)) {
> +		/*
> +		 * 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.

IIUC, the read_poll_timeout() below is waiting for the PME_TO_Ack
responses to the PME_Turn_Off message.

The Link state transition to L2/L3 Ready (or the subsequent L2, L3, or
LDn states) is the indication that the downstream components have
"performed any necessary local conditioning in preparation for power
removal" and then responded with PME_TO_Ack.

And the PCIE_PME_TO_L2_TIMEOUT_US timeout is the deadlock avoidance
delay for cases where "one or more devices do not respond with a
PME_TO_Ack".

In the QUIRK_NOL2POLL_IN_PM case, I think the problem is that we can't
*read* the LTSSM state to learn whether the Link transitioned to L2/L3
Ready, L2, L3, or LDn.  That wouldn't be surprising because per sec
5.2, "the LTSSM is typically powered by main power (not Vaux), so
LTSSM will not be powered in either the L2 or the L3 state."

I don't know what's different about i.MX6QP and i.MX7D.  Maybe on most
DWC platforms the LTSSM *is* powered in L2/L3/LDn, but on i.MX6QP and
i.MX7D, it *isn't* powered in those states?

If that's the case, we should say that somewhere here.  And we should
say what happens when we try to read the LTSSM when it's not powered.
Does the read hang or cause some kind of error?

> +		 */
> +		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;
> +		}
>  	}
>  
>  	/*
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 00f52d472dcdd..4e5bf6cb6ce80 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
> 

RE: [RESEND v3 1/5] PCI: dwc: Don't poll L2 if QUIRK_NOL2POLL_IN_PM is existing in suspend

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年8月20日 3:29
> 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:
> > 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
> > 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;
> 
> I think it's pointless to initialize "ret" because in every case where ret is set,
> we return it immediately if it is non-zero.  We should just return 0 explicitly
> at the end of the function and skip this initialization.
> 
You're right, thanks.
> >       /*
> >        * If L1SS is supported, then do not put the link into L2 as
> > some @@ -1024,15 +1024,26 @@ 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)) {
> > +		/*
> > +		 * 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.
> 
> IIUC, the read_poll_timeout() below is waiting for the PME_TO_Ack
> responses to the PME_Turn_Off message.
> 
> The Link state transition to L2/L3 Ready (or the subsequent L2, L3, or LDn
> states) is the indication that the downstream components have "performed
> any necessary local conditioning in preparation for power removal" and then
> responded with PME_TO_Ack.
> 
> And the PCIE_PME_TO_L2_TIMEOUT_US timeout is the deadlock avoidance
> delay for cases where "one or more devices do not respond with a
> PME_TO_Ack".
> 
> In the QUIRK_NOL2POLL_IN_PM case, I think the problem is that we can't
> *read* the LTSSM state to learn whether the Link transitioned to L2/L3
> Ready, L2, L3, or LDn.  That wouldn't be surprising because per sec 5.2, "the
> LTSSM is typically powered by main power (not Vaux), so LTSSM will not be
> powered in either the L2 or the L3 state."
> 
> I don't know what's different about i.MX6QP and i.MX7D.  Maybe on most
> DWC platforms the LTSSM *is* powered in L2/L3/LDn, but on i.MX6QP and
> i.MX7D, it *isn't* powered in those states?
> 
> If that's the case, we should say that somewhere here.  And we should say
> what happens when we try to read the LTSSM when it's not powered.
> Does the read hang or cause some kind of error?
The read is hang directly, no any error messages are dumped.
How about add the following comments in the QUIRK_NOL2_POLL_IN_PM?
"Add the QUIRK_NOL2_POLL_IN_PM case to avoid the read hang, since LTSSM
 might not be powered in L2/L3/LDn on some platforms. "

Best Regards
Richard Zhu
> 
> > +		 */
> > +		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;
> > +		}
> >  	}
> >
> >  	/*
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 00f52d472dcdd..4e5bf6cb6ce80 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
> >

[RESEND v3 2/5] PCI: imx6: Don't poll LTSSM state of i.MX6QP PCIe in PM operations

[Richard Zhu]

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 of i.MX6QP PCIe are inaccessible after the PME_Turn_Off
is kicked off. To handle this case, don't poll L2 state and add one max
10ms delay if QUIRK_NOL2POLL_IN_PM flag is existing in suspend.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 80e48746bbaf6..18b97bd0462bb 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -125,6 +125,7 @@ struct imx_pcie_drvdata {
 	enum imx_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
 	u32 flags;
+	u32 quirk;
 	int dbi_length;
 	const char *gpr;
 	const u32 ltssm_off;
@@ -1765,6 +1766,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pci->quirk_flag = imx_pcie->drvdata->quirk;
 	pci->use_parent_dt_ranges = true;
 	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
 		ret = imx_add_pcie_ep(imx_pcie, pdev);
@@ -1849,6 +1851,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.enable_ref_clk = imx6q_pcie_enable_ref_clk,
 		.core_reset = imx6qp_pcie_core_reset,
 		.ops = &imx_pcie_host_ops,
+		.quirk = QUIRK_NOL2POLL_IN_PM,
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
-- 
2.37.1

[RESEND v3 3/5] PCI: imx6: Don't poll LTSSM state of i.MX7D PCIe in PM operations

[Richard Zhu]

Add a quirk flag(QUIRK_NOL2POLL_IN_PM) for i.MX7D PCIe. Don't poll the
LTSSM states after issue the PME_Turn_Off message.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 18b97bd0462bb..a59b5282c3cca 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1863,6 +1863,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.enable_ref_clk = imx7d_pcie_enable_ref_clk,
 		.core_reset = imx7d_pcie_core_reset,
+		.quirk = QUIRK_NOL2POLL_IN_PM,
 	},
 	[IMX8MQ] = {
 		.variant = IMX8MQ,
-- 
2.37.1

[RESEND v3 4/5] PCI: dwc: Skip PME_Turn_Off message if there is no endpoint connected

[Richard Zhu]

Skip PME_Turn_Off message if there is no endpoint connected.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 20a7f827babbf..868e7db4e3381 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1016,12 +1016,15 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
 		return 0;
 
-	if (pci->pp.ops->pme_turn_off) {
-		pci->pp.ops->pme_turn_off(&pci->pp);
-	} else {
-		ret = dw_pcie_pme_turn_off(pci);
-		if (ret)
-			return ret;
+	/* Skip PME_Turn_Off message if there is no endpoint connected */
+	if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_WAIT) {
+		if (pci->pp.ops->pme_turn_off) {
+			pci->pp.ops->pme_turn_off(&pci->pp);
+		} else {
+			ret = dw_pcie_pme_turn_off(pci);
+			if (ret)
+				return ret;
+		}
 	}
 
 	if (dwc_quirk(pci, QUIRK_NOL2POLL_IN_PM)) {
-- 
2.37.1

Re: [RESEND v3 4/5] PCI: dwc: Skip PME_Turn_Off message if there is no endpoint connected

[Bjorn Helgaas]

On Mon, Aug 18, 2025 at 03:32:04PM +0800, Richard Zhu wrote:
> Skip PME_Turn_Off message if there is no endpoint connected.

What's the value of doing this?  Is this to make something faster?  If
so, what and by how much?

Or does it fix something that's currently broken?

Seems like the discussion at
https://lore.kernel.org/linux-pci/20241107084455.3623576-1-hongxing.zhu@nxp.com/t/#u
might be relevant.

This commit log only restates what the code does.  In my opinion we
need actual justification for making this change.

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 20a7f827babbf..868e7db4e3381 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1016,12 +1016,15 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
>  		return 0;
>  
> -	if (pci->pp.ops->pme_turn_off) {
> -		pci->pp.ops->pme_turn_off(&pci->pp);
> -	} else {
> -		ret = dw_pcie_pme_turn_off(pci);
> -		if (ret)
> -			return ret;
> +	/* Skip PME_Turn_Off message if there is no endpoint connected */
> +	if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_WAIT) {
> +		if (pci->pp.ops->pme_turn_off) {
> +			pci->pp.ops->pme_turn_off(&pci->pp);
> +		} else {
> +			ret = dw_pcie_pme_turn_off(pci);
> +			if (ret)
> +				return ret;
> +		}
>  	}
>
>  	if (dwc_quirk(pci, QUIRK_NOL2POLL_IN_PM)) {
> -- 
> 2.37.1
> 

RE: [RESEND v3 4/5] PCI: dwc: Skip PME_Turn_Off message if there is no endpoint connected

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2025年8月20日 3:07
> 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 4/5] PCI: dwc: Skip PME_Turn_Off message if there is
> no endpoint connected
>
> On Mon, Aug 18, 2025 at 03:32:04PM +0800, Richard Zhu wrote:
> > Skip PME_Turn_Off message if there is no endpoint connected.
>
> What's the value of doing this?  Is this to make something faster?  If so,
> what and by how much?
>
> Or does it fix something that's currently broken?
>
> Seems like the discussion at
> https://lore.kern/
> el.org%2Flinux-pci%2F20241107084455.3623576-1-hongxing.zhu%40nxp.com%
> 2Ft%2F%23u&data=05%7C02%7Chongxing.zhu%40nxp.com%7Ced46fe10aeb74
> 21c88a508dddf53a24f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638912272493755203%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOn
> RydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%
> 3D%3D%7C0%7C%7C%7C&sdata=lIE7%2FlS5jiGxGPGVm5Hr5efpMbT19CLqrwu
> YNvAEdLY%3D&reserved=0
> might be relevant.
>
> This commit log only restates what the code does.  In my opinion we need
> actual justification for making this change.
Hi Bjorn:
Thanks for your comments.
This commit is mainly used to fix suspend/resume broken on i.MX7D PCIe.
A chip freeze is observed on i.MX7D when PCIe RC kicks off the PM_PME message
and no any devices are connected on the port.

Because i.MX7D is a very old design, and out of IP design technical support.
 I don't know what's going on inside the PCIe IP design when kick off the
 PM_PME message.

From SW perspective view, what I can do is to find out a quirk method to
 workaround this broken. Hope this can clear up your confusions.

Best Regards
Richard Zhu
>
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 15
> > +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 20a7f827babbf..868e7db4e3381 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1016,12 +1016,15 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
> >     if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) &
> PCI_EXP_LNKCTL_ASPM_L1)
> >             return 0;
> >
> > -   if (pci->pp.ops->pme_turn_off) {
> > -           pci->pp.ops->pme_turn_off(&pci->pp);
> > -   } else {
> > -           ret = dw_pcie_pme_turn_off(pci);
> > -           if (ret)
> > -                   return ret;
> > +   /* Skip PME_Turn_Off message if there is no endpoint connected */
> > +   if (dw_pcie_get_ltssm(pci) > DW_PCIE_LTSSM_DETECT_WAIT) {
> > +           if (pci->pp.ops->pme_turn_off) {
> > +                   pci->pp.ops->pme_turn_off(&pci->pp);
> > +           } else {
> > +                   ret = dw_pcie_pme_turn_off(pci);
> > +                   if (ret)
> > +                           return ret;
> > +           }
> >     }
> >
> >     if (dwc_quirk(pci, QUIRK_NOL2POLL_IN_PM)) {
> > --
> > 2.37.1
> >

Re: [RESEND v3 4/5] PCI: dwc: Skip PME_Turn_Off message if there is no endpoint connected

[Bjorn Helgaas]

On Thu, Aug 21, 2025 at 05:44:00AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2025年8月20日 3:07
> > 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 4/5] PCI: dwc: Skip PME_Turn_Off message if there is
> > no endpoint connected
> >
> > On Mon, Aug 18, 2025 at 03:32:04PM +0800, Richard Zhu wrote:
> > > Skip PME_Turn_Off message if there is no endpoint connected.
> >
> > What's the value of doing this?  Is this to make something faster?  If so,
> > what and by how much?
> >
> > Or does it fix something that's currently broken?
> >
> > Seems like the discussion at
> > https://lore.kern/
> > el.org%2Flinux-pci%2F20241107084455.3623576-1-hongxing.zhu%40nxp.com%
> > 2Ft%2F%23u&data=05%7C02%7Chongxing.zhu%40nxp.com%7Ced46fe10aeb74
> > 21c88a508dddf53a24f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C638912272493755203%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOn
> > RydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%
> > 3D%3D%7C0%7C%7C%7C&sdata=lIE7%2FlS5jiGxGPGVm5Hr5efpMbT19CLqrwu
> > YNvAEdLY%3D&reserved=0
> > might be relevant.
> >
> > This commit log only restates what the code does.  In my opinion we need
> > actual justification for making this change.
> Hi Bjorn:
> Thanks for your comments.
> This commit is mainly used to fix suspend/resume broken on i.MX7D PCIe.
> A chip freeze is observed on i.MX7D when PCIe RC kicks off the PM_PME message
> and no any devices are connected on the port.
> 
> Because i.MX7D is a very old design, and out of IP design technical support.
>  I don't know what's going on inside the PCIe IP design when kick off the
>  PM_PME message.
> 
> From SW perspective view, what I can do is to find out a quirk method to
>  workaround this broken. Hope this can clear up your confusions.

OK, will look for some of this background in the commit log of the
next version.

[RESEND v3 5/5] PCI: dwc: Don't return error when wait for link up

[Richard Zhu]

When waiting for the PCIe link to come up, both link up and link down
are valid results depending on the device state. Do not return an error,
as the outcome has already been reported in dw_pcie_wait_for_link().

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 868e7db4e3381..e90fd34925702 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1089,9 +1089,7 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
 	if (ret)
 		return ret;
 
-	ret = dw_pcie_wait_for_link(pci);
-	if (ret)
-		return ret;
+	dw_pcie_wait_for_link(pci);
 
 	return ret;
 }
-- 
2.37.1

Re: [RESEND v3 5/5] PCI: dwc: Don't return error when wait for link up

[Bjorn Helgaas]

On Mon, Aug 18, 2025 at 03:32:05PM +0800, Richard Zhu wrote:
> When waiting for the PCIe link to come up, both link up and link down
> are valid results depending on the device state. Do not return an error,
> as the outcome has already been reported in dw_pcie_wait_for_link().

The reporting in dw_pcie_wait_for_link() is only a note in dmesg (and
the -EDTIMEDOUT return, which we're throwing away here).

We need an explanation here about why the caller of
dw_pcie_resume_noirq() doesn't need to know whether the link came up.
A short comment in the code would be useful as well.

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 868e7db4e3381..e90fd34925702 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1089,9 +1089,7 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
>  	if (ret)
>  		return ret;
>  
> -	ret = dw_pcie_wait_for_link(pci);
> -	if (ret)
> -		return ret;
> +	dw_pcie_wait_for_link(pci);
>  
>  	return ret;

This should be "return 0" because if "ret" was non-zero, we returned
that earlier.

>  }
> -- 
> 2.37.1
>