* [PATCH v4] PCI: qcom: Add system PM support
@ 2022-03-07 18:55 Prasad Malisetty
2022-03-17 22:07 ` Stephen Boyd
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Prasad Malisetty @ 2022-03-07 18:55 UTC (permalink / raw)
To: agross, bjorn.andersson, lorenzo.pieralisi, robh, kw, bhelgaas,
linux-pci, linux-arm-msm, linux-kernel, rajatja, refactormyself
Cc: quic_vbadigan, quic_ramkri, manivannan.sadhasivam, swboyd,
Prasad Malisetty
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 since v3:
- Replaced noirq hooks with normal suspend/resume hooks.
- Removed local variable and placed in function itself.
Changes since 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 | 97 ++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6ab9089..4d29c80 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
@@ -1645,6 +1650,97 @@ 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;
+ 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, 100000);
+ 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->cfg->ops->post_deinit)
+ pcie->cfg->ops->post_deinit(pcie);
+
+ /* Disable PCIe clocks and regulators */
+ pcie->cfg->ops->deinit(pcie);
+}
+
+static int __maybe_unused qcom_pcie_pm_suspend(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(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 ret;
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops qcom_pcie_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend, qcom_pcie_pm_resume)
+};
+
static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
@@ -1679,6 +1775,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,
},
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-03-07 18:55 [PATCH v4] PCI: qcom: Add system PM support Prasad Malisetty
@ 2022-03-17 22:07 ` Stephen Boyd
2022-03-18 14:34 ` Manivannan Sadhasivam
2022-04-12 6:01 ` Manivannan Sadhasivam
2 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-03-17 22:07 UTC (permalink / raw)
To: Prasad Malisetty, agross, bhelgaas, bjorn.andersson, kw,
linux-arm-msm, linux-kernel, linux-pci, lorenzo.pieralisi,
rajatja, refactormyself, robh
Cc: quic_vbadigan, quic_ramkri, manivannan.sadhasivam
Quoting Prasad Malisetty (2022-03-07 10:55:06)
> Add suspend_noirq and resume_noirq callbacks to handle
Nitpick: They're no longer noirq so this should say "Add suspend and
resume callbacks"
> 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.
I think Bjorn A. and Dmitry are trying to avoid this whole parking thing
in another series? Can you take a look at that?
>
> When system resumes, PCIe link will be re-established and
> setup rc settings.
>
> Signed-off-by: Prasad Malisetty <quic_pmaliset@quicinc.com>
>
> ---
> Changes since v3:
> - Replaced noirq hooks with normal suspend/resume hooks.
> - Removed local variable and placed in function itself.
>
> Changes since 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 | 97 ++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..4d29c80 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
>
> @@ -1645,6 +1650,97 @@ 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;
> + 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, 100000);
> + 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->cfg->ops->post_deinit)
> + pcie->cfg->ops->post_deinit(pcie);
> +
> + /* Disable PCIe clocks and regulators */
> + pcie->cfg->ops->deinit(pcie);
> +}
> +
> +static int __maybe_unused qcom_pcie_pm_suspend(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(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");
Capitalize cannot?
> + 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 ret;
Drop return and braces.
> + }
> +
> + return 0;
return ret;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend, qcom_pcie_pm_resume)
> +};
> +
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-03-07 18:55 [PATCH v4] PCI: qcom: Add system PM support Prasad Malisetty
2022-03-17 22:07 ` Stephen Boyd
@ 2022-03-18 14:34 ` Manivannan Sadhasivam
2022-04-12 6:01 ` Manivannan Sadhasivam
2 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-18 14:34 UTC (permalink / raw)
To: Prasad Malisetty
Cc: agross, bjorn.andersson, lorenzo.pieralisi, robh, kw, bhelgaas,
linux-pci, linux-arm-msm, linux-kernel, rajatja, refactormyself,
quic_vbadigan, quic_ramkri, swboyd
On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
> 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 since v3:
> - Replaced noirq hooks with normal suspend/resume hooks.
> - Removed local variable and placed in function itself.
>
> Changes since 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 | 97 ++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..4d29c80 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
BIT(0)
>
> #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
>
> @@ -1645,6 +1650,97 @@ 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;
> + 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, 100000);
> + 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);
> +
In the modem usecase, the endpoint uses the LINK_DOWN event for freeing up the
resources. If PERST# gets asserted before turing off the PHY, the LINK_DOWN
event will be missed in the endpoint. And moreover, modem cannot free up the
resources in PERST# handler as it is a hard IRQ handler and the cleanup action
might sleep. The deferring also doesn't work because, once PERST# event get's
handled, the host will turn off the refclk and the access to DBI region will
not be allowed.
For this reason, I'd prefer to move this PERST# assertion to the end of the
funtion.
> + /* Put PHY into POWER DOWN state */
> + phy_power_off(pcie->phy);
> +
> + writel(PHY_POWER_DOWN, pcie->parf + PCIE20_PARF_PHY_CTRL);
> +
> + if (pcie->cfg->ops->post_deinit)
> + pcie->cfg->ops->post_deinit(pcie);
> +
> + /* Disable PCIe clocks and regulators */
> + pcie->cfg->ops->deinit(pcie);
It is also required to add a 100ms delay here as PERST# reaches quickly before
LINK_DOWN.
/*
* Allow the LINK_DOWN event to reach the endpoint before PERST# assert.
* This is required for resource cleanup in the endpoint for modem usecase.
*/
usleep_range(50000, 100000);
qcom_ep_reset_assert(pcie);
> +}
> +
> +static int __maybe_unused qcom_pcie_pm_suspend(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");
I think this debug message doesn't add any value, so just skip it.
> + return 0;
> + }
> +
> + ret = qcom_pcie_send_pme_turnoff_msg(pcie);
> + if (ret)
> + return ret;
> +
> + /* Power down the PHY, disable clock and regulators */
You are also asserting PERST#.
> + qcom_pcie_host_disable(pcie);
> +
> + return 0;
> +}
> +
> +/* Resume the PCIe link */
> +static int __maybe_unused qcom_pcie_pm_resume(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");
dw_pcie_wait_for_link() itself prints the error in failure case. So just return
directly.
return dw_pcie_wait_for_link(pci);
Thanks,
Mani
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend, qcom_pcie_pm_resume)
> +};
> +
> static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
> { .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
> @@ -1679,6 +1775,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,
> },
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-03-07 18:55 [PATCH v4] PCI: qcom: Add system PM support Prasad Malisetty
2022-03-17 22:07 ` Stephen Boyd
2022-03-18 14:34 ` Manivannan Sadhasivam
@ 2022-04-12 6:01 ` Manivannan Sadhasivam
2022-04-12 10:40 ` Kalle Valo
2 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-12 6:01 UTC (permalink / raw)
To: Prasad Malisetty
Cc: agross, bjorn.andersson, lorenzo.pieralisi, robh, kw, bhelgaas,
linux-pci, linux-arm-msm, linux-kernel, rajatja, refactormyself,
quic_vbadigan, quic_ramkri, swboyd, kvalo, linux-wireless
+Kalle, linux-wireless
On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
> 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.
>
Kalle, is this behaviour appropriate for WLAN devices as well? The devices
will be put into poweroff state (assuming no Vaux provided in D3cold) during
system suspend.
Thanks,
Mani
> When system resumes, PCIe link will be re-established and
> setup rc settings.
>
> Signed-off-by: Prasad Malisetty <quic_pmaliset@quicinc.com>
>
> ---
> Changes since v3:
> - Replaced noirq hooks with normal suspend/resume hooks.
> - Removed local variable and placed in function itself.
>
> Changes since 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 | 97 ++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..4d29c80 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
>
> @@ -1645,6 +1650,97 @@ 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;
> + 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, 100000);
> + 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->cfg->ops->post_deinit)
> + pcie->cfg->ops->post_deinit(pcie);
> +
> + /* Disable PCIe clocks and regulators */
> + pcie->cfg->ops->deinit(pcie);
> +}
> +
> +static int __maybe_unused qcom_pcie_pm_suspend(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(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 ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(qcom_pcie_pm_suspend, qcom_pcie_pm_resume)
> +};
> +
> static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
> { .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
> @@ -1679,6 +1775,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,
> },
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-04-12 6:01 ` Manivannan Sadhasivam
@ 2022-04-12 10:40 ` Kalle Valo
2022-04-13 5:49 ` Manivannan Sadhasivam
0 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2022-04-12 10:40 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Prasad Malisetty, agross, bjorn.andersson, lorenzo.pieralisi,
robh, kw, bhelgaas, linux-pci, linux-arm-msm, linux-kernel,
rajatja, refactormyself, quic_vbadigan, quic_ramkri, swboyd,
linux-wireless, ath11k
+ ath11k
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> +Kalle, linux-wireless
>
> On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
>> 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.
>>
>
> Kalle, is this behaviour appropriate for WLAN devices as well? The devices
> will be put into poweroff state (assuming no Vaux provided in D3cold) during
> system suspend.
ath11k leaves the firmware running during suspend. I don't fully
understand what the patch is doing, but if it cuts the power from the
WLAN chip during suspend that will break ath11k.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-04-12 10:40 ` Kalle Valo
@ 2022-04-13 5:49 ` Manivannan Sadhasivam
2022-04-13 6:36 ` Dmitry Baryshkov
0 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-13 5:49 UTC (permalink / raw)
To: Kalle Valo
Cc: Prasad Malisetty, agross, bjorn.andersson, lorenzo.pieralisi,
robh, kw, bhelgaas, linux-pci, linux-arm-msm, linux-kernel,
rajatja, refactormyself, quic_vbadigan, quic_ramkri, swboyd,
linux-wireless, ath11k
On Tue, Apr 12, 2022 at 01:40:08PM +0300, Kalle Valo wrote:
> + ath11k
>
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
>
> > +Kalle, linux-wireless
> >
> > On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
> >> 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.
> >>
> >
> > Kalle, is this behaviour appropriate for WLAN devices as well? The devices
> > will be put into poweroff state (assuming no Vaux provided in D3cold) during
> > system suspend.
>
> ath11k leaves the firmware running during suspend. I don't fully
> understand what the patch is doing, but if it cuts the power from the
> WLAN chip during suspend that will break ath11k.
>
Thanks Kalle for the confirmation. Yes the device will be put into D3cold state
and that will most likely equal to poweroff state.
Prasad, you should try to just turn off the host resources like clocks and
regulators (not refclk) and let the device be in the default state
(D3hot/L{0/1}?) during suspend.
Thanks,
Mani
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-04-13 5:49 ` Manivannan Sadhasivam
@ 2022-04-13 6:36 ` Dmitry Baryshkov
2022-04-13 7:05 ` Manivannan Sadhasivam
0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2022-04-13 6:36 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Kalle Valo, Prasad Malisetty, agross, bjorn.andersson,
lorenzo.pieralisi, robh, kw, bhelgaas, linux-pci, linux-arm-msm,
linux-kernel, rajatja, refactormyself, quic_vbadigan,
quic_ramkri, swboyd, linux-wireless, ath11k
On Wed, 13 Apr 2022 at 08:49, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Tue, Apr 12, 2022 at 01:40:08PM +0300, Kalle Valo wrote:
> > + ath11k
> >
> > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> >
> > > +Kalle, linux-wireless
> > >
> > > On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
> > >> 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.
> > >>
> > >
> > > Kalle, is this behaviour appropriate for WLAN devices as well? The devices
> > > will be put into poweroff state (assuming no Vaux provided in D3cold) during
> > > system suspend.
> >
> > ath11k leaves the firmware running during suspend. I don't fully
> > understand what the patch is doing, but if it cuts the power from the
> > WLAN chip during suspend that will break ath11k.
> >
>
> Thanks Kalle for the confirmation. Yes the device will be put into D3cold state
> and that will most likely equal to poweroff state.
Just to remind that ath11k on Qualcomm boards has a separate power
supply, not directly tied to the PCIe power supply.
> Prasad, you should try to just turn off the host resources like clocks and
> regulators (not refclk) and let the device be in the default state
> (D3hot/L{0/1}?) during suspend.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] PCI: qcom: Add system PM support
2022-04-13 6:36 ` Dmitry Baryshkov
@ 2022-04-13 7:05 ` Manivannan Sadhasivam
0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-13 7:05 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Kalle Valo, Prasad Malisetty, agross, bjorn.andersson,
lorenzo.pieralisi, robh, kw, bhelgaas, linux-pci, linux-arm-msm,
linux-kernel, rajatja, refactormyself, quic_vbadigan,
quic_ramkri, swboyd, linux-wireless, ath11k
On Wed, Apr 13, 2022 at 09:36:30AM +0300, Dmitry Baryshkov wrote:
> On Wed, 13 Apr 2022 at 08:49, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Tue, Apr 12, 2022 at 01:40:08PM +0300, Kalle Valo wrote:
> > > + ath11k
> > >
> > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> > >
> > > > +Kalle, linux-wireless
> > > >
> > > > On Tue, Mar 08, 2022 at 12:25:06AM +0530, Prasad Malisetty wrote:
> > > >> 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.
> > > >>
> > > >
> > > > Kalle, is this behaviour appropriate for WLAN devices as well? The devices
> > > > will be put into poweroff state (assuming no Vaux provided in D3cold) during
> > > > system suspend.
> > >
> > > ath11k leaves the firmware running during suspend. I don't fully
> > > understand what the patch is doing, but if it cuts the power from the
> > > WLAN chip during suspend that will break ath11k.
> > >
> >
> > Thanks Kalle for the confirmation. Yes the device will be put into D3cold state
> > and that will most likely equal to poweroff state.
>
> Just to remind that ath11k on Qualcomm boards has a separate power
> supply, not directly tied to the PCIe power supply.
>
It may change in the future or on a different OEM setup. Irrespective of that,
this patch sends the PME_Turn_Off event to the ep devices. The devices in turn
will shutdown the internal resources for poweroff/refclk removal. Therefore the
device state shall be lost.
Thanks,
Mani
> > Prasad, you should try to just turn off the host resources like clocks and
> > regulators (not refclk) and let the device be in the default state
> > (D3hot/L{0/1}?) during suspend.
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-04-13 7:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 18:55 [PATCH v4] PCI: qcom: Add system PM support Prasad Malisetty
2022-03-17 22:07 ` Stephen Boyd
2022-03-18 14:34 ` Manivannan Sadhasivam
2022-04-12 6:01 ` Manivannan Sadhasivam
2022-04-12 10:40 ` Kalle Valo
2022-04-13 5:49 ` Manivannan Sadhasivam
2022-04-13 6:36 ` Dmitry Baryshkov
2022-04-13 7:05 ` Manivannan Sadhasivam
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).