[PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters

ashirvadm04@gmail.com posted 1 patch 2 months, 1 week ago
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 5 ++---
drivers/staging/rtl8723bs/include/rtl8723b_hal.h  | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
[PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters
Posted by ashirvadm04@gmail.com 2 months, 1 week ago
From: Ashirvad Mohanty <ashirvadm04@gmail.com>

Replace magic numbers 0x6404 and 0x660F in rtl8723b_InitBeaconParameters
with macros TBTT_PROHIBIT_TIME_8723B and BCN_AIFS_CFG_8723B, respectively,
defined in rtl8723b_hal.h. This addresses the TODO to remove magic numbers,
improving code readability and maintainability per Linux kernel coding style.

Signed-off-by: Ashirvad Mohanty <ashirvadm04@gmail.com>
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 5 ++---
 drivers/staging/rtl8723bs/include/rtl8723b_hal.h  | 2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 893cab0532ed..d6c75854f27a 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -1197,8 +1197,7 @@ void rtl8723b_InitBeaconParameters(struct adapter *padapter)
 
 	rtw_write16(padapter, REG_BCN_CTRL, val16);
 
-	/*  TODO: Remove these magic number */
-	rtw_write16(padapter, REG_TBTT_PROHIBIT, 0x6404);/*  ms */
+	rtw_write16(padapter, REG_TBTT_PROHIBIT, TBTT_PROHIBIT_TIME_8723B); /*  ms */
 	/*  Firmware will control REG_DRVERLYINT when power saving is enable, */
 	/*  so don't set this register on STA mode. */
 	if (check_fwstate(&padapter->mlmepriv, WIFI_STATION_STATE) == false)
@@ -1207,7 +1206,7 @@ void rtl8723b_InitBeaconParameters(struct adapter *padapter)
 
 	/*  Suggested by designer timchen. Change beacon AIFS to the largest number */
 	/*  because test chip does not contension before sending beacon. by tynli. 2009.11.03 */
-	rtw_write16(padapter, REG_BCNTCFG, 0x660F);
+	rtw_write16(padapter, REG_BCNTCFG, BCN_AIFS_CFG_8723B);
 
 	pHalData->RegBcnCtrlVal = rtw_read8(padapter, REG_BCN_CTRL);
 	pHalData->RegTxPause = rtw_read8(padapter, REG_TXPAUSE);
diff --git a/drivers/staging/rtl8723bs/include/rtl8723b_hal.h b/drivers/staging/rtl8723bs/include/rtl8723b_hal.h
index a4a14474c35d..5b4e303e20eb 100644
--- a/drivers/staging/rtl8723bs/include/rtl8723b_hal.h
+++ b/drivers/staging/rtl8723bs/include/rtl8723b_hal.h
@@ -71,6 +71,8 @@ struct rt_firmware_hdr {
 
 #define DRIVER_EARLY_INT_TIME_8723B  0x05
 #define BCN_DMA_ATIME_INT_TIME_8723B 0x02
+#define TBTT_PROHIBIT_TIME_8723B     0x6404
+#define BCN_AIFS_CFG_8723B           0x660F
 
 /* for 8723B */
 /* TX 32K, RX 16K, Page size 128B for TX, 8B for RX */
-- 
2.43.0
Re: [PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters
Posted by Dan Carpenter 2 months ago
On Tue, Jul 29, 2025 at 07:42:10PM +0530, ashirvadm04@gmail.com wrote:
> From: Ashirvad Mohanty <ashirvadm04@gmail.com>
> 
> Replace magic numbers 0x6404 and 0x660F in rtl8723b_InitBeaconParameters
> with macros TBTT_PROHIBIT_TIME_8723B and BCN_AIFS_CFG_8723B, respectively,
> defined in rtl8723b_hal.h.

I don't really think these names add any information.  The number is
related to TIME but no one really knows how it works.  Someone posted
a theory about it, but it turned out that was just AI hallucination.
The spec they quoted was real but the sub section was fake.

The 8723B is the name of the driver, but without the "s" for some
reason.  It's just words but with no meaning.  It's a map to nowhere.
It's a waste of time.  I would say that there is also an element where
these names are a bit misleading because the BCN_AIFS_CFG_8723B is
really specific and the name suggests it's more of a generally config
button.

Just leave it as-is.  I would suggest that you could delete the TODO,
but it's good practice to just learn to accept that we can't fix
everything.

regards,
dan carpenter