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.nikula@xxxxxxxxxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html