On Fri, Apr 18, 2025 at 08:11:47AM +0530, Krishna Chaitanya Chundru wrote:
>
>
> On 4/17/2025 10:46 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>
> >
> > The PCIe link can go down under circumstances such as the device firmware
> > crash, link instability, etc... When that happens, the PCIe slot needs to
> > be reset 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 resetting the slot.
> >
> > 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_handle_link_down() API
> > in the handler to let the PCI core handle the link down condition.
> >
> > The API will internally call, 'pci_host_bridge::reset_slot()' callback to
> > reset the slot in a platform specific way. So implement the callback to
> > reset the slot by first resetting the PCIe core, followed by reinitializing
> > the resources and then finally starting the link again.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 1 +
> > drivers/pci/controller/dwc/pcie-qcom.c | 90
> > +++++++++++++++++++++++++++++++++-
> > 2 files changed, 89 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig
> > b/drivers/pci/controller/dwc/Kconfig
> > index
> > d9f0386396edf66ad0e514a0f545ed24d89fcb6c..ce04ee6fbd99cbcce5d2f3a75ebd72a17070b7b7
> > 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -296,6 +296,7 @@ config PCIE_QCOM
> > select PCIE_DW_HOST
> > select CRC8
> > select PCIE_QCOM_COMMON
> > + select PCI_HOST_COMMON
> > help
> > Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> > PCIe controller uses the DesignWare core plus Qualcomm-specific
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > b/drivers/pci/controller/dwc/pcie-qcom.c
> > index
> > dc98ae63362db0422384b1879a2b9a7dc564d091..6b18a2775e7fcde1d634b3f58327ecc7d028e4ec
> > 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -34,6 +34,7 @@
> > #include <linux/units.h>
> > #include "../../pci.h"
> > +#include "../pci-host-common.h"
> > #include "pcie-designware.h"
> > #include "pcie-qcom-common.h"
> > @@ -55,6 +56,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 +132,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 +150,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 +177,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))
> > @@ -274,11 +283,14 @@ 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;
> > };
> > #define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
> > +static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge,
> > + struct pci_dev *pdev);
> > static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> > {
> > @@ -1263,6 +1275,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > goto err_assert_reset;
> > }
> > + pp->bridge->reset_slot = qcom_pcie_reset_slot;
> > +
> > return 0;
> > err_assert_reset:
> > @@ -1300,6 +1314,73 @@ static const struct dw_pcie_host_ops
> > qcom_pcie_dw_ops = {
> > .post_init = qcom_pcie_host_post_init,
> > };
> > +static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge,
> > + struct pci_dev *pdev)
> > +{
> > + struct pci_bus *bus = bridge->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);
> do we need to enable linkup again here, since all the devices are
> enumerated previously, the linkup irq will do a rescan again which is
> not needed.
Right. I was trying to keep the irq enablement on par with probe(), but LINK_UP
is strictly not needed. I will drop it.
> Instead of linkup we update icc & opp bandwidths after
> dw_pcie_wait_for_link() in the below.
>
Why do we need to update ICC and OPP?
- Mani
--
மணிவண்ணன் சதாசிவம்
