staging: rtl8723bs: remove unused diagnostic
cover staging: rtl8723bs: remove unused diagnostic
This series performs a driver-wide audit and cleanup of dead diagnostic code within the rtl8723bs staging driver.
The vendor-originated code included a variety of diagnostic counters housed within 'struct debug_priv'. These counters were updated throughout the driver's hot paths—including MLME, receive, power control, and SDIO allocation—but were never read, exposed via debugfs, or utilized for any internal logic.
Patch 1: Removes unused RX info reset logic and associated counters. Patch 2: Removes dead RX sequence gap calculation logic. Patch 3: Performs a final scrub of 'struct debug_priv' and all
remaining write-only counters throughout the driver.
Ethan Tidmore (3):
staging: rtl8723bs: remove dead RX info reset logic
staging: rtl8723bs: remove dead RX sequence logic
staging: rtl8723bs: remove unused struct debug_priv and all counters
drivers/staging/rtl8723bs/core/rtw_mlme.c | 12 ----- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 -- drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 6 +-- drivers/staging/rtl8723bs/core/rtw_recv.c | 23 +-------- .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 4 -- drivers/staging/rtl8723bs/hal/sdio_halinit.c | 8 +--- drivers/staging/rtl8723bs/include/drv_types.h | 41 ---------------- drivers/staging/rtl8723bs/os_dep/os_intfs.c | 14 ++---- drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 48 +++++-------------- 9 files changed, 17 insertions(+), 142 deletions(-)
Signature · 1 line
2.52.0
PATCH 1/3 staging: rtl8723bs: remove dead RX info reset logic
The function rtw_reset_rx_info() and its associated counters in struct debug_priv are used to track AMPDU and management frame statistics that are never read by the driver.
Remove the unused function, the write-only struct members, and the now-unused pdbgpriv/psdpriv pointers in rtw_free_assoc_resources() to clean up the MLME code.
--- drivers/staging/rtl8723bs/core/rtw_mlme.c | 12 ------------ 1 file changed, 12 deletions(-)
drivers/staging/rtl8723bs/core/rtw_mlme.c +0 -12
@@ -819,15 +819,6 @@ static void free_scanqueue(struct mlme_priv *pmlmepriv)spin_unlock_bh (&scan_queue->lock); } -static void rtw_reset_rx_info (struct debug_priv *pdbgpriv) -{ - pdbgpriv->dbg_rx_ampdu_drop_count =0 ; - pdbgpriv->dbg_rx_ampdu_forced_indicate_count =0 ; - pdbgpriv->dbg_rx_ampdu_loss_count =0 ; - pdbgpriv->dbg_rx_dup_mgt_frame_drop_count =0 ; - pdbgpriv->dbg_rx_ampdu_window_shift_cnt =0 ; -} -static void find_network (struct adapter *adapter) {struct wlan_network *pwlan =NULL ;
@@ -848,8 +839,6 @@ void rtw_free_assoc_resources(struct adapter *adapter, int lock_scanned_queue) {struct mlme_priv *pmlmepriv = &adapter->mlmepriv;struct wlan_network *tgt_network = &pmlmepriv->cur_network; -struct dvobj_priv *psdpriv = adapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;if (check_fwstate (pmlmepriv,WIFI_STATION_STATE |WIFI_AP_STATE )) {struct sta_info *psta;
@@ -874,7 +863,6 @@ void rtw_free_assoc_resources(struct adapter *adapter, int lock_scanned_queue)if (lock_scanned_queue) adapter->securitypriv.key_mask =0 ; -rtw_reset_rx_info (pdbgpriv); }/* rtw_indicate_connect: the caller has to lock pmlmepriv->lock */
Signature · 1 line
2.52.0
Re: [PATCH v1 1/3] staging: rtl8723bs: remove dead RX info reset logic
Quoted 9 lines · Ethan Tidmore
The function
rtw_reset_rx_info()and its associated counters in structdebug_privare used to track AMPDU and management frame statistics that are never read by the driver.Remove the unused function, the write-only struct members, and the now-unused pdbgpriv/psdpriv pointers in
rtw_free_assoc_resources()to clean up the MLME code.Signed-off-by Ethan Tidmore <[email protected]>
If they're never used, why can we not remove them from the struct definition as well?
Quoted 34 lines
--- drivers/staging/rtl8723bs/core/rtw_mlme.c | 12 ------------ 1 file changed, 12 deletions(-)drivers/staging/rtl8723bs/core/rtw_mlme.c +0
-11@@ -819,15 +819,6 @@ static void free_scanqueue(struct mlme_priv *pmlmepriv)spin_unlock_bh (&scan_queue->lock); } -static void rtw_reset_rx_info (struct debug_priv *pdbgpriv) -{ - pdbgpriv->dbg_rx_ampdu_drop_count =0 ; - pdbgpriv->dbg_rx_ampdu_forced_indicate_count =0 ; - pdbgpriv->dbg_rx_ampdu_loss_count =0 ; - pdbgpriv->dbg_rx_dup_mgt_frame_drop_count =0 ; - pdbgpriv->dbg_rx_ampdu_window_shift_cnt =0 ; -} -static void find_network (struct adapter *adapter) {struct wlan_network *pwlan =NULL ;@@ -848,8 +839,6 @@ void rtw_free_assoc_resources(struct adapter *adapter, int lock_scanned_queue) {struct mlme_priv *pmlmepriv = &adapter->mlmepriv;struct wlan_network *tgt_network = &pmlmepriv->cur_network; -struct dvobj_priv *psdpriv = adapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;if (check_fwstate (pmlmepriv,WIFI_STATION_STATE |WIFI_AP_STATE )) {struct sta_info *psta;
drivers/staging/rtl8723bs/core/rtw_mlme.c void rtw_free_assoc_resources(struct adapter *adapter, int lock_scanned_queue) · line 865
Quoted 3 lines
drivers/staging/rtl8723bs/core/rtw_mlme.c +0
-0@@ -874,7 +863,6 @@ void rtw_free_assoc_resources(struct adapter *adapter, int lock_scanned_queue)if (lock_scanned_queue) adapter->securitypriv.key_mask =0 ;
This blank line should be deleted as well.
regards,
dan carpenter
Quoted 7 lines
drivers/staging/rtl8723bs/core/rtw_mlme.c +0
-1-rtw_reset_rx_info (pdbgpriv); }/* rtw_indicate_connect: the caller has to lock pmlmepriv->lock */ Signature · 1 line
2.52.0
PATCH 2/3 staging: rtl8723bs: remove dead RX sequence logic
In reply to Ethan TidmoreThe function recv_indicatepkts_pkt_loss_cnt() is used to calculate values for dbg_rx_ampdu_loss_count, but this variable is never used anywhere in the driver.
Remove the function and its call site.
--- drivers/staging/rtl8723bs/core/rtw_recv.c | 10 ---------- 1 file changed, 10 deletions(-)
drivers/staging/rtl8723bs/core/rtw_recv.c +0 -10
@@ -1861,15 +1861,6 @@ static int enqueue_reorder_recvframe(struct recv_reorder_ctrl *preorder_ctrl, un } -static void recv_indicatepkts_pkt_loss_cnt (struct debug_priv *pdbgpriv,u64 prev_seq,u64 current_seq) -{ -if (current_seq < prev_seq) - pdbgpriv->dbg_rx_ampdu_loss_count += (4096 + current_seq - prev_seq); -else - pdbgpriv->dbg_rx_ampdu_loss_count += (current_seq - prev_seq); - -} -static int rtw_recv_indicatepkt (struct adapter *padapter,union recv_frame *precv_frame) {struct recv_priv *precvpriv;
@@ -1937,7 +1928,6 @@ static int recv_indicatepkts_in_order(struct adapter *padapter, struct recv_reor prframe = (union recv_frame *)plist; pattrib = &prframe->u.hdr.attrib; -recv_indicatepkts_pkt_loss_cnt (pdbgpriv, preorder_ctrl->indicate_seq, pattrib->seq_num); preorder_ctrl->indicate_seq = pattrib->seq_num; }
Signature · 1 line
2.52.0
PATCH 3/3 staging: rtl8723bs: remove unused struct debug_priv and all counters
In reply to Ethan TidmoreThe struct debug_priv is a collection of diagnostic counters that are updated throughout the driver's hot paths (RX/TX, MLME, Power Control, and SDIO) but are never read, exposed via debugfs, or used for logic.
This cleanup removes:
- The entire struct debug_priv definition and its instance in dvobj_priv.
- All write-only counter increments across the core, hal, and os_dep.
- Unused local pointers (psdpriv, pdbgpriv) that were only serving the dead diagnostic logic.
- Redundant logic blocks that only existed to update these counters.
--- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 3 -- drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 6 +-- drivers/staging/rtl8723bs/core/rtw_recv.c | 13 +---- .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 4 -- drivers/staging/rtl8723bs/hal/sdio_halinit.c | 8 +--- drivers/staging/rtl8723bs/include/drv_types.h | 41 ---------------- drivers/staging/rtl8723bs/os_dep/os_intfs.c | 14 ++---- drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 48 +++++-------------- 8 files changed, 17 insertions(+), 120 deletions(-)
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c +0 -3
@@ -438,8 +438,6 @@ void mgt_dispatcher(struct adapter *padapter, union recv_frame *precv_frame)struct mlme_priv *pmlmepriv = &padapter->mlmepriv;u8 *pframe = precv_frame->u.hdr.rx_data;struct sta_info *psta =rtw_get_stainfo (&padapter->stapriv,GetAddr2Ptr (pframe)); -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;if (GetFrameType (pframe) !=WIFI_MGT_TYPE )return ;
@@ -463,7 +461,6 @@ void mgt_dispatcher(struct adapter *padapter, union recv_frame *precv_frame)if (GetRetry (pframe)) {if (precv_frame->u.hdr.attrib.seq_num == psta->RxMgmtFrameSeqNum) {/* drop the duplicate management frame */ - pdbgpriv->dbg_rx_dup_mgt_frame_drop_count++;return ; } }
drivers/staging/rtl8723bs/core/rtw_pwrctrl.c +1 -5
@@ -139,8 +139,6 @@ static bool rtw_pwr_unassociated_idle(struct adapter *adapter)void rtw_ps_processor (struct adapter *padapter) {struct pwrctrl_priv *pwrpriv =adapter_to_pwrctl (padapter); -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;u32 ps_deny =0 ;mutex_lock (&adapter_to_pwrctl (padapter)->lock);
@@ -149,10 +147,8 @@ void rtw_ps_processor(struct adapter *padapter)if (ps_deny !=0 ) goto exit; -if (pwrpriv->bInSuspend) {/* system suspend or autosuspend */ - pdbgpriv->dbg_ps_insuspend_cnt++; +if (pwrpriv->bInSuspend)return ; - } pwrpriv->ps_processing = true;
drivers/staging/rtl8723bs/core/rtw_recv.c +1 -12
@@ -1782,9 +1782,6 @@ static int amsdu_to_msdu(struct adapter *padapter, union recv_frame *prframe)static int check_indicate_seq (struct recv_reorder_ctrl *preorder_ctrl,u16 seq_num) { -struct adapter *padapter = preorder_ctrl->padapter; -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;u8 wsize = preorder_ctrl->wsize_b;u16 wend = (preorder_ctrl->indicate_seq + wsize -1 ) %4096u ;
@@ -1810,7 +1807,6 @@ static int check_indicate_seq(struct recv_reorder_ctrl *preorder_ctrl, u16 seq_n preorder_ctrl->indicate_seq = seq_num +1 - wsize; else preorder_ctrl->indicate_seq = 0xFFF - (wsize - (seq_num + 1)) + 1; - pdbgpriv->dbg_rx_ampdu_window_shift_cnt++; }return true;
@@ -1907,8 +1903,6 @@ static int recv_indicatepkts_in_order(struct adapter *padapter, struct recv_reorint bPktInBuf = false;struct recv_priv *precvpriv = &padapter->recvpriv;struct __queue *ppending_recvframe_queue = &preorder_ctrl->pending_recvframe_queue; -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;/* spin_lock_irqsave(&ppending_recvframe_queue->lock, irql); */ /* spin_lock(&ppending_recvframe_queue->lock); */
@@ -1918,7 +1912,6 @@ static int recv_indicatepkts_in_order(struct adapter *padapter, struct recv_reor/* Handling some condition for forced indicate case. */ if (bforced == true) { - pdbgpriv->dbg_rx_ampdu_forced_indicate_count++;if (list_empty (phead)) {/* spin_unlock_irqrestore(&ppending_recvframe_queue->lock, irql); */ /* spin_unlock(&ppending_recvframe_queue->lock); */
@@ -1988,8 +1981,6 @@ static int recv_indicatepkt_reorder(struct adapter *padapter, union recv_frame *struct rx_pkt_attrib *pattrib = &prframe->u.hdr.attrib;struct recv_reorder_ctrl *preorder_ctrl = prframe->u.hdr.preorder_ctrl;struct __queue *ppending_recvframe_queue = &preorder_ctrl->pending_recvframe_queue; -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;if (!pattrib->amsdu) {/* s1. */
@@ -2032,10 +2023,8 @@ static int recv_indicatepkt_reorder(struct adapter *padapter, union recv_frame *spin_lock_bh (&ppending_recvframe_queue->lock);/* s2. check if winstart_b(indicate_seq) needs to been updated */ -if (!check_indicate_seq (preorder_ctrl, pattrib->seq_num)) { - pdbgpriv->dbg_rx_ampdu_drop_count++; +if (!check_indicate_seq (preorder_ctrl, pattrib->seq_num)) goto _err_exit; - }/* s3. Insert all packet into Reorder Queue to maintain its ordering. */
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +0 -4
@@ -304,8 +304,6 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, bool bUsedWoWLANFw)const struct firmware *fw;struct device *device =dvobj_to_dev (padapter->dvobj);u8 *fwfilepath; -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;u8 tmp_ps; pFirmware =kzalloc (sizeof (struct rt_firmware ),GFP_KERNEL );
@@ -324,8 +322,6 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, bool bUsedWoWLANFw)/* 2. read power_state = 0xA0[1:0] */ tmp_ps =rtw_read8 (padapter,0xa0 ); tmp_ps &=0x03 ; -if (tmp_ps !=0x01 ) - pdbgpriv->dbg_downloadfw_pwr_state_cnt++; fwfilepath ="rtlwifi/rtl8723bs_nic.bin" ;
drivers/staging/rtl8723bs/hal/sdio_halinit.c +1 -7
@@ -886,9 +886,6 @@ static void CardDisableRTL8723BSdio(struct adapter *padapter)u32 rtl8723bs_hal_deinit (struct adapter *padapter) { -struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg; -if (padapter->hw_init_completed) {if (adapter_to_pwrctl (padapter)->bips_processing) {if (padapter->netif_up) {
@@ -921,18 +918,15 @@ u32 rtl8723bs_hal_deinit(struct adapter *padapter)adapter_to_pwrctl (padapter)->pre_ips_type =0 ; }else { - pdbgpriv->dbg_carddisable_cnt++;CardDisableRTL8723BSdio (padapter);adapter_to_pwrctl (padapter)->pre_ips_type =1 ; } }else { - pdbgpriv->dbg_carddisable_cnt++;CardDisableRTL8723BSdio (padapter); } - }else - pdbgpriv->dbg_deinit_fail_cnt++; + }return _SUCCESS; }
drivers/staging/rtl8723bs/include/drv_types.h +0 -41
53 lines not on this page · raw message
drivers/staging/rtl8723bs/os_dep/os_intfs.c +3 -11
@@ -964,8 +964,6 @@ void rtw_ndev_destructor(struct net_device *ndev)void rtw_dev_unload (struct adapter *padapter) {struct pwrctrl_priv *pwrctl =adapter_to_pwrctl (padapter); -struct dvobj_priv *pobjpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &pobjpriv->drv_dbg;struct cmd_priv *pcmdpriv = &padapter->cmdpriv;u8 cnt =0 ;
@@ -994,7 +992,6 @@ void rtw_dev_unload(struct adapter *padapter)/* check HW status and SW state */ netdev_dbg (padapter->pnetdev,"%s: driver in IPS-FWLPS\n" , __func__); - pdbgpriv->dbg_dev_unload_inIPS_cnt++;LeaveAllPowerSaveMode (padapter); }else { netdev_dbg(padapter->pnetdev,
@@ -1077,24 +1074,21 @@ static void rtw_suspend_normal(struct adapter *padapter)void rtw_suspend_common (struct adapter *padapter) {struct dvobj_priv *psdpriv = padapter->dvobj; -struct debug_priv *pdbgpriv = &psdpriv->drv_dbg;struct pwrctrl_priv *pwrpriv =dvobj_to_pwrctl (psdpriv);struct mlme_priv *pmlmepriv = &padapter->mlmepriv;unsigned long start_time = jiffies;netdev_dbg (padapter->pnetdev," suspend start\n" ); - pdbgpriv->dbg_suspend_cnt++; pwrpriv->bInSuspend = true;while (pwrpriv->bips_processing)msleep (1 ); -if ((!padapter->bup) || (padapter->bDriverStopped) || (padapter->bSurpriseRemoved)) { - pdbgpriv->dbg_suspend_error_cnt++; +if ((!padapter->bup) || (padapter->bDriverStopped) || (padapter->bSurpriseRemoved))return ; - } +rtw_ps_deny (padapter,PS_DENY_SUSPEND );rtw_cancel_all_timer (padapter);
@@ -1123,7 +1117,6 @@ static int rtw_resume_process_normal(struct adapter *padapter)struct pwrctrl_priv *pwrpriv;struct mlme_priv *pmlmepriv;struct dvobj_priv *psdpriv; -struct debug_priv *pdbgpriv;int ret = _SUCCESS;
@@ -1136,7 +1129,7 @@ static int rtw_resume_process_normal(struct adapter *padapter) pwrpriv =adapter_to_pwrctl (padapter); pmlmepriv = &padapter->mlmepriv; psdpriv = padapter->dvobj; - pdbgpriv = &psdpriv->drv_dbg; +/* interface init */ /* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */ if ((padapter->intf_init) && (padapter->intf_init (adapter_to_dvobj (padapter)) != _SUCCESS)) {
@@ -1155,7 +1148,6 @@ static int rtw_resume_process_normal(struct adapter *padapter)if (pm_netdev_open (pnetdev, true) !=0 ) { ret =-1 ; - pdbgpriv->dbg_resume_error_cnt++; goto exit; }
drivers/staging/rtl8723bs/os_dep/sdio_intf.c +11 -37
112 lines not on this page · raw message
Signature · 1 line
2.52.0
Re: [PATCH v1 3/3] staging: rtl8723bs: remove unused struct debug_priv and all counters
Quoted 4 lines · Ethan Tidmore
The struct
debug_privis a collection of diagnostic counters that are updated throughout the driver's hot paths (RX/TX, MLME, Power Control, and SDIO) but are never read, exposed via debugfs, or used for logic.
Quoted 8 lines
This cleanup removes:
- The entire structdebug_privdefinition and its instance indvobj_priv.
- All write-only counter increments across the core, hal, andos_dep.
- Unused local pointers (psdpriv, pdbgpriv) that were only serving the dead diagnostic logic.
- Redundant logic blocks that only existed to update these counters.Signed-off-by Ethan Tidmore <[email protected]>
So in the end we did remove the struct members, but I expected that to happen in patch 1 and 2, and I spent some time figuring out if there was a reason we didn't remove the struct members.
It's hard to know how to break up a patch like this. I guess you divided it up into patches which deleted whole functions and then everything else. Which is fine... But you need to delete the struct members as you go along.
regards,
dan carpenter