Hi Jarkko,
On Fri, 13 Feb 2015 13:13:18 +0200, Jarkko Nikula wrote:
> On 02/13/2015 12:33 PM, Jean Delvare wrote:
> > 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.
OK, you convinced me. I'll still test the updated driver on my two
IBM/Lenovo Thinkpad laptops (T60p and X230) to make sure we didn't miss
anything.
Thanks,
--
Jean Delvare
SUSE L3 Support
--
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
