[PATCH v9 0/3] staging: rtl8723bs: fix several style issues in hal/sdio_halinit.c

[PATCH v9 0/3] staging: rtl8723bs: fix several style issues in hal/sdio_halinit.c

[artur.ugnivenko]

From: Artur Ugnivenko <artur.ugnivenko@gmx.de>

Fix the following checkpatch warnings in hal/sdio_halinit.c:
- lines longer than 100 chars
- inconsistent brackets around else statements
And refactor queue priority initialization in hal/sdio_halinit.c
to fix a "lines ending in parentheses" warning.

Changes in v9:
- don't declare variables in the middle of functions in 3/3
- remove unnecessary whitespace in 3/3
Changes in v8:
- Remove the struct from v7, use an array instead.
- Remove other "lines ending in parentheses" fixes to keep
  3/3 small
Changes in v7:
- Remove whitespace patch.
- Add transmit_queues struct to improve style of
  _InitNormalChipRegPriority
- Add changelog to individual patches.
Changes in v6: Make the patch series apply to gregkh/staging-testing.
Changes in v5: Added the entire changelog to the cover letter.
Changes in v4: Fixed the long line checkpatch warning in patch 2.
Changes in v3: Split the patch into multiple patches.
Changes in v2: Make the patch apply to gregkh/staging-testing.

Artur Ugnivenko (3):
  staging: rtl8723bs: shorten long lines in hal/sdio_halinit.c
  staging: rtl8723bs: fix inconsistent braces in hal/sdio_halinit.c
  staging: rtl8723bs: refactor queue priority initialization

 drivers/staging/rtl8723bs/hal/sdio_halinit.c  | 163 +++++++++++-------
 drivers/staging/rtl8723bs/include/drv_types.h |  10 ++
 2 files changed, 108 insertions(+), 65 deletions(-)

-- 
2.54.0

[PATCH v9 1/3] staging: rtl8723bs: shorten long lines in hal/sdio_halinit.c

[artur.ugnivenko]

From: Artur Ugnivenko <artur.ugnivenko@gmx.de>

Fix checkpatch warnings on lines above 100 chars in length by splitting
long function calls and long comments into multiple lines.

Signed-off-by: Artur Ugnivenko <artur.ugnivenko@gmx.de>
---
Changes in v9: No changes in this patch.
Changes in v8: No changes in this patch.
Changes in v7: Add changelog.
Changes in v6: Make the patch apply to gregkh/staging-testing.
Changes in v5: No changes in this patch.
Changes in v4: No changes in this patch.
Changes in v3: Split the patch into multiple patches.
Changes in v2: Make the patch apply to gregkh/staging-testing.

 drivers/staging/rtl8723bs/hal/sdio_halinit.c | 52 ++++++++++++++++----
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
index ed9b8fb07fec..43f9c45967f4 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
@@ -27,7 +27,11 @@ static u8 CardEnable(struct adapter *padapter)
 		/*  unlock ISO/CLK/Power control register */
 		rtw_write8(padapter, REG_RSV_CTRL, 0x0);
 
-		ret = HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, rtl8723B_card_enable_flow);
+		ret = HalPwrSeqCmdParsing(padapter,
+					  PWR_CUT_ALL_MSK,
+					  PWR_FAB_ALL_MSK,
+					  PWR_INTF_SDIO_MSK,
+					  rtl8723B_card_enable_flow);
 		if (ret == _SUCCESS) {
 			bMacPwrCtrlOn = true;
 			rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
@@ -104,7 +108,11 @@ u8 _InitPowerOn_8723BS(struct adapter *padapter)
 }
 
 /* Tx Page FIFO threshold */
