[RFC][PATCH] PCI PM: Make new suspend-resume callbacks carry out core operations (rev. 2)

From: Rafael J. Wysocki
Date: Sun Dec 28 2008 - 10:07:35 EST

Next message: Arnaldo Carvalho de Melo: "Re: 2.6.24.7-rt25 and 2.6.26.8-rt12"
Previous message: Matthew Garrett: "Re: Patch enabling Bluetooth for Asus EEE 901"
Next in thread: Rafael J. Wysocki: "[RFC][PATCH 0/10] PCI PM: Make new suspend-resume callbacks carry out core operations"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Friday 19 December 2008, Rafael J. Wysocki wrote:
> Hi Len,
>
> The patch below adds the callbacks that save/restore the standard PCI config
> registers, change the power state of the device etc. to the new suspend-resume
> callbacks (which are not yet used by device drivers), so that the drivers don't
> have to worry about these operations.
>
> The saving and restoring of the standard PCI config registers is done with
> interrupts off, the other things are done with interrupts enabled.
>
> The patch should apply to the current linux-next tree.

Below is a new version of the patch.

Now, the standard config spaces of devices are restored with interrupts
disabled, but only if the bridges the devices are behind are in D0. Otherwise,
the operation is attempted again with interrupts enabled (presumably the
bridge would be put into D0 before that happens).

Also, pci_disable_device() is not called during resume and
pci_reenable_device() is used to enable the devices that were enabled during
suspend. This allows us to automatically call pci_set_master() for devices
that were bus masters before suspend. Additionally, the pci_reenable_device()
may be overriden by the driver, by disabling the device during suspend.

