On Friday 07 December 2007 01:05:11 Larry Finger wrote:
> Sorry it has taken so long to answer, but I am traveling.
>
> Michael Buesch wrote:
> > On Thursday 29 November 2007 00:48:40 Larry Finger wrote:
> >
> >> @@ -2802,6 +2800,13 @@ static int b43_op_config(struct ieee8021
> >> out_unlock_mutex:
> >> mutex_unlock(&wl->mutex);
> >>
> >> + /* if a LED is devoted to the radio and the switch is on, send
> >> + * KEY_WLAN press/release keystrokes */
> >> + if (!err && dev->radio_hw_enable && &dev->led_radio) {
> > ^^^^^^^^^^^^^^^
> > This condition is always true.
>
> It looks to me that the code uses the contents of the LED section of the SPROM to initialize
> dev->led_radio. Can we be certain that the initialization will always be done?
I think you didn't quite understand what I tried to say.
Let's give a more obvious example:
int a;
if (&a) {
/* This is always true, as (&a) can't be a NULL pointer. */
}
Your condition above can never be false. (in practice)
I guess you tried to check if a radio led exists.
I'd suggest you do
if (... && dev->led_radio.dev)
The code in led.c does also assume that the LED is used, if struct b43_led->dev was
assigned to something non-NULL. So I think that kind of check would be OK.
But it might need an additional comment if done here outside of the led.c context.
> >> + input_report_key(rfk->poll_dev->input, KEY_WLAN, 1);
> >> + input_report_key(rfk->poll_dev->input, KEY_WLAN, 0);
> >> + }
> >> +
> >
> > Anyway, sending a key event here seems pretty bogus. The comment
> > doesn't really say anything useful why this is needed.
>
> If a key event (down/up) is not sent, the LED remains off. This location may not be the best place,
> but it is needed someplace, otherwise the switch must be cycled off/on to get the LED on.
So I guess we should do this at initialization and not in the config callback.
--
Greetings Michael.
-
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
