On Wednesday 14 October 2009 03:25:30 Larry Finger wrote:
> Commit 93bad2b757586fb153ef73b028953a8dcaccde77 entitled "b43: Fix PPC crash
> in rfkill polling on unload" fixed the bug reported in Bugzilla No. 14181;
> however, it introduced a new bug. Whenever the radio switch was turned off,
> it was necessary to unload and reload the driver for it to recognize the
> switch again.
>
> I believe this patch fixes the original problem without introducing any new
> problems.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> John,
>
> This is 2.6.32 material to fix the mess I made earlier.
>
> Larry
> ---
>
> Index: wireless-testing/drivers/net/wireless/b43/main.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/b43/main.c
> +++ wireless-testing/drivers/net/wireless/b43/main.c
> @@ -4501,7 +4501,6 @@ static void b43_op_stop(struct ieee80211
>
> cancel_work_sync(&(wl->beacon_update_trigger));
>
> - wiphy_rfkill_stop_polling(hw->wiphy);
> mutex_lock(&wl->mutex);
> if (b43_status(dev) >= B43_STAT_STARTED) {
> dev = b43_wireless_core_stop(dev);
> Index: wireless-testing/drivers/net/wireless/b43/rfkill.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/b43/rfkill.c
> +++ wireless-testing/drivers/net/wireless/b43/rfkill.c
> @@ -33,7 +33,8 @@ bool b43_is_hw_radio_enabled(struct b43_
> & B43_MMIO_RADIO_HWENABLED_HI_MASK))
> return 1;
> } else {
> - if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
> + if (b43_status(dev) >= B43_STAT_STARTED &&
> + b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
> & B43_MMIO_RADIO_HWENABLED_LO_MASK)
> return 1;
> }
I'm still wondering, however, why you only need that check in the else branch.
I guess the newer devices which use the other branch just fail silently.
I don't think this fix is correct at all.
The only way we can have status<STARTED here is through the rfkill_poll
function being called before the device is initialized. So it will enable
the ssb device and try to fetch the rfkill register. _However_, with this check
added it will _always_ return False. So why do we do the polling anyway, if it's
always guaranteed to be false? I think that's buggy.
To get the same effect, we could just do that:
44 void b43_rfkill_poll(struct ieee80211_hw *hw)
45 {
46 struct b43_wl *wl = hw_to_b43_wl(hw);
47 struct b43_wldev *dev = wl->current_dev;
48 struct ssb_bus *bus = dev->dev->bus;
49 bool enabled;
50 bool brought_up = false;
51
52 mutex_lock(&wl->mutex);
53 if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
wiphy_rfkill_set_hw_state(false);
unlock;
return;
// Remove the powerup logic...
54 if (ssb_bus_powerup(bus, 0)) {
55 mutex_unlock(&wl->mutex);
56 return;
57 }
58 ssb_device_enable(dev->dev, 0);
59 brought_up = true;
60 }
And no, it's not correct to force device state to STARTED or something else
here, because
that won't fix the issue and introduce other bugs.
--
Greetings, Michael.
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
