Date: Sun, 10 Aug 2014 22:30:30 -0700 From: Justin Hibbits <chmeeedalf@gmail.com> (by way of Justin Hibbits <chmeeedalf@gmail.com>) To: <freebsd-current-unsubscribe@freebsd.org> Subject: Child suspend/resume Message-ID: <20140810223030.479badbc@zhabar.att.net> Resent-Message-ID: <20140810224409.4f1b028e@zhabar.att.net>
next in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140810223030.479badbc>