the-darkvoid / BrcmPatchRAM

Broadcom PatchRAM driver for OS X
GNU General Public License v2.0
88 stars 62 forks source link

Fix for USB delay, intermittent bluetooth operation post sleep. #9

Closed RehabMan closed 9 years ago

RehabMan commented 9 years ago

You can read here for details: https://github.com/RehabMan/OS-X-BrcmPatchRAM/issues/1

I also fixed a few other things along the way:

Please test. So far, only tested on my u430. I'm going to add a BCM card to my 4540s and will also test there soon.

RehabMan commented 9 years ago

Here's a few other notes on some of the other changes:

the-darkvoid commented 9 years ago

The deployed build for BrcmPatchRAM is the continuous_read branch. I will see about merging your changes in both branches.

RehabMan commented 9 years ago

By the looks of the diffs between your 'master' and 'continous_read' (I had a hell of a time with continous vs. continuous), it is probably easier to apply the diffs (manually) from continous_read into my master (except for the changes in Info.plist), but give it a shot and see what you find.

Had I known, I would have started with the continous_read branch. Typically 'master' is the main/deployed branch and other branches are the experiments.

BTW, I've found something interesting. With a slight tweak, if I use my version of BrcmPatchRAM with a bluetooth that does not have loadable firmware, bluetooth comes back after wake from sleep faster. That is, the unload/reload process works better than just letting the driver do what it normally would.

That tweak for my bluetooth device 13d3:3295 on a BCM943225HMB:

diff --git a/BrcmPatchRAM/BrcmPatchRAM.cpp b/BrcmPatchRAM/BrcmPatchRAM.cpp
index 45b2c89..15e9fa8 100644
--- a/BrcmPatchRAM/BrcmPatchRAM.cpp
+++ b/BrcmPatchRAM/BrcmPatchRAM.cpp
@@ -165,7 +165,7 @@ void BrcmPatchRAM::uploadFirmware()
             instructions = firmwareStore->getFirmware(OSDynamicCast(OSString, getProperty("Firmware

         // Set device configuration to composite configuration index 0
-        if (!setConfiguration(0))
+        if (!instructions || !setConfiguration(0))
         {
             mDevice->close(this);
             return;
diff --git a/BrcmPatchRAM/Info.plist b/BrcmPatchRAM/Info.plist
index 1ce45a0..13e3fa7 100644
--- a/BrcmPatchRAM/Info.plist
+++ b/BrcmPatchRAM/Info.plist
@@ -419,6 +419,25 @@
                        <key>idVendor</key>
                        <integer>2821</integer>
                </dict>
+               <key>13d3_3295</key>
+               <dict>
+                       <key>CFBundleIdentifier</key>
+                       <string>com.no-one.$(PRODUCT_NAME:rfc1034identifier)</string>
+                       <key>DisplayName</key>
+                       <string>Azurewave BCM943225 (20702A built-in firmwaer)</string>
+                       <key>IOClass</key>
+                       <string>BrcmPatchRAM</string>
+                       <key>IOMatchCategory</key>
+                       <string>BrcmPatchRAM</string>
+                       <key>IOProviderClass</key>
+                       <string>IOUSBDevice</string>
+                       <key>idProduct</key>
+                       <integer>12949</integer>
+                       <key>idVendor</key>
+                       <integer>5075</integer>
+               </dict>
                <key>13d3_3404</key>

In other words, allow the firmware to be unspecified and then do no checking/uploading... just handle the publishing/unpublishing/unloading, and it makes bluetooth come back faster after sleep...

the-darkvoid commented 9 years ago

Continuous reading was initially experimental, but I think its the right way forward. I have not heard any reports of it not working since I have switched the released versions to that branch.

Ideally its more resistant in case USB devices are not immediately available or timing out. The normal branch is a bit more time sensitive and failure prone.

So ideally we should indeed merge continuous read into master and deprecate the existing master functionality.

the-darkvoid commented 9 years ago

I now merged continuous read (oops about the typo in the branch name) into the master branch. Next I will see how easily your changes are ported into the continuous merge functionality.

RehabMan commented 9 years ago

Let me know if you'd rather I redo my pull request.

the-darkvoid commented 9 years ago

I will see what I can do. Let me go through the changes and see how easy it is to apply to the continuous branch.

Only 2 files different between both branches are BrcmPatchRam.cpp and BrcmPatchRam.h, so only those need manual merging.

the-darkvoid commented 9 years ago

Your code is now merged with the continuous read functionality. It seems to work as intended. I will review it some more and make a release build after that.

Regarding your other remark, yes BrcmPatchRam can be used where a firmware is not configured for a bluetooth device (simply because it does not require one), but still enables the device by publishing its personality into the IOCatalogue.

RehabMan commented 9 years ago

Your code is now merged with the continuous read functionality.

Thanks! I'll test too.

Regarding your other remark, yes BrcmPatchRam can be used where a firmware is not configured for a bluetooth device (simply because it does not require one), but still enables the device by publishing its personality into the IOCatalogue.

I think you missed my point. Yes it can be used to publish the personality... but with my changes, it causes bluetooth to 'wake up' faster because of the unload of BroadcomBluetooth prior to sleep.

It calls for a refactor of the kexts. The BrcmFirmwareStore should probably be placed in a separate injector, so that if your hardware has builtin firmware, there is no need to carry around 1MB+ of firmware in the kernel that will never be used. I'll work on that after I test/review the current code a bit...

the-darkvoid commented 9 years ago

I think I have some approach on how to do what you are suggesting. It would not load any firmwares into the kernel unless the bluetooth device requires it.

RehabMan commented 9 years ago

I think I have some approach on how to do what you are suggesting.

I figure out a nice way...

See this commit: https://github.com/RehabMan/OS-X-BrcmPatchRAM/commit/6f6ad88f5c0c68e58da9885237a386f29551928c

Basic idea: Have BrcmFirmwareStore reside in iocatalog but with a bogus IOProviderClass. When BrcmPatchRAM loads and decides it needs firmware store, publish the real personality based on the bogus one already there (eg. change entry from 'disabled_IOResources' to 'IOResources')

This also has the nice side effect of when you install BrcmPatchRAM without any matching bluetooth devices installed, it never loads at all.