Closed
Bug 859486
Opened 11 years ago
Closed 11 years ago
Don't don't unload wireless drivers
Categories
(Core :: DOM: Device Interfaces, defect, P1)
Tracking
()
People
(Reporter: m1, Assigned: m1)
Details
(Whiteboard: [caf priority: p1][CR 472163])
Attachments
(2 files, 1 obsolete file)
2.32 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
1.79 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
There have been reports of significant power drain as a result of not unloading the wifi drivers. Even if the assumption made by bug 801935 [1] is correct, Android does unload the wifi drivers as SOP and bug 801935 was raised due to issues with preproduction hardware/software. So if unloading wifi drivers still causes issues on Unagi can Moz work with the vendor to get a fix in parallel to backing out bug 801935 for v1.1? [1] Unloading wireless drivers can trigger bugs on improperly written drivers and is unnecessary on properly written drivers where taking the interface down powers down the hardware.
Comment 1•11 years ago
|
||
Unloading wifi drivers is not SOP on Jellybean. AFAICT, there is no module loading at all on Jellybean. Not unloading modules is really the right thing to do here. The driver needs to be fixed, but if it can't be done in time, we can put in a quirk to have it unload.
Assignee | ||
Comment 2•11 years ago
|
||
We're not on a Jellybean-based gonk yet, so Jellybean-era fixes are not necessarily relevant for v1.0.1/v1.1 unfortunately (too much regression risk). Does the latest unagi kernel still suffer from bug 801935? I'm happy to contribute a patch that adds this quirk though. How about calling it "ro.moz.wifi.unloadmodules=true"?
Comment 3•11 years ago
|
||
(In reply to Michael Vines [:m1] [:evilmachines] from comment #2) > We're not on a Jellybean-based gonk yet, so Jellybean-era fixes are not > necessarily relevant for v1.0.1/v1.1 unfortunately (too much regression > risk). > They're not - I agree. Just pointed that out as a reference for what we'll want going forward. > I'm happy to contribute a patch that adds this quirk though. How about > calling it "ro.moz.wifi.unloadmodules=true"? Sounds good to me.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → mvines
Updated•11 years ago
|
Summary: Don't Don't unload wireless drivers → Don't unload wireless drivers
Assignee | ||
Comment 4•11 years ago
|
||
Heh, that 2nd don't was intentional but perhaps unclear due to the incorrect capitalization of the 2nd don't. I've corrected this now.
Summary: Don't unload wireless drivers → Don't don't unload wireless drivers
Assignee | ||
Comment 5•11 years ago
|
||
|adb shell setprop ro.moz.wifi.unloaddriver 1| to enable
Attachment #735267 -
Flags: review?(mwu)
Comment 6•11 years ago
|
||
Comment on attachment 735267 [details] [diff] [review] attempt #1 (git flavour) Looks good to me, though I think mrbkap is a more appropriate reviewer (if he has time).
Attachment #735267 -
Flags: review?(mwu) → review?(mrbkap)
Comment 7•11 years ago
|
||
Comment on attachment 735267 [details] [diff] [review] attempt #1 (git flavour) Review of attachment 735267 [details] [diff] [review]: ----------------------------------------------------------------- We probably won't be able to turn off driver unloading for Unagis or Otoros (to wit: Android on those phones doesn't unload the driver) but this looks good to me. ::: dom/wifi/WifiWorker.js @@ +46,5 @@ > return parseInt(sdkVersion, 10); > } > + function getUnloadDriverEnabled() { > + let enabled = libcutils.property_get("ro.moz.wifi.unloaddriver"); > + return parseInt(enabled, 10) == 1; Nit: === instead of ==.
Attachment #735267 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Sorry to bug you for r? again Blake. Turns out I based my patch on v1-train and had to make more changes to it when rebasing to m-c than I'm currently comfortable carrying r+ forward with. But the good news is that the patch just got much smaller.
Attachment #736114 -
Flags: review?(mrbkap)
Assignee | ||
Updated•11 years ago
|
status-b2g18:
--- → affected
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #735267 -
Attachment is obsolete: true
Attachment #736116 -
Flags: review?(mrbkap)
Updated•11 years ago
|
Attachment #736114 -
Flags: review?(mrbkap) → review+
Updated•11 years ago
|
Attachment #736116 -
Flags: review?(mrbkap) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbf9ab7dec31
Keywords: checkin-needed
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dbf9ab7dec31
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment 12•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/950db80b82cb
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → wontfix
status-firefox23:
--- → fixed
Updated•11 years ago
|
Flags: in-moztrap-
Updated•10 years ago
|
Whiteboard: [CR 472163] → [caf priority: p1][CR 472163]
You need to log in
before you can comment on or make changes to this bug.
Description
•