comments:
becker has new stuff posted at http://www.scyld.com/ check out the new
code there, and also update links in your docs/code...
Just increase the version to LK1.1.3, no need for AKPM added on to
version. Or if you are gonna actively maintain the driver like I do
with 8139too/tulip, ie. where the driver diverges from donald's quite a
bit, then you might as well use your own version scheme.
Do you even need #warning near the top, why not just the #error line?
bug: you reverted a change which moved 'version' to the file of the
file, removing '__initdata' marker in the process. (this needs to be
below linux/init.h...) and it should be __devinitdata not __initdata.
can vortex_info_tbl be marked __devinitdata? (make sure to check, often
it cannot be marked __*initdata due to usage after xxx_init_one has
ended)
any feedback on whether or not EISA works? (not a showstopped of
course...)
return error value from pci_enable_device
suggestion: using __setup() create a setup function which allows people
to pass options into the driver using "3c59x=....", if using the third
option of ether= bugs you :)
can you make ram_split[] const as well?
bug: when freeing a netdev, you should call unregister_netdevice(dev)
then kfree(dev). vortex_probe1's 'free_dev' label doesn't call
unregister_netdevice(dev). Make sure vortex_remove_one also calls
unregister_netdevice.
bug: there is at least one place in the diff which reverts Dave
Miller's PCI DMA changes. Basically grep for 'virt_to_bus' and
similar. These should NEVER occur in the source code, you should be
referencing the handles returned from pci_alloc_xxx: rx_ring_dma, etc.
example bad change:
- outl(vp->rx_ring_dma, ioaddr + UpListPtr);
+ outl(virt_to_bus(&vp->rx_ring[0]), ioaddr + UpListPtr);
it might be better to use a semaphore or something other than a spinlock
to protect ioctl and get-stats.
what is the purpose of vp->open? it seems redundant to netif_xxx bits,
and possibly racy
xxx_start_xmit should not call netif_stop_queue at the beginning of the
routine, but instead should probably look like this:
give packet to hardware
if (no more TX room)
netif_stop_queue ()
i wouldn't include wake-on-lan stuff in the code unless you've tested
it...
in vortex_remove_one, if dev==NULL you should probably call BUG()
i would ditch the comment in drivers/net/pcmcia/Config.in saying
"3c575_cb has been moved"
-- Jeff Garzik | Nothing cures insomnia like the Building 1024 | realization that it's time to get up. MandrakeSoft, Inc. | -- random fortune- 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 : Sun Apr 30 2000 - 21:00:08 EST