2010-10-12 08:31:45

by Jon Masters

[permalink] [raw]
Subject: [PATCH] [staging] brcm80211: fix radio disabled on attempt to bring up interface

The brcm80211 driver does not correctly handle the case that the wireless
radio hardware is physically disabled during interface initialization. An
attempt is made to check whether the radio is disabled, and in the case
that it is, a background worker is setup to monitor for the radio coming
online, but the interface queues are incorrectly brought up anyway. The
value BCME_RADIOOFF should be returned in wlc_up in such error case.

Signed-off-by: Jon Masters <[email protected]>

diff -urNp brcm80211_orig/sys/wlc_mac80211.c brcm80211/sys/wlc_mac80211.c
--- brcm80211_orig/sys/wlc_mac80211.c 2010-10-12 04:18:07.940125220 -0400
+++ brcm80211/sys/wlc_mac80211.c 2010-10-12 04:19:19.814961117 -0400
@@ -2783,7 +2783,7 @@ int BCMINITFN(wlc_up) (wlc_info_t *wlc)

if (wlc->pub->radio_disabled) {
wlc_radio_monitor_start(wlc);
- return 0;
+ return BCME_RADIOOFF;
}

/* wlc_bmac_up_prep has done wlc_corereset(). so clk is on, set it */


2010-10-12 18:34:45

by Brett Rudley

[permalink] [raw]
Subject: RE: [PATCH] [staging] brcm80211: fix radio disabled on attempt to bring up interface

Signed-off-by: Brett Rudley <[email protected]>
---

> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: Tuesday, October 12, 2010 1:30 AM
> To: [email protected]
> Cc: Brett Rudley; Henry Ptasinski; Nohee Ko; Jon Masters; LKML
> Subject: [PATCH] [staging] brcm80211: fix radio disabled on attempt to
> bring up interface
>
> The brcm80211 driver does not correctly handle the case that the wireless
> radio hardware is physically disabled during interface initialization. An
> attempt is made to check whether the radio is disabled, and in the case
> that it is, a background worker is setup to monitor for the radio coming
> online, but the interface queues are incorrectly brought up anyway. The
> value BCME_RADIOOFF should be returned in wlc_up in such error case.
>
> Signed-off-by: Jon Masters <[email protected]>
>
> diff -urNp brcm80211_orig/sys/wlc_mac80211.c brcm80211/sys/wlc_mac80211.c
> --- brcm80211_orig/sys/wlc_mac80211.c 2010-10-12 04:18:07.940125220 -
> 0400
> +++ brcm80211/sys/wlc_mac80211.c 2010-10-12 04:19:19.814961117 -0400
> @@ -2783,7 +2783,7 @@ int BCMINITFN(wlc_up) (wlc_info_t *wlc)
>
> if (wlc->pub->radio_disabled) {
> wlc_radio_monitor_start(wlc);
> - return 0;
> + return BCME_RADIOOFF;
> }
>
> /* wlc_bmac_up_prep has done wlc_corereset(). so clk is on, set it
> */



2010-10-12 19:19:23

by Jon Masters

[permalink] [raw]
Subject: RE: [PATCH] [staging] brcm80211: fix radio disabled on attempt to bring up interface

On Tue, 2010-10-12 at 11:34 -0700, Brett Rudley wrote:

> > The brcm80211 driver does not correctly handle the case that the wireless
> > radio hardware is physically disabled during interface initialization. An
> > attempt is made to check whether the radio is disabled, and in the case
> > that it is, a background worker is setup to monitor for the radio coming
> > online, but the interface queues are incorrectly brought up anyway. The
> > value BCME_RADIOOFF should be returned in wlc_up in such error case.

> Signed-off-by: Brett Rudley <[email protected]>

Thanks. The patch actually didn't use full paths to the staging tree
(was relative to the driver directory) because it was crazy late and my
brain was tired by the time I found the problem. You'll sort it out.

Anyway. So rfkill works with soft block/unblock, but doesn't seem to
correctly report the state of the physical button on this laptop. I
don't think that's your domain (as I said, I freely admit that I need to
find some time, sometime, to understand how rfkill is supposed to work).
What I do think is your domain is the fact that suspend/resume with this
driver isn't working. The system does suspend, and it does resume, but
the driver gets itself in a twist and never talks to the outside world
again - just keeps logging an error. I'll send you some debug info
tonight or in the next few days so we can get that fixed up too.

Can I ask, also, do you plan on cleaning up the WLC HIGH/LOW stuff or
adding some comments to the code so that people realize this is for
PCI/USB dongle stuff? I spent some time going through the code trying to
figure out what the WL_LOCK/UNLOCK stuff was about and why it was
missing in the case of a PCI device :) It seems like this driver is
partly based on generic code used in other drivers, and that's fine, but
I suspect some of it will need more cleanup before it is merged.

Jon.