Re: [PATCHv2 4/5] i2c: i801: Remove pci_enable_device() call from i801_resume()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux