Re: [PATCH] staging: rtl8723bs: replace beacon timing magic numbers with named constants

Re: [PATCH] staging: rtl8723bs: replace beacon timing magic numbers with named constants

[Jad Keskes]

Ping. Any chance this can be picked up?

Re: [PATCH] staging: rtl8723bs: replace beacon timing magic numbers with named constants

[Greg Kroah-Hartman]

On Tue, Jun 16, 2026 at 05:29:04PM +0100, Jad Keskes wrote:
> Ping. Any chance this can be picked up?
> 

I have no context here :(

Re: [PATCH] staging: rtl8723bs: replace magic numbers with named constants

[Dan Carpenter]

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.

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

[PATCH] staging: rtl8723bs: replace beacon timing magic numbers with named constants

[Jad Keskes]

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