Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <af79fc4d-4996-bb2c-7388-2d9afd991e7a@quicinc.com>
Date:   Mon, 12 Sep 2022 21:40:30 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v6 0/5] PCI: qcom: Add system suspend & resume support
Content-Language: en-US
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     <linux-pci@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <mka@chromium.org>,
        <quic_vbadigan@quicinc.com>, <quic_hemantk@quicinc.com>,
        <quic_nitegupt@quicinc.com>, <quic_skananth@quicinc.com>,
        <quic_ramkri@quicinc.com>, <manivannan.sadhasivam@linaro.org>,
        <swboyd@chromium.org>, <dmitry.baryshkov@linaro.org>
References: <20220909195110.GA310839@bhelgaas>
From:   Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
In-Reply-To: <20220909195110.GA310839@bhelgaas>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 9/10/2022 1:21 AM, Bjorn Helgaas wrote:
> On Fri, Sep 09, 2022 at 02:14:39PM +0530, Krishna chaitanya chundru wrote:
>> Add suspend and resume syscore ops.
>>
>> When system suspends, and if the link is in L1ss, disable the clocks
>> and power down the phy so that system enters into low power state by
>> parking link in L1ss to save the maximum power. And when the system
>> resumes, enable the clocks back and power on phy if they are disabled
>> in the suspend path.
>>
>> we are doing this only when link is in l1ss but not in L2/L3 as
>> nowhere we are forcing link to L2/L3 by sending PME turn off.
>>
>> is_suspended flag indicates if the clocks are disabled in the suspend
>> path or not.
>>
>> There is access to Ep PCIe space to mask MSI/MSIX after pm suspend ops
>> (getting hit by affinity changes while making CPUs offline during suspend,
>> this will happen after devices are suspended (all phases of suspend ops)).
>> When registered with pm ops there is a crash due to un-clocked access,
>> as in the pm suspend op clocks are disabled. So, registering with syscore
>> ops which will called after making CPUs offline.
>>
>> Make GDSC always on to ensure controller and its dependent clocks
>> won't go down during system suspend.
>>
>> 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 power down & power up
>>    phy: qcom: Add power down/up callbacks to pcie phy
>>    clk: qcom: Alwaya on pcie gdsc
> This seems fairly ugly because it doesn't fit nicely into the PM
> framework.  Why is this a qcom-specific thing?  What about other
> DWC-based controllers?
We wanted to allow system S3 state by turning off all PCIe clocks but at 
the same time
retaining NVMe device in D0 state and PCIe link in l1ss state.

Here nothing really specific to DWC as PCIe controller remains intact.

And the Qcom PHY allows this scheme  (that is to retain the link state 
in l1ss
even though all pcie clocks are turned off).

Since clocks are completely managed by qcom platform driver, we are 
trying to manage them
during S3/S0 transitions with PM callbacks.
>>   drivers/clk/qcom/gcc-sc7280.c            |   2 +-
>>   drivers/pci/controller/dwc/pcie-qcom.c   | 156 ++++++++++++++++++++++++++++++-
>>   drivers/phy/phy-core.c                   |  30 ++++++
>>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c |  50 ++++++++++
>>   include/linux/phy/phy.h                  |  20 ++++
>>   5 files changed, 256 insertions(+), 2 deletions(-)
>>
>> -- 
>> 2.7.4
>>