diff mbox series

[RFC,6/7] pci: qcom: Add support for start_link() & stop_link()

Message ID 20240626-qps615-v1-6-2ade7bd91e02@quicinc.com (mailing list archive)
State RFC
Delegated to: Krzysztof WilczyƄski
Headers show
Series PCI: enable Power and configure the QPS615 PCIe switch | expand

Commit Message

Krishna Chaitanya Chundru June 26, 2024, 12:37 p.m. UTC
In the stop_link() if the PCIe link is not up, disable LTSSM enable
bit to stop link training otherwise keep the link in D3cold.
And in the start_link() the enable LTSSM bit if the resources are
turned on other wise do the all the initialization and then start
the link.

Introduce ltssm_disable function op to stop the link training.

Use a flag 'pci_pwrctl_turned_off" to indicate the resources are
turned off by the pci pwrctl framework.

If the link is stopped using the stop_link() then just return with
doing anything in suspend and resume.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 108 +++++++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 11 deletions(-)

Comments

Bjorn Andersson June 26, 2024, 2:31 p.m. UTC | #1
On Wed, Jun 26, 2024 at 06:07:54PM GMT, Krishna chaitanya chundru wrote:

Please start your commit message with a description of the problem that
you're solving, then followed by the technical description of your
solution (which you have here).

Also, uppercase "PCI:" in your subject prefix.

> In the stop_link() if the PCIe link is not up, disable LTSSM enable
> bit to stop link training otherwise keep the link in D3cold.
> And in the start_link() the enable LTSSM bit if the resources are
> turned on other wise do the all the initialization and then start
> the link.
> 
> Introduce ltssm_disable function op to stop the link training.
> 
> Use a flag 'pci_pwrctl_turned_off" to indicate the resources are
> turned off by the pci pwrctl framework.
> 
> If the link is stopped using the stop_link() then just return with
> doing anything in suspend and resume.

