staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication

[PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication

Posted by Bera Yüzlü 5 days, 20 hours ago

Refactor ODM_SetIQCbyRFpath to remove duplicated PHY_SetBBReg()
calls and improve readability.

The original implementation duplicated the same PHY_SetBBReg()
calls for both RF paths (S0 / S1) with only the path index
changing. Introduce a small static inline helper, set_iqc(),
to encapsulate a single PHY_SetBBReg() invocation and select
the RF path once based on RFpath. This reduces code duplication,
makes the intent clearer and eases future maintenance.

No functional change intended: register keys/values and
the selection logic remain the same.

Signed-off-by: Bera Yüzlü <b9788213@gmail.com>
---
 .../staging/rtl8723bs/hal/HalPhyRf_8723B.c    | 33 +++++++++----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
index 34692cca33f5..bd535f774852 100644
--- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
+++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
@@ -1074,10 +1074,17 @@ static void _PHY_PathBFillIQKMatrix8723B(
 /*  */
 /*  MP Already declare in odm.c */
 
+/* Helper */
+static inline void set_iqc(struct dm_odm_t *Odm, u32 *table)
+{
+	PHY_SetBBReg(Odm->Adapter, table[KEY], bMaskDWord, table[VAL]);
+}
+
 void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
 {
 
 	struct odm_rf_cal_t *pRFCalibrateInfo = &pDM_Odm->RFCalibrateInfo;
+	u8 path;
 
 	if (
 		(pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL] != 0x0) &&
@@ -1085,23 +1092,15 @@ void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
 		(pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL] != 0x0) &&
 		(pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL] != 0x0)
 	) {
-		if (RFpath) { /* S1: RFpath = 0, S0:RFpath = 1 */
-			/* S0 TX IQC */
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][VAL]);
-			/* S0 RX IQC */
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][VAL]);
-		} else {
-			/* S1 TX IQC */
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][VAL]);
-			/* S1 RX IQC */
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL]);
-			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][VAL]);
-		}
+		path = RFpath ? PATH_S0 : PATH_S1; /* S1: RFpath = 0, S0:RFpath = 1 */
+
+		/* TX IQC */
+		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC94]);
+		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC80]);
+		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC4C]);
+		/* RX IQC */
+		set_iqc(pDM_Odm, pRFCalibrateInfo->RxIQC_8723B[path][IDX_0xC14]);
+		set_iqc(pDM_Odm, pRFCalibrateInfo->RxIQC_8723B[path][IDX_0xCA0]);
 	}
 }
 
-- 
2.43.0

Re: [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication

Posted by Dan Carpenter 5 days ago

On Sun, Feb 01, 2026 at 04:27:33PM +0300, Bera Yüzlü wrote:
> diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> index 34692cca33f5..bd535f774852 100644
> --- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> +++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> @@ -1074,10 +1074,17 @@ static void _PHY_PathBFillIQKMatrix8723B(
>  /*  */
>  /*  MP Already declare in odm.c */
>  
> +/* Helper */
   ^^^^^^^^^^^^

Is this an AI patch?  I really am starting to despise AI.

> +static inline void set_iqc(struct dm_odm_t *Odm, u32 *table)
> +{
> +	PHY_SetBBReg(Odm->Adapter, table[KEY], bMaskDWord, table[VAL]);
> +}
> +
>  void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
>  {
>  
>  	struct odm_rf_cal_t *pRFCalibrateInfo = &pDM_Odm->RFCalibrateInfo;
> +	u8 path;
>  
>  	if (
>  		(pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL] != 0x0) &&
> @@ -1085,23 +1092,15 @@ void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
>  		(pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL] != 0x0) &&
>  		(pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL] != 0x0)
>  	) {
> -		if (RFpath) { /* S1: RFpath = 0, S0:RFpath = 1 */
> -			/* S0 TX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][VAL]);
> -			/* S0 RX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][VAL]);
> -		} else {
> -			/* S1 TX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][VAL]);
> -			/* S1 RX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][VAL]);
> -		}
> +		path = RFpath ? PATH_S0 : PATH_S1; /* S1: RFpath = 0, S0:RFpath = 1 */
                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This comment came from the original code but it's junk.  I guess the
comment is trying to explain that S0 is one and s1 is zero?  Ugh...
Why?  Anyway, just delete the comment because it's useless.

regards,
dan carpenter

> +
> +		/* TX IQC */
> +		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC94]);
> +		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC80]);
> +		set_iqc(pDM_Odm, pRFCalibrateInfo->TxIQC_8723B[path][IDX_0xC4C]);
> +		/* RX IQC */
> +		set_iqc(pDM_Odm, pRFCalibrateInfo->RxIQC_8723B[path][IDX_0xC14]);
> +		set_iqc(pDM_Odm, pRFCalibrateInfo->RxIQC_8723B[path][IDX_0xCA0]);
>  	}
>  }
>  
> -- 
> 2.43.0
>

Re: [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication

Posted by Greg KH 5 days, 20 hours ago

On Sun, Feb 01, 2026 at 04:27:33PM +0300, Bera Yüzlü wrote:
> Refactor ODM_SetIQCbyRFpath to remove duplicated PHY_SetBBReg()
> calls and improve readability.
> 
> The original implementation duplicated the same PHY_SetBBReg()
> calls for both RF paths (S0 / S1) with only the path index
> changing. Introduce a small static inline helper, set_iqc(),
> to encapsulate a single PHY_SetBBReg() invocation and select
> the RF path once based on RFpath. This reduces code duplication,
> makes the intent clearer and eases future maintenance.
> 
> No functional change intended: register keys/values and
> the selection logic remain the same.
> 
> Signed-off-by: Bera Yüzlü <b9788213@gmail.com>
> ---
>  .../staging/rtl8723bs/hal/HalPhyRf_8723B.c    | 33 +++++++++----------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> index 34692cca33f5..bd535f774852 100644
> --- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> +++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
> @@ -1074,10 +1074,17 @@ static void _PHY_PathBFillIQKMatrix8723B(
>  /*  */
>  /*  MP Already declare in odm.c */
>  
> +/* Helper */

Function comment is odd, and not really needed.

> +static inline void set_iqc(struct dm_odm_t *Odm, u32 *table)

Don't use inline unless you are forced to.  the compiler should get it
right without it.


> +{
> +	PHY_SetBBReg(Odm->Adapter, table[KEY], bMaskDWord, table[VAL]);

This is just a wrapper #define as well, right?  Why not spell it out to
call the real function instead?

> +}
> +
>  void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
>  {
>  
>  	struct odm_rf_cal_t *pRFCalibrateInfo = &pDM_Odm->RFCalibrateInfo;
> +	u8 path;
>  
>  	if (
>  		(pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL] != 0x0) &&
> @@ -1085,23 +1092,15 @@ void ODM_SetIQCbyRFpath(struct dm_odm_t *pDM_Odm, u32 RFpath)
>  		(pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL] != 0x0) &&
>  		(pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL] != 0x0)
>  	) {
> -		if (RFpath) { /* S1: RFpath = 0, S0:RFpath = 1 */
> -			/* S0 TX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC94][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC80][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S0][IDX_0xC4C][VAL]);
> -			/* S0 RX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xC14][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S0][IDX_0xCA0][VAL]);
> -		} else {
> -			/* S1 TX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC94][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC80][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][KEY], bMaskDWord, pRFCalibrateInfo->TxIQC_8723B[PATH_S1][IDX_0xC4C][VAL]);
> -			/* S1 RX IQC */
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xC14][VAL]);
> -			PHY_SetBBReg(pDM_Odm->Adapter, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][KEY], bMaskDWord, pRFCalibrateInfo->RxIQC_8723B[PATH_S1][IDX_0xCA0][VAL]);
> -		}
> +		path = RFpath ? PATH_S0 : PATH_S1; /* S1: RFpath = 0, S0:RFpath = 1 */

Please do not use ?: statements unless you HAVE to.  Use real if()
statements instead, as that's easier and simpler for people to read and
understand.  The output should be the same, yet you create more readable
code as we write for people first, compilers second.

thanks,

greg k-h