Re: [PATCH 00/61] staging: rtl8723bs: monolithic HAL cleanup and tx power limit fix

[Арсений Пащенко]

Hi Greg,

Thank you for the feedback, and I apologize for the messy submission.

To be completely transparent: I used an LLM to brainstorm refactoring
ideas, draft some of the verbose commit messages, and assist with
migrating the custom CMAC logic by proposing to replace the driver's
custom function with rtw_bip_cmac() using the kernel Crypto API.
However, all the standard cleanups—such as removing legacy wrappers,
dead code, and empty stubs—were executed and verified manually.

I have build-tested the entire series. However, since I do not possess
the physical rtl8723bs hardware, I realize that dropping a massive 60+
patchset without runtime testing is bad practice and disrespectful of
your time.

I must admit that I do not fully understand the internal mechanics of
this driver yet. I was mostly tracing function usage to identify
superficial wrappers, but your feedback helped me see the flaws in
this approach and understand the logic behind the desired changes. I
need to step back, learn the codebase better, and look deeper into the
underlying subsystem design.

I will drop this entire series. Instead, I would like to start over by
sending very small batches (up to 5 patches at a time), focusing
strictly on undeniable dead code and empty stubs that cannot introduce
runtime regressions. Later on, once I am more familiar with the code,
I would also like to look into eliminating or inlining trivial
pass-through wrappers that merely forward arguments.

Please let me know if it is okay to send the first small batch of stub removals.

Thanks,
Arsenii


ср, 15 июл. 2026 г. в 20:03, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>:
>
> On Wed, Jul 15, 2026 at 07:47:28PM +0700, Arsenii Pashchenko wrote:
> > Hello,
> >
> > This patch series performs a massive, systematic cleanup of the legacy
> > vendor HAL layer and redundant abstraction wrappers in the rtl8723bs
> > driver, concluding with a functional fix for the TX power limit lookup.
> >
> > The primary goal of this refactoring is to drastically improve the
> > readability and maintainability of the codebase, transforming the
> > highly tangled, Windows-legacy Realtek abstractions into a cleaner,
> > more idiomatic Linux driver structure.
> >
> > KEY CHANGES IN THIS SERIES:
> >
> > 1. Dead Code & Stub Removal: Eliminated several unimplemented functions,
> >    empty stubs (like OnAtim, DoReserved), duplicate prototypes, and
> >    completely unused helpers (NULL_hdl, set_csa_hdl, tdls_hdl).
> > 2. Wrapper Collapse & Inlining: Removed multiple layers of superficial
> >    wrappers around core operations (e.g., rtw_ap_set_group_key,
> >    issue_deauth, issue_probereq, rtw_free_mlme_priv, etc.). High-frequency
> >    or single-use HAL entry points (like SetHwReg8723BS, GetHwReg8723BS,
> >    and xmit helpers) have been carefully inlined directly into their
> >    respective dispatchers to enhance readability.
> > 3. Scope & Warning Fixes: Restructured local functions (like
> >    lps_ctrl_wk_hdl) by marking them static and aligning headers, which
> >    resolves missing-prototypes compiler warnings.
> > 4. Logical Fixes: In the final patch, a critical copy-paste bug in
> >    phy_get_tx_pwr_lmt() was resolved. The computed channel index from
> >    phy_GetChannelIndexOfTxPowerLimit() was mistakenly assigned back to
> >    the 'channel' parameter and discarded, leaving 'idx_channel' at -1.
> >    This caused the function to always fail validation and return
> >    MAX_POWER_INDEX. Correcting this restores the intended hardware tx
> >    power limit logic and resolves a compiler warning.
> > 5. Refined Abstractions: In places where removing a wrapper would
> >    decrease overall elegance or require awkward type workarounds, the
> >    cleanest architectural approach was intentionally preserved.
> >
> > TESTING & VERIFICATION:
> >
> > - Hardware: I do not possess the physical rtl8723bs hardware, so
> >   real-world runtime wireless testing was not performed.
> > - Compilation: The entire series compiles flawlessly against the
> >   latest tree.
> > - Code Style: All 61 patches have been fully validated with
> >   `scripts/checkpatch.pl` and return 0 errors / 0 warnings per patch.
> >   (Note: A few pre-existing vendor style warnings remain in the modified
> >   files, but no new ones were introduced).
> >
> > This cleanup significantly reduces the boilerplate footprint, making
> > future maintenance and eventual mac80211 migration tasks much more
> > approachable.
>
> How did you do all of these?  Without testing I am loath to take such
> large changes, right?  Did you use a llm?
>
> thanks,
>
> greg k-h