Search Linux Wireless

Re: [PATCH] ar9170usb: LEDs are confused

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 1, 2009 at 3:54 PM, Christian Lamparter
<chunkeey@xxxxxxxxxxxxxx> 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 <chunkeey@xxxxxxxxxxxxxx>
> 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 <malte.gell@xxxxxx>
> Signed-off-by: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux