From: Johan Hovold <johan+linaro@kernel.org>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: "Jingoo Han" <jingoohan1@gmail.com>,
"Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Rob Herring" <robh@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Johan Hovold" <johan+linaro@kernel.org>,
"Bjorn Andersson" <quic_bjorande@quicinc.com>,
"Sajid Dalvi" <sdalvi@google.com>,
"Ajay Agarwal" <ajayagarwal@google.com>
Subject: [PATCH] Revert "PCI: dwc: Wait for link up only if link is started"
Date: Thu, 6 Jul 2023 10:26:10 +0200 [thread overview]
Message-ID: <20230706082610.26584-1-johan+linaro@kernel.org> (raw)
This reverts commit da56a1bfbab55189595e588f1d984bdfb5cf5924.
A recent commit broke controller probe by returning an error in case the
link does not come up during host initialisation.
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, and 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.
Fixes: da56a1bfbab5 ("PCI: dwc: Wait for link up only if link is started")
Reported-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Cc: Sajid Dalvi <sdalvi@google.com>
Cc: Ajay Agarwal <ajayagarwal@google.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
.../pci/controller/dwc/pcie-designware-host.c | 13 ++++--------
drivers/pci/controller/dwc/pcie-designware.c | 20 +++++++------------
drivers/pci/controller/dwc/pcie-designware.h | 1 -
3 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cf61733bf78d..9952057c8819 100644
--- 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 df092229e97d..8e33e6e59e68 100644
--- 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 615660640801..79713ce075cc 100644
--- 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)
{
--
2.39.3
next reply other threads:[~2023-07-06 8:26 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-06 8:26 Johan Hovold [this message]
2023-07-06 12:58 ` [PATCH] Revert "PCI: dwc: Wait for link up only if link is started" Manivannan Sadhasivam
2023-07-07 12:47 ` Johan Hovold
2023-07-10 16:21 ` Ajay Agarwal
2023-07-10 16:42 ` Ajay Agarwal
2023-07-10 17:06 ` Krzysztof Wilczyński
2023-07-11 6:52 ` Johan Hovold
2023-07-12 17:45 ` Ajay Agarwal
2023-07-14 8:55 ` Johan Hovold
2024-01-11 15:43 ` Ajay Agarwal
2023-07-11 7:37 ` Manivannan Sadhasivam
2024-01-12 10:00 ` Ajay Agarwal
2024-01-19 7:40 ` Manivannan Sadhasivam
2023-07-25 20:05 ` Bjorn Helgaas
2023-07-26 8:30 ` Johan Hovold
2023-07-26 15:58 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230706082610.26584-1-johan+linaro@kernel.org \
--to=johan+linaro@kernel.org \
--cc=ajayagarwal@google.com \
--cc=bhelgaas@google.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=jingoohan1@gmail.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=quic_bjorande@quicinc.com \
--cc=robh@kernel.org \
--cc=sdalvi@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).