On Fri, 13 Feb 2015 13:47:07 +0200, Jarkko Nikula wrote:
> On 02/13/2015 12:47 PM, Jean Delvare wrote:
> > On Wed, 11 Feb 2015 14:32:08 +0200, Jarkko Nikula wrote:
> >> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> >> index 5fb35464f693..9f7b69743233 100644
> >> --- a/drivers/i2c/busses/i2c-i801.c
> >> +++ b/drivers/i2c/busses/i2c-i801.c
> >> @@ -1180,12 +1180,13 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >> }
> >> priv->features &= ~disable_features;
> >>
> >> - err = pci_enable_device(dev);
> >> + err = pcim_enable_device(dev);
> >> if (err) {
> >> dev_err(&dev->dev, "Failed to enable SMBus PCI device (%d)\n",
> >> err);
> >> goto exit;
> >> }
> >> + pcim_pin_device(dev);
> >>
> >> /* Determine the address of the SMBus area */
> >> priv->smba = pci_resource_start(dev, SMBBAR);
> >
> > What is the benefit of this change, compared to just leaving the call
> > to pci_enable_device() in place?
>
> If you mean the patch itself I think easy pcim_pin_device() removal is
> the biggest benefit if we ever want to experiment does hang during
> power-off problem still exist in some machines. All PCI, ACPI and
> power-off code have evolved in 9 years so original problem may not exist
> anymore. Who knows.
>
> Without this patch pcm_disable_device() needs to be added to error paths
> and i801_remove(). (of course trivial but it's always more nice to get
> regression from one-liner).
>
> If you mean plain pcim_enable_device() it is required for other pcim_
> functions since it allocates pcim_release which takes care of cleanup
> bunch of things including pci_release_region().
>
> E.g. pcim_iomap_regions() without pcim_enable_device() causes an oops
> when doing "cat /proc/ioports" after module removal since
> pcim_iomap_release() only unmaps but does not release regions.
I meant the latter. If there's a good reason for calling
pcim_enable_device then alright.
--
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
