[v7,0/5] PCI: qcom: Add system suspend & resume support

Few PCIe endpoints like NVMe and WLANs are always expecting the device
to be in D0 state and the link to be active (or in l1ss) all the
time (including in S3 state).

This patch series adds this support for allowing the system to enter
S3 state (and further low power states) with PCIe Device being in D0
state and with link being in l1ss on qcom platforms.

And to get to the lowest power state on Qcom SoC, all the clocks need
to be voted off. Since PCIe clocks are managed by qcom platform driver,
this logic was added to the qcom platform driver.

And when we turn off PCIe PHY-specific clocks, PHY may go off and along
with it the link also will go down. To retain, the link state in l1ss with
PHY clocks turned off, we need park PCIe PHY in the power-down state so
that it can maintain the link state in l1ss with the help of the always-on
power domain (aka MX).
To support this PHY Power-down state PHY driver has been updated with new
interface APIs.

Its observed that access to Ep PCIe space to mask MSI/MSIX is happening at
the very late stage of suspend path (access by affinity changes while
making CPUs offline during suspend, this will happen after devices are
suspended (after all phases of suspend ops)). If we turn off clocks in any
PM callback, afterwards running into crashes due to un-clocked access due
to above mentioned MSI/MSIx access. So, we are making use of syscore
framework to turn off the PCIe clocks which will be called after making
CPUs offline.

During this process, The controller should remain powered on. For this made
changes to GDSC. 

Few endpoints are taking time more time to settle the link in L1ss.
So Waiting for max time of 200ms for the link to be stable in L1ss.

Krishna chaitanya chundru (5):
  PCI: qcom: Add system suspend and resume support
  PCI: qcom: Add retry logic for link to be stable in L1ss
  phy: core: Add support for phy suspend & resume
  phy: qcom: Add power suspend & resume callbacks to pcie phy
  clk: qcom: Enabling PCIe GDSC retention

 drivers/clk/qcom/gcc-sc7280.c            |   2 +-
 drivers/pci/controller/dwc/pcie-qcom.c   | 163 ++++++++++++++++++++++++++++++-
 drivers/phy/phy-core.c                   |  30 ++++++
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c |  50 ++++++++++
 include/linux/phy/phy.h                  |  20 ++++
 5 files changed, 263 insertions(+), 2 deletions(-)

