Blacklist brcm* drivers from preload-wifi-driver

If a Broadcom wireless driver is pre-loaded, the entire system state can become indeterminate (other driver modules are not properly loaded by the kernel).  This behavior has been seen on multiple broadcom drivers (brcmfmac and brcmsmac) on multiple platforms (both x86 and ARM).

BUG=chromium-os:13152
TEST=Add debugging logic to file and restart udev with brcmfmac in preload-wifie-driver
 Restart udev without it
 Observe in both cases the proper execution path (skipping or performing preload)
  Burnt a fresh image with the change.  Rebooted multiple times and observed correct behavior each reboot

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=c85e54f

[jrbarnette, 2011-04-06 18:18:09]

Can we put this blacklisting behavior into udev-addon.conf, where it's
not taking cycles from getting to login screen?

Also, it's not clear to me why the misbehavior of the modprobe
is isn't some sort of problem with the driver; could you clarify that?

[kliegs, 2011-04-06 18:34:04]

Yah, I can move it. Had wanted it in udev.conf to cover boards which had it
(in the case of a gmerge) but I'll move it now.

I'm not clear it isn't a function of the driver.  However a solution is
needed so people can use their devices.

While doing this (and realizing that [[ ]] isn't supported in the upstart
script) I did notice that if the udev start was delayed/failed that similar
problems happened.  That is running 'initctl start udev' later the system
was not in a useable state.

On Wed, Apr 6, 2011 at 2:18 PM, <jrbarnette@chromium.org> wrote:

> Can we put this blacklisting behavior into udev-addon.conf, where it's
> not taking cycles from getting to login screen?
>
> Also, it's not clear to me why the misbehavior of the modprobe
> is isn't some sort of problem with the driver; could you clarify that?
>
>
>
> http://codereview.chromium.org/6677181/
>

[ggg, 2011-04-06 18:45:22]

On Wed, Apr 6, 2011 at 11:33 AM, Jonathan Kliegman <kliegs@chromium.org> wrote:
> Yah, I can move it. Had wanted it in udev.conf to cover boards which had it
> (in the case of a gmerge) but I'll move it now.

Jonathan,.
I'd just leave the CL as-is. I'm investigating this more right now. I
see this Cl as a "workaround" for others to use for now.


> I'm not clear it isn't a function of the driver.  However a solution is
> needed so people can use their devices.

Right - for today, developers can just apply your CL, no?
No one else will be pulling ToT anyway.

> While doing this (and realizing that [[ ]] isn't supported in the upstart
> script) I did notice that if the udev start was delayed/failed that similar
> problems happened.  That is running 'initctl start udev' later the system
> was not in a useable state.

Interesting. I'm still very curious (and looking for) what the race
condition really is.

thanks,
grant

> On Wed, Apr 6, 2011 at 2:18 PM, <jrbarnette@chromium.org> wrote:
>>
>> Can we put this blacklisting behavior into udev-addon.conf, where it's
>> not taking cycles from getting to login screen?
>>
>> Also, it's not clear to me why the misbehavior of the modprobe
>> is isn't some sort of problem with the driver; could you clarify that?
>>
>>
>> http://codereview.chromium.org/6677181/
>
>

[ggg, 2011-04-06 20:28:31]

I'm not able to reproduce the failure on Seaboard with this CL:
    http://codereview.chromium.org/6802002/

I've rebooted 5 times (power off/on).

(As an aside, I'm able to suspend/resume for real now too!)

Jonathan, since the two changes in 6802002 are only for brcmfmac, can you retest
on brcmfmac or tell me who can?


BTW, the original problem report suggested the issue was seen on 4 combinations
of platforms+drivers when AFAIK, there are only two:
1) L13 + bcm4313 (x86 + brcmsmac)
2) seaboard + bcm4329 (ARM + brcmfmac)

[kliegs, 2011-04-06 20:39:48]

We saw it on Lenovo S10-3T bcm4313 (x86 + brcmsmac) as well.

I'll test the patch shortly and let you know.

On Wed, Apr 6, 2011 at 4:28 PM, <grundler@chromium.org> wrote:

> I'm not able to reproduce the failure on Seaboard with this CL:
>    http://codereview.chromium.org/6802002/
>
> I've rebooted 5 times (power off/on).
>
> (As an aside, I'm able to suspend/resume for real now too!)
>
> Jonathan, since the two changes in 6802002 are only for brcmfmac, can you
> retest
> on brcmfmac or tell me who can?
>
>
> BTW, the original problem report suggested the issue was seen on 4
> combinations
> of platforms+drivers when AFAIK, there are only two:
> 1) L13 + bcm4313 (x86 + brcmsmac)
> 2) seaboard + bcm4329 (ARM + brcmfmac)
>
>
> http://codereview.chromium.org/6677181/
>

[ggg, 2011-04-06 21:56:35]

LGTM even though I can not reproduce this on ARM.

Please documentation (e.g. bug reports) on x86 where this is a problem so I can
test for and know when I can revert this workaround in the future.

[jrbarnette, 2011-04-06 22:20:51]

Two nits, LGTM otherwise.

Thanks!

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf
File udev-addon.conf (right):

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode36
udev-addon.conf:36: # Blacklist brcm drivers
Can you add a TODO here?  I want to make sure it's clear that we
know this is a hack, and to put a boundary around the reason why
we need it.

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode38
udev-addon.conf:38: [ -s $drvfile ] && rm $drvfile
Ugh.  I'm not fond of this idiom.  This will work equivalently:
    rm -f $drvfile

[kliegs, 2011-04-06 22:30:12]

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf
File udev-addon.conf (right):

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode36
udev-addon.conf:36: # Blacklist brcm drivers
On 2011/04/06 22:20:51, jrbarnette wrote:
> Can you add a TODO here?  I want to make sure it's clear that we
> know this is a hack, and to put a boundary around the reason why
> we need it.

Done.

http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode38
udev-addon.conf:38: [ -s $drvfile ] && rm $drvfile
On 2011/04/06 22:20:51, jrbarnette wrote:
> Ugh.  I'm not fond of this idiom.  This will work equivalently:
>     rm -f $drvfile

Done.

[jrbarnette, 2011-04-06 23:08:53]

LGTM!  Thanks!


On 2011/04/06 22:30:12, kliegs wrote:
> http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf
> File udev-addon.conf (right):
> 
> http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode36
> udev-addon.conf:36: # Blacklist brcm drivers
> On 2011/04/06 22:20:51, jrbarnette wrote:
> > Can you add a TODO here?  I want to make sure it's clear that we
> > know this is a hack, and to put a boundary around the reason why
> > we need it.
> 
> Done.
> 
> http://codereview.chromium.org/6677181/diff/5002/udev-addon.conf#newcode38
> udev-addon.conf:38: [ -s $drvfile ] && rm $drvfile
> On 2011/04/06 22:20:51, jrbarnette wrote:
> > Ugh.  I'm not fond of this idiom.  This will work equivalently:
> >     rm -f $drvfile
> 
> Done.