Messages in this thread Patch in this message | | | Date | Mon, 14 Aug 2000 00:20:15 +0200 | From | Vojtech Pavlik <> | Subject | Re: [patch] Fix for: OOPS with ns558 and usb-ohci |
| |
On Sun, Aug 13, 2000 at 06:05:18PM -0400, Jeff Garzik wrote: > Vojtech Pavlik wrote: > > Yes, this is a genuine bug in ns558.c. It calls pci_register_driver and > > then exits if no gameport is found, without unregistering with the PCI > > layer. This is wrong. It should stay loaded, even if no cards were found > > yet, because the pci code could find them later. > > > > The attached patch fixes that in a simple way. > > ------------------------------------------------------------------------ > > > > ns558.diffName: ns558.diff > > Type: Plain Text (text/plain) > > You should probably use pci_module_init in this case. Note there > appears to be leak where the structure which is kmalloc'd in the pci > probe routine is never freed. > > Also attached is a patch which fixes a resource leak.
Ok, integrated that, used pci_module_init, the above mentioned memory leak was there, fixed too ... patch attached. Does it seem ok to you now?
-- Vojtech Pavlik SuSE Labs --- /usr/src/linux/drivers/char/joystick/ns558.c Wed Jun 21 17:22:21 2000 +++ ns558.c Mon Aug 14 00:17:57 2000 @@ -1,5 +1,5 @@ /* - * $Id: ns558.c,v 1.11 2000/06/20 23:35:03 vojtech Exp $ + * $Id: ns558.c,v 1.15 2000/08/13 22:17:57 vojtech Exp $ * * Copyright (c) 1999-2000 Vojtech Pavlik * Copyright (c) 1999 Brian Gerst @@ -182,6 +182,7 @@ if (!(port = kmalloc(sizeof(struct ns558), GFP_KERNEL))) { printk(KERN_ERR "Memory allocation failed.\n"); + release_region(ioport, iolen); return -ENOMEM; } memset(port, 0, sizeof(struct ns558)); @@ -208,6 +209,7 @@ { struct ns558 *port = (struct ns558 *)pdev->driver_data; release_region(port->gameport.io, port->gameport.size); + kfree(port); } static struct pci_driver ns558_pci_driver = { @@ -216,10 +218,16 @@ probe: ns558_pci_probe, remove: ns558_pci_remove, }; +#else +static struct pci_driver ns558_pci_driver; #endif /* CONFIG_PCI */ -#ifdef CONFIG_ISAPNP +#if defined(CONFIG_ISAPNP) || (defined(CONFIG_ISAPNP_MODULE) && defined(MODULE)) +#define NSS558_ISAPNP +#endif + +#ifdef NSS558_ISAPNP /* * PnP IDs: * @@ -297,7 +305,7 @@ int __init ns558_init(void) { int i = 0; -#ifdef CONFIG_ISAPNP +#ifdef NSS558_ISAPNP struct pci_dev *dev = NULL; struct pnp_devid *devid; #endif @@ -310,17 +318,10 @@ ns558 = ns558_isa_probe(ns558_isa_portlist[i++], ns558); /* - * Probe for PCI ports. - */ -#ifdef CONFIG_PCI - pci_register_driver(&ns558_pci_driver); -#endif - -/* * Probe for PnP ports. */ -#ifdef CONFIG_ISAPNP +#ifdef NSS558_ISAPNP for (devid = pnp_devids; devid->vendor; devid++) { while ((dev = isapnp_find_dev(NULL, devid->vendor, devid->device, dev))) { ns558 = ns558_pnp_probe(dev, ns558); @@ -328,7 +329,14 @@ } #endif - return -!ns558; +/* + * Probe for PCI ports. + */ + + if (!ns558 && pci_module_init(&ns558_pci_driver)) + return -ENODEV; + + return 0; } void __exit ns558_exit(void) @@ -339,7 +347,7 @@ gameport_unregister_port(&port->gameport); switch (port->type) { -#ifdef CONFIG_ISAPNP +#ifdef NSS558_ISAPNP case NS558_PNP: if (port->dev->deactivate) port->dev->deactivate(port->dev); @@ -357,9 +365,7 @@ port = port->next; } -#ifdef CONFIG_PCI pci_unregister_driver(&ns558_pci_driver); -#endif } module_init(ns558_init); | |