netdev
[Top] [All Lists]

Re: [PATCH] ethtool_ops rev 4

To: Matthew Wilcox <willy@xxxxxxxxxx>
Subject: Re: [PATCH] ethtool_ops rev 4
From: Jeff Garzik <jgarzik@xxxxxxxxx>
Date: Fri, 1 Aug 2003 11:40:21 -0400
Cc: netdev@xxxxxxxxxxx
In-reply-to: <20030801150232.GV22222@parcelfarce.linux.theplanet.co.uk>
References: <20030801150232.GV22222@parcelfarce.linux.theplanet.co.uk>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
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




<Prev in Thread] Current Thread [Next in Thread>