[v2,0/5] Trivial code cleanup patches

Message ID cover.1686149467.git.yogi.kernel@gmail.com
Headers
Series Trivial code cleanup patches |

Message

Yogesh Hegde June 7, 2023, 3:01 p.m. UTC
  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(-)
  

Comments

Dan Carpenter June 7, 2023, 3:10 p.m. UTC | #1
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
  
Greg KH June 15, 2023, 10:36 a.m. UTC | #2
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
  
Yogesh Hegde June 15, 2023, 2:09 p.m. UTC | #3
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
  
Greg KH June 15, 2023, 2:34 p.m. UTC | #4
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
  
Yogesh Hegde June 15, 2023, 4:11 p.m. UTC | #5
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