On Fri, Mar 23, 2018 at 05:01:27PM -0500, Bjorn Helgaas wrote: > On Fri, Mar 23, 2018 at 10:11:53PM +0100, Rafael J. Wysocki wrote: > > On Fri, Mar 23, 2018 at 10:08 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > On Fri, Mar 23, 2018 at 01:18:56PM +0200, Mika Westerberg wrote: > > >> On Thu, Mar 22, 2018 at 02:36:30PM -0500, Bjorn Helgaas wrote: > > >> > I hope we can avoid adding suspend_late/resume_early callbacks in > > >> > struct pcie_port_service_driver, and I also hope we can avoid adding > > >> > device links. Those both sound pretty complicated. > > >> > > > >> > Can you do something like the patch below, which does something > > >> > similar for PME? > > >> > > >> AFAICT the core PCI PM code follows the same ordering than what PM core > > >> does so it may be possible that not all service drivers get > > >> resumed/suspended before other children (PCI buses). Basically this > > >> would be the same than just using core PM ops in DPC driver (in which > > >> case DPC specific things are still kept in DPC driver not in PCI core). > > > > > > I'm not sure I follow this. I assume the core PCI PM code guarantees > > > that a bridge is suspended after its children and resumed before them. > > > Are you saying that's not the case? > > > > if this is a PCIe port, then there are two categories of childres: > > port services and the PCI devices below it. > > > > There are no ordering constraints between the former and the latter, > > which appears to be a problem here. > > It seems like a pretty fundamental problem if port services can be > suspended before PCI devices below the port. I assume that would have > to be fixed somehow in the PCI core and the port driver, but this > patch only touches the DPC service driver. > > I'd really like to get rid of the port services driver altogether, and > this ordering issue seems like a good reason to do that. As it is, there is nothing to fix. The port services driver enforces that the services are resumed before PCI child devices (and after them on suspend) by iterating over the services' ->resume / ->suspend callbacks, see pcie_port_device_resume() and pcie_port_device_suspend(), which in turn are the port's ->resume / ->suspend callbacks. There is (or was, before your reorganization this cycle) an inconsistency in the handling of ->resume / ->suspend stages vis-à-vis the ->resume_noirq / ->runtime_resume / ->runtime_>suspend / ->runtime_idle stages. The latter are handled directly by portdrv callbacks whereas the former are handled by per-service callbacks which we iterate over. I cooked up these commits two years ago to make the handling more consistent but ended up not needing them: https://github.com/l1k/linux/commit/4f59372f830899e89fea9ba7c090680900d5998e https://github.com/l1k/linux/commit/ce63b6ed01602e77c56043ee6dfa52ca9d257f88 Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
