Date: Fri, 24 May 2013 08:49:27 -0400 From: John Baldwin <jhb@freebsd.org> To: Justin Hibbits <jhibbits@freebsd.org> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r250956 - projects/pmac_pmu/sys/kern Message-ID: <201305240849.27877.jhb@freebsd.org> In-Reply-To: <201305240358.r4O3wVw5026183@svn.freebsd.org> References: <201305240358.r4O3wVw5026183@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, May 23, 2013 11:58:31 pm Justin Hibbits wrote: > Author: jhibbits > Date: Fri May 24 03:58:31 2013 > New Revision: 250956 > URL: http://svnweb.freebsd.org/changeset/base/250956 > > Log: > Add multipass for suspend/resume. This allows some devices to be resumed before > others, even their peers, as required by PowerPC Mac hardware. I think this is a good start. One question I have is why not reuse the pass number from the driver instead of stuffing it in the devclass? (It is conceivable that different drivers with different passes might share a devclass even, though unlikely). That is, can't you use child->driver->dl_pass directly? (If a child doesn't have a driver it can't suspend or resume anyway). Also, can you explain the EAGAIN logic? It's not obvious. Is this how you are enforcing that already resumed drivers keep traversing their tree in subsequent passes (if not you need to deal with that in some fashion). I think we might want to be more explicit about forcing suspend and resume to traverse the tree for each pass. The boot-time probe has a BUS_NEW_PASS callback it uses for this, and bus_generic_new_pass() is what it uses for that. I think you might want to create BUS_SUSPEND_PASS and BUS_RESUME_PASS with generic versions similar to bus_generic_new_pass. You would check the DF_SUSPENDED flag there to decide whether or not you wanted to call BUS_SUSPEND/RESUME_PASS or DEVICE_SUSPEND(). The other thing that makes this more complicated is that unlike probe/attach, we might need to actually ask the bus to suspend and resume the device so that the bus can do power management. Right now this works because the bus devices suspend everything in one pass so they can order things correctly (e.g. pci_suspend()). If we want to shut down some devices early we might consider having a new bus_if method which takes a child and handles suspending that specific child (it would call DEVICE_SUSPEND). For PCI you might get something like this: int pci_suspend_child(device_t bus, device_t child) { struct pci_devinfo *dinfo; int error; dinfo = device_get_ivars(child); pci_cfg_save(child, dinfo, 0); error = DEVICE_SUSPEND(child); if (error) return (error); if (pci_do_power_suspend) /* set power state for just this child to D3 */ return (error); } I need to think a bit more, but I think this is more of a correct model to handle passes, and will also be cleaner for suspend/resume in general. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201305240849.27877.jhb>