| Message ID | 20240626-qps615-v1-6-2ade7bd91e02@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | PCI: enable Power and configure the QPS615 PCIe switch | expand |
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 >
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 --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)
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(-)