Finally, bridges are handled a bit differently from the regular devices (they
are not put into low power states during suspend and pci_enable_wake() is
not called for them.

The patch should apply on top of linux-next with the patch fixing
pci_update_current_state() I sent yesterday
(http://marc.info/?l=linux-kernel&m=123039199607159&w=4).

Please review.

Thanks,
Rafael

---
There are a few standard PCI callbacks that should be used by any
PCI driver implementing suspend-resume handlers. The drivers,
however, tend to use these callbacks in different ways, in different
order or even not at all. This sometimes causes problems to appear
and makes PCI suspend-resume diagnostics troublesome. Thus it seems
reasonable to make the PCI core execute the standard callbacks so
that the drivers don't have to do it.

Modify the power management framework for PCI devices so that the
standard PCI PM callbacks are executed by the core in the following
way:

* Right after executing the ->suspend() callback from the device
driver's pm object (ie. with interrupts enabled) the core will
execute pci_save_state() and, if the device is not a PCI bridge,
the core will execute pci_prepare_to_sleep().

* Right prior to executing the ->resume_noirq() callback from the
device driver's pm object (ie. with interrupts disabled), the core
will execute pci_restore_state() and will update the device's
pci_dev structure to reflect the actual current power state of the
device. However, this is only going to succeed if the device's bus
is operational (ie. its parent bridge in is the D0 power state) at
that time, so the last operation has to be repeated later.

* Prior to executing the ->resume() callback from the device driver's
pm object (ie. with interrupts enabled) the core will check if the
device's standard config registers need to be restored and it will
call pci_restore_state() if necessary. Next, if the device is not
a PCI bridge, the core will disable the device's wake-up capability
using pci_enable_wake(). Finally, it will execute
pci_reenable_device() and, if necessary, pci_set_master(), in this
order (all of this happens before the ->resume() callback is
invoked).

The standard callbacks mentioned above will always be executed for
all devices without drivers providing the legacy PCI PM support
(ie. the ->suspend(), ->suspend_late(), ->resume_early()
or ->resume() hooks in the pci_driver structure). In particular,
they will be executed for devices that don't have drivers at all.

Analogous changes are made in the hibernation-related PCI PM
routines.

Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
---
drivers/pci/pci-driver.c | 356 +++++++++++++++++++++++++++--------------------
drivers/pci/pci.c | 2
drivers/pci/pci.h | 1
3 files changed, 212 insertions(+), 147 deletions(-)

Index: linux-2.6/drivers/pci/pci-driver.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-driver.c
+++ linux-2.6/drivers/pci/pci-driver.c
@@ -300,98 +300,83 @@ static void pci_device_shutdown(struct d

#ifdef CONFIG_PM_SLEEP

-static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
-{
- struct pci_driver *drv = pci_dev->driver;
-
- return drv && (drv->suspend || drv->suspend_late || drv->resume
- || drv->resume_early);
-}
+/* Routines used in both the legacy and new power management callbacks */

-/*
- * Default "suspend" method for devices that have no driver provided suspend,
- * or not even a driver at all.
- */
-static void pci_default_pm_suspend(struct pci_dev *pci_dev)
+static int pci_restore_standard_config(struct pci_dev *pci_dev)
{
- pci_save_state(pci_dev);
- /*
- * mark its power state as "unknown", since we don't know if
- * e.g. the BIOS will change its device state when we suspend.
- */
- if (pci_dev->current_state == PCI_D0)
+ struct pci_dev *parent = pci_dev->bus->self;
+ int error = 0;
+
+ /* Check if the device's bus is operational */
+ if (!parent || parent->current_state == PCI_D0) {
+ pci_restore_state(pci_dev);
+ pci_update_current_state(pci_dev, PCI_D0);
+ } else {
+ dev_warn(&pci_dev->dev, "unable to restore config, "
+ "bridge %s in low power state D%d\n", pci_name(parent),
+ parent->current_state);
pci_dev->current_state = PCI_UNKNOWN;
-}
+ error = -EAGAIN;
+ }

-/*
- * Default "resume" method for devices that have no driver provided resume,
- * or not even a driver at all (first part).
- */
-static void pci_default_pm_resume_early(struct pci_dev *pci_dev)
-{
- /* restore the PCI config space */
- pci_restore_state(pci_dev);
+ return error;
}

-/*
- * Default "resume" method for devices that have no driver provided resume,
- * or not even a driver at all (second part).
- */
-static int pci_default_pm_resume_late(struct pci_dev *pci_dev)
-{
- int retval;
-
- /* if the device was enabled before suspend, reenable */
- retval = pci_reenable_device(pci_dev);
- /*
- * if the device was busmaster before the suspend, make it busmaster
- * again
- */
- if (pci_dev->is_busmaster)
- pci_set_master(pci_dev);
-
- return retval;
-}
+/* Legacy power management callbacks */

static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev * pci_dev = to_pci_dev(dev);
struct pci_driver * drv = pci_dev->driver;
- int i = 0;
+ int error = 0;

if (drv && drv->suspend) {
- i = drv->suspend(pci_dev, state);
- suspend_report_result(drv->suspend, i);
+ error = drv->suspend(pci_dev, state);
+ suspend_report_result(drv->suspend, error);
} else {
- pci_default_pm_suspend(pci_dev);
+ error = pci_save_state(pci_dev);
+ /*
+ * mark its power state as "unknown", since we don't know if
+ * e.g. the BIOS will change its device state when we suspend.
+ */
+ if (!error && pci_dev->current_state == PCI_D0)
+ pci_dev->current_state = PCI_UNKNOWN;
}
- return i;
+ return error;
}

static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
{
struct pci_dev * pci_dev = to_pci_dev(dev);
struct pci_driver * drv = pci_dev->driver;
- int i = 0;
+ int error = 0;

if (drv && drv->suspend_late) {
- i = drv->suspend_late(pci_dev, state);
- suspend_report_result(drv->suspend_late, i);
+ error = drv->suspend_late(pci_dev, state);
+ suspend_report_result(drv->suspend_late, error);
}
- return i;
+ return error;
}

static int pci_legacy_resume(struct device *dev)
{
- int error;
struct pci_dev * pci_dev = to_pci_dev(dev);
struct pci_driver * drv = pci_dev->driver;
+ int error;

if (drv && drv->resume) {
error = drv->resume(pci_dev);
} else {
- pci_default_pm_resume_early(pci_dev);
- error = pci_default_pm_resume_late(pci_dev);
+ /* restore the PCI config space */
+ pci_restore_state(pci_dev);
+ /* if the device was enabled before suspend, reenable */
+ error = pci_reenable_device(pci_dev);
+ /*
+ * if the device was busmaster before the suspend, make
+ * it busmaster again
+ */
+ if (pci_dev->is_busmaster)
+ pci_set_master(pci_dev);
}
return error;
}
@@ -407,6 +392,62 @@ static int pci_legacy_resume_early(struc
return error;
}

+/* Routines used in the new power management callbacks */
+
+static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
+{
+ struct pci_driver *drv = pci_dev->driver;
+
+ return drv && (drv->suspend || drv->suspend_late || drv->resume
+ || drv->resume_early);
+}
+
+static bool pci_is_bridge(struct pci_dev *pci_dev)
+{
+ return !!(pci_dev->subordinate);
+}
+
+static void pci_pm_default_suspend_noirq(struct pci_dev *pci_dev)
+{
+ /* The device's power state may be changed by the BIOS before resume */
+ if( pci_dev->current_state == PCI_D0)
+ pci_dev->current_state = PCI_UNKNOWN;
+}
+
+static int pci_pm_default_resume(struct pci_dev *pci_dev)
+{
+ int error = 0;
+
+ /*
+ * pci_restore_standard_config() should have been called before, but it
+ * would have failed if the device's bus had not been operational at
+ * that time. Check it and try again if necessary.
+ */
+ if (pci_dev->current_state == PCI_UNKNOWN) {
+ error = pci_restore_standard_config(pci_dev);
+ if (error)
+ return error;
+ }
+
+ pci_fixup_device(pci_fixup_resume, pci_dev);
+
+ /* Disable wake-up capability of regular devices */
+ if (!pci_is_bridge(pci_dev))
+ pci_enable_wake(pci_dev, PCI_D0, false);
+ /* If device was enabled before suspend, reenable it. */
+ error = pci_reenable_device(pci_dev);
+ /*
+ * If device was a bus master before the suspend, make it a bus master
+ * again.
+ */
+ if (pci_dev->is_busmaster)
+ pci_set_master(pci_dev);
+
+ return error;
+}
+
+/* New power management callbacks */
+
static int pci_pm_prepare(struct device *dev)
{
struct device_driver *drv = dev->driver;
@@ -434,14 +475,23 @@ static int pci_pm_suspend(struct device
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->suspend) {
- error = drv->pm->suspend(dev);
- suspend_report_result(drv->pm->suspend, error);
- }
- } else if (pci_has_legacy_pm_support(pci_dev)) {
+ if (pci_has_legacy_pm_support(pci_dev)) {
error = pci_legacy_suspend(dev, PMSG_SUSPEND);
+ goto Exit;
+ }
+
+ if (drv && drv->pm && drv->pm->suspend) {
+ error = drv->pm->suspend(dev);
+ suspend_report_result(drv->pm->suspend, error);
+ }
+
+ if (!error) {
+ error = pci_save_state(pci_dev);
+ if (!error && !pci_is_bridge(pci_dev))
+ pci_prepare_to_sleep(pci_dev);
}
+
+ Exit:
pci_fixup_device(pci_fixup_suspend, pci_dev);

return error;
@@ -453,17 +503,17 @@ static int pci_pm_suspend_noirq(struct d
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->suspend_noirq) {
- error = drv->pm->suspend_noirq(dev);
- suspend_report_result(drv->pm->suspend_noirq, error);
- }
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_suspend_late(dev, PMSG_SUSPEND);
- } else {
- pci_default_pm_suspend(pci_dev);
+ if (pci_has_legacy_pm_support(pci_dev))
+ return pci_legacy_suspend_late(dev, PMSG_SUSPEND);
+
+ if (drv && drv->pm && drv->pm->suspend_noirq) {
+ error = drv->pm->suspend_noirq(dev);
+ suspend_report_result(drv->pm->suspend_noirq, error);
}

+ if (!error)
+ pci_pm_default_suspend_noirq(pci_dev);
+
return error;
}

@@ -473,17 +523,16 @@ static int pci_pm_resume(struct device *
struct device_driver *drv = dev->driver;
int error = 0;

- pci_fixup_device(pci_fixup_resume, pci_dev);
-
- if (drv && drv->pm) {
- if (drv->pm->resume)
- error = drv->pm->resume(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_resume(dev);
- } else {
- error = pci_default_pm_resume_late(pci_dev);
+ if (pci_has_legacy_pm_support(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume, pci_dev);
+ return pci_legacy_resume(dev);
}

+ error = pci_pm_default_resume(pci_dev);
+
+ if (!error && drv && drv->pm && drv->pm->resume)
+ error = drv->pm->resume(dev);
+
return error;
}

@@ -493,15 +542,16 @@ static int pci_pm_resume_noirq(struct de
struct device_driver *drv = dev->driver;
int error = 0;

- pci_fixup_device(pci_fixup_resume_early, to_pci_dev(dev));
+ if (pci_has_legacy_pm_support(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume_early, pci_dev);
+ return pci_legacy_resume_early(dev);
+ }

- if (drv && drv->pm) {
- if (drv->pm->resume_noirq)
+ if (!pci_restore_standard_config(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume_early, pci_dev);
+
+ if (drv && drv->pm && drv->pm->resume_noirq)
error = drv->pm->resume_noirq(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_resume_early(dev);
- } else {
- pci_default_pm_resume_early(pci_dev);
}

return error;
@@ -524,16 +574,20 @@ static int pci_pm_freeze(struct device *
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->freeze) {
- error = drv->pm->freeze(dev);
- suspend_report_result(drv->pm->freeze, error);
- }
- } else if (pci_has_legacy_pm_support(pci_dev)) {
+ if (pci_has_legacy_pm_support(pci_dev)) {
error = pci_legacy_suspend(dev, PMSG_FREEZE);
pci_fixup_device(pci_fixup_suspend, pci_dev);
+ return error;
}

+ if (drv && drv->pm && drv->pm->freeze) {
+ error = drv->pm->freeze(dev);
+ suspend_report_result(drv->pm->freeze, error);
+ }
+
+ if (!error)
+ error = pci_save_state(pci_dev);
+
return error;
}

@@ -543,17 +597,17 @@ static int pci_pm_freeze_noirq(struct de
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->freeze_noirq) {
- error = drv->pm->freeze_noirq(dev);
- suspend_report_result(drv->pm->freeze_noirq, error);
- }
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_suspend_late(dev, PMSG_FREEZE);
- } else {
- pci_default_pm_suspend(pci_dev);
+ if (pci_has_legacy_pm_support(pci_dev))
+ return pci_legacy_suspend_late(dev, PMSG_FREEZE);
+
+ if (drv && drv->pm && drv->pm->freeze_noirq) {
+ error = drv->pm->freeze_noirq(dev);
+ suspend_report_result(drv->pm->freeze_noirq, error);
}

+ if (!error)
+ pci_pm_default_suspend_noirq(pci_dev);
+
return error;
}

@@ -563,14 +617,14 @@ static int pci_pm_thaw(struct device *de
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->thaw)
- error = drv->pm->thaw(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
+ if (pci_has_legacy_pm_support(pci_dev)) {
pci_fixup_device(pci_fixup_resume, pci_dev);
- error = pci_legacy_resume(dev);
+ return pci_legacy_resume(dev);
}

+ if (drv && drv->pm && drv->pm->thaw)
+ error = drv->pm->thaw(dev);
+
return error;
}

@@ -580,14 +634,17 @@ static int pci_pm_thaw_noirq(struct devi
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->thaw_noirq)
- error = drv->pm->thaw_noirq(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- pci_fixup_device(pci_fixup_resume_early, to_pci_dev(dev));
- error = pci_legacy_resume_early(dev);
+ if (pci_has_legacy_pm_support(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume_early, pci_dev);
+ return pci_legacy_resume_early(dev);
}

+ if (pci_dev->current_state == PCI_UNKNOWN)
+ pci_update_current_state(pci_dev, PCI_D0);
+
+ if (drv && drv->pm && drv->pm->thaw_noirq)
+ error = drv->pm->thaw_noirq(dev);
+
return error;
}

@@ -597,32 +654,39 @@ static int pci_pm_poweroff(struct device
struct device_driver *drv = dev->driver;
int error = 0;

- pci_fixup_device(pci_fixup_suspend, pci_dev);
-
- if (drv && drv->pm) {
- if (drv->pm->poweroff) {
- error = drv->pm->poweroff(dev);
- suspend_report_result(drv->pm->poweroff, error);
- }
- } else if (pci_has_legacy_pm_support(pci_dev)) {
+ if (pci_has_legacy_pm_support(pci_dev)) {
error = pci_legacy_suspend(dev, PMSG_HIBERNATE);
+ goto Exit;
+ }
+
+ if (drv && drv->pm && drv->pm->poweroff) {
+ error = drv->pm->poweroff(dev);
+ suspend_report_result(drv->pm->poweroff, error);
+ }
+
+ if (!error && !pci_is_bridge(pci_dev)) {
+ pci_disable_device(pci_dev);
+ pci_prepare_to_sleep(pci_dev);
}

+ Exit:
+ pci_fixup_device(pci_fixup_suspend, pci_dev);
+
return error;
}

static int pci_pm_poweroff_noirq(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->poweroff_noirq) {
- error = drv->pm->poweroff_noirq(dev);
- suspend_report_result(drv->pm->poweroff_noirq, error);
- }
- } else if (pci_has_legacy_pm_support(to_pci_dev(dev))) {
- error = pci_legacy_suspend_late(dev, PMSG_HIBERNATE);
+ if (pci_has_legacy_pm_support(pci_dev))
+ return pci_legacy_suspend_late(dev, PMSG_HIBERNATE);
+
+ if (drv && drv->pm && drv->pm->poweroff_noirq) {
+ error = drv->pm->poweroff_noirq(dev);
+ suspend_report_result(drv->pm->poweroff_noirq, error);
}

return error;
@@ -634,15 +698,15 @@ static int pci_pm_restore(struct device
struct device_driver *drv = dev->driver;
int error = 0;

- if (drv && drv->pm) {
- if (drv->pm->restore)
- error = drv->pm->restore(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_resume(dev);
- } else {
- error = pci_default_pm_resume_late(pci_dev);
+ if (pci_has_legacy_pm_support(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume, pci_dev);
+ return pci_legacy_resume(dev);
}
- pci_fixup_device(pci_fixup_resume, pci_dev);
+
+ error = pci_pm_default_resume(pci_dev);
+
+ if (!error && drv && drv->pm && drv->pm->restore)
+ error = drv->pm->restore(dev);

return error;
}
@@ -653,17 +717,17 @@ static int pci_pm_restore_noirq(struct d
struct device_driver *drv = dev->driver;
int error = 0;

- pci_fixup_device(pci_fixup_resume, pci_dev);
+ if (pci_has_legacy_pm_support(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume_early, pci_dev);
+ return pci_legacy_resume_early(dev);
+ }
+
+ if (!pci_restore_standard_config(pci_dev)) {
+ pci_fixup_device(pci_fixup_resume_early, pci_dev);

- if (drv && drv->pm) {
- if (drv->pm->restore_noirq)
+ if (drv && drv->pm && drv->pm->restore_noirq)
error = drv->pm->restore_noirq(dev);
- } else if (pci_has_legacy_pm_support(pci_dev)) {
- error = pci_legacy_resume_early(dev);
- } else {
- pci_default_pm_resume_early(pci_dev);
}
- pci_fixup_device(pci_fixup_resume_early, pci_dev);

return error;
}
Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -526,7 +526,7 @@ pci_raw_set_power_state(struct pci_dev *
* @dev: PCI device to handle.
* @state: State to cache in case the device doesn't have the PM capability
*/
-static void pci_update_current_state(struct pci_dev *dev, pci_power_t state)
+void pci_update_current_state(struct pci_dev *dev, pci_power_t state)
{
if (dev->pm_cap) {
u16 pmcsr;
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -121,6 +121,7 @@ static inline int pci_no_d1d2(struct pci
return (dev->no_d1d2 || parent_dstates);

}
+extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
extern struct device_attribute dev_attr_cpuaffinity;

N?§˛ćěr¸?yúč?Řb˛XŹśÇ§vŘ^?)Ţş{.nÇ+?ˇĽ?{ą?ęçzX§ś?ĄÜ¨}Š?˛Ć zÚ&j:+v?¨žŤ?ęçzZ+?Ę+zfŁ˘ˇh??§~??Űi˙űŕzšŽwĽ˘¸??¨čÚ&˘)ß˘f?ů^jÇŤy§m?á@AŤaśÚ˙0śěhŽĺ?i

Next message: Arnaldo Carvalho de Melo: "Re: 2.6.24.7-rt25 and 2.6.26.8-rt12"
Previous message: Matthew Garrett: "Re: Patch enabling Bluetooth for Asus EEE 901"
Next in thread: Rafael J. Wysocki: "[RFC][PATCH 0/10] PCI PM: Make new suspend-resume callbacks carry out core operations"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]