staging: rtl8192u: RFC - harmonisation of rtl819x_HT.h ?
This patch set includes two fairly trivial patches but the third patch is possibly controversial.
There are two files called rtl819x_HT.h
$ find -name rtl819x_HT.h -print ./drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h ./drivers/staging/rtl8192e/rtl819x_HT.h
The two files are very similar but the rtl8192u file includes more, and I think unused, definitions definitions. My, possibly flawed, thinking is that it might be easier to maintain a single file if the two could be merged into a single include file.
As a first step towards this my third patch comments out unused definitions, from ./drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
At this point I have compiled all c files under ./drivers/staging/rtl8192u and all still compile. I may well have missed something, but I assume that no source file outside that sub tree should be including header files from that subtree.
PATCH 1/3 staging: rtl8192u: Use __func__ instead of hardcoded string - Style
Chnaged logging statements to use %s and __func__ instead of hard coding the function name in a string.
Signed-off-by John Whitmore <[email protected]>
--- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +6 -6
@@ -534,7 +534,7 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u//u8 bIsDeclareMCS13; if (!posHTCap || !pHT) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTCap or pHTInfo can't be null in HTConstructCapabilityElement()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTCap or pHTInfo can't be null in %s\n" , __func__);return ; }memset (posHTCap,0 , *len);
@@ -645,7 +645,7 @@ void HTConstructInfoElement(struct ieee80211_device *ieee, u8 *posHTInfo, u8 *lePHT_INFORMATION_ELE pHTInfoEle = (PHT_INFORMATION_ELE )posHTInfo;if (!posHTInfo || !pHTInfoEle) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTInfo or pHTInfoEle can't be null in HTConstructInfoElement()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTInfo or pHTInfoEle can't be null in %s\n" , __func__);return ; }
@@ -709,7 +709,7 @@ void HTConstructInfoElement(struct ieee80211_device *ieee, u8 *posHTInfo, u8 *levoid HTConstructRT2RTAggElement (struct ieee80211_device *ieee,u8 *posRT2RTAgg,u8 *len) {if (!posRT2RTAgg) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posRT2RTAgg can't be null in HTConstructRT2RTAggElement()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posRT2RTAgg can't be null in %s\n" , __func__);return ; }memset (posRT2RTAgg,0 , *len);
@@ -758,7 +758,7 @@ static u8 HT_PickMCSRate(struct ieee80211_device *ieee, u8 *pOperateMCS)u8 i;if (!pOperateMCS) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"pOperateMCS can't be null in HT_PickMCSRate()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"pOperateMCS can't be null in %s\n" , __func__);return false; }
@@ -820,7 +820,7 @@ u8 HTGetHighestMCSRate(struct ieee80211_device *ieee, u8 *pMCSRateSet, u8 *pMCSFu8 availableMcsRate[16 ];if (!pMCSRateSet || !pMCSFilter) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"pMCSRateSet or pMCSFilter can't be null in HTGetHighestMCSRate()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"pMCSRateSet or pMCSFilter can't be null in %s\n" , __func__);return false; }for (i =0 ; i <16 ; i++)
@@ -900,7 +900,7 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)static u8 EWC11NHTInfo[] = {0x00 ,0x90 ,0x4c ,0x34 };// For 11n EWC definition, 2007.07.17, by Emily if (!pHTInfo->bCurrentHTSupport) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"<=== HTOnAssocRsp(): HT_DISABLE\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"<=== %s: HT_DISABLE\n" , __func__);return ; }IEEE80211_DEBUG (IEEE80211_DL_HT ,"===> HTOnAssocRsp_wq(): HT_ENABLE\n" );
Signature · 1 line
2.17.1
Greg KH
maintainerRe: [PATCH 1/3] staging: rtl8192u: Use __func__ instead of hardcoded string - Style
Quoted 13 lines · John Whitmore
Chnaged logging statements to use %s and
__func__instead of hard coding the function name in a string.Signed-off-by John Whitmore <[email protected]>--- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +0
-0
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u · line 537
Quoted 5 lines
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +1
-1@@ -534,7 +534,7 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u//u8 bIsDeclareMCS13; if (!posHTCap || !pHT) { -IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTCap or pHTInfo can't be null in HTConstructCapabilityElement()\n" ); +IEEE80211_DEBUG (IEEE80211_DL_ERR ,"posHTCap or pHTInfo can't be null in %s\n" , __func__);
You should properly line-wrap thse lines, otherwise you will have to go back and fix them after this patch.
thanks,
greg k-h
A message that is not in the archive (sent off-list or not yet received): CAA_1HzEkZZmE1skOs=mLNFfKf9aBnR_bunddh_fcYX=RhcprzA@mail.gmail.com
Greg KH
maintainerRe: [PATCH 1/3] staging: rtl8192u: Use __func__ instead of hardcoded string - Style
Quoted 1 line · John Whitmore
I'll fix that up and re-submit
Please do not top-post, or send html email, as the mailing lists will reject it (as you saw...)
greg k-h
PATCH 2/3 staging: rtl8192u Remove redundant #include directive
In reply to John WhitmoreThe file includes the file rtl819x_HT.h, which has already been included by the previously included file ieee80211.h
Signed-off-by John Whitmore <[email protected]>
--- drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c +1 -1
@@ -5,7 +5,7 @@* little changed. If any confusion caused, tell me. Created by WB. 2008.05.08 */ #include "ieee80211.h" -#include "rtl819x_HT.h" +u8 MCS_FILTER_ALL [16 ] = {0xff ,0xff ,0xff ,0xff ,0xff ,0xff ,0xff ,0xff ,0xff ,0x1f ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 };u8 MCS_FILTER_1SS [16 ] = {0xff ,0x00 ,0x00 ,0x01 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 };
Signature · 1 line
2.17.1
PATCH 3/3 staging: rtl8192u: Prune the rtl819x_HT.h file of unused definitions.
In reply to John WhitmoreThere are two files named "rtl819x_HT.h"
$ find . -name rtl819x_HT.h -print ./drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h ./drivers/staging/rtl8192e/rtl819x_HT.h
The two files are very similar but differ slightly. Unsed definitions have been removed from "drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h" as a first step towards possibly merging the two files into one.
Signed-off-by John Whitmore <[email protected]>
--- .../staging/rtl8192u/ieee80211/rtl819x_HT.h | 190 +++++++++--------- 1 file changed, 93 insertions(+), 97 deletions(-)
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h +93 -97
@@ -10,17 +10,17 @@// // Operation mode value // -#define HT_OPMODE_NO_PROTECT 0 -#define HT_OPMODE_OPTIONAL 1 -#define HT_OPMODE_40MHZ_PROTECT 2 -#define HT_OPMODE_MIXED 3 +//#define HT_OPMODE_NO_PROTECT 0 +//#define HT_OPMODE_OPTIONAL 1 +//#define HT_OPMODE_40MHZ_PROTECT 2 +//#define HT_OPMODE_MIXED 3 // // MIMO Power Save Settings // #define MIMO_PS_STATIC 0 -#define MIMO_PS_DYNAMIC 1 -#define MIMO_PS_NOLIMIT 3 +//#define MIMO_PS_DYNAMIC 1 +//#define MIMO_PS_NOLIMIT 3 //
@@ -35,26 +35,25 @@#define HT_SUPPORTED_MCS_2SS_BITMAP 0x0000ff00#define HT_SUPPORTED_MCS_1SS_2SS_BITMAP HT_MCS_1SS_BITMAP|HT_MCS_1SS_2SS_BITMAP - -typedef enum _HT_MCS_RATE { -HT_MCS0 =0x00000001 , -HT_MCS1 =0x00000002 , -HT_MCS2 =0x00000004 , -HT_MCS3 =0x00000008 , -HT_MCS4 =0x00000010 , -HT_MCS5 =0x00000020 , -HT_MCS6 =0x00000040 , -HT_MCS7 =0x00000080 , -HT_MCS8 =0x00000100 , -HT_MCS9 =0x00000200 , -HT_MCS10 =0x00000400 , -HT_MCS11 =0x00000800 , -HT_MCS12 =0x00001000 , -HT_MCS13 =0x00002000 , -HT_MCS14 =0x00004000 , -HT_MCS15 =0x00008000 , -// Do not define MCS32 here although 8190 support MCS32 -}HT_MCS_RATE , *PHT_MCS_RATE ; +//typedef enum _HT_MCS_RATE { +// HT_MCS0 = 0x00000001, +// HT_MCS1 = 0x00000002, +// HT_MCS2 = 0x00000004, +// HT_MCS3 = 0x00000008, +// HT_MCS4 = 0x00000010, +// HT_MCS5 = 0x00000020, +// HT_MCS6 = 0x00000040, +// HT_MCS7 = 0x00000080, +// HT_MCS8 = 0x00000100, +// HT_MCS9 = 0x00000200, +// HT_MCS10 = 0x00000400, +// HT_MCS11 = 0x00000800, +// HT_MCS12 = 0x00001000, +// HT_MCS13 = 0x00002000, +// HT_MCS14 = 0x00004000, +// HT_MCS15 = 0x00008000, +// // Do not define MCS32 here although 8190 support MCS32 +//} HT_MCS_RATE, *PHT_MCS_RATE; // // Represent Channel Width in HT Capabilities
@@ -120,27 +119,26 @@ typedef union _HT_CAPABILITY_MACPARA{}HT_CAPABILITY_MACPARA, *PHT_CAPABILITY_MACPARA; */ -typedef enum _HT_ACTION { -ACT_RECOMMAND_WIDTH =0 , -ACT_MIMO_PWR_SAVE =1 , -ACT_PSMP =2 , -ACT_SET_PCO_PHASE =3 , -ACT_MIMO_CHL_MEASURE =4 , -ACT_RECIPROCITY_CORRECT =5 , -ACT_MIMO_CSI_MATRICS =6 , -ACT_MIMO_NOCOMPR_STEER =7 , -ACT_MIMO_COMPR_STEER =8 , -ACT_ANTENNA_SELECT =9 , -}HT_ACTION , *PHT_ACTION ; - +//typedef enum _HT_ACTION { +// ACT_RECOMMAND_WIDTH = 0, +// ACT_MIMO_PWR_SAVE = 1, +// ACT_PSMP = 2, +// ACT_SET_PCO_PHASE = 3, +// ACT_MIMO_CHL_MEASURE = 4, +// ACT_RECIPROCITY_CORRECT = 5, +// ACT_MIMO_CSI_MATRICS = 6, +// ACT_MIMO_NOCOMPR_STEER = 7, +// ACT_MIMO_COMPR_STEER = 8, +// ACT_ANTENNA_SELECT = 9, +//} HT_ACTION, *PHT_ACTION; /* 2007/06/07 MH Define sub-carrier mode for 40MHZ. */ -typedef enum _HT_Bandwidth_40MHZ_Sub_Carrier { -SC_MODE_DUPLICATE =0 , -SC_MODE_LOWER =1 , -SC_MODE_UPPER =2 , -SC_MODE_FULL40MHZ =3 , -}HT_BW40_SC_E ; +//typedef enum _HT_Bandwidth_40MHZ_Sub_Carrier { +// SC_MODE_DUPLICATE = 0, +// SC_MODE_LOWER = 1, +// SC_MODE_UPPER = 2, +// SC_MODE_FULL40MHZ = 3, +//}HT_BW40_SC_E; typedef struct _HT_CAPABILITY_ELE {
@@ -216,11 +214,11 @@ typedef struct _HT_INFORMATION_ELE {// MIMO Power Save control field. // This is appear in MIMO Power Save Action Frame // -typedef struct _MIMOPS_CTRL { -u8 MimoPsEnable:1 ; -u8 MimoPsMode:1 ; -u8 Reserved:6 ; -}MIMOPS_CTRL , *PMIMOPS_CTRL ; +//typedef struct _MIMOPS_CTRL { +// u8 MimoPsEnable:1; +// u8 MimoPsMode:1; +// u8 Reserved:6; +//} MIMOPS_CTRL, *PMIMOPS_CTRL; typedef enum _HT_SPEC_VER { HT_SPEC_VER_IEEE = 0,
@@ -348,26 +346,26 @@ typedef struct _RT_HIGH_THROUGHPUT {// when card is configured as "AP mode" //------------------------------------------------------------ -typedef struct _RT_HTINFO_STA_ENTRY { -u8 bEnableHT; - -u8 bSupportCck; - -u16 AMSDU_MaxSize; - -u8 AMPDU_Factor; -u8 MPDU_Density; - -u8 HTHighestOperaRate; - -u8 bBw40MHz; - -u8 MimoPs; - -u8 McsRateSet[16 ]; - - -}RT_HTINFO_STA_ENTRY , *PRT_HTINFO_STA_ENTRY ; +//typedef struct _RT_HTINFO_STA_ENTRY { +// u8 bEnableHT; +// +// u8 bSupportCck; +// +// u16 AMSDU_MaxSize; +// +// u8 AMPDU_Factor; +// u8 MPDU_Density; +// +// u8 HTHighestOperaRate; +// +// u8 bBw40MHz; +// +// u8 MimoPs; +// +// u8 McsRateSet[16]; +// +// +//}RT_HTINFO_STA_ENTRY, *PRT_HTINFO_STA_ENTRY;
@@ -396,26 +394,26 @@ typedef struct _BSS_HT {u8 bdRT2RTLongSlotTime; } __attribute__ ((packed)) BSS_HT, *PBSS_HT; -typedef struct _MIMO_RSSI { -u32 EnableAntenna; -u32 AntennaA; -u32 AntennaB; -u32 AntennaC; -u32 AntennaD; -u32 Average; -}MIMO_RSSI , *PMIMO_RSSI ; +//typedef struct _MIMO_RSSI { +// u32 EnableAntenna; +// u32 AntennaA; +// u32 AntennaB; +// u32 AntennaC; +// u32 AntennaD; +// u32 Average; +//}MIMO_RSSI, *PMIMO_RSSI; -typedef struct _MIMO_EVM { -u32 EVM1; -u32 EVM2; -}MIMO_EVM , *PMIMO_EVM ; +//typedef struct _MIMO_EVM { +// u32 EVM1; +// u32 EVM2; +//}MIMO_EVM, *PMIMO_EVM; -typedef struct _FALSE_ALARM_STATISTICS { -u32 Cnt_Parity_Fail; -u32 Cnt_Rate_Illegal; -u32 Cnt_Crc8_fail; -u32 Cnt_all; -}FALSE_ALARM_STATISTICS , *PFALSE_ALARM_STATISTICS ; +//typedef struct _FALSE_ALARM_STATISTICS { +// u32 Cnt_Parity_Fail; +// u32 Cnt_Rate_Illegal; +// u32 Cnt_Crc8_fail; +// u32 Cnt_all; +//}FALSE_ALARM_STATISTICS, *PFALSE_ALARM_STATISTICS; extern u8 MCS_FILTER_ALL [16 ];
@@ -424,16 +422,14 @@ extern u8 MCS_FILTER_1SS[16];/* 2007/07/11 MH Modify the macro. Becaus STA may link with a N-AP. If we set STA in A/B/G mode and AP is still in N mode. The macro will be wrong. We have to add a macro to judge wireless mode. */ -#define PICK_RATE (_nLegacyRate, _nMcsRate) \ - (_nMcsRate==0)?(_nLegacyRate&0x7f):(_nMcsRate) +#define PICK_RATE (_nLegacyRate, _nMcsRate) \ + (_nMcsRate==0)?(_nLegacyRate&0x7f):(_nMcsRate)/* 2007/07/12 MH We only define legacy and HT wireless mode now. */ #define LEGACY_WIRELESS_MODE IEEE_MODE_MASK - -#define CURRENT_RATE (WirelessMode, LegacyRate, HTRate) \ - ((WirelessMode & (LEGACY_WIRELESS_MODE))!=0)?\ - (LegacyRate):\ - (PICK_RATE(LegacyRate, HTRate)) - +#define CURRENT_RATE (WirelessMode, LegacyRate, HTRate) \ + ((WirelessMode & (LEGACY_WIRELESS_MODE))!=0)? \ + (LegacyRate): \ + (PICK_RATE(LegacyRate, HTRate))// MCS Bw 40 {1~7, 12~15,32}
@@ -441,7 +437,7 @@ extern u8 MCS_FILTER_1SS[16];#define RATE_ADPT_2SS_MASK 0xF0 //Skip MCS8~11 because mcs7 > mcs6, 9, 10, 11. 2007.01.16 by Emily#define RATE_ADPT_MCS32_MASK 0x01 -#define IS_11N_MCS_RATE (rate) (rate&0x80) +//#define IS_11N_MCS_RATE(rate) (rate&0x80) typedef enum _HT_AGGRE_SIZE { HT_AGG_SIZE_8K = 0,
Signature · 1 line
2.17.1
Re: [PATCH 3/3] staging: rtl8192u: Prune the rtl819x_HT.h file of unused definitions.
Quoted 11 lines · John Whitmore
There are two files named "
rtl819x_HT.h"$ find . -name rtl819x_HT.h -print ./drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h ./drivers/staging/rtl8192e/rtl819x_HT.hThe two files are very similar but differ slightly. Unsed definitions have
been removed from "drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h" as a first
step towards possibly merging the two files into one.
Just delete the lines if they are unused. Don't worry about commenting them out. That way it is easier to visually diff the patches for review. (Plus, the previous code is never lost in the git tree, anyway)
:-)
Justin