Re: [PATCH 2/2] PCI: qcom: Add support for retraining the link due to link down event

Re: [PATCH 2/2] PCI: qcom: Add support for retraining the link due to link down event

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20250221172309.120009-3-manivannan.sadhasivam@linaro.org>
References: <20250221172309.120009-3-manivannan.sadhasivam@linaro.org>
TO: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
TO: lpieralisi@kernel.org
TO: kw@linux.com
TO: bhelgaas@google.com
CC: linux-pci@vger.kernel.org
CC: linux-arm-msm@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: dingwei@marvell.com
CC: cassel@kernel.org
CC: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Hi Manivannan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.14-rc5 next-20250303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam/PCI-Add-pci_host_bridge_handle_link_down-API-to-handle-the-PCI-link-down-event/20250222-012526
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250221172309.120009-3-manivannan.sadhasivam%40linaro.org
patch subject: [PATCH 2/2] PCI: qcom: Add support for retraining the link due to link down event
:::::: branch date: 10 days ago
:::::: commit date: 10 days ago
config: sparc64-randconfig-r071-20250303 (https://download.01.org/0day-ci/archive/20250304/202503040526.4JdFuLaA-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202503040526.4JdFuLaA-lkp@intel.com/

smatch warnings:
drivers/pci/controller/dwc/pcie-qcom.c:1352 qcom_pcie_retrain_link() warn: missing unwind goto?

vim +1352 drivers/pci/controller/dwc/pcie-qcom.c

82a823833f4e37 drivers/pci/host/pcie-qcom.c           Stanimir Varbanov     2015-12-18  1314  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1315  static int qcom_pcie_retrain_link(struct pci_bus *bus)
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1316  {
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1317  	struct dw_pcie_rp *pp = bus->sysdata;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1318  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1319  	struct qcom_pcie *pcie = to_qcom_pcie(pci);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1320  	struct device *dev = pcie->pci->dev;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1321  	u32 val;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1322  	int ret;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1323  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1324  	/* Wait for the pending transactions to be completed */
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1325  	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_STATUS, val,
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1326  					 val & FLUSH_COMPLETED, 10,
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1327  					 FLUSH_TIMEOUT_US);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1328  	if (ret) {
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1329  		dev_err(dev, "Flush completion failed: %d\n", ret);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1330  		goto err_host_deinit;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1331  	}
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1332  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1333  	/* Clear the FLUSH_MODE to allow the core to be reset */
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1334  	val = readl(pcie->parf + PARF_LTSSM);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1335  	val |= SW_CLEAR_FLUSH_MODE;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1336  	writel(val, pcie->parf + PARF_LTSSM);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1337  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1338  	/* Wait for the FLUSH_MODE to clear */
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1339  	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_LTSSM, val,
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1340  					 !(val & FLUSH_MODE), 10,
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1341  					 FLUSH_TIMEOUT_US);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1342  	if (ret) {
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1343  		dev_err(dev, "Flush mode clear failed: %d\n", ret);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1344  		goto err_host_deinit;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1345  	}
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1346  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1347  	qcom_pcie_host_deinit(pp);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1348  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1349  	ret = qcom_pcie_host_init(pp);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1350  	if (ret) {
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1351  		dev_err(dev, "Host init failed\n");
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21 @1352  		return ret;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1353  	}
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1354  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1355  	ret = dw_pcie_setup_rc(pp);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1356  	if (ret)
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1357  		goto err_host_deinit;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1358  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1359  	/*
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1360  	 * Re-enable global IRQ events as the PARF_INT_ALL_MASK register is
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1361  	 * non-sticky.
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1362  	 */
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1363  	if (pcie->global_irq)
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1364  		writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_ALL_LINK_DOWN |
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1365  			       PARF_INT_MSI_DEV_0_7, pcie->parf + PARF_INT_ALL_MASK);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1366  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1367  	qcom_pcie_start_link(pci);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1368  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1369  	return 0;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1370  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1371  err_host_deinit:
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1372  	qcom_pcie_host_deinit(pp);
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1373  
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1374  	return ret;
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1375  }
6d1377b2f82331 drivers/pci/controller/dwc/pcie-qcom.c Manivannan Sadhasivam 2025-02-21  1376  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 0/2] PCI: Add support for handling link down event from host bridge drivers

