On Thu, Apr 28, 2016 at 5:13 PM, Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote: > On Thu, Apr 28, 2016 at 04:22:16PM +0200, Lukas Wunner wrote: >> Hi Mika, >> >> I've rebased my Thunderbolt runtime pm patches on v4 of your patches >> and everything seems to still work fine. d3cold_allowed also works >> as it should now. > > Cool, thanks for testing. > >> As said I've amended my series to allow runtime pm on hotplug ports >> if they're Thunderbolt ports on a Mac: >> https://github.com/l1k/linux/commit/a6810db929485c7fc8677f265b1c68e31879ce61 > > If we are going to add more conditions, I think it makes sense to > introduce pcie_port_runtime_pm_possible() or similar which includes all > these checks. > >> I've also reviewed the patches one more time and spotted only this >> small nit: >> >> On Mon, Apr 25, 2016 at 12:53:24PM +0300, Mika Westerberg wrote: >> > +static int pcie_port_runtime_resume(struct device *dev) >> > +{ >> > + pm_runtime_mark_last_busy(dev); >> > + return 0; >> > +} >> >> The PM core seems to do this automatically, see rpm_resume(): >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/runtime.c#n749 >> >> So you could just drop the .runtime_resume entry here and it shouldn't >> result in any functional change: >> >> > @@ -101,6 +122,9 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = { >> > .poweroff = pcie_port_device_suspend, >> > .restore = pcie_port_device_resume, >> > .resume_noirq = pcie_port_resume_noirq, >> > + .runtime_suspend = pcie_port_runtime_suspend, >> > + .runtime_resume = pcie_port_runtime_resume, >> > + .runtime_idle = pcie_port_runtime_idle, > > Hmm, PM core calls pci_pm_runtime_resume() for PCI drivers which then > delegates to driver->runtime_resume() if set. However, if it is missing > it just returns -ENOSYS and does not put the device back to D0. > > So if I'm reading this right we actually need to provide > pcie_port_runtime_resume(). You do, but it could just return 0. The suggestion seems to be that pm_runtime_mark_last_busy() is not needed as the core will invoke it anyway. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
