linux-kernel - Re: [PATCH] staging: rtl8723bs: refactor ODM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2026020146-kilogram-undercut-b849@gregkh>
Date: Sun, 1 Feb 2026 14:38:30 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Bera Yüzlü <b9788213@...il.com>
Cc: linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to
 reduce duplication

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@...il.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