Re: [PATCH] [PATCH] staging: rtl8723bs: clean up coding style in sdio_halinit.c V2

[Luka Gejak <luka.gejak@linux.dev>]

Hi Aadarsh,

Thanks for sending the v2. It is great that you are working on these cleanups!

However, it looks like something went wrong with your local git tree when generating this patch. Instead of removing the dead code, this patch actually adds 161 lines to the file. It re-adds the commented-out code (e.g., `+/* sdio_AggSettingTxUpdate(padapter); */`) and inserts several large functions that shouldn't be part of a simple style cleanup. 

You might have accidentally reverted your changes, diffed against the wrong branch, or merged something incorrectly. I recommend doing a fresh "git fetch" from the staging tree, creating a new branch, and doing the cleanups again.

Also, for your v3, here are two quick formatting tips for the patch email:

1. Subject Line: Your subject currently says "[PATCH] [PATCH] ... V2". The standard kernel format is "[PATCH v2] staging: rtl8723bs: clean up...". You can generate this automatically by using the "-v" flag:  "git format-patch -v3 ..."

2. Changelog Placement: Your version history ("update since v1...") is currently in the main commit body. Anything above the "---" line becomes the permanent commit message in the Linux kernel history. 

For a single patch, you should place your changelog notes below the "---" line so they are dropped when the maintainer applies the patch. Alternatively, if you decide to break your cleanups into multiple patches (a patch series), you should generate a cover letter ("git format-patch -v3 --cover-letter ...") and put your version history in the cover letter instead.

Always double-check your generated ".patch" file with a text editor or "cat" before sending to make sure the diff only contains the exact lines you intended to change.

Also, for future versions of this series or any further discussions, please use my developer address luka.gejak@linux.dev in the CC list instead of my Gmail account. This helps me keep my kernel work organized and consistent with my own patches.

Looking forward to v3!

Best regards,
Luka Gejak

