2018-09-26 19:20:05

by John Whitmore

[permalink] [raw]
Subject: [PATCH 00/13] staging:rtl8192u: Style & memory leak fix

Back again. I previously went through the struct ht_capability_ele
renaming member variables as being unused. At the time I wasn't
100% sure that the structure wasn't memory mapped, or used by
external callers to the module, through function pointers or exported
symbols. Having had a more detailed look over the code I'm happy
that these member variables are not used and have removed them from
the code.

The last patch is a potential memory leak, which only happens if two
memory allocations succeed and the third allocation fails. So it's
probably never going to happen, but fixed it anyhow.

I've tried to find this device, or a datasheet on it, so that I
could actually make sure it still worked but that's not been possible.
Apparently it's a discontinued device, but that possibly makes it a
safer sandbox to start messing with.

John Whitmore (13):
staging:rtl8192u: Remove HTHalfMcsToDataRate() - Style
staging:rtl8192u: Make HTMcsToDataRate static - Style
staging:rtl8192u: Remove definition of HTSetConnectBwMode - Style
staging:rtl8192u: Move HTSetConnectBwModeCallback() - Style
staging:rtl8192u: Remove AdvCoding and GreenField - Style
staging:rtl8192u: Remove TxSTBC and RxSTBC - Style
staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style
staging:rtl8192u: Remove LSigTxopProtect - Style
staging:rtl8192u: Remove Rsvd2 - Style
staging:rtl8192u: Remove ExtHTCapInfo - Style
staging:rtl8192u: Remove TxBFCap - Style
staging:rtl8192u: Remove ASCap - Style
staging:rtl8192u: Remove potential memory leak

.../staging/rtl8192u/ieee80211/ieee80211.h | 3 -
.../rtl8192u/ieee80211/ieee80211_module.c | 5 +
.../staging/rtl8192u/ieee80211/rtl819x_HT.h | 20 +--
.../rtl8192u/ieee80211/rtl819x_HTProc.c | 123 ++++--------------
4 files changed, 30 insertions(+), 121 deletions(-)

--
2.18.0



2018-09-26 19:18:37

by John Whitmore

[permalink] [raw]
Subject: [PATCH 02/13] staging:rtl8192u: Make HTMcsToDataRate static - Style

The function HTMcsToDataRate() is not used outside the file in which
it is defined, so has been declared as 'static' and the prototype
removed from the header file.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/ieee80211.h | 1 -
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index cc694e567040..8c91cb29fc9d 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2382,7 +2382,6 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
//extern void HTSetConnectBwModeCallback(unsigned long data);
void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
-u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
//function in BAPROC.c
int ieee80211_rx_ADDBAReq(struct ieee80211_device *ieee, struct sk_buff *skb);
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index ea1261e33a68..ebe8d527a080 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -216,7 +216,7 @@ void HTDebugHTInfo(u8 *InfoIE, u8 *TitleString)
pHTInfoEle->BasicMSC[1], pHTInfoEle->BasicMSC[2], pHTInfoEle->BasicMSC[3], pHTInfoEle->BasicMSC[4]);
}

-u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
+static u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
{
PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;

--
2.18.0


2018-09-26 19:18:42

by John Whitmore

[permalink] [raw]
Subject: [PATCH 13/13] staging:rtl8192u: Remove potential memory leak

Add call to ieee80211_networks_free() to avoid potential memory
leak if allocation of pHTInfo fails.

If the third allocation fails only the first successful allocation
is freed, not the second.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/ieee80211_module.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..aada077ced0f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -159,6 +159,11 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
ieee->pHTInfo = kzalloc(sizeof(RT_HIGH_THROUGHPUT), GFP_KERNEL);
if (ieee->pHTInfo == NULL) {
IEEE80211_DEBUG(IEEE80211_DL_ERR, "can't alloc memory for HTInfo\n");
+
+ /* By this point in code ieee80211_networks_allocate() has been
+ * successfully called so the memory allocated should be freed
+ */
+ ieee80211_networks_free(ieee);
goto failed;
}
HTUpdateDefaultSetting(ieee);
--
2.18.0


2018-09-26 19:18:50

by John Whitmore

[permalink] [raw]
Subject: [PATCH 12/13] staging:rtl8192u: Remove ASCap - Style

Remove the unused member variable ASCap from code.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 4 ----
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 204e4a5495a1..85a8716c6b05 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -53,10 +53,6 @@ struct ht_capability_ele {

//Supported MCS set
u8 MCS[16];
-
- //Antenna Selection Capabilities
- u8 ASCap;
-
} __packed;

/*
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9188bd925262..4f6e9da465e2 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,8 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
pCapELE->MCS[i] = 0;
}

- //Antenna Selection Capabilities
- pCapELE->ASCap = 0;
//add 2 to give space for element ID and len when construct frames
if (pHT->ePeerHTSpecVer == HT_SPEC_VER_EWC)
*len = 30 + 2;
--
2.18.0


2018-09-26 19:18:57

by John Whitmore

[permalink] [raw]
Subject: [PATCH 08/13] staging:rtl8192u: Remove LSigTxopProtect - Style

Remove the unused member bit LSigTxopProtect. This member is not used
in code so has been replaced with a 'padding' bit, to keep byte
boundary alignment.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 +-
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 09596a7a6814..4154b4e27e59 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -45,7 +45,7 @@ struct ht_capability_ele {
u8 ShortGI40Mhz:1;
u8 MaxAMSDUSize:1;
u8 DssCCk:1;
- u8 LSigTxopProtect:1;
+ u8 padding:1;

//MAC HT parameters info
u8 MaxRxAMPDUFactor:2;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 94cedf5f1163..58c94440aab0 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -505,7 +505,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
pCapELE->MaxAMSDUSize = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
pCapELE->DssCCk = ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
- pCapELE->LSigTxopProtect = 0; // Do not support now!!

/*
* MAC HT parameters info
--
2.18.0


2018-09-26 19:19:01

by John Whitmore

[permalink] [raw]
Subject: [PATCH 10/13] staging:rtl8192u: Remove ExtHTCapInfo - Style

The member variable ExtHTCapInfo is not used in code so has been
removed.

This is a coding style change which should not impact runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 3 ---
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 0c5ded7ad98c..f36fbf56c585 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -54,9 +54,6 @@ struct ht_capability_ele {
//Supported MCS set
u8 MCS[16];

- //Extended HT Capability Info
- u16 ExtHTCapInfo;
-
//TXBF Capabilities
u8 TxBFCap[4];

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 58c94440aab0..e20488ecbd96 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,9 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
pCapELE->MCS[i] = 0;
}

- //Extended HT Capability Info
- memset(&pCapELE->ExtHTCapInfo, 0, 2);
-
//TXBF Capabilities
memset(pCapELE->TxBFCap, 0, 4);

--
2.18.0


2018-09-26 19:19:12

by John Whitmore

[permalink] [raw]
Subject: [PATCH 09/13] staging:rtl8192u: Remove Rsvd2 - Style

Remove the unused member variable Rsvd2 from structure.

This is a coding style change which should not impact runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 4154b4e27e59..0c5ded7ad98c 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -50,7 +50,6 @@ struct ht_capability_ele {
//MAC HT parameters info
u8 MaxRxAMPDUFactor:2;
u8 MPDUDensity:3;
- u8 Rsvd2:3;

//Supported MCS set
u8 MCS[16];
--
2.18.0


2018-09-26 19:19:32

by John Whitmore

[permalink] [raw]
Subject: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

Remove the member variables TxSTBC and RxSTBC as neither is used in
code.

This is a coding style change which should not impact runtime code
execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
2 files changed, 5 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 83fb8f34ccbd..52cce0dcf9a5 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -43,8 +43,6 @@ struct ht_capability_ele {
u8 MimoPwrSave:2;
u8 ShortGI20Mhz:1;
u8 ShortGI40Mhz:1;
- u8 TxSTBC:1;
- u8 RxSTBC:2;
u8 DelayBA:1;
u8 MaxAMSDUSize:1;
u8 DssCCk:1;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index d1fbe65ed428..b4220df1f833 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -145,7 +145,6 @@ void HTDebugHTCapability(u8 *CapIE, u8 *TitleString)
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tSupported Channel Width = %s\n", (pCapELE->ChlWidth) ? "20MHz" : "20/40MHz");
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tSupport Short GI for 20M = %s\n", (pCapELE->ShortGI20Mhz) ? "YES" : "NO");
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tSupport Short GI for 40M = %s\n", (pCapELE->ShortGI40Mhz) ? "YES" : "NO");
- IEEE80211_DEBUG(IEEE80211_DL_HT, "\tSupport TX STBC = %s\n", (pCapELE->TxSTBC) ? "YES" : "NO");
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tMax AMSDU Size = %s\n", (pCapELE->MaxAMSDUSize) ? "3839" : "7935");
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tSupport CCK in 20/40 mode = %s\n", (pCapELE->DssCCk) ? "YES" : "NO");
IEEE80211_DEBUG(IEEE80211_DL_HT, "\tMax AMPDU Factor = %d\n", pCapELE->MaxRxAMPDUFactor);
@@ -504,8 +503,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
pCapELE->ShortGI40Mhz = 1; // We can receive Short GI!!
//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
- pCapELE->TxSTBC = 1;
- pCapELE->RxSTBC = 0;
pCapELE->DelayBA = 0; // Do not support now!!
pCapELE->MaxAMSDUSize = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
pCapELE->DssCCk = ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
--
2.18.0


2018-09-26 19:19:57

by John Whitmore

[permalink] [raw]
Subject: [PATCH 07/13] staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style

Remove the unused member variables DelayBA, PSMP and Rsvd1.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 3 ---
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
2 files changed, 5 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 52cce0dcf9a5..09596a7a6814 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -43,11 +43,8 @@ struct ht_capability_ele {
u8 MimoPwrSave:2;
u8 ShortGI20Mhz:1;
u8 ShortGI40Mhz:1;
- u8 DelayBA:1;
u8 MaxAMSDUSize:1;
u8 DssCCk:1;
- u8 PSMP:1;
- u8 Rsvd1:1;
u8 LSigTxopProtect:1;

//MAC HT parameters info
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index b4220df1f833..94cedf5f1163 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -503,10 +503,8 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
pCapELE->ShortGI40Mhz = 1; // We can receive Short GI!!
//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
//pCapELE->ChlWidth, pCapELE->ShortGI20Mhz, pCapELE->ShortGI40Mhz);
- pCapELE->DelayBA = 0; // Do not support now!!
pCapELE->MaxAMSDUSize = (MAX_RECEIVE_BUFFER_SIZE >= 7935) ? 1 : 0;
pCapELE->DssCCk = ((pHT->bRegBW40MHz) ? (pHT->bRegSuppCCK ? 1 : 0) : 0);
- pCapELE->PSMP = 0; // Do not support now!!
pCapELE->LSigTxopProtect = 0; // Do not support now!!

/*
--
2.18.0


2018-09-26 19:20:00

by John Whitmore

[permalink] [raw]
Subject: [PATCH 11/13] staging:rtl8192u: Remove TxBFCap - Style

The member variable TxBFCap is not used in code so has simply been
removed from the code.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 3 ---
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
2 files changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index f36fbf56c585..204e4a5495a1 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -54,9 +54,6 @@ struct ht_capability_ele {
//Supported MCS set
u8 MCS[16];

- //TXBF Capabilities
- u8 TxBFCap[4];
-
//Antenna Selection Capabilities
u8 ASCap;

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index e20488ecbd96..9188bd925262 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -542,9 +542,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
pCapELE->MCS[i] = 0;
}

- //TXBF Capabilities
- memset(pCapELE->TxBFCap, 0, 4);
-
//Antenna Selection Capabilities
pCapELE->ASCap = 0;
//add 2 to give space for element ID and len when construct frames
--
2.18.0


2018-09-26 19:20:03

by John Whitmore

[permalink] [raw]
Subject: [PATCH 01/13] staging:rtl8192u: Remove HTHalfMcsToDataRate() - Style

The function HTHalfMcsToDataRate() is unused in code so has simply
been removed from the code.

As a result two static functions, (IsHTHalfNmode40Bandwidth and
IsHTHalfNmodeSGI), are not longer called, so they have been removed
as well.

These are coding style changes which should have no impact on
runtime code execution.

Signed-off-by: John Whitmore <[email protected]>
---
.../staging/rtl8192u/ieee80211/ieee80211.h | 1 -
.../rtl8192u/ieee80211/rtl819x_HTProc.c | 57 -------------------
2 files changed, 58 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index b4e7ae60ac1c..cc694e567040 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2382,7 +2382,6 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
//extern void HTSetConnectBwModeCallback(unsigned long data);
void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
-u16 HTHalfMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate);
u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
//function in BAPROC.c
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 9bf52cebe4cd..ea1261e33a68 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -216,63 +216,6 @@ void HTDebugHTInfo(u8 *InfoIE, u8 *TitleString)
pHTInfoEle->BasicMSC[1], pHTInfoEle->BasicMSC[2], pHTInfoEle->BasicMSC[3], pHTInfoEle->BasicMSC[4]);
}

-/*
- * Return: true if station in half n mode and AP supports 40 bw
- */
-static bool IsHTHalfNmode40Bandwidth(struct ieee80211_device *ieee)
-{
- bool retValue = false;
- PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
-
- if (!pHTInfo->bCurrentHTSupport) // wireless is n mode
- retValue = false;
- else if (!pHTInfo->bRegBW40MHz) // station supports 40 bw
- retValue = false;
- else if (!ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev)) // station in half n mode
- retValue = false;
- else if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ChlWidth) // ap support 40 bw
- retValue = true;
- else
- retValue = false;
-
- return retValue;
-}
-
-static bool IsHTHalfNmodeSGI(struct ieee80211_device *ieee, bool is40MHz)
-{
- bool retValue = false;
- PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
-
- if (!pHTInfo->bCurrentHTSupport) // wireless is n mode
- retValue = false;
- else if (!ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev)) // station in half n mode
- retValue = false;
- else if (is40MHz) { // ap support 40 bw
- if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ShortGI40Mhz) // ap support 40 bw short GI
- retValue = true;
- else
- retValue = false;
- } else {
- if (((struct ht_capability_ele *)(pHTInfo->PeerHTCapBuf))->ShortGI20Mhz) // ap support 40 bw short GI
- retValue = true;
- else
- retValue = false;
- }
-
- return retValue;
-}
-
-u16 HTHalfMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
-{
- u8 is40MHz;
- u8 isShortGI;
-
- is40MHz = (IsHTHalfNmode40Bandwidth(ieee)) ? 1 : 0;
- isShortGI = (IsHTHalfNmodeSGI(ieee, is40MHz)) ? 1 : 0;
-
- return MCS_DATA_RATE[is40MHz][isShortGI][(nMcsRate & 0x7f)];
-}
-
u16 HTMcsToDataRate(struct ieee80211_device *ieee, u8 nMcsRate)
{
PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
--
2.18.0


2018-09-26 19:20:48

by John Whitmore

[permalink] [raw]
Subject: [PATCH 04/13] staging:rtl8192u: Move HTSetConnectBwModeCallback() - Style

The function HTSetConnectBwModeCallback() is only used in the file
in which it is defined, so has been changed to being of type 'static'.

Additionally the function is defined after it is used so a prototype
was included in the file. This prototype, in the middle of the file
has been removed and the function implementation moved so that it is
defined before it is used in the file.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
.../staging/rtl8192u/ieee80211/ieee80211.h | 1 -
.../rtl8192u/ieee80211/rtl819x_HTProc.c | 47 +++++++++----------
2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 8c91cb29fc9d..8aa536d79900 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2379,7 +2379,6 @@ u8 HTGetHighestMCSRate(struct ieee80211_device *ieee,
extern u8 MCS_FILTER_ALL[];
extern u16 MCS_DATA_RATE[2][2][77];
u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame);
-//extern void HTSetConnectBwModeCallback(unsigned long data);
void HTResetIOTSetting(PRT_HIGH_THROUGHPUT pHTInfo);
bool IsHTHalfNmodeAPs(struct ieee80211_device *ieee);
u16 TxCountToDataRate(struct ieee80211_device *ieee, u8 nDataRate);
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index 977a1e5c9b52..c73a8058cf87 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -1011,7 +1011,6 @@ void HTOnAssocRsp(struct ieee80211_device *ieee)
pHTInfo->CurrentOpMode = pPeerHTInfo->OptMode;
}