And an empty line between commit message and the tags.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 108 +++++++++++++++++++++++++++++----
>  1 file changed, 97 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 14772edcf0d3..1ab3ffdb3914 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -37,6 +37,7 @@
>  /* PARF registers */
>  #define PARF_SYS_CTRL				0x00
>  #define PARF_PM_CTRL				0x20
> +#define PARF_PM_STTS				0x24
>  #define PARF_PCS_DEEMPH				0x34
>  #define PARF_PCS_SWING				0x38
>  #define PARF_PHY_CTRL				0x40
> @@ -83,6 +84,9 @@
>  /* PARF_PM_CTRL register fields */
>  #define REQ_NOT_ENTR_L1				BIT(5)
>  
> +/* PARF_PM_STTS register fields */
> +#define PM_ENTER_L23				BIT(5)
> +
>  /* PARF_PCS_DEEMPH register fields */
>  #define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		FIELD_PREP(GENMASK(21, 16), x)
>  #define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	FIELD_PREP(GENMASK(13, 8), x)
> @@ -126,6 +130,7 @@
>  
>  /* ELBI_SYS_CTRL register fields */
>  #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
> +#define ELBI_SYS_CTRL_PME_TURNOFF_MSG		BIT(4)
>  
>  /* AXI_MSTR_RESP_COMP_CTRL0 register fields */
>  #define CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K	0x4
> @@ -228,6 +233,7 @@ struct qcom_pcie_ops {
>  	void (*host_post_init)(struct qcom_pcie *pcie);
>  	void (*deinit)(struct qcom_pcie *pcie);
>  	void (*ltssm_enable)(struct qcom_pcie *pcie);
> +	void (*ltssm_disable)(struct qcom_pcie *pcie);
>  	int (*config_sid)(struct qcom_pcie *pcie);
>  };
>  
> @@ -248,10 +254,13 @@ struct qcom_pcie {
>  	const struct qcom_pcie_cfg *cfg;
>  	struct dentry *debugfs;
>  	bool suspended;
> +	bool pci_pwrctl_turned_off;
>  };
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>  
> +static void qcom_pcie_icc_update(struct qcom_pcie *pcie);
> +
>  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  {
>  	gpiod_set_value_cansleep(pcie->reset, 1);
> @@ -266,17 +275,6 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
>  	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
>  }
>  
> -static int qcom_pcie_start_link(struct dw_pcie *pci)
> -{
> -	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> -
> -	/* Enable Link Training state machine */
> -	if (pcie->cfg->ops->ltssm_enable)
> -		pcie->cfg->ops->ltssm_enable(pcie);
> -
> -	return 0;
> -}
> -
>  static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
>  {
>  	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -556,6 +554,15 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
>  	return 0;
>  }
>  
> +static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
> +{
> +	u32 val;
> +
> +	val = readl(pcie->parf + PARF_LTSSM);
> +	val &= ~LTSSM_EN;
> +	writel(val, pcie->parf + PARF_LTSSM);
> +}
> +
>  static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
>  {
>  	u32 val;
> @@ -1336,6 +1343,7 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
>  	.post_init = qcom_pcie_post_init_2_7_0,
>  	.deinit = qcom_pcie_deinit_2_7_0,
>  	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>  };
>  
>  /* Qcom IP rev.: 1.9.0 */
> @@ -1346,6 +1354,7 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>  	.host_post_init = qcom_pcie_host_post_init_2_7_0,
>  	.deinit = qcom_pcie_deinit_2_7_0,
>  	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>  	.config_sid = qcom_pcie_config_sid_1_9_0,
>  };
>  
> @@ -1395,9 +1404,81 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>  	.no_l0s = true,
>  };
>  
> +static int qcom_pcie_turnoff_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +	u32 ret_l23, val, ret;
> +
> +	if (!dw_pcie_link_up(pcie->pci)) {
> +		if (pcie->cfg->ops->ltssm_disable)
> +			pcie->cfg->ops->ltssm_disable(pcie);
> +	} else {
> +		writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pcie->elbi + ELBI_SYS_CTRL);
> +
> +		ret_l23 = readl_poll_timeout(pcie->parf + PARF_PM_STTS, val,
> +					     val & PM_ENTER_L23, 10000, 100000);
> +		if (ret_l23) {
> +			dev_err(pci->dev, "Failed to enter L2/L3\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		qcom_pcie_host_deinit(&pcie->pci->pp);
> +
> +		ret = icc_disable(pcie->icc_mem);
> +		if (ret)
> +			dev_err(pci->dev, "Failed to disable PCIe-MEM interconnect path: %d\n",
> +				ret);
> +
> +		pcie->pci_pwrctl_turned_off = true;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_pcie_turnon_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (pcie->pci_pwrctl_turned_off) {
> +		qcom_pcie_host_init(&pcie->pci->pp);
> +
> +		dw_pcie_setup_rc(&pcie->pci->pp);
> +	}
> +
> +	if (pcie->cfg->ops->ltssm_enable)
> +		pcie->cfg->ops->ltssm_enable(pcie);
> +
> +	/* Ignore the retval, the devices may come up later. */
> +	dw_pcie_wait_for_link(pcie->pci);
> +
> +	qcom_pcie_icc_update(pcie);
> +
> +	pcie->pci_pwrctl_turned_off = false;
> +
> +	return 0;
> +}
> +
> +static int qcom_pcie_start_link(struct dw_pcie *pci)
> +{
> +	return qcom_pcie_turnon_link(pci);

If you inline qcom_pcie_turnon_link() here, you don't need to have two
different words for "start"/"turnon".

> +}
> +
> +static void qcom_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (!dw_pcie_link_up(pcie->pci))  {

Unless I'm reading it wrong, qcom_pcie_turnoff_link() has exactly the
same prologue, so you should be able to just inline
qcom_pcie_turnoff_link() in this function and avoid the "stop"/"turnoff"
naming.

> +		if (pcie->cfg->ops->ltssm_disable)
> +			pcie->cfg->ops->ltssm_disable(pcie);
> +	} else {
> +		qcom_pcie_turnoff_link(pci);
> +	}
> +}
> +
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.link_up = qcom_pcie_link_up,
>  	.start_link = qcom_pcie_start_link,
> +	.stop_link = qcom_pcie_stop_link,
>  };
>  
>  static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
> @@ -1604,6 +1685,8 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
>  	int ret;
>  

