[PATCH v2 0/2] PCI: dwc: Suspend/resume rework

[PATCH v2 0/2] PCI: dwc: Suspend/resume rework

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series is a rework of [1] to allow DWC vendor glue drivers to use the
dw_pcie_suspend_noirq() and dw_pcie_resume_noirq() APIs without failures as
reported in [2][3].

Currently, both of these APIs will fail if there is no device connected to the
bus. This is not fair as suspend/resume should continue even if there is no
device. Hence, this series tries to address this limitation.

- Mani

[1] https://lore.kernel.org/linux-pci/20251107044319.8356-4-manivannan.sadhasivam@oss.qualcomm.com/
[2] https://lore.kernel.org/linux-pci/CAKfTPtCtHquxtK=Zx2WSNm15MmqeUXO8XXi8FkS4EpuP80PP7g@mail.gmail.com/
[3] https://lore.kernel.org/linux-pci/27516921.17f2.1997bb2a498.Coremail.zhangsenchuan@eswincomputing.com/

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v2:
- Changed the logic to check for Detect.Quiet/Active states
- Collected tags and rebased on top of v6.19-rc1
- Link to v1: https://lore.kernel.org/r/20251119-pci-dwc-suspend-rework-v1-0-aad104828562@oss.qualcomm.com

---
Manivannan Sadhasivam (2):
      PCI: dwc: Skip PME_Turn_Off broadcast and L2/L3 transition during suspend if link is not up
      PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

 drivers/pci/controller/dwc/pcie-designware-host.c | 12 +++++++++---
 drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
 2 files changed, 20 insertions(+), 4 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251119-pci-dwc-suspend-rework-8b0515a38679

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

[PATCH v2 1/2] PCI: dwc: Skip PME_Turn_Off broadcast and L2/L3 transition during suspend if link is not up

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

During system suspend, if the PCIe link is not up, then there is no need
to broadcast PME_Turn_Off message and wait for L2/L3 transition. So skip
them.

Tested-by: Vincent Guittot <vincent.guittot@linaro.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 372207c33a85..43d091128ef7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1161,6 +1161,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	u32 val;
 	int ret;
 
+	if (!dw_pcie_link_up(pci))
+		goto stop_link;
+
 	/*
 	 * If L1SS is supported, then do not put the link into L2 as some
 	 * devices such as NVMe expect low resume latency.
@@ -1194,6 +1197,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	 */
 	udelay(1);
 
+stop_link:
 	dw_pcie_stop_link(pci);
 	if (pci->pp.ops->deinit)
 		pci->pp.ops->deinit(&pci->pp);

-- 
2.48.1

[PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

dw_pcie_wait_for_link() API waits for the link to be up and returns failure
if the link is not up within the 1 second interval. But if there was no
device connected to the bus, then the link up failure would be expected.
In that case, the callers might want to skip the failure in a hope that the
link will be up later when a device gets connected.

One of the callers, dw_pcie_host_init() is currently skipping the failure
irrespective of the link state, in an assumption that the link may come up
later. But this assumption is wrong, since LTSSM states other than
Detect.Quiet and Detect.Active during link training phase are considered to
be fatal and the link needs to be retrained.

So to avoid callers making wrong assumptions, skip returning failure from
dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
Detect.Active states after timeout and also check the return value of the
API in dw_pcie_host_init().

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
 drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 43d091128ef7..ef6d9ae6eddb 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	 * If there is no Link Up IRQ, we should not bypass the delay
 	 * because that would require users to manually rescan for devices.
 	 */
-	if (!pp->use_linkup_irq)
-		/* Ignore errors, the link may come up later */
-		dw_pcie_wait_for_link(pci);
+	if (!pp->use_linkup_irq) {
+		ret = dw_pcie_wait_for_link(pci);
+		if (ret)
+			goto err_stop_link;
+	}
 
 	ret = pci_host_probe(bridge);
 	if (ret)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 75fc8b767fcc..b58baf26ce58 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
 
 int dw_pcie_wait_for_link(struct dw_pcie *pci)
 {
-	u32 offset, val;
+	u32 offset, val, ltssm;
 	int retries;
 
 	/* Check if the link is up or not */
@@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
 	}
 
 	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
+		/*
+		 * If the link is in Detect.Quiet or Detect.Active state, it
+		 * indicates that no device is detected. So return success to
+		 * allow the device to show up later.
+		 */
+		ltssm = dw_pcie_get_ltssm(pci);
+		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
+		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
+			return 0;
+
 		dev_info(pci->dev, "Phy link never came up\n");
 		return -ETIMEDOUT;
 	}

