diff mbox series

PCI: dwc: Do not fail to probe when link is down

Message ID 20230704122635.1362156-1-festevam@gmail.com (mailing list archive)
State Not Applicable
Delegated to: Krzysztof Wilczyński
Headers show
Series PCI: dwc: Do not fail to probe when link is down | expand

Commit Message

Fabio Estevam July 4, 2023, 12:26 p.m. UTC
From: Fabio Estevam <festevam@denx.de>

Since commit da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is
started") the following probe error is observed when the PCI link is down:

imx6q-pcie 33800000.pcie: Phy link never came up
imx6q-pcie: probe of 33800000.pcie failed with error -110

The intention of commit 886a9c134755 ("PCI: dwc: Move link handling into
common code") was to standardize the behavior of link down as explained
in its 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."

Restore the original behavior by ignoring the error from
dw_pcie_wait_for_link().

Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Fabio Estevam July 7, 2023, 1:29 p.m. UTC | #1
On Tue, Jul 4, 2023 at 9:26 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> From: Fabio Estevam <festevam@denx.de>
>
> Since commit da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is
> started") the following probe error is observed when the PCI link is down:
>
> imx6q-pcie 33800000.pcie: Phy link never came up
> imx6q-pcie: probe of 33800000.pcie failed with error -110
>
> The intention of commit 886a9c134755 ("PCI: dwc: Move link handling into
> common code") was to standardize the behavior of link down as explained
> in its 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."
>
> Restore the original behavior by ignoring the error from
> dw_pcie_wait_for_link().
>
> Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index cf61733bf78d..af6a7cd060b1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -492,11 +492,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>                 if (ret)
>                         goto err_remove_edma;
>
> -               if (pci->ops && pci->ops->start_link) {
> -                       ret = dw_pcie_wait_for_link(pci);
> -                       if (ret)
> -                               goto err_stop_link;
> -               }
> +               if (pci->ops && pci->ops->start_link)
> +                       dw_pcie_wait_for_link(PCI);

We can discard this one as Johan sent a patch doing the full revert:
https://lkml.org/lkml/2023/7/6/204
Ajay Agarwal July 10, 2023, 4:45 p.m. UTC | #2
On Tue, Jul 04, 2023 at 09:26:35AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> Since commit da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is
> started") the following probe error is observed when the PCI link is down:
> 
> imx6q-pcie 33800000.pcie: Phy link never came up
> imx6q-pcie: probe of 33800000.pcie failed with error -110
> 
> The intention of commit 886a9c134755 ("PCI: dwc: Move link handling into
> common code") was to standardize the behavior of link down as explained
> in its 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."
> 
> Restore the original behavior by ignoring the error from
> dw_pcie_wait_for_link().
> 
> Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
> Signed-off-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Ajay Agarwal <ajayagarwal@google.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index cf61733bf78d..af6a7cd060b1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -492,11 +492,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  		if (ret)
>  			goto err_remove_edma;
>  
> -		if (pci->ops && pci->ops->start_link) {
> -			ret = dw_pcie_wait_for_link(pci);
> -			if (ret)
> -				goto err_stop_link;
> -		}
> +		if (pci->ops && pci->ops->start_link)
> +			dw_pcie_wait_for_link(pci);
>  	}
>  
>  	bridge->sysdata = pp;
> -- 
> 2.34.1
> 
LGTM.
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cf61733bf78d..af6a7cd060b1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -492,11 +492,8 @@  int dw_pcie_host_init(struct dw_pcie_rp *pp)
 		if (ret)
 			goto err_remove_edma;
 
-		if (pci->ops && pci->ops->start_link) {
-			ret = dw_pcie_wait_for_link(pci);
-			if (ret)
-				goto err_stop_link;
-		}
+		if (pci->ops && pci->ops->start_link)
+			dw_pcie_wait_for_link(pci);
 	}
 
 	bridge->sysdata = pp;