-void HTSetConnectBwModeCallback(struct ieee80211_device *ieee);
/*
*function: initialize HT info(struct PRT_HIGH_THROUGHPUT)
* input: struct ieee80211_device* ieee
@@ -1228,6 +1227,29 @@ u8 HTCCheck(struct ieee80211_device *ieee, u8 *pFrame)
return false;
}

+static void HTSetConnectBwModeCallback(struct ieee80211_device *ieee)
+{
+ PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
+
+ IEEE80211_DEBUG(IEEE80211_DL_HT, "======>%s()\n", __func__);
+
+ if (pHTInfo->bCurBW40MHz) {
+ if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_UPPER)
+ ieee->set_chan(ieee->dev, ieee->current_network.channel + 2);
+ else if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_LOWER)
+ ieee->set_chan(ieee->dev, ieee->current_network.channel - 2);
+ else
+ ieee->set_chan(ieee->dev, ieee->current_network.channel);
+
+ ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20_40, pHTInfo->CurSTAExtChnlOffset);
+ } else {
+ ieee->set_chan(ieee->dev, ieee->current_network.channel);
+ ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
+ }
+
+ pHTInfo->bSwBwInProgress = false;
+}
+
/*
* This function set bandwidth mode in protocol layer.
*/
@@ -1278,26 +1300,3 @@ void HTSetConnectBwMode(struct ieee80211_device *ieee, enum ht_channel_width Ban

// spin_unlock_irqrestore(&(ieee->bw_spinlock), flags);
}
-
-void HTSetConnectBwModeCallback(struct ieee80211_device *ieee)
-{
- PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
-
- IEEE80211_DEBUG(IEEE80211_DL_HT, "======>%s()\n", __func__);
-
- if (pHTInfo->bCurBW40MHz) {
- if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_UPPER)
- ieee->set_chan(ieee->dev, ieee->current_network.channel + 2);
- else if (pHTInfo->CurSTAExtChnlOffset == HT_EXTCHNL_OFFSET_LOWER)
- ieee->set_chan(ieee->dev, ieee->current_network.channel - 2);
- else
- ieee->set_chan(ieee->dev, ieee->current_network.channel);
-
- ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20_40, pHTInfo->CurSTAExtChnlOffset);
- } else {
- ieee->set_chan(ieee->dev, ieee->current_network.channel);
- ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
- }
-
- pHTInfo->bSwBwInProgress = false;
-}
--
2.18.0


