staging: rtl8192u: RFC - harmonisation of rtl819x_HT.h ?

staging: rtl8192u: RFC - harmonisation of rtl819x_HT.h ?

[John Whitmore]

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

[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 <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 1dd4c6ae7319..a4fbc0435de5 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -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 *le
 	PHT_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 *le
 void 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 *pMCSF
 	u8		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");
-- 
2.17.1

Re: [PATCH 1/3] staging: rtl8192u: Use __func__ instead of hardcoded string - Style

[Greg KH]

On Fri, Jun 29, 2018 at 07:10:05PM +0100, John Whitmore wrote:
> Chnaged logging statements to use %s and __func__ instead of hard coding the
> function name in a string.
> 
> Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> index 1dd4c6ae7319..a4fbc0435de5 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
> @@ -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

Re: [PATCH 1/3] staging: rtl8192u: Use __func__ instead of hardcoded string - Style

[Greg KH]

On Mon, Jul 02, 2018 at 09:59:34AM +0100, John Whitmore wrote:
> 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

[John Whitmore]

The file includes the file rtl819x_HT.h, which has already been included by
the previously included file ieee80211.h

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index a4fbc0435de5..3ce9ab6ab64f 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -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};
-- 
2.17.1

[PATCH 3/3] staging: rtl8192u: Prune the rtl819x_HT.h file of unused definitions.

[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.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 <johnfwhitmore@gmail.com>
---
 .../staging/rtl8192u/ieee80211/rtl819x_HT.h   | 190 +++++++++---------
 1 file changed, 93 insertions(+), 97 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index a85036022aa8..30a00c73dd06 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -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,
-- 
2.17.1

Re: [PATCH 3/3] staging: rtl8192u: Prune the rtl819x_HT.h file of unused definitions.

[Justin Skists]

On 29 June 2018 19:10:07 BST, John Whitmore <johnfwhitmore@gmail.com> wrote:
>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.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.

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