drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 5 ++--- drivers/staging/rtl8723bs/include/rtl8723b_hal.h | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-)
Remove the magic numbers in rtl8723b_InitBeaconParameters() as requested
by the outstanding TODO comment. Replace 0x6404 and 0x660F with named
constants defined in rtl8723b_hal.h alongside the existing beacon timing
constants.
Signed-off-by: Jad Keskes <inasj268@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 e794fe3ca..c5da5d0be 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -878,8 +878,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);
/* 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)
@@ -888,7 +887,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 ffd039278..311acfa0f 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.54.0On Wed, Jun 03, 2026 at 03:31:27PM +0100, Jad Keskes wrote: > Remove the magic numbers in rtl8723b_InitBeaconParameters() as requested > by the outstanding TODO comment. Replace 0x6404 and 0x660F with named > constants defined in rtl8723b_hal.h alongside the existing beacon timing > constants. > > Signed-off-by: Jad Keskes <inasj268@gmail.com> Just having the same define but as a goto doesn't add any information or value. This is actually the third attempt to do this. The first two were AI patches. On the first time, I asked how the author came up with the definition and the had the AI generate a halucinated fake spec. On the second patch, the AI found the first fake spec and assumed it was correct. :P Your patch at least doesn't try to pass off any wrong information so that's good. I was able to find this explanation from an out of tree driver. https://gitlab.elettra.eu/intel_socfpga/linux-socfpga/-/blob/socfpga-5.15.60-lts/drivers/net/wireless/realtek/rtw88/rtw8822b.c?ref_type=heads #define WLAN_TBTT_PROHIBIT 0x04 /* unit : 32us */ #define WLAN_TBTT_HOLD_TIME 0x064 /* unit : 32us */ But it's weird that the comment here says ms. I don't know what is correct. That driver doesn't have any explanation for the 0x660f value. regards, dan carpenter
On Thu, Jun 04, 2026 at 05:31:56PM +0300, Dan Carpenter wrote: > On Wed, Jun 03, 2026 at 03:31:27PM +0100, Jad Keskes wrote: > > Remove the magic numbers in rtl8723b_InitBeaconParameters() as requested > > by the outstanding TODO comment. Replace 0x6404 and 0x660F with named > > constants defined in rtl8723b_hal.h alongside the existing beacon timing > > constants. > > > > Signed-off-by: Jad Keskes <inasj268@gmail.com> > > Just having the same define but as a goto doesn't add any information > or value. Ugh... Sorry, I meant to say "just having the same value but as a define doesn't add any information." regards, dan carpenter
Decompose the beacon timing magic numbers in
rtl8723b_InitBeaconParameters() into named constants with bitfield
expressions to document the register layout.
REG_TBTT_PROHIBIT (0x0540): The value 0x6404 splits into hold time
(0x64) in the upper byte and prohibit setup time (0x04) in the lower
byte, in 32us units — matching the layout used in the rtw88 driver
(WLAN_TBTT_HOLD_TIME << 8 | WLAN_TBTT_PROHIBIT).
REG_BCNTCFG (0x0510): The value 0x660F follows the same bitfield
layout as the adjacent EDCA AC parameter registers:
AIFS = bits[7:0] = 0x0F (disables contention before beacon Tx);
CWmin = bits[11:8] = 0x06; CWmax = bits[15:12] = 0x06.
This layout is confirmed by rtl8192cu which writes 0x66FF (test chips)
and 0x660F (normal chips) — only AIFS varies, the CW byte stays 0x66.
Drop the misleading "ms" unit comment on REG_TBTT_PROHIBIT and the
outstanding TODO marker since this was the last item it referenced.
Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 5 ++---
drivers/staging/rtl8723bs/include/rtl8723b_hal.h | 13 +++++++++++++
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index e794fe3ca..4afba1252 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -878,8 +878,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);
/* 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)
@@ -888,7 +887,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, BCNTCFG_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 ffd039278..81105c55a 100644
--- a/drivers/staging/rtl8723bs/include/rtl8723b_hal.h
+++ b/drivers/staging/rtl8723bs/include/rtl8723b_hal.h
@@ -72,6 +72,19 @@ struct rt_firmware_hdr {
#define DRIVER_EARLY_INT_TIME_8723B 0x05
#define BCN_DMA_ATIME_INT_TIME_8723B 0x02
+/* REG_TBTT_PROHIBIT (0x0540) - TBTT prohibit hold/setup in 32us units */
+#define TBTT_PROHIBIT_SETUP_8723B 0x04
+#define TBTT_PROHIBIT_HOLD_8723B 0x64
+#define TBTT_PROHIBIT_TIME_8723B \
+ ((TBTT_PROHIBIT_HOLD_8723B << 8) | TBTT_PROHIBIT_SETUP_8723B)
+
+/* REG_BCNTCFG (0x0510) - beacon AIFS, CWmin, CWmax (EDCA-like layout) */
+#define BCN_AIFS_8723B 0x0F
+#define BCN_CW_MIN_8723B 0x06
+#define BCN_CW_MAX_8723B 0x06
+#define BCNTCFG_8723B \
+ ((BCN_CW_MAX_8723B << 12) | (BCN_CW_MIN_8723B << 8) | BCN_AIFS_8723B)
+
/* for 8723B */
/* TX 32K, RX 16K, Page size 128B for TX, 8B for RX */
#define PAGE_SIZE_TX_8723B 128
--
2.54.0
© 2016 - 2026 Red Hat, Inc.