-static void _init_available_page_threshold(struct adapter *padapter, u8 numHQ, u8 numNQ, u8 numLQ, u8 numPubQ)
+static void _init_available_page_threshold(struct adapter *padapter,
+					   u8 numHQ,
+					   u8 numNQ,
+					   u8 numLQ,
+					   u8 numPubQ)
 {
 	u16 HQ_threshold, NQ_threshold, LQ_threshold;
 
@@ -781,7 +789,11 @@ u32 rtl8723bs_hal_init(struct adapter *padapter)
 
 			restore_iqk_rst = pwrpriv->bips_processing;
 			b2Ant = pHalData->EEPROMBluetoothAntNum == Ant_x2;
-			PHY_IQCalibrate_8723B(padapter, false, restore_iqk_rst, b2Ant, pHalData->ant_path);
+			PHY_IQCalibrate_8723B(padapter,
+					      false,
+					      restore_iqk_rst,
+					      b2Ant,
+					      pHalData->ant_path);
 			pHalData->odmpriv.RFCalibrateInfo.bIQKInitialized = true;
 
 			hal_btcoex_IQKNotify(padapter, false);
@@ -812,7 +824,11 @@ static void CardDisableRTL8723BSdio(struct adapter *padapter)
 	u8 bMacPwrCtrlOn;
 
 	/*  Run LPS WL RFOFF flow */
-	HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, rtl8723B_enter_lps_flow);
+	HalPwrSeqCmdParsing(padapter,
+			    PWR_CUT_ALL_MSK,
+			    PWR_FAB_ALL_MSK,
+			    PWR_INTF_SDIO_MSK,
+			    rtl8723B_enter_lps_flow);
 
 	/*	==== Reset digital sequence   ====== */
 
@@ -841,7 +857,11 @@ static void CardDisableRTL8723BSdio(struct adapter *padapter)
 
 	bMacPwrCtrlOn = false;	/*  Disable CMD53 R/W */
 	rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
-	HalPwrSeqCmdParsing(padapter, PWR_CUT_ALL_MSK, PWR_FAB_ALL_MSK, PWR_INTF_SDIO_MSK, rtl8723B_card_disable_flow);
+	HalPwrSeqCmdParsing(padapter,
+			    PWR_CUT_ALL_MSK,
+			    PWR_FAB_ALL_MSK,
+			    PWR_INTF_SDIO_MSK,
+			    rtl8723B_card_disable_flow);
 }
 
 u32 rtl8723bs_hal_deinit(struct adapter *padapter)
@@ -853,7 +873,10 @@ u32 rtl8723bs_hal_deinit(struct adapter *padapter)
 				u8 val8 = 0;
 
 				rtl8723b_set_FwPwrModeInIPS_cmd(padapter, 0x3);
-				/* poll 0x1cc to make sure H2C command already finished by FW; MAC_0x1cc = 0 means H2C done by FW. */
+				/*
+				 * poll 0x1cc to make sure H2C command already finished by FW;
+				 * MAC_0x1cc = 0 means H2C done by FW.
+				 */
 				do {
 					val8 = rtw_read8(padapter, REG_HMETFR);
 					cnt++;
@@ -862,10 +885,13 @@ u32 rtl8723bs_hal_deinit(struct adapter *padapter)
 				/* H2C done, enter 32k */
 				if (val8 == 0) {
 					/* set rpwm to enter 32k */
-					val8 = rtw_read8(padapter, SDIO_LOCAL_BASE | SDIO_REG_HRPWM1);
+					val8 = rtw_read8(padapter,
+							 SDIO_LOCAL_BASE | SDIO_REG_HRPWM1);
 					val8 += 0x80;
 					val8 |= BIT(0);
-					rtw_write8(padapter, SDIO_LOCAL_BASE | SDIO_REG_HRPWM1, val8);
+					rtw_write8(padapter,
+						   SDIO_LOCAL_BASE | SDIO_REG_HRPWM1,
+						   val8);
 					adapter_to_pwrctl(padapter)->tog = (val8 + 0x80) & 0x80;
 					cnt = val8 = 0;
 					do {
@@ -1063,8 +1089,14 @@ static s32 _ReadAdapterInfo8723BS(struct adapter *padapter)
 	_ReadPROMContent(padapter);
 
 	if (!padapter->hw_init_completed) {
-		rtw_write8(padapter, 0x67, 0x00); /*  for BT, Switch Ant control to BT */
-		CardDisableRTL8723BSdio(padapter);/* for the power consumption issue,  wifi ko module is loaded during booting, but wifi GUI is off */
+		/*  for BT, Switch Ant control to BT */
+		rtw_write8(padapter, 0x67, 0x00);
+
+		/*
+		 * for the power consumption issue, wifi ko module is loaded during booting,
+		 * but wifi GUI is off
+		 */
+		CardDisableRTL8723BSdio(padapter);
 	}
 
 	return _SUCCESS;
-- 
2.54.0

[PATCH v9 2/3] staging: rtl8723bs: fix inconsistent braces in hal/sdio_halinit.c

[artur.ugnivenko]

From: Artur Ugnivenko <artur.ugnivenko@gmx.de>

Fix checkpatch warnings on inconsistent braces around if/else statements
in hal/sdio_halinit.c

Signed-off-by: Artur Ugnivenko <artur.ugnivenko@gmx.de>
---
Changes in v9: No changes in this patch.
Changes in v8: No changes in this patch.
Changes in v7: Add changelog.
Changes in v6: Make the patch apply to gregkh/staging-testing.
Changes in v5: No changes in this patch.
Changes in v4: Fix long line checkpatch warning.
Changes in v3: Split the patch into multiple patches.
Changes in v2: Make patch apply to gregkh/staging-testing.

 drivers/staging/rtl8723bs/hal/sdio_halinit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
index 43f9c45967f4..6f2aea984b30 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
@@ -36,8 +36,9 @@ static u8 CardEnable(struct adapter *padapter)
 			bMacPwrCtrlOn = true;
 			rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
 		}
-	} else
+	} else {
 		ret = _SUCCESS;
+	}
 
 	return ret;
 }
