[PATCH v2 0/5] Trivial code cleanup patches

[PATCH v2 0/5] Trivial code cleanup patches

[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(-)

-- 
2.25.1

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

[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 <yogi.kernel@gmail.com>
---

v2: Removed the variable and called the function direction instead of
    just renaming the variable as suggested by Greg Kroah-Hartman
    <gregkh@linuxfoundation.org>.
---
 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(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 17b70dde7eeb..bbe0864e1348 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -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;
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 4aa5ce9f7792..68a3b8af3119 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -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);
diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
index 7c4cba6dcf46..ed171eca4916 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -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;
-- 
2.25.1

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

[Dan Carpenter]

On Wed, Jun 07, 2023 at 08:31:41PM +0530, Yogesh Hegde wrote:
> 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 <yogi.kernel@gmail.com>
> ---
> 
> v2: Removed the variable and called the function direction instead of
>     just renaming the variable as suggested by Greg Kroah-Hartman
>     <gregkh@linuxfoundation.org>.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

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

[Philipp Hortmann]

On 6/7/23 17:07, Dan Carpenter wrote:
> On Wed, Jun 07, 2023 at 08:31:41PM +0530, Yogesh Hegde wrote:
>> 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 <yogi.kernel@gmail.com>
>> ---
>>
>> v2: Removed the variable and called the function direction instead of
>>      just renaming the variable as suggested by Greg Kroah-Hartman
>>      <gregkh@linuxfoundation.org>.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> 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

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

[Dan Carpenter]

On Wed, Jun 07, 2023 at 10:18:28PM +0200, Philipp Hortmann wrote:
> On 6/7/23 17:07, Dan Carpenter wrote:
> > On Wed, Jun 07, 2023 at 08:31:41PM +0530, Yogesh Hegde wrote:
> > > 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 <yogi.kernel@gmail.com>
> > > ---
> > > 
> > > v2: Removed the variable and called the function direction instead of
> > >      just renaming the variable as suggested by Greg Kroah-Hartman
> > >      <gregkh@linuxfoundation.org>.
> > 
> > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> > 
> > regards,
> > dan carpenter
> > 
> > 
> 
> Hi Dan,
> 
> thanks for all the work you do. I always appreciate your comments.
> 

No need to butter me up.  :P


> 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

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

[Yogesh Hegde]

On Wed, Jun 07, 2023 at 10:18:28PM +0200, Philipp Hortmann wrote:
> 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. 

> 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.

> 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 

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

[Dan Carpenter]

On Thu, Jun 08, 2023 at 03:29:38PM +0530, Yogesh Hegde wrote:
> 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

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

[Philipp Hortmann]

On 6/8/23 11:59, Yogesh Hegde wrote:
> On Wed, Jun 07, 2023 at 10:18:28PM +0200, Philipp Hortmann wrote:
>> 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.
To build one module you need to use:
make -C . M=drivers/staging/rtl8192e

> 
>> 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.
> 
>> 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.

> 2.  Revert back this patchset to v1.
> 
> Looking forward for your response.
> 
> Regards
> Yogesh

Bye Philipp

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

[kernel test robot]

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/202306120923.uMeOfojk-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306120923.uMeOfojk-lkp@intel.com/

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

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

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[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 <yogi.kernel@gmail.com>
---

v2: Removed the variable and called the function direction instead of
    just renaming the variable as suggested by Greg Kroah-Hartman
    <gregkh@linuxfoundation.org>
---
 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(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index bbe0864e1348..6f6c31344441 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -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;
diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c b/drivers/staging/rtl8192e/rtl819x_HTProc.c
index fe30a291e64c..ad92a8230055 100644
--- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
+++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
@@ -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;
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 68a3b8af3119..d22a586043f2 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -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);
diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
index d85a32d2d050..9fb4fee93990 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
@@ -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);
 	}
-- 
2.25.1

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

[kernel test robot]

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/202306131731.yvRofzzm-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306131731.yvRofzzm-lkp@intel.com/

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!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[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 <yogi.kernel@gmail.com>
---

v2: Removed the variable and called the function direction instead of
    just renaming the variable as suggested by Greg Kroah-Hartman
    <gregkh@linuxfoundation.org>
---
 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(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 6f6c31344441..231903002233 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -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;
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index d22a586043f2..f7fd8b77db99 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -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
diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
index d44bf261de54..d8acede4b808 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -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);
 			}
 		}
 	}
