Re: [PATCH 0/2] cfg80211: firmware and hardware version

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[John W. Linville]

On Fri, Sep 25, 2009 at 09:53:35AM -0700, Luis R. Rodriguez wrote:
> On Fri, Sep 25, 2009 at 9:47 AM, Kalle Valo <kalle.valo@iki.fi> wrote:

> > But we want to export two strings to user space (at least for now), is
> > it really worth the effort to add ethtool support for such a minor
> > feature? Also I have understood that ethtool is implemented only for
> > ethernet drivers, I don't feel comfortable that we use ethernet driver
> > interfaces with 802.11 device drivers. They are so much different that
> > there isn't that much common functionality. That's why I prefer
> > nl80211 over ethtool.
> >
> > What do people think?
> 
> So for Wake-on-Wireless I ran into the same, ethtool just did not
> offer the same wake up events needed for wireless. I could have
> technically used ethtool and expanded it to support wireless but it
> just seemed dirty.
> 
> I agree that using ethtool seems overkill compared to the patches you posted.

I think you either overestimate the amount of trouble for implementing
(minimal) ethtool support or you underestimate the amount of
functionality available through that interface.  That, or you just
don't like using something named "eth"tool for wireless -- but hey,
let's be honest about the frames we send/receive to/from the kernel...
:-)

The ethtool interface provides functionality for viewing and modifying
eeprom contents, dumping registers, trigger self-tests, basic driver
info, getting and setting message reporting levels, external card
identification (hey, _could_ be useful!), and some other bits like
checksum offload that might(?) be useful in the future.  I understand
regarding the WoW vs. WoL issue but probably the answer is just to
add a new method for WoW...?

I'll post a patch series based on what Kalle sent but using ethtool
instead of nl80211...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[PATCH 1/3] wireless: implement basic ethtool support for cfg80211 devices

[John W. Linville]

Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
---
 net/wireless/Makefile  |    2 +-
 net/wireless/core.c    |    3 +++
 net/wireless/ethtool.c |   27 +++++++++++++++++++++++++++
 net/wireless/ethtool.h |   10 ++++++++++
 4 files changed, 41 insertions(+), 1 deletions(-)
 create mode 100644 net/wireless/ethtool.c
 create mode 100644 net/wireless/ethtool.h

diff --git a/net/wireless/Makefile b/net/wireless/Makefile
index c814150..f07c8dc 100644
--- a/net/wireless/Makefile
+++ b/net/wireless/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_WEXT_SPY) += wext-spy.o
 obj-$(CONFIG_WEXT_PRIV) += wext-priv.o
 
 cfg80211-y += core.o sysfs.o radiotap.o util.o reg.o scan.o nl80211.o
-cfg80211-y += mlme.o ibss.o sme.o chan.o
+cfg80211-y += mlme.o ibss.o sme.o chan.o ethtool.o
 cfg80211-$(CONFIG_CFG80211_DEBUGFS) += debugfs.o
 cfg80211-$(CONFIG_CFG80211_WEXT) += wext-compat.o wext-sme.o
 
diff --git a/net/wireless/core.c b/net/wireless/core.c
index c761532..faada5c 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -21,6 +21,7 @@
 #include "sysfs.h"
 #include "debugfs.h"
 #include "wext-compat.h"
+#include "ethtool.h"
 
 /* name for sysfs, %d is appended */
 #define PHY_NAME "phy"
@@ -683,6 +684,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
 				wdev->wext.ps = false;
 			}
 #endif
+		if (!dev->ethtool_ops)
+			dev->ethtool_ops = &cfg80211_ethtool_ops;
 		break;
 	case NETDEV_GOING_DOWN:
 		switch (wdev->iftype) {
diff --git a/net/wireless/ethtool.c b/net/wireless/ethtool.c
new file mode 100644
index 0000000..94ca377
--- /dev/null
+++ b/net/wireless/ethtool.c
@@ -0,0 +1,27 @@
+#include <linux/utsrelease.h>
+#include <net/cfg80211.h>
+#include "ethtool.h"
+
+const struct ethtool_ops cfg80211_ethtool_ops = {
+	.get_drvinfo = cfg80211_get_drvinfo,
+	.get_link = ethtool_op_get_link,
+};
+
+void cfg80211_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+
+	strncpy(info->driver, wiphy_dev(wdev->wiphy)->driver->name,
+		sizeof(info->driver));
+	info->driver[sizeof(info->driver) - 1] = '\0';
+
+	strncpy(info->version, UTS_RELEASE, sizeof(info->version));
+	info->version[sizeof(info->version) - 1] = '\0';
+
+	strncpy(info->fw_version, "N/A", sizeof(info->fw_version));
+	info->fw_version[sizeof(info->fw_version) - 1] = '\0';
+
+	strncpy(info->bus_info, dev_name(wiphy_dev(wdev->wiphy)),
+		sizeof(info->bus_info));
+	info->bus_info[sizeof(info->bus_info) - 1] = '\0';
+}
diff --git a/net/wireless/ethtool.h b/net/wireless/ethtool.h
new file mode 100644
index 0000000..a51b470
--- /dev/null
+++ b/net/wireless/ethtool.h
@@ -0,0 +1,10 @@
+#ifndef __CFG80211_ETHTOOL__
+#define __CFG80211_ETHTOOL__
+
+#include <linux/ethtool.h>
+
+extern void cfg80211_get_drvinfo(struct net_device *, struct ethtool_drvinfo *);
+
+extern const struct ethtool_ops cfg80211_ethtool_ops;
+
+#endif /* __CFG80211_ETHTOOL__ */
-- 
1.6.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] cfg80211: add firmware and hardware version to wiphy