-- 
2.48.1

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Krishna Chaitanya Chundru]

On 12/18/2025 5:34 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> if the link is not up within the 1 second interval. But if there was no
> device connected to the bus, then the link up failure would be expected.
> In that case, the callers might want to skip the failure in a hope that the
> link will be up later when a device gets connected.
>
> One of the callers, dw_pcie_host_init() is currently skipping the failure
> irrespective of the link state, in an assumption that the link may come up
> later. But this assumption is wrong, since LTSSM states other than
> Detect.Quiet and Detect.Active during link training phase are considered to
> be fatal and the link needs to be retrained.
>
> So to avoid callers making wrong assumptions, skip returning failure from
> dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
> Detect.Active states after timeout and also check the return value of the
> API in dw_pcie_host_init().
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
>   drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
>   2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 43d091128ef7..ef6d9ae6eddb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	 * If there is no Link Up IRQ, we should not bypass the delay
>   	 * because that would require users to manually rescan for devices.
>   	 */
> -	if (!pp->use_linkup_irq)
> -		/* Ignore errors, the link may come up later */
> -		dw_pcie_wait_for_link(pci);
> +	if (!pp->use_linkup_irq) {
> +		ret = dw_pcie_wait_for_link(pci);
> +		if (ret)
> +			goto err_stop_link;
> +	}
>   
>   	ret = pci_host_probe(bridge);
>   	if (ret)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 75fc8b767fcc..b58baf26ce58 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
>   
>   int dw_pcie_wait_for_link(struct dw_pcie *pci)
>   {
> -	u32 offset, val;
> +	u32 offset, val, ltssm;
>   	int retries;
>   
>   	/* Check if the link is up or not */
> @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>   	}
>   
>   	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> +		/*
> +		 * If the link is in Detect.Quiet or Detect.Active state, it
> +		 * indicates that no device is detected. So return success to
> +		 * allow the device to show up later.
> +		 */
> +		ltssm = dw_pcie_get_ltssm(pci);
> +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
> +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
> +			return 0;
> +
>   		dev_info(pci->dev, "Phy link never came up\n");
Can you move this print above, as this print is useful for the user to 
know that, link is not up yet.

- Krishna Chaitanya.
>   		return -ETIMEDOUT;
>   	}
>

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Shawn Lin]

在 2025/12/18 星期四 20:04, Manivannan Sadhasivam via B4 Relay 写道:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> if the link is not up within the 1 second interval. But if there was no
> device connected to the bus, then the link up failure would be expected.
> In that case, the callers might want to skip the failure in a hope that the
> link will be up later when a device gets connected.
> 
> One of the callers, dw_pcie_host_init() is currently skipping the failure
> irrespective of the link state, in an assumption that the link may come up
> later. But this assumption is wrong, since LTSSM states other than
> Detect.Quiet and Detect.Active during link training phase are considered to
> be fatal and the link needs to be retrained.
> 
> So to avoid callers making wrong assumptions, skip returning failure from
> dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
> Detect.Active states after timeout and also check the return value of the
> API in dw_pcie_host_init().
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
>   drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
>   2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 43d091128ef7..ef6d9ae6eddb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	 * If there is no Link Up IRQ, we should not bypass the delay
>   	 * because that would require users to manually rescan for devices.
>   	 */
> -	if (!pp->use_linkup_irq)
> -		/* Ignore errors, the link may come up later */
> -		dw_pcie_wait_for_link(pci);
> +	if (!pp->use_linkup_irq) {
> +		ret = dw_pcie_wait_for_link(pci);
> +		if (ret)
> +			goto err_stop_link;
> +	}
>   
>   	ret = pci_host_probe(bridge);
>   	if (ret)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 75fc8b767fcc..b58baf26ce58 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
>   
>   int dw_pcie_wait_for_link(struct dw_pcie *pci)
>   {
> -	u32 offset, val;
> +	u32 offset, val, ltssm;
>   	int retries;
>   
>   	/* Check if the link is up or not */
> @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>   	}
>   
>   	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> +		/*
> +		 * If the link is in Detect.Quiet or Detect.Active state, it
> +		 * indicates that no device is detected. So return success to
> +		 * allow the device to show up later.
> +		 */
> +		ltssm = dw_pcie_get_ltssm(pci);
> +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
> +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
> +			return 0;

