Skip to content

Commit c5097b9

Browse files
jhovoldbjorn-helgaas
authored andcommitted
Revert "PCI: dwc: Wait for link up only if link is started"
This reverts commit da56a1b. Bjorn Andersson, Fabio Estevam, Xiaolei Wang, and Jon Hunter reported that da56a1b ("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 886a9c1 ("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 c0e1eb4 ("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: da56a1b ("PCI: dwc: Wait for link up only if link is started") Link: https://lore.kernel.org/r/[email protected] Reported-by: Bjorn Andersson <[email protected]> Signed-off-by: Johan Hovold <[email protected]> Reported-by: Fabio Estevam <[email protected]> Link: https://lore.kernel.org/r/[email protected]/ Reported-by: Xiaolei Wang <[email protected]> Link: https://lore.kernel.org/r/[email protected]/ Reported-by: Jon Hunter <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Bjorn Helgaas <[email protected]> Reviewed-by: Manivannan Sadhasivam <[email protected]> Cc: Sajid Dalvi <[email protected]> Cc: Ajay Agarwal <[email protected]>
1 parent 37540db commit c5097b9

File tree

3 files changed

+11
-23
lines changed

3 files changed

+11
-23
lines changed

drivers/pci/controller/dwc/pcie-designware-host.c

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -485,20 +485,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
485485
if (ret)
486486
goto err_remove_edma;
487487

488-
if (dw_pcie_link_up(pci)) {
489-
dw_pcie_print_link_status(pci);
490-
} else {
488+
if (!dw_pcie_link_up(pci)) {
491489
ret = dw_pcie_start_link(pci);
492490
if (ret)
493491
goto err_remove_edma;
494-
495-
if (pci->ops && pci->ops->start_link) {
496-
ret = dw_pcie_wait_for_link(pci);
497-
if (ret)
498-
goto err_stop_link;
499-
}
500492
}
501493

494+
/* Ignore errors, the link may come up later */
495+
dw_pcie_wait_for_link(pci);
496+
502497
bridge->sysdata = pp;
503498

504499
ret = pci_host_probe(bridge);

drivers/pci/controller/dwc/pcie-designware.c

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -644,20 +644,9 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index)
644644
dw_pcie_writel_atu(pci, dir, index, PCIE_ATU_REGION_CTRL2, 0);
645645
}
646646

647-
void dw_pcie_print_link_status(struct dw_pcie *pci)
648-
{
649-
u32 offset, val;
650-
651-
offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
652-
val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
653-
654-
dev_info(pci->dev, "PCIe Gen.%u x%u link up\n",
655-
FIELD_GET(PCI_EXP_LNKSTA_CLS, val),
656-
FIELD_GET(PCI_EXP_LNKSTA_NLW, val));
657-
}
658-
659647
int dw_pcie_wait_for_link(struct dw_pcie *pci)
660648
{
649+
u32 offset, val;
661650
int retries;
662651

663652
/* Check if the link is up or not */
@@ -673,7 +662,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
673662
return -ETIMEDOUT;
674663
}
675664

676-
dw_pcie_print_link_status(pci);
665+
offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
666+
val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
667+
668+
dev_info(pci->dev, "PCIe Gen.%u x%u link up\n",
669+
FIELD_GET(PCI_EXP_LNKSTA_CLS, val),
670+
FIELD_GET(PCI_EXP_LNKSTA_NLW, val));
677671

678672
return 0;
679673
}

drivers/pci/controller/dwc/pcie-designware.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,6 @@ void dw_pcie_setup(struct dw_pcie *pci);
429429
void dw_pcie_iatu_detect(struct dw_pcie *pci);
430430
int dw_pcie_edma_detect(struct dw_pcie *pci);
431431
void dw_pcie_edma_remove(struct dw_pcie *pci);
432-
void dw_pcie_print_link_status(struct dw_pcie *pci);
433432

434433
static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
435434
{

0 commit comments

Comments
 (0)