This deserves a comment.

> +	if (pcie->pci_pwrctl_turned_off)
> +		return 0;
>  	/*
>  	 * Set minimum bandwidth required to keep data path functional during
>  	 * suspend.
> @@ -1642,6 +1725,9 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
>  	int ret;
>  

Ditto.

> +	if (pcie->pci_pwrctl_turned_off)
> +		return 0;
> +

Regards,
Bjorn

>  	if (pcie->suspended) {
>  		ret = qcom_pcie_host_init(&pcie->pci->pp);
>  		if (ret)
> 
> -- 
> 2.42.0
>
Bjorn Helgaas July 1, 2024, 8:14 p.m. UTC | #2
On Wed, Jun 26, 2024 at 06:07:54PM +0530, Krishna chaitanya chundru wrote:
> In the stop_link() if the PCIe link is not up, disable LTSSM enable
> bit to stop link training otherwise keep the link in D3cold.

s/disable LTSSM enable bit/clear LTSSM enable bit/ ?

D3cold is a device state that could apply to a component on the other
end of the link but doesn't apply directly to the link itself, AFAIK.
I assume this would be L2 or L3 for the link.

I think this would be easier to understand if you describe it as "if
the link is up, do something; otherwise disable LTSSM" (similar
comment in the code below).

It would be helpful to explain *why* you want to do this.  So far this
basically transcribes the C code but doesn't explain the benefit.

> And in the start_link() the enable LTSSM bit if the resources are
> turned on other wise do the all the initialization and then start
> the link.
> 
> Introduce ltssm_disable function op to stop the link training.
> 
> Use a flag 'pci_pwrctl_turned_off" to indicate the resources are
> turned off by the pci pwrctl framework.

Match quote style (' vs ").

> If the link is stopped using the stop_link() then just return with
> doing anything in suspend and resume.

Add blank line before signed-off-by.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 108 +++++++++++++++++++++++++++++----
>  1 file changed, 97 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 14772edcf0d3..1ab3ffdb3914 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -37,6 +37,7 @@
>  /* PARF registers */
>  #define PARF_SYS_CTRL				0x00
>  #define PARF_PM_CTRL				0x20
> +#define PARF_PM_STTS				0x24
>  #define PARF_PCS_DEEMPH				0x34
>  #define PARF_PCS_SWING				0x38
>  #define PARF_PHY_CTRL				0x40
> @@ -83,6 +84,9 @@
>  /* PARF_PM_CTRL register fields */
>  #define REQ_NOT_ENTR_L1				BIT(5)
>  
> +/* PARF_PM_STTS register fields */
> +#define PM_ENTER_L23				BIT(5)
> +
>  /* PARF_PCS_DEEMPH register fields */
>  #define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		FIELD_PREP(GENMASK(21, 16), x)
>  #define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	FIELD_PREP(GENMASK(13, 8), x)
> @@ -126,6 +130,7 @@
>  
>  /* ELBI_SYS_CTRL register fields */
>  #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
> +#define ELBI_SYS_CTRL_PME_TURNOFF_MSG		BIT(4)
>  
>  /* AXI_MSTR_RESP_COMP_CTRL0 register fields */
>  #define CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K	0x4
> @@ -228,6 +233,7 @@ struct qcom_pcie_ops {
>  	void (*host_post_init)(struct qcom_pcie *pcie);
>  	void (*deinit)(struct qcom_pcie *pcie);
>  	void (*ltssm_enable)(struct qcom_pcie *pcie);
> +	void (*ltssm_disable)(struct qcom_pcie *pcie);
>  	int (*config_sid)(struct qcom_pcie *pcie);
>  };
>  
> @@ -248,10 +254,13 @@ struct qcom_pcie {
>  	const struct qcom_pcie_cfg *cfg;
>  	struct dentry *debugfs;
>  	bool suspended;
> +	bool pci_pwrctl_turned_off;
>  };
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>  
> +static void qcom_pcie_icc_update(struct qcom_pcie *pcie);
> +
>  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  {
>  	gpiod_set_value_cansleep(pcie->reset, 1);
> @@ -266,17 +275,6 @@ static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
>  	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
>  }
>  
> -static int qcom_pcie_start_link(struct dw_pcie *pci)
> -{
> -	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> -
> -	/* Enable Link Training state machine */
> -	if (pcie->cfg->ops->ltssm_enable)
> -		pcie->cfg->ops->ltssm_enable(pcie);
> -
> -	return 0;
> -}
> -
>  static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
>  {
>  	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -556,6 +554,15 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
>  	return 0;
>  }
>  
> +static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
> +{
> +	u32 val;
> +
> +	val = readl(pcie->parf + PARF_LTSSM);
> +	val &= ~LTSSM_EN;
> +	writel(val, pcie->parf + PARF_LTSSM);
> +}
> +
>  static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
>  {
>  	u32 val;
> @@ -1336,6 +1343,7 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
>  	.post_init = qcom_pcie_post_init_2_7_0,
>  	.deinit = qcom_pcie_deinit_2_7_0,
>  	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>  };
>  
>  /* Qcom IP rev.: 1.9.0 */
> @@ -1346,6 +1354,7 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>  	.host_post_init = qcom_pcie_host_post_init_2_7_0,
>  	.deinit = qcom_pcie_deinit_2_7_0,
>  	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>  	.config_sid = qcom_pcie_config_sid_1_9_0,
>  };
>  
> @@ -1395,9 +1404,81 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>  	.no_l0s = true,
>  };
>  
> +static int qcom_pcie_turnoff_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +	u32 ret_l23, val, ret;
> +
> +	if (!dw_pcie_link_up(pcie->pci)) {
> +		if (pcie->cfg->ops->ltssm_disable)
> +			pcie->cfg->ops->ltssm_disable(pcie);
> +	} else {
> +		writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pcie->elbi + ELBI_SYS_CTRL);
> +
> +		ret_l23 = readl_poll_timeout(pcie->parf + PARF_PM_STTS, val,
> +					     val & PM_ENTER_L23, 10000, 100000);
> +		if (ret_l23) {
> +			dev_err(pci->dev, "Failed to enter L2/L3\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		qcom_pcie_host_deinit(&pcie->pci->pp);
> +
> +		ret = icc_disable(pcie->icc_mem);
> +		if (ret)
> +			dev_err(pci->dev, "Failed to disable PCIe-MEM interconnect path: %d\n",
> +				ret);
> +
> +		pcie->pci_pwrctl_turned_off = true;
> +	}

Can you invert the condition?  It's always a pain to read a negated
condition:
  
  if (dw_pcie_link_up(pcie->pci)) {
    ...
  } else {
    if (pcie->cfg->ops->ltssm_disable)
      pcie->cfg->ops->ltssm_disable(pcie);
  }

Is there any race here between checking for link up and performing the
action?  Does anything bad happen if the link goes down after you do
the check but before you do the deinit?

> +	return 0;
> +}
> +
> +static int qcom_pcie_turnon_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (pcie->pci_pwrctl_turned_off) {
> +		qcom_pcie_host_init(&pcie->pci->pp);
> +
> +		dw_pcie_setup_rc(&pcie->pci->pp);
> +	}
> +
> +	if (pcie->cfg->ops->ltssm_enable)
> +		pcie->cfg->ops->ltssm_enable(pcie);
> +
> +	/* Ignore the retval, the devices may come up later. */
> +	dw_pcie_wait_for_link(pcie->pci);
> +
> +	qcom_pcie_icc_update(pcie);
> +
> +	pcie->pci_pwrctl_turned_off = false;
> +
> +	return 0;
> +}
> +
> +static int qcom_pcie_start_link(struct dw_pcie *pci)
> +{
> +	return qcom_pcie_turnon_link(pci);
> +}
> +
> +static void qcom_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (!dw_pcie_link_up(pcie->pci))  {
> +		if (pcie->cfg->ops->ltssm_disable)
> +			pcie->cfg->ops->ltssm_disable(pcie);
> +	} else {
> +		qcom_pcie_turnoff_link(pci);
> +	}