[John W. Linville]

From: Kalle Valo <kalle.valo@nokia.com>

It's useful to provide firmware and hardware version to user space and have a
generic interface to retrieve them. Users can provide the version information
in bug reports etc.

Add fields for firmware and hardware version to struct wiphy.

(Dropped nl80211 bits for now and modified remaining bits in favor of
ethtool. -- JWL)

Cc: Kalle Valo <kalle.valo@nokia.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 include/net/cfg80211.h |    3 +++
 net/wireless/ethtool.c |   23 ++++++++++++++++++++++-
 net/wireless/ethtool.h |    3 +++
 3 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 241ea14..6f4862b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1142,6 +1142,9 @@ struct wiphy {
 	u32 frag_threshold;
 	u32 rts_threshold;
 
+	char fw_version[ETHTOOL_BUSINFO_LEN];
+	u32 hw_version;
+
 	/* If multiple wiphys are registered and you're handed e.g.
 	 * a regular netdev with assigned ieee80211_ptr, you won't
 	 * know whether it points to a wiphy your driver has registered
diff --git a/net/wireless/ethtool.c b/net/wireless/ethtool.c
index 94ca377..3c59549 100644
--- a/net/wireless/ethtool.c
+++ b/net/wireless/ethtool.c
@@ -4,6 +4,8 @@
 
 const struct ethtool_ops cfg80211_ethtool_ops = {
 	.get_drvinfo = cfg80211_get_drvinfo,
+	.get_regs_len = cfg80211_get_regs_len,
+	.get_regs = cfg80211_get_regs,
 	.get_link = ethtool_op_get_link,
 };
 
@@ -18,10 +20,29 @@ void cfg80211_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 	strncpy(info->version, UTS_RELEASE, sizeof(info->version));
 	info->version[sizeof(info->version) - 1] = '\0';
 
-	strncpy(info->fw_version, "N/A", sizeof(info->fw_version));
+	if (wdev->wiphy->fw_version[0])
+		strncpy(info->fw_version, wdev->wiphy->fw_version,
+			sizeof(info->fw_version));
+	else
+		strncpy(info->fw_version, "N/A", sizeof(info->fw_version));
 	info->fw_version[sizeof(info->fw_version) - 1] = '\0';
 
 	strncpy(info->bus_info, dev_name(wiphy_dev(wdev->wiphy)),
 		sizeof(info->bus_info));
 	info->bus_info[sizeof(info->bus_info) - 1] = '\0';
 }
+
+int cfg80211_get_regs_len(struct net_device *dev)
+{
+	/* For now, return 0... */
+	return 0;
+}
+
+void cfg80211_get_regs(struct net_device *dev, struct ethtool_regs *regs,
+			void *data)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+
+	regs->version = wdev->wiphy->hw_version;
+	regs->len = 0;
+}
diff --git a/net/wireless/ethtool.h b/net/wireless/ethtool.h
index a51b470..2d4602a 100644
--- a/net/wireless/ethtool.h
+++ b/net/wireless/ethtool.h
@@ -4,6 +4,9 @@
 #include <linux/ethtool.h>
 
 extern void cfg80211_get_drvinfo(struct net_device *, struct ethtool_drvinfo *);
+extern int cfg80211_get_regs_len(struct net_device *);
+extern void cfg80211_get_regs(struct net_device *, struct ethtool_regs *,
+				void *);
 
 extern const struct ethtool_ops cfg80211_ethtool_ops;
 
-- 
1.6.2.5

[PATCH 3/3] at76c50x-usb: set firmware and hardware version in wiphy

[John W. Linville]

From: Kalle Valo <kalle.valo-X3B1VOXEql0@public.gmane.org>

Set firmware and hardware version in wiphy so that user space can access
it.

(Modification from original in favor of cfg80211 ethtool support. -- JWL)

Cc: Kalle Valo <kalle.valo-X3B1VOXEql0@public.gmane.org>
Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
---
 drivers/net/wireless/at76c50x-usb.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/at76c50x-usb.c b/drivers/net/wireless/at76c50x-usb.c