2018-09-26 19:21:00

by John Whitmore

[permalink] [raw]
Subject: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

The member variables AdvCoding and GreenField are unused in code so
have been removed from the structure and associated initialisation
function.

This is a coding style change which should have no impact on runtime
code execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
2 files changed, 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
index 64d5359cf7e2..83fb8f34ccbd 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
@@ -39,10 +39,8 @@ enum ht_extension_chan_offset {

struct ht_capability_ele {
//HT capability info
- u8 AdvCoding:1;
u8 ChlWidth:1;
u8 MimoPwrSave:2;
- u8 GreenField:1;
u8 ShortGI20Mhz:1;
u8 ShortGI40Mhz:1;
u8 TxSTBC:1;
diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index c73a8058cf87..d1fbe65ed428 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -493,7 +493,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u
}

//HT capability info
- pCapELE->AdvCoding = 0; // This feature is not supported now!!
if (ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev))
pCapELE->ChlWidth = 0;
else
@@ -501,7 +500,6 @@ void HTConstructCapabilityElement(struct ieee80211_device *ieee, u8 *posHTCap, u

// pCapELE->ChlWidth = (pHT->bRegBW40MHz?1:0);
pCapELE->MimoPwrSave = pHT->SelfMimoPs;
- pCapELE->GreenField = 0; // This feature is not supported now!!
pCapELE->ShortGI20Mhz = 1; // We can receive Short GI!!
pCapELE->ShortGI40Mhz = 1; // We can receive Short GI!!
//DbgPrint("TX HT cap/info ele BW=%d SG20=%d SG40=%d\n\r",
--
2.18.0


2018-09-26 19:21:10

by John Whitmore

[permalink] [raw]
Subject: [PATCH 03/13] staging:rtl8192u: Remove definition of HTSetConnectBwMode - Style

Remove the redundant declaration of the function HTSetConnectBwMode()
as it is not needed in the code. The function is already declared
in header file, multiple declarations add nothing.

This is a style change which should have no impact on runtime code
execution.

Signed-off-by: John Whitmore <[email protected]>
---
drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
index ebe8d527a080..977a1e5c9b52 100644
--- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c
@@ -837,7 +837,6 @@ static u8 HTFilterMCSRate(struct ieee80211_device *ieee, u8 *pSupportMCS,
return true;
}

-void HTSetConnectBwMode(struct ieee80211_device *ieee, enum ht_channel_width Bandwidth, enum ht_extension_chan_offset Offset);
void HTOnAssocRsp(struct ieee80211_device *ieee)
{
PRT_HIGH_THROUGHPUT pHTInfo = ieee->pHTInfo;
--
2.18.0


2018-09-28 14:33:49

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
>
> This is a coding style change which should have no impact on runtime
> code execution.
>
> Signed-off-by: John Whitmore <[email protected]>
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>
> struct ht_capability_ele {
> //HT capability info
> - u8 AdvCoding:1;
> u8 ChlWidth:1;
> u8 MimoPwrSave:2;
> - u8 GreenField:1;
> u8 ShortGI20Mhz:1;
> u8 ShortGI40Mhz:1;
> u8 TxSTBC:1;

I feel like we discussed this before. I'm pretty sure this comes from
the firmware and so the format can't be changed. When I look at
rtllib_parse_mife_generic() then I think that "info_element" probably
comes from the firmware.

I wouldn't want to accept this with out someone testing it.

regards,
dan carpenter


2018-09-28 14:34:07

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
>
> This is a coding style change which should not impact runtime code
> execution.
>
> Signed-off-by: John Whitmore <[email protected]>
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
> 2 files changed, 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 83fb8f34ccbd..52cce0dcf9a5 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -43,8 +43,6 @@ struct ht_capability_ele {
> u8 MimoPwrSave:2;
> u8 ShortGI20Mhz:1;
> u8 ShortGI40Mhz:1;
> - u8 TxSTBC:1;
> - u8 RxSTBC:2;
> u8 DelayBA:1;
> u8 MaxAMSDUSize:1;
> u8 DssCCk:1;

Same thing. I suspect his is set by the firmware.

regards,
dan carpenter


2018-09-28 14:34:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 07/13] staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style

Yeah... :( All the remaining patches are similar and risky.

regards,
dan carpenter


2018-09-28 16:23:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
>
> This is a coding style change which should have no impact on runtime
> code execution.
>
> Signed-off-by: John Whitmore <[email protected]>
> ---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>
> struct ht_capability_ele {
> //HT capability info
> - u8 AdvCoding:1;
> u8 ChlWidth:1;
> u8 MimoPwrSave:2;
> - u8 GreenField:1;

Don't these fields come from the hardware itself? By removing them
here, you just changed the memory layout of the structure. Does the
driver still work properly after this? If you can't test it, I can't
take this patch as it's too risky...

sorry,

greg k-h

2018-09-28 16:24:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
>
> This is a coding style change which should not impact runtime code
> execution.

Same as before, I think this _does_ impact runtime :(

I'll stop here in the series.

thanks,

greg k-h

2018-09-28 22:32:12

by John Whitmore

[permalink] [raw]
Subject: Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

On Fri, Sep 28, 2018 at 05:31:40PM +0300, Dan Carpenter wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> >
> > This is a coding style change which should have no impact on runtime
> > code execution.
> >
> > Signed-off-by: John Whitmore <[email protected]>
> > ---
> > drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> > drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> > 2 files changed, 4 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >
> > struct ht_capability_ele {
> > //HT capability info
> > - u8 AdvCoding:1;
> > u8 ChlWidth:1;
> > u8 MimoPwrSave:2;
> > - u8 GreenField:1;
> > u8 ShortGI20Mhz:1;
> > u8 ShortGI40Mhz:1;
> > u8 TxSTBC:1;
>
> I feel like we discussed this before. I'm pretty sure this comes from
> the firmware and so the format can't be changed. When I look at
> rtllib_parse_mife_generic() then I think that "info_element" probably
> comes from the firmware.
>
> I wouldn't want to accept this with out someone testing it.
>
> regards,
> dan carpenter
>

Thank you and sorry about not helping the signal to noise ratio on here.

I agree that a bit field like that and it looks like it comes from
firmware, but my question or possibly obsession was where. There are
structures inside structures, but they are all allocated from RAM.
Because of that I expected to find a memcopy from the device, or given
that the bitfield is initialised with values that it might be memcopy'd
to the device. I just couldn't find that memcopy, but that's down to
my untrained eye. I'll stumble across it in some obscure corner of the
driver.

2018-09-28 22:40:02

by John Whitmore

[permalink] [raw]
Subject: Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

On Fri, Sep 28, 2018 at 02:35:50PM +0200, Greg KH wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> >
> > This is a coding style change which should have no impact on runtime
> > code execution.
> >
> > Signed-off-by: John Whitmore <[email protected]>
> > ---
> > drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> > drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> > 2 files changed, 4 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >
> > struct ht_capability_ele {
> > //HT capability info
> > - u8 AdvCoding:1;
> > u8 ChlWidth:1;
> > u8 MimoPwrSave:2;
> > - u8 GreenField:1;
>
> Don't these fields come from the hardware itself? By removing them
> here, you just changed the memory layout of the structure. Does the
> driver still work properly after this? If you can't test it, I can't
> take this patch as it's too risky...
>

Sorry, yes the structure looks like it should come from the hardware
but as the structure is allocated from memory I expected to find a
memcopy either to or from the hardware. Yes risky, just because I
couldn't find it don't mean the connection to hardware ain't there.

I'll lay off the risky and who knows if I keep wondering through the
driver I'll find that illusive connection.