[PATCH] bcm43xx: remove ethtool

[PATCH] bcm43xx: remove ethtool

[Larry Finger]

Ethtool is useless for bcm43xx - remove it.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---


Index: wireless-2.6/drivers/net/wireless/bcm43xx/Makefile
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/Makefile
+++ wireless-2.6/drivers/net/wireless/bcm43xx/Makefile
@@ -7,6 +7,6 @@ bcm43xx-obj-$(CONFIG_BCM43XX_PIO) += bcm
 bcm43xx-objs := bcm43xx_main.o bcm43xx_ilt.o \
 		bcm43xx_radio.o bcm43xx_phy.o \
 		bcm43xx_power.o bcm43xx_wx.o \
-		bcm43xx_leds.o bcm43xx_ethtool.o \
+		bcm43xx_leds.o  \
 		bcm43xx_xmit.o bcm43xx_sysfs.o \
 		$(bcm43xx-obj-y)
Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_ethtool.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_ethtool.c
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
-
-  Broadcom BCM43xx wireless driver
-
-  ethtool support
-
-  Copyright (c) 2006 Jason Lunz <lunz@falooley.org>
-
-  Some code in this file is derived from the 8139too.c driver
-  Copyright (C) 2002 Jeff Garzik
-
-  This program is free software; you can redistribute it and/or modify
-  it under the terms of the GNU General Public License as published by
-  the Free Software Foundation; either version 2 of the License, or
-  (at your option) any later version.
-
-  This program is distributed in the hope that it will be useful,
-  but WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-  GNU General Public License for more details.
-
-  You should have received a copy of the GNU General Public License
-  along with this program; see the file COPYING.  If not, write to
-  the Free Software Foundation, Inc., 51 Franklin Steet, Fifth Floor,
-  Boston, MA 02110-1301, USA.
-
-*/
-
-#include "bcm43xx.h"
-#include "bcm43xx_ethtool.h"
-
-#include <linux/netdevice.h>
-#include <linux/pci.h>
-#include <linux/string.h>
-#include <linux/utsrelease.h>
-
-
-static void bcm43xx_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
-{
-	struct bcm43xx_private *bcm = bcm43xx_priv(dev);
-
-	strncpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
-	strncpy(info->version, UTS_RELEASE, sizeof(info->version));
-	strncpy(info->bus_info, pci_name(bcm->pci_dev), ETHTOOL_BUSINFO_LEN);
-}
-
-const struct ethtool_ops bcm43xx_ethtool_ops = {
-	.get_drvinfo = bcm43xx_get_drvinfo,
-	.get_link = ethtool_op_get_link,
-};
Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_ethtool.h
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_ethtool.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef BCM43xx_ETHTOOL_H_
-#define BCM43xx_ETHTOOL_H_
-
-#include <linux/ethtool.h>
-
-extern const struct ethtool_ops bcm43xx_ethtool_ops;
-
-#endif /* BCM43xx_ETHTOOL_H_ */
Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -50,7 +50,6 @@
 #include "bcm43xx_pio.h"
 #include "bcm43xx_power.h"
 #include "bcm43xx_wx.h"
-#include "bcm43xx_ethtool.h"
 #include "bcm43xx_xmit.h"
 #include "bcm43xx_sysfs.h"
 
