Re: [PATCH 8/21] advansys: Rewrite resource management

[Jeff Garzik <jeff@garzik.org>]

Matthew Wilcox wrote:
> @@ -18787,8 +18765,10 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	int ioport;
>  	struct Scsi_Host *shost;
>  
> -	if (pci_enable_device(pdev))
> +	if (pci_request_regions(pdev, "advansys"))
>  		goto fail;
> +	if (pci_enable_device(pdev))
> +		goto release_regions;
>  
>  	ioport = pci_resource_start(pdev, 0);
>  	shost = advansys_board_found(ioport, &pdev->dev, ASC_IS_PCI);
> @@ -18799,6 +18779,8 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	}
>  
>  	pci_disable_device(pdev);
> + release_regions:
> +	pci_release_regions(pdev);
>   fail:
>  	return -ENODEV;
>  }
> @@ -18807,6 +18789,7 @@ static void __devexit advansys_pci_remove(struct pci_dev *pdev)
>  {
>  	advansys_remove(pci_get_drvdata(pdev));
>  	pci_disable_device(pdev);
> +	pci_release_regions(pdev);

Your ordering here is completely backwards.

You do not have resources to reserve, until you have enabled the device.

pci_enable_device() ALWAYS comes first, and pci_disable_device() always 
comes next-to-last (pci_set_drvdata(pdev, NULL) is last).

	Jeff