@@ -1011,8 +1012,9 @@ static void Hal_EfuseParseBoardType_8723BS(
 		pHalData->BoardType = (hwinfo[EEPROM_RF_BOARD_OPTION_8723B] & 0xE0) >> 5;
 		if (pHalData->BoardType == 0xFF)
 			pHalData->BoardType = (EEPROM_DEFAULT_BOARD_OPTION & 0xE0) >> 5;
-	} else
+	} else {
 		pHalData->BoardType = 0;
+	}
 }
 
 static void _ReadEfuseInfo8723BS(struct adapter *padapter)
-- 
2.54.0

[PATCH v9 3/3] staging: rtl8723bs: refactor queue priority initialization

[artur.ugnivenko]

From: Artur Ugnivenko <artur.ugnivenko@gmx.de>

Pack individual priority values into an enum-indexed array
to simplify the initialization code and prevent passing around
six loose values as function parameters.

Signed-off-by: Artur Ugnivenko <artur.ugnivenko@gmx.de>
---
Changes in v9:
- don't declare variables in the middle of functions
- remove unnecessary whitespace
Changes in v8:
- Remove the struct from v7, use an enum-indexed array instead
- Remove other lines ending in parentheses fixes to keep the
  commit small
Changes in v7:
- Add changelog.
- Add transmit_queues struct in include/drv_types.h to improve
  style of _InitNormalChipRegPriority
Changes in v6: Make the patch apply to gregkh/staging-testing.
Changes in v5: No changes in this patch.
Changes in v4: No changes in this patch.
Changes in v3: Split the patch into multiple patches.
Changes in v2: Make patch apply to gregkh/staging-testing.

 drivers/staging/rtl8723bs/hal/sdio_halinit.c  | 105 +++++++++---------
 drivers/staging/rtl8723bs/include/drv_types.h |  10 ++
 2 files changed, 62 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
index 6f2aea984b30..1246c5fdfc32 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
@@ -188,25 +188,18 @@ static void _InitTxBufferBoundary(struct adapter *padapter)
 	rtw_write8(padapter, REG_TDECTRL + 1, txpktbuf_bndy);
 }
 
-static void _InitNormalChipRegPriority(
-	struct adapter *Adapter,
-	u16 beQ,
-	u16 bkQ,
-	u16 viQ,
-	u16 voQ,
-	u16 mgtQ,
-	u16 hiQ
-)
+static void _InitNormalChipRegPriority(struct adapter *Adapter,
+				       u16 queues[TX_Q_MAX])
 {
 	u16 value16 = (rtw_read16(Adapter, REG_TRXDMA_CTRL) & 0x7);
 
 	value16 |=
-		_TXDMA_BEQ_MAP(beQ)  |
-		_TXDMA_BKQ_MAP(bkQ)  |
-		_TXDMA_VIQ_MAP(viQ)  |
-		_TXDMA_VOQ_MAP(voQ)  |
-		_TXDMA_MGQ_MAP(mgtQ) |
-		_TXDMA_HIQ_MAP(hiQ);
+		_TXDMA_BEQ_MAP(queues[TX_Q_BE])  |
+		_TXDMA_BKQ_MAP(queues[TX_Q_BK])  |
+		_TXDMA_VIQ_MAP(queues[TX_Q_VI])  |
+		_TXDMA_VOQ_MAP(queues[TX_Q_VO])  |
+		_TXDMA_MGQ_MAP(queues[TX_Q_MGT]) |
+		_TXDMA_HIQ_MAP(queues[TX_Q_HI]);
 
 	rtw_write16(Adapter, REG_TRXDMA_CTRL, value16);
 }
