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

[Greg KH <gregkh@linuxfoundation.org>]

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