On 20-09-22, 15:52, Krishna chaitanya chundru wrote:
> When link is in L1ss(L1.1 or L1.2), all the clocks will gate off and there
> will be no activity on the link. At that point clocks and phy
> can be turned off. If clocks got disabled before link enters
> L1ss the PCIe link goes down.
> 
> Few endpoints are taking time more time to settle the link in L1 substates.
> When we check the traffic in protocol analyzer, we see some DLLP packets
> going on still. So Wait for max time of 200ms for the link to be stable in
> L1 substates.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> changes since v6:
> 	- updated comments.
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 46 ++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 3f5424a..7a6f69e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1809,23 +1809,47 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  static int __maybe_unused qcom_pcie_pm_suspend(struct qcom_pcie *pcie)
>  {
>  	u32 val;
> +	ktime_t timeout, start;
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
>  
> -	/* if the link is not active turn off clocks */
> -	if (!dw_pcie_link_up(pci)) {
> -		dev_dbg(dev, "Link is not active\n");
> -		goto suspend;
> -	}
> +	/*
> +	 * When link is in L1ss, all the clocks will gate off and
> +	 * there will be no activity on the link. At that point clocks
> +	 * and phy can be turned off. If clocks got disabled before
> +	 * link enters L1ss the PCIe link goes down.
> +	 *
> +	 * Few endpoints are taking time more time to settle the link
> +	 * in L1ss. Wait for max of 200ms for the link to be stable in
> +	 * L1ss.
> +	 */
> +	start = ktime_get();
> +	/* Wait max 200 ms */
> +	timeout = ktime_add_ms(start, 200);
> +
> +	while (1) {
> +		/* if the liink is not active turn off clocks */
> +		if (!dw_pcie_link_up(pci)) {
> +			dev_dbg(dev, "Link is not active\n");
> +			break;
> +		}
>  
> -	/* if the link is not in l1ss don't turn off clocks */
> -	val = readl(pcie->parf + PCIE20_PARF_PM_STTS);
> -	if (!(val & PCIE20_PARF_PM_STTS_LINKST_IN_L1SUB)) {
> -		dev_warn(dev, "Link is not in L1ss\n");
> -		return 0;
> +		/* if the link is not in l1ss don't turn off clocks */
> +		val = readl(pcie->parf + PCIE20_PARF_PM_STTS);
> +		if ((val & PCIE20_PARF_PM_STTS_LINKST_IN_L1SUB)) {
> +			dev_dbg(dev, "Link enters L1ss after %lld  ms\n",
> +					ktime_to_ms(ktime_get() - start));
> +			break;
> +		}
> +
> +		if (ktime_after(ktime_get(), timeout)) {
> +			dev_warn(dev, "Link is not in L1ss\n");
> +			return 0;
> +		}
> +

ugh, why not use readl_poll_timeout()?

> +		udelay(1000);
>  	}
>  
> -suspend:
>  	if (pcie->cfg->ops->suspend)
>  		pcie->cfg->ops->suspend(pcie);
>  
> -- 
> 2.7.4

On 9/24/2022 11:35 AM, Vinod Koul wrote:
> On 20-09-22, 15:52, Krishna chaitanya chundru wrote:
>> When link is in L1ss(L1.1 or L1.2), all the clocks will gate off and there
>> will be no activity on the link. At that point clocks and phy
>> can be turned off. If clocks got disabled before link enters
>> L1ss the PCIe link goes down.
>>
>> Few endpoints are taking time more time to settle the link in L1 substates.
>> When we check the traffic in protocol analyzer, we see some DLLP packets
>> going on still. So Wait for max time of 200ms for the link to be stable in
>> L1 substates.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> changes since v6:
>> 	- updated comments.
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 46 ++++++++++++++++++++++++++--------
>>   1 file changed, 35 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 3f5424a..7a6f69e 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -1809,23 +1809,47 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>   static int __maybe_unused qcom_pcie_pm_suspend(struct qcom_pcie *pcie)
>>   {
>>   	u32 val;
>> +	ktime_t timeout, start;
>>   	struct dw_pcie *pci = pcie->pci;
>>   	struct device *dev = pci->dev;
>>   
>> -	/* if the link is not active turn off clocks */
>> -	if (!dw_pcie_link_up(pci)) {
>> -		dev_dbg(dev, "Link is not active\n");
>> -		goto suspend;
>> -	}
>> +	/*
>> +	 * When link is in L1ss, all the clocks will gate off and
>> +	 * there will be no activity on the link. At that point clocks
>> +	 * and phy can be turned off. If clocks got disabled before
>> +	 * link enters L1ss the PCIe link goes down.
>> +	 *
>> +	 * Few endpoints are taking time more time to settle the link
>> +	 * in L1ss. Wait for max of 200ms for the link to be stable in
>> +	 * L1ss.
>> +	 */
>> +	start = ktime_get();
>> +	/* Wait max 200 ms */
>> +	timeout = ktime_add_ms(start, 200);
>> +
>> +	while (1) {
>> +		/* if the liink is not active turn off clocks */
>> +		if (!dw_pcie_link_up(pci)) {
>> +			dev_dbg(dev, "Link is not active\n");
>> +			break;
>> +		}
>>   
>> -	/* if the link is not in l1ss don't turn off clocks */
>> -	val = readl(pcie->parf + PCIE20_PARF_PM_STTS);
>> -	if (!(val & PCIE20_PARF_PM_STTS_LINKST_IN_L1SUB)) {
>> -		dev_warn(dev, "Link is not in L1ss\n");
>> -		return 0;
>> +		/* if the link is not in l1ss don't turn off clocks */
>> +		val = readl(pcie->parf + PCIE20_PARF_PM_STTS);
>> +		if ((val & PCIE20_PARF_PM_STTS_LINKST_IN_L1SUB)) {
>> +			dev_dbg(dev, "Link enters L1ss after %lld  ms\n",
>> +					ktime_to_ms(ktime_get() - start));
>> +			break;
>> +		}
>> +
>> +		if (ktime_after(ktime_get(), timeout)) {
>> +			dev_warn(dev, "Link is not in L1ss\n");
>> +			return 0;
>> +		}
>> +
> ugh, why not use readl_poll_timeout()?
As this is called from the syscore ops, all the interrupts will be 
disabled by the time the execution reaches here.
readl_poll_timeout uses interrupt internally and cause some issues.

So we are using this method instead of readl_poll_timeout.
>
>> +		udelay(1000);
>>   	}
>>   
>> -suspend:
>>   	if (pcie->cfg->ops->suspend)
>>   		pcie->cfg->ops->suspend(pcie);
>>   
>> -- 
>> 2.7.4

On Tue, 20 Sep 2022 15:52:22 +0530, Krishna chaitanya chundru wrote:
> Few PCIe endpoints like NVMe and WLANs are always expecting the device
> to be in D0 state and the link to be active (or in l1ss) all the
> time (including in S3 state).
> 
> This patch series adds this support for allowing the system to enter
> S3 state (and further low power states) with PCIe Device being in D0
> state and with link being in l1ss on qcom platforms.
> 
> [...]

Applied, thanks!

[5/5] clk: qcom: gcc-sc7280: Update the .pwrsts for PCIe GDSC
      commit: 1a58ee1330b2cb6d71feb0aaf827cc10030f78b4

Best regards,