On Fri, Jul 28, 2023 at 11:43:43AM +0300, Dmitry Antipov wrote: > Since 'mwifiex_process_tx()' is the only place from where both > 'mwifiex_process_sta_txpd()' and 'mwifiex_process_uap_txpd()' > are called, these functions may be converted to 'void' after > moving skb layout check to the caller, which may be simplified > as well. Also adjust somewhat obfuscating error messages and > add 'mwifiex_interface_name()' to make them a bit more useful. Do you actually run this driver on anything, or are you just compile testing / running static analysis? Because IIUC, all these messages are perfectly clear when using the mwifiex_dbg() macro, which usually ends up in dev_info(), and includes things like "mwifiex_pcie", "mwifiex_sdio", and "mwifiex_usb" already. So the mwifiex_interface_name() stuff just seems superfluous. I'd suggest removing that. At the same time, it seems like you're working hard on trying *not* to get your stuff merged in a timely manner, as you shift the goalposts every time you refactor your series. Specifically, you're now violating basic guidelines like these: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_messages "If you find yourself listing out a number of changes in the commit message as a bulleted list or similar, consider splitting up the patch into discrete changes that each do one thing. Similarly, if one of the additional considerations is refactoring, try to shift that into a separate patch." This started out as "avoid BUG_ON()", which is a great goal on its own. But now you haven't even mentioned that in your laundry list of refactors. This seems like you have at least two patch candidates here: refactoring the error handling, and dropping the BUG_ON(). (If the refactoring becomes extremely trivial, maybe this is OK as 1 patch. But in its current form, it's not.) Before you resubmit, please read the patch submission guidelines again. I'd also suggest sticking this (as multiple patches) at the end of the series, so the other patches (which have been reviewed/ack'd multiple times) can land cleanly. Brian > Suggested-by: Brian Norris <briannorris@xxxxxxxxxxxx> > Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx> > --- > v2: some redesign in attempt to integrate Brian's feedback [...]