I think this would be easier to read as:

  if (dw_pcie_link_up(pcie->pci)) {
    qcom_pcie_turnoff_link(pci);
  } else {
    if (pcie->cfg->ops->ltssm_disable)
      pcie->cfg->ops->ltssm_disable(pcie);
  }

> +}
> +
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.link_up = qcom_pcie_link_up,
>  	.start_link = qcom_pcie_start_link,
> +	.stop_link = qcom_pcie_stop_link,
>  };
>  
>  static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
> @@ -1604,6 +1685,8 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
>  	int ret;
>  
> +	if (pcie->pci_pwrctl_turned_off)
> +		return 0;
>  	/*
>  	 * Set minimum bandwidth required to keep data path functional during
>  	 * suspend.
> @@ -1642,6 +1725,9 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
>  	int ret;
>  
> +	if (pcie->pci_pwrctl_turned_off)
> +		return 0;
> +
>  	if (pcie->suspended) {
>  		ret = qcom_pcie_host_init(&pcie->pci->pp);
>  		if (ret)
> 
> -- 
> 2.42.0
>
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 14772edcf0d3..1ab3ffdb3914 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -37,6 +37,7 @@ 
 /* PARF registers */
 #define PARF_SYS_CTRL				0x00
 #define PARF_PM_CTRL				0x20
