Skip to contentSkip to conversation

Trivial code cleanup patches

PATCH Superseded Yogesh Hegde 5 participants 19 messages Jun 7, 2023 – Jun 15, 2023 Staging

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Yogesh Hegde

author
#0 lore raw

v2 cover Trivial code cleanup patches

These patches remove variables in the struct rtllib_device that were set only once throughout the driver. Instead of using these variables, the patches modify the relevant code to directly call the corresponding functions, eliminating unnecessary indirection and removing CamelCase. The patches are required to be applied in sequence.

Yogesh Hegde (5):
staging: rtl8192e: Remove variable SetWirelessMode
staging: rtl8192e: Remove variable SetBWModeHandler
staging: rtl8192e: Remove variable LeisurePSLeave
staging: rtl8192e: Remove variable InitialGainHandler
staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c |  4 ++--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c   |  4 ----
 drivers/staging/rtl8192e/rtl819x_HTProc.c      |  8 ++++----
 drivers/staging/rtl8192e/rtllib.h              |  6 ------
 drivers/staging/rtl8192e/rtllib_debug.h        |  5 -----
 drivers/staging/rtl8192e/rtllib_rx.c           |  3 ++-
 drivers/staging/rtl8192e/rtllib_softmac.c      | 16 +++++++---------
 drivers/staging/rtl8192e/rtllib_softmac_wx.c   |  9 +++++----
 8 files changed, 20 insertions(+), 35 deletions(-)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v2 1/5 staging: rtl8192e: Remove variable SetWirelessMode

The variable SetWirelessMode is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>.

---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c |  1 -
 drivers/staging/rtl8192e/rtllib.h            |  1 -
 drivers/staging/rtl8192e/rtllib_softmac.c    | 16 +++++++---------
 3 files changed, 7 insertions(+), 11 deletions(-)
drivers/staging/rtl8192e/rtl8192e/rtl_core.c +0 -1
index 17b70dde7eeb..bbe0864e1348 100644
@@ -716,7 +716,6 @@ static void _rtl92e_init_priv_handler(struct net_device *dev)
 	priv->rtllib->check_nic_enough_desc	= _rtl92e_check_nic_enough_desc;
 	priv->rtllib->handle_assoc_response	= _rtl92e_handle_assoc_response;
 	priv->rtllib->handle_beacon		= _rtl92e_handle_beacon;
-	priv->rtllib->SetWirelessMode		= rtl92e_set_wireless_mode;
 	priv->rtllib->LeisurePSLeave		= rtl92e_leisure_ps_leave;
 	priv->rtllib->SetBWModeHandler		= rtl92e_set_bw_mode;
 	priv->rf_set_chan			= rtl92e_set_channel;
drivers/staging/rtl8192e/rtllib.h +0 -1
index 4aa5ce9f7792..68a3b8af3119 100644
@@ -1710,7 +1710,6 @@ struct rtllib_device {
 				 enum ht_channel_width bandwidth,
 				 enum ht_extchnl_offset Offset);
 	bool (*GetNmodeSupportBySecCfg)(struct net_device *dev);
-	void (*SetWirelessMode)(struct net_device *dev, u8 wireless_mode);
 	bool (*GetHalfNmodeSupportByAPsHandler)(struct net_device *dev);
 	u8   (*rtllib_ap_sec_type)(struct rtllib_device *ieee);
 	void (*InitialGainHandler)(struct net_device *dev, u8 Operation);
drivers/staging/rtl8192e/rtllib_softmac.c +7 -9
index 7c4cba6dcf46..ed171eca4916 100644
@@ -19,6 +19,7 @@
 #include <linux/etherdevice.h>
 #include <linux/ieee80211.h>
 #include "dot11d.h"
+#include "rtl8192e/rtl_core.h"
 
 static void rtllib_sta_wakeup(struct rtllib_device *ieee, short nl);
 

@@ -1503,7 +1504,7 @@ static void rtllib_associate_complete_wq(void *data)
 		netdev_info(ieee->dev, "Using G rates:%d\n", ieee->rate);
 	} else {
 		ieee->rate = 22;
-		ieee->SetWirelessMode(ieee->dev, IEEE_B);
+		rtl92e_set_wireless_mode(ieee->dev, IEEE_B);
 		netdev_info(ieee->dev, "Using B rates:%d\n", ieee->rate);
 	}
 	if (ieee->ht_info->bCurrentHTSupport && ieee->ht_info->enable_ht) {

@@ -1687,14 +1688,12 @@ inline void rtllib_softmac_new_net(struct rtllib_device *ieee,
 				    (ieee->modulation &
 				     RTLLIB_OFDM_MODULATION)) {
 					ieee->rate = 108;
-					ieee->SetWirelessMode(ieee->dev,
-							      IEEE_G);
+					rtl92e_set_wireless_mode(ieee->dev, IEEE_G);
 					netdev_info(ieee->dev,
 						    "Using G rates\n");
 				} else {
 					ieee->rate = 22;
-					ieee->SetWirelessMode(ieee->dev,
-							      IEEE_B);
+					rtl92e_set_wireless_mode(ieee->dev, IEEE_B);
 					netdev_info(ieee->dev,
 						    "Using B rates\n");
 				}

@@ -2276,11 +2275,10 @@ static void rtllib_rx_auth_resp(struct rtllib_device *ieee, struct sk_buff *skb)
 		}
 		/* Dummy wirless mode setting to avoid encryption issue */
 		if (bSupportNmode) {
-			ieee->SetWirelessMode(ieee->dev,
-					      ieee->current_network.mode);
+			rtl92e_set_wireless_mode(ieee->dev, ieee->current_network.mode);
 		} else {
 			/*TODO*/
-			ieee->SetWirelessMode(ieee->dev, IEEE_G);
+			rtl92e_set_wireless_mode(ieee->dev, IEEE_G);
 		}
 
 		if ((ieee->current_network.mode == IEEE_N_24G) &&

@@ -2615,7 +2613,7 @@ static void rtllib_start_ibss_wq(void *data)
 		}
 
 		ieee->current_network.qos_data.supported = 0;
-		ieee->SetWirelessMode(ieee->dev, IEEE_G);
+		rtl92e_set_wireless_mode(ieee->dev, IEEE_G);
 		ieee->current_network.mode = ieee->mode;
 		ieee->current_network.atim_window = 0;
 		ieee->current_network.capability = WLAN_CAPABILITY_IBSS;
Signature · 1 line
2.25.1
author
#9 lore raw

PATCH v2 2/5 staging: rtl8192e: Remove variable SetBWModeHandler

In reply to Yogesh Hegde

The variable SetBWModeHandler is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>

---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 1 -
 drivers/staging/rtl8192e/rtl819x_HTProc.c    | 8 ++++----
 drivers/staging/rtl8192e/rtllib.h            | 3 ---
 drivers/staging/rtl8192e/rtllib_softmac_wx.c | 6 +++---
 4 files changed, 7 insertions(+), 11 deletions(-)
drivers/staging/rtl8192e/rtl8192e/rtl_core.c +0 -1
index bbe0864e1348..6f6c31344441 100644
@@ -717,7 +717,6 @@ static void _rtl92e_init_priv_handler(struct net_device *dev)
 	priv->rtllib->handle_assoc_response	= _rtl92e_handle_assoc_response;
 	priv->rtllib->handle_beacon		= _rtl92e_handle_beacon;
 	priv->rtllib->LeisurePSLeave		= rtl92e_leisure_ps_leave;
-	priv->rtllib->SetBWModeHandler		= rtl92e_set_bw_mode;
 	priv->rf_set_chan			= rtl92e_set_channel;
 
 	priv->rtllib->start_send_beacons = rtl92e_start_beacon;
drivers/staging/rtl8192e/rtl819x_HTProc.c +4 -4
index fe30a291e64c..ad92a8230055 100644
@@ -6,6 +6,8 @@
  */
 #include "rtllib.h"
 #include "rtl819x_HT.h"
+#include "rtl8192e/r8192E_phy.h"
+
 u8 MCS_FILTER_ALL[16] = {
 	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 	0xff, 0x1f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00

@@ -835,12 +837,10 @@ static void HTSetConnectBwModeCallback(struct rtllib_device *ieee)
 			ieee->set_chan(ieee->dev,
 				       ieee->current_network.channel);
 
-		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20_40,
-				       ht_info->CurSTAExtChnlOffset);
+		rtl92e_set_bw_mode(ieee->dev, HT_CHANNEL_WIDTH_20_40, ht_info->CurSTAExtChnlOffset);
 	} else {
 		ieee->set_chan(ieee->dev, ieee->current_network.channel);
-		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20,
-				       HT_EXTCHNL_OFFSET_NO_EXT);
+		rtl92e_set_bw_mode(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
 	}
 
 	ht_info->sw_bw_in_progress = false;