@@ -214,6 +207,7 @@ static void _InitNormalChipRegPriority(
 static void _InitNormalChipOneOutEpPriority(struct adapter *Adapter)
 {
 	struct hal_com_data *pHalData = GET_HAL_DATA(Adapter);
+	u16 queues[TX_Q_MAX];
 
 	u16 value = 0;
 
@@ -231,17 +225,19 @@ static void _InitNormalChipOneOutEpPriority(struct adapter *Adapter)
 		break;
 	}
 
-	_InitNormalChipRegPriority(
-		Adapter, value, value, value, value, value, value
-	);
+	for (int i = 0; i < TX_Q_MAX; i++)
+		queues[i] = value;
 
+	_InitNormalChipRegPriority(Adapter, queues);
 }
 
 static void _InitNormalChipTwoOutEpPriority(struct adapter *Adapter)
 {
 	struct hal_com_data *pHalData = GET_HAL_DATA(Adapter);
 	struct registry_priv *pregistrypriv = &Adapter->registrypriv;
-	u16 beQ, bkQ, viQ, voQ, mgtQ, hiQ;
+
+	u16 typical_queues[TX_Q_MAX];
+	u16 wmm_queues[TX_Q_MAX];
 
 	u16 valueHi = 0;
 	u16 valueLow = 0;
@@ -263,50 +259,53 @@ static void _InitNormalChipTwoOutEpPriority(struct adapter *Adapter)
 		break;
 	}
 
+	typical_queues[TX_Q_BE] = valueLow;
+	typical_queues[TX_Q_BK] = valueLow;
+	typical_queues[TX_Q_VI] = valueHi;
+	typical_queues[TX_Q_VO] = valueHi;
+	typical_queues[TX_Q_MGT] = valueHi;
+	typical_queues[TX_Q_HI] = valueHi;
+
+	wmm_queues[TX_Q_BE] = valueLow;
+	wmm_queues[TX_Q_BK] = valueHi;
+	wmm_queues[TX_Q_VI] = valueHi;
+	wmm_queues[TX_Q_VO] = valueLow;
+	wmm_queues[TX_Q_MGT] = valueHi;
+	wmm_queues[TX_Q_HI] = valueHi;
+
 	if (!pregistrypriv->wifi_spec) {
-		beQ = valueLow;
-		bkQ = valueLow;
-		viQ = valueHi;
-		voQ = valueHi;
-		mgtQ = valueHi;
-		hiQ = valueHi;
+		_InitNormalChipRegPriority(Adapter, typical_queues);
 	} else {
 		/* for WMM , CONFIG_OUT_EP_WIFI_MODE */
-		beQ = valueLow;
-		bkQ = valueHi;
-		viQ = valueHi;
-		voQ = valueLow;
-		mgtQ = valueHi;
-		hiQ = valueHi;
+		_InitNormalChipRegPriority(Adapter, wmm_queues);
 	}
-
-	_InitNormalChipRegPriority(Adapter, beQ, bkQ, viQ, voQ, mgtQ, hiQ);
-
 }
 
 static void _InitNormalChipThreeOutEpPriority(struct adapter *padapter)
 {
 	struct registry_priv *pregistrypriv = &padapter->registrypriv;
-	u16 beQ, bkQ, viQ, voQ, mgtQ, hiQ;
 
-	if (!pregistrypriv->wifi_spec) {
-		/*  typical setting */
-		beQ = QUEUE_LOW;
-		bkQ = QUEUE_LOW;
-		viQ = QUEUE_NORMAL;
-		voQ = QUEUE_HIGH;
-		mgtQ = QUEUE_HIGH;
-		hiQ = QUEUE_HIGH;
-	} else {
-		/*  for WMM */
-		beQ = QUEUE_LOW;
-		bkQ = QUEUE_NORMAL;
-		viQ = QUEUE_NORMAL;
-		voQ = QUEUE_HIGH;
-		mgtQ = QUEUE_HIGH;
-		hiQ = QUEUE_HIGH;
-	}
-	_InitNormalChipRegPriority(padapter, beQ, bkQ, viQ, voQ, mgtQ, hiQ);
+	u16 typical_queues[TX_Q_MAX] = {
+		[TX_Q_BE] = QUEUE_LOW,
+		[TX_Q_BK] = QUEUE_LOW,
+		[TX_Q_VI] = QUEUE_NORMAL,
+		[TX_Q_VO] = QUEUE_HIGH,
+		[TX_Q_MGT] = QUEUE_HIGH,
+		[TX_Q_HI] = QUEUE_HIGH,
+	};
+	u16 wmm_queues[TX_Q_MAX] = {
+		[TX_Q_BE] = QUEUE_LOW,
+		[TX_Q_BK] = QUEUE_NORMAL,
+		[TX_Q_VI] = QUEUE_NORMAL,
+		[TX_Q_VO] = QUEUE_HIGH,
+		[TX_Q_MGT] = QUEUE_HIGH,
+		[TX_Q_HI] = QUEUE_HIGH,
+	};
+
+	if (!pregistrypriv->wifi_spec)
+		_InitNormalChipRegPriority(padapter, typical_queues);
+	else
+		_InitNormalChipRegPriority(padapter, wmm_queues);
 }
 
 static void _InitQueuePriority(struct adapter *Adapter)
