2009-04-16 18:22:58

by Larry Finger

[permalink] [raw]
Subject: [RFT/RFC V4] rtl8187: Implement TX/RX blink for LED

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.

Note: For those RTL8187X devices that are built into the computer and have
a LED that is expected to be controlled with a radio switch, this patch will
not operate that LED. That will take a separate patch to be prepared later.

Please test and comment.

The behavior described above is found for my Level One RTL8187B. Prior to this
patch, the LED was on continuously. On a Netgear WG111V2 RTL8187L, the LED
blinks erratically. Before the patch, the LED was off.

If you test, please report the status of the LED before and after applying the
patch. In addition, find the line that looks like "rtl8187: Customer ID 0x00" in
the dmesg output and report it. I also need to know if you have a B or L model.

Signed-off-by: Larry Finger <[email protected]>
---

V2 fixed a locking problem that caused the RTL8187L to fail.

V3 removes the code that turned the LED on at device startup, and turns
it off when the driver is unloaded.

V4 turns the TX LED on when registered and turns it off in rtl8187_leds_exit().
In addition, it corrects a typo in V3.

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 LEDs 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]>");
@@ -729,10 +732,10 @@ static const u8 rtl8187b_reg_table[][3]
{0x85, 0x24, 0}, {0x88, 0x54, 0}, {0x8B, 0xB8, 0}, {0x8C, 0x07, 0},
{0x8D, 0x00, 0}, {0x94, 0x1B, 0}, {0x95, 0x12, 0}, {0x96, 0x00, 0},
{0x97, 0x06, 0}, {0x9D, 0x1A, 0}, {0x9F, 0x10, 0}, {0xB4, 0x22, 0},
- {0xBE, 0x80, 0}, {0xDB, 0x00, 0}, {0xEE, 0x00, 0}, {0x91, 0x03, 0},
+ {0xBE, 0x80, 0}, {0xDB, 0x00, 0}, {0xEE, 0x00, 0}, {0x4C, 0x00, 2},

- {0x4C, 0x00, 2}, {0x9F, 0x00, 3}, {0x8C, 0x01, 0}, {0x8D, 0x10, 0},
- {0x8E, 0x08, 0}, {0x8F, 0x00, 0}
+ {0x9F, 0x00, 3}, {0x8C, 0x01, 0}, {0x8D, 0x10, 0}, {0x8E, 0x08, 0},
+ {0x8F, 0x00, 0}
};

static int rtl8187b_init_hw(struct ieee80211_hw *dev)
@@ -1498,6 +1501,12 @@ 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
+ eeprom_93cx6_read(&eeprom, 0x3F, &reg);
+ reg &= 0xFF;
+ rtl8187_leds_init(dev, reg);
+#endif
+
return 0;

