Re: [PATCH] [RFC] New driver "sfc" for Solarstorm SFC4000 controller

[Robert Stonehouse <rstonehouse@solarflare.com>]

Stephen Hemminger wrote:
> On Fri, 23 Nov 2007 17:08:15 +0000
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>   
>> 1. When we enable NAPI polling, we need to set __LINK_STATE_START in
>>    the net device used for NAPI.  This bit is commented as private in
>>    netdevice.h, but e1000 also does this.  Is this incorrect?
>>     
> Why are you using it directly?
>   
It seems this line is historic and we can remove it.

> The driver is pretty big (28K loc), and non trivial to get a good review.

We are aware that it appears to be a large amount of code.

The driver does support many types of PHY (10Gbase-T, XFP, CX4) on five
different 10G reference designs and one 1G NIC ref design.

There is also support for two generations of controller silicon, full
ethtool support, start of day self-tests and an mtd driver for putting
PXE images into flash.

Perhaps we could help making it more reviewable by suggesting some sets
of files that can be reviewed together that represent different parts of
the functionality. The main functionality is contained in efx.c, rx.c,
tx.c and falcon.c if that helps.


> Minor note:
> * use u8 not uint8_t (etc.)
> * gone overboard with docbook style comments, they are only needed
>   on external API's.
> * Please use dev_err() rather than reinventing own message macros.

OK - we will look at addressing these as well as checkpatch violations
and resubmit a patch shortly.

> * why are you exporting symbol's?
>   
We need a small API so that other drivers can use parts of the hardware
after the main driver has performed initialisation.

One example is the mtd driver to access the flash. Another example is an
accelerated driver for Xen that Solarflare is in the process of
submitting.

Regards

-- 
Rob Stonehouse