On Wed, 2009-02-04 at 01:59 +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@xxxxxxx> > > Make pci_legacy_suspend() save the state of the device if it is > in PCI_UNKNOWN after its suspend callback has run and warn only if > the power state of the device has been changed by its suspend > callback. > > Also, use WARN_ONCE(), which is more useful, in pci_legacy_suspend(), > so that the name of the offending function is printed. > > Additionaly, remove the unnecessary line of code setting > pci_dev->state_saved. Minor nit: Should the warning be preceeded by a message ? The reason is, right now, all we get is a backtrace, it doesn't actually tell you which device or driver caused it which makes it pretty pointless. I think you should add a printk(KERN_ERR... just before that which gives those informations along with a little blurb along the lines of "driver changed device state without saving config space state"). > Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx> > Reported-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> > --- > drivers/pci/pci-driver.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 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 > @@ -355,6 +355,8 @@ static int pci_legacy_suspend(struct dev > int i = 0; > > if (drv && drv->suspend) { > + pci_power_t prev = pci_dev->current_state; > + > pci_dev->state_saved = false; > > i = drv->suspend(pci_dev, state); > @@ -365,12 +367,16 @@ static int pci_legacy_suspend(struct dev > if (pci_dev->state_saved) > goto Fixup; > > - if (WARN_ON_ONCE(pci_dev->current_state != PCI_D0)) > + if (pci_dev->current_state != PCI_D0 > + && pci_dev->current_state != PCI_UNKNOWN) { > + WARN_ONCE(pci_dev->current_state != prev, > + "PCI PM: Device state not saved by %pF\n", > + drv->suspend); > goto Fixup; > + } > } > > pci_save_state(pci_dev); > - pci_dev->state_saved = true; > /* > * This is for compatibility with existing code with legacy PM support. > */ -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html