2018-06-29 20:39:51

by John Whitmore

[permalink] [raw]
Subject: 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.





2018-06-29 20:07:51

by John Whitmore

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

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 <[email protected]>
---
.../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


2018-06-29 20:09:07

by John Whitmore

[permalink] [raw]
Subject: [PATCH 2/3] staging: rtl8192u Remove redundant #include directive

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 <[email protected]>
---
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


2018-06-29 20:39:57

by John Whitmore

[permalink] [raw]
Subject: [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(-)

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


2018-06-30 08:35:32

by Justin Skists

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



On 29 June 2018 19:10:07 BST, John Whitmore <[email protected]> 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





2018-07-02 09:39:52

by Greg Kroah-Hartman

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

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

2018-07-02 12:36:09

by Greg Kroah-Hartman

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

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 <[email protected]>
> ---
> 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