index 8e1a55d..533954d 100644
--- a/drivers/net/wireless/at76c50x-usb.c
+++ b/drivers/net/wireless/at76c50x-usb.c
@@ -2217,6 +2217,8 @@ static struct ieee80211_supported_band at76_supported_band = {
 static int at76_init_new_device(struct at76_priv *priv,
 				struct usb_interface *interface)
 {
+	struct wiphy *wiphy;
+	size_t len;
 	int ret;
 
 	/* set up the endpoint information */
@@ -2254,6 +2256,7 @@ static int at76_init_new_device(struct at76_priv *priv,
 	priv->device_unplugged = 0;
 
 	/* mac80211 initialisation */
+	wiphy = priv->hw->wiphy;
 	priv->hw->wiphy->max_scan_ssids = 1;
 	priv->hw->wiphy->max_scan_ie_len = 0;
 	priv->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
@@ -2265,6 +2268,15 @@ static int at76_init_new_device(struct at76_priv *priv,
 	SET_IEEE80211_DEV(priv->hw, &interface->dev);
 	SET_IEEE80211_PERM_ADDR(priv->hw, priv->mac_addr);
 
+	len = sizeof(wiphy->fw_version);
+	snprintf(wiphy->fw_version, len, "%d.%d.%d-%d",
+		 priv->fw_version.major, priv->fw_version.minor,
+		 priv->fw_version.patch, priv->fw_version.build);
+	/* null terminate the strings in case they were truncated */
+	wiphy->fw_version[len - 1] = '\0';
+
+	wiphy->hw_version = priv->board_type;
+
 	ret = ieee80211_register_hw(priv->hw);
 	if (ret) {
 		printk(KERN_ERR "cannot register mac80211 hw (status %d)!\n",
-- 
1.6.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] at76c50x-usb: set firmware and hardware version in wiphy

[Ben Hutchings]

On Wed, 2009-09-30 at 21:19 -0400, John W. Linville wrote:
[...]
> +	len = sizeof(wiphy->fw_version);
> +	snprintf(wiphy->fw_version, len, "%d.%d.%d-%d",
> +		 priv->fw_version.major, priv->fw_version.minor,
> +		 priv->fw_version.patch, priv->fw_version.build);
> +	/* null terminate the strings in case they were truncated */
> +	wiphy->fw_version[len - 1] = '\0';
[...]

This last statement is unnecessary; snprintf() always null-terminates
(unless the length is zero).

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 3/3] at76c50x-usb: set firmware and hardware version in wiphy

[Kalle Valo]

Ben Hutchings <bhutchings@solarflare.com> writes:

> On Wed, 2009-09-30 at 21:19 -0400, John W. Linville wrote:
> [...]
>> +	len = sizeof(wiphy->fw_version);
>> +	snprintf(wiphy->fw_version, len, "%d.%d.%d-%d",
>> +		 priv->fw_version.major, priv->fw_version.minor,
>> +		 priv->fw_version.patch, priv->fw_version.build);
>> +	/* null terminate the strings in case they were truncated */
>> +	wiphy->fw_version[len - 1] = '\0';
> [...]
>
> This last statement is unnecessary; snprintf() always null-terminates
> (unless the length is zero).

Yes, the extra null termination is unnecessary. This was my mistake in
the first patchset I sent.

-- 
Kalle Valo

Re: [PATCH 1/3] wireless: implement basic ethtool support for cfg80211 devices

[Ben Hutchings]

On Wed, 2009-09-30 at 21:19 -0400, John W. Linville wrote:
[...]
> +void cfg80211_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
> +{
> +	struct wireless_dev *wdev = dev->ieee80211_ptr;
> +
> +	strncpy(info->driver, wiphy_dev(wdev->wiphy)->driver->name,
> +		sizeof(info->driver));
> +	info->driver[sizeof(info->driver) - 1] = '\0';
[...]

Use strlcpy() instead of these two statements.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 1/3] wireless: implement basic ethtool support for cfg80211 devices

[Johannes Berg]

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

On Wed, 2009-09-30 at 21:19 -0400, John W. Linville wrote:

> +		if (!dev->ethtool_ops)
> +			dev->ethtool_ops = &cfg80211_ethtool_ops;
>  		break;

I might go so far and do it unconditionally so we get consistent
functionality across things. OTOH, full-mac drivers might be able to
support more.

> +const struct ethtool_ops cfg80211_ethtool_ops = {
> +	.get_drvinfo = cfg80211_get_drvinfo,
> +	.get_link = ethtool_op_get_link,
> +};
> +
> +void cfg80211_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)

if you change the order, you can make the latter static

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[Kalle Valo]

"John W. Linville" <linville@tuxdriver.com> writes:

> On Fri, Sep 25, 2009 at 09:53:35AM -0700, Luis R. Rodriguez wrote:
>
>> So for Wake-on-Wireless I ran into the same, ethtool just did not
>> offer the same wake up events needed for wireless. I could have
>> technically used ethtool and expanded it to support wireless but it
>> just seemed dirty.
>> 
>> I agree that using ethtool seems overkill compared to the patches
>> you posted.
>
> I think you either overestimate the amount of trouble for implementing
> (minimal) ethtool support or you underestimate the amount of
> functionality available through that interface.

I'm not worried about the implementation complexity, and as your
patches show it was easy. My concern is the overall design for
wireless devices. Instead of using nl80211 for everything, with some
features we would use nl80211/iw and with some ethtool. That's just
confusing and I don't like that. I would prefer that nl80211 provides
everything, it makes things so much easier.

> That, or you just don't like using something named "eth"tool for
> wireless -- but hey, let's be honest about the frames we
> send/receive to/from the kernel... :-)

I don't have a problem with the name :) But ethernet is still so much
different from 802.11 that there isn't that much to share and we in
wireless will need different features.

One example is the hw version, ethtool only provides u32 to userspace
and moves the burden of translating hw id to the user. For us a string
is much better choise because when debuggin we need to often (or
always?) know the chip version.

But this is not something I will start fighting about. If you still
think that ethtool is the way to go, I'm perfectly fine with it.

>> The ethtool interface provides functionality for viewing and modifying
> eeprom contents, dumping registers, trigger self-tests, basic driver
> info, getting and setting message reporting levels, external card
> identification (hey, _could_ be useful!), and some other bits like
> checksum offload that might(?) be useful in the future.  I understand
> regarding the WoW vs. WoL issue but probably the answer is just to
> add a new method for WoW...?

I took a look at ethtool help output from debian unstable and I think
this is the set of features we can use in wireless:

        ethtool -i|--driver DEVNAME     Show driver information
        ethtool -d|--register-dump DEVNAME      Do a register dump
                [ raw on|off ]
                [ file FILENAME ]
        ethtool -e|--eeprom-dump DEVNAME        Do a EEPROM dump
                [ raw on|off ]
                [ offset N ]
                [ length N ]
        ethtool -E|--change-eeprom DEVNAME      Change bytes in device
        EEPROM
                [ magic N ]
                [ offset N ]
                [ value N ]
        ethtool -p|--identify DEVNAME   Show visible port
        identification (e.g. blinking)
               [ TIME-IN-SECONDS ]
        ethtool -t|--test DEVNAME       Execute adapter self test
               [ online | offline ]

But here are the features which I doubt we will ever use:

        ethtool -s|--change DEVNAME     Change generic options
                [ speed %%d ]
                [ duplex half|full ]
                [ port tp|aui|bnc|mii|fibre ]
                [ autoneg on|off ]
                [ advertise %%x ]
                [ phyad %%d ]
                [ xcvr internal|external ]
                [ wol p|u|m|b|a|g|s|d... ]
                [ sopass %%x:%%x:%%x:%%x:%%x:%%x ]
                [ msglvl %%d ] 
        ethtool -a|--show-pause DEVNAME Show pause options
        ethtool -A|--pause DEVNAME      Set pause options
                [ autoneg on|off ]
                [ rx on|off ]
                [ tx on|off ]
        ethtool -c|--show-coalesce DEVNAME      Show coalesce options
        ethtool -C|--coalesce DEVNAME   Set coalesce options
                [adaptive-rx on|off]
                [adaptive-tx on|off]
                [rx-usecs N]
                [rx-frames N]
                [rx-usecs-irq N]
                [rx-frames-irq N]
                [tx-usecs N]
                [tx-frames N]
                [tx-usecs-irq N]
                [tx-frames-irq N]
                [stats-block-usecs N]
                [pkt-rate-low N]
                [rx-usecs-low N]
                [rx-frames-low N]
                [tx-usecs-low N]
                [tx-frames-low N]
                [pkt-rate-high N]
                [rx-usecs-high N]
                [rx-frames-high N]
                [tx-usecs-high N]
                [tx-frames-high N]
                [sample-interval N]
        ethtool -g|--show-ring DEVNAME  Query RX/TX ring parameters
        ethtool -G|--set-ring DEVNAME   Set RX/TX ring parameters
                [ rx N ]
                [ rx-mini N ]
                [ rx-jumbo N ]
                [ tx N ]
        ethtool -k|--show-offload DEVNAME       Get protocol offload
                information
        ethtool -K|--offload DEVNAME    Set protocol offload
                [ rx on|off ]
                [ tx on|off ]
                [ sg on|off ]
                [ tso on|off ]
                [ ufo on|off ]
                [ gso on|off ]
                [ gro on|off ]
                [ lro on|off ]
        ethtool -r|--negotiate DEVNAME  Restart N-WAY negotation
        ethtool -n|--show-nfc DEVNAME   Show Rx network flow
                classificationoptions
                [ rx-flow-hash
                tcp4|udp4|ah4|sctp4|tcp6|udp6|ah6|sctp6 ]
        ethtool -N|--config-nfc DEVNAME Configure Rx network flow
                classification options
                [ rx-flow-hash tcp4|udp4|ah4|sctp4|tcp6|udp6|ah6|sctp6
                m|v|t|s|d|f|n|r... ]

-- 
Kalle Valo

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[John W. Linville]

On Thu, Oct 01, 2009 at 05:18:33PM +0300, Kalle Valo wrote:
> "John W. Linville" <linville@tuxdriver.com> writes:
> 
> > On Fri, Sep 25, 2009 at 09:53:35AM -0700, Luis R. Rodriguez wrote:
> >
> >> So for Wake-on-Wireless I ran into the same, ethtool just did not
> >> offer the same wake up events needed for wireless. I could have
> >> technically used ethtool and expanded it to support wireless but it
> >> just seemed dirty.
> >> 
> >> I agree that using ethtool seems overkill compared to the patches
> >> you posted.
> >
> > I think you either overestimate the amount of trouble for implementing
> > (minimal) ethtool support or you underestimate the amount of
> > functionality available through that interface.
> 
> I'm not worried about the implementation complexity, and as your
> patches show it was easy. My concern is the overall design for
> wireless devices. Instead of using nl80211 for everything, with some
> features we would use nl80211/iw and with some ethtool. That's just
> confusing and I don't like that. I would prefer that nl80211 provides
> everything, it makes things so much easier.

Well, if the hw/fw version numbers were the only thing then I'd
probably say it's not a big deal.  But having ethtool support is nice
in that it makes a familiar tool work for us.  Among other things,
this probably helps with some distro scripts that don't work quite
right without it.  Plus, there is lots of debugging stuff that could
be turned-on without having to write new tools.

I suppose I understand the 'one API' idea, but why duplicate
functionality?  Anyway, adding a couple of ioctl calls isn't a
big deal.  And don't forget, we are still network drivers too...

> > That, or you just don't like using something named "eth"tool for
> > wireless -- but hey, let's be honest about the frames we
> > send/receive to/from the kernel... :-)
> 
> I don't have a problem with the name :) But ethernet is still so much
> different from 802.11 that there isn't that much to share and we in
> wireless will need different features.
> 
> One example is the hw version, ethtool only provides u32 to userspace
> and moves the burden of translating hw id to the user. For us a string
> is much better choise because when debuggin we need to often (or
> always?) know the chip version.

Look at the way most drivers set the version (using each byte as a
field).  If you want prettier output, adding a parser to the userland
ethtool is fairly trivial.  It looks something like the patch below...

> But this is not something I will start fighting about. If you still
> think that ethtool is the way to go, I'm perfectly fine with it.
> 
> >> The ethtool interface provides functionality for viewing and modifying
> > eeprom contents, dumping registers, trigger self-tests, basic driver
> > info, getting and setting message reporting levels, external card
> > identification (hey, _could_ be useful!), and some other bits like
> > checksum offload that might(?) be useful in the future.  I understand
> > regarding the WoW vs. WoL issue but probably the answer is just to
> > add a new method for WoW...?
> 
> I took a look at ethtool help output from debian unstable and I think
> this is the set of features we can use in wireless:
> 
>         ethtool -i|--driver DEVNAME     Show driver information
>         ethtool -d|--register-dump DEVNAME      Do a register dump
>                 [ raw on|off ]
>                 [ file FILENAME ]
>         ethtool -e|--eeprom-dump DEVNAME        Do a EEPROM dump
>                 [ raw on|off ]
>                 [ offset N ]
>                 [ length N ]
>         ethtool -E|--change-eeprom DEVNAME      Change bytes in device
>         EEPROM
>                 [ magic N ]
>                 [ offset N ]
>                 [ value N ]
>         ethtool -p|--identify DEVNAME   Show visible port
>         identification (e.g. blinking)
>                [ TIME-IN-SECONDS ]
>         ethtool -t|--test DEVNAME       Execute adapter self test
>                [ online | offline ]
 
I agree with the above.

> But here are the features which I doubt we will ever use:
> 
>         ethtool -s|--change DEVNAME     Change generic options
>                 [ speed %%d ]
>                 [ duplex half|full ]
>                 [ port tp|aui|bnc|mii|fibre ]
>                 [ autoneg on|off ]
>                 [ advertise %%x ]
>                 [ phyad %%d ]
>                 [ xcvr internal|external ]
>                 [ wol p|u|m|b|a|g|s|d... ]
>                 [ sopass %%x:%%x:%%x:%%x:%%x:%%x ]
>                 [ msglvl %%d ] 
>         ethtool -a|--show-pause DEVNAME Show pause options
>         ethtool -A|--pause DEVNAME      Set pause options
>                 [ autoneg on|off ]
>                 [ rx on|off ]
>                 [ tx on|off ]

I agree that the above are ethernet-specific.

>         ethtool -c|--show-coalesce DEVNAME      Show coalesce options
>         ethtool -C|--coalesce DEVNAME   Set coalesce options
>                 [adaptive-rx on|off]
>                 [adaptive-tx on|off]
>                 [rx-usecs N]
>                 [rx-frames N]
>                 [rx-usecs-irq N]
>                 [rx-frames-irq N]
>                 [tx-usecs N]
>                 [tx-frames N]
>                 [tx-usecs-irq N]
>                 [tx-frames-irq N]
>                 [stats-block-usecs N]
>                 [pkt-rate-low N]
>                 [rx-usecs-low N]
>                 [rx-frames-low N]
>                 [tx-usecs-low N]
>                 [tx-frames-low N]
>                 [pkt-rate-high N]
>                 [rx-usecs-high N]
>                 [rx-frames-high N]
>                 [tx-usecs-high N]
>                 [tx-frames-high N]
>                 [sample-interval N]

These _could_ be useful if wireless becomes more
performance-oriented...

>         ethtool -g|--show-ring DEVNAME  Query RX/TX ring parameters
>         ethtool -G|--set-ring DEVNAME   Set RX/TX ring parameters
>                 [ rx N ]
>                 [ rx-mini N ]
>                 [ rx-jumbo N ]
>                 [ tx N ]

Wireless devices have ring buffers, no?

>         ethtool -k|--show-offload DEVNAME       Get protocol offload
>                 information
>         ethtool -K|--offload DEVNAME    Set protocol offload
>                 [ rx on|off ]
>                 [ tx on|off ]
>                 [ sg on|off ]
>                 [ tso on|off ]
>                 [ ufo on|off ]
>                 [ gso on|off ]
>                 [ gro on|off ]
>                 [ lro on|off ]

Again, if wireless devices become performance-oriented...

>         ethtool -r|--negotiate DEVNAME  Restart N-WAY negotation

Ethernet-specific...might could be overloaded for wireless to trigger
reassoc...?

>         ethtool -n|--show-nfc DEVNAME   Show Rx network flow
>                 classificationoptions
>                 [ rx-flow-hash
>                 tcp4|udp4|ah4|sctp4|tcp6|udp6|ah6|sctp6 ]
>         ethtool -N|--config-nfc DEVNAME Configure Rx network flow
>                 classification options
>                 [ rx-flow-hash tcp4|udp4|ah4|sctp4|tcp6|udp6|ah6|sctp6
>                 m|v|t|s|d|f|n|r... ]

Long-shot, but no reason it couldn't be used in wireless... :-)

Anyway, it doesn't really matter if we don't use the whole API -- many
older ethernet devices don't support all these features.  The point
is that the API exists and has some overlap with our needs.  It is a
driver-oriented API, with nitty-gritty stuff that need not clutter a
configuraiton API like cfg80211.  There is even the potential of us
adding our own extensions (e.g. WoW) that are also device-oriented.

Anyway, between the link detection and making distro scripts work
plus enabling a familiar tool for basic driver info I think this is
a win.  So much the better if some drivers move to ethtool for register
dumping, setting message verbosity, querying/changing eeprom values,
etc, etc...

John

P.S.  The aforementioned path for userland ethtool...(theorhetical,
not even compiled...)

>From aa92d32ac1cca57bdd3439013b0c7777bdf1217c Mon Sep 17 00:00:00 2001
From: John W. Linville <linville@tuxdriver.com>
Date: Thu, 1 Oct 2009 11:01:32 -0400
Subject: [PATCH] add support for at76c50x-usb driver.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 Makefile.am    |    2 +-
 at76c50x-usb.c |   32 ++++++++++++++++++++++++++++++++
 ethtool.c      |    1 +
 3 files changed, 34 insertions(+), 1 deletions(-)
 create mode 100644 at76c50x-usb.c

diff --git a/Makefile.am b/Makefile.am
index eac65fe..a384949 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,7 +8,7 @@ ethtool_SOURCES = ethtool.c ethtool-copy.h ethtool-util.h	\
 		  amd8111e.c de2104x.c e100.c e1000.c igb.c	\
 		  fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c	\
 		  pcnet32.c realtek.c tg3.c marvell.c vioc.c	\
-		  smsc911x.c
+		  smsc911x.c at76c50x-usb.c
 
 dist-hook:
 	cp $(top_srcdir)/ethtool.spec $(distdir)
diff --git a/at76c50x-usb.c b/at76c50x-usb.c
new file mode 100644
index 0000000..295d1cb
--- /dev/null
+++ b/at76c50x-usb.c
@@ -0,0 +1,32 @@
+#include <stdio.h>
+#include "ethtool-util.h"
+
+static char hw_versions[] = {
+        "503_ISL3861",
+        "503_ISL3863",
+        "        503",
+        "    503_ACC",
+        "        505",
+        "   505_2958",
+        "       505A",
+        "     505AMX",
+};
+
+int
+at76c50x_usb_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
+{
+	u8 version = (u8)(regs->version >> 24);
+	u8 rev_id = (u8)(regs->version);
+	char *ver_string;
+
+	if(version != 0)
+		return -1;
+
+	ver_string = hw_versions[rev_id];
+	fprintf(stdout,
+		"Hardware Version                    %s\n",
+		ver_string);
+
+	return 0;
+}
+
diff --git a/ethtool.c b/ethtool.c
index 0110682..7608750 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1189,6 +1189,7 @@ static struct {
 	{ "sky2", sky2_dump_regs },
         { "vioc", vioc_dump_regs },
         { "smsc911x", smsc911x_dump_regs },
+        { "at76c50x-usb", at76c50x_usb_dump_regs },
 };
 
 static int dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
-- 
1.6.2.5
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[Ben Hutchings]

On Thu, 2009-10-01 at 11:18 -0400, John W. Linville wrote:
[...]
> > But here are the features which I doubt we will ever use:
> > 
> >         ethtool -s|--change DEVNAME     Change generic options
> >                 [ speed %%d ]
> >                 [ duplex half|full ]
> >                 [ port tp|aui|bnc|mii|fibre ]
> >                 [ autoneg on|off ]
> >                 [ advertise %%x ]
> >                 [ phyad %%d ]
> >                 [ xcvr internal|external ]
> >                 [ wol p|u|m|b|a|g|s|d... ]
> >                 [ sopass %%x:%%x:%%x:%%x:%%x:%%x ]
> >                 [ msglvl %%d ] 
> >         ethtool -a|--show-pause DEVNAME Show pause options
> >         ethtool -A|--pause DEVNAME      Set pause options
> >                 [ autoneg on|off ]
> >                 [ rx on|off ]
> >                 [ tx on|off ]
> 
> I agree that the above are ethernet-specific.
[...]

Message level isn't and WoL arguably isn't.  It's a shame that these
original ethtool settings are still bundled together...

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[John W. Linville]

On Thu, Oct 01, 2009 at 04:33:19PM +0100, Ben Hutchings wrote:
> On Thu, 2009-10-01 at 11:18 -0400, John W. Linville wrote:
> [...]
> > > But here are the features which I doubt we will ever use:
> > > 
> > >         ethtool -s|--change DEVNAME     Change generic options
> > >                 [ speed %%d ]
> > >                 [ duplex half|full ]
> > >                 [ port tp|aui|bnc|mii|fibre ]
> > >                 [ autoneg on|off ]
> > >                 [ advertise %%x ]
> > >                 [ phyad %%d ]
> > >                 [ xcvr internal|external ]
> > >                 [ wol p|u|m|b|a|g|s|d... ]
> > >                 [ sopass %%x:%%x:%%x:%%x:%%x:%%x ]
> > >                 [ msglvl %%d ] 
> > >         ethtool -a|--show-pause DEVNAME Show pause options
> > >         ethtool -A|--pause DEVNAME      Set pause options
> > >                 [ autoneg on|off ]
> > >                 [ rx on|off ]
> > >                 [ tx on|off ]
> > 
> > I agree that the above are ethernet-specific.
> [...]
> 
> Message level isn't and WoL arguably isn't.  It's a shame that these
> original ethtool settings are still bundled together...

Oh, yes!  Missed those in the noise...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[Kalle Valo]

"John W. Linville" <linville@tuxdriver.com> writes:

> On Thu, Oct 01, 2009 at 05:18:33PM +0300, Kalle Valo wrote:
>> 
>> I'm not worried about the implementation complexity, and as your
>> patches show it was easy. My concern is the overall design for
>> wireless devices. Instead of using nl80211 for everything, with some
>> features we would use nl80211/iw and with some ethtool. That's just
>> confusing and I don't like that. I would prefer that nl80211 provides
>> everything, it makes things so much easier.
>
> Well, if the hw/fw version numbers were the only thing then I'd
> probably say it's not a big deal.  But having ethtool support is nice
> in that it makes a familiar tool work for us.  Among other things,
> this probably helps with some distro scripts that don't work quite
> right without it.  Plus, there is lots of debugging stuff that could
> be turned-on without having to write new tools.

Agreed, maybe expect the distro scripts part. To me that just sounds
as a bug in the scripts.

> I suppose I understand the 'one API' idea, but why duplicate
> functionality?

Just because the common functionality in this case isn't high enough.
I'm worried that we will use 10% of the functionality in nl80211 and
the rest 90% will be something we can't use and have to reimplement in
nl80211.

> Anyway, adding a couple of ioctl calls isn't a big deal.

Sure, but we need to support this forever. If, say after two years, we
decide that ethtool is not the way to go, it's very difficult to
remove it. The less interfaces we have, the easier it is to maintain
them.

> And don't forget, we are still network drivers too...

I hope ethtool isn't a strict requirement for a network driver, at
least I haven't heard about that.

>> One example is the hw version, ethtool only provides u32 to userspace
>> and moves the burden of translating hw id to the user. For us a string
>> is much better choise because when debuggin we need to often (or
>> always?) know the chip version.
>
> Look at the way most drivers set the version (using each byte as a
> field).

Yes, that's how it is also with wl1251. A number like '0x7030101' is
just not that user friendly.

> If you want prettier output, adding a parser to the userland ethtool
> is fairly trivial. It looks something like the patch below...

Oh wow, that's cool and a truly useful feature. One complaint less
from me :)