err_free_dev:
@@ -1515,6 +1524,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,215 @@
+/*
+ * Linux LED driver for RTL8187
+ *
+ * Copyright 2009 Larry Finger <[email protected]>
+ *
+ * Based on the LED handling in the r8187 driver, which is:
+ * Copyright (c) Realtek Semiconductor Corp. All rights reserved.
+ *
+ * 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)
+{
+ /* As this routine does read/write operations on the hardware, it must
+ * be run from a work queue.
+ */
+ u8 reg;
+ struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
+ led_on.work);
+ struct rtl8187_led *led = &priv->led_tx;
+
+ /* Don't change the LED, when the device is down. */
+ if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
+ return ;
+
+ /* Skip if the LED is not registered. */
+ if (!led->dev)
+ return;
+ mutex_lock(&priv->conf_mutex);
+ 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;
+ }
+ mutex_unlock(&priv->conf_mutex);
+}
+
+static void led_turn_off(struct work_struct *work)
+{
+ /* As this routine does read/write operations on the hardware, it must
+ * be run from a work queue.
+ */
+ u8 reg;
+ struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
+ led_off.work);
+ struct rtl8187_led *led = &priv->led_tx;
+
+ /* Don't change the LED, when the device is down. */
+ if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
+ return ;
+
+ mutex_lock(&priv->conf_mutex);
+ 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;
+ }
+ mutex_unlock(&priv->conf_mutex);
+}
+
+/* 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);
+ /* The LED is off for 1/20 sec so that it just blinks. */
+ queue_delayed_work(priv->dev->workqueue, &priv->led_on, HZ / 20);
+ } 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)
+{
+ led_classdev_unregister(&led->led_dev);
+ led->dev = NULL;
+}
+
+void rtl8187_leds_init(struct ieee80211_hw *dev, u16 custid)
+{
+ struct rtl8187_priv *priv = dev->priv;
+ char name[RTL8187_LED_MAX_NAME_LEN + 1];
+ u8 ledpin;
+ int err;
+
+ /* According to the vendor driver, the LED operation depends on the
+ * customer ID encoded in the EEPROM
+ */
+ printk(KERN_INFO "rtl8187: Customer ID is 0x%02X\n", custid);
+ switch (custid) {
+ case EEPROM_CID_RSVD0:
+ case EEPROM_CID_RSVD1:
+ case EEPROM_CID_SERCOMM_PS:
+ case EEPROM_CID_QMI:
+ case EEPROM_CID_DELL:
+ case EEPROM_CID_TOSHIBA:
+ 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));
+ err = rtl8187_register_led(dev, &priv->led_tx, name,
+ ieee80211_get_tx_led_name(dev), ledpin);
+ if (err)
+ goto error;
+ snprintf(name, sizeof(name),
+ "rtl8187-%s::rx", wiphy_name(dev->wiphy));
+ err = rtl8187_register_led(dev, &priv->led_rx, name,
+ ieee80211_get_rx_led_name(dev), ledpin);
+ if (!err) {
+ queue_delayed_work(dev->workqueue, &priv->led_on, 0);
+ return;
+ }
+ /* registration of RX LED failed - unregister TX */
+ rtl8187_unregister_led(&priv->led_tx);
+error:
+ /* If registration of either failed, cancel delayed work */
+ cancel_delayed_work_sync(&priv->led_off);
+ cancel_delayed_work_sync(&priv->led_on);
+}
+
+void rtl8187_leds_exit(struct ieee80211_hw *dev)
+{
+ struct rtl8187_priv *priv = dev->priv;
+
+ rtl8187_unregister_led(&priv->led_tx);
+ /* turn the LED off before exiting */
+ queue_delayed_work(dev->workqueue, &priv->led_off, 0);
+ cancel_delayed_work_sync(&priv->led_off);
+ 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,57 @@
+/*
+ * Definitions for RTL8187 leds
+ *
+ * Copyright 2009 Larry Finger <[email protected]>
+ *
+ * Based on the LED handling in the r8187 driver, which is:
+ * Copyright (c) Realtek Semiconductor Corp. All rights reserved.
+ *
+ * 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_TOSHIBA = 0x04,
+ 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, u16 code);
+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;


2009-04-16 23:26:47

by Larry Finger

[permalink] [raw]
Subject: Re: [RFT/RFC V4] rtl8187: Implement TX/RX blink for LED

Herton Ronaldo Krzesinski wrote:
> Em Quinta-feira 16 Abril 2009, =E0s 15:12:37, Larry Finger escreveu:
>> The following patch implements some control over the LED on RTL8187B=
and
>> RTL8187L devices. Triggers are registered for TX and RX. Whenever th=
e
>> trigger event occurs, the LED is turned off for 1/20 second, then tu=
rned
>> back on.
>>
>> Note: For those RTL8187X devices that are built into the computer an=
d have
>> a LED that is expected to be controlled with a radio switch, this pa=
tch
>> will not operate that LED. That will take a separate patch to be pre=
pared
>> later.
>>
>> Please test and comment.
>>
>> The behavior described above is found for my Level One RTL8187B. Pri=
or to
>> this patch, the LED was on continuously. On a Netgear WG111V2 RTL818=
7L, the
>> LED blinks erratically. Before the patch, the LED was off.
>>
>> If you test, please report the status of the LED before and after ap=
plying
>> the patch. In addition, find the line that looks like "rtl8187: Cust=
omer ID
>> 0x00" in the dmesg output and report it. I also need to know if you =
have a
>> B or L model.
>>
>> Signed-off-by: Larry Finger <[email protected]>
>> ---
>>
>> V2 fixed a locking problem that caused the RTL8187L to fail.
>>
>> V3 removes the code that turned the LED on at device startup, and tu=
rns
>> it off when the driver is unloaded.
>>
>> V4 turns the TX LED on when registered and turns it off in
>> rtl8187_leds_exit(). In addition, it corrects a typo in V3.
>=20
> All looks fine now, but sorry I got a new problem :)
> Now that I'm able to rmmod rtl8187 after 2.6.30-rc2 sync in wireless-=
testing,=20
> I got an oops when inserting/removing rtl8187 in sequence (modprobe r=
tl8187;=20
> modprobe -r rtl8187)
>=20
> BUG: unable to handle kernel NULL pointer dereference at 00000024
> IP: [<d89b853f>] rtl8187_led_brightness_set+0x5f/0x70 [rtl8187]
> *pde =3D 00000000
> Oops: 0000 [#1] SMP
> last sysfs file: /sys/module/rtl8187/refcnt
> Modules linked in: rtl8187(-) mac80211 led_class eeprom_93cx6 cfg8021=
1=20
> af_packet ipv6 binfmt_misc loop dm_mod sha256_generic sha1_generic pa=
dlock_sha=20
> padlock_aes aes_generic nvram arc4 ecb uvcvideo snd_hda_codec_realtek=
videodev=20
> v4l1_compat snd_hda_intel snd_hda_codec snd_hwdep snd_seq_dummy snd_s=
eq_oss=20
> snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss 8139cp 8139too =
snd_pcm=20
> mii snd_timer snd_mixer_oss snd i2c_viapro shpchp video soundcore=20
> snd_page_alloc pci_hotplug sg i2c_core pcspkr evdev ehci_hcd uhci_hcd=
output=20
> rtc_cmos thermal ac battery processor button usbcore ata_generic=20
> ide_pci_generic pata_acpi via82cxxx ide_gd_mod ide_core pata_via liba=
ta sd_mod=20
> scsi_mod crc_t10dif ext4 jbd2 crc16 [last unloaded: cfg80211]
>=20
> Pid: 4607, comm: modprobe Not tainted (2.6.30-rc2-wl #4) Positivo Mob=
ile
> EIP: 0060:[<d89b853f>] EFLAGS: 00010286 CPU: 0
> EIP is at rtl8187_led_brightness_set+0x5f/0x70 [rtl8187]
> EAX: 00000000 EBX: d78e7940 ECX: 00000032 EDX: d78e7d88
> ESI: 00000000 EDI: d78e7cbc EBP: d6863e2c ESP: d6863e24
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> Process modprobe (pid: 4607, ti=3Dd6862000 task=3Dd792ca40 task.ti=3D=
d6862000)
> Stack:
> 5cf2b074 d78e7cbc d6863e48 c037152f 5cf2b074 5cf2b074 d78e7cbc d78e7=
ce8
> d78e7180 d6863e5c d8992156 5cf2b074 d78e7940 d74adb00 d6863e74 d89b8=
57a
> 5cf2b074 d78e7180 d74adb00 d89bb7e0 d6863e88 d89b8cb3 5cf2b074 d74ad=
b00
> Call Trace:
> [<c037152f>] ? led_trigger_set+0xcf/0xe0
> [<d8992156>] ? led_classdev_unregister+0x56/0xb0 [led_class]
> [<d89b857a>] ? rtl8187_leds_exit+0x2a/0x80 [rtl8187]
> [<d89b8cb3>] ? rtl8187_disconnect+0x23/0x68 [rtl8187]
> [<d8b734ac>] ? usb_unbind_interface+0x4c/0x140 [usbcore]
> [<c032ef2f>] ? __device_release_driver+0x5f/0xb0
> [<c032f027>] ? driver_detach+0xa7/0xc0
> [<c032de5c>] ? bus_remove_driver+0x8c/0x100
> [<c032f841>] ? driver_unregister+0x41/0x60
> [<d8b73257>] ? usb_deregister+0xa7/0xd0 [usbcore]
> [<d89b8c6b>] ? rtl8187_exit+0x1b/0x40 [rtl8187]
> [<c01719d5>] ? sys_delete_module+0x185/0x240
> [<c01c17c3>] ? do_munmap+0x243/0x2a0
> [<c042c72a>] ? do_page_fault+0x1da/0x320
> [<c01045bb>] ? sysenter_do_call+0x12/0x28
> Code: 00 00 00 75 30 83 c4 04 5b 5d c3 90 8b 40 24 8d 93 88 04 00 00 =
e8 32 d7=20
> 79 e7 8b 83 74 03 00 00 8d 93 48 04 00 00 b9 32 00 00 00 <8b> 40 24 e=
8 19 d7=20
> 79 e7 eb c4 e8 72 85 78 e7 66 90 55 89 e5 83
> EIP: [<d89b853f>] rtl8187_led_brightness_set+0x5f/0x70 [rtl8187] SS:E=
SP=20
> 0068:d6863e24
> CR2: 0000000000000024
> ---[ end trace ae006a68f9740f08 ]---
>=20
> this is because priv->dev is only set at rtl8187_start, which only ru=
ns after=20
> you bring the interface up. One possible fix:
>=20
> diff -p -up ./drivers/net/wireless/rtl818x/rtl8187_leds.c.orig=20
> ./drivers/net/wireless/rtl818x/rtl8187_leds.c
> --- ./drivers/net/wireless/rtl818x/rtl8187_leds.c.orig 2009-04-16=20
> 17:21:56.000000000 -0300
> +++ ./drivers/net/wireless/rtl818x/rtl8187_leds.c 2009-04-16=20
> 18:16:50.000000000 -0300
> @@ -107,7 +107,7 @@ static void rtl8187_led_brightness_set(s
> if (brightness =3D=3D LED_OFF) {
> queue_delayed_work(hw->workqueue, &priv->led_off, 0);
> /* The LED is off for 1/20 sec so that it just blinks. */
> - queue_delayed_work(priv->dev->workqueue, &priv->led_on, HZ / 20);
> + queue_delayed_work(hw->workqueue, &priv->led_on, HZ / 20);
> } else
> queue_delayed_work(hw->workqueue, &priv->led_on, 0);
> }