[Manivannan Sadhasivam]

Hi,

This series adds support for handling the PCIe link down event from the host
bridge drivers. This series moves the link down event handling to the PCI core
(atleast the generic bits) to prevent the host bridge drivers from retraining
the link on the back of the PCI core as done in [1].

Also, the newly introduced callback 'pci_ops::retrain_link()' could be extended
to recover the bus in the case of errors in the future.

I've implemented the 'pci_ops::retrain_link()' callback in pcie-qcom driver as a
reference.

Testing
=======

This series is tested on Qcom RB5 and SA8775p Ride boards.

[1] https://lore.kernel.org/linux-pci/20241112064813.751736-1-jpatel2@marvell.com

Manivannan Sadhasivam (2):
  PCI: Add pci_host_bridge_handle_link_down() API to handle the PCI link
    down event
  PCI: qcom: Add support for retraining the link due to link down event

 drivers/pci/controller/dwc/pcie-qcom.c | 90 +++++++++++++++++++++++++-
 drivers/pci/probe.c                    | 34 ++++++++++
 include/linux/pci.h                    |  2 +
 3 files changed, 124 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 2/2] PCI: qcom: Add support for retraining the link due to link down event

[Manivannan Sadhasivam]

The PCIe link can go down under circumstances such as the device removed
from the bus, reset condition, etc... When that happens, the link needs to
be retrained back to make it operational again. Currently, the driver is
not handling the link down event, due to which the users have to restart
the machine to make PCIe link operational again. So fix it by detecting the
link down event and adding support to retraining the link.

Since the Qcom PCIe controllers report the link down event through the
'global' IRQ, enable the link down event by setting PARF_INT_ALL_LINK_DOWN
bit in PARF_INT_ALL_MASK register.

Then in the case of the event, call pci_host_bridge_handle_link_down() API
in the handler to let the PCI core handle the link down condition.

The API will internally call, 'pci_ops::retrain_link()' callback to retrain
the link in a platform specific way. So implement the callback to retrain
the link by first resetting the PCIe core, followed by reinitializing the
resources and then finally starting the link again. The PCI core will
finally rescan the bus to enumerate the devices.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 90 +++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index e4d3366ead1f..ebc58e88161e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -55,6 +55,7 @@
 #define PARF_INT_ALL_STATUS			0x224
 #define PARF_INT_ALL_CLEAR			0x228
 #define PARF_INT_ALL_MASK			0x22c
+#define PARF_STATUS				0x230
 #define PARF_SID_OFFSET				0x234
 #define PARF_BDF_TRANSLATE_CFG			0x24c
 #define PARF_DBI_BASE_ADDR_V2			0x350
@@ -130,8 +131,11 @@
 
 /* PARF_LTSSM register fields */
 #define LTSSM_EN				BIT(8)
+#define SW_CLEAR_FLUSH_MODE			BIT(10)
+#define FLUSH_MODE				BIT(11)
 
 /* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
+#define PARF_INT_ALL_LINK_DOWN			BIT(1)
 #define PARF_INT_ALL_LINK_UP			BIT(13)
 #define PARF_INT_MSI_DEV_0_7			GENMASK(30, 23)
 
@@ -145,6 +149,9 @@
 /* PARF_BDF_TO_SID_CFG fields */
 #define BDF_TO_SID_BYPASS			BIT(0)
 
+/* PARF_STATUS fields */
+#define FLUSH_COMPLETED				BIT(8)
+
 /* ELBI_SYS_CTRL register fields */
 #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
 
@@ -169,6 +176,7 @@
 						PCIE_CAP_SLOT_POWER_LIMIT_SCALE)
 
 #define PERST_DELAY_US				1000
+#define FLUSH_TIMEOUT_US			100
 
 #define QCOM_PCIE_CRC8_POLYNOMIAL		(BIT(2) | BIT(1) | BIT(0))
 
