| Message ID | 20091001151820.GA2895@tuxdriver.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
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.
"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.
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
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
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. Luis -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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).
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)