diederikdehaas / rtl8812AU

Realtek 8812AU USB WiFi driver
Other
475 stars 177 forks source link

Duplicate USB IDs in 4.3.20 and 4.3.22-beta #47

Closed MrEngman closed 5 years ago

MrEngman commented 7 years ago

Hi,

Just been trying your drivers with the latest rpi-update from raspberry pi, kernel 4.8.13, and noticed another duplicate USB ID - 0BDA:A811 in driver-4.3.20 and driver-4,3,22-beta. driver-4.3.14 is OK.

In file os_dep/linux/usb_intf.c

Line 181 - {USB_DEVICE(USB_VENDER_ID_REALTEK, 0xA811) , .driver_info = RTL8821},/* Default ID */

and line 190 - {USB_DEVICE(0x0BDA, 0xA811),.driver_info = RTL8821}, /* OUTLINK - Edimax */

both create the USB Id 0BDA:A811

Tried updating the driver I was using, 4.3.14, and it crashed with 4.8.13 so decided to try yours. 4.3.14, 4.3.20 and 4.3.22-beta compile. No warnings or errors compiling 4.3.14 or 4.3.20 but 4.3.22-beta gives a couple of warnings, all versions cross compiled on a PC using Ubuntu.

  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_edcaturbocheck.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dig.o
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dig.c: In function ‘odm_PauseDIG’:
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dig.c:573:4: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    for (max_level = (pause_level - 1); max_level >= 0; max_level--) {
    ^
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dig.c: In function ‘odm_PauseCCKPacketDetection’:
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dig.c:1737:4: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    for (max_level = (pause_level - 1); max_level >= 0; max_level--) {
    ^
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_pathdiv.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_rainfo.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dynamicbbpowersaving.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_powertracking_ce.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_dynamictxpower.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_adaptivity.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_cfotracking.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_noisemonitor.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_acs.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_beamforming.o
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_beamforming.c: In function ‘Beamforming_GetHTNDPTxRate’:
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_beamforming.c:117:3: warning: comparison is always false due to limited range of data type [-Wtype-limits]
   Nr_index = TxBF_Nr(halTxbf8814A_GetNtx(pDM_Odm), CompSteeringNumofBFer);
   ^
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_beamforming.c: In function ‘Beamforming_GetVHTNDPTxRate’:
/home/richard/src-4.8/rtl8812AU/hal/phydm/phydm_beamforming.c:154:3: warning: comparison is always false due to limited range of data type [-Wtype-limits]
   Nr_index = TxBF_Nr(halTxbf8814A_GetNtx(pDM_Odm), CompSteeringNumofBFer);
   ^
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/txbf/halcomtxbf.o
  CC [M]  /home/richard/src-4.8/rtl8812AU/hal/phydm/txbf/haltxbfinterface.o

Could not try 4.3.20 as I do not have a wifi device supported by that version but tried 4.3.14 and currently running 4.3.22-beta on a Pi 2B and so far its been OK although the warning during compiling is a little worrying, mind you my knowledge of C is almost none ;). Using an Edimax EW-7811UTC. USB id 7392:A812, on a Pi 2B.

However, a good job. Keep up the good work and I will be using your driver from now on whenever a new kernel version is available via Hexxeh/rpi-firmware. If you ever need any specific testing let me know.

MrEngman

diederikdehaas commented 6 years ago

Thanks again :+1: I had removed the ID from the 4.3.20 branch (the new default) and when I think that branch is fully up-to-date, I'll try to backport the fixes to the 4.22-beta branch. Maybe even 4.3.14 though not sure about that one.

diederikdehaas commented 6 years ago

I think I've figured out why you see those warning. I've changed the Makefile in 4.3.22-beta to be much stricter/noisier when compiling. Most relevant differences between 4.3.22-beta and 4.3.20:

diff --git a/Makefile b/Makefile
index 25d6906..6e091b9 100755
--- a/Makefile
+++ b/Makefile
@@ -1,12 +1,20 @@
 EXTRA_CFLAGS += $(USER_EXTRA_CFLAGS)
 EXTRA_CFLAGS += -O1
 #EXTRA_CFLAGS += -O3
-#EXTRA_CFLAGS += -Wall
-#EXTRA_CFLAGS += -Wextra
+EXTRA_CFLAGS += -Wall
+EXTRA_CFLAGS += -Wextra
 #EXTRA_CFLAGS += -Werror
 #EXTRA_CFLAGS += -pedantic
 #EXTRA_CFLAGS += -Wshadow -Wpointer-arith -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes

+# The "$(call cc-option,-Wxxx)" macro only includes that option when it's
+# supported by the compiler used. It may only work on Debian systems.
+
+# Wdate-time was added in gcc-4.9
+EXTRA_CFLAGS += $(call cc-option,-Werror=date-time)
+# Wincompatible-pointer-types was added in gcc-5.0
+EXTRA_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
+
 EXTRA_CFLAGS += -Wno-unused-variable
 EXTRA_CFLAGS += -Wno-unused-value
 EXTRA_CFLAGS += -Wno-unused-label
@@ -16,6 +24,10 @@ EXTRA_CFLAGS += -Wno-unused
 #EXTRA_CFLAGS += -Wno-uninitialized
 #EXTRA_CFLAGS += -Wno-error=date-time  # Fix compile error on gcc 4.9 and later

+# Relax some warnings from '-Wextra' so we won't get flooded with warnings
+EXTRA_CFLAGS += -Wno-sign-compare
+EXTRA_CFLAGS += -Wno-missing-field-initializers
MrEngman commented 6 years ago

Hi diederikdehaas,

Just cloned your latest commits and am now able to compile 4.3.20 and 4.3.22-beta for the raspberry pi 4.14 kernels.

Great job you're doing, well done.

Just one minor issue.

Compiling 4.3.20 and 4.3.22-beta have a warning when compiling for armv7l.

  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm.o
  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/halphyrf_ce.o
  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_edcaturbocheck.o
/home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_hwconfig.c: In function ‘odm_RxPhyStatusJaguarSeries_Parsing’:
/home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_hwconfig.c:1543:40: warning: ‘EVM’ may be used uninitialized in this function [-Wmaybe-uninitialized]
       pPhyInfo->RxMIMOSignalQuality[i] = EVM;
                                        ^
  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_dig.o
  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_pathdiv.o
  CC [M]  /home/pi/wifi-rtl8812AU/diederikdehaas/rtl8812AU/hal/phydm/phydm_rainfo.o

This can be fixed with the following patch

diff -Naur '--exclude=.git' rtl8812AU-orig/hal/phydm/phydm_hwconfig.c rtl8812AU/hal/phydm/phydm_hwconfig.c
--- rtl8812AU-orig/hal/phydm/phydm_hwconfig.c   2018-03-12 12:57:52.854736002 +0000
+++ rtl8812AU/hal/phydm/phydm_hwconfig.c        2018-03-12 13:08:11.042165029 +0000
@@ -1207,7 +1207,7 @@
 {
        u1Byte                                  i, Max_spatial_stream;
        s1Byte                                  rx_pwr[4], rx_pwr_all = 0;
-       u1Byte                                  EVM, EVMdbm, PWDB_ALL = 0, PWDB_ALL_BT;
+       u1Byte                                  EVM = 0, EVMdbm, PWDB_ALL = 0, PWDB_ALL_BT;
        u1Byte                                  RSSI, avg_rssi = 0, best_rssi = 0, second_rssi = 0;
        u1Byte                                  isCCKrate = 0;
        u1Byte                                  rf_rx_num = 0;
diederikdehaas commented 6 years ago

It's indeed the one that has remained open. See the (full) commit message of https://github.com/diederikdehaas/rtl8812AU/commit/dab3c024b9fab3e628d8e719dcd941b880f9d2c1

It will make the warning go away and 0 is indeed often used to initialize a variable. But does that make it an appropriate value (also) in this case? The EVMdbm variable is only declared, but not initialized for example.

I may apply the patch, but I actually don't mind warnings as they indicate a potential issue in the code and it really does in this case. Sometimes I do suppress them (https://github.com/diederikdehaas/rtl8812AU/commit/85106905a4f2b6d5b6fedcddf070209db27ed608) if they clutter up the output too much, but I do have the habit of then detailing explicitly why and including a reminder to myself (and hopefully others) that it actually should be fixed (and not ignored as I usually see).

diederikdehaas commented 6 years ago

I think that 4.3.20 is the best version (less features, but more stable then 4.3.22-beta), but now that I'm getting the hang of it again, I may also update the 4.3.14 branch. Then I'm going to work on quite a new version ;-)

diederikdehaas commented 6 years ago

I just pushed a fix to the 4.3.20 and 4.3.22-beta branch 'fixing' this issue. I did it by assigning it with '0' in the place where the issue was and with a snarky comment ;-)