Re: [PATCH] ethtool_ops rev 4

[Jeff Garzik <jgarzik@pobox.com>]

On Fri, Aug 01, 2003 at 04:02:32PM +0100, Matthew Wilcox wrote:
> and here's the diffstat
> 
>  drivers/net/8139too.c     |  330 ++++++++--------------
>  drivers/net/tg3.c         |  584 ++++++++++++++++------------------------
>  include/linux/ethtool.h   |  100 ++++++
>  include/linux/netdevice.h |    5 
>  net/core/Makefile         |    4 
>  net/core/dev.c            |   16 -
>  net/core/ethtool.c        |  671 ++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 1154 insertions(+), 556 deletions(-)

Comments:

* need SET_ETHTOOL_OPS macro or HAVE_ETHTOOL_OPS test macro or similar

* I still do not see the need to change a simple storage of a constant
  (into ethtool_gdrvinfo) into _four_ separate function call hooks (reg
  dump len, eeprom dump len, nic-specific stats len, self-test len).
  Internal kernel code that needs this information is always a slow path
  anyway, so just call the ->get_drvinfo hook internally.

* I prefer not to add '#include <linux/types.h>' to ethtool.h

Other than those, looks real good.

	Jeff