+#define PARF_PM_STTS				0x24
 #define PARF_PCS_DEEMPH				0x34
 #define PARF_PCS_SWING				0x38
 #define PARF_PHY_CTRL				0x40
@@ -83,6 +84,9 @@ 
 /* PARF_PM_CTRL register fields */
 #define REQ_NOT_ENTR_L1				BIT(5)
 
+/* PARF_PM_STTS register fields */
+#define PM_ENTER_L23				BIT(5)
+
 /* PARF_PCS_DEEMPH register fields */
 #define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		FIELD_PREP(GENMASK(21, 16), x)
 #define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	FIELD_PREP(GENMASK(13, 8), x)
@@ -126,6 +130,7 @@ 
 
 /* ELBI_SYS_CTRL register fields */
 #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
+#define ELBI_SYS_CTRL_PME_TURNOFF_MSG		BIT(4)
 
 /* AXI_MSTR_RESP_COMP_CTRL0 register fields */
 #define CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K	0x4
@@ -228,6 +233,7 @@  struct qcom_pcie_ops {
 	void (*host_post_init)(struct qcom_pcie *pcie);
 	void (*deinit)(struct qcom_pcie *pcie);
 	void (*ltssm_enable)(struct qcom_pcie *pcie);
+	void (*ltssm_disable)(struct qcom_pcie *pcie);
 	int (*config_sid)(struct qcom_pcie *pcie);
 };
 