By looking more closely, this changes the behaviour of pcie-tegra194.c
which relies on it in tegra_pcie_dw_start_link() to do some retries.

pcie-intel-gw.c/pci-imx6.c also continue to do some setups in this case,
not sure if it's safe.

> +
>   		dev_info(pci->dev, "Phy link never came up\n");
>   		return -ETIMEDOUT;
>   	}
> 

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Manivannan Sadhasivam]

On Thu, Dec 18, 2025 at 05:57:30PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 12/18/2025 5:34 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> > if the link is not up within the 1 second interval. But if there was no
> > device connected to the bus, then the link up failure would be expected.
> > In that case, the callers might want to skip the failure in a hope that the
> > link will be up later when a device gets connected.
> > 
> > One of the callers, dw_pcie_host_init() is currently skipping the failure
> > irrespective of the link state, in an assumption that the link may come up
> > later. But this assumption is wrong, since LTSSM states other than
> > Detect.Quiet and Detect.Active during link training phase are considered to
> > be fatal and the link needs to be retrained.
> > 
> > So to avoid callers making wrong assumptions, skip returning failure from
> > dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
> > Detect.Active states after timeout and also check the return value of the
> > API in dw_pcie_host_init().
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >   drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
> >   drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
> >   2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 43d091128ef7..ef6d9ae6eddb 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> >   	 * If there is no Link Up IRQ, we should not bypass the delay
> >   	 * because that would require users to manually rescan for devices.
> >   	 */
> > -	if (!pp->use_linkup_irq)
> > -		/* Ignore errors, the link may come up later */
> > -		dw_pcie_wait_for_link(pci);
> > +	if (!pp->use_linkup_irq) {
> > +		ret = dw_pcie_wait_for_link(pci);
> > +		if (ret)
> > +			goto err_stop_link;
> > +	}
> >   	ret = pci_host_probe(bridge);
> >   	if (ret)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 75fc8b767fcc..b58baf26ce58 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
> >   int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >   {
> > -	u32 offset, val;
> > +	u32 offset, val, ltssm;
> >   	int retries;
> >   	/* Check if the link is up or not */
> > @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >   	}
> >   	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> > +		/*
> > +		 * If the link is in Detect.Quiet or Detect.Active state, it
> > +		 * indicates that no device is detected. So return success to
> > +		 * allow the device to show up later.
> > +		 */
> > +		ltssm = dw_pcie_get_ltssm(pci);
> > +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
> > +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
> > +			return 0;
> > +
> >   		dev_info(pci->dev, "Phy link never came up\n");
> Can you move this print above, as this print is useful for the user to know
> that, link is not up yet.
> 

If the device is not connected to the bus, what information does this log
provide to the user?

- Mani

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

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Manivannan Sadhasivam]

On Thu, Dec 18, 2025 at 08:45:15PM +0800, Shawn Lin wrote:
> 在 2025/12/18 星期四 20:04, Manivannan Sadhasivam via B4 Relay 写道:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> > if the link is not up within the 1 second interval. But if there was no
> > device connected to the bus, then the link up failure would be expected.
> > In that case, the callers might want to skip the failure in a hope that the
> > link will be up later when a device gets connected.
> > 
> > One of the callers, dw_pcie_host_init() is currently skipping the failure
> > irrespective of the link state, in an assumption that the link may come up
> > later. But this assumption is wrong, since LTSSM states other than
> > Detect.Quiet and Detect.Active during link training phase are considered to
> > be fatal and the link needs to be retrained.
> > 
> > So to avoid callers making wrong assumptions, skip returning failure from
> > dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
> > Detect.Active states after timeout and also check the return value of the
> > API in dw_pcie_host_init().
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >   drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
> >   drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
> >   2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 43d091128ef7..ef6d9ae6eddb 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> >   	 * If there is no Link Up IRQ, we should not bypass the delay
> >   	 * because that would require users to manually rescan for devices.
> >   	 */
> > -	if (!pp->use_linkup_irq)
> > -		/* Ignore errors, the link may come up later */
> > -		dw_pcie_wait_for_link(pci);
> > +	if (!pp->use_linkup_irq) {
> > +		ret = dw_pcie_wait_for_link(pci);
> > +		if (ret)
> > +			goto err_stop_link;
> > +	}
> >   	ret = pci_host_probe(bridge);
> >   	if (ret)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 75fc8b767fcc..b58baf26ce58 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
> >   int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >   {
> > -	u32 offset, val;
> > +	u32 offset, val, ltssm;
> >   	int retries;
> >   	/* Check if the link is up or not */
> > @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >   	}
> >   	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> > +		/*
> > +		 * If the link is in Detect.Quiet or Detect.Active state, it
> > +		 * indicates that no device is detected. So return success to
> > +		 * allow the device to show up later.
> > +		 */
> > +		ltssm = dw_pcie_get_ltssm(pci);
> > +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
> > +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
> > +			return 0;
> 
> By looking more closely, this changes the behaviour of pcie-tegra194.c
> which relies on it in tegra_pcie_dw_start_link() to do some retries.
> 
> pcie-intel-gw.c/pci-imx6.c also continue to do some setups in this case,
> not sure if it's safe.
> 

