On 2009-10-01 06:32 AM, Malte Gell wrote:
>I first noticed the LEDs are confused,
no, the LED colors is not _confused_ at all.
This is simply different on.... well, you know: on a per-device base!
For example: The Netgear uses a single bi-color LED for their WNDA3100 stick.
It glows blue or/and orange depending on the selected band and current
operation mode and state...
No idea why they didn't stick to the usual red/green mix.
Maybe because someone told the hw-devs about the existence of
red/green colorblind people??!
The original vendor driver (located: drivers/staging/otus/80211core/ledmgr.c)
goes to great lengths to describe what's behind the variety of
blinking signals. Which is nice, if you like expensive light shows...
>when I ran my AVM Fritz USBN N under Windows,
>where the LEDs had the right order. I made a little patch that put
>the assoc and the tx LEDs in the right order.
FYI: you can assign the LEDs under "/sys/class/leds/" with a different tigger
without messing with the kernel source... An up-to-date README can be found in
the kernel's documentation directory: Documentation/leds-class.txt ,
or you can look it up online as well:
http://www.mjmwired.net/kernel/Documentation/leds-class.txt (from 2.6.31)
but back to the patch and the problem with the wide diversity of
over-customized solutions for a direct feedback mechanism to the
mindful human operator...
what about the (inline) _attached_ approach?
Sure, this idea needs some more code... but, it covers all/most
possible scenarios from beloved "no, no, no" vendors.
---
From: Christian Lamparter <[email protected]>
Subject: [PATCH] ar9170usb: flexible LED mapping
This patch adds two more quirk flags which are useful to:
- reduce the number of virtual/ghost LEDs
( for low-budget devices: Netgear WN111 v2 )
- select an alternative LED mapping.
( for AVM FRITZ!WLAN USB Stick N )
Reported-by: Malte Gell <[email protected]>
Signed-off-by: Christian Lamparter <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ar9170/ar9170.h b/drivers/net/wireless/ath/ar9170/ar9170.h
index ec034af..a451bb1 100644
--- a/drivers/net/wireless/ath/ar9170/ar9170.h
+++ b/drivers/net/wireless/ath/ar9170/ar9170.h
@@ -155,6 +155,12 @@ struct ar9170_sta_tid {
#define AR9170_NUM_TX_STATUS 128
#define AR9170_NUM_TX_AGG_MAX 30
+enum ar9170_quirks {
+ AR9170_REQ_FW1_ONLY = BIT(0),
+ AR9170_ONLY_ONE_LED = BIT(1),
+ AR9170_SWAPPED_LEDS = BIT(2),
+};
+
struct ar9170 {
struct ieee80211_hw *hw;
struct ath_common common;
@@ -241,6 +247,9 @@ struct ar9170 {
/* (cached) HW A-MPDU settings */
u8 global_ampdu_density;
u8 global_ampdu_factor;
+
+ /* device quirks */
+ unsigned long quirks;
};
struct ar9170_sta_info {
diff --git a/drivers/net/wireless/ath/ar9170/led.c b/drivers/net/wireless/ath/ar9170/led.c
index 86c4e79..36ab738 100644
--- a/drivers/net/wireless/ath/ar9170/led.c
+++ b/drivers/net/wireless/ath/ar9170/led.c
@@ -155,18 +155,30 @@ void ar9170_unregister_leds(struct ar9170 *ar)
cancel_delayed_work_sync(&ar->led_work);
}
+const static int std_led_map[AR9170_NUM_LEDS] = { 0, 1 };
+const static int avm_led_map[AR9170_NUM_LEDS] = { 1, 0 };
+
int ar9170_register_leds(struct ar9170 *ar)
{
+ const int *led_map;
int err;
+ if (ar->quirks & AR9170_SWAPPED_LEDS)
+ led_map = avm_led_map;
+ else
+ led_map = std_led_map;
+
INIT_DELAYED_WORK(&ar->led_work, ar9170_update_leds);
- err = ar9170_register_led(ar, 0, "tx",
+ err = ar9170_register_led(ar, led_map[0], "tx",
ieee80211_get_tx_led_name(ar->hw));
if (err)
goto fail;
- err = ar9170_register_led(ar, 1, "assoc",
+ if (ar->quirks & AR9170_ONLY_ONE_LED)
+ return 0;
+
+ err = ar9170_register_led(ar, led_map[1], "assoc",
ieee80211_get_assoc_led_name(ar->hw));
if (err)
goto fail;
diff --git a/drivers/net/wireless/ath/ar9170/usb.c b/drivers/net/wireless/ath/ar9170/usb.c
index e974e58..3be19e4 100644
--- a/drivers/net/wireless/ath/ar9170/usb.c
+++ b/drivers/net/wireless/ath/ar9170/usb.c
@@ -55,10 +55,6 @@ MODULE_FIRMWARE("ar9170.fw");
MODULE_FIRMWARE("ar9170-1.fw");
MODULE_FIRMWARE("ar9170-2.fw");
-enum ar9170_requirements {
- AR9170_REQ_FW1_ONLY = 1,
-};
-
static struct usb_device_id ar9170_usb_ids[] = {
/* Atheros 9170 */
{ USB_DEVICE(0x0cf3, 0x9170) },
@@ -73,7 +69,7 @@ static struct usb_device_id ar9170_usb_ids[] = {
/* Netgear WNDA3100 */
{ USB_DEVICE(0x0846, 0x9010) },
/* Netgear WN111 v2 */
- { USB_DEVICE(0x0846, 0x9001) },
+ { USB_DEVICE(0x0846, 0x9001), .driver_info = AR9170_ONLY_ONE_LED },
/* Zydas ZD1221 */
{ USB_DEVICE(0x0ace, 0x1221) },
/* ZyXEL NWD271N */
@@ -89,7 +85,7 @@ static struct usb_device_id ar9170_usb_ids[] = {
/* IO-Data WNGDNUS2 */
{ USB_DEVICE(0x04bb, 0x093f) },
/* AVM FRITZ!WLAN USB Stick N */
- { USB_DEVICE(0x057C, 0x8401) },
+ { USB_DEVICE(0x057C, 0x8401), .driver_info = AR9170_SWAPPED_LEDS },
/* AVM FRITZ!WLAN USB Stick N 2.4 */
{ USB_DEVICE(0x057C, 0x8402), .driver_info = AR9170_REQ_FW1_ONLY },
@@ -589,7 +585,7 @@ static int ar9170_usb_request_firmware(struct ar9170_usb *aru)
return 0;
}
- if (aru->req_one_stage_fw) {
+ if (aru->common.quirks & AR9170_REQ_FW1_ONLY) {
dev_err(&aru->udev->dev, "ar9170.fw firmware file "
"not found and is required for this device\n");
return -EINVAL;
@@ -753,15 +749,6 @@ err_out:
return err;
}
-static bool ar9170_requires_one_stage(const struct usb_device_id *id)
-{
- if (!id->driver_info)
- return false;
- if (id->driver_info == AR9170_REQ_FW1_ONLY)
- return true;
- return false;
-}
-
static int ar9170_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -781,8 +768,7 @@ static int ar9170_usb_probe(struct usb_interface *intf,
aru->udev = udev;
aru->intf = intf;
ar = &aru->common;
-
- aru->req_one_stage_fw = ar9170_requires_one_stage(id);
+ ar->quirks = id->driver_info;
usb_set_intfdata(intf, aru);
SET_IEEE80211_DEV(ar->hw, &intf->dev);
diff --git a/drivers/net/wireless/ath/ar9170/usb.h b/drivers/net/wireless/ath/ar9170/usb.h
index d098f4d..714436d 100644
--- a/drivers/net/wireless/ath/ar9170/usb.h
+++ b/drivers/net/wireless/ath/ar9170/usb.h
@@ -64,8 +64,6 @@ struct ar9170_usb {
struct usb_anchor tx_pending;
struct usb_anchor tx_submitted;
- bool req_one_stage_fw;
-
spinlock_t tx_urb_lock;
unsigned int tx_submitted_urbs;
unsigned int tx_pending_urbs;
Christian Lamparter <[email protected]> wrote
> If you take a look at the ledtriggers values in /sys/call/leds/
> "none rfkill0 phy0rx phy0tx [phy0assoc] phy0radio",
> you instantly see that the phyXname trigger is dynamic.
Can I change the link(s) there at any time or will the ar9170usb change it
again?
> in my opinion, finding a simple udev rule is harder than simply
> modify the driver code :-D.
That's true! I just had to exchange the words "tx" with "assoc" in the driver,
whereas a udev rule is complicated!
Regards
Malte
On 10/02/2009 01:45 PM, Malte Gell wrote:
> Christian Lamparter <[email protected]> wrote
>
>>> The Netgear (WN?) 111 even only has one blue LED as far as I know.
>> the question is if it's the only device with this deficit, or not?
>
> Is it feasable to write to the well known stick makers (Netgear, AVM,
> Belkin,Asus...) and just ask them?
After looking into staging/otus/80211core/ledmgr.c, which has functions
zfLedCtrlType1,2,3 for:
- "Traditional single-LED state"
- "Netgear Dual-LED state"
- "Netgear Single-LED state"
(althrough they are not used there, as noone initializes wd->ledStruct.LEDCtrlType correctly)
I guess there is a way to determine the number of LED in a device from the EEPROM.
I really doubt that Netgear would built different drivers/firmwares (if they built any instead of getting them from Atheros)
for both WNDA3100 and WN111v2.
hal/hpmain.c, line 2322:
#define ZM_SEEPROM_HARDWARE_TYPE_OFFSET (0x1374)
the value from the above address is retrieved in rsp[5] and processed in
hal/hprw.c, lines 601f.
wd->ledStruct.ledMode[0] = (u16_t)(rsp[5]&0xffff);
wd->ledStruct.ledMode[1] = (u16_t)(rsp[5]>>16);
If the bits of ledMode[] are explained in 80211/ledmgr.c, lines 34ff., Atheros provided a generic way for vendors to
program LED behaviour via the EEPROM and Netgear got some extra handling in the driver (if otus is close the windows driver).
Regards,
Joerg.
On Thursday 01 October 2009 23:24:59 Hin-Tak Leung wrote:
> On Thu, Oct 1, 2009 at 9:34 PM, Christian Lamparter
> <[email protected]> wrote:
> > On Thursday 01 October 2009 20:06:35 Hin-Tak Leung wrote:
> >> On Thu, Oct 1, 2009 at 3:54 PM, Christian Lamparter
> >> <[email protected]> wrote:
> >> > On 2009-10-01 06:32 AM, Malte Gell wrote:
> >> >>I first noticed the LEDs are confused,
> >> > no, the LED colors are not _confused_ at all.
> >> > This is simply different on.... well, you know: on a per-device base!
> >> >
> >> > For example: The Netgear uses a single bi-color LED for their WNDA3100 stick.
> >> > It glows blue or/and orange depending on the selected band and current
> >> > operation mode and state...
> >> >
> >> > No idea why they didn't stick to the usual red/green mix.
> >> > Maybe because someone told the hw-devs about the existence of
> >> > red/green colorblind people??!
> >> >
> >> > The original vendor driver (located: drivers/staging/otus/80211core/ledmgr.c)
> >> > goes to great lengths to describe what's behind the variety of
> >> > blinking signals. Which is nice, if you like expensive light shows...
> >>
> >> This is just based on my brief look at the relevant code itself - I
> >> think the driver actually enumerates how many LEDs the device has, so
> >> the ONLY_ONE_LED construct is a bit bogus. Also I think the
> >> 0x0846:0x9001 is Malte's with swapped LEDs, not ONLY_ONE?
> > ?
> >
> > 0x0846:0x9001 is a Netgear WN111 v2.
> > (one LED reference: otus' ledmgr.c ~line 547)
> >
> > and AFAIK: Malte uses an AVM FRITZ!WLAN Stick N (2.4?).
>
> I just remember the ID from Malte's ndiswrapper-list posting.
indeed, he must have bought a WN111 v2 as well?!
but just in case he's trying to use the WN111 driver:
This may not work, AVM FRITZ!WLAN is not 100% compatible with
most other AR9170 variants and could refuse to work properly with
other 3rd party drivers.
> It is not unthinkable of a rebranded device, or even vendor abusing the
> system a bit and put out a device which is subtly different with the
> same ids.
yes, I've heard of such cases.
But in all of them a reseller simply _borrowed_ the usb VID of
the hardware manufacture (e.g Ralink/Atheros) and not from
other resellers.
I hope you agree with me on this occasion that we don't have
to be paranoid about pid/vid collisions, unless someone
can prove that he got a genuine ar9170 with a bi-color LED/two LEDs
which identifies itself as WN111 v2.
> >> I had a look when Malte first wrote about the swap - the code
> >> basically just do assoc/data-tx in enumeration order (first LED found
> >> is assoc, 2nd is tx, which make sense if some variant only have a
> >> LED).
> > Depends. I think it's more important to have some sort of an "an activity LED"
> > than a connection indicator, because most desktops-guis nowadays have lots
> > of fancy applets which are constantly monitoring your connection status and
> > start to bark as soon as it changes...
> >
> > BTW: my laptop (with an IWL 5300) only has one LED assigned for wlan as well.
> > But, someone had the genius idea to put the activity LED into _inverted_
> > mode when the connection is established. It stays on after association
> > and flashes under TX activity... This is nice, but it has a downside:
> > two trigger _drive_ one LED => no real exclusive access anymore.
> > If you want to reassign the LEDs the clueless user has to be aware about
> > this trigger dependency, or he see some _blinking interference_.
> >
> >> As for the color - it is probably just a matter of commercial
> >> interests (if they can get a particular color from a source cheaper)
> > unlikely, the bi-color LED is a super bright one.
> >
> >> or simply variety to catch some customers - as you say, some *do* like
> >> expensive light shows, and there is a market for it and money to be
> >> made that way.
> > :-D
> >
> > well to be fair, I think they tried to implement some sort of
> > complex blinking language code. You can tell apart if
> > the device is idling/scanning/connected/sending in the 5GHz or 2GHz
> > band just by looking at the blue and orange rhythms...
> > (maybe this visual feedback is indeed easier to comprehend for the generic
> > customer than any hard and honest numbers)
> >
> > But back to the topic:
> > This elaborate morse code is clearly way beyond the scope and
> > abilities of ledclass. I think we should really stick to the
> > simple rule: one trigger corresponds to only one physical LED.
> >
> >> Anyway, I am just writing regarding the ONLY_ONE_LED construct and its
> >> association with 0x0846:0x9001 ...
> > hmm, not sure what I should do here...
> > Do you think the driver should simply ignore the lack of a second LED (color)?
> > Or is it just that the ONLY_ONE_LED construct sounds really lame
> > and needs a more cunning name?
>
> Hmm, I mean the ONLY_ONE_LED config should not be a compiled in config
> associated with an vid/pid, but dynamically determined from the
> enumeration.
dynamically determined? how?
Neither the EEPROM, nor any hw/fw register contain any information about
the number of available LEDs on the platform. The only feasible clue
comes from the devices' usb descriptors (=> VID and PID).
> In the case of only one LED, the it sounds quite neat to represent
> both assciation status and transmission status by blinking pattern
> :-).
It is... but the logic which programs the GPIOs is nowhere to be found
inside the driver... I think the vendor implemented it somewhere
deep inside the embedded controller (firmware).
And the card, driver, ledclass and userspace is unaware of this and
treats it as two independent things => this is BAD.
Let me explain (why this is BAD), just take this topic as a example:
Malte (our eagle-eyed user) discovered that his LEDs don't flash the
same way as with a customized vendor driver.
In his attempt to fix is issue, he could inadvertently break the
configuration of others...
And there's no denying here, at some point, we all had to
suffer from similar situations which are knows as the
fallout of -rc1 kernels. :-D
> But one of them should take priority in case of conflict, and in this
> regard it seems that you and I disagree on which should be.
hmm, when I think about it: the only (proper) way to accomplish this feat
would be to extend the LED(class), so it can have several different triggers
at the same time...
(or as an alternative: implement trigger filters. So some LEDs can only
be used with a specific trigger event.
however, there is a possibility that this might to more harm than good.)
Regards,
Chr
On Friday 02 October 2009 08:52:36 Malte Gell wrote:
> Christian Lamparter <[email protected]> wrote
> > On 2009-10-01 06:32 AM, Malte Gell wrote:
> > For example: The Netgear uses a single bi-color LED for their WNDA3100
> > stick. It glows blue or/and orange depending on the selected band and
> > current operation mode and state...
>
> The Netgear (WN?) 111 even only has one blue LED as far as I know.
the question is if it's the only device with this deficit, or not?
> > FYI: you can assign the LEDs under "/sys/class/leds/" with a different
> > tigger without messing with the kernel source... An up-to-date README can
> > be found in the kernel's documentation directory:
> > Documentation/leds-class.txt , or you can look it up online as well:
> > http://www.mjmwired.net/kernel/Documentation/leds-class.txt (from 2.6.31)
>
> Very good to know. This needs a udev rule, right?
yes, should be possible one way or another...
If you do find an easy ACTION== rule, then let us know!
It would make a nice addition to the wiki @ wireless.kernel.org .
> But, what about a USB ID based decission in the driver how to handle the LED?
> I have a Fritz WLAN N, USB ID: 057c:8401
The first post had a patch attached:
http://patchwork.kernel.org/patch/50977/
(raw patch)
=> http://patchwork.kernel.org/patch/50977/raw/
it should do exactly what you want.
just make sure, you reverted any previous LED related patch,
or it will not apply cleanly.
Regards,
Chr
Christian Lamparter <[email protected]> wrote
> On Saturday 03 October 2009 04:53:41 Malte Gell wrote:
> > Can I change the link(s) there at any time
>
> yes, you can change then any time you want.
> > That's true! I just had to exchange the words "tx" with "assoc"
> > in the driver, whereas a udev rule is complicated!
>
> well, there's at least one upside: this rule could be easily
> modified to work with different wireless drivers.
Maybe a udev rule would even be the best solution, this way Linux distributors
could easily adapt it to different hardware.
> by the way: Could you please do me a flavour (since you are
> the only WN111 v2 owner I know) and tell us what "hwtype"
> result you get with Joerg Albert's patch:
>
> http://marc.info/?l=linux-wireless&m=125452461322729&q=raw
I'm so sorry, I do not have the WN111v2 any more! I gave it back a few days
ago after I was able to get the Fritz WLAN N running... I had the WN111v2 just
for a few days while the Fritz wasn't running.
Regards
Malte
Hi,
could anyone with a WN111v2 (or any other device with one LED only)
apply the patch below and look into syslog for the value of "hwtype"?
I get 22212221 for an AVM stick (057c:8401) and 22211111 for a Netgear WNDA3100 (0846:9010).
Thanks,
Joerg.
--
diff --git a/drivers/net/wireless/ath/ar9170/hw.h b/drivers/net/wireless/ath/ar9170/hw.h
index 6cbfb2f..74b619c 100644
--- a/drivers/net/wireless/ath/ar9170/hw.h
+++ b/drivers/net/wireless/ath/ar9170/hw.h
@@ -77,6 +77,7 @@ enum ar9170_cmd {
#define AR9170_EP_IRQ 3
#define AR9170_EP_CMD 4
+#define AR9170_EEPROM_HWTYPE 0x1374
#define AR9170_EEPROM_START 0x1600
#define AR9170_GPIO_REG_BASE 0x1d0100
diff --git a/drivers/net/wireless/ath/ar9170/main.c b/drivers/net/wireless/ath/ar9170/main.c
index c1f8c69..bffd6c4 100644
--- a/drivers/net/wireless/ath/ar9170/main.c
+++ b/drivers/net/wireless/ath/ar9170/main.c
@@ -2598,6 +2598,7 @@ static int ar9170_read_eeprom(struct ar9170 *ar)
__le32 offsets[RW];
unsigned int rx_streams, tx_streams, tx_params = 0;
int i, j, err, bands = 0;
+ u32 hwtype;
BUILD_BUG_ON(sizeof(ar->eeprom) & 3);
@@ -2665,6 +2666,14 @@ static int ar9170_read_eeprom(struct ar9170 *ar)
/* second part of wiphy init */
SET_IEEE80211_PERM_ADDR(ar->hw, addr);
+ /* read hw type (aka the led modes) */
+ err = ar9170_read_reg(ar, AR9170_EEPROM_HWTYPE, &hwtype);
+ if (err)
+ return err;
+ /* jal: for debugging only */
+ printk(KERN_INFO "%s: hwtype %08x\n",
+ wiphy_name(ar->hw->wiphy), hwtype);
+
return bands ? 0 : -EINVAL;
}
Christian Lamparter <[email protected]> wrote
> > >
> > > and AFAIK: Malte uses an AVM FRITZ!WLAN Stick N (2.4?).
> >
> > I just remember the ID from Malte's ndiswrapper-list posting.
> indeed, he must have bought a WN111 v2 as well?!
Carefully observed ;-) Yes, I _had_ a WN111v2, but gave it back, my goal was
to get the Fritz WLAN N ( not the 2.4 version) working it it works now. I had
the WN111v2 just for a few days to be able to be online.
Regards
Malte
Christian Lamparter <[email protected]> wrote
> > The Netgear (WN?) 111 even only has one blue LED as far as I know.
>
> the question is if it's the only device with this deficit, or not?
Is it feasable to write to the well known stick makers (Netgear, AVM,
Belkin,Asus...) and just ask them?
>
> > > FYI: you can assign the LEDs under "/sys/class/leds/"
> > Very good to know. This needs a udev rule, right?
>
> yes, should be possible one way or another...
> If you do find an easy ACTION== rule, then let us know!
Do I hear irony? Is it not easy? Isn't it just a link that needs to be made /
changed? Does it matter, when to do this, e.g. after the module ist loaded or
the network system is started?
> > But, what about a USB ID based decission in the driver how to handle the
> > LED? I have a Fritz WLAN N, USB ID: 057c:8401
>
> The first post had a patch attached:
> http://patchwork.kernel.org/patch/50977/
Have not tried it, is it well tested? If it works, will the patch become
official part of compat-wireless?
Regards
Malte
On Saturday 03 October 2009 00:25:47 Joerg Albert wrote:
> On 10/02/2009 01:45 PM, Malte Gell wrote:
> > Christian Lamparter <[email protected]> wrote
> >
> >>> The Netgear (WN?) 111 even only has one blue LED as far as I know.
> >> the question is if it's the only device with this deficit, or not?
> >
> > Is it feasable to write to the well known stick makers (Netgear, AVM,
> > Belkin,Asus...) and just ask them?
>
> After looking into staging/otus/80211core/ledmgr.c, which has functions
> zfLedCtrlType1,2,3 for:
> - "Traditional single-LED state"
> - "Netgear Dual-LED state"
> - "Netgear Single-LED state"
>
> (althrough they are not used there, as noone initializes wd->ledStruct.LEDCtrlType correctly)
On Windows platforms this setting is initialized by the Driver .inf file:
(ref: http://ftp.dlink.ru/pub/Wireless/DWA-160/Drivers/Rev.A-DWA-160_S0009/Drivers/Vista32/arusb_lh.inf )
[NTGR9010.reg]
HKR, Ndi, Service, 0, "WNDA3100"
HKR, Ndi\Interfaces, UpperRange, 0, "ndis5"
HKR, Ndi\Interfaces, LowerRange, 0, "wlan,ethernet"
[...]
HKR,, DfsChDisable, 0, 1
HKR,, LEDCtrlType, 0, 2
vs.
[NTGR9001.reg]
HKR, Ndi, Service, 0, "WN111v2"
HKR, Ndi\Interfaces, UpperRange, 0, "ndis5"
HKR, Ndi\Interfaces, LowerRange, 0, "wlan,ethernet"
[...]
HKR,, DfsChDisable, 0, 1
HKR,, LEDCtrlType, 0, 3
the situation is different for AVM:
there is not any reference of LEDCtrlType in any of AVM's inf files,
but the LEDCtrlType string does turn up in the driver file.
> I guess there is a way to determine the number of LED in a device from the EEPROM.
> I really doubt that Netgear would built different drivers/firmwares
> (if they built any instead of getting them from Atheros)
> for both WNDA3100 and WN111v2.
>
> hal/hpmain.c, line 2322:
> #define ZM_SEEPROM_HARDWARE_TYPE_OFFSET (0x1374)
>
> the value from the above address is retrieved in rsp[5] and processed in
>
> hal/hprw.c, lines 601f.
> wd->ledStruct.ledMode[0] = (u16_t)(rsp[5]&0xffff);
> wd->ledStruct.ledMode[1] = (u16_t)(rsp[5]>>16);
>
> If the bits of ledMode[] are explained in 80211/ledmgr.c, lines 34ff.,
> Atheros provided a generic way for vendors to program LED behaviour
> via the EEPROM and Netgear got some extra handling in the driver
> (if otus is close the windows driver).
nice catch!
On Saturday 03 October 2009 01:03:28 Joerg Albert wrote:
> Hi,
>
> could anyone with a WN111v2 (or any other device with one LED only)
> apply the patch below and look into syslog for the value of "hwtype"?
>
> I get 22212221 for an AVM stick (057c:8401) and 22211111 for a Netgear WNDA3100 (0846:9010).
>
> Thanks,
> Joerg.
>
>
Netgear WNDA3100:
LedMode[0] = 0x1111 => (blue LED)
Bit 0 = 1 => Power-on state: On
Bit 6 = 0 => Connect state
Bit 4 = 1 => always on (~ assoc/link LED?)
Ton = 1
Toff = 1
LedMode[1] = 0x2221 => (orange LED)
Bit 0 = 1 => Power-on state: On
Bit 6 = 0 => Connect state
Bit 5 = 1 => Idle off, acitve on (~ traffic/tx LED?)
Ton = 2
Toff = 2
this looks promising. The setting here does indeed match
the current driver behavior!
AVM:
LedMode[0] = 0x2221 => (red LED?)
Bit 0 = 1 => Power-on state: On
Bit 6 = 0 => Connect state
Bit 5 = 1 => Idle off, acitve on (~ traffic/tx LED?)
Ton = 2
Toff = 2
LedMode[1] = 0x2221 => (green LED?)
Bit 0 = 1 => Power-on state: On
Bit 6 = 0 => Connect state
Bit 5 = 1 => Idle off, acitve on (~ traffic/tx LED?)
Ton = 2
Toff = 2
hmm, by this definition both LEDs are assigned as "traffic indicators".
This can not be right?! I did expect (based on Malte's description of
the Windows driver) something more like: 0x11112221.
Regards,
Chr
On Thu, Oct 1, 2009 at 9:34 PM, Christian Lamparter
<[email protected]> wrote:
> On Thursday 01 October 2009 20:06:35 Hin-Tak Leung wrote:
>> On Thu, Oct 1, 2009 at 3:54 PM, Christian Lamparter
>> <[email protected]> wrote:
>> > On 2009-10-01 06:32 AM, Malte Gell wrote:
>> >>I first noticed the LEDs are confused,
>> > no, the LED colors are not _confused_ at all.
>> > This is simply different on.... well, you know: on a per-device base!
>> >
>> > For example: The Netgear uses a single bi-color LED for their WNDA3100 stick.
>> > It glows blue or/and orange depending on the selected band and current
>> > operation mode and state...
>> >
>> > No idea why they didn't stick to the usual red/green mix.
>> > Maybe because someone told the hw-devs about the existence of
>> > red/green colorblind people??!
>> >
>> > The original vendor driver (located: drivers/staging/otus/80211core/ledmgr.c)
>> > goes to great lengths to describe what's behind the variety of
>> > blinking signals. Which is nice, if you like expensive light shows...
>>
>> This is just based on my brief look at the relevant code itself - I
>> think the driver actually enumerates how many LEDs the device has, so
>> the ONLY_ONE_LED construct is a bit bogus. Also I think the
>> 0x0846:0x9001 is Malte's with swapped LEDs, not ONLY_ONE?
> ?
>
> 0x0846:0x9001 is a Netgear WN111 v2.
> (one LED reference: otus' ledmgr.c ~line 547)
>
> and AFAIK: Malte uses an AVM FRITZ!WLAN Stick N (2.4?).
I just remember the ID from Malte's ndiswrapper-list posting. It is
not unthinkable of a rebranded device, or even vendor abusing the
system a bit and put out a device which is subtly different with the
same ids.
>
>> I had a look when Malte first wrote about the swap - the code
>> basically just do assoc/data-tx in enumeration order (first LED found
>> is assoc, 2nd is tx, which make sense if some variant only have a
>> LED).
> Depends. I think it's more important to have some sort of an "an activity LED"
> than a connection indicator, because most desktops-guis nowadays have lots
> of fancy applets which are constantly monitoring your connection status and
> start to bark as soon as it changes...
>
> BTW: my laptop (with an IWL 5300) only has one LED assigned for wlan as well.
> But, someone had the genius idea to put the activity LED into _inverted_
> mode when the connection is established. It stays on after association
> and flashes under TX activity... This is nice, but it has a downside:
> two trigger _drive_ one LED => no real exclusive access anymore.
> If you want to reassign the LEDs the clueless user has to be aware about
> this trigger dependency, or he see some _blinking interference_.
>
>> As for the color - it is probably just a matter of commercial
>> interests (if they can get a particular color from a source cheaper)
> unlikely, the bi-color LED is a super bright one.
>
>> or simply variety to catch some customers - as you say, some *do* like
>> expensive light shows, and there is a market for it and money to be
>> made that way.
> :-D
>
> well to be fair, I think they tried to implement some sort of
> complex blinking language code. You can tell apart if
> the device is idling/scanning/connected/sending in the 5GHz or 2GHz
> band just by looking at the blue and orange rhythms...
> (maybe this visual feedback is indeed easier to comprehend for the generic
> customer than any hard and honest numbers)
>
> But back to the topic:
> This elaborate morse code is clearly way beyond the scope and
> abilities of ledclass. I think we should really stick to the
> simple rule: one trigger corresponds to only one physical LED.
>
>> Anyway, I am just writing regarding the ONLY_ONE_LED construct and its
>> association with 0x0846:0x9001 ...
> hmm, not sure what I should do here...
> Do you think the driver should simply ignore the lack of a second LED (color)?
> Or is it just that the ONLY_ONE_LED construct sounds really lame
> and needs a more cunning name?
>
> Regards,
> Chr
>
Hmm, I mean the ONLY_ONE_LED config should not be a compiled in config
associated with an vid/pid, but dynamically determined from the
enumeration.
In the case of only one LED, the it sounds quite neat to represent
both assciation status and transmission status by blinking pattern
:-).
But one of them should take priority in case of conflict, and in this
regard it seems that you and I disagree on which should be.
On Friday 02 October 2009 13:45:55 Malte Gell wrote:
> Christian Lamparter <[email protected]> wrote
> > > The Netgear (WN?) 111 even only has one blue LED as far as I know.
> >
> > the question is if it's the only device with this deficit, or not?
> Is it feasable to write to the well known stick makers (Netgear, AVM,
> Belkin, Asus...) and just ask them?
No idea, but from my past experience about Linux support from resellers,
I somehow doubt it... Instead it was always more fruitful to ask around
in various (vendor) support forums for _help_.
Of course, the situation might have improved, so if you're considering
to write to the resellers, then please do! :)
> > > > FYI: you can assign the LEDs under "/sys/class/leds/"
> > > Very good to know. This needs a udev rule, right?
> >
> > yes, should be possible one way or another...
>
> > If you do find an easy ACTION== rule, then let us know!
>
> Do I hear irony? Is it not easy? Isn't it just a link that needs to be made /
> changed? Does it matter, when to do this, e.g. after the module ist loaded or
> the network system is started?
If you take a look at the ledtriggers values in /sys/call/leds/
"none rfkill0 phy0rx phy0tx [phy0assoc] phy0radio",
you instantly see that the phyXname trigger is dynamic.
in my opinion, finding a simple udev rule is harder than simply
modify the driver code :-D.
> > > But, what about a USB ID based decission in the driver how to handle the
> > > LED? I have a Fritz WLAN N, USB ID: 057c:8401
> >
> > The first post had a patch attached:
> > http://patchwork.kernel.org/patch/50977/
>
> Have not tried it, is it well tested?
Hin-Tak Leung has (or had?) some comments about the ONLY_ONE_LED (WN111 v2)
implementation. Other than that, no: I haven't received any horror stories to date.
> If it works, will the patch become official part of compat-wireless?
Sure! But best of all: the change will eventually tickle down into
2.6.x vanilla.
Regards,
Chr
Christian Lamparter <[email protected]> wrote
> On 2009-10-01 06:32 AM, Malte Gell wrote:
> >I first noticed the LEDs are confused,
>
> no, the LED colors is not _confused_ at all.
> This is simply different on.... well, you know: on a per-device base!
Yeah, I guessed it. But, guess... i am not a real hacker so my dirty little
patch just is a trial by combat solution for my own boy ;-)
> For example: The Netgear uses a single bi-color LED for their WNDA3100
> stick. It glows blue or/and orange depending on the selected band and
> current operation mode and state...
The Netgear (WN?) 111 even only has one blue LED as far as I know. And
Neatgear sticks look ugly.
> FYI: you can assign the LEDs under "/sys/class/leds/" with a different
> tigger without messing with the kernel source... An up-to-date README can
> be found in the kernel's documentation directory:
> Documentation/leds-class.txt , or you can look it up online as well:
> http://www.mjmwired.net/kernel/Documentation/leds-class.txt (from 2.6.31)
Very good to know. This needs a udev rule, right?
But, what about a USB ID based decission in the driver how to handle the LED?
I have a Fritz WLAN N, USB ID: 057c:8401
Regards
Malte
On Thu, Oct 1, 2009 at 3:54 PM, Christian Lamparter
<[email protected]> wrote:
> On 2009-10-01 06:32 AM, Malte Gell wrote:
>>I first noticed the LEDs are confused,
> no, the LED colors is not _confused_ at all.
> This is simply different on.... well, you know: on a per-device base!
>
> For example: The Netgear uses a single bi-color LED for their WNDA3100 stick.
> It glows blue or/and orange depending on the selected band and current
> operation mode and state...
>
> No idea why they didn't stick to the usual red/green mix.
> Maybe because someone told the hw-devs about the existence of
> red/green colorblind people??!
>
> The original vendor driver (located: drivers/staging/otus/80211core/ledmgr.c)
> goes to great lengths to describe what's behind the variety of
> blinking signals. Which is nice, if you like expensive light shows...
This is just based on my brief look at the relevant code itself - I
think the driver actually enumerates how many LEDs the device has, so
the ONLY_ONE_LED construct is a bit bogus. Also I think the
0x0846:0x9001 is Malte's with swapped LEDs, not ONLY_ONE?
I had a look when Malte first wrote about the swap - the code
basically just do asoc/data-tx in enumeration order (first LED found
is assoc, 2nd is tx, which make sense if some variant only have a
LED).
As for the color - it is probably just a matter of commercial
interests (if they can get a particular color from a source cheaper)
or simply variety to catch some customers - as you say, some *do* like
expensive light shows, and there is a market for it and money to be
made that way.
Anyway, I am just writing regarding the ONLY_ONE_LED construct and its
association with 0x0846:0x9001 ...
>
>>when I ran my AVM Fritz USBN N under Windows,
>>where the LEDs had the right order. I made a little patch that put
>>the assoc and the tx LEDs in the right order.
> FYI: you can assign the LEDs under "/sys/class/leds/" with a different tigger
> without messing with the kernel source... An up-to-date README can be found in
> the kernel's documentation directory: Documentation/leds-class.txt ,
> or you can look it up online as well:
> http://www.mjmwired.net/kernel/Documentation/leds-class.txt (from 2.6.31)
>
> but back to the patch and the problem with the wide diversity of
> over-customized solutions for a direct feedback mechanism to the
> mindful human operator...
>
> what about the (inline) _attached_ approach?
> Sure, this idea needs some more code... but, it covers all/most
> possible scenarios from beloved "no, no, no" vendors.
> ---
> From: Christian Lamparter <[email protected]>
> Subject: [PATCH] ar9170usb: flexible LED mapping
>
> This patch adds two more quirk flags which are useful to:
>
> - reduce the number of virtual/ghost LEDs
> ( for low-budget devices: Netgear WN111 v2 )
> - select an alternative LED mapping.
> ( for AVM FRITZ!WLAN USB Stick N )
>
> Reported-by: Malte Gell <[email protected]>
> Signed-off-by: Christian Lamparter <[email protected]>
> ---
> diff --git a/drivers/net/wireless/ath/ar9170/ar9170.h b/drivers/net/wireless/ath/ar9170/ar9170.h
> index ec034af..a451bb1 100644
> --- a/drivers/net/wireless/ath/ar9170/ar9170.h
> +++ b/drivers/net/wireless/ath/ar9170/ar9170.h
> @@ -155,6 +155,12 @@ struct ar9170_sta_tid {
> #define AR9170_NUM_TX_STATUS 128
> #define AR9170_NUM_TX_AGG_MAX 30
>
> +enum ar9170_quirks {
> + AR9170_REQ_FW1_ONLY = BIT(0),
> + AR9170_ONLY_ONE_LED = BIT(1),
> + AR9170_SWAPPED_LEDS = BIT(2),
> +};
> +
> struct ar9170 {
> struct ieee80211_hw *hw;
> struct ath_common common;
> @@ -241,6 +247,9 @@ struct ar9170 {
> /* (cached) HW A-MPDU settings */
> u8 global_ampdu_density;
> u8 global_ampdu_factor;
> +
> + /* device quirks */
> + unsigned long quirks;
> };
>
> struct ar9170_sta_info {
> diff --git a/drivers/net/wireless/ath/ar9170/led.c b/drivers/net/wireless/ath/ar9170/led.c
> index 86c4e79..36ab738 100644
> --- a/drivers/net/wireless/ath/ar9170/led.c
> +++ b/drivers/net/wireless/ath/ar9170/led.c
> @@ -155,18 +155,30 @@ void ar9170_unregister_leds(struct ar9170 *ar)
> cancel_delayed_work_sync(&ar->led_work);
> }
>
> +const static int std_led_map[AR9170_NUM_LEDS] = { 0, 1 };
> +const static int avm_led_map[AR9170_NUM_LEDS] = { 1, 0 };
> +
> int ar9170_register_leds(struct ar9170 *ar)
> {
> + const int *led_map;
> int err;
>
> + if (ar->quirks & AR9170_SWAPPED_LEDS)
> + led_map = avm_led_map;
> + else
> + led_map = std_led_map;
> +
> INIT_DELAYED_WORK(&ar->led_work, ar9170_update_leds);
>
> - err = ar9170_register_led(ar, 0, "tx",
> + err = ar9170_register_led(ar, led_map[0], "tx",
> ieee80211_get_tx_led_name(ar->hw));
> if (err)
> goto fail;
>
> - err = ar9170_register_led(ar, 1, "assoc",
> + if (ar->quirks & AR9170_ONLY_ONE_LED)
> + return 0;
> +
> + err = ar9170_register_led(ar, led_map[1], "assoc",
> ieee80211_get_assoc_led_name(ar->hw));
> if (err)
> goto fail;
> diff --git a/drivers/net/wireless/ath/ar9170/usb.c b/drivers/net/wireless/ath/ar9170/usb.c
> index e974e58..3be19e4 100644
> --- a/drivers/net/wireless/ath/ar9170/usb.c
> +++ b/drivers/net/wireless/ath/ar9170/usb.c
> @@ -55,10 +55,6 @@ MODULE_FIRMWARE("ar9170.fw");
> MODULE_FIRMWARE("ar9170-1.fw");
> MODULE_FIRMWARE("ar9170-2.fw");
>
> -enum ar9170_requirements {
> - AR9170_REQ_FW1_ONLY = 1,
> -};
> -
> static struct usb_device_id ar9170_usb_ids[] = {
> /* Atheros 9170 */
> { USB_DEVICE(0x0cf3, 0x9170) },
> @@ -73,7 +69,7 @@ static struct usb_device_id ar9170_usb_ids[] = {
> /* Netgear WNDA3100 */
> { USB_DEVICE(0x0846, 0x9010) },
> /* Netgear WN111 v2 */
> - { USB_DEVICE(0x0846, 0x9001) },
> + { USB_DEVICE(0x0846, 0x9001), .driver_info = AR9170_ONLY_ONE_LED },
> /* Zydas ZD1221 */
> { USB_DEVICE(0x0ace, 0x1221) },
> /* ZyXEL NWD271N */
> @@ -89,7 +85,7 @@ static struct usb_device_id ar9170_usb_ids[] = {
> /* IO-Data WNGDNUS2 */
> { USB_DEVICE(0x04bb, 0x093f) },
> /* AVM FRITZ!WLAN USB Stick N */
> - { USB_DEVICE(0x057C, 0x8401) },
> + { USB_DEVICE(0x057C, 0x8401), .driver_info = AR9170_SWAPPED_LEDS },
> /* AVM FRITZ!WLAN USB Stick N 2.4 */
> { USB_DEVICE(0x057C, 0x8402), .driver_info = AR9170_REQ_FW1_ONLY },
>
> @@ -589,7 +585,7 @@ static int ar9170_usb_request_firmware(struct ar9170_usb *aru)
> return 0;
> }
>
> - if (aru->req_one_stage_fw) {
> + if (aru->common.quirks & AR9170_REQ_FW1_ONLY) {
> dev_err(&aru->udev->dev, "ar9170.fw firmware file "
> "not found and is required for this device\n");
> return -EINVAL;
> @@ -753,15 +749,6 @@ err_out:
> return err;
> }
>
> -static bool ar9170_requires_one_stage(const struct usb_device_id *id)
> -{
> - if (!id->driver_info)
> - return false;
> - if (id->driver_info == AR9170_REQ_FW1_ONLY)
> - return true;
> - return false;
> -}
> -
> static int ar9170_usb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> @@ -781,8 +768,7 @@ static int ar9170_usb_probe(struct usb_interface *intf,
> aru->udev = udev;
> aru->intf = intf;
> ar = &aru->common;
> -
> - aru->req_one_stage_fw = ar9170_requires_one_stage(id);
> + ar->quirks = id->driver_info;
>
> usb_set_intfdata(intf, aru);
> SET_IEEE80211_DEV(ar->hw, &intf->dev);
> diff --git a/drivers/net/wireless/ath/ar9170/usb.h b/drivers/net/wireless/ath/ar9170/usb.h
> index d098f4d..714436d 100644
> --- a/drivers/net/wireless/ath/ar9170/usb.h
> +++ b/drivers/net/wireless/ath/ar9170/usb.h
> @@ -64,8 +64,6 @@ struct ar9170_usb {
> struct usb_anchor tx_pending;
> struct usb_anchor tx_submitted;
>
> - bool req_one_stage_fw;
> -
> spinlock_t tx_urb_lock;
> unsigned int tx_submitted_urbs;
> unsigned int tx_pending_urbs;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Saturday 03 October 2009 04:53:41 Malte Gell wrote:
> Christian Lamparter <[email protected]> wrote
> > If you take a look at the ledtriggers values in /sys/call/leds/
> > "none rfkill0 phy0rx phy0tx [phy0assoc] phy0radio",
> > you instantly see that the phyXname trigger is dynamic.
>
> Can I change the link(s) there at any time
yes, you can change then any time you want.
> or will the ar9170usb change it again?
no, the setting stays. But of course,
it will be gone if the module is reloaded, or
the device has been replugged.
> > in my opinion, finding a simple udev rule is harder than simply
> > modify the driver code :-D.
>
> That's true! I just had to exchange the words "tx" with "assoc"
> in the driver, whereas a udev rule is complicated!
well, there's at least one upside: this rule could be easily
modified to work with different wireless drivers.
by the way: Could you please do me a flavour (since you are
the only WN111 v2 owner I know) and tell us what "hwtype"
result you get with Joerg Albert's patch:
http://marc.info/?l=linux-wireless&m=125452461322729&q=raw
Regards,
Chr
On Thursday 01 October 2009 20:06:35 Hin-Tak Leung wrote:
> On Thu, Oct 1, 2009 at 3:54 PM, Christian Lamparter
> <[email protected]> wrote:
> > On 2009-10-01 06:32 AM, Malte Gell wrote:
> >>I first noticed the LEDs are confused,
> > no, the LED colors are not _confused_ at all.
> > This is simply different on.... well, you know: on a per-device base!
> >
> > For example: The Netgear uses a single bi-color LED for their WNDA3100 stick.
> > It glows blue or/and orange depending on the selected band and current
> > operation mode and state...
> >
> > No idea why they didn't stick to the usual red/green mix.
> > Maybe because someone told the hw-devs about the existence of
> > red/green colorblind people??!
> >
> > The original vendor driver (located: drivers/staging/otus/80211core/ledmgr.c)
> > goes to great lengths to describe what's behind the variety of
> > blinking signals. Which is nice, if you like expensive light shows...
>
> This is just based on my brief look at the relevant code itself - I
> think the driver actually enumerates how many LEDs the device has, so
> the ONLY_ONE_LED construct is a bit bogus. Also I think the
> 0x0846:0x9001 is Malte's with swapped LEDs, not ONLY_ONE?
?
0x0846:0x9001 is a Netgear WN111 v2.
(one LED reference: otus' ledmgr.c ~line 547)
and AFAIK: Malte uses an AVM FRITZ!WLAN Stick N (2.4?).
> I had a look when Malte first wrote about the swap - the code
> basically just do assoc/data-tx in enumeration order (first LED found
> is assoc, 2nd is tx, which make sense if some variant only have a
> LED).
Depends. I think it's more important to have some sort of an "an activity LED"
than a connection indicator, because most desktops-guis nowadays have lots
of fancy applets which are constantly monitoring your connection status and
start to bark as soon as it changes...
BTW: my laptop (with an IWL 5300) only has one LED assigned for wlan as well.
But, someone had the genius idea to put the activity LED into _inverted_
mode when the connection is established. It stays on after association
and flashes under TX activity... This is nice, but it has a downside:
two trigger _drive_ one LED => no real exclusive access anymore.
If you want to reassign the LEDs the clueless user has to be aware about
this trigger dependency, or he see some _blinking interference_.
> As for the color - it is probably just a matter of commercial
> interests (if they can get a particular color from a source cheaper)
unlikely, the bi-color LED is a super bright one.
> or simply variety to catch some customers - as you say, some *do* like
> expensive light shows, and there is a market for it and money to be
> made that way.
:-D
well to be fair, I think they tried to implement some sort of
complex blinking language code. You can tell apart if
the device is idling/scanning/connected/sending in the 5GHz or 2GHz
band just by looking at the blue and orange rhythms...
(maybe this visual feedback is indeed easier to comprehend for the generic
customer than any hard and honest numbers)
But back to the topic:
This elaborate morse code is clearly way beyond the scope and
abilities of ledclass. I think we should really stick to the
simple rule: one trigger corresponds to only one physical LED.
> Anyway, I am just writing regarding the ONLY_ONE_LED construct and its
> association with 0x0846:0x9001 ...
hmm, not sure what I should do here...
Do you think the driver should simply ignore the lack of a second LED (color)?
Or is it just that the ONLY_ONE_LED construct sounds really lame
and needs a more cunning name?
Regards,
Chr