[PATCHv2 6/6] nvme-pci: Use host managed power state for suspend

[kai.heng.feng@canonical.com (Kai-Heng Feng)]

at 16:39, Rafael J. Wysocki <rafael@kernel.org> wrote:

> On Thu, May 16, 2019 at 10:56 PM Rafael J. Wysocki <rafael at kernel.org>  
> wrote:
>> On Thu, May 16, 2019@10:45 PM Keith Busch <kbusch@kernel.org> wrote:
>>> On Thu, May 16, 2019@10:25:47PM +0200, Rafael J. Wysocki wrote:
>>>> On Thu, May 16, 2019@9:43 PM Keith Busch <kbusch@kernel.org> wrote:
>>>>> If we're going to replace our SIMPLE_DEV_PM_OPS as Rafael suggests,
>>>>> might as well add a .suspend_noirq callback. We can just save the
>>>>> state again to work around this too.
>>>>
>>>> Relax, pretty please.  Saving the state again shouldn't be necessary.
>>>>
>>>> Let's first understand what's going on.
>>>
>>> Hah, okay fair enough.
>>>
>>> FWIW, I've tried current mainline on two different platforms and I see
>>> only the expected number of calls to pci_pm_suspend_noirq, so everything
>>> works for me.
>>
>> And I don't see why it might not work anywhere else other than a
>> kernel source hacked into pieces or a platform with fundamental
>> problems. :-)
>
> I forgot about one thing which is relevant here, sorry about that.
>
> If there is a spurious EC wakeup during s2idle, pci_pm_suspend_noirq()
> *will* be run again after pci_pm_resume_noirq() without going through
> the entire system resume and next suspend.  In that case the second
> iteration of pci_pm_suspend_noirq() will put the device into D3, if
> that's the target power state of the device as determined by
> pci_prepare_to_sleep(), because pci_pm_resume_noirq() calls
> pci_restore_state() which clears state_saved.
>
> That's not what appears to happen on the test platform as per the
> posted log, but nevertheless it can happen.

The patch Mario pointed solves the issue for the test platform I used.

I?ve tested a dozen NVMes, all of them work properly with Keith?s patch  
series, once the LPIT fix is applied.

>
> Arguably, the driver should not need to worry about that, so that
> needs to be addressed in the PCI core IMO.
>
> The attached patch should help (not inline to avoid gmail-induced
> white space damage).
>
> I will post it properly with a changelog shortly.
> <pci-pm-skip-noirq.patch>

Since the patch Mario suggested solves the issue, do you still want me to  
test this patch?

Kai-Heng