* [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication
@ 2026-02-01 13:27 Bera Yüzlü
2026-02-01 13:38 ` Greg KH
2026-02-02 9:03 ` Dan Carpenter
0 siblings, 2 replies; 3+ messages in thread
From: Bera Yüzlü @ 2026-02-01 13:27 UTC (permalink / raw)
To: gregkh, linux-staging; +Cc: linux-kernel
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication
2026-02-01 13:27 [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication Bera Yüzlü
@ 2026-02-01 13:38 ` Greg KH
2026-02-02 9:03 ` Dan Carpenter
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2026-02-01 13:38 UTC (permalink / raw)
To: Bera Yüzlü; +Cc: linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication
2026-02-01 13:27 [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication Bera Yüzlü
2026-02-01 13:38 ` Greg KH
@ 2026-02-02 9:03 ` Dan Carpenter
1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-02-02 9:03 UTC (permalink / raw)
To: Bera Yüzlü; +Cc: gregkh, linux-staging, linux-kernel
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-02-02 9:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-01 13:27 [PATCH] staging: rtl8723bs: refactor ODM_SetIQCbyRFpath to reduce duplication Bera Yüzlü
2026-02-01 13:38 ` Greg KH
2026-02-02 9:03 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox