On 9/20/2022 11:46 PM, Bjorn Helgaas wrote:
On Tue, Sep 20, 2022 at 03:52:23PM +0530, Krishna chaitanya chundru wrote:
Add suspend and resume syscore ops.
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).
What does this have to do with the patch? I don't see any NVMe or
WLAN patches here.
Existing NVMe driver expecting NVMe device to be in D0 during S3 also.
If we turn off the link in
suspend, the NVMe resume path is broken as the state machine is getting
reset in the NVMe device.
Due to this, the host driver state machine and the device state machine
are going out of sync, and all NVMe commands
after resumes are getting timed out.
IIRC, Tegra is also facing this issue with NVMe.
This issue has been discussed below threads:
https://lore.kernel.org/all/Yl+6V3pWuyRYuVV8@xxxxxxxxxxxxx/T/
https://lore.kernel.org/linux-nvme/20220201165006.3074615-1-kbusch@xxxxxxxxxx/
In qcom platform PCIe resources( clocks, phy etc..) can released
when the link is in L1ss to reduce the power consumption. So if the link
is in L1ss, release the PCIe resources. And when the system resumes,
enable the PCIe resources if they released in the suspend path.
What's the connection with L1.x? Links enter L1.x based on activity
and timing. That doesn't seem like a reliable indicator to turn PHYs
off and disable clocks.
This is a Qcom PHY-specific feature (retaining the link state in L1.x
with clocks turned off).
It is possible only with the link being in l1.x. PHY can't retain the
link state in L0 with the
clocks turned off and we need to re-train the link if it's in L2 or L3.
So we can support this feature only with L1.x.
That is the reason we are taking l1.x as the trigger to turn off clocks
(in only suspend path).
is_suspended flag indicates if the PCIe resources are released or not
in the suspend path.
Why is "is_suspended" important for the commit log? It looks like
just a standard implementation detail.
Someone in one of the previous patch asked to include this in the commit
text.
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.
Add blank lines between paragraphs. Or rewrap into a single paragraph.
s/Its observed/It's observed/
s/MSIX/MSI-X/ throughout
s/MSIx/MSI-X/ throughout
Bjorn
