PCI: dw-rockchip: add system suspend support

[PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume

Posted by Sebastian Reichel 1 month, 2 weeks ago

When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
will return -ETIMEDOUT. During probe time this does not happen, since
the platform sets 'use_linkup_irq'.

This adds the same logic from dw_pcie_host_init() to the PM resume
function to avoid the problem.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda5..f25f1c136900 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
 	if (ret)
 		return ret;
 
-	ret = dw_pcie_wait_for_link(pci);
-	if (ret)
-		return ret;
+	/*
+	 * Note: Skip the link up delay only when a Link Up IRQ is present.
+	 * If there is no Link Up IRQ, we should not bypass the delay
+	 * because that would require users to manually rescan for devices.
+	 */
+	if (!pci->pp.use_linkup_irq) {
+		ret = dw_pcie_wait_for_link(pci);
+		if (ret)
+			return ret;
+	}
 
 	return ret;
 }

-- 
2.51.0

Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume

Posted by Krishna Chaitanya Chundru 1 month, 2 weeks ago

On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> will return -ETIMEDOUT. During probe time this does not happen, since
> the platform sets 'use_linkup_irq'.
>
> This adds the same logic from dw_pcie_host_init() to the PM resume
> function to avoid the problem.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda5..f25f1c136900 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
>   	if (ret)
>   		return ret;
>   
> -	ret = dw_pcie_wait_for_link(pci);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Note: Skip the link up delay only when a Link Up IRQ is present.
> +	 * If there is no Link Up IRQ, we should not bypass the delay
> +	 * because that would require users to manually rescan for devices.
> +	 */

In the resume scenario, we should explicitly wait for the link to be up, 
there is no IRQ support at this resume phase
and secondly after controller resume pm framework will start resuming 
the bridges & endpoints. what happens
if the link is not up by the time endpoint is resume is called. And 
entire save & restore states might also gets messed up.
There will be no way to recover from this.

- Krishna Chaitanya.

> +	if (!pci->pp.use_linkup_irq) {
> +		ret = dw_pcie_wait_for_link(pci);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	return ret;
>   }
>

Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume

Posted by Manivannan Sadhasivam 1 month, 1 week ago

On Thu, Oct 30, 2025 at 11:07:19AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> > When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> > slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> > will return -ETIMEDOUT. During probe time this does not happen, since
> > the platform sets 'use_linkup_irq'.
> > 
> > This adds the same logic from dw_pcie_host_init() to the PM resume
> > function to avoid the problem.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >   drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> >   1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index e92513c5bda5..f25f1c136900 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> >   	if (ret)
> >   		return ret;
> > -	ret = dw_pcie_wait_for_link(pci);
> > -	if (ret)
> > -		return ret;
> > +	/*
> > +	 * Note: Skip the link up delay only when a Link Up IRQ is present.
> > +	 * If there is no Link Up IRQ, we should not bypass the delay
> > +	 * because that would require users to manually rescan for devices.
> > +	 */
> 
> In the resume scenario, we should explicitly wait for the link to be up,
> there is no IRQ support at this resume phase

This could be fixed if the PM handlers are moved to non-no_irq ones.

> and secondly after controller resume pm framework will start resuming the
> bridges & endpoints. what happens
> if the link is not up by the time endpoint is resume is called. And entire
> save & restore states might also gets messed up.
> There will be no way to recover from this.
> 

This is true if there were any devices connected to the bus during suspend. If
there were no devices, then it is fine to skip waiting for the link to be up.

- Mani

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

Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume

Posted by Sebastian Reichel 1 month, 1 week ago

Hi,

On Sat, Nov 01, 2025 at 07:50:58PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Oct 30, 2025 at 11:07:19AM +0530, Krishna Chaitanya Chundru wrote:
> > 
> > On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> > > When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> > > slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> > > will return -ETIMEDOUT. During probe time this does not happen, since
> > > the platform sets 'use_linkup_irq'.
> > > 
> > > This adds the same logic from dw_pcie_host_init() to the PM resume
> > > function to avoid the problem.
> > > 
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> > >   1 file changed, 10 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index e92513c5bda5..f25f1c136900 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> > >   	if (ret)
> > >   		return ret;
> > > -	ret = dw_pcie_wait_for_link(pci);
> > > -	if (ret)
> > > -		return ret;
> > > +	/*
> > > +	 * Note: Skip the link up delay only when a Link Up IRQ is present.
> > > +	 * If there is no Link Up IRQ, we should not bypass the delay
> > > +	 * because that would require users to manually rescan for devices.
> > > +	 */
> > 
> > In the resume scenario, we should explicitly wait for the link to be up,
> > there is no IRQ support at this resume phase
> 
> This could be fixed if the PM handlers are moved to non-no_irq ones.
> 
> > and secondly after controller resume pm framework will start resuming the
> > bridges & endpoints. what happens
> > if the link is not up by the time endpoint is resume is called. And entire
> > save & restore states might also gets messed up.
> > There will be no way to recover from this.
> > 
> 
> This is true if there were any devices connected to the bus during suspend. If
> there were no devices, then it is fine to skip waiting for the link to be up.

I thought about setting a flag in the suspend routine, that a device
is expected to be there at resume time. But I suppose it might also
have been removed during the system suspend?

Greetings,

-- Sebastian

Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume

Posted by Bjorn Helgaas 1 month, 2 weeks ago

On Wed, Oct 29, 2025 at 06:56:48PM +0100, Sebastian Reichel wrote:
> When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> will return -ETIMEDOUT. During probe time this does not happen, since
> the platform sets 'use_linkup_irq'.
> 
> This adds the same logic from dw_pcie_host_init() to the PM resume
> function to avoid the problem.

s/PCI: dwc: support/PCI: dwc: Support/  (in subject; capitalize first word)
s/This adds/Add/

Can you mention 8d3bf19f1b58 ("PCI: dwc: Don't wait for link up if
driver can detect Link Up event") here?  I think that's what added the
similar probe-time code.

I see you did copy the comment from 8d3bf19f1b58, thanks for that!

Maybe also word your subject and commit log along the same lines so
the two commits are easier to connect to each other.

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda5..f25f1c136900 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
>  	if (ret)
>  		return ret;
>  
> -	ret = dw_pcie_wait_for_link(pci);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Note: Skip the link up delay only when a Link Up IRQ is present.
> +	 * If there is no Link Up IRQ, we should not bypass the delay
> +	 * because that would require users to manually rescan for devices.
> +	 */
> +	if (!pci->pp.use_linkup_irq) {
> +		ret = dw_pcie_wait_for_link(pci);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return ret;
>  }
> 
> -- 
> 2.51.0
>