On March 18, 2026 10:26:39 AM GMT+01:00, Aadarsh Mandal <aadarshmandal9354@gmail.com> wrote:
>update since v1:
>- fixed the changes mentioned by the Luka Gejak
>- removed the commented code and space
>
>Signed-off-by: Aadarsh Mandal <aadarshmandal9354@gmail.com>
>---
>Note:
>* This patch is part of the GSoC2026 application process for device tree bindings conversions
>* https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings
>
> drivers/staging/rtl8723bs/hal/sdio_halinit.c | 163 ++++++++++++++++++-
> 1 file changed, 161 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>index 1026554bcff0..063a0737933e 100644
>--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>@@ -6,7 +6,6 @@
>  ******************************************************************************/
> #include <drv_types.h>
> #include <rtl8723b_hal.h>
>-
> #include "hal_com_h2c.h"
> /*
>  * Description:
>@@ -69,8 +68,8 @@ u8 _InitPowerOn_8723BS(struct adapter *padapter)
> 	rtw_write16(padapter, REG_APS_FSMCO, value16);
> 
> 	/*  Enable CMD53 R/W Operation */
>-
> 	rtw_write8(padapter, REG_CR, 0x00);
>+
> 	/*  Enable MAC DMA/WMAC/SCHEDULE/SEC block */
> 	value16 = rtw_read16(padapter, REG_CR);
> 	value16 |= (
>@@ -483,6 +479,158 @@ static void _initSdioAggregationSetting(struct adapter *padapter)
> 	struct hal_com_data	*pHalData = GET_HAL_DATA(padapter);
> 
> 	/*  Tx aggregation setting */
>+/*	sdio_AggSettingTxUpdate(padapter); */
>+
>+	/*  Rx aggregation setting */
>+	HalRxAggr8723BSdio(padapter);
>+
>+	sdio_AggSettingRxUpdate(padapter);
>+
>+	/*  201/12/10 MH Add for USB agg mode dynamic switch. */
>+	pHalData->UsbRxHighSpeedMode = false;
>+}
>+
>+static void _InitOperationMode(struct adapter *padapter)
>+{
>+	struct mlme_ext_priv *pmlmeext;
>+	u8 regBwOpMode = 0;
>+
>+	pmlmeext = &padapter->mlmeextpriv;
>+
>+	/* 1 This part need to modified according to the rate set we filtered!! */
>+	/*  */
>+	/*  Set RRSR, RATR, and REG_BWOPMODE registers */
>+	/*  */
>+	switch (pmlmeext->cur_wireless_mode) {
>+	case WIRELESS_MODE_B:
>+		regBwOpMode = BW_OPMODE_20MHZ;
>+		break;
>+	case WIRELESS_MODE_G:
>+		regBwOpMode = BW_OPMODE_20MHZ;
>+		break;
>+	case WIRELESS_MODE_AUTO:
>+		regBwOpMode = BW_OPMODE_20MHZ;
>+		break;
>+	case WIRELESS_MODE_N_24G:
>+		/*  It support CCK rate by default. */
>+		/*  CCK rate will be filtered out only when associated AP does not support it. */
>+		regBwOpMode = BW_OPMODE_20MHZ;
>+		break;
>+
>+	default: /* for MacOSX compiler warning. */
>+		break;
>+	}
>+
>+	rtw_write8(padapter, REG_BWOPMODE, regBwOpMode);
>+}
>+
>+static void _InitInterrupt(struct adapter *padapter)
>+{
>+	/*  HISR - turn all off */
>+	rtw_write32(padapter, REG_HISR, 0);
>+
>+	/*  HIMR - turn all off */
>+	rtw_write32(padapter, REG_HIMR, 0);
>+
>+	/*  */
>+	/*  Initialize and enable SDIO Host Interrupt. */
>+	/*  */
>+	InitInterrupt8723BSdio(padapter);
>+
>+	/*  */
>+	/*  Initialize system Host Interrupt. */
>+	/*  */
>+	InitSysInterrupt8723BSdio(padapter);
>+}
>+
>+static void _InitRFType(struct adapter *padapter)
>+{
>+	struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
>+
>+	pHalData->rf_chip	= RF_6052;
>+}
>+
>+static void _RfPowerSave(struct adapter *padapter)
>+{
>+/* YJ, TODO */
>+}
>+
>+/*  */
>+/*  2010/08/09 MH Add for power down check. */
>+/*  */
>+static bool HalDetectPwrDownMode(struct adapter *Adapter)
>+{
>+	u8 tmpvalue;
>+	struct hal_com_data *pHalData = GET_HAL_DATA(Adapter);
>+	struct pwrctrl_priv *pwrctrlpriv = adapter_to_pwrctl(Adapter);
>+
>+	EFUSE_ShadowRead(Adapter, 1, 0x7B/*EEPROM_RF_OPT3_92C*/, (u32 *)&tmpvalue);
>+
>+	/*  2010/08/25 MH INF priority > PDN Efuse value. */
>+	if (tmpvalue & BIT4 && pwrctrlpriv->reg_pdnmode)
>+		pHalData->pwrdown = true;
>+	else
>+		pHalData->pwrdown = false;
>+
>+	return pHalData->pwrdown;
>+}	/*  HalDetectPwrDownMode */
>+
>+u32 rtl8723bs_hal_init(struct adapter *padapter)
>+{
>+	s32 ret;
>+	struct hal_com_data *pHalData;
>+	struct pwrctrl_priv *pwrctrlpriv;
>+	u32 NavUpper = WiFiNavUpperUs;
>+	u8 val;
>+
>+	pHalData = GET_HAL_DATA(padapter);
>+	pwrctrlpriv = adapter_to_pwrctl(padapter);
>+
>+	if (
>+		adapter_to_pwrctl(padapter)->bips_processing == true &&
>+		adapter_to_pwrctl(padapter)->pre_ips_type == 0
>+	) {
>+		unsigned long start_time;
>+		u8 cpwm_orig, cpwm_now;
>+		u8 val8, bMacPwrCtrlOn = true;
>+
>+		/* for polling cpwm */
>+		cpwm_orig = 0;
>+		rtw_hal_get_hwreg(padapter, HW_VAR_CPWM, &cpwm_orig);
>+
>+		/* set rpwm */
>+		val8 = rtw_read8(padapter, SDIO_LOCAL_BASE | SDIO_REG_HRPWM1);
>+		val8 &= 0x80;
>+		val8 += 0x80;
>+		val8 |= BIT(6);
>+		rtw_write8(padapter, SDIO_LOCAL_BASE | SDIO_REG_HRPWM1, val8);
>+		adapter_to_pwrctl(padapter)->tog = (val8 + 0x80) & 0x80;
>+
>+		/* do polling cpwm */
>+		start_time = jiffies;
>+		do {
>+			mdelay(1);
>+
>+			rtw_hal_get_hwreg(padapter, HW_VAR_CPWM, &cpwm_now);
>+			if ((cpwm_orig ^ cpwm_now) & 0x80)
>+				break;
>+
>+			if (jiffies_to_msecs(jiffies - start_time) > 100)
>+				break;
>+
>+		} while (1);
>+
>+		rtl8723b_set_FwPwrModeInIPS_cmd(padapter, 0);
>+
>+		rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn);
>+
>+		hal_btcoex_InitHwConfig(padapter, false);
>+
>+		return _SUCCESS;
>+	}
>+
>+	/*  Disable Interrupt first. */
>+/*	rtw_hal_disable_interrupt(padapter); */
> 
> 	ret = _InitPowerOn_8723BS(padapter);
> 	if (ret == _FAIL)
>@@ -588,6 +739,8 @@ static void _initSdioAggregationSetting(struct adapter *padapter)
> 
> 	/*  Record original value for template. This is arough data, we can only use the data */
> 	/*  for power adjust. The value can not be adjustde according to different power!!! */
>+/*	pHalData->OriginalCckTxPwrIdx = pHalData->CurrentCckTxPwrIdx; */
>+/*	pHalData->OriginalOfdm24GTxPwrIdx = pHalData->CurrentOfdm24GTxPwrIdx; */
> 
> 	rtl8723b_InitAntenna_Selection(padapter);
> 
>@@ -630,6 +783,7 @@ static void _initSdioAggregationSetting(struct adapter *padapter)
> 	/* ack for xmit mgmt frames. */
> 	rtw_write32(padapter, REG_FWHW_TXQ_CTRL, rtw_read32(padapter, REG_FWHW_TXQ_CTRL) | BIT(12));
> 
>+/*	pHalData->PreRpwmVal = SdioLocalCmd52Read1Byte(padapter, SDIO_REG_HRPWM1) & 0x80; */
> 
> 	{
> 		pwrctrlpriv->rf_pwrstate = rf_on;
>@@ -918,6 +1072,8 @@ static void _ReadPROMContent(struct adapter *padapter)
> 	pEEPROM->EepromOrEfuse = (eeValue & BOOT_FROM_EEPROM) ? true : false;
> 	pEEPROM->bautoload_fail_flag = (eeValue & EEPROM_EN) ? false : true;
> 
>+/*	pHalData->EEType = IS_BOOT_FROM_EEPROM(Adapter) ? EEPROM_93C46 : EEPROM_BOOT_EFUSE; */
>+
> 	_ReadEfuseInfo8723BS(padapter);
> }
>