Thanks, applied to 2.4 and 2.5. I removed the definition of
PCI_DEVICE_xxx from the top of 3c359.h...
Comments:
1) buggy use of PCI DMA API -- you should use memory returned from
pci_alloc_consistent, do not directly map memory created by
alloc_trdev() nor depend on the alignment returned by alloc_trdev()
2) support for ETHTOOL_GDRVINFO ioctl
3) support for ETHTOOL_[GS]MSGLVL ioctls... this implies that
'message_level' would only serve as a default for a value that can be
changed per-interface
4) ideally "\n" should not be in MODULE_DESCRIPTION
5) style: no need to cast to/from a void pointer, such as
> struct xl_private *xl_priv = (struct xl_private *)dev->priv ;
6) hardcoded magic numbers for PKT_BUF_SZ limits (100 and 18000)
7) xl_probe duplicates error handling code... use the standard kernel
style of multiple goto targets, one for each needed stage of
cleanup-after-error:
err_out3:
pci_release_regions(pdev)
err_out2:
kfree(dev)
err_out:
return rc;
8) in xl_hw_reset, you probably want to call yield [in 2.5] or
schedule_timeout
9) jiffies comparison bug: never directly compare jiffies, use
time_before() or time_after()
10) in xl_open, return the error value returned by request_irq, on error
11) same comment for xl_open as #7
12) xl_wait_misr_flags needs to call yield() or -something-, don't just
empty loop. cpu_relax(), for example, if you cannot schedule...
Overall... good job, it's a readable, clean driver.
Jeff
--
Jeff Garzik | "Why is it that attractive girls like you
Building 1024 | always seem to have a boyfriend?"
MandrakeSoft | "Because I'm a nympho that owns a brewery?"
| - BBC TV show "Coupling"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Feb 23 2002 - 21:00:25 EST