cilynx / rtl88x2bu

rtl88x2bu driver updated for current kernels.
http://www.wolfteck.com/2018/02/22/wsky_1200mbps_wireless_usb_wifi_adapter/
GNU General Public License v2.0
1.63k stars 318 forks source link

Removing unused variables #223

Open sevan opened 1 year ago

sevan commented 1 year ago

Locally I have managed to reach building the driver without -Wno-unused-variable and just having -Werror turned on to catch all the unused variables which gcc flagged up. There is a combination of stuff that's just not referenced and can be deleted, stuff that's reliant on CONFIG_RTW_DEBUG for macros to work which reference the variables, and things that need guarding off for specific configuration definition. This pull request starts off covering these cases. locally there are changes that are bodges just to skip over things but as I said, I do have the driver building and working without -Wno-unused-variableand just having -Werror, the build is silent.

MaxG87 commented 1 year ago

Thank you for your contribution, I appreciate it very much.

I too think that the source code is in a shape that requires some clean-ups. I have tried to make only the most necessary adjustments to avoid merge conflicts. However, with RTW88-USB raising, I do not think that a new revision of RTL88x2BU will appear, so fixing the issues that annoy us might be a good thing.

If we go this route, some other adaptions come to my mind too. For instance, I would get rid of the support for other operating systems (e.g. PLATFORM_WINDOWS preprocessor directives) and drop support for old Kernel versions too. Especially these accumulated guards for old Kernel versions are a bit annoying to me when I implement support for a recent version.

Thus, before I give your version a try and merge it if it runs for me, I would like to have the agreement of another collaborator here. @cilynx , @CGarces , what would you say?

CGarces commented 1 year ago

Cleanup a RTL driver is a lot of work.

As reference you can see the evolution of rtl8723bs to land in the linux kernel as staging driver. 805 commits of really boring work. Plus hundreds of commits to replace the custom RTL code to the equivalent functions of Linux kernel (check r8188eu)

Tools like unifdef or coccinelle maybe can help you to made the work faster.

I tryed to clean up rtl8189fs to maybe reach the staging area of Linux kernel but after 60+ commits I lost interest.

If RTW88-USB adds support to rtl88x2bu chipset, maybe is a better idea to contribute directly to the Linux kernel. Contribute to linux kernel sound complicated but sometimes is just a single line of code like #180

CGarces commented 1 year ago

BTW the PR looks fine for me, but should include Makefile change to remove -Wno-unused-variable

sevan commented 1 year ago

This change is incomplete for removing -Wno-unused-variable from the Makefile. This removes all the obvious unused variables. I have local changes which I've not committed which allow me to complete the driver compile and that's how I identified these variables in this PR.

I'll take a look at the upstream process / cleanup in the repos you pointed to @CGarces.

sevan commented 1 year ago

@MaxG87 do you want to merge this?