pvaret / rtl8192cu-fixes

Realtek 8192 chipset driver, ported to kernel 3.11.
800 stars 208 forks source link

Merge in changes for kernel 4.15.0 #132

Closed rburcham closed 6 years ago

rburcham commented 6 years ago

313eacc contains the material changes for the port to kernel 4.15.0. These changes may merit a version bump in the driver itself so folks can determine at a glance if they have a 4.15.0 candidate module or not. Your discretion.

I also think it worthwhile to update the README to reflect the fact that this 8192cu driver remains the superior option for many usb dongles that simply do not function well or at all with either the rtl8192cu driver or the rtl8xxxu driver.

Subsequent commits contain my attempts at communicating this, based in my forked repo.

pvaret commented 6 years ago

Thanks for the PR! Two things.

  1. Rather than changing the README with things we'd roll back, can you send the PR without those changes in the first place? If you understandably don't want to remove them from your own repo, you can send the PR from a dedicated branch rather than master.

  2. As you can tell from the continuous integration results, the driver doesn't build. :) That's a blocker for merging. On that note it would be nice to have a test for kernel 4.15 added, but I can do it if you'd like.

rburcham commented 6 years ago

Fixed the pre-4.15.0 build regression and applied @CGarces suggestions. Yes, adding a test for 4.15.0, specifically with gcc-7.3.0 would be appreciated - I don't have an ubuntu env to test with.

As for the README, I humbly submit that a revision to the current would help to clarify that yes, the 8192cu driver isn't explicitly being maintained, but also that for CU cases in which the in-kernel drivers rtl8192cu and rtl8xxxu simply don't work or perform poorly, this driver can be a good alternative.

I think there's some better wordsmithing that can help there :) Your call. There are a few people out there who have hunted for the right driver and have suffered similarly, like these guys:

https://archlinuxarm.org/forum/viewtopic.php?f=60&t=11391

CGarces commented 6 years ago

@rburcham To add a test for 4.15 you don't need a linux machine, Travis CI will do it for you.

pvaret commented 6 years ago

I already added a test for kernel 4.15 in branch travis-update. I'll merge into master after this PR.

As to the README, there are things in the PR as it currently stands that I'll definitely have to roll back, including but not limited to the part that says "So I have forked pvaret's 8192cu driver". :)

So, I'd really rather the PR saved me the trouble and only contained the code change. I'm editing the README in master right now to include those of your suggestions that are relevant, and not having to disentangle the other changes in the PR would make my life that much easier. Thanks. :)

pvaret commented 6 years ago

There, I updated the README. I hope it does a fair job of reflecting your point.

I'll also increase the version number after merging the revised PR, that's a good suggestion.

As to that Arch thread, that sounds very much like a power management issue, BTW. Power management is the bane of WiFi dongles.

rburcham commented 6 years ago

Applied/committed your README changes. Sorry, I should have cherry-picked to begin with.