[PATCH v3] PCI: qcom-ep: Enable controller resources like PHY only after refclk is available

[PATCH v3] PCI: qcom-ep: Enable controller resources like PHY only after refclk is available

[Manivannan Sadhasivam]

qcom_pcie_enable_resources() is called by qcom_pcie_ep_probe() and it
enables the controller resources like clocks, regulator, PHY. On one of the
new unreleased Qcom SoC, PHY enablement depends on the active refclk. And
on all of the supported Qcom endpoint SoCs, refclk comes from the host
(RC). So calling qcom_pcie_enable_resources() without refclk causes the
NoC (Network On Chip) error in the endpoint SoC and in turn results in a
whole SoC crash and rebooting into EDL (Emergency Download) mode which is
an unrecoverable state.

But qcom_pcie_enable_resources() is already called by
qcom_pcie_perst_deassert() when PERST# is deasserted, and refclk is
available at that time.

Hence, remove the unnecessary call to qcom_pcie_enable_resources() from
qcom_pcie_ep_probe() to prevent the above mentioned crash.

It should be noted that this commit prevents the crash only under normal
working condition (booting endpoint before host), but the crash may also
occur if PERST# assert happens at the wrong time. For avoiding the crash
completely, it is recommended to use SRIS mode which allows the endpoint
SoC to generate its own refclk. The driver is not supporting SRIS mode
currently, but will be added in the future.

Fixes: 869bc5253406 ("PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host")
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

Changes in v3:

* Improved the patch description to mention that the crash in an unrecoverable
  state and also that it may occur if PERST# is asserted at the wrong time.

Changes in v2:

- Changed the patch description to mention the crash clearly as suggested by
  Bjorn
- Added the Fixes tag

Bjorn: This is not going to be a 6.11 material as it is not fixing a regression
introduced in 6.11 (offending commit got merged in 6.10). So please merge this
patch for 6.12.

 drivers/pci/controller/dwc/pcie-qcom-ep.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 236229f66c80..2319ff2ae9f6 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -846,21 +846,15 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = qcom_pcie_enable_resources(pcie_ep);
-	if (ret) {
-		dev_err(dev, "Failed to enable resources: %d\n", ret);
-		return ret;
-	}
-
 	ret = dw_pcie_ep_init(&pcie_ep->pci.ep);
 	if (ret) {
 		dev_err(dev, "Failed to initialize endpoint: %d\n", ret);
-		goto err_disable_resources;
+		return ret;
 	}
 
 	ret = qcom_pcie_ep_enable_irq_resources(pdev, pcie_ep);
 	if (ret)
-		goto err_disable_resources;
+		goto err_ep_deinit;
 
 	name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node);
 	if (!name) {
@@ -877,8 +871,8 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
 	disable_irq(pcie_ep->global_irq);
 	disable_irq(pcie_ep->perst_irq);
 
-err_disable_resources:
-	qcom_pcie_disable_resources(pcie_ep);
+err_ep_deinit:
+	dw_pcie_ep_deinit(&pcie_ep->pci.ep);
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH v3] PCI: qcom-ep: Enable controller resources like PHY only after refclk is available

[Krzysztof Wilczyński]

Hello,

> qcom_pcie_enable_resources() is called by qcom_pcie_ep_probe() and it
> enables the controller resources like clocks, regulator, PHY. On one of the
> new unreleased Qcom SoC, PHY enablement depends on the active refclk. And
> on all of the supported Qcom endpoint SoCs, refclk comes from the host
> (RC). So calling qcom_pcie_enable_resources() without refclk causes the
> NoC (Network On Chip) error in the endpoint SoC and in turn results in a
> whole SoC crash and rebooting into EDL (Emergency Download) mode which is
> an unrecoverable state.
> 
> But qcom_pcie_enable_resources() is already called by
> qcom_pcie_perst_deassert() when PERST# is deasserted, and refclk is
> available at that time.
> 
> Hence, remove the unnecessary call to qcom_pcie_enable_resources() from
> qcom_pcie_ep_probe() to prevent the above mentioned crash.
> 
> It should be noted that this commit prevents the crash only under normal
> working condition (booting endpoint before host), but the crash may also
> occur if PERST# assert happens at the wrong time. For avoiding the crash
> completely, it is recommended to use SRIS mode which allows the endpoint
> SoC to generate its own refclk. The driver is not supporting SRIS mode
> currently, but will be added in the future.

Applied to controller/qcom, thank you!

[1/1] PCI: qcom-ep: Enable controller resources like PHY only after refclk is available
      https://git.kernel.org/pci/pci/c/d3745e3ae6c0

	Krzysztof