Date: Mon, 23 Sep 2013 12:23:54 -0700 From: Justin Hibbits <jhibbits@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r250956 - projects/pmac_pmu/sys/kern Message-ID: <CAHSQbTDdMjXT7ABoffFkbW=HEB_7B9S6tS3FYyPkynqyntJTXg@mail.gmail.com> In-Reply-To: <201305240849.27877.jhb@freebsd.org> References: <201305240358.r4O3wVw5026183@svn.freebsd.org> <201305240849.27877.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi John, I know it's an ancient thread I'm replying to, but it's the same thing. On Fri, May 24, 2013 at 5:49 AM, John Baldwin <jhb@freebsd.org> wrote: > 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 > Have you had any time to put more thought into imrpoving this? I've fixed I think the last few bugs with the PowerPC-side of things, and want to push this into HEAD after the thaw. Warner had mentioned in a peer thread that a second interface, to pass the bus pass along, with a default of calling the existing bus_resume(). Thoughts? - Justin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHSQbTDdMjXT7ABoffFkbW=HEB_7B9S6tS3FYyPkynqyntJTXg>