[PATCH 0/4] staging:rtl8192u: Style and a question?

[PATCH 0/4] staging:rtl8192u: Style and a question?

[John Whitmore]

This short series of coding style changes clears the remaining checkpatch
issues in the files r8180_93cx6.h and dot11d.h. The file dot11d.h does
still contain lines over the 80 character limit, but most of these lines
are MACRO definitions, and I'm not sure splitting them will add anything
to the style and readability of the code.

My question is with respect to the member variable 'enabled' which has
been refactored in the third patch of the series. I've renamed the
variable to 'dot11d_enabled', (in addition to changing it's type as
suggested by checkpatch), so that it is easier to search for in code.
Now that it is easier to search for 'dot11d_enabled' and it's associated
macro IS_DOT11D_ENABLE(ieee) it becomes apparent that the variable is
initialised to FALSE, (and elsewhere '0'), but appears never to be set.

The function dot11d_update_country_ie(), in file dot11d.c, states in its
header block comment that it assumes that the value of dot11d_enabled is
true.

The dot11d_info structure is allocated:

ieee->dot11d_info = kzalloc(sizeof(struct rt_dot11d_info), GFP_KERNEL);

And dot11d_enabled could be set with a memcopy or a memset, but if it
is being set I've missed it completely. There may be a kernel specific
mechanism which I'm not yet familiar with?

In my ignorance I'd be tempted to remove all code which relies on the
variable being True, but then that function dot11d_update_country_ie()
specifically assumes it's true and is an exported symbol.

John Whitmore (4):
  staging:rtl8192u: Remove unused definitions - Style
  staging:rtl8192u: Add required SPDX-License-Identifier - Style
  staging:rtl8192u: Refactor member variable enabled - Style
  staging:rtl8192u: Rename member pDot11dInfo - Style

 drivers/staging/rtl8192u/ieee80211/dot11d.c   | 66 +++++++++----------
 drivers/staging/rtl8192u/ieee80211/dot11d.h   |  6 +-
 .../staging/rtl8192u/ieee80211/ieee80211.h    |  2 +-
 .../rtl8192u/ieee80211/ieee80211_softmac.c    |  8 +--
 drivers/staging/rtl8192u/r8180_93cx6.h        | 19 +-----
 drivers/staging/rtl8192u/r8192U_core.c        |  2 +-
 6 files changed, 43 insertions(+), 60 deletions(-)

-- 
2.18.0

[PATCH 1/4] staging:rtl8192u: Remove unused definitions - Style

[John Whitmore]

Remove unused definitions from the file r8180_93cx6.h.

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

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/r8180_93cx6.h | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8180_93cx6.h b/drivers/staging/rtl8192u/r8180_93cx6.h
index 643d465e7105..bfbef1a16159 100644
--- a/drivers/staging/rtl8192u/r8180_93cx6.h
+++ b/drivers/staging/rtl8192u/r8180_93cx6.h
@@ -21,22 +21,4 @@
 
 #define EPROM_DELAY 10
 
-#define EPROM_ANAPARAM_ADDRLWORD 0xd
-#define EPROM_ANAPARAM_ADDRHWORD 0xe
-
-#define EPROM_RFCHIPID 0x6
-#define EPROM_TXPW_BASE 0x05
-#define EPROM_RFCHIPID_RTL8225U 5
-#define EPROM_RF_PARAM 0x4
-#define EPROM_CONFIG2 0xc
-
-#define EPROM_VERSION 0x1E
-#define MAC_ADR 0x7
-
-#define CIS 0x18
-
-#define EPROM_TXPW0 0x16
-#define EPROM_TXPW2 0x1b
-#define EPROM_TXPW1 0x3d
-
 int eprom_read(struct net_device *dev, u32 addr); /* reads a 16 bits word */
-- 
2.18.0

[PATCH 2/4] staging:rtl8192u: Add required SPDX-License-Identifier - Style

[John Whitmore]

Add the required SPDX-License-Identifier to the file r8180_93cx6.h,
this clears a checkpatch issue.

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

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/r8180_93cx6.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/rtl8192u/r8180_93cx6.h b/drivers/staging/rtl8192u/r8180_93cx6.h
index bfbef1a16159..0cdd00a4f7b8 100644
--- a/drivers/staging/rtl8192u/r8180_93cx6.h
+++ b/drivers/staging/rtl8192u/r8180_93cx6.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *	This is part of rtl8187 OpenSource driver
  *	Copyright (C) Andrea Merello 2004-2005  <andrea.merello@gmail.com>
-- 
2.18.0

[PATCH 3/4] staging:rtl8192u: Refactor member variable enabled - Style

[John Whitmore]

The structure rt_dot11d_info contains a memeber variable 'enabled',
which causes a checkpatch issue as it is declared as being of type
bool. The type of the variable has been changed to a 'u8', to clear
the issue.

