Re: [ide] clean up error path in do_ide_setup_pci_device()

[Alan Cox]

This changeset will break support for several systems because the PCI
IDE controller uses some BARs on a multifunction PCI northbridge. The
old IDE code was extremely careful *NOT* to play pci_disable_device
games because of this.

Nothing in the IDE specification requires the PCI IDE controller be the
only use of that PCI function. The damage is probably minimal as it
deals with error paths but this change should be reverted (and will be
for -ac).


On Iau, 2004-12-30 at 19:08, Linux Kernel Mailing List wrote:
> ChangeSet 1.2034.118.8, 2004/12/30 20:08:53+01:00, bzolnier@trik.(none)
> 
> 	[ide] clean up error path in do_ide_setup_pci_device()
> 	
> 	ide_setup_pci_controller() puts the device in a PCI enabled state.
> 	The patch adds a small helper to balance it when things go wrong.
> 	
> 	Actually the helper does not *exactly* balance the setup: if it can
> 	not do a better job, ide_setup_pci_controller() may only enable some
> 	BARS whereas the new counterpart will try to disable everything.
> 	
> 	Signed-off-by: Francois Romieu <romieu@xxxxxxxxxxxxx>
> 	Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> 
> 
> 
>  setup-pci.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> 
> diff -Nru a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> --- a/drivers/ide/setup-pci.c	2004-12-30 19:44:05 -08:00
> +++ b/drivers/ide/setup-pci.c	2004-12-30 19:44:05 -08:00
> @@ -542,6 +542,13 @@
>  	return 0;
>  }
>  
> +static void ide_release_pci_controller(struct pci_dev *dev, ide_pci_device_t *d,
> +				       int noisy)
> +{
> +	/* Balance ide_pci_enable() */
> +	pci_disable_device(dev);
> +}
> +
>  /**
>   *	ide_pci_setup_ports	-	configure ports/devices on PCI IDE
>   *	@dev: PCI device
> @@ -672,7 +679,7 @@
>  		 */
>  		ret = d->init_chipset ? d->init_chipset(dev, d->name) : 0;
>  		if (ret < 0)
> -			goto out;
> +			goto err_release_pci_controller;
>  		pciirq = ret;
>  	} else if (tried_config) {
>  		if (noisy)
> @@ -687,7 +694,7 @@
>  		if (d->init_chipset) {
>  			ret = d->init_chipset(dev, d->name);
>  			if (ret < 0)
> -				goto out;
> +				goto err_release_pci_controller;
>  		}
>  		if (noisy)
>  #ifdef __sparc__
> @@ -705,6 +712,10 @@
>  	ide_pci_setup_ports(dev, d, pciirq, index);
>  out:
>  	return ret;
> +
> +err_release_pci_controller:
> +	ide_release_pci_controller(dev, d, noisy);
> +	goto out;
>  }
>  
>  int ide_setup_pci_device(struct pci_dev *dev, ide_pci_device_t *d)
> -
> To unsubscribe from this list: send the line "unsubscribe bk-commits-head" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/