@@ -248,10 +254,13 @@  struct qcom_pcie {
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
 	bool suspended;
+	bool pci_pwrctl_turned_off;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
+static void qcom_pcie_icc_update(struct qcom_pcie *pcie);
+
 static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
 {
 	gpiod_set_value_cansleep(pcie->reset, 1);
@@ -266,17 +275,6 @@  static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
 	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
 }
 
-static int qcom_pcie_start_link(struct dw_pcie *pci)
-{
-	struct qcom_pcie *pcie = to_qcom_pcie(pci);
-
-	/* Enable Link Training state machine */
-	if (pcie->cfg->ops->ltssm_enable)
-		pcie->cfg->ops->ltssm_enable(pcie);
-
-	return 0;
-}
-
 static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
 {
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
@@ -556,6 +554,15 @@  static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
+{
+	u32 val;
+
+	val = readl(pcie->parf + PARF_LTSSM);
+	val &= ~LTSSM_EN;
+	writel(val, pcie->parf + PARF_LTSSM);
+}
+
 static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -1336,6 +1343,7 @@  static const struct qcom_pcie_ops ops_2_7_0 = {
 	.post_init = qcom_pcie_post_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
 };
 
 /* Qcom IP rev.: 1.9.0 */
@@ -1346,6 +1354,7 @@  static const struct qcom_pcie_ops ops_1_9_0 = {
 	.host_post_init = qcom_pcie_host_post_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
 	.config_sid = qcom_pcie_config_sid_1_9_0,
 };
 
@@ -1395,9 +1404,81 @@  static const struct qcom_pcie_cfg cfg_sc8280xp = {
 	.no_l0s = true,
 };
 
+static int qcom_pcie_turnoff_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	u32 ret_l23, val, ret;
+
+	if (!dw_pcie_link_up(pcie->pci)) {
+		if (pcie->cfg->ops->ltssm_disable)
+			pcie->cfg->ops->ltssm_disable(pcie);
+	} else {
+		writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pcie->elbi + ELBI_SYS_CTRL);
+
+		ret_l23 = readl_poll_timeout(pcie->parf + PARF_PM_STTS, val,
+					     val & PM_ENTER_L23, 10000, 100000);
+		if (ret_l23) {
+			dev_err(pci->dev, "Failed to enter L2/L3\n");
+			return -ETIMEDOUT;
+		}
+
+		qcom_pcie_host_deinit(&pcie->pci->pp);
+
+		ret = icc_disable(pcie->icc_mem);
+		if (ret)
+			dev_err(pci->dev, "Failed to disable PCIe-MEM interconnect path: %d\n",
+				ret);
+
+		pcie->pci_pwrctl_turned_off = true;
+	}
+
+	return 0;
+}
+
+static int qcom_pcie_turnon_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	if (pcie->pci_pwrctl_turned_off) {
+		qcom_pcie_host_init(&pcie->pci->pp);
+
+		dw_pcie_setup_rc(&pcie->pci->pp);
+	}
+
+	if (pcie->cfg->ops->ltssm_enable)
+		pcie->cfg->ops->ltssm_enable(pcie);
+
+	/* Ignore the retval, the devices may come up later. */
+	dw_pcie_wait_for_link(pcie->pci);
+
+	qcom_pcie_icc_update(pcie);
+
+	pcie->pci_pwrctl_turned_off = false;
+
+	return 0;
+}
+
+static int qcom_pcie_start_link(struct dw_pcie *pci)
+{
+	return qcom_pcie_turnon_link(pci);
+}
+
+static void qcom_pcie_stop_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	if (!dw_pcie_link_up(pcie->pci))  {
+		if (pcie->cfg->ops->ltssm_disable)
+			pcie->cfg->ops->ltssm_disable(pcie);
+	} else {
+		qcom_pcie_turnoff_link(pci);
+	}
+}
+
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
+	.stop_link = qcom_pcie_stop_link,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
@@ -1604,6 +1685,8 @@  static int qcom_pcie_suspend_noirq(struct device *dev)
 	struct qcom_pcie *pcie = dev_get_drvdata(dev);
 	int ret;
 
+	if (pcie->pci_pwrctl_turned_off)
+		return 0;
 	/*
 	 * Set minimum bandwidth required to keep data path functional during
 	 * suspend.
@@ -1642,6 +1725,9 @@  static int qcom_pcie_resume_noirq(struct device *dev)
 	struct qcom_pcie *pcie = dev_get_drvdata(dev);
 	int ret;
 
+	if (pcie->pci_pwrctl_turned_off)
+		return 0;
+
 	if (pcie->suspended) {
 		ret = qcom_pcie_host_init(&pcie->pci->pp);
 		if (ret)