Re: [PATCH] ioc3-eth.c: add missing pci_enable_device()

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Tuesday 24 August 2004 6:40 pm, Jeff Garzik wrote:
> Linux Kernel Mailing List wrote:
> > ChangeSet 1.1843.1.74, 2004/08/24 11:21:53-07:00, bjorn.helgaas@hp.com
> > 
> > 	[PATCH] ioc3-eth.c: add missing pci_enable_device()
> > 	
> > 	Add pci_enable_device()/pci_disable_device().  In the past, drivers often
> > 	worked without this, but it is now required in order to route PCI interrupts
> > 	correctly.
> > 	
> > 	Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > 	Signed-off-by: Andrew Morton <akpm@osdl.org>
> > 	Signed-off-by: Linus Torvalds <torvalds@osdl.org>

> I don't see a "signed-off-by" line from Ralf.  I noticed you never 
> bothered to send this patch to me.  Did you send it to Ralf either?

As a matter of fact, I did send it to Ralf, based on this:

	IOC3 DRIVER
	P:      Ralf Baechle
	M:      ralf@linux-mips.org
	L:      linux-mips@linux-mips.org
	S:      Maintained

Once I found the specific IOC3 entry, I neglected to search farther
and find the more generic "NETWORK DEVICE DRIVERS" entry, so I didn't
send it to you, Jeff; sorry about that.

> ioc3 is _very_ strange device and not fully compliant to the PCI spec.

OK, I don't know anything about ioc3, other than the fact that it
appeared to use pci_dev->irq without doing pci_enable_device().
All ACPI-based PCI interrupt routing is now done in pci_enable_device()
(in -mm, not yet in mainline), so if ioc3 were used in an ACPI-based
system, it would likely be broken.

I'm fine with reverting the change.  Here's a patch to do that:


Revert addition of pci_enable_device().

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

===== drivers/net/ioc3-eth.c 1.27 vs edited =====
--- 1.27/drivers/net/ioc3-eth.c	2004-08-24 03:08:34 -06:00
+++ edited/drivers/net/ioc3-eth.c	2004-08-25 08:56:24 -06:00
@@ -1172,14 +1172,9 @@
 	u32 vendor, model, rev;
 	int err;
 
-	if (pci_enable_device(pdev))
-		return -ENODEV;
-
 	dev = alloc_etherdev(sizeof(struct ioc3_private));
-	if (!dev) {
-		err = -ENOMEM;
-		goto out_disable;
-	}
+	if (!dev)
+		return -ENOMEM;
 
 	err = pci_request_regions(pdev, "ioc3");
 	if (err)
@@ -1274,8 +1269,6 @@
 	pci_release_regions(pdev);
 out_free:
 	free_netdev(dev);
-out_disable:
-	pci_disable_device(pdev);
 	return err;
 }
 
@@ -1289,7 +1282,6 @@
 	iounmap(ioc3);
 	pci_release_regions(pdev);
 	free_netdev(dev);
-	pci_disable_device(pdev);
 }
 
 static struct pci_device_id ioc3_pci_tbl[] = {