Additionally to aid searching for the member variable in code it
has been renamed from 'enabled' to 'dot11d_enabled'.

These are purely coding style changes which should not impact runtime
code execution.

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +-
 drivers/staging/rtl8192u/ieee80211/dot11d.h | 4 ++--
 drivers/staging/rtl8192u/r8192U_core.c      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index d3b13f0cfc09..f3aa60cb3227 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -7,7 +7,7 @@ void dot11d_init(struct ieee80211_device *ieee)
 {
 	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(ieee);
 
-	pDot11dInfo->enabled = false;
+	pDot11dInfo->dot11d_enabled = false;
 
 	pDot11dInfo->state = DOT11D_STATE_NONE;
 	pDot11dInfo->country_ie_len = 0;
diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.h b/drivers/staging/rtl8192u/ieee80211/dot11d.h
index be59f720ae20..df37678b806d 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.h
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.h
@@ -28,12 +28,12 @@ struct rt_dot11d_info {
 	u8  max_tx_pwr_dbm_list[MAX_CHANNEL_NUMBER + 1];
 
 	enum dot11d_state state;
-	bool enabled; /* dot11MultiDomainCapabilityEnabled */
+	u8  dot11d_enabled; /* dot11MultiDomainCapabilityEnabled */
 };
 
 #define GET_DOT11D_INFO(ieee_dev) ((struct rt_dot11d_info *)((ieee_dev)->pDot11dInfo))
 
-#define IS_DOT11D_ENABLE(ieee_dev) (GET_DOT11D_INFO(ieee_dev)->enabled)
+#define IS_DOT11D_ENABLE(ieee_dev) (GET_DOT11D_INFO(ieee_dev)->dot11d_enabled)
 #define IS_COUNTRY_IE_VALID(ieee_dev) (GET_DOT11D_INFO(ieee_dev)->country_ie_len > 0)
 
 #define IS_EQUAL_CIE_SRC(ieee_dev, addr) ether_addr_equal(GET_DOT11D_INFO(ieee_dev)->country_ie_src_addr, addr)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 6b22aaa40ff9..34ea47366b72 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -211,7 +211,7 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv)
 		/* this flag enabled to follow 11d country IE setting,
 		 * otherwise, it shall follow global domain settings.
 		 */
-		GET_DOT11D_INFO(ieee)->enabled = 0;
+		GET_DOT11D_INFO(ieee)->dot11d_enabled = 0;
 		dot11d_reset(ieee);
 		ieee->bGlobalDomain = true;
 		break;
-- 
2.18.0

Re: [PATCH 3/4] staging:rtl8192u: Refactor member variable enabled - Style

[Dan Carpenter]

Like you mentioned in the 0/4 email, the right thing to do here is just
to delete all references to ->enabled.

regards,
dan carpenter

Re: [PATCH 3/4] staging:rtl8192u: Refactor member variable enabled - Style

[John Whitmore]

On Wed, Aug 29, 2018 at 02:20:33PM +0300, Dan Carpenter wrote:
> Like you mentioned in the 0/4 email, the right thing to do here is just
> to delete all references to ->enabled.
> 

Thank you for your response, I'll do that in a future series of patches.

jwhitmore

[PATCH 4/4] staging:rtl8192u: Rename member pDot11dInfo - Style

[John Whitmore]

Rename 'pDot11dInfo', this member variable of the structure
ieee80211_device causes a checkpatch issue, CamelCase naming. The
member has been renamed 'dot11d_info' to clear this issue.

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

Signed-off-by: John Whitmore <johnfwhitmore@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/dot11d.c   | 66 +++++++++----------
 drivers/staging/rtl8192u/ieee80211/dot11d.h   |  2 +-
 .../staging/rtl8192u/ieee80211/ieee80211.h    |  2 +-
 .../rtl8192u/ieee80211/ieee80211_softmac.c    |  8 +--
 4 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index f3aa60cb3227..e152528c9409 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -5,14 +5,14 @@
 
 void dot11d_init(struct ieee80211_device *ieee)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(ieee);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(ieee);
 
-	pDot11dInfo->dot11d_enabled = false;
+	dot11d_info->dot11d_enabled = false;
 
-	pDot11dInfo->state = DOT11D_STATE_NONE;
-	pDot11dInfo->country_ie_len = 0;
-	memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1);
-	memset(pDot11dInfo->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
+	dot11d_info->state = DOT11D_STATE_NONE;
+	dot11d_info->country_ie_len = 0;
+	memset(dot11d_info->channel_map, 0, MAX_CHANNEL_NUMBER + 1);
+	memset(dot11d_info->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
 	RESET_CIE_WATCHDOG(ieee);
 
 	netdev_info(ieee->dev, "dot11d_init()\n");
@@ -23,19 +23,19 @@ EXPORT_SYMBOL(dot11d_init);
 void dot11d_reset(struct ieee80211_device *ieee)
 {
 	u32 i;
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(ieee);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(ieee);
 	/* Clear old channel map */
-	memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1);
-	memset(pDot11dInfo->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
+	memset(dot11d_info->channel_map, 0, MAX_CHANNEL_NUMBER+1);
+	memset(dot11d_info->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
 	/* Set new channel map */
 	for (i = 1; i <= 11; i++)
-		(pDot11dInfo->channel_map)[i] = 1;
+		(dot11d_info->channel_map)[i] = 1;
 
 	for (i = 12; i <= 14; i++)
-		(pDot11dInfo->channel_map)[i] = 2;
+		(dot11d_info->channel_map)[i] = 2;
 
-	pDot11dInfo->state = DOT11D_STATE_NONE;
-	pDot11dInfo->country_ie_len = 0;
+	dot11d_info->state = DOT11D_STATE_NONE;
+	dot11d_info->country_ie_len = 0;
 	RESET_CIE_WATCHDOG(ieee);
 }
 EXPORT_SYMBOL(dot11d_reset);
@@ -52,12 +52,12 @@ EXPORT_SYMBOL(dot11d_reset);
 void dot11d_update_country_ie(struct ieee80211_device *dev, u8 *pTaddr,
 			    u16 CoutryIeLen, u8 *pCoutryIe)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(dev);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
 	u8 i, j, NumTriples, MaxChnlNum;
 	struct chnl_txpower_triple *pTriple;
 
-	memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1);
-	memset(pDot11dInfo->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
+	memset(dot11d_info->channel_map, 0, MAX_CHANNEL_NUMBER+1);
+	memset(dot11d_info->max_tx_pwr_dbm_list, 0xFF, MAX_CHANNEL_NUMBER+1);
 	MaxChnlNum = 0;
 	NumTriples = (CoutryIeLen - 3) / 3; /* skip 3-byte country string. */
 	pTriple = (struct chnl_txpower_triple *)(pCoutryIe + 3);
@@ -78,8 +78,8 @@ void dot11d_update_country_ie(struct ieee80211_device *dev, u8 *pTaddr,
 		}
 
 		for (j = 0; j < pTriple->num_channels; j++) {
-			pDot11dInfo->channel_map[pTriple->first_channel + j] = 1;
-			pDot11dInfo->max_tx_pwr_dbm_list[pTriple->first_channel + j] = pTriple->max_tx_pwr_dbm;
+			dot11d_info->channel_map[pTriple->first_channel + j] = 1;
+			dot11d_info->max_tx_pwr_dbm_list[pTriple->first_channel + j] = pTriple->max_tx_pwr_dbm;
 			MaxChnlNum = pTriple->first_channel + j;
 		}
 
@@ -87,29 +87,29 @@ void dot11d_update_country_ie(struct ieee80211_device *dev, u8 *pTaddr,
 	}
 	netdev_info(dev->dev, "Channel List:");
 	for (i = 1; i <= MAX_CHANNEL_NUMBER; i++)
-		if (pDot11dInfo->channel_map[i] > 0)
+		if (dot11d_info->channel_map[i] > 0)
 			netdev_info(dev->dev, " %d", i);
 	netdev_info(dev->dev, "\n");
 
 	UPDATE_CIE_SRC(dev, pTaddr);
 
-	pDot11dInfo->country_ie_len = CoutryIeLen;
-	memcpy(pDot11dInfo->country_ie_buf, pCoutryIe, CoutryIeLen);
-	pDot11dInfo->state = DOT11D_STATE_LEARNED;
+	dot11d_info->country_ie_len = CoutryIeLen;
+	memcpy(dot11d_info->country_ie_buf, pCoutryIe, CoutryIeLen);
+	dot11d_info->state = DOT11D_STATE_LEARNED;
 }
 EXPORT_SYMBOL(dot11d_update_country_ie);
 
 u8 dot11d_get_max_tx_pwr_in_dbm(struct ieee80211_device *dev, u8 Channel)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(dev);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
 	u8 MaxTxPwrInDbm = 255;
 
 	if (Channel > MAX_CHANNEL_NUMBER) {
 		netdev_err(dev->dev, "dot11d_get_max_tx_pwr_in_dbm(): Invalid Channel\n");
 		return MaxTxPwrInDbm;
 	}
-	if (pDot11dInfo->channel_map[Channel])
-		MaxTxPwrInDbm = pDot11dInfo->max_tx_pwr_dbm_list[Channel];
+	if (dot11d_info->channel_map[Channel])
+		MaxTxPwrInDbm = dot11d_info->max_tx_pwr_dbm_list[Channel];
 
 	return MaxTxPwrInDbm;
 }
@@ -117,11 +117,11 @@ EXPORT_SYMBOL(dot11d_get_max_tx_pwr_in_dbm);
 
 void dot11d_scan_complete(struct ieee80211_device *dev)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(dev);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
 
-	switch (pDot11dInfo->state) {
+	switch (dot11d_info->state) {
 	case DOT11D_STATE_LEARNED:
-		pDot11dInfo->state = DOT11D_STATE_DONE;
+		dot11d_info->state = DOT11D_STATE_DONE;
 		break;
 
 	case DOT11D_STATE_DONE:
@@ -138,13 +138,13 @@ EXPORT_SYMBOL(dot11d_scan_complete);
 
 int is_legal_channel(struct ieee80211_device *dev, u8 channel)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(dev);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
 
 	if (channel > MAX_CHANNEL_NUMBER) {
 		netdev_err(dev->dev, "is_legal_channel(): Invalid Channel\n");
 		return 0;
 	}
-	if (pDot11dInfo->channel_map[channel] > 0)
+	if (dot11d_info->channel_map[channel] > 0)
 		return 1;
 	return 0;
 }
@@ -152,12 +152,12 @@ EXPORT_SYMBOL(is_legal_channel);
 
 int to_legal_channel(struct ieee80211_device *dev, u8 channel)
 {
-	struct rt_dot11d_info *pDot11dInfo = GET_DOT11D_INFO(dev);
+	struct rt_dot11d_info *dot11d_info = GET_DOT11D_INFO(dev);
 	u8 default_chn = 0;
 	u32 i = 0;
 
 	for (i = 1; i <= MAX_CHANNEL_NUMBER; i++) {
-		if (pDot11dInfo->channel_map[i] > 0) {
+		if (dot11d_info->channel_map[i] > 0) {
 			default_chn = i;
 			break;
 		}
@@ -168,7 +168,7 @@ int to_legal_channel(struct ieee80211_device *dev, u8 channel)
 		return default_chn;
 	}
 
-	if (pDot11dInfo->channel_map[channel] > 0)
+	if (dot11d_info->channel_map[channel] > 0)
 		return channel;
 
 	return default_chn;
diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.h b/drivers/staging/rtl8192u/ieee80211/dot11d.h
index df37678b806d..414b74abfb74 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.h
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.h
@@ -31,7 +31,7 @@ struct rt_dot11d_info {
 	u8  dot11d_enabled; /* dot11MultiDomainCapabilityEnabled */
 };
 
-#define GET_DOT11D_INFO(ieee_dev) ((struct rt_dot11d_info *)((ieee_dev)->pDot11dInfo))
+#define GET_DOT11D_INFO(ieee_dev) ((struct rt_dot11d_info *)((ieee_dev)->dot11d_info))
 
 #define IS_DOT11D_ENABLE(ieee_dev) (GET_DOT11D_INFO(ieee_dev)->dot11d_enabled)
 #define IS_COUNTRY_IE_VALID(ieee_dev) (GET_DOT11D_INFO(ieee_dev)->country_ie_len > 0)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index e2e503d24c4c..b4e7ae60ac1c 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -1772,7 +1772,7 @@ struct ieee80211_device {
 
 	/* map of allowed channels. 0 is dummy */
 	// FIXME: remember to default to a basic channel plan depending of the PHY type
-	void *pDot11dInfo;
+	void *dot11d_info;
 	bool bGlobalDomain;
 	int rate;       /* current rate */
 	int basic_rate;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index a20c79401dab..8635faf84316 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -2542,8 +2542,8 @@ void ieee80211_softmac_init(struct ieee80211_device *ieee)
 	for (i = 0; i < 5; i++)
 		ieee->seq_ctrl[i] = 0;
 
-	ieee->pDot11dInfo = kzalloc(sizeof(struct rt_dot11d_info), GFP_KERNEL);
-	if (!ieee->pDot11dInfo)
+	ieee->dot11d_info = kzalloc(sizeof(struct rt_dot11d_info), GFP_KERNEL);
+	if (!ieee->dot11d_info)
 		IEEE80211_DEBUG(IEEE80211_DL_ERR, "can't alloc memory for DOT11D\n");
 	//added for  AP roaming
 	ieee->LinkDetectInfo.SlotNum = 2;
@@ -2603,8 +2603,8 @@ void ieee80211_softmac_init(struct ieee80211_device *ieee)
 void ieee80211_softmac_free(struct ieee80211_device *ieee)
 {
 	mutex_lock(&ieee->wx_mutex);
-	kfree(ieee->pDot11dInfo);
-	ieee->pDot11dInfo = NULL;
+	kfree(ieee->dot11d_info);
+	ieee->dot11d_info = NULL;
 	del_timer_sync(&ieee->associate_timer);
 
 	cancel_delayed_work(&ieee->associate_retry_wq);
-- 
2.18.0