Hi

On 02/13/2015 12:33 PM, Jean Delvare wrote:
Hi Jarkko,

On Wed, 11 Feb 2015 14:32:07 +0200, Jarkko Nikula wrote:
Since pci_disable_device() is not called from i801_suspend() and power
state is set already it means that subsequent pci_enable_device() calls do
practically nothing but monotonically increase struct pci_dev enable_cnt.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
  drivers/i2c/busses/i2c-i801.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b1d725d758bb..5fb35464f693 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1324,7 +1324,7 @@ static int i801_resume(struct pci_dev *dev)
  {
        pci_set_power_state(dev, PCI_D0);
        pci_restore_state(dev);
-       return pci_enable_device(dev);
+       return 0;
  }
  #else
  #define i801_suspend NULL

This looks reasonable but have you tested this change on a range of
actual laptops to make sure it has no unexpected side effect?

Unfortunately I have only limited amount of test hw which are working fine even if PCI device is disabled so I cannot hit those issues that were seen in the past.

I suppose this patch unlikely cause regression since if you look at the call chain pci_enable_device() -> pci_enable_device_flags() it doesn't go beyond taking the current power state since enable_cnt is always greater than 1.

drivers/pci/pci.c: pci_enable_device_flags():

if (dev->pm_cap) {
        u16 pmcsr;
        pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
        dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
}

if (atomic_inc_return(&dev->enable_cnt) > 1)
        return 0;               /* already enabled */

To me it seems current power state taking is practically no-op when device is enabled since pci_set_power_state() calls in i801_suspend() and i801_resume() have already cached it.

--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to