@@ -4155,7 +4154,6 @@ static int __devinit bcm43xx_init_one(st
 #endif
 	net_dev->wireless_handlers = &bcm43xx_wx_handlers_def;
 	net_dev->irq = pdev->irq;
-	SET_ETHTOOL_OPS(net_dev, &bcm43xx_ethtool_ops);
 
 	/* initialize the bcm43xx_private struct */
 	bcm = bcm43xx_priv(net_dev);

Re: [PATCH] bcm43xx: remove ethtool

[Jeff Garzik]

Larry Finger wrote:
> Ethtool is useless for bcm43xx - remove it.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>

How is GDRVINFO useless?  Where does mac80211 provide equivalent 
information?

	Jeff

Re: [PATCH] bcm43xx: remove ethtool

[Larry Finger]

Jeff Garzik wrote:
> Larry Finger wrote:
>> Ethtool is useless for bcm43xx - remove it.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> 
> How is GDRVINFO useless?  Where does mac80211 provide equivalent
> information?

I cannot speak for mac80211, but the current implementation for SoftMAC only implements the GDRVINFO
call. From it, you get that the driver is bcm43xx (big surprise), the kernel version (easier gotten
by a uname -r), and the bus info. None of the operational parameters can be changed or interrogated.

Is this enough to keep the ethtool interface? Because of the EOL for SoftMAC, this code will never
be enhanced or extended. Does some userland code need this info?

Larry

Re: [PATCH] bcm43xx: remove ethtool

[Jeff Garzik]

Larry Finger wrote:
> Jeff Garzik wrote:
>> Larry Finger wrote:
>>> Ethtool is useless for bcm43xx - remove it.
>>>
>>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> How is GDRVINFO useless?  Where does mac80211 provide equivalent
>> information?
> 
> I cannot speak for mac80211, but the current implementation for SoftMAC only implements the GDRVINFO
> call. From it, you get that the driver is bcm43xx (big surprise), the kernel version (easier gotten
> by a uname -r), and the bus info. None of the operational parameters can be changed or interrogated.
> 
> Is this enough to keep the ethtool interface? Because of the EOL for SoftMAC, this code will never
> be enhanced or extended. Does some userland code need this info?

It's a highly standardized interface that provides information that's 
either impossible or highly difficult to obtain elsewhere.

If you are a userland process querying a network interface, that's the 
only way to know which driver is attached, or the only way to build an 
association between a PCI device and a network interface.

So NAK this change.  All network drivers should implement GDRVINFO, even 
if they are not strictly ethernet drivers.

	Jeff

Re: [PATCH] bcm43xx: remove ethtool

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Saturday 03 March 2007 22:58, Jeff Garzik wrote:
> It's a highly standardized interface that provides information that's
> either impossible or highly difficult to obtain elsewhere.
>
> If you are a userland process querying a network interface, that's the
> only way to know which driver is attached, or the only way to build an
> association between a PCI device and a network interface.
>
FWIW, on my system:
/sys/class/net/eth1/device -> /sys/devices/pci0000:00/0000:00:10.4/usb1/1-7/1-7:1.0
/sys/class/net/eth1/device/bus -> /sys/bus/usb
/sys/class/net/eth1/device/driver -> /sys/bus/usb/drivers/zd1211rw_mac80211

And the other way around:
/sys/devices/pci0000:00/0000:00:10.4/usb1/1-7/1-7:1.0/net:eth1
/sys/devices/pci0000:00/0000:00:10.4/usb1/1-7/1-7:1.0/net:wmaster0

Is this considered highly difficult or not standardized enough?

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] bcm43xx: remove ethtool

[Pavel Roskin]

Quoting Jeff Garzik <jeff@garzik.org>:

> It's a highly standardized interface that provides information that's
> either impossible or highly difficult to obtain elsewhere.

For what it's worth, ifrename in the forthcoming Wireless Tools 29 will support
reading symlinks on sysfs, which would allow matching the device and  the
driver name.  Other userspace tools can do the same.

$ readlink /sys/class/net/eth0/device
../../../devices/pci0000:00/0000:00:0d.0/0000:01:02.0
$ readlink /sys/class/net/eth0/device/driver
../../../../bus/pci/drivers/orinoco_plx

That's 2.6.20, things may be slightly different on 2.6.21+

> If you are a userland process querying a network interface, that's the
> only way to know which driver is attached, or the only way to build an
> association between a PCI device and a network interface.
>
> So NAK this change.  All network drivers should implement GDRVINFO, even
> if they are not strictly ethernet drivers.

I would say it's not yet time to remove it.  But if a driver doesn't implement
it (that's the case for bcm43xx_d80211), I don't think we should bother
implementing it.

--
Regards,
Pavel Roskin

Re: [PATCH] bcm43xx: remove ethtool

[Michael Buesch]

On Sunday 04 March 2007 04:58, Jeff Garzik wrote:
> Larry Finger wrote:
> > Jeff Garzik wrote:
> >> Larry Finger wrote:
> >>> Ethtool is useless for bcm43xx - remove it.
> >>>
> >>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >> How is GDRVINFO useless?  Where does mac80211 provide equivalent
> >> information?
> > 
> > I cannot speak for mac80211, but the current implementation for SoftMAC only implements the GDRVINFO
> > call. From it, you get that the driver is bcm43xx (big surprise), the kernel version (easier gotten
> > by a uname -r), and the bus info. None of the operational parameters can be changed or interrogated.
> > 
> > Is this enough to keep the ethtool interface? Because of the EOL for SoftMAC, this code will never
> > be enhanced or extended. Does some userland code need this info?
> 
> It's a highly standardized interface that provides information that's 
> either impossible or highly difficult to obtain elsewhere.
> 
> If you are a userland process querying a network interface, that's the 
> only way to know which driver is attached, or the only way to build an 
> association between a PCI device and a network interface.

sysfs provides all this information.

-- 
Greetings Michael.