Re: [PATCH v2] staging: rtl8723bs: replace magic number in InitBeaconParameters
From: Dan Carpenter
Date: Sat Jul 11 2026 - 02:41:20 EST
On Sat, Jul 11, 2026 at 11:52:49AM +0530, Anantha Krishnan wrote:
> Per the bitfield comment in hal_com_reg.h, REG_TBTT_PROHIBIT packs
> a "setup" time in bits 3:0 and a "hold" time in bits 19:8. The
> rtw_write16() call here only writes the low 16 bits of that
> register, covering the full setup field and the low byte of the
> hold field.
>
> Replace the magic number 0x6404 with two named constants
> representing these fields.
>
> No functional change.
>
> Bit-field positions are documented in the comment above
> REG_TBTT_PROHIBIT in drivers/staging/rtl8723bs/include/hal_com_reg.h.
>
> Signed-off-by: Anantha Krishnan <ananthanr.off@xxxxxxxxx>
> ---
> drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index bcaf63b2893c..699e24f52392 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -11,6 +11,13 @@
> #include <rtl8723b_hal.h>
> #include "hal_com_h2c.h"
>
> +/*
> + * Bitfield layout per the comment above REG_TBTT_PROHIBIT in
> + * drivers/staging/rtl8723bs/include/hal_com_reg.h.
> + */
> +#define TBTT_PROHIBIT_SETUP 0x04 /* bits 3:0: setup time, unit 32us */
> +#define TBTT_PROHIBIT_HOLD_LOW 0x64 /* bits 15:8: low byte of 12-bit hold time, unit 32us */
Jad already sent a version of this.
https://lore.kernel.org/all/20260708150930.1813224-1-inasj268@xxxxxxxxx/
The "_LOW", I guess, means "low byte" which is misleading because
it suggests that this is a low byte and that a high byte exists...
The 15 should really be 19.
> +
> static void _FWDownloadEnable(struct adapter *padapter, bool enable)
> {
> u8 tmp, count = 0;
> @@ -861,8 +868,8 @@ 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_HOLD_LOW << 8) | TBTT_PROHIBIT_SETUP);/* ms */
The comments still say "ms" when really it's in units of 32us.
regards,
dan carpenter
> /* 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))
> --
> 2.43.0
>