@@ -239,6 +247,7 @@ union qcom_pcie_resources {
 };
 
 struct qcom_pcie;
+static struct pci_ops qcom_pcie_bridge_ops;
 
 struct qcom_pcie_ops {
 	int (*get_resources)(struct qcom_pcie *pcie);
@@ -274,6 +283,7 @@ struct qcom_pcie {
 	struct icc_path *icc_cpu;
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
+	int global_irq;
 	bool suspended;
 	bool use_pm_opp;
 };
@@ -1263,6 +1273,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 			goto err_assert_reset;
 	}
 
+	pp->bridge->ops = &qcom_pcie_bridge_ops;
+
 	return 0;
 
 err_assert_reset:
@@ -1300,6 +1312,75 @@ static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
 	.post_init	= qcom_pcie_host_post_init,
 };
 
+static int qcom_pcie_retrain_link(struct pci_bus *bus)
+{
+	struct dw_pcie_rp *pp = bus->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	struct device *dev = pcie->pci->dev;
+	u32 val;
+	int ret;
+
+	/* Wait for the pending transactions to be completed */
+	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_STATUS, val,
+					 val & FLUSH_COMPLETED, 10,
+					 FLUSH_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Flush completion failed: %d\n", ret);
+		goto err_host_deinit;
+	}
+
+	/* Clear the FLUSH_MODE to allow the core to be reset */
+	val = readl(pcie->parf + PARF_LTSSM);
+	val |= SW_CLEAR_FLUSH_MODE;
+	writel(val, pcie->parf + PARF_LTSSM);
+
+	/* Wait for the FLUSH_MODE to clear */
+	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_LTSSM, val,
+					 !(val & FLUSH_MODE), 10,
+					 FLUSH_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Flush mode clear failed: %d\n", ret);
+		goto err_host_deinit;
+	}
+
+	qcom_pcie_host_deinit(pp);
+
+	ret = qcom_pcie_host_init(pp);
+	if (ret) {
+		dev_err(dev, "Host init failed\n");
+		return ret;
+	}
+
+	ret = dw_pcie_setup_rc(pp);
+	if (ret)
+		goto err_host_deinit;
+
+	/*
+	 * Re-enable global IRQ events as the PARF_INT_ALL_MASK register is
+	 * non-sticky.
+	 */
+	if (pcie->global_irq)
+		writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_ALL_LINK_DOWN |
+			       PARF_INT_MSI_DEV_0_7, pcie->parf + PARF_INT_ALL_MASK);
+
+	qcom_pcie_start_link(pci);
+
+	return 0;
+
+err_host_deinit:
+	qcom_pcie_host_deinit(pp);
+
+	return ret;
+}
+
+static struct pci_ops qcom_pcie_bridge_ops = {
+	.map_bus = dw_pcie_own_conf_map_bus,
+	.read = pci_generic_config_read,
+	.write = pci_generic_config_write,
+	.retrain_link = qcom_pcie_retrain_link,
+};
+
 /* Qcom IP rev.: 2.1.0	Synopsys IP rev.: 4.01a */
 static const struct qcom_pcie_ops ops_2_1_0 = {
 	.get_resources = qcom_pcie_get_resources_2_1_0,
@@ -1571,6 +1652,9 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
 		pci_unlock_rescan_remove();
 
 		qcom_pcie_icc_opp_update(pcie);
+	} else if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
+		dev_dbg(dev, "Received Link down event\n");
+		pci_host_bridge_handle_link_down(pp->bridge);
 	} else {
 		dev_WARN_ONCE(dev, 1, "Received unknown event. INT_STATUS: 0x%08x\n",
 			      status);
@@ -1732,8 +1816,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 			goto err_host_deinit;
 		}
 
-		writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_MSI_DEV_0_7,
-			       pcie->parf + PARF_INT_ALL_MASK);
+		writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_ALL_LINK_DOWN |
+			       PARF_INT_MSI_DEV_0_7, pcie->parf + PARF_INT_ALL_MASK);
+
+		pcie->global_irq = irq;
 	}
 
 	qcom_pcie_icc_opp_update(pcie);
-- 
2.25.1