On Mon, Jun 13, 2016 at 11:45:00AM +0200, Pali Rohár wrote:

On Monday 13 June 2016 11:19:46 Jean Delvare wrote:

quoted

Hi Benjamin,

On Wed, 8 Jun 2016 18:29:13 +0200, Benjamin Tissoires wrote:

quoted

quoted

 static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	unsigned char temp;

@@ -1277,6 +1368,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	priv->adapter.dev.parent = &dev->dev;
 	ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&dev->dev));
 	priv->adapter.retries = 3;
+	mutex_init(&priv->acpi_lock);
 
 	priv->pci_dev = dev;
 	switch (dev->device) {

@@ -1339,10 +1431,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENODEV;
 	}
 
-	err = acpi_check_resource_conflict(&dev->resource[SMBBAR]);
-	if (err) {
+	err = i801_acpi_probe(priv);
+	if (err)
 		return -ENODEV;
-	}

I'd say that once this has been set, we need to call
acpi_remove_address_space_handler() in case of failure later (in the 2
returns after).

Good catch, sorry for missing it during my review.

The first error return can probably be left unchanged by calling
i801_acpi_probe() after it rather than before. The second will need a
call to i801_acpi_remove(priv) for sure.

Maybe we should call acpi_remove_address_space_handler() after first
ACPI access to driver?

I fixed it already in v6 which got applied by Wolfram.