On 4/13/22 14:42, Michael Straube wrote:
On 4/13/22 18:27, Michael Straube wrote:Hi all, I think the rt_firmware_hdr structure in rtw_fw.c has some issues. struct rt_firmware_hdr { /* 8-byte alinment required */ /* LONG WORD 0 ---- */ __le16 Signature; /* 92C0: test chip; 92C, * 88C0: test chip; 88C1: MP A-cut; * 92C1: MP A-cut */ u8 Category; /* AP/NIC and USB/PCI */ u8 Function; /* Reserved for different FW function * indcation, for further use when * driver needs to download different * FW for different conditions */ __le16 Version; /* FW Version */ u8 Subversion; /* FW Subversion, default 0x00 */ u16 Rsvd1; /* LONG WORD 1 ---- */ u8 Month; /* Release time Month field */ u8 Date; /* Release time Date field */ u8 Hour; /* Release time Hour field */ u8 Minute; /* Release time Minute field */ __le16 RamCodeSize; /* The size of RAM code */ u8 Foundry; u8 Rsvd2; /* LONG WORD 2 ---- */ __le32 SvnIdx; /* The SVN entry index */ u32 Rsvd3; /* LONG WORD 3 ---- */ u32 Rsvd4; u32 Rsvd5; }; Then we have in rtl8188e_firmware_download(): fwhdr = (struct rt_firmware_hdr *)dvobj->firmware.data; <snip> if (IS_FW_HEADER_EXIST(fwhdr)) { /* Shift 32 bytes for FW header */ fw_data = fw_data + 32; fw_size = fw_size - 32; } We add/sub 32 bytes but the size of struct rt_firmware_hdr is actually 33 bytes. I noticed this when I wanted to replace: fw_data = fw_data + 32; fw_size = fw_size - 32; with: fw_data = fw_data + sizeof(struct rt_firmware_hdr); fw_size = fw_size - sizeof(struct rt_firmware_hdr);; To me it looks add/sub 32 is correct here but the struct is wrong. I don't know if the firmware for this driver is so much different from firmware for the drivers in drivers/net/wireless/realtek/rtlwifi. They use a struct of size 32. Also, souldn't the u16 and u32 variables in the struct be __le16 and __le32 ? I wonder if we can just use the rtlwifi_firmware_header structure from drivers/net/wireless/realtek/rtlwifi/wifi.h ? Comments from people with better knowledge appreciated. :) regards, MichaelOk, I figured it out by looking at the hexdumps of firmware files. The field Rsvd1 should be u8 instead of u16. I'll prepare a patch for this.
I agree. I had prepared an analysis, but did not get it mailed before I got this one.
Larry
