Re: [PATCH v7 1/5] PCI: qcom: Add system suspend and resume support

[Bjorn Helgaas]

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.

> 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.

> 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.

> 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