On Fri, 21 Dec 2007 16:53:40 +0000 Robert Stonehouse wrote: > This is a resubmission of a new driver for Solarflare network controllers. > > The driver supports several types of PHY (10Gbase-T, XFP, CX4) on six > different 10G and 1G boards. > > The previous thread was: > [PATCH] [RFC] New driver "sfc" for Solarstorm SFC4000 controller > http://marc.info/?l=linux-netdev&m=119757352830103&w=2 > > Thanks to the people who looked at the previous patches. We have addressed > the following from received comments after the 2nd submission: > - Use kzalloc where appropriate > - Remove deprecated fastcall attributes > - Use kernel routines for hex dumps and MAC address printing > - Changes to logging MACROs > > The last two patches were marked with RFC but I now think that this driver > is ready (withstanding any further review comments) and I would like to ask > that this driver is considered for merging. > > > The patch (against net-2.6.25) is at: > https://support.solarflare.com/netdev/3/net-2.6.25-sfc-2.2.0029.patch
wow, 750+ KB How many drivers is this? > The new files may also be downloaded as a tarball: > https://support.solarflare.com/netdev/3/net-2.6.25-sfc-2.2.0029.tgz > > And for verification there is: > https://support.solarflare.com/netdev/3/MD5SUMS Hi, I have just a few comments, mostly related to infrastructure. a. Don't use kernel-doc comment flags (i.e., "/**" to begin a comment block) when the comment is not in kernel-doc format. Or use kernel-doc format. :) (as documented in Documentation/kernel-doc-nano-HOWTO.txt) E.g.: +/** @file + * + * Efx driverlink + * + * This header file defines the portions of the Efx driverlink + * interface that are used only by the kernel net driver, and are not + * part of the public interface specification. + */ b. Kconfig file: Use 1 tab (not 8 spaces) to indent below "config" items. Use 1 tab + 2 spaces to indent help text below "help" lines. c. Driver contains MTD, SPI, & I2C (at least) code and needs to be reviewed by people in those areas as well (IMO). I see an MTD dependency in the Kconfig file. What about the SPI and I2C parts? Are they conditional or how is that handled? or does the driver not use the kernel infrastructure for these? --- ~Randy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
