As part of checkpatch.pl from TODO file for future work, remove
commented code and use dyn debug api for remaining printk statements.

Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
---
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()

Don't have the hw, just loaded the module inside qemu.

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c   |  6 +++---
 drivers/staging/rtl8723bs/hal/hal_com.c         |  7 +++----
 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, 16 insertions(+), 44 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");
 	}
 
 	return _SUCCESS;
diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c
index 719dd116d807..f766af76da79 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 */
@@ -910,10 +909,10 @@ 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/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 37ebbbf408ec..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;
@@ -62,7 +60,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 +84,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 +127,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 +138,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/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, &param->u.crypt.key[16], 8);
 						memcpy(psta->dot11tkiprxmickey.skey, &param->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, &param->u.crypt.key[16], 8);
 				memcpy(psecuritypriv->dot118021XGrprxmickey[param->u.crypt.idx].skey, &param->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, &param->u.crypt.key[16], 8);
 					memcpy(psta->dot11tkiprxmickey.skey, &param->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, &param->u.crypt.key[16], 8);
 					memcpy(rxkey, &param->u.crypt.key[24], 8);
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

Hey Rodrigo,

On 10/3/24 7:01 PM, Rodrigo Gobbi wrote:
> As part of checkpatch.pl from TODO file for future work, remove
> commented code and use dyn debug api for remaining printk statements.
> 

Here I'm under the impression that there are 2 things being done:
1- remove commented code
2- replace printk statements

I think it can be splited in 2 commits to facilitate reviewing

> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
> ---
> 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()
> 
> Don't have the hw, just loaded the module inside qemu.
> 
>   drivers/staging/rtl8723bs/core/rtw_mlme_ext.c   |  6 +++---
>   drivers/staging/rtl8723bs/hal/hal_com.c         |  7 +++----
>   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, 16 insertions(+), 44 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");
>   	}
>   
>   	return _SUCCESS;
> diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c
> index 719dd116d807..f766af76da79 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 */
> @@ -910,10 +909,10 @@ 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]);

I think this line exceeds 100 columns, also maybe better not to change 
lines with alignment-only fixes (it depends a bit on the maintainer to 
accept or not, but it is usually good practice not to do it).
But you can make another commit changing this :)

>   		} else {
> -			printk("\n");
> +			pr_debug("\n");
>   		}
>   	}
>   }
> 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 37ebbbf408ec..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;
> @@ -62,7 +60,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 +84,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 +127,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 +138,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/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); */

I'm not sure removing is the best approach or if it's better to 
substitute `DEBUG_ERR` with `pr_err`. You can propose to remove and add 
a comment asking for the community if you should just delete it or to 
substitute with the appropriate `pr_`

>   				/* 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, &param->u.crypt.key[16], 8);
>   						memcpy(psta->dot11tkiprxmickey.skey, &param->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, &param->u.crypt.key[16], 8);
>   				memcpy(psecuritypriv->dot118021XGrprxmickey[param->u.crypt.idx].skey, &param->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, &param->u.crypt.key[16], 8);
>   					memcpy(psta->dot11tkiprxmickey.skey, &param->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, &param->u.crypt.key[16], 8);
>   					memcpy(rxkey, &param->u.crypt.key[24], 8);
> 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;

Nice patch :)
You can send it to the staging mailing list (and whatever else is 
pertinent and lkcamp hehe :3 ).

Thanks,
Gabriela Bittencourt

> Here I'm under the impression that there are 2 things being done:
> 1- remove commented code
> 2- replace printk statements
> I think it can be splited in 2 commits to facilitate reviewing

Yeah, I'll do that, it's gonna be more cleaner. In this case a patch
series is needed or two independent patches?

> I think this line exceeds 100 columns, also maybe better not to change 
> lines with alignment-only fixes

Sorry but in this case it was not "alignment-only fix", there was a replacement 
for pr_xxx() api and my intention was also to fix the original checkpatch warning:

CHECK: Alignment should match open parenthesis
#914: FILE: drivers/staging/rtl8723bs/hal/hal_com.c:914:
+                       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]);

Originally, I haven't seen "line exceed error" around line 914.
Even so we should keep the original style or I didn't understand you previous point?

> I'm not sure removing is the best approach or if it's better to 
> substitute `DEBUG_ERR` with `pr_err`
 I agree, it makes sense, good point.

Tks and regards!

Hi Rodrigo,

On 10/3/24 11:50 PM, Rodrigo Gobbi wrote:
>> Here I'm under the impression that there are 2 things being done:
>> 1- remove commented code
>> 2- replace printk statements
>> I think it can be splited in 2 commits to facilitate reviewing
> 
> Yeah, I'll do that, it's gonna be more cleaner. In this case a patch
> series is needed or two independent patches?
> 

I think a patch series may be nice :)

>> I think this line exceeds 100 columns, also maybe better not to change
>> lines with alignment-only fixes
> 
> Sorry but in this case it was not "alignment-only fix", there was a replacement
> for pr_xxx() api and my intention was also to fix the original checkpatch warning:
> 
> CHECK: Alignment should match open parenthesis
> #914: FILE: drivers/staging/rtl8723bs/hal/hal_com.c:914:
> +                       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]);
> 
> Originally, I haven't seen "line exceed error" around line 914.
> Even so we should keep the original style or I didn't understand you previous point?
> 

Sorry I haven't seen line 915 was a part of the print in the previous line.
Although the line is part of the previous command, in your commit, it's 
not necessary to change line 915 to apply your alterations, so I think 
it's up to you in this case, if you want to modify just add an extra 
break line in a good point to respect the max of 100 columns and it's 
all good :)

>> I'm not sure removing is the best approach or if it's better to
>> substitute `DEBUG_ERR` with `pr_err`
>   I agree, it makes sense, good point.
> 
> Tks and regards!

Regards (sorry for the delay replying)

-- 
Gabriela Bittencourt