2007-03-04 01:01:19

by Larry Finger

[permalink] [raw]
Subject: [PATCH] bcm43xx: remove ethtool

Ethtool is useless for bcm43xx - remove it.

Signed-off-by: Larry Finger <[email protected]>
---


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 <[email protected]>
-
- 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);


2007-03-04 05:28:07

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

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


Attachments:
(No filename) (857.00 B)
(No filename) (189.00 B)
Download all attachments

2007-03-04 02:44:13

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

Larry Finger wrote:
> Ethtool is useless for bcm43xx - remove it.
>
> Signed-off-by: Larry Finger <[email protected]>

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

Jeff




2007-03-04 05:10:03

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

Quoting Jeff Garzik <[email protected]>:

> 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

2007-03-04 03:29:50

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

Jeff Garzik wrote:
> Larry Finger wrote:
>> Ethtool is useless for bcm43xx - remove it.
>>
>> Signed-off-by: Larry Finger <[email protected]>
>
> 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

2007-03-04 03:58:44

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

Larry Finger wrote:
> Jeff Garzik wrote:
>> Larry Finger wrote:
>>> Ethtool is useless for bcm43xx - remove it.
>>>
>>> Signed-off-by: Larry Finger <[email protected]>
>> 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




2007-03-04 13:00:17

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] bcm43xx: remove ethtool

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 <[email protected]>
> >> 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.