From owner-svn-src-projects@FreeBSD.ORG Fri May 24 12:49:43 2013 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 1957B873; Fri, 24 May 2013 12:49:43 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) by mx1.freebsd.org (Postfix) with ESMTP id EA87CB67; Fri, 24 May 2013 12:49:42 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 4FE85B949; Fri, 24 May 2013 08:49:41 -0400 (EDT) From: John Baldwin To: Justin Hibbits Subject: Re: svn commit: r250956 - projects/pmac_pmu/sys/kern Date: Fri, 24 May 2013 08:49:27 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <201305240358.r4O3wVw5026183@svn.freebsd.org> In-Reply-To: <201305240358.r4O3wVw5026183@svn.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201305240849.27877.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 24 May 2013 08:49:41 -0400 (EDT) Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 May 2013 12:49:43 -0000 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