I noticed that recently some debug sources were removed at [1], but some commented code were left inside some files, which I think was used at development stage. [1] https://lore.kernel.org/all/ab3d501e2ef0bb3980d8d271fb667ce20ed8dca5.1725826273.git.philipp.g.hortmann@gmail.com/ The change to pr_debug over hal/hal_com.c, function rtw_dump_raw_rssi_info(), doesn't impact the user since is reported as 'debug only' on the original caller: _linked_info_dump() I don't have the hw, just loaded the module inside qemu after those changes. Also, I'm suggesting in the series to remove the commented statements rather than changing them to pr_xxx() but I would like to ask an opinion about this. Tks and regards. Rodrigo Gobbi (2): staging: rtl8723bs: change remaining printk to proper api staging: rtl8723bs: remove unused debug statements drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 6 +++--- drivers/staging/rtl8723bs/hal/hal_com.c | 8 ++++---- drivers/staging/rtl8723bs/hal/odm.c | 4 ---- .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 12 ++++++------ .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 12 +----------- drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 17 ++--------------- drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- 7 files changed, 17 insertions(+), 44 deletions(-) -- 2.34.1
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~lkcamp/patches/patches/55481/mbox | git am -3Learn more about email & git
As part of TODO file for future work, use dyn debug api for remaining printk statements. Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com> --- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 6 +++--- drivers/staging/rtl8723bs/hal/hal_com.c | 7 ++++--- drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 10 ++++++---- drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c index bbdd5fce28a1..58da34f125db 100644 --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c @@ -1870,10 +1870,10 @@ unsigned int OnAction_sa_query(struct adapter *padapter, union recv_frame *precv if (0) { int pp; - printk("pattrib->pktlen = %d =>", pattrib->pkt_len); + pr_debug("pattrib->pktlen = %d =>", pattrib->pkt_len); for (pp = 0; pp < pattrib->pkt_len; pp++) - printk(" %02x ", pframe[pp]); - printk("\n"); + pr_debug(" %02x ", pframe[pp]); + pr_debug("\n");
Dan Carpenter <dan.carpenter@linaro.org>No, this isn't right. You'd need to use a mix of dev_dbg() and pr_cont(). Basically in drivers it should always be dev_ printks except for pr_cont().Tks, Dan, for the answer and suggestion. In this case, rather the dev_xxx(), do you think we can go with netdev_dbg() since this is a network driver and around the rtw_mlme_ext.c is already being used? (actually, I don't see a direct reference for struct device). If yes, I would mix the netdev_dbg() and pr_cont().Please use netdev_dbg() for a networking driver. thanks, greg k-hAbout [2], it is acceptable to remove the statements like I'm proposing? Tks and regards. [2] https://lore.kernel.org/linux-staging/20241015014738.41685-1-rodrigo.gobbi.7@gmail.com/T/#m85d88af4dd8158a8e62ccb6c7257478b7de46d95regards, dan carpenter
} return _SUCCESS; diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c index 719dd116d807..484e0b4e489f 100644 --- a/drivers/staging/rtl8723bs/hal/hal_com.c +++ b/drivers/staging/rtl8723bs/hal/hal_com.c @@ -910,10 +910,11 @@ void rtw_dump_raw_rssi_info(struct adapter *padapter) for (rf_path = 0; rf_path < pHalData->NumTotalRFPath; rf_path++) { if (!isCCKrate) { - printk(", rx_ofdm_pwr:%d(dBm), rx_ofdm_snr:%d(dB)\n", - psample_pkt_rssi->ofdm_pwr[rf_path], psample_pkt_rssi->ofdm_snr[rf_path]); + pr_debug(", rx_ofdm_pwr:%d(dBm), rx_ofdm_snr:%d(dB)\n", + psample_pkt_rssi->ofdm_pwr[rf_path], + psample_pkt_rssi->ofdm_snr[rf_path]); } else { - printk("\n"); + pr_debug("\n"); } } } diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c index 37ebbbf408ec..b9c6cd1f80d6 100644 --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c @@ -62,7 +62,8 @@ static int _BlockWrite(struct adapter *padapter, void *buffer, u32 buffSize) for (i = 0; i < blockCount_p1; i++) { ret = rtw_write32(padapter, (FW_8723B_START_ADDRESS + i * blockSize_p1), *((u32 *)(bufferPtr + i * blockSize_p1))); if (ret == _FAIL) { - printk("====>%s %d i:%d\n", __func__, __LINE__, i); + pr_debug("write failed at %s %d, block:%d\n", + __func__, __LINE__, i); goto exit; } } @@ -85,7 +86,8 @@ static int _BlockWrite(struct adapter *padapter, void *buffer, u32 buffSize) ret = rtw_write8(padapter, (FW_8723B_START_ADDRESS + offset + i), *(bufferPtr + offset + i)); if (ret == _FAIL) { - printk("====>%s %d i:%d\n", __func__, __LINE__, i); + pr_debug("write failed at %s %d, block:%d\n", + __func__, __LINE__, i); goto exit; } } @@ -127,7 +129,7 @@ static int _WriteFW(struct adapter *padapter, void *buffer, u32 size) ret = _PageWrite(padapter, page, bufferPtr+offset, MAX_DLFW_PAGE_SIZE); if (ret == _FAIL) { - printk("====>%s %d\n", __func__, __LINE__); + pr_debug("page write failed at %s %d\n", __func__, __LINE__); goto exit; } } @@ -138,7 +140,7 @@ static int _WriteFW(struct adapter *padapter, void *buffer, u32 size) ret = _PageWrite(padapter, page, bufferPtr+offset, remainSize); if (ret == _FAIL) { - printk("====>%s %d\n", __func__, __LINE__); + pr_debug("remaining page write failed at %s %d\n", __func__, __LINE__); goto exit; } } diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c index d18fde4e5d6c..b845089e8d8e 100644 --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c @@ -72,7 +72,7 @@ static int sdio_alloc_irq(struct dvobj_priv *dvobj) err = sdio_claim_irq(func, &sd_sync_int_hdl); if (err) { dvobj->drv_dbg.dbg_sdio_alloc_irq_error_cnt++; - printk(KERN_CRIT "%s: sdio_claim_irq FAIL(%d)!\n", __func__, err); + pr_crit("%s: sdio_claim_irq FAIL(%d)!\n", __func__, err); } else { dvobj->drv_dbg.dbg_sdio_alloc_irq_cnt++; dvobj->irq_alloc = 1; -- 2.34.1
Remove both commented printk() and commented DEBUG_ERR() statements around the driver. Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com> --- drivers/staging/rtl8723bs/hal/hal_com.c | 1 - drivers/staging/rtl8723bs/hal/odm.c | 4 ---- .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 -- .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 12 +----------- drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 17 ++--------------- 5 files changed, 3 insertions(+), 33 deletions(-) diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c index 484e0b4e489f..5994e574ae99 100644 --- a/drivers/staging/rtl8723bs/hal/hal_com.c +++ b/drivers/staging/rtl8723bs/hal/hal_com.c @@ -883,7 +883,6 @@ void rtw_hal_check_rxfifo_full(struct adapter *adapter) int save_cnt = false; /* switch counter to RX fifo */ - /* printk("8723b or 8192e , MAC_667 set 0xf0\n"); */ rtw_write8(adapter, REG_RXERR_RPT+3, rtw_read8(adapter, REG_RXERR_RPT+3)|0xf0); save_cnt = true; /* todo: other chips */ diff --git a/drivers/staging/rtl8723bs/hal/odm.c b/drivers/staging/rtl8723bs/hal/odm.c index ea3b4cd32360..2cec8d8aeaef 100644 --- a/drivers/staging/rtl8723bs/hal/odm.c +++ b/drivers/staging/rtl8723bs/hal/odm.c @@ -433,7 +433,6 @@ static void odm_RefreshRateAdaptiveMaskCE(struct dm_odm_t *pDM_Odm) continue; if (true == ODM_RAStateCheck(pDM_Odm, pstat->rssi_stat.UndecoratedSmoothedPWDB, false, &pstat->rssi_level)) { - /* printk("RSSI:%d, RSSI_LEVEL:%d\n", pstat->rssi_stat.UndecoratedSmoothedPWDB, pstat->rssi_level); */ rtw_hal_update_ra_mask(pstat, pstat->rssi_level); } @@ -512,7 +511,6 @@ bool ODM_RAStateCheck( RATRState = DM_RATR_STA_MIDDLE; else RATRState = DM_RATR_STA_LOW; - /* printk("==>%s, RATRState:0x%02x , RSSI:%d\n", __func__, RATRState, RSSI); */ if (*pRATRState != RATRState || bForceUpdate) { *pRATRState = RATRState; @@ -593,8 +591,6 @@ static void odm_RSSIMonitorCheckCE(struct dm_odm_t *pDM_Odm) } } - /* printk("%s ==> sta_cnt(%d)\n", __func__, sta_cnt); */ - for (i = 0; i < sta_cnt; i++) { if (PWDB_rssi[i] != (0)) { if (pHalData->fw_ractrl == true)/* Report every sta's RSSI to FW */ diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c index b9c6cd1f80d6..1b1929061083 100644 --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c @@ -53,8 +53,6 @@ static int _BlockWrite(struct adapter *padapter, void *buffer, u32 buffSize) u8 *bufferPtr = buffer; u32 i = 0, offset = 0; -/* printk("====>%s %d\n", __func__, __LINE__); */ - /* 3 Phase #1 */ blockCount_p1 = buffSize / blockSize_p1; remainSize_p1 = buffSize % blockSize_p1; diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c index b63a74e669bc..c053ee9c1361 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c @@ -581,7 +581,6 @@ static int rtw_cfg80211_ap_set_encryption(struct net_device *dev, struct ieee_pa memcpy(grpkey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(txkey, &(param->u.crypt.key[16]), 8); memcpy(rxkey, &(param->u.crypt.key[24]), 8); @@ -626,7 +625,6 @@ static int rtw_cfg80211_ap_set_encryption(struct net_device *dev, struct ieee_pa } else if (strcmp(param->u.crypt.alg, "TKIP") == 0) { psta->dot118021XPrivacy = _TKIP_; - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(psta->dot11tkiptxmickey.skey, &(param->u.crypt.key[16]), 8); memcpy(psta->dot11tkiprxmickey.skey, &(param->u.crypt.key[24]), 8); @@ -657,7 +655,6 @@ static int rtw_cfg80211_ap_set_encryption(struct net_device *dev, struct ieee_pa memcpy(grpkey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(txkey, &(param->u.crypt.key[16]), 8); memcpy(rxkey, &(param->u.crypt.key[24]), 8); @@ -785,7 +782,6 @@ static int rtw_cfg80211_set_encryption(struct net_device *dev, struct ieee_param memcpy(psta->dot118021x_UncstKey.skey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); if (strcmp(param->u.crypt.alg, "TKIP") == 0) { /* set mic key */ - /* DEBUG_ERR(("\nset key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len)); */ memcpy(psta->dot11tkiptxmickey.skey, &(param->u.crypt.key[16]), 8); memcpy(psta->dot11tkiprxmickey.skey, &(param->u.crypt.key[24]), 8); @@ -806,10 +802,6 @@ static int rtw_cfg80211_set_encryption(struct net_device *dev, struct ieee_param } else if (strcmp(param->u.crypt.alg, "BIP") == 0) { /* save the IGTK key, length 16 bytes */ memcpy(padapter->securitypriv.dot11wBIPKey[param->u.crypt.idx].skey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /* - for (no = 0;no<16;no++) - printk(" %02x ", padapter->securitypriv.dot11wBIPKey[param->u.crypt.idx].skey[no]); - */ padapter->securitypriv.dot11wBIPKeyid = param->u.crypt.idx; padapter->securitypriv.binstallBIPkey = true; } @@ -817,9 +809,7 @@ static int rtw_cfg80211_set_encryption(struct net_device *dev, struct ieee_param } pbcmc_sta = rtw_get_bcmc_stainfo(padapter); - if (!pbcmc_sta) { - /* DEBUG_ERR(("Set OID_802_11_ADD_KEY: bcmc stainfo is null\n")); */ - } else { + if (pbcmc_sta) { /* Jeff: don't disable ieee8021x_blocked while clearing key */ if (strcmp(param->u.crypt.alg, "none") != 0) pbcmc_sta->ieee8021x_blocked = false; diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c index a9e481e182ad..793b051536f3 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c @@ -138,9 +138,7 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param, if (check_fwstate(pmlmepriv, WIFI_STATION_STATE | WIFI_MP_STATE) == true) { /* sta mode */ psta = rtw_get_stainfo(pstapriv, get_bssid(pmlmepriv)); - if (!psta) { - /* DEBUG_ERR(("Set wpa_set_encryption: Obtain Sta_info fail\n")); */ - } else { + if (psta) { /* Jeff: don't disable ieee8021x_blocked while clearing key */ if (strcmp(param->u.crypt.alg, "none") != 0) psta->ieee8021x_blocked = false; @@ -154,7 +152,6 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param, memcpy(psta->dot118021x_UncstKey.skey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); if (strcmp(param->u.crypt.alg, "TKIP") == 0) { /* set mic key */ - /* DEBUG_ERR(("\nset key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len)); */ memcpy(psta->dot11tkiptxmickey.skey, ¶m->u.crypt.key[16], 8); memcpy(psta->dot11tkiprxmickey.skey, ¶m->u.crypt.key[24], 8); @@ -177,13 +174,8 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param, rtw_set_key(padapter, &padapter->securitypriv, param->u.crypt.idx, 1, true); } else if (strcmp(param->u.crypt.alg, "BIP") == 0) { - /* printk("BIP key_len =%d , index =%d @@@@@@@@@@@@@@@@@@\n", param->u.crypt.key_len, param->u.crypt.idx); */ /* save the IGTK key, length 16 bytes */ memcpy(padapter->securitypriv.dot11wBIPKey[param->u.crypt.idx].skey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /*printk("IGTK key below:\n"); - for (no = 0;no<16;no++) - printk(" %02x ", padapter->securitypriv.dot11wBIPKey[param->u.crypt.idx].skey[no]); - printk("\n");*/ padapter->securitypriv.dot11wBIPKeyid = param->u.crypt.idx; padapter->securitypriv.binstallBIPkey = true; } @@ -191,9 +183,7 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param, } pbcmc_sta = rtw_get_bcmc_stainfo(padapter); - if (!pbcmc_sta) { - /* DEBUG_ERR(("Set OID_802_11_ADD_KEY: bcmc stainfo is null\n")); */ - } else { + if (pbcmc_sta) { /* Jeff: don't disable ieee8021x_blocked while clearing key */ if (strcmp(param->u.crypt.alg, "none") != 0) pbcmc_sta->ieee8021x_blocked = false; @@ -629,7 +619,6 @@ static int rtw_set_encryption(struct net_device *dev, struct ieee_param *param, memcpy(grpkey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(txkey, ¶m->u.crypt.key[16], 8); memcpy(psecuritypriv->dot118021XGrprxmickey[param->u.crypt.idx].skey, ¶m->u.crypt.key[24], 8); @@ -674,7 +663,6 @@ static int rtw_set_encryption(struct net_device *dev, struct ieee_param *param, } else if (strcmp(param->u.crypt.alg, "TKIP") == 0) { psta->dot118021XPrivacy = _TKIP_; - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(psta->dot11tkiptxmickey.skey, ¶m->u.crypt.key[16], 8); memcpy(psta->dot11tkiprxmickey.skey, ¶m->u.crypt.key[24], 8); @@ -703,7 +691,6 @@ static int rtw_set_encryption(struct net_device *dev, struct ieee_param *param, memcpy(grpkey, param->u.crypt.key, (param->u.crypt.key_len > 16 ? 16 : param->u.crypt.key_len)); - /* DEBUG_ERR("set key length :param->u.crypt.key_len =%d\n", param->u.crypt.key_len); */ /* set mic key */ memcpy(txkey, ¶m->u.crypt.key[16], 8); memcpy(rxkey, ¶m->u.crypt.key[24], 8); -- 2.34.1