At Wed, 7 Jan 2015 13:07:41 +0200, Andy Shevchenko wrote:
On Wed, Jan 7, 2015 at 12:41 PM, Takashi Iwai tiwai@suse.de wrote:
At Wed, 7 Jan 2015 12:35:10 +0200, Andy Shevchenko wrote:
On Wed, Jan 7, 2015 at 8:54 AM, Takashi Iwai tiwai@suse.de wrote:
At Tue, 6 Jan 2015 22:39:15 +0200, Andy Shevchenko wrote:
There is no need to duplicate the work that is already done in the PCI driver core. The patch removes excerpts from suspend and resume callbacks.
While here amend a definition of the snd_fm801_pm. The PM core has the stubs for non-PM_SLEEP cases, thus we can always define the variable.
But this will result in a waste of bytes, no? It's not a big deal, but still...
We can leave it under #ifdef, the main purpose of the patch to remove duplicate PM work.
Yes.
Okay for patch v2.
above all, I couldn't find the code enabling/disabling pci device in the default pci pm handlers. pci_pm_default_resume_early() powers up and restore the state. These can be removed indeed. But pci_pm_restore() calls pci_pm_reenable_device() only as a fallback. So it looks to me like the restore callback still needs to enable the device explicitly.
Am I missing anything obvious...?
The suspend_noirq is called at the end of the suspend process. It gets device to the lower power state. I think this was implemented by 46939f8b15e44f065d052e89ea4f2adc81fdc740.
There is a really nice documentation in dev_pm_ops description (linux/pm.h).
Well, this does only power up/down and register save/restore. Where pci_disable_device() and pci_enable_device() (or their variants) are called in the framework?
pci_pm_suspend_noirq() -> pci_prepare_to_sleep() -> pci_set_power_state() [pci_target_state()] So, this regarding to power state.
I doubt we have to do this during suspend/resume cycle. What is the point?
The calls are needed in the past, especially when the interrupt number varies at resume (and thus re-acquiring the interrupt at resume).
I guess these calls are nowadays superfluous. But, even if so, you must describe it clearly. It's no duplicated calls, after all, and this has to be clarified in a more exact context.
Takashi