Herton Ronaldo Krzesinski wrote:
> Em Quarta-feira 15 Abril 2009, às 17:33:40, 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 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.
>
> It didn't work for me, I need to fix this typo in V3 patch:
>
> ./drivers/net/wireless/rtl818x/rtl8187_leds.c
> --- ./drivers/net/wireless/rtl818x/rtl8187_leds.c.orig 2009-04-15
> 18:47:24.000000000 -0300
> +++ ./drivers/net/wireless/rtl818x/rtl8187_leds.c 2009-04-16
> 11:32:11.000000000 -0300
> @@ -64,7 +64,7 @@ static void led_turn_off(struct work_str
> */
> u8 reg;
> struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
> - led_on.work);
> + led_off.work);
> struct rtl8187_led *led = &priv->led_tx;
>
> /* Don't change the LED, when the device is down. */
Thanks for catching this. That was a copy-and-paste gone wrong.
> Also, it doens't turn on led on interface up, probably because we don't have a
> 'radio' led and now removed led on from init_regs, may be we could do this:
>
> @@ -186,8 +187,10 @@ void rtl8187_leds_init(struct ieee80211_
> "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)
> + if (!err) {
> + queue_delayed_work(priv->dev->workqueue, &priv->led_on, 0);
> return;
> + }
I had thought of this as a feature in that the LED comes on when the first
transmit occurs, but it is more instructive to have it on as soon as the LED
system is ready.
> /* registration of RX LED failed - unregister TX */
> rtl8187_unregister_led(&priv->led_tx);
> error:
>
>
> For the led off/stop code you added on rtl8187_stop, may be we would want to
> protect:
> rtl818x_iowrite8(priv, &priv->map->GPIO, 0x01);
> rtl818x_iowrite8(priv, &priv->map->GP_ENABLE, 0x01);
> inside an ifdef CONFIG_RTL8187_LEDS, but it will not work also for other led
> hardware (the ones which don't use GPIO/GP_ENABLE to set leds. we could do
> this instead on rtl8187_leds_exit:
>
> @@ -200,6 +203,7 @@ void rtl8187_leds_exit(struct ieee80211_
> {
> struct rtl8187_priv *priv = dev->priv;
>
> + queue_delayed_work(priv->dev->workqueue, &priv->led_off, 0);
> cancel_delayed_work_sync(&priv->led_off);
> cancel_delayed_work_sync(&priv->led_on);
> rtl8187_unregister_led(&priv->led_tx);
>
> but then it will not turn off led on interface stop, just on module removal...
If we are turning the LED on in rtl8187_leds_init(), it should be off in
rtl8187_leds_exit(). I'll rework for V4.
Thanks for the review.
Larry
--
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