Hmm, I agree. I did check the callers during v1, but fail to notice pci-imx6
which writes to LNKCAP and PCIE_LINK_WIDTH_SPEED_CONTROL registers after
success.

To be on the safe side, dw_pcie_wait_for_link() should return -ENODEV if the
device is not found and the callers can check for this errno and skip the
failure.

- Mani

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

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Krishna Chaitanya Chundru]

On 12/18/2025 6:16 PM, Manivannan Sadhasivam wrote:
> On Thu, Dec 18, 2025 at 05:57:30PM +0530, Krishna Chaitanya Chundru wrote:
>>
>> On 12/18/2025 5:34 PM, Manivannan Sadhasivam via B4 Relay wrote:
>>> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>
>>> dw_pcie_wait_for_link() API waits for the link to be up and returns failure
>>> if the link is not up within the 1 second interval. But if there was no
>>> device connected to the bus, then the link up failure would be expected.
>>> In that case, the callers might want to skip the failure in a hope that the
>>> link will be up later when a device gets connected.
>>>
>>> One of the callers, dw_pcie_host_init() is currently skipping the failure
>>> irrespective of the link state, in an assumption that the link may come up
>>> later. But this assumption is wrong, since LTSSM states other than
>>> Detect.Quiet and Detect.Active during link training phase are considered to
>>> be fatal and the link needs to be retrained.
>>>
>>> So to avoid callers making wrong assumptions, skip returning failure from
>>> dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
>>> Detect.Active states after timeout and also check the return value of the
>>> API in dw_pcie_host_init().
>>>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>> ---
>>>    drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
>>>    drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
>>>    2 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> index 43d091128ef7..ef6d9ae6eddb 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>>>    	 * If there is no Link Up IRQ, we should not bypass the delay
>>>    	 * because that would require users to manually rescan for devices.
>>>    	 */
>>> -	if (!pp->use_linkup_irq)
>>> -		/* Ignore errors, the link may come up later */
>>> -		dw_pcie_wait_for_link(pci);
>>> +	if (!pp->use_linkup_irq) {
>>> +		ret = dw_pcie_wait_for_link(pci);
>>> +		if (ret)
>>> +			goto err_stop_link;
>>> +	}
>>>    	ret = pci_host_probe(bridge);
>>>    	if (ret)
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>> index 75fc8b767fcc..b58baf26ce58 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>> @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
>>>    int dw_pcie_wait_for_link(struct dw_pcie *pci)
>>>    {
>>> -	u32 offset, val;
>>> +	u32 offset, val, ltssm;
>>>    	int retries;
>>>    	/* Check if the link is up or not */
>>> @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>>>    	}
>>>    	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
>>> +		/*
>>> +		 * If the link is in Detect.Quiet or Detect.Active state, it
>>> +		 * indicates that no device is detected. So return success to
>>> +		 * allow the device to show up later.
>>> +		 */
>>> +		ltssm = dw_pcie_get_ltssm(pci);
>>> +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
>>> +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
>>> +			return 0;
>>> +
>>>    		dev_info(pci->dev, "Phy link never came up\n");
>> Can you move this print above, as this print is useful for the user to know
>> that, link is not up yet.
>>
> If the device is not connected to the bus, what information does this log
> provide to the user?
Not every user is aware that device is not connected, at-least this log 
will give info
that there is no device connected.