diff --git a/drivers/staging/rtl8723bs/include/drv_types.h b/drivers/staging/rtl8723bs/include/drv_types.h
index 552d0c5fa47f..b2c43f53571a 100644
--- a/drivers/staging/rtl8723bs/include/drv_types.h
+++ b/drivers/staging/rtl8723bs/include/drv_types.h
@@ -373,6 +373,16 @@ struct adapter {
 	unsigned char     in_cta_test;
 };
 
+enum transmit_queues {
+	TX_Q_BE = 0,
+	TX_Q_BK,
+	TX_Q_VI,
+	TX_Q_VO,
+	TX_Q_MGT,
+	TX_Q_HI,
+	TX_Q_MAX,
+};
+
 #define adapter_to_dvobj(adapter) (adapter->dvobj)
 #define adapter_to_pwrctl(adapter) (dvobj_to_pwrctl(adapter->dvobj))
 #define adapter_wdev_data(adapter) (&((adapter)->wdev_data))
-- 
2.54.0

Re: [PATCH v9 3/3] staging: rtl8723bs: refactor queue priority initialization

[Greg KH]

On Thu, May 28, 2026 at 08:07:57PM +0200, artur.ugnivenko@gmx.de wrote:
> From: Artur Ugnivenko <artur.ugnivenko@gmx.de>
> 
> Pack individual priority values into an enum-indexed array
> to simplify the initialization code and prevent passing around
> six loose values as function parameters.
> 
> Signed-off-by: Artur Ugnivenko <artur.ugnivenko@gmx.de>
> ---
> Changes in v9:
> - don't declare variables in the middle of functions
> - remove unnecessary whitespace
> Changes in v8:
> - Remove the struct from v7, use an enum-indexed array instead
> - Remove other lines ending in parentheses fixes to keep the
>   commit small
> Changes in v7:
> - Add changelog.
> - Add transmit_queues struct in include/drv_types.h to improve
>   style of _InitNormalChipRegPriority
> Changes in v6: Make the patch apply to gregkh/staging-testing.
> Changes in v5: No changes in this patch.
> Changes in v4: No changes in this patch.
> Changes in v3: Split the patch into multiple patches.
> Changes in v2: Make patch apply to gregkh/staging-testing.
> 
>  drivers/staging/rtl8723bs/hal/sdio_halinit.c  | 105 +++++++++---------
>  drivers/staging/rtl8723bs/include/drv_types.h |  10 ++
>  2 files changed, 62 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> index 6f2aea984b30..1246c5fdfc32 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> @@ -188,25 +188,18 @@ static void _InitTxBufferBoundary(struct adapter *padapter)
>  	rtw_write8(padapter, REG_TDECTRL + 1, txpktbuf_bndy);
>  }
>  
> -static void _InitNormalChipRegPriority(
> -	struct adapter *Adapter,
> -	u16 beQ,
> -	u16 bkQ,
> -	u16 viQ,
> -	u16 voQ,
> -	u16 mgtQ,
> -	u16 hiQ
> -)
> +static void _InitNormalChipRegPriority(struct adapter *Adapter,
> +				       u16 queues[TX_Q_MAX])

Wow, that's an odd thing to be passing by value in a function, do you
really mean to do this?  Yes, the compiler will let you, but it's not a
good idea to do that at all in the kernel as it messes with the stack,
and all sorts of "odd" things can happen.

Please always pass structures by reference.

Also, you are "open coding" the structure size here, which is not a good
idea as you have to manually keep everything in sync.  What's wrong with
that original structure you used a few versions before?

thanks,

greg k-h

Re: [PATCH v9 0/3] staging: rtl8723bs: fix several style issues in hal/sdio_halinit.c

[Dan Carpenter]

Looks okay to me now.

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter