On Fri, Dec 06, 2024 at 07:53:15PM -0300, Rodrigo Gobbi wrote: > Remove code that is depending on DBG_RX_SIGNAL_DISPLAY_RAW_DATA cflag > since it's not compiling and there is no reference for it's usage; > > Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@xxxxxxxxx> > --- > The previous version suggested to fix the tiny error over the cflag usage. > Greg pointed at [1] that is not common to build using cflags. Since there > is no reference about this debug usage flag, I'm removing all the code depending on it. > Tks and regards. > > [1] https://lore.kernel.org/lkml/20241125225308.8702-1-rodrigo.gobbi.7@xxxxxxxxx/t/#med5299468bc43fa89d18892d6d869a93d6138475 > --- > Changelog > v3: remove code as suggested; > v2: https://lore.kernel.org/lkml/20241125225308.8702-1-rodrigo.gobbi.7@xxxxxxxxx/t/#mf22f30a9c689bd793988d7e7a58c0b119206116c > v1: https://lore.kernel.org/linux-staging/2024112500-authentic-primarily-b5da@gregkh/T/#mea4fba3775a1015f345dfe322854c55db0cddf43 > --- > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 1 - > drivers/staging/rtl8723bs/hal/hal_com.c | 55 ------------------- > .../staging/rtl8723bs/hal/rtl8723b_rxdesc.c | 4 -- > drivers/staging/rtl8723bs/include/hal_com.h | 5 -- > drivers/staging/rtl8723bs/include/rtw_recv.h | 18 ------ > 5 files changed, 83 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > index 317f3db19397..952ce6dd5af9 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > @@ -4959,7 +4959,6 @@ void _linked_info_dump(struct adapter *padapter) > rtw_hal_get_def_var(padapter, HW_DEF_RA_INFO_DUMP, &i); > } > } > - rtw_hal_set_def_var(padapter, HAL_DEF_DBG_RX_INFO_DUMP, NULL); When I'm reading this commit I think, "How is this related to removing the ifdef?" The commit message should answer any kind of reasonable questions a review is probably going to ask but it doesn't give any information on this. I figured it out by reviewing the code, but I shouldn't *have* to. It should be in the commit message. This commit would easier to review if it were broken up in a different way. [patch 1] just delete DBG_RX_SIGNAL_DISPLAY_RAW_DATA ifdefed code [patch 2] delete HAL_DEF_DBG_RX_INFO_DUMP and related code [patch 3] delete SetHalDefVar() and all the callers > } > } > > diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c > index 95fb38283c58..b41ec89932af 100644 > --- a/drivers/staging/rtl8723bs/hal/hal_com.c > +++ b/drivers/staging/rtl8723bs/hal/hal_com.c > @@ -682,14 +682,6 @@ u8 SetHalDefVar( > u8 bResult = _SUCCESS; > > switch (variable) { > - case HAL_DEF_DBG_RX_INFO_DUMP: > - > - if (odm->bLinked) { > - #ifdef DBG_RX_SIGNAL_DISPLAY_RAW_DATA > - rtw_dump_raw_rssi_info(adapter); > - #endif > - } > - break; So just leave this like: case HAL_DEF_DBG_RX_INFO_DUMP: break; Otherwise I have to wonder, are there any other callers which pass HAL_DEF_DBG_RX_INFO_DUMP? Because if there are, then now they go to the default case and that triggers a bug. patch 2 would delete all HAL_DEF_DBG_RX_INFO_DUMP related code. That is an easy patch to review. There are probably other unused values in the enum. patch 3 would delete SetHalDefVar() and all related code. Again pretty easy to review because any missed callers would trigger a build error. regards, dan carpenter