>>         ethtool -c|--show-coalesce DEVNAME      Show coalesce options
>>         ethtool -C|--coalesce DEVNAME   Set coalesce options
>>                 [adaptive-rx on|off]
>>                 [adaptive-tx on|off]
>>                 [rx-usecs N]
>>                 [rx-frames N]
>>                 [rx-usecs-irq N]
>>                 [rx-frames-irq N]
>>                 [tx-usecs N]
>>                 [tx-frames N]
>>                 [tx-usecs-irq N]
>>                 [tx-frames-irq N]
>>                 [stats-block-usecs N]
>>                 [pkt-rate-low N]
>>                 [rx-usecs-low N]
>>                 [rx-frames-low N]
>>                 [tx-usecs-low N]
>>                 [tx-frames-low N]
>>                 [pkt-rate-high N]
>>                 [rx-usecs-high N]
>>                 [rx-frames-high N]
>>                 [tx-usecs-high N]
>>                 [tx-frames-high N]
>>                 [sample-interval N]
>
> These _could_ be useful if wireless becomes more
> performance-oriented...

Maybe, or maybe not. We will only find out within the next few years.

And what will we do if the parameters are actually a bit different? Is
it ok to extend ethtool for supporting wireless or do we later on have
to add separate support to nl80211? The latter would suck big time.