drivers/staging/rtl8192e/rtllib.h +0 -3
index 68a3b8af3119..d22a586043f2 100644
@@ -1706,9 +1706,6 @@ struct rtllib_device {
 
 	/* check whether Tx hw resource available */
 	short (*check_nic_enough_desc)(struct net_device *dev, int queue_index);
-	void (*SetBWModeHandler)(struct net_device *dev,
-				 enum ht_channel_width bandwidth,
-				 enum ht_extchnl_offset Offset);
 	bool (*GetNmodeSupportBySecCfg)(struct net_device *dev);
 	bool (*GetHalfNmodeSupportByAPsHandler)(struct net_device *dev);
 	u8   (*rtllib_ap_sec_type)(struct rtllib_device *ieee);
drivers/staging/rtl8192e/rtllib_softmac_wx.c +3 -3
index d85a32d2d050..9fb4fee93990 100644
@@ -15,6 +15,7 @@
 
 #include "rtllib.h"
 #include "dot11d.h"
+#include "rtl8192e/r8192E_phy.h"
 /* FIXME: add A freqs */
 
 const long rtllib_wlan_frequencies[] = {

@@ -359,8 +360,7 @@ void rtllib_wx_sync_scan_wq(void *data)
 		b40M = 1;
 		chan_offset = ieee->ht_info->CurSTAExtChnlOffset;
 		bandwidth = (enum ht_channel_width)ieee->ht_info->bCurBW40MHz;
-		ieee->SetBWModeHandler(ieee->dev, HT_CHANNEL_WIDTH_20,
-				       HT_EXTCHNL_OFFSET_NO_EXT);
+		rtl92e_set_bw_mode(ieee->dev, HT_CHANNEL_WIDTH_20, HT_EXTCHNL_OFFSET_NO_EXT);
 	}
 
 	rtllib_start_scan_syncro(ieee, 0);

@@ -372,7 +372,7 @@ void rtllib_wx_sync_scan_wq(void *data)
 			ieee->set_chan(ieee->dev, chan - 2);
 		else
 			ieee->set_chan(ieee->dev, chan);
-		ieee->SetBWModeHandler(ieee->dev, bandwidth, chan_offset);
+		rtl92e_set_bw_mode(ieee->dev, bandwidth, chan_offset);
 	} else {
 		ieee->set_chan(ieee->dev, chan);
 	}
Signature · 1 line
2.25.1
author
#11 lore raw

PATCH v2 3/5 staging: rtl8192e: Remove variable LeisurePSLeave

In reply to Yogesh Hegde

The variable LeisurePSLeave is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>

---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 1 -
 drivers/staging/rtl8192e/rtllib.h            | 1 -
 drivers/staging/rtl8192e/rtllib_rx.c         | 3 ++-
 drivers/staging/rtl8192e/rtllib_softmac_wx.c | 3 ++-
 4 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/rtl8192e/rtl8192e/rtl_core.c +0 -1
index 6f6c31344441..231903002233 100644
@@ -716,7 +716,6 @@ static void _rtl92e_init_priv_handler(struct net_device *dev)
 	priv->rtllib->check_nic_enough_desc	= _rtl92e_check_nic_enough_desc;
 	priv->rtllib->handle_assoc_response	= _rtl92e_handle_assoc_response;
 	priv->rtllib->handle_beacon		= _rtl92e_handle_beacon;
-	priv->rtllib->LeisurePSLeave		= rtl92e_leisure_ps_leave;
 	priv->rf_set_chan			= rtl92e_set_channel;
 
 	priv->rtllib->start_send_beacons = rtl92e_start_beacon;
