Revert "PCI: dwc: Wait for link up only if link is started"

This reverts commit da56a1bfbab55189595e588f1d984bdfb5cf5924.

Bjorn Andersson, Fabio Estevam, Xiaolei Wang, and Jon Hunter reported that
da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started") broke
controller probing by returning an error in case the link does not come up
during host initialisation, for example when the slot is empty.

As explained in commit 886a9c134755 ("PCI: dwc: Move link handling into
common code") and as indicated by the comment "Ignore errors, the link may
come up later" in the code, waiting for link up and ignoring errors is the
intended behaviour:

  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.

Reverting the offending commit specifically fixes a regression on Qualcomm
platforms like the Lenovo ThinkPad X13s which no longer reach the
interconnect sync state if a slot does not have a device populated (e.g. an
optional modem).

Note that enabling asynchronous probing by default as was done for Qualcomm
platforms by commit c0e1eb441b1d ("PCI: qcom: Enable async probe by
default"), should take care of any related boot time concerns.

Finally, note that the intel-gw driver is the only driver currently not
providing a .start_link() callback and instead starts the link in its
.host_init() callback, which may avoid an additional one-second timeout
during probe by making the link-up wait conditional. If anyone cares, that
can be done in a follow-up patch with a proper motivation.

[bhelgaas: add Fabio Estevam, Xiaolei Wang, Jon Hunter reports]
Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
Link: https://lore.kernel.org/r/20230706082610.26584-1-johan+linaro@kernel.org
Reported-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Reported-by: Fabio Estevam <festevam@gmail.com>
Link: https://lore.kernel.org/r/20230704122635.1362156-1-festevam@gmail.com/
Reported-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Link: https://lore.kernel.org/r/20230705010624.3912934-1-xiaolei.wang@windriver.com/
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Link: https://lore.kernel.org/r/6ca287a1-6c7c-7b90-9022-9e73fb82b564@nvidia.com
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Sajid Dalvi <sdalvi@google.com>
Cc: Ajay Agarwal <ajayagarwal@google.com>

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cf61733bf78d24a99114bb8bdf3262a771d5b44b..9952057c8819c86ac7bb97dfe709f6337ddfa7c6 100644 (file)
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -485,20 +485,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
        if (ret)
                goto err_remove_edma;
 
-       if (dw_pcie_link_up(pci)) {
-               dw_pcie_print_link_status(pci);
-       } else {
+       if (!dw_pcie_link_up(pci)) {
                ret = dw_pcie_start_link(pci);
                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;
-               }
        }
 
+       /* Ignore errors, the link may come up later */
+       dw_pcie_wait_for_link(pci);
+
        bridge->sysdata = pp;
 
        ret = pci_host_probe(bridge);

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c87848cd8686c0337bbd2db21f9c7b6d66bf9001..1f2ee71da4da2ad87655eaac3172186ae4c58124 100644 (file)
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -644,20 +644,9 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
        dw_pcie_writel_atu(pci, dir, index, PCIE_ATU_REGION_CTRL2, 0);
 }
 
-void dw_pcie_print_link_status(struct dw_pcie *pci)
-{
-       u32 offset, val;
-
-       offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-       val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
-
-       dev_info(pci->dev, "PCIe Gen.%u x%u link up\n",
-                FIELD_GET(PCI_EXP_LNKSTA_CLS, val),
-                FIELD_GET(PCI_EXP_LNKSTA_NLW, val));
-}
-
 int dw_pcie_wait_for_link(struct dw_pcie *pci)
 {
+       u32 offset, val;
        int retries;
 
        /* Check if the link is up or not */
@@ -673,7 +662,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
                return -ETIMEDOUT;
        }
 
-       dw_pcie_print_link_status(pci);
+       offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+       val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+
+       dev_info(pci->dev, "PCIe Gen.%u x%u link up\n",
+                FIELD_GET(PCI_EXP_LNKSTA_CLS, val),
+                FIELD_GET(PCI_EXP_LNKSTA_NLW, val));
 
        return 0;
 }

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 615660640801968cf5b76d36b9ceab3c3984d3f1..79713ce075cc1b6b3c1eff6b8535813c87837076 100644 (file)
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -429,7 +429,6 @@ void dw_pcie_setup(struct dw_pcie *pci);
 void dw_pcie_iatu_detect(struct dw_pcie *pci);
 int dw_pcie_edma_detect(struct dw_pcie *pci);
 void dw_pcie_edma_remove(struct dw_pcie *pci);
-void dw_pcie_print_link_status(struct dw_pcie *pci);
 
 static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
 {