Re: [PATCH v2] staging: rtl8723bs: replace magic number in InitBeaconParameters

[Dan Carpenter <error27@gmail.com>]

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@gmail.com>
> ---
>  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@gmail.com/

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
>