Re: [PATCH] olympic.c: fixes to olympic_scan

From: Jeff Garzik (jgarzik@mandrakesoft.com)
Date: Wed Aug 09 2000 - 12:22:14 EST


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/



This archive was generated by hypermail 2b29 : Tue Aug 15 2000 - 21:00:18 EST