The following patch implements some control over the LED on RTL8187B and
RTL8187L devices. Triggers are registered for TX and RX. Whenever the
trigger event occurs, the LED is turned off for 1/20 second, then turned
back on.
Please test and comment.
Signed-off-by: Larry Finger <[email protected]>
---
Index: wireless-testing/drivers/net/wireless/Kconfig
===================================================================
--- wireless-testing.orig/drivers/net/wireless/Kconfig
+++ wireless-testing/drivers/net/wireless/Kconfig
@@ -433,6 +433,13 @@ config RTL8187
Thanks to Realtek for their support!
+# If possible, automatically enable LED's for RTL8187.
+
+config RTL8187_LEDS
+ bool
+ depends on RTL8187 && MAC80211_LEDS && (LEDS_CLASS = y || LEDS_CLASS = RTL8187)
+ default y
+
config ADM8211
tristate "ADMtek ADM8211 support"
depends on MAC80211 && PCI && WLAN_80211 && EXPERIMENTAL
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -29,6 +29,9 @@
#include "rtl8187.h"
#include "rtl8187_rtl8225.h"
+#ifdef CONFIG_RTL8187_LEDS
+#include "rtl8187_leds.h"
+#endif
MODULE_AUTHOR("Michael Wu <[email protected]>");
MODULE_AUTHOR("Andrea Merello <[email protected]>");
@@ -1506,6 +1509,10 @@ static int __devinit rtl8187_probe(struc
wiphy_name(dev->wiphy), dev->wiphy->perm_addr,
chip_name, priv->asic_rev, priv->rf->name);
+#ifdef CONFIG_RTL8187_LEDS
+ rtl8187_leds_init(dev, eeprom);
+#endif
+
return 0;
err_free_dev:
@@ -1523,6 +1530,9 @@ static void __devexit rtl8187_disconnect
if (!dev)
return;
+#ifdef CONFIG_RTL8187_LEDS
+ rtl8187_leds_exit(dev);
+#endif
ieee80211_unregister_hw(dev);
priv = dev->priv;
Index: wireless-testing/drivers/net/wireless/rtl818x/Makefile
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/Makefile
+++ wireless-testing/drivers/net/wireless/rtl818x/Makefile
@@ -1,5 +1,5 @@
rtl8180-objs := rtl8180_dev.o rtl8180_rtl8225.o rtl8180_sa2400.o rtl8180_max2820.o rtl8180_grf5101.o
-rtl8187-objs := rtl8187_dev.o rtl8187_rtl8225.o
+rtl8187-objs := rtl8187_dev.o rtl8187_rtl8225.o rtl8187_leds.o
obj-$(CONFIG_RTL8180) += rtl8180.o
obj-$(CONFIG_RTL8187) += rtl8187.o
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
===================================================================
--- /dev/null
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
@@ -0,0 +1,180 @@
+/*
+ * Linux LED driver for RTL8187
+ *
+ * Copyright 2009 Larry Finger <[email protected]>
+ *
+ * Based on the r8187 driver, which is:
+ * Copyright 2005 Andrea Merello <[email protected]>, et al.
+ *
+ * Thanks to Realtek for their support!
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifdef CONFIG_RTL8187_LEDS
+
+#include <net/mac80211.h>
+#include <linux/usb.h>
+#include <linux/eeprom_93cx6.h>
+
+#include "rtl8187.h"
+#include "rtl8187_leds.h"
+
+static void led_turn_on(struct work_struct *work)
+{
+ u8 reg;
+ struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
+ led_on.work);
+ struct rtl8187_led *led = &priv->led_tx;
+
+ switch (led->ledpin) {
+ case LED_PIN_GPIO0:
+ rtl818x_iowrite8(priv, &priv->map->GPIO, 0x01);
+ rtl818x_iowrite8(priv, &priv->map->GP_ENABLE, 0x00);
+ break;
+ case LED_PIN_LED0:
+ reg = rtl818x_ioread8(priv, &priv->map->PGSELECT) & ~(1 << 4);
+ rtl818x_iowrite8(priv, &priv->map->PGSELECT, reg);
+ break;
+ case LED_PIN_LED1:
+ reg = rtl818x_ioread8(priv, &priv->map->PGSELECT) & ~(1 << 5);
+ rtl818x_iowrite8(priv, &priv->map->PGSELECT, reg);
+ break;
+ case LED_PIN_HW:
+ default:
+ break;
+ }
+}
+
+static void led_turn_off(struct work_struct *work)
+{
+ u8 reg;
+ struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
+ led_off.work);
+ struct rtl8187_led *led = &priv->led_tx;
+
+ switch (led->ledpin) {
+ case LED_PIN_GPIO0:
+ rtl818x_iowrite8(priv, &priv->map->GPIO, 0x01);
+ rtl818x_iowrite8(priv, &priv->map->GP_ENABLE, 0x01);
+ break;
+ case LED_PIN_LED0:
+ reg = rtl818x_ioread8(priv, &priv->map->PGSELECT) | (1 << 4);
+ rtl818x_iowrite8(priv, &priv->map->PGSELECT, reg);
+ break;
+ case LED_PIN_LED1:
+ reg = rtl818x_ioread8(priv, &priv->map->PGSELECT) | (1 << 5);
+ rtl818x_iowrite8(priv, &priv->map->PGSELECT, reg);
+ break;
+ case LED_PIN_HW:
+ default:
+ break;
+ }
+ queue_delayed_work(priv->dev->workqueue, &priv->led_on, HZ / 20);
+}
+
+/* Callback from the LED subsystem. */
+static void rtl8187_led_brightness_set(struct led_classdev *led_dev,
+ enum led_brightness brightness)
+{
+ struct rtl8187_led *led = container_of(led_dev, struct rtl8187_led, led_dev);
+ struct ieee80211_hw *hw = led->dev;
+ struct rtl8187_priv *priv = hw->priv;
+
+ if (brightness == LED_OFF)
+ queue_delayed_work(hw->workqueue, &priv->led_off, 0);
+ else
+ queue_delayed_work(hw->workqueue, &priv->led_on, 0);
+}
+
+static int rtl8187_register_led(struct ieee80211_hw *dev, struct rtl8187_led *led,
+ const char *name, const char *default_trigger, u8 ledpin)
+{
+ int err;
+ struct rtl8187_priv *priv = dev->priv;
+
+ if (led->dev)
+ return -EEXIST;
+ if (!default_trigger)
+ return -EINVAL;
+ led->dev = dev;
+ led->ledpin = ledpin;
+ strncpy(led->name, name, sizeof(led->name));
+
+ led->led_dev.name = led->name;
+ led->led_dev.default_trigger = default_trigger;
+ led->led_dev.brightness_set = rtl8187_led_brightness_set;
+
+ err = led_classdev_register(&priv->udev->dev, &led->led_dev);
+ if (err) {
+ printk(KERN_INFO "LEDs: Failed to register %s\n", name);
+ led->dev = NULL;
+ return err;
+ }
+ return 0;
+}
+
+static void rtl8187_unregister_led(struct rtl8187_led *led)
+{
+ if (!led->dev)
+ return;
+ led_classdev_unregister(&led->led_dev);
+ led->dev = NULL;
+}
+
+void rtl8187_leds_init(struct ieee80211_hw *dev, struct eeprom_93cx6 eeprom)
+{
+ struct rtl8187_priv *priv = dev->priv;
+ char name[RTL8187_LED_MAX_NAME_LEN + 1];
+ u16 reg;
+ u8 ledpin;
+
+ eeprom_93cx6_read(&eeprom, 0x3F, ®);
+ printk(KERN_INFO "rtl8187: Customer ID 0x%X\n", reg & 0xFF);
+ /* According to the vendor driver, the LED operation depends on the
+ * customer ID encoded in the EEPROM
+ */
+ switch (reg) {
+ case EEPROM_CID_RSVD0:
+ case EEPROM_CID_RSVD1:
+ case EEPROM_CID_SERCOMM_PS:
+ case EEPROM_CID_QMI:
+ case EEPROM_CID_DELL:
+ ledpin = LED_PIN_GPIO0;
+ break;
+ case EEPROM_CID_ALPHA0:
+ ledpin = LED_PIN_LED0;
+ break;
+ case EEPROM_CID_HW:
+ ledpin = LED_PIN_HW;
+ break;
+ default:
+ ledpin = LED_PIN_GPIO0;
+ }
+
+ INIT_DELAYED_WORK(&priv->led_on, led_turn_on);
+ INIT_DELAYED_WORK(&priv->led_off, led_turn_off);
+
+ snprintf(name, sizeof(name),
+ "rtl8187-%s::tx", wiphy_name(dev->wiphy));
+ rtl8187_register_led(dev, &priv->led_tx, name,
+ ieee80211_get_tx_led_name(dev), ledpin);
+ snprintf(name, sizeof(name),
+ "rtl8187-%s::rx", wiphy_name(dev->wiphy));
+ rtl8187_register_led(dev, &priv->led_rx, name,
+ ieee80211_get_rx_led_name(dev), ledpin);
+}
+
+void rtl8187_leds_exit(struct ieee80211_hw *dev)
+{
+ struct rtl8187_priv *priv = dev->priv;
+
+ cancel_delayed_work(&priv->led_on);
+ cancel_delayed_work(&priv->led_off);
+ rtl8187_unregister_led(&priv->led_tx);
+ rtl8187_unregister_led(&priv->led_rx);
+}
+#endif /* def CONFIG_RTL8187_LED */
+
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.h
===================================================================
--- /dev/null
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.h
@@ -0,0 +1,56 @@
+/*
+ * Definitions for RTL8187 leds
+ *
+ * Copyright 2009 Larry Finger <[email protected]>
+ *
+ * Based on the r8187 driver, which is:
+ * Copyright 2005 Andrea Merello <[email protected]>, et al.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef RTL8187_LED_H
+#define RTL8187_LED_H
+
+#ifdef CONFIG_RTL8187_LEDS
+
+#define RTL8187_LED_MAX_NAME_LEN 21
+
+#include <linux/leds.h>
+#include <linux/types.h>
+
+enum {
+ LED_PIN_LED0,
+ LED_PIN_LED1,
+ LED_PIN_GPIO0,
+ LED_PIN_HW
+};
+
+enum {
+ EEPROM_CID_RSVD0 = 0x00,
+ EEPROM_CID_RSVD1 = 0xFF,
+ EEPROM_CID_ALPHA0 = 0x01,
+ EEPROM_CID_SERCOMM_PS = 0x02,
+ EEPROM_CID_HW = 0x03,
+ EEPROM_CID_QMI = 0x07,
+ EEPROM_CID_DELL = 0x08
+};
+
+struct rtl8187_led {
+ struct ieee80211_hw *dev;
+ /* The LED class device */
+ struct led_classdev led_dev;
+ /* The pin/method used to control the led */
+ u8 ledpin;
+ /* The unique name string for this LED device. */
+ char name[RTL8187_LED_MAX_NAME_LEN + 1];
+};
+
+void rtl8187_leds_init(struct ieee80211_hw *dev, struct eeprom_93cx6 eeprom);
+void rtl8187_leds_exit(struct ieee80211_hw *dev);
+
+#endif /* def CONFIG_RTL8187_LED */
+
+#endif /* RTL8187_LED_H */
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187.h
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
@@ -16,6 +16,7 @@
#define RTL8187_H
#include "rtl818x.h"
+#include "rtl8187_leds.h"
#define RTL8187_EEPROM_TXPWR_BASE 0x05
#define RTL8187_EEPROM_MAC_ADDR 0x07
@@ -102,6 +103,12 @@ struct rtl8187_priv {
struct usb_anchor anchored;
struct delayed_work work;
struct ieee80211_hw *dev;
+#ifdef CONFIG_RTL8187_LEDS
+ struct rtl8187_led led_tx;
+ struct rtl8187_led led_rx;
+ struct delayed_work led_on;
+ struct delayed_work led_off;
+#endif
u16 txpwr_base;
u8 asic_rev;
u8 is_rtl8187b;
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_rtl8225.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_rtl8225.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_rtl8225.c
@@ -18,6 +18,7 @@
#include <linux/init.h>
#include <linux/usb.h>
+#include <linux/eeprom_93cx6.h>
#include <net/mac80211.h>
#include "rtl8187.h"
On Thu, Apr 9, 2009 at 6:23 AM, Larry Finger <[email protected]=
> wrote:
> The following patch implements some control over the LED on RTL8187B =
and
> RTL8187L devices. Triggers are registered for TX and RX. Whenever the
> trigger event occurs, the LED is turned off for 1/20 second, then tur=
ned
> back on.
>
> Please test and comment.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> Index: wireless-testing/drivers/net/wireless/Kconfig
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- wireless-testing.orig/drivers/net/wireless/Kconfig
> +++ wireless-testing/drivers/net/wireless/Kconfig
> @@ -433,6 +433,13 @@ config RTL8187
>
> =A0 =A0 =A0 =A0 =A0Thanks to Realtek for their support!
>
> +# If possible, automatically enable LED's for RTL8187.
Spelling error: "LED's" should be "LEDs".
> +
> +config RTL8187_LEDS
> + =A0 =A0 =A0 bool
> + =A0 =A0 =A0 depends on RTL8187 && MAC80211_LEDS && (LEDS_CLASS =3D =
y || LEDS_CLASS =3D RTL8187)
> + =A0 =A0 =A0 default y
> +
> =A0config ADM8211
> =A0 =A0 =A0 =A0tristate "ADMtek ADM8211 support"
> =A0 =A0 =A0 =A0depends on MAC80211 && PCI && WLAN_80211 && EXPERIMENT=
AL
--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
On Sun, Apr 12, 2009 at 6:44 AM, Larry Finger <[email protected]> wrote:
> It is not user configurable. The revised Kconfig selects it automatically if
> MAC80211_LEDS is set and LEDS_CLASS is y or m. The user never sees it in the
> configuration process.
That's great and that's how I think it should work.
> Thanks for the heads up. I have not been able to suspend my computer due to
> other problems, but I'll see what I can find.
My guess is that it is probably related to resource to do with the new
work-queue.
> On another subject, I have been working with the vendor driver from
> rtl8187B_linux_26.1036.0708.2008. I stripped out a lot of the dead code and made
> the modifications so that it compiles cleanly with kernel 2.6.30-rc1. I also
> modified it to handle my RTL8187B stick with the 8187 ID. The driver scans OK,
> but so far it has not associated with my WPA2 AP. My hope is to get it running
> so that I can compare throughput with the current Linux driver.
That's quite some work! I wouldn't mind seeing the diff, or even the
incremental diffs, if you have done it in git.
I had an old patch for beyond 2.6.28 but never got it to work, I
think. Have some problem with porting forward another vendor driver
(the zd1211) and can certainly benefit from some tips.
Hin-Tak Leung wrote:
> On Thu, Apr 9, 2009 at 5:23 AM, Larry Finger <[email protected]> wrote:
>
> On the LED functionality - the LED on my laptop actually works rather
> differently under windows vs under linux.
> On windows it is tied to the hardware switch, and the windows driver
> depends on the state of the hardware switch.
> The linux driver's behavior has no relationship with the LED what so
> ever. (I suppose it might blink under windows but I don't know for
> sure).
I take it that your device is built in. If it works with the hardware switch,
then we need a "radio" LED and the rfkill subsystem. I won't be able to test
that configuration, but I will try to spin that code, but I'll do it separately
from the TX/RX LEDs.
>> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.h
>> ===================================================================
>
>> + * Based on the r8187 driver, which is:
>> + * Copyright 2005 Andrea Merello <[email protected]>, et al.
>
> I found these two comments a bit odd, so I went back to the vendor
> driver code to have a look.
> The LED code is AFAIK a rather new addition to the vendor driver,
> within the 2008, I think. It is in
> 0708.2008 but not in 0822.2007, and does not seem to be in 0125.2008
> driver either.
>
> The two new LED related files in 0708.2008 have these headers:
> --------------------------------------------------------------------------------------------------
> /*++
> Copyright (c) Realtek Semiconductor Corp. All rights reserved.
>
> Module Name:
> r8187_led.c
>
> Abstract:
> RTL8187 LED control functions
>
> Major Change History:
> When Who What
> ---------- --------------- -------------------------------
> 2006-09-07 Xiong Created
>
> Notes:
>
> --*/
> /*++
>
> Copyright (c) Microsoft Corporation. All rights reserved.
>
> Module Name:
> r8187_led.h
>
> Abstract:
> definitions and stuctures for rtl8187 led control.
>
> Major Change History:
> When Who What
> ---------- ------ ----------------------------------------------
> 2006-09-07 Xiong Created
>
> Notes:
>
> --*/
> -------------------------------------------------------------------------------------------------
>
> I am slightly worried about the microsoft copyright in the latter :-),
> but presumably it is a visual studio or dev tool artefact :-).
I used the rtl8187 code from rtl8187_linux_26.1025.0328.2007, and the rtl8187B
code from rtl8187B_linux_24.6.1031.0125.2008. It turned out that the code was
identical, thus no need to have two distinct paths. As you have likely seen, my
implementation was a lot smaller than theirs.
I used the rtl8187_led.c file to generate my code. It only has a Realtek
copyright. The only part of the .h file that was used was the interpretation of
the EEPROM customer code. I don't think that Microsoft has a copyright on that.
>> #include <linux/init.h>
>> #include <linux/usb.h>
>> +#include <linux/eeprom_93cx6.h>
>> #include <net/mac80211.h>
>>
>> #include "rtl8187.h"
>
> This trunk seems to be redundant - why would a new include be needed
> for no code addition?
In my implementation of the LEDs init function, I pass the EEPROM structure as
one of the arguments so that I can read the customer code, which is the reason
that that header ended up there. I'll modify my patch so that the customer code
is read in rtl8187_dev.c and pass that to the init routine. Then the EEPROM
struct will not be needed.
Thanks for your comments.
Larry
On Thu, Apr 9, 2009 at 5:23 AM, Larry Finger <[email protected]=
> wrote:
> The following patch implements some control over the LED on RTL8187B =
and
> RTL8187L devices. Triggers are registered for TX and RX. Whenever the
> trigger event occurs, the LED is turned off for 1/20 second, then tur=
ned
> back on.
On the LED functionality - the LED on my laptop actually works rather
differently under windows vs under linux.
On windows it is tied to the hardware switch, and the windows driver
depends on the state of the hardware switch.
The linux driver's behavior has no relationship with the LED what so
ever. (I suppose it might blink under windows but I don't know for
sure).
> Please test and comment.
I'll have a little go at it later, but a few comments below.
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- /dev/null
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> + * Based on the r8187 driver, which is:
> + * Copyright 2005 Andrea Merello <[email protected]>, et al.
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> + * Based on the r8187 driver, which is:
> + * Copyright 2005 Andrea Merello <[email protected]>, et al.
I found these two comments a bit odd, so I went back to the vendor
driver code to have a look.
The LED code is AFAIK a rather new addition to the vendor driver,
within the 2008, I think. It is in
0708.2008 but not in 0822.2007, and does not seem to be in 0125.2008
driver either.
The two new LED related files in 0708.2008 have these headers:
-----------------------------------------------------------------------=
---------------------------
/*++
Copyright (c) Realtek Semiconductor Corp. All rights reserved.
Module Name:
r8187_led.c
=09
Abstract:
RTL8187 LED control functions
=09
Major Change History:
When Who What
---------- --------------- -------------------------------
2006-09-07 Xiong Created
Notes:=09
=09
--*/
/*++
Copyright (c) Microsoft Corporation. All rights reserved.
Module Name:
r8187_led.h
Abstract:
definitions and stuctures for rtl8187 led control.
Major Change History:
When Who What
---------- ------ ----------------------------------------------
2006-09-07 Xiong Created
Notes:
--*/
-----------------------------------------------------------------------=
--------------------------
I am slightly worried about the microsoft copyright in the latter :-),
but presumably it is a visual studio or dev tool artefact :-).
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_rtl822=
5.c
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_rtl8225.c
> @@ -18,6 +18,7 @@
>
> =A0#include <linux/init.h>
> =A0#include <linux/usb.h>
> +#include <linux/eeprom_93cx6.h>
> =A0#include <net/mac80211.h>
>
> =A0#include "rtl8187.h"
This trunk seems to be redundant - why would a new include be needed
for no code addition?
On Sat, Apr 11, 2009 at 5:23 AM, Hin-Tak Leung <[email protected]> wrote:
> I just gave your patch against compat-wireless, and there isn't any
> noticeable effect other than
> these three lines in dmesg - which at least shows that the new code is active:
Oh, I forgot to mention that to test this code against
compat-wireless, appending "CONFIG_RTL8187_LEDS=y" to config.mk does
the job.
I am wonder if we actually needs an extra Kconfig for this - at least
in my case, the code has no ill effect with device which hasn't got an
RX/TX LED. (other than possibly a small theoretical performance loss
from do extra non-useful work).
On Sat, Apr 11, 2009 at 5:36 PM, Larry Finger <[email protected]> wrote:
> Hin-Tak Leung wrote:
>> On Sat, Apr 11, 2009 at 5:23 AM, Hin-Tak Leung <[email protected]> wrote:
>>
>> Oh, I forgot to mention that to test this code against
>> compat-wireless, appending "CONFIG_RTL8187_LEDS=y" to config.mk does
>> the job.
>>
>> I am wonder if we actually needs an extra Kconfig for this - at least
>> in my case, the code has no ill effect with device which hasn't got an
>> RX/TX LED. (other than possibly a small theoretical performance loss
>> from do extra non-useful work).
>
> I do have to compile the LED code only if the kernel has support for them,
> otherwise the registration entry points would be missing. The necessary
> conditions would be more complicated, thus I put them in one place in the
> Kconfig and let the source only be dependent on CONFIG_RTL8187_LEDS.
Thanks for the explanation. Depending (silently) on other kernel
requirements is certainly fine.
I am just thinking we can keep it simple, and wondering if we need
another user-configurable option.
BTW, I had a hard lock-up on suspend - it hasn't happened for
months... while the module doesn't suspend properly, I have
unload-on-sleep configured and it has work mostly fine for some
months. The vendor code is quite lacking in suspend/resume support,
and if you are going to prepare another iteration of the LED patch,
please give this a thought.
on possible problem with this.
On Fri, Apr 10, 2009 at 12:01 AM, Larry Finger
<[email protected]> wrote:
> I take it that your device is built in. If it works with the hardware switch,
> then we need a "radio" LED and the rfkill subsystem. I won't be able to test
> that configuration, but I will try to spin that code, but I'll do it separately
> from the TX/RX LEDs.
I just gave your patch against compat-wireless, and there isn't any
noticeable effect other than
these three lines in dmesg - which at least shows that the new code is active:
rtl8187: Customer ID 0x4
Registered led device: rtl8187-phy0::tx
Registered led device: rtl8187-phy0::rx
Since I don't have the Tx/Rx LEDs (my device is
OEM/on-board/built-in), this is expected.
Also, note that my Customer is one of invalid value that's not
documented - I guess you can add a comment in
with 0x04 is found on built-in OEM device on toshiba laptops, just for
completeness.
One day we might want to distinguish 0x4 from the default treatment,
you never know.
Oh, also can you change that from 0x%X to 0x%02X ? I just like hex
numbers to have even number of digits.
Actually that code is slightly ambiguous/misleading - the printfk
customer id is bit-masked for the lower 8-bit, but the actual decision
itself is based on the full 16-bit value read off the register. It is
possible for the dmesg customer id to show one of the special values
in dmesg and yet and code path goes to the default: case, due to
non-zero higher 8-bit.
Hmm, I went back to check the vendor driver code, its decision is
based on the lower 8-bit only, so you should either do a reg &=0xFF
right after read, or do "switch(reg &0xFF) {}", just in case there is
garbage in the upper 8-bit...
Hin-Tak Leung wrote:
>
> Thanks for the explanation. Depending (silently) on other kernel
> requirements is certainly fine.
> I am just thinking we can keep it simple, and wondering if we need
> another user-configurable option.
It is not user configurable. The revised Kconfig selects it automatically if
MAC80211_LEDS is set and LEDS_CLASS is y or m. The user never sees it in the
configuration process.
> BTW, I had a hard lock-up on suspend - it hasn't happened for
> months... while the module doesn't suspend properly, I have
> unload-on-sleep configured and it has work mostly fine for some
> months. The vendor code is quite lacking in suspend/resume support,
> and if you are going to prepare another iteration of the LED patch,
> please give this a thought.
> on possible problem with this.
Thanks for the heads up. I have not been able to suspend my computer due to
other problems, but I'll see what I can find.
On another subject, I have been working with the vendor driver from
rtl8187B_linux_26.1036.0708.2008. I stripped out a lot of the dead code and made
the modifications so that it compiles cleanly with kernel 2.6.30-rc1. I also
modified it to handle my RTL8187B stick with the 8187 ID. The driver scans OK,
but so far it has not associated with my WPA2 AP. My hope is to get it running
so that I can compare throughput with the current Linux driver.
Larry
Hin-Tak Leung wrote:
> On Sat, Apr 11, 2009 at 5:23 AM, Hin-Tak Leung <[email protected]> wrote:
>
> Oh, I forgot to mention that to test this code against
> compat-wireless, appending "CONFIG_RTL8187_LEDS=y" to config.mk does
> the job.
>
> I am wonder if we actually needs an extra Kconfig for this - at least
> in my case, the code has no ill effect with device which hasn't got an
> RX/TX LED. (other than possibly a small theoretical performance loss
> from do extra non-useful work).
I do have to compile the LED code only if the kernel has support for them,
otherwise the registration entry points would be missing. The necessary
conditions would be more complicated, thus I put them in one place in the
Kconfig and let the source only be dependent on CONFIG_RTL8187_LEDS.
Larry