mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters
@ 2025-07-29 14:12 ashirvadm04
  2025-07-31 16:05 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: ashirvadm04 @ 2025-07-29 14:12 UTC (permalink / raw)
  To: gregkh
  Cc: philipp.g.hortmann, dan.carpenter, sayyad.abid16, linux-staging,
	linux-kernel, Ashirvad Mohanty

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters
  2025-07-29 14:12 [PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters ashirvadm04
@ 2025-07-31 16:05 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2025-07-31 16:05 UTC (permalink / raw)
  To: ashirvadm04
  Cc: gregkh, philipp.g.hortmann, sayyad.abid16, linux-staging, linux-kernel

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



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-07-31 16:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-29 14:12 [PATCH] staging: rtl8723bs: Replace magic numbers in rtl8723b_InitBeaconParameters ashirvadm04
2025-07-31 16:05 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®