>>         ethtool -g|--show-ring DEVNAME  Query RX/TX ring parameters
>>         ethtool -G|--set-ring DEVNAME   Set RX/TX ring parameters
>>                 [ rx N ]
>>                 [ rx-mini N ]
>>                 [ rx-jumbo N ]
>>                 [ tx N ]
>
> Wireless devices have ring buffers, no?

Yes, there is hardware which have them but again the question is this
relevant for wireless devices. In ethernet the hardware is the
bottleneck but in 802.11 the wireless medium is the bottleneck, so the
parameters we need to configure are usually different.

>>         ethtool -r|--negotiate DEVNAME  Restart N-WAY negotation
>
> Ethernet-specific...might could be overloaded for wireless to trigger
> reassoc...?

Please no, I don't want to see any reassociation or anything else
802.11 state related in ethtool, nl80211 was created for this. This is
something I would object loudly :)

> Anyway, it doesn't really matter if we don't use the whole API -- many
> older ethernet devices don't support all these features.  The point
> is that the API exists and has some overlap with our needs.  It is a
> driver-oriented API, with nitty-gritty stuff that need not clutter a
> configuraiton API like cfg80211.  There is even the potential of us
> adding our own extensions (e.g. WoW) that are also device-oriented.
>
> Anyway, between the link detection and making distro scripts work
> plus enabling a familiar tool for basic driver info I think this is
> a win.  So much the better if some drivers move to ethtool for register
> dumping, setting message verbosity, querying/changing eeprom values,
> etc, etc...

