On Thu, May 9, 2019 at 11:21 PM Keith Busch <kbu...@kernel.org> wrote: > > On Thu, May 09, 2019 at 10:54:04PM +0200, Rafael J. Wysocki wrote: > > On Thu, May 9, 2019 at 9:33 PM Keith Busch <kbu...@kernel.org> wrote: > > > #include <linux/io-64-nonatomic-lo-hi.h> > > > @@ -2851,6 +2852,8 @@ static int nvme_suspend(struct device *dev) > > > struct pci_dev *pdev = to_pci_dev(dev); > > > struct nvme_dev *ndev = pci_get_drvdata(pdev); > > > > > > + if (!pm_suspend_via_firmware()) > > > + return nvme_set_power(&ndev->ctrl, ndev->ctrl.npss); > > > > You probably want to call pci_save_state(pdev) in the branch above to > > prevent pci_pm_suspend_noirq() from calling pci_prepare_to_sleep() > > going forward, so I would write this routine as > > > > if (pm_suspend_via_firmware()) { > > nvme_dev_disable(ndev, true); > > return 0; > > } > > > > pci_save_state(pdev) > > return nvme_set_power(&ndev->ctrl, ndev->ctrl.npss); > > Ah, good point. I'll make sure that's added and will wait to see hear if > there's any other feedback. > > I am trying to test the paths by faking out PS capabilities, and have > a question on how to force each: > > Running "rtcwake -m freeze ...", that takes the !pm_suspend_via_firmware() > path as I expected. > > But trying to test the original path, I thought using "-m mem" would > have been a suspend via firmware, but that is still returning false. > > Is that expected?
Yes, if s2idle is the default on that platform. You should be able to switch over to S3 by writing "deep" into /sys/power/mem_sleep as long as it is supported on that platform at all. > I've only tried this on one platform so far, so might > just be this particular one is missing a firmware capability. You can check that by looking into /sys/power/mem_sleep (if there is only "[s2idle]" in there, S3 is not supported).