On Tuesday 13 January 2009, Alan Stern wrote: > On Wed, 7 Jan 2009, Rafael J. Wysocki wrote: > > > Hi, > > > > Sorry for being late, but -> > > > > On Wednesday 07 January 2009, Greg Kroah-Hartman wrote: > > > From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > > > > > This patch (as1192) rearranges the USB PCI host controller suspend and > > > resume and resume routines: > > > > > > Use pci_wake_from_d3() for enabling and disabling wakeup, > > > instead of pci_enable_wake(). > > > > > > Carry out the actual state change while interrupts are > > > disabled. > > > > > > Change the order of the preparations to agree with the > > > general recommendation for PCI devices, instead of > > > messing around with the wakeup settings while the device > > > is in D3. > > > > > > In .suspend: > > > Call the underlying driver to disable IRQ > > > generation; > > > pci_wake_from_d3(device_may_wakeup()); > > > pci_disable_device(); > > > > > > In .suspend_late: > > > pci_save_state(); > > > pci_set_power_state(D3hot); > > > > -> > > pci_set_power_state() does things that shouldn't be done with interrupts off, > > so I'd move the above two into .suspend. > > > > > (for PPC_PMAC) Disable ASIC clocks > > > > > In .resume_early: > > > (for PPC_PMAC) Enable ASIC clocks > > > pci_set_power_state(D0); > > > > the pci_set_power_state(D0) is not necessary here IMO. If the device is > > accessible at all, its config space is accessible too, even in D3. > > > > pci_enable_device() does pci_set_power_state(D0) anyway. > > > > > pci_restore_state(); > > > > > > In .resume: > > > pci_enable_device(); > > > pci_set_master(); > > > pci_wake_from_d3(0); > > > Call the underlying driver to reenable IRQ > > > generation > > You are basically suggesting that everything except the PPC_PMAC stuff > be moved out of the interrupts-off region. Is there in fact any > remaining reason for us to use suspend_late or resume_early? It > doesn't seem like it. Yes, it is. Please have a look at the new PCI PM callbacks in drivers/pci/pci-driver.c . Basically, IMO the standard config spaces should be restored with interrupts off if possible. BTW, I think USB might benefit from implementing the new framework. ;-) > Yet Linus constantly goes on complaining about how PCI drivers should > be doing more of their suspend/resume work with interrupts disabled. > And the patches you have posted since writing the email above don't do > exactly what you recommend here. Actually, the last one is doing exactly that or else it's buggy. > So what's the correct story? As in drivers/pci/pci-driver.c , I believe. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