Sounds good enough. As I said in my earlier email, I'm not going argue
about this for too long. You know this better than I do. So let's go
forward with ethtool. 

Thanks for listening to my concerns.

-- 
Kalle Valo

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[John W. Linville]

On Thu, Oct 01, 2009 at 07:20:09PM +0300, Kalle Valo wrote:
> "John W. Linville" <linville@tuxdriver.com> writes:
> 
> > On Thu, Oct 01, 2009 at 05:18:33PM +0300, Kalle Valo wrote:

> > Anyway, adding a couple of ioctl calls isn't a big deal.
> 
> Sure, but we need to support this forever. If, say after two years, we
> decide that ethtool is not the way to go, it's very difficult to
> remove it. The less interfaces we have, the easier it is to maintain
> them.

Just to be clear, I was taling about adding ioctl calls to a
userland application (if you didn't want to use the ethtool utility).
The required ioctls are already defined for ethtool in the kernel.

> >>         ethtool -r|--negotiate DEVNAME  Restart N-WAY negotation
> >
> > Ethernet-specific...might could be overloaded for wireless to trigger
> > reassoc...?
> 
> Please no, I don't want to see any reassociation or anything else
> 802.11 state related in ethtool, nl80211 was created for this. This is
> something I would object loudly :)

Well, it was just a thought... :-)

