| Message ID | 1645695814-21102-1-git-send-email-quic_pmaliset@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] PCI: qcom: Add system PM support | expand |
Quoting Prasad Malisetty (2022-02-24 01:43:34) > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index c19cd506..f4a5e3c 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -1616,6 +1621,96 @@ static int qcom_pcie_probe(struct platform_device *pdev) > return ret; > } > > +static int qcom_pcie_send_pme_turnoff_msg(struct qcom_pcie *pcie) > +{ > + int ret; > + u32 val, poll_val; > + u64 l23_rdy_poll_timeout = 100000; /* microseconds */ unsigned long instead of u64? But why is it a local variable at all? Just inline it in the one place it is used? > + struct dw_pcie *pci = pcie->pci; > + struct device *dev = pci->dev; > + > + val = readl(pcie->elbi + PCIE20_ELBI_SYS_CTRL); > + val |= PCIE_PME_TURNOFF_MSG; > + writel(val, pcie->elbi + PCIE20_ELBI_SYS_CTRL); > + > + ret = readl_poll_timeout((pcie->parf + PCIE20_PARF_PM_STTS), poll_val, > + (poll_val & PCIE_PM_LINKST_IN_L2), > + 10000, l23_rdy_poll_timeout); > + if (!ret) > + dev_dbg(dev, "Device entered L23_Ready state\n"); > + else > + dev_err(dev, "Device failed to enter L23_Ready. PM_STTS 0x%x\n", > + readl_relaxed(pcie->parf + PCIE20_PARF_PM_STTS)); > + > + return ret; > +} > + > +static void qcom_pcie_host_disable(struct qcom_pcie *pcie) > +{ > + qcom_ep_reset_assert(pcie); > + > + /* Put PHY into POWER DOWN state */ > + phy_power_off(pcie->phy); > + > + writel(PHY_POWER_DOWN, pcie->parf + PCIE20_PARF_PHY_CTRL); > + > + if (pcie->ops->post_deinit) > + pcie->ops->post_deinit(pcie); > + > + /* Disable PCIe clocks and regulators */ > + pcie->ops->deinit(pcie); > +} > + > +static int __maybe_unused qcom_pcie_pm_suspend_noirq(struct device *dev) > +{ > + int ret; > + struct qcom_pcie *pcie = dev_get_drvdata(dev); > + struct dw_pcie *pci = pcie->pci; > + > + if (!dw_pcie_link_up(pci)) { > + dev_dbg(dev, "Power has been turned off already\n"); > + return 0; > + } > + > + ret = qcom_pcie_send_pme_turnoff_msg(pcie); > + if (ret) > + return ret; > + > + /* Power down the PHY, disable clock and regulators */ > + qcom_pcie_host_disable(pcie); > + > + return 0; > +} > + > +/* Resume the PCIe link */ > +static int __maybe_unused qcom_pcie_pm_resume_noirq(struct device *dev) > +{ > + int ret; > + struct qcom_pcie *pcie = dev_get_drvdata(dev); > + struct dw_pcie *pci = pcie->pci; > + struct pcie_port *pp = &pci->pp; > + > + ret = qcom_pcie_host_init(pp); > + if (ret) { > + dev_err(dev, "cannot initialize host\n"); > + return ret; > + } > + > + dw_pcie_setup_rc(pp); > + > + qcom_pcie_start_link(pci); > + > + ret = dw_pcie_wait_for_link(pci); > + if (ret) > + dev_err(dev, "Link never came up, Resume failed\n"); But we ignore and don't return ret? Please add a comment about why that's done, or return ret below. > + > + return 0; > +} > + > +static const struct dev_pm_ops qcom_pcie_pm_ops = { > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend_noirq, qcom_pcie_pm_resume_noirq) Why is noirq used? Please add a comment. And better yet don't use noirq hooks and use the normal suspend/resume hooks. > +}; > + > static const struct of_device_id qcom_pcie_match[] = { > { .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg }, > { .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index c19cd506..f4a5e3c 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -48,6 +48,7 @@ #define PCIE20_PARF_PHY_REFCLK 0x4C #define PHY_REFCLK_SSP_EN BIT(16) #define PHY_REFCLK_USE_PAD BIT(12) +#define PHY_POWER_DOWN 0x1 #define PCIE20_PARF_DBI_BASE_ADDR 0x168 #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE 0x16C @@ -62,6 +63,8 @@ #define PCIE20_ELBI_SYS_CTRL 0x04 #define PCIE20_ELBI_SYS_CTRL_LT_ENABLE BIT(0) +#define PCIE_PME_TURNOFF_MSG BIT(4) +#define PCIE_PM_LINKST_IN_L2 BIT(5) #define PCIE20_AXI_MSTR_RESP_COMP_CTRL0 0x818 #define CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K 0x4 @@ -73,6 +76,8 @@ #define PCIE20_PARF_Q2A_FLUSH 0x1AC +#define PCIE20_PARF_PM_STTS 0x24 + #define PCIE20_MISC_CONTROL_1_REG 0x8BC #define DBI_RO_WR_EN 1 @@ -1616,6 +1621,96 @@ static int qcom_pcie_probe(struct platform_device *pdev) return ret; } +static int qcom_pcie_send_pme_turnoff_msg(struct qcom_pcie *pcie) +{ + int ret; + u32 val, poll_val; + u64 l23_rdy_poll_timeout = 100000; /* microseconds */ + struct dw_pcie *pci = pcie->pci; + struct device *dev = pci->dev; + + val = readl(pcie->elbi + PCIE20_ELBI_SYS_CTRL); + val |= PCIE_PME_TURNOFF_MSG; + writel(val, pcie->elbi + PCIE20_ELBI_SYS_CTRL); + + ret = readl_poll_timeout((pcie->parf + PCIE20_PARF_PM_STTS), poll_val, + (poll_val & PCIE_PM_LINKST_IN_L2), + 10000, l23_rdy_poll_timeout); + if (!ret) + dev_dbg(dev, "Device entered L23_Ready state\n"); + else + dev_err(dev, "Device failed to enter L23_Ready. PM_STTS 0x%x\n", + readl_relaxed(pcie->parf + PCIE20_PARF_PM_STTS)); + + return ret; +} + +static void qcom_pcie_host_disable(struct qcom_pcie *pcie) +{ + qcom_ep_reset_assert(pcie); + + /* Put PHY into POWER DOWN state */ + phy_power_off(pcie->phy); + + writel(PHY_POWER_DOWN, pcie->parf + PCIE20_PARF_PHY_CTRL); + + if (pcie->ops->post_deinit) + pcie->ops->post_deinit(pcie); + + /* Disable PCIe clocks and regulators */ + pcie->ops->deinit(pcie); +} + +static int __maybe_unused qcom_pcie_pm_suspend_noirq(struct device *dev) +{ + int ret; + struct qcom_pcie *pcie = dev_get_drvdata(dev); + struct dw_pcie *pci = pcie->pci; + + if (!dw_pcie_link_up(pci)) { + dev_dbg(dev, "Power has been turned off already\n"); + return 0; + } + + ret = qcom_pcie_send_pme_turnoff_msg(pcie); + if (ret) + return ret; + + /* Power down the PHY, disable clock and regulators */ + qcom_pcie_host_disable(pcie); + + return 0; +} + +/* Resume the PCIe link */ +static int __maybe_unused qcom_pcie_pm_resume_noirq(struct device *dev) +{ + int ret; + struct qcom_pcie *pcie = dev_get_drvdata(dev); + struct dw_pcie *pci = pcie->pci; + struct pcie_port *pp = &pci->pp; + + ret = qcom_pcie_host_init(pp); + if (ret) { + dev_err(dev, "cannot initialize host\n"); + return ret; + } + + dw_pcie_setup_rc(pp); + + qcom_pcie_start_link(pci); + + ret = dw_pcie_wait_for_link(pci); + if (ret) + dev_err(dev, "Link never came up, Resume failed\n"); + + return 0; +} + +static const struct dev_pm_ops qcom_pcie_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend_noirq, qcom_pcie_pm_resume_noirq) +}; + static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg }, { .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg }, @@ -1648,6 +1743,7 @@ static struct platform_driver qcom_pcie_driver = { .probe = qcom_pcie_probe, .driver = { .name = "qcom-pcie", + .pm = &qcom_pcie_pm_ops, .suppress_bind_attrs = true, .of_match_table = qcom_pcie_match, },
Add suspend_noirq and resume_noirq callbacks to handle system suspend and resume in dwc PCIe controller driver. When system suspends, send PME turnoff message to enter link into L2 state. Along with powerdown the PHY, disable pipe clock, switch gcc_pcie_1_pipe_clk_src to XO if mux is supported and disable the pcie clocks, regulators. When system resumes, PCIe link will be re-established and setup rc settings. Signed-off-by: Prasad Malisetty <quic_pmaliset@quicinc.com> --- Changes in v2: - Removed unnecessary variable initializations and comments. - Removed platform specific variables declarations. - Added MACRO names for the BIT shiftings. Changes since v1: - Removed unnecessary logs and modified log level suggested by Manivannan. - Removed platform specific callbacks as PM support is generic. --- drivers/pci/controller/dwc/pcie-qcom.c | 96 ++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+)