I have no idea why that one was different. That is fixed - in addition,=
I also
check that led->dev is non-NULL in led_off as well as led_on. I was abl=
e to
insmod/rmmod rtl8187 about 10 times in a row without any problems. Than=
ks for
finding this problem.
>=20
> You can add Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva=
=2Ecom.br>=20
> to the patch (or Tested-by/Reviewed-by, what's more appropriate, now =
hopefully=20
> we will have the last version of the patch :) )

I think a signed-off-by is appropriate. I usually get it right in 4 tri=
es. ;)

Larry

Subject: Re: [RFT/RFC V4] rtl8187: Implement TX/RX blink for LED

Em Quinta-feira 16 Abril 2009, =E0s 15:12:37, Larry Finger escreveu:
> 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.
>
> Note: For those RTL8187X devices that are built into the computer and=
have
> a LED that is expected to be controlled with a radio switch, this pat=
ch
> will not operate that LED. That will take a separate patch to be prep=
ared
> later.
>
> Please test and comment.
>
> The behavior described above is found for my Level One RTL8187B. Prio=
r to
> this patch, the LED was on continuously. On a Netgear WG111V2 RTL8187=
L, the
> LED blinks erratically. Before the patch, the LED was off.
>
> If you test, please report the status of the LED before and after app=
lying
> the patch. In addition, find the line that looks like "rtl8187: Custo=
mer ID
> 0x00" in the dmesg output and report it. I also need to know if you h=
ave a
> B or L model.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> V2 fixed a locking problem that caused the RTL8187L to fail.
>
> V3 removes the code that turned the LED on at device startup, and tur=
ns
> it off when the driver is unloaded.
>
> V4 turns the TX LED on when registered and turns it off in
> rtl8187_leds_exit(). In addition, it corrects a typo in V3.

All looks fine now, but sorry I got a new problem :)
Now that I'm able to rmmod rtl8187 after 2.6.30-rc2 sync in wireless-te=
sting,=20
I got an oops when inserting/removing rtl8187 in sequence (modprobe rtl=
8187;=20
modprobe -r rtl8187)

BUG: unable to handle kernel NULL pointer dereference at 00000024
IP: [<d89b853f>] rtl8187_led_brightness_set+0x5f/0x70 [rtl8187]
*pde =3D 00000000
Oops: 0000 [#1] SMP
last sysfs file: /sys/module/rtl8187/refcnt
Modules linked in: rtl8187(-) mac80211 led_class eeprom_93cx6 cfg80211=20
af_packet ipv6 binfmt_misc loop dm_mod sha256_generic sha1_generic padl=
ock_sha=20
padlock_aes aes_generic nvram arc4 ecb uvcvideo snd_hda_codec_realtek v=
ideodev=20
v4l1_compat snd_hda_intel snd_hda_codec snd_hwdep snd_seq_dummy snd_seq=
_oss=20
snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss 8139cp 8139too sn=
d_pcm=20
mii snd_timer snd_mixer_oss snd i2c_viapro shpchp video soundcore=20
snd_page_alloc pci_hotplug sg i2c_core pcspkr evdev ehci_hcd uhci_hcd o=
utput=20
rtc_cmos thermal ac battery processor button usbcore ata_generic=20
ide_pci_generic pata_acpi via82cxxx ide_gd_mod ide_core pata_via libata=
sd_mod=20
scsi_mod crc_t10dif ext4 jbd2 crc16 [last unloaded: cfg80211]

Pid: 4607, comm: modprobe Not tainted (2.6.30-rc2-wl #4) Positivo Mobil=
e
EIP: 0060:[<d89b853f>] EFLAGS: 00010286 CPU: 0
EIP is at rtl8187_led_brightness_set+0x5f/0x70 [rtl8187]
EAX: 00000000 EBX: d78e7940 ECX: 00000032 EDX: d78e7d88
ESI: 00000000 EDI: d78e7cbc EBP: d6863e2c ESP: d6863e24
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Process modprobe (pid: 4607, ti=3Dd6862000 task=3Dd792ca40 task.ti=3Dd6=
862000)
Stack:
5cf2b074 d78e7cbc d6863e48 c037152f 5cf2b074 5cf2b074 d78e7cbc d78e7ce=
8
d78e7180 d6863e5c d8992156 5cf2b074 d78e7940 d74adb00 d6863e74 d89b857=
a
5cf2b074 d78e7180 d74adb00 d89bb7e0 d6863e88 d89b8cb3 5cf2b074 d74adb0=
0
Call Trace:
[<c037152f>] ? led_trigger_set+0xcf/0xe0
[<d8992156>] ? led_classdev_unregister+0x56/0xb0 [led_class]
[<d89b857a>] ? rtl8187_leds_exit+0x2a/0x80 [rtl8187]
[<d89b8cb3>] ? rtl8187_disconnect+0x23/0x68 [rtl8187]
[<d8b734ac>] ? usb_unbind_interface+0x4c/0x140 [usbcore]
[<c032ef2f>] ? __device_release_driver+0x5f/0xb0
[<c032f027>] ? driver_detach+0xa7/0xc0
[<c032de5c>] ? bus_remove_driver+0x8c/0x100
[<c032f841>] ? driver_unregister+0x41/0x60
[<d8b73257>] ? usb_deregister+0xa7/0xd0 [usbcore]
[<d89b8c6b>] ? rtl8187_exit+0x1b/0x40 [rtl8187]
[<c01719d5>] ? sys_delete_module+0x185/0x240
[<c01c17c3>] ? do_munmap+0x243/0x2a0
[<c042c72a>] ? do_page_fault+0x1da/0x320
[<c01045bb>] ? sysenter_do_call+0x12/0x28
Code: 00 00 00 75 30 83 c4 04 5b 5d c3 90 8b 40 24 8d 93 88 04 00 00 e8=
32 d7=20
79 e7 8b 83 74 03 00 00 8d 93 48 04 00 00 b9 32 00 00 00 <8b> 40 24 e8 =
19 d7=20
79 e7 eb c4 e8 72 85 78 e7 66 90 55 89 e5 83
EIP: [<d89b853f>] rtl8187_led_brightness_set+0x5f/0x70 [rtl8187] SS:ESP=
=20
0068:d6863e24
CR2: 0000000000000024
---[ end trace ae006a68f9740f08 ]---

this is because priv->dev is only set at rtl8187_start, which only runs=
after=20
you bring the interface up. One possible fix:

diff -p -up ./drivers/net/wireless/rtl818x/rtl8187_leds.c.orig=20
=2E/drivers/net/wireless/rtl818x/rtl8187_leds.c
--- ./drivers/net/wireless/rtl818x/rtl8187_leds.c.orig 2009-04-16=20
17:21:56.000000000 -0300
+++ ./drivers/net/wireless/rtl818x/rtl8187_leds.c 2009-04-16=20
18:16:50.000000000 -0300
@@ -107,7 +107,7 @@ static void rtl8187_led_brightness_set(s
if (brightness =3D=3D LED_OFF) {
queue_delayed_work(hw->workqueue, &priv->led_off, 0);
/* The LED is off for 1/20 sec so that it just blinks. */
- queue_delayed_work(priv->dev->workqueue, &priv->led_on, HZ / 20);
+ queue_delayed_work(hw->workqueue, &priv->led_on, HZ / 20);
} else
queue_delayed_work(hw->workqueue, &priv->led_on, 0);
}

You can add Signed-off-by: Herton Ronaldo Krzesinski <[email protected]=
om.br>=20
to the patch (or Tested-by/Reviewed-by, what's more appropriate, now ho=
pefully=20
we will have the last version of the patch :) )

<snip>

--
[]'s
Herton