diff --git a/drivers/staging/rtl8192e/rtllib_softmac_wx.c b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
index 9fb4fee93990..07c46ad4f20e 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac_wx.c
@@ -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);
-- 
2.25.1

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

[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 <yogi.kernel@gmail.com>
---

v2: Removed the variable and called the function direction instead of
    just renaming the variable as suggested by Greg Kroah-Hartman
    <gregkh@linuxfoundation.org>
---
 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(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
index 641e993aaa86..5385bb099c56 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
@@ -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;
 		}
 	}
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 231903002233..8218319ad834 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -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;
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index f7fd8b77db99..92ed1b493f6b 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -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);
-- 
2.25.1

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

[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 <yogi.kernel@gmail.com>
---

v2: No changes.
---
 drivers/staging/rtl8192e/rtllib_debug.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_debug.h b/drivers/staging/rtl8192e/rtllib_debug.h
index f6b23defe225..ab8bd5fc4ca0 100644
--- a/drivers/staging/rtl8192e/rtllib_debug.h
+++ b/drivers/staging/rtl8192e/rtllib_debug.h
@@ -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 */
-- 
2.25.1

Re: [PATCH v2 0/5] Trivial code cleanup patches

[Dan Carpenter]

On Wed, Jun 07, 2023 at 08:31:19PM +0530, Yogesh Hegde wrote:
> 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
> 

These all look good.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH v2 0/5] Trivial code cleanup patches

[Greg Kroah-Hartman]

On Wed, Jun 07, 2023 at 08:31:19PM +0530, Yogesh Hegde wrote:
> 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(-)
> 
> -- 
> 2.25.1
> 
> 

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

thanks,

greg k-h

Re: [PATCH v2 0/5] Trivial code cleanup patches

[Yogesh Hegde]

On Thu, Jun 15, 2023 at 12:36:45PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 07, 2023 at 08:31:19PM +0530, Yogesh Hegde wrote:
> > 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(-)
> > 
> > -- 
> > 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 <dan.carpenter@linaro.org> and
Philipp Hortmann <philipp.g.hortmann@gmail.com>, in their opinion both the
suggested fixes are incorrect, which I agree with.

Link to conversation -
https://lore.kernel.org/linux-staging/90813181-02a1-b54f-132b-c68e7b797bb6@gmail.com/

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

Regards
Yogesh

Re: [PATCH v2 0/5] Trivial code cleanup patches

[Greg Kroah-Hartman]

On Thu, Jun 15, 2023 at 07:39:08PM +0530, Yogesh Hegde wrote:
> On Thu, Jun 15, 2023 at 12:36:45PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jun 07, 2023 at 08:31:19PM +0530, Yogesh Hegde wrote:
> > > 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(-)
> > > 
> > > -- 
> > > 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 <dan.carpenter@linaro.org> and
> Philipp Hortmann <philipp.g.hortmann@gmail.com>, in their opinion both the
> suggested fixes are incorrect, which I agree with.
> 
> Link to conversation -
> https://lore.kernel.org/linux-staging/90813181-02a1-b54f-132b-c68e7b797bb6@gmail.com/
> 
> 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

Re: [PATCH v2 0/5] Trivial code cleanup patches

[Yogesh Hegde]

On Thu, Jun 15, 2023 at 04:34:36PM +0200, Greg Kroah-Hartman wrote:
> 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