Messages in this thread | | | Date | Wed, 09 Aug 2000 13:22:14 -0400 | From | Jeff Garzik <> | Subject | Re: [PATCH] olympic.c: fixes to olympic_scan |
| |
Arnaldo Carvalho de Melo wrote: > > Hi, > > Please take a look and consider applying. > > - Arnaldo > > --- linux-2.4.0-test6-pre9/drivers/net/tokenring/olympic.c Fri Jul 28 06:34:46 2000 > +++ linux-2.4.0-test6-pre9.acme/drivers/net/tokenring/olympic.c Wed Aug 9 13:32:41 2000
Comments:
* (bug) Your failure cleanup is incomplete: iounmap is not called to reverse the effects of the ioremap * Testing pci_present is useless when it is immediately followed by a call to pci_find_device. Since pci_find_device produces correct results even when PCI isn't present, there is no need to call pci_present at all(). * The "paragraph" of code right below the call to pci_enable_device can be replaced with a simple call to pci_set_master. pci_enable_device+pci_set_master enable all the PCI_COMMAND bits needed for operation. * would be nice, but not necessary, if check-region was eliminated and request-region was used instead. * replace references to pci_device->resource[x].start with pci_resource_start(pci_device, x) * use the new PCI driver API to clean this sucker up. boy, I'll bet this driver would lose a hundred lines of code if you did that. The module init code is really hairy and has more code paths/cases than absolutely necessary.
Let me know if you have any questions, I'll be more than happy to answer them. Thanks for the cleanup work... you are doing the Linux community favor, while also helping to eliminate some of the items on my kernel todo list :):)
Jeff
-- Jeff Garzik | Building 1024 | Andre the Giant has a posse. MandrakeSoft, Inc. |
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |