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

Yogesh Hegde
2023-06-15T14:09:13Z

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