- Krishna Chaitanya.
> - Mani
>

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Manivannan Sadhasivam]

On Thu, Dec 18, 2025 at 06:26:12PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 12/18/2025 6:16 PM, Manivannan Sadhasivam wrote:
> > On Thu, Dec 18, 2025 at 05:57:30PM +0530, Krishna Chaitanya Chundru wrote:
> > > 
> > > On 12/18/2025 5:34 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > > 
> > > > dw_pcie_wait_for_link() API waits for the link to be up and returns failure
> > > > if the link is not up within the 1 second interval. But if there was no
> > > > device connected to the bus, then the link up failure would be expected.
> > > > In that case, the callers might want to skip the failure in a hope that the
> > > > link will be up later when a device gets connected.
> > > > 
> > > > One of the callers, dw_pcie_host_init() is currently skipping the failure
> > > > irrespective of the link state, in an assumption that the link may come up
> > > > later. But this assumption is wrong, since LTSSM states other than
> > > > Detect.Quiet and Detect.Active during link training phase are considered to
> > > > be fatal and the link needs to be retrained.
> > > > 
> > > > So to avoid callers making wrong assumptions, skip returning failure from
> > > > dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
> > > > Detect.Active states after timeout and also check the return value of the
> > > > API in dw_pcie_host_init().
> > > > 
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > > ---
> > > >    drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
> > > >    drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
> > > >    2 files changed, 16 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > index 43d091128ef7..ef6d9ae6eddb 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > > >    	 * If there is no Link Up IRQ, we should not bypass the delay
> > > >    	 * because that would require users to manually rescan for devices.
> > > >    	 */
> > > > -	if (!pp->use_linkup_irq)
> > > > -		/* Ignore errors, the link may come up later */
> > > > -		dw_pcie_wait_for_link(pci);
> > > > +	if (!pp->use_linkup_irq) {
> > > > +		ret = dw_pcie_wait_for_link(pci);
> > > > +		if (ret)
> > > > +			goto err_stop_link;
> > > > +	}
> > > >    	ret = pci_host_probe(bridge);
> > > >    	if (ret)
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 75fc8b767fcc..b58baf26ce58 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
> > > >    int dw_pcie_wait_for_link(struct dw_pcie *pci)
> > > >    {
> > > > -	u32 offset, val;
> > > > +	u32 offset, val, ltssm;
> > > >    	int retries;
> > > >    	/* Check if the link is up or not */
> > > > @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> > > >    	}
> > > >    	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
> > > > +		/*
> > > > +		 * If the link is in Detect.Quiet or Detect.Active state, it
> > > > +		 * indicates that no device is detected. So return success to
> > > > +		 * allow the device to show up later.
> > > > +		 */
> > > > +		ltssm = dw_pcie_get_ltssm(pci);
> > > > +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
> > > > +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
> > > > +			return 0;
> > > > +
> > > >    		dev_info(pci->dev, "Phy link never came up\n");
> > > Can you move this print above, as this print is useful for the user to know
> > > that, link is not up yet.
> > > 
> > If the device is not connected to the bus, what information does this log
> > provide to the user?
> Not every user is aware that device is not connected, at-least this log will
> give info
> that there is no device connected.
> 

Users won't grep the dmesg log to check whether the device is connected to the
bus or not. They will use lspci.

- Mani

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

Re: (subset) [PATCH v2 0/2] PCI: dwc: Suspend/resume rework

[Manivannan Sadhasivam]

On Thu, 18 Dec 2025 17:34:51 +0530, Manivannan Sadhasivam wrote:
> This series is a rework of [1] to allow DWC vendor glue drivers to use the
> dw_pcie_suspend_noirq() and dw_pcie_resume_noirq() APIs without failures as
> reported in [2][3].
> 
> Currently, both of these APIs will fail if there is no device connected to the
> bus. This is not fair as suspend/resume should continue even if there is no
> device. Hence, this series tries to address this limitation.
> 
> [...]

Applied, thanks!

[1/2] PCI: dwc: Skip PME_Turn_Off broadcast and L2/L3 transition during suspend if link is not up
      commit: c4a86e6600fa082d6646044fcce2183ad5e52283

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

Re: [PATCH v2 2/2] PCI: dwc: Do not return failure if link is in Detect.Quiet/Active states

[Krishna Chaitanya Chundru]

On 12/18/2025 6:30 PM, Manivannan Sadhasivam wrote:
> On Thu, Dec 18, 2025 at 06:26:12PM +0530, Krishna Chaitanya Chundru wrote:
>>
>> On 12/18/2025 6:16 PM, Manivannan Sadhasivam wrote:
>>> On Thu, Dec 18, 2025 at 05:57:30PM +0530, Krishna Chaitanya Chundru wrote:
>>>> On 12/18/2025 5:34 PM, Manivannan Sadhasivam via B4 Relay wrote:
>>>>> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>>>
>>>>> dw_pcie_wait_for_link() API waits for the link to be up and returns failure
>>>>> if the link is not up within the 1 second interval. But if there was no
>>>>> device connected to the bus, then the link up failure would be expected.
>>>>> In that case, the callers might want to skip the failure in a hope that the
>>>>> link will be up later when a device gets connected.
>>>>>
>>>>> One of the callers, dw_pcie_host_init() is currently skipping the failure
>>>>> irrespective of the link state, in an assumption that the link may come up
>>>>> later. But this assumption is wrong, since LTSSM states other than
>>>>> Detect.Quiet and Detect.Active during link training phase are considered to
>>>>> be fatal and the link needs to be retrained.
>>>>>
>>>>> So to avoid callers making wrong assumptions, skip returning failure from
>>>>> dw_pcie_wait_for_link() only if the link is in Detect.Quiet or
>>>>> Detect.Active states after timeout and also check the return value of the
>>>>> API in dw_pcie_host_init().
>>>>>
>>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>>> ---
>>>>>     drivers/pci/controller/dwc/pcie-designware-host.c |  8 +++++---
>>>>>     drivers/pci/controller/dwc/pcie-designware.c      | 12 +++++++++++-
>>>>>     2 files changed, 16 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>>>> index 43d091128ef7..ef6d9ae6eddb 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>>>> @@ -670,9 +670,11 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>>>>>     	 * If there is no Link Up IRQ, we should not bypass the delay
>>>>>     	 * because that would require users to manually rescan for devices.
>>>>>     	 */
>>>>> -	if (!pp->use_linkup_irq)
>>>>> -		/* Ignore errors, the link may come up later */
>>>>> -		dw_pcie_wait_for_link(pci);
>>>>> +	if (!pp->use_linkup_irq) {
>>>>> +		ret = dw_pcie_wait_for_link(pci);
>>>>> +		if (ret)
>>>>> +			goto err_stop_link;
>>>>> +	}
>>>>>     	ret = pci_host_probe(bridge);
>>>>>     	if (ret)
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>>>> index 75fc8b767fcc..b58baf26ce58 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>>>> @@ -641,7 +641,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
>>>>>     int dw_pcie_wait_for_link(struct dw_pcie *pci)
>>>>>     {
>>>>> -	u32 offset, val;
>>>>> +	u32 offset, val, ltssm;
>>>>>     	int retries;
>>>>>     	/* Check if the link is up or not */
>>>>> @@ -653,6 +653,16 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
>>>>>     	}
>>>>>     	if (retries >= PCIE_LINK_WAIT_MAX_RETRIES) {
>>>>> +		/*
>>>>> +		 * If the link is in Detect.Quiet or Detect.Active state, it
>>>>> +		 * indicates that no device is detected. So return success to
>>>>> +		 * allow the device to show up later.
>>>>> +		 */
>>>>> +		ltssm = dw_pcie_get_ltssm(pci);
>>>>> +		if (ltssm == DW_PCIE_LTSSM_DETECT_QUIET ||
>>>>> +		    ltssm == DW_PCIE_LTSSM_DETECT_ACT)
>>>>> +			return 0;
>>>>> +
>>>>>     		dev_info(pci->dev, "Phy link never came up\n");
>>>> Can you move this print above, as this print is useful for the user to know
>>>> that, link is not up yet.
>>>>
>>> If the device is not connected to the bus, what information does this log
>>> provide to the user?
>> Not every user is aware that device is not connected, at-least this log will
>> give info
>> that there is no device connected.
>>
> Users won't grep the dmesg log to check whether the device is connected to the
> bus or not. They will use lspci.
ack.

- Krishna Chaitanya.
> - Mani
>