Re: [PATCH 1/9] libata: PCI device should be powered up before being accessed

[Jeff Garzik <jeff@garzik.org>]

Tejun Heo wrote:
> diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
> index 7e68edf..5ef6594 100644
> --- a/drivers/ata/pata_ali.c
> +++ b/drivers/ata/pata_ali.c
> @@ -597,6 +597,11 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	const struct ata_port_info *ppi[] = { NULL, NULL };
>  	u8 tmp;
>  	struct pci_dev *isa_bridge;
> +	int rc;
> +
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return rc;
>  
>  	/*
>  	 * The chipset revision selects the driver operations and
> @@ -632,8 +637,15 @@ static int ali_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  #ifdef CONFIG_PM
>  static int ali_reinit_one(struct pci_dev *pdev)
>  {
> +	struct ata_host *host = dev_get_drvdata(&pdev->dev);
> +	int rc;
> +
> +	rc = ata_pci_device_do_resume(pdev);
> +	if (rc)
> +		return rc;
>  	ali_init_chipset(pdev);
> -	return ata_pci_device_resume(pdev);
> +	ata_host_resume(host);
> +	return 0;
>  }
>  #endif
>  
> diff --git a/drivers/ata/pata_amd.c b/drivers/ata/pata_amd.c
> index 761a666..567fe6b 100644
> --- a/drivers/ata/pata_amd.c
> +++ b/drivers/ata/pata_amd.c
> @@ -662,10 +662,15 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	static int printed_version;
>  	int type = id->driver_data;
>  	u8 fifo;
> +	int rc;
>  
>  	if (!printed_version++)
>  		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
>  
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return rc;
> +
>  	pci_read_config_byte(pdev, 0x41, &fifo);
>  
>  	/* Check for AMD7409 without swdma errata and if found adjust type */
> @@ -709,6 +714,13 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  #ifdef CONFIG_PM
>  static int amd_reinit_one(struct pci_dev *pdev)
>  {
> +	struct ata_host *host = dev_get_drvdata(&pdev->dev);
> +	int rc;
> +
> +	rc = ata_pci_device_do_resume(pdev);
> +	if (rc)
> +		return rc;
> +
>  	if (pdev->vendor == PCI_VENDOR_ID_AMD) {
>  		u8 fifo;
>  		pci_read_config_byte(pdev, 0x41, &fifo);
> @@ -721,7 +733,9 @@ static int amd_reinit_one(struct pci_dev *pdev)
>  		    pdev->device == PCI_DEVICE_ID_AMD_COBRA_7401)
>  		    	ata_pci_clear_simplex(pdev);
>  	}
> -	return ata_pci_device_resume(pdev);
> +
> +	ata_host_resume(host);
> +	return 0;
>  }
>  #endif
>  
> diff --git a/drivers/ata/pata_artop.c b/drivers/ata/pata_artop.c
> index d421831..2f81480 100644
> --- a/drivers/ata/pata_artop.c
> +++ b/drivers/ata/pata_artop.c
> @@ -446,11 +446,16 @@ static int artop_init_one (struct pci_dev *pdev, const struct pci_device_id *id)
>  		.port_ops	= &artop6260_ops,
>  	};
>  	const struct ata_port_info *ppi[] = { NULL, NULL };
> +	int rc;
>  
>  	if (!printed_version++)
>  		dev_printk(KERN_DEBUG, &pdev->dev,
>  			   "version " DRV_VERSION "\n");
>  
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return rc;
> +
>  	if (id->driver_data == 0) {	/* 6210 variant */
>  		ppi[0] = &info_6210;
>  		ppi[1] = &ata_dummy_port_info;

While _I_ do not object, I'm curious to see what Alan thinks about 
adding enable-device to all these drivers.  Seems like the right (and 
necessary) thing to do, from my perspective...

	Jeff