[PATCH v2] PCI: dwc: do not ignore link errors

[PATCH v2] PCI: dwc: do not ignore link errors

[Christian Gmeiner]

This fixes long boot delays of about 10 seconds.

I am working on a device powered by an TI am65 SoC where
we have a PCIe expansion slot. If there is no PCIe device
connected I see boot delays caused by pci_host_probe(..).

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---

V2: fix compile

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index a608ae1fad57..ce7d3a6f9a03 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -408,8 +408,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			goto err_free_msi;
 	}
 
-	/* Ignore errors, the link may come up later */
-	dw_pcie_wait_for_link(pci);
+	ret = dw_pcie_wait_for_link(pci);
+	if (ret)
+		goto err_free_msi;
 
 	bridge->sysdata = pp;
 
-- 
2.31.1

Re: [PATCH v2] PCI: dwc: do not ignore link errors

[Bjorn Helgaas]

On Wed, Jul 21, 2021 at 05:28:21PM +0200, Christian Gmeiner wrote:
> This fixes long boot delays of about 10 seconds.
> 
> I am working on a device powered by an TI am65 SoC where
> we have a PCIe expansion slot. If there is no PCIe device
> connected I see boot delays caused by pci_host_probe(..).

I agree this is problematic.  Surely the controller can generate an
interrupt when the link comes up?  If so, can we make an interrupt
handler for that "Link Up" interrupt that calls or schedules
pci_host_probe(), and get rid of dw_pcie_wait_for_link() altogether?

I doubt we want to just return failure here if the link doesn't come
up right away, because then we can never hot-add a device later.

886a9c134755 ("PCI: dwc: Move link handling into common code")
addresses this in the commit log:

  The behavior for a link down was inconsistent as some drivers would
  fail probe in that case while others succeed. Let's standardize this
  to succeed as there are usecases where devices (and the link) appear
  later even without hotplug. For example, a reconfigured FPGA device.

> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
> 
> V2: fix compile
> 
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a608ae1fad57..ce7d3a6f9a03 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -408,8 +408,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			goto err_free_msi;
>  	}
>  
> -	/* Ignore errors, the link may come up later */
> -	dw_pcie_wait_for_link(pci);
> +	ret = dw_pcie_wait_for_link(pci);
> +	if (ret)
> +		goto err_free_msi;
>  
>  	bridge->sysdata = pp;
>  
> -- 
> 2.31.1
> 

Re: [PATCH v2] PCI: dwc: do not ignore link errors

[Bjorn Helgaas]

On Wed, Jul 21, 2021 at 12:11:42PM -0500, Bjorn Helgaas wrote:
> On Wed, Jul 21, 2021 at 05:28:21PM +0200, Christian Gmeiner wrote:
> > This fixes long boot delays of about 10 seconds.
> > 
> > I am working on a device powered by an TI am65 SoC where
> > we have a PCIe expansion slot. If there is no PCIe device
> > connected I see boot delays caused by pci_host_probe(..).

Oh, and I forgot to mention: Please follow the convention of
capitalizing the first word of the subject (use "git log --oneline
drivers/pci/controller/dwc/pcie-designware-host.c" to see what the
convention is).

Also, the commit log does not actually say what the patch *does*.
Please include an imperative statement about what it does in the
commit log.  https://chris.beams.io/posts/git-commit/ has good tips.