> > Anyway, between the link detection and making distro scripts work
> > plus enabling a familiar tool for basic driver info I think this is
> > a win.  So much the better if some drivers move to ethtool for register
> > dumping, setting message verbosity, querying/changing eeprom values,
> > etc, etc...
> 
> Sounds good enough. As I said in my earlier email, I'm not going argue
> about this for too long. You know this better than I do. So let's go
> forward with ethtool. 
> 
> Thanks for listening to my concerns.

Sure, np.  And FWIW, I don't predict a huge problem if there are
valid extensions required for use by wireless drivers in the future.
But for now, I'd like to see us make use of some of the debugging
facilities available in the ethtool API -- hopefully the iwlwifi guys
are listening... ;-)

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[Luis R. Rodriguez]

On Thu, Oct 1, 2009 at 5:07 PM, John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:

> I don't predict a huge problem if there are
> valid extensions required for use by wireless drivers in the future.
> But for now, I'd like to see us make use of some of the debugging
> facilities available in the ethtool API -- hopefully the iwlwifi guys
> are listening... ;-)

Does the same apply to wimax then? Ethtool for 802.11 and wimax? Eh.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] cfg80211: firmware and hardware version

[Inaky Perez-Gonzalez]

On Thu, 2009-10-01 at 13:56 -0600, Luis R. Rodriguez wrote:
> On Thu, Oct 1, 2009 at 5:07 PM, John W. Linville <linville@tuxdriver.com> wrote:
> 
> > I don't predict a huge problem if there are
> > valid extensions required for use by wireless drivers in the future.
> > But for now, I'd like to see us make use of some of the debugging
> > facilities available in the ethtool API -- hopefully the iwlwifi guys
> > are listening... ;-)
> 
> Does the same apply to wimax then? Ethtool for 802.11 and wimax? Eh.

Not really -- WiMAX is not eth-frame based, but IP based.

The WiMAX stack doesn't require any type of framing/network device
typing requirement. That is left up to the device driver writer
(although yes, emulating eth is easier).

-- 
-- Inaky