drivers/staging/rtl8192e/rtllib.h +0 -1
index d22a586043f2..f7fd8b77db99 100644
@@ -1719,7 +1719,6 @@ struct rtllib_device {
 
 	void (*rtllib_ips_leave_wq)(struct net_device *dev);
 	void (*rtllib_ips_leave)(struct net_device *dev);
-	void (*LeisurePSLeave)(struct net_device *dev);
 
 	/* This must be the last item so that it points to the data
 	 * allocated beyond this structure by alloc_rtllib
drivers/staging/rtl8192e/rtllib_rx.c +2 -1
index d44bf261de54..d8acede4b808 100644
@@ -35,6 +35,7 @@
 
 #include "rtllib.h"
 #include "dot11d.h"
+#include "rtl8192e/rtl_ps.h"
 
 static void rtllib_rx_mgt(struct rtllib_device *ieee, struct sk_buff *skb,
 			  struct rtllib_rx_stats *stats);

@@ -1212,7 +1213,7 @@ static void rtllib_rx_check_leave_lps(struct rtllib_device *ieee, u8 unicast,
 			if (((ieee->link_detect_info.NumRxUnicastOkInPeriod +
 			    ieee->link_detect_info.NumTxOkInPeriod) > 8) ||
 			    (ieee->link_detect_info.NumRxUnicastOkInPeriod > 2)) {
-				ieee->LeisurePSLeave(ieee->dev);
+				rtl92e_leisure_ps_leave(ieee->dev);
 			}
 		}
 	}
drivers/staging/rtl8192e/rtllib_softmac_wx.c +2 -1
index 9fb4fee93990..07c46ad4f20e 100644
@@ -16,6 +16,7 @@
 #include "rtllib.h"
 #include "dot11d.h"
 #include "rtl8192e/r8192E_phy.h"
+#include "rtl8192e/rtl_ps.h"
 /* FIXME: add A freqs */
 
 const long rtllib_wlan_frequencies[] = {

@@ -341,7 +342,7 @@ void rtllib_wx_sync_scan_wq(void *data)
 
 	chan = ieee->current_network.channel;
 
-	ieee->LeisurePSLeave(ieee->dev);
+	rtl92e_leisure_ps_leave(ieee->dev);
 	/* notify AP to be in PS mode */
 	rtllib_sta_ps_send_null_frame(ieee, 1);
 	rtllib_sta_ps_send_null_frame(ieee, 1);
Signature · 1 line
2.25.1
author
#12 lore raw

PATCH v2 4/5 staging: rtl8192e: Remove variable InitialGainHandler

In reply to Yogesh Hegde

The variable InitialGainHandler is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>

---
 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c | 4 ++--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c   | 1 -
 drivers/staging/rtl8192e/rtllib.h              | 1 -
 3 files changed, 2 insertions(+), 4 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c +2 -2
index 641e993aaa86..5385bb099c56 100644
@@ -1129,11 +1129,11 @@ void rtl92e_scan_op_backup(struct net_device *dev, u8 Operation)
 	if (priv->up) {
 		switch (Operation) {
 		case SCAN_OPT_BACKUP:
-			priv->rtllib->InitialGainHandler(dev, IG_Backup);
+			rtl92e_init_gain(dev, IG_Backup);
 			break;
 
 		case SCAN_OPT_RESTORE:
-			priv->rtllib->InitialGainHandler(dev, IG_Restore);
+			rtl92e_init_gain(dev, IG_Restore);
 			break;
 		}
 	}
drivers/staging/rtl8192e/rtl8192e/rtl_core.c +0 -1
index 231903002233..8218319ad834 100644
@@ -731,7 +731,6 @@ static void _rtl92e_init_priv_handler(struct net_device *dev)
 
 	priv->rtllib->SetHwRegHandler = rtl92e_set_reg;
 	priv->rtllib->AllowAllDestAddrHandler = rtl92e_set_monitor_mode;
-	priv->rtllib->InitialGainHandler = rtl92e_init_gain;
 	priv->rtllib->rtllib_ips_leave_wq = rtl92e_rtllib_ips_leave_wq;
 	priv->rtllib->rtllib_ips_leave = rtl92e_rtllib_ips_leave;
 	priv->rtllib->ScanOperationBackupHandler = rtl92e_scan_op_backup;
drivers/staging/rtl8192e/rtllib.h +0 -1
index f7fd8b77db99..92ed1b493f6b 100644
@@ -1709,7 +1709,6 @@ struct rtllib_device {
 	bool (*GetNmodeSupportBySecCfg)(struct net_device *dev);
 	bool (*GetHalfNmodeSupportByAPsHandler)(struct net_device *dev);
 	u8   (*rtllib_ap_sec_type)(struct rtllib_device *ieee);
-	void (*InitialGainHandler)(struct net_device *dev, u8 Operation);
 	void (*ScanOperationBackupHandler)(struct net_device *dev,
 					   u8 Operation);
 	void (*SetHwRegHandler)(struct net_device *dev, u8 variable, u8 *val);
Signature · 1 line
2.25.1
author
#13 lore raw

PATCH v2 5/5 staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

In reply to Yogesh Hegde

The macro DRV_NAME is defined twice within the driver, once in "rtl_core.h" and again in "rtllib_debug.h". The definition in "rtllib_debug.h" overrides the definition in "rtl_core.h", resulting in warnings during compilation. This patch removes the redundant definition in "rtllib_debug.h" to avoid the warnings.

Signed-off-by Yogesh Hegde <[email protected]>

---

v2: No changes.

---
 drivers/staging/rtl8192e/rtllib_debug.h | 5 -----
 1 file changed, 5 deletions(-)
drivers/staging/rtl8192e/rtllib_debug.h +0 -5
index f6b23defe225..ab8bd5fc4ca0 100644
@@ -9,11 +9,6 @@
 
 #include <linux/bits.h>
 
-/* Allow files to override DRV_NAME */
-#ifndef DRV_NAME
-#define DRV_NAME "rtllib_92e"
-#endif
-
 extern u32 rt_global_debug_component;
 
 /* These are the defines for rt_global_debug_component */
Signature · 1 line
2.25.1
Reviewed-by on v2 1/5
#2 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

In reply to Yogesh Hegde
Quoted 6 lines · Yogesh Hegde

The variable SetWirelessMode is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Quoted 6 lines
Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>.

Reviewed-by Dan Carpenter <[email protected]>

regards,
dan carpenter

Reviewed-by on the v2 series
#14 lore raw
In reply to Yogesh Hegde
Quoted 6 lines · Yogesh Hegde

These patches remove variables in the struct rtllib_device that were set only once throughout the driver. Instead of using these variables, the patches modify the relevant code to directly call the corresponding functions, eliminating unnecessary indirection and removing CamelCase. The patches are required to be applied in sequence.

Quoted 7 lines

Yogesh Hegde (5):
staging: rtl8192e: Remove variable SetWirelessMode
staging: rtl8192e: Remove variable SetBWModeHandler
staging: rtl8192e: Remove variable LeisurePSLeave
staging: rtl8192e: Remove variable InitialGainHandler
staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

These all look good.

Reviewed-by Dan Carpenter <[email protected]>

regards,
dan carpenter

#3 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

In reply to Dan Carpenter
Quoted 10 lines · Dan Carpenter
Quoted 9 lines · Yogesh Hegde

The variable SetWirelessMode is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Signed-off-by Yogesh Hegde <[email protected]>

---

Quoted 10 lines
Quoted 3 lines

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>.

Reviewed-by Dan Carpenter <[email protected]>

regards,
dan carpenter

Hi Dan,

thanks for all the work you do. I always appreciate your comments.

But I think it does not work because this driver is divided into two modules.

To load the driver I am using the following lines:
sudo insmod drivers/staging/rtl8192e/rtllib.ko sudo insmod drivers/staging/rtl8192e/rtl8192e/r8192e_pci.ko

So this line is required:
priv->rtllib->SetWirelessMode = rtl92e_set_wireless_mode; as one name is used in one module and one in the other module.

But the change was proposed by the masters so I thought that I must be wrong.

My compiler does not compile this patch:
LD [M] drivers/staging/rtl8192e/rtl8192e/r8192e_pci.o
MODPOST drivers/staging/rtl8192e/Module.symvers
ERROR: modpost: "rtl92e_set_wireless_mode" [drivers/staging/rtl8192e/rtllib.ko] undefined! make[1]: *** [scripts/Makefile.modpost:136: drivers/staging/rtl8192e/Module.symvers] Error 1
make: *** [Makefile:1978: modpost] Error 2

Did this compile on your system Yogesh?

So I am looking forward to your response what I am doing wrong.

Bye Philipp

#4 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

Quoted 17 lines · Philipp Hortmann
Quoted 16 lines · Dan Carpenter
Quoted 6 lines · Yogesh Hegde

The variable SetWirelessMode is set in only one place throughout the driver. This patch removes the variable and calls the real function directly instead, eliminating the unnecessary indirection. Additionally, the removal of the variable aligns with the checkpatch guidelines by removing the use of CamelCase.

Quoted 6 lines
Signed-off-by Yogesh Hegde <[email protected]>

---

v2: Removed the variable and called the function direction instead of
just renaming the variable as suggested by Greg Kroah-Hartman <[email protected]>.

Reviewed-by Dan Carpenter <[email protected]>
Quoted 9 lines
Quoted 4 lines

regards,
dan carpenter

Hi Dan,

thanks for all the work you do. I always appreciate your comments.

No need to butter me up. :P

Quoted 3 lines

But I think it does not work because this driver is divided into two modules.

Yeah, you're right. I'm wrong. We can't break the compile.

regards,
dan carpenter

author
#5 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

In reply to Philipp Hortmann
Quoted 10 lines · Philipp Hortmann

My compiler does not compile this patch:
LD [M] drivers/staging/rtl8192e/rtl8192e/r8192e_pci.o MODPOST drivers/staging/rtl8192e/Module.symvers
ERROR: modpost: "rtl92e_set_wireless_mode" [drivers/staging/rtl8192e/rtllib.ko] undefined! make[1]: *** [scripts/Makefile.modpost:136:
drivers/staging/rtl8192e/Module.symvers] Error 1
make: *** [Makefile:1978: modpost] Error 2

Did this compile on your system Yogesh?

No, while sending the patch I partially compiled it
make drivers/staging/rtl8192e but when I fully compile it make all it gives me the same error. It is a mistake on my part.

Quoted 6 lines

But I think it does not work because this driver is divided into two modules.

To load the driver I am using the following lines:
sudo insmod drivers/staging/rtl8192e/rtllib.ko sudo insmod drivers/staging/rtl8192e/rtl8192e/r8192e_pci.ko

I was not aware of this and assumed that ideally the driver has only one (PCI) interface so all the files should be compiled into one .ko file and loaded.

Quoted 3 lines

So this line is required:
priv->rtllib->SetWirelessMode = rtl92e_set_wireless_mode; as one name is used in one module and one in the other module.

Yes, this makes sense now.

Moving forward,
Dan, Greg and Philipp what would be the best fix for this issue,
1. Patchset to combine both the modules into one module
2. Revert back this patchset to v1.

Looking forward for your response.

Regards
Yogesh

#6 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

Quoted 4 lines · Yogesh Hegde

On Wed, Jun 07, 2023 at 10:18:28PM +0200, Philipp Hortmann wrote:
Dan, Greg and Philipp what would be the best fix for this issue,
1. Patchset to combine both the modules into one module
2. Revert back this patchset to v1.

Neither of those options are good. I haven't investigated this problem to figure out what the correct option is but we're unlikely to merge either of those unless you make some very persuasive arguments.

regards,
dan carpenter

#7 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

In reply to Yogesh Hegde
Quoted 10 lines · Yogesh Hegde
Quoted 9 lines · Philipp Hortmann

My compiler does not compile this patch:

   LD [M]  drivers/staging/rtl8192e/rtl8192e/r8192e_pci.o
   MODPOST drivers/staging/rtl8192e/Module.symvers

ERROR: modpost: "rtl92e_set_wireless_mode"
[drivers/staging/rtl8192e/rtllib.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:136:
drivers/staging/rtl8192e/Module.symvers] Error 1
make: *** [Makefile:1978: modpost] Error 2

Quoted 4 lines
Quoted 1 line

Did this compile on your system Yogesh?

No, while sending the patch I partially compiled it
make drivers/staging/rtl8192e but when I fully compile it make all it gives me the same error. It is a mistake on my part.

To build one module you need to use:
make -C . M=drivers/staging/rtl8192e

Quoted 10 lines
Quoted 7 lines

But I think it does not work because this driver is divided into two modules.

To load the driver I am using the following lines:
sudo insmod drivers/staging/rtl8192e/rtllib.ko sudo insmod drivers/staging/rtl8192e/rtl8192e/r8192e_pci.ko

I was not aware of this and assumed that ideally the driver has only one (PCI) interface so all the files should be compiled into one .ko file and loaded.

Quoted 8 lines
Quoted 3 lines

So this line is required:
priv->rtllib->SetWirelessMode = rtl92e_set_wireless_mode; as one name is used in one module and one in the other module.

Yes, this makes sense now.

Moving forward,
Dan, Greg and Philipp what would be the best fix for this issue,
1. Patchset to combine both the modules into one module

I had a look for this and it was not difficult at all to combine. But a patch long ago divided the two modules, to make it more like the driver this one should be merged to, in the wireless subsystem. Please see TODO file.

Quoted 6 lines

2. Revert back this patchset to v1.

Looking forward for your response.

Regards
Yogesh

Bye Philipp

build bot
#8 lore raw

Re: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode

In reply to Yogesh Hegde
kernel test robot noticed the following build errors:

Hi Yogesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on staging/staging-next staging/staging-linus linus/master v6.4-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yogesh-Hegde/staging-rtl8192e-Remove-variable-SetWirelessMode/20230607-230636
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/fba56522e419351b05e33df8cd0ac31806534d8c.1686149467.git.yogi.kernel%40gmail.com
patch subject: [PATCH v2 1/5] staging: rtl8192e: Remove variable SetWirelessMode
config: x86_64-randconfig-s053-20230611 (https://download.01.org/0day-ci/archive/20230612/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/823187b988791852e562dba90e5eb7c7e7df8eca
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yogesh-Hegde/staging-rtl8192e-Remove-variable-SetWirelessMode/20230607-230636
        git checkout 823187b988791852e562dba90e5eb7c7e7df8eca
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 olddefconfig
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "rtl92e_set_wireless_mode" [drivers/staging/rtl8192e/rtllib.ko] undefined!
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
build bot
#10 lore raw

Re: [PATCH v2 2/5] staging: rtl8192e: Remove variable SetBWModeHandler

In reply to Yogesh Hegde
kernel test robot noticed the following build errors:

Hi Yogesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on staging/staging-next staging/staging-linus linus/master v6.4-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yogesh-Hegde/staging-rtl8192e-Remove-variable-SetWirelessMode/20230607-230636
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/0af9595916447a28e27c249f3696fe107382ca53.1686149467.git.yogi.kernel%40gmail.com
patch subject: [PATCH v2 2/5] staging: rtl8192e: Remove variable SetBWModeHandler
config: x86_64-randconfig-s053-20230611 (https://download.01.org/0day-ci/archive/20230613/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/58c941ed304bd37db4311cd7151f399f8e77daa6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yogesh-Hegde/staging-rtl8192e-Remove-variable-SetWirelessMode/20230607-230636
        git checkout 58c941ed304bd37db4311cd7151f399f8e77daa6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 olddefconfig
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "rtl92e_set_bw_mode" [drivers/staging/rtl8192e/rtllib.ko] undefined!

ERROR: modpost: "rtl92e_set_wireless_mode" [drivers/staging/rtl8192e/rtllib.ko] undefined!

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#15 lore raw
In reply to Yogesh Hegde
Quoted 13 lines · Yogesh Hegde

These patches remove variables in the struct rtllib_device that were set only once throughout the driver. Instead of using these variables, the patches modify the relevant code to directly call the corresponding functions, eliminating unnecessary indirection and removing CamelCase. The patches are required to be applied in sequence.

Yogesh Hegde (5):
staging: rtl8192e: Remove variable SetWirelessMode
staging: rtl8192e: Remove variable SetBWModeHandler
staging: rtl8192e: Remove variable LeisurePSLeave
staging: rtl8192e: Remove variable InitialGainHandler
staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

Quoted 14 lines
 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c |  4 ++--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c   |  4 ----
 drivers/staging/rtl8192e/rtl819x_HTProc.c      |  8 ++++----
 drivers/staging/rtl8192e/rtllib.h              |  6 ------
 drivers/staging/rtl8192e/rtllib_debug.h        |  5 -----
 drivers/staging/rtl8192e/rtllib_rx.c           |  3 ++-
 drivers/staging/rtl8192e/rtllib_softmac.c      | 16 +++++++---------
 drivers/staging/rtl8192e/rtllib_softmac_wx.c   |  9 +++++----
 8 files changed, 20 insertions(+), 35 deletions(-)
Signature · 1 line
2.25.1

These break the build, as reported by the test robot, please fix up.

thanks,

greg k-h

author
#16 lore raw
Quoted 24 lines · Greg Kroah-Hartman
Quoted 23 lines · Yogesh Hegde

These patches remove variables in the struct rtllib_device that were set only once throughout the driver. Instead of using these variables, the patches modify the relevant code to directly call the corresponding functions, eliminating unnecessary indirection and removing CamelCase. The patches are required to be applied in sequence.

Yogesh Hegde (5):
staging: rtl8192e: Remove variable SetWirelessMode
staging: rtl8192e: Remove variable SetBWModeHandler
staging: rtl8192e: Remove variable LeisurePSLeave
staging: rtl8192e: Remove variable InitialGainHandler
staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c |  4 ++--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c   |  4 ----
 drivers/staging/rtl8192e/rtl819x_HTProc.c      |  8 ++++----
 drivers/staging/rtl8192e/rtllib.h              |  6 ------
 drivers/staging/rtl8192e/rtllib_debug.h        |  5 -----
 drivers/staging/rtl8192e/rtllib_rx.c           |  3 ++-
 drivers/staging/rtl8192e/rtllib_softmac.c      | 16 +++++++---------
 drivers/staging/rtl8192e/rtllib_softmac_wx.c   |  9 +++++----
 8 files changed, 20 insertions(+), 35 deletions(-)
Quoted 6 lines
Quoted 4 lines
Signature · 1 line
2.25.1

These break the build, as reported by the test robot, please fix up.

Hi Greg,

These patches do not work because this driver is divided into two modules.

The driver is loaded in 2 different modules:
sudo insmod drivers/staging/rtl8192e/rtllib.ko sudo insmod drivers/staging/rtl8192e/rtl8192e/r8192e_pci.ko

So this line is required:
priv->rtllib->SetWirelessMode = rtl92e_set_wireless_mode; as one name is used in one module and one in the other module.

So when I suggested these fixes
1. Patchset to combine both the modules into one module.
2. Or resending v1 of the patchset.

to Dan Carpenter <[email protected]> and
Philipp Hortmann <[email protected]>, in their opinion both the suggested fixes are incorrect, which I agree with.

Link to conversation -
https://lore.kernel.org/linux-staging/[email protected]/

I have yet to find a correct way to solve this problem. Please let me know if you have any suggestions.

Regards
Yogesh

maintainer
#17 lore raw
Quoted 53 lines · Yogesh Hegde
Quoted 24 lines · Greg Kroah-Hartman
Quoted 23 lines · Yogesh Hegde

These patches remove variables in the struct rtllib_device that were set only once throughout the driver. Instead of using these variables, the patches modify the relevant code to directly call the corresponding functions, eliminating unnecessary indirection and removing CamelCase. The patches are required to be applied in sequence.

Yogesh Hegde (5):
staging: rtl8192e: Remove variable SetWirelessMode
staging: rtl8192e: Remove variable SetBWModeHandler
staging: rtl8192e: Remove variable LeisurePSLeave
staging: rtl8192e: Remove variable InitialGainHandler
staging: rtl8192e: Remove DRV_NAME definition in rtllib_debug.h

 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c |  4 ++--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c   |  4 ----
 drivers/staging/rtl8192e/rtl819x_HTProc.c      |  8 ++++----
 drivers/staging/rtl8192e/rtllib.h              |  6 ------
 drivers/staging/rtl8192e/rtllib_debug.h        |  5 -----
 drivers/staging/rtl8192e/rtllib_rx.c           |  3 ++-
 drivers/staging/rtl8192e/rtllib_softmac.c      | 16 +++++++---------
 drivers/staging/rtl8192e/rtllib_softmac_wx.c   |  9 +++++----
 8 files changed, 20 insertions(+), 35 deletions(-)
Quoted 6 lines
Quoted 4 lines
Signature · 1 line
2.25.1

These break the build, as reported by the test robot, please fix up.

Hi Greg,

These patches do not work because this driver is divided into two modules.

The driver is loaded in 2 different modules:
sudo insmod drivers/staging/rtl8192e/rtllib.ko sudo insmod drivers/staging/rtl8192e/rtl8192e/r8192e_pci.ko

So this line is required:
priv->rtllib->SetWirelessMode = rtl92e_set_wireless_mode; as one name is used in one module and one in the other module.

So when I suggested these fixes
1. Patchset to combine both the modules into one module.
2. Or resending v1 of the patchset.

to Dan Carpenter <[email protected]> and
Philipp Hortmann <[email protected]>, in their opinion both the suggested fixes are incorrect, which I agree with.

Quoted 5 lines

Link to conversation -
https://lore.kernel.org/linux-staging/[email protected]/

I have yet to find a correct way to solve this problem. Please let me know if you have any suggestions.

I don't know, sorry, that's up to you. If a symbol is used in one module, then of course you can't remove it :)

don't mess with the modules split for now, just get the existing code cleaner, that's probably much easier, right?

thanks,

greg k-h

author
#18 lore raw
Quoted 5 lines · Greg Kroah-Hartman

I don't know, sorry, that's up to you. If a symbol is used in one module, then of course you can't remove it :)

don't mess with the modules split for now, just get the existing code cleaner, that's probably much easier, right?

Yes, clean up is easier. I will resend the v1 patchset. It only cleans up the variable names leaving the rest untouched.

Thank you all for reviewing the patches.

Thanks and Regards
Yogesh