From owner-freebsd-current@FreeBSD.ORG Mon Aug 11 05:44:14 2014 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id DC3183E8 for ; Mon, 11 Aug 2014 05:44:14 +0000 (UTC) Received: from mail-yh0-x235.google.com (mail-yh0-x235.google.com [IPv6:2607:f8b0:4002:c01::235]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 99BEA2035 for ; Mon, 11 Aug 2014 05:44:14 +0000 (UTC) Received: by mail-yh0-f53.google.com with SMTP id c41so5897140yho.26 for ; Sun, 10 Aug 2014 22:44:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:message-id:mime-version:content-type:resent-date :resent-from:subject:resent-message-id:resent-to; bh=esPHSUhSo6EGTeDg68HQXUZb/44Fr+zjpWk+G5r9ku0=; b=RACyVyS9vdBG7N/8r6PETYg5h1iuKxIsHuX60cWWNIim/CfOJvZfo4PiWLbEYyBiUB Xv+DfuKhb4kNCMgonCqzlyYsfuUcafEUfAdxzAFUewi4bE7CdT2NBnHxyy2Hy4iDmQiX YF070khAuP4SrSLSx0cFB72reOjMZhs5cieY+YIqZ2gXpDloOH93SG2A5K8L+Jld+D+P R0djd4AZYEZ9ZlqIQWgf3n7od488perg+3uDTV8xDzIjtCRGRZUTCfvqwAEjTFYv8Clu aGzlB7B+HJdrH+qAk1jAT88YvnD2JmipNOMhVbgE9VB+LQ/id7hpS+cuug6QkgSBvzOb thNw== X-Received: by 10.236.86.144 with SMTP id w16mr39345422yhe.5.1407735853128; Sun, 10 Aug 2014 22:44:13 -0700 (PDT) Received: from zhabar.att.net (107-222-186-3.lightspeed.sntcca.sbcglobal.net. [107.222.186.3]) by mx.google.com with ESMTPSA id k106sm21758749yhq.49.2014.08.10.22.44.12 for (version=SSLv3 cipher=RC4-SHA bits=128/128); Sun, 10 Aug 2014 22:44:12 -0700 (PDT) Date: Sun, 10 Aug 2014 22:30:30 -0700 From: Justin Hibbits (by way of Justin Hibbits ) To: Message-ID: <20140810223030.479badbc@zhabar.att.net> X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.22; powerpc64-portbld-freebsd11.0) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/J8yUo/400lG2N5dYyImkHpS" Resent-Date: Sun, 10 Aug 2014 22:44:09 -0700 Resent-From: Justin Hibbits Subject: Child suspend/resume Resent-Message-ID: <20140810224409.4f1b028e@zhabar.att.net> Resent-To: FreeBSD Current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Aug 2014 05:44:14 -0000 --MP_/J8yUo/400lG2N5dYyImkHpS Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline Hi all, The attached patch is completely untested, due to lack of existing suspendable hardware (no x86 machines). It does compile cleanly against head, though. I don't think it should change any behavior, I tried to keep the essence of the code path the same. It was suggested that I break up my multipass suspend/resume code into incremental parts, so this is part one. It adds a BUS_SUSPEND_CHILD/BUS_RESUME_CHILD, as well as helper functions, bus_generic_suspend_child()/bus_generic_resume_child(), and modifies the PCI driver to use this new facility. I'd like some feedback, and testing of this, to make sure I didn't break anything. Thanks, Justin --MP_/J8yUo/400lG2N5dYyImkHpS Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=suspend_child.diff Index: sys/kern/bus_if.m =================================================================== --- sys/kern/bus_if.m (revision 269798) +++ sys/kern/bus_if.m (working copy) @@ -670,3 +670,25 @@ device_t _child; u_int _irq; } DEFAULT null_remap_intr; + +/** + * @brief Suspend a given child + * + * @param _dev the parent device of @p _child + * @param _child the device to suspend + */ +METHOD int suspend_child { + device_t _dev; + device_t _child; +} DEFAULT bus_generic_suspend_child; + +/** + * @brief Resume a given child + * + * @param _dev the parent device of @p _child + * @param _child the device to resume + */ +METHOD int resume_child { + device_t _dev; + device_t _child; +} DEFAULT bus_generic_suspend_child; Index: sys/kern/subr_bus.c =================================================================== --- sys/kern/subr_bus.c (revision 269798) +++ sys/kern/subr_bus.c (working copy) @@ -135,6 +135,7 @@ #define DF_DONENOMATCH 0x20 /* don't execute DEVICE_NOMATCH again */ #define DF_EXTERNALSOFTC 0x40 /* softc not allocated by us */ #define DF_REBID 0x80 /* Can rebid after attach */ +#define DF_SUSPENDED 0x100 /* Device is suspended. */ u_int order; /**< order from device_add_child_ordered() */ void *ivars; /**< instance variables */ void *softc; /**< current driver's variables */ @@ -3631,6 +3632,37 @@ } /** + * @brief Default function for suspending a child device. + * + * This function is to be used by a bus's DEVICE_SUSPEND_CHILD(). + */ +int +bus_generic_suspend_child(device_t dev, device_t child) +{ + int error; + + error = DEVICE_SUSPEND(child); + + if (error == 0) + dev->flags |= DF_SUSPENDED; + return (error); +} + +/** + * @brief Default function for resuming a child device. + * + * This function is to be used by a bus's DEVICE_RESUME_CHILD(). + */ +int +bus_generic_resume_child(device_t dev, device_t child) +{ + DEVICE_RESUME(child); + + dev->flags &= ~DF_SUSPENDED; + return (0); +} + +/** * @brief Helper function for implementing DEVICE_SUSPEND() * * This function can be used to help implement the DEVICE_SUSPEND() @@ -3646,12 +3678,12 @@ device_t child, child2; TAILQ_FOREACH(child, &dev->children, link) { - error = DEVICE_SUSPEND(child); + error = BUS_SUSPEND_CHILD(dev, child); if (error) { for (child2 = TAILQ_FIRST(&dev->children); child2 && child2 != child; child2 = TAILQ_NEXT(child2, link)) - DEVICE_RESUME(child2); + BUS_RESUME_CHILD(dev, child2); return (error); } } @@ -3670,7 +3702,7 @@ device_t child; TAILQ_FOREACH(child, &dev->children, link) { - DEVICE_RESUME(child); + BUS_RESUME_CHILD(dev, child); /* if resume fails, there's nothing we can usefully do... */ } return (0); Index: sys/dev/pci/pci.c =================================================================== --- sys/dev/pci/pci.c (revision 269798) +++ sys/dev/pci/pci.c (working copy) @@ -162,6 +162,8 @@ DEVMETHOD(bus_child_pnpinfo_str, pci_child_pnpinfo_str_method), DEVMETHOD(bus_child_location_str, pci_child_location_str_method), DEVMETHOD(bus_remap_intr, pci_remap_intr_method), + DEVMETHOD(bus_suspend_child, pci_suspend_child), + DEVMETHOD(bus_resume_child, pci_resume_child), /* PCI interface */ DEVMETHOD(pci_read_config, pci_read_config_method), @@ -3614,12 +3616,11 @@ #endif static void -pci_set_power_children(device_t dev, device_t *devlist, int numdevs, - int state) +pci_set_power_child(device_t dev, device_t child, int state) { - device_t child, pcib; struct pci_devinfo *dinfo; - int dstate, i; + device_t pcib; + int dstate; /* * Set the device to the given state. If the firmware suggests @@ -3629,74 +3630,74 @@ * are handled separately. */ pcib = device_get_parent(dev); - for (i = 0; i < numdevs; i++) { - child = devlist[i]; - dinfo = device_get_ivars(child); - dstate = state; - if (device_is_attached(child) && - PCIB_POWER_FOR_SLEEP(pcib, dev, &dstate) == 0) - pci_set_powerstate(child, dstate); - } + dinfo = device_get_ivars(child); + dstate = state; + if (device_is_attached(child) && + PCIB_POWER_FOR_SLEEP(pcib, dev, &dstate) == 0) + pci_set_powerstate(child, dstate); } int -pci_suspend(device_t dev) +pci_suspend_child(device_t dev, device_t child) { - device_t child, *devlist; struct pci_devinfo *dinfo; - int error, i, numdevs; + int error; + dinfo = device_get_ivars(child); + /* - * Save the PCI configuration space for each child and set the + * Save the PCI configuration space for child child and set the * device in the appropriate power state for this sleep state. */ - if ((error = device_get_children(dev, &devlist, &numdevs)) != 0) - return (error); - for (i = 0; i < numdevs; i++) { - child = devlist[i]; - dinfo = device_get_ivars(child); - pci_cfg_save(child, dinfo, 0); - } + pci_cfg_save(child, dinfo, 0); /* Suspend devices before potentially powering them down. */ - error = bus_generic_suspend(dev); - if (error) { - free(devlist, M_TEMP); + error = bus_generic_suspend_child(dev, child); + + if (error) return (error); - } - if (pci_do_power_suspend) - pci_set_power_children(dev, devlist, numdevs, - PCI_POWERSTATE_D3); - free(devlist, M_TEMP); + + pci_set_power_child(dev, child, PCI_POWERSTATE_D3); + return (0); } int +pci_suspend(device_t dev) +{ + int error; + + error = bus_generic_suspend(dev); + return (error); +} + +int +pci_resume_child(device_t dev, device_t child) +{ + struct pci_devinfo *dinfo; + + if (pci_do_power_resume) + pci_set_power_child(dev, child, PCI_POWERSTATE_D0); + + dinfo = device_get_ivars(child); + pci_cfg_restore(child, dinfo); + if (!device_is_attached(child)) + pci_cfg_save(child, dinfo, 1); + + bus_generic_resume_child(dev, child); + + return (0); +} + +int pci_resume(device_t dev) { device_t child, *devlist; - struct pci_devinfo *dinfo; int error, i, numdevs; - /* - * Set each child to D0 and restore its PCI configuration space. - */ if ((error = device_get_children(dev, &devlist, &numdevs)) != 0) return (error); - if (pci_do_power_resume) - pci_set_power_children(dev, devlist, numdevs, - PCI_POWERSTATE_D0); - /* Now the device is powered up, restore its config space. */ - for (i = 0; i < numdevs; i++) { - child = devlist[i]; - dinfo = device_get_ivars(child); - - pci_cfg_restore(child, dinfo); - if (!device_is_attached(child)) - pci_cfg_save(child, dinfo, 1); - } - /* * Resume critical devices first, then everything else later. */ @@ -3707,7 +3708,7 @@ case PCIC_MEMORY: case PCIC_BRIDGE: case PCIC_BASEPERIPH: - DEVICE_RESUME(child); + BUS_RESUME_CHILD(dev, child); break; } } @@ -3720,7 +3721,7 @@ case PCIC_BASEPERIPH: break; default: - DEVICE_RESUME(child); + BUS_RESUME_CHILD(dev, child); } } free(devlist, M_TEMP); Index: sys/dev/pci/pci_private.h =================================================================== --- sys/dev/pci/pci_private.h (revision 269798) +++ sys/dev/pci/pci_private.h (working copy) @@ -118,7 +118,9 @@ char *buf, size_t buflen); int pci_assign_interrupt_method(device_t dev, device_t child); int pci_resume(device_t dev); +int pci_resume_child(device_t dev, device_t child); int pci_suspend(device_t dev); +int pci_suspend_child(device_t dev, device_t child); bus_dma_tag_t pci_get_dma_tag(device_t bus, device_t dev); /** Restore the config register state. The state must be previously Index: sys/sys/bus.h =================================================================== --- sys/sys/bus.h (revision 269798) +++ sys/sys/bus.h (working copy) @@ -339,6 +339,7 @@ int bus_generic_release_resource(device_t bus, device_t child, int type, int rid, struct resource *r); int bus_generic_resume(device_t dev); +int bus_generic_resume_child(device_t dev, device_t child); int bus_generic_setup_intr(device_t dev, device_t child, struct resource *irq, int flags, driver_filter_t *filter, driver_intr_t *intr, @@ -357,6 +358,7 @@ int bus_generic_shutdown(device_t dev); int bus_generic_suspend(device_t dev); +int bus_generic_suspend_child(device_t dev, device_t child); int bus_generic_teardown_intr(device_t dev, device_t child, struct resource *irq, void *cookie); int bus_generic_write_ivar(device_t dev, device_t child, int which, --MP_/J8yUo/400lG2N5dYyImkHpS--