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

Greg Kroah-Hartman
2023-06-15T14:34:39Z

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