Skip to contentSkip to conversation

New driver "sfc" for Solarstorm SFC4000 controller - 3nd try

PATCH In review Robert Stonehouse 3 participants 3 messages Dec 21, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 New driver "sfc" for Solarstorm SFC4000 controller - 3nd try

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

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

Happy Christmas

Signature · 1 line
Robert Stonehouse
#1 lore raw
Quoted 16 lines · 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

Quoted 7 lines

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?

Quoted 5 lines

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

#2 lore raw
Quoted 3 lines · Randy Dunlap
Quoted 1 line · On Fri, 21 Dec 2007 16:53:40 +0000 Robert Stonehouse wrote:

This is a resubmission of a new driver for Solarflare network controllers.

<snip>

Quoted 11 lines
Quoted 7 lines

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?

Just two: sfc (net) and sfc_mtd (MTD). As Robert said, the net driver supports a variety of PHYs. It also has a fair amount of self-test and (conditional) debug code, and comments.

I've noted your comments on kernel-doc and Kconfig format and will address those in the next version.

<snip>

Quoted 7 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?

We are not currently using the kernel infrastructure for those. I'm not sure whether we could do.

I think everyone working on the net driver here will be away until the new year, so please forgive our silence in the mean time.

Ben.

Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.