Re: [PATCH] staging: rtl8723bs: fixed the trailing whitespaces/commented code

[Luka Gejak]

On March 18, 2026 3:10:05 AM GMT+01:00, Aadarsh Mandal <aadarshmandal9354@xxxxxxxxx> wrote:
>fixed the warnings,reported by checkpatch in
>drivers/staging/rtl8723bs/hal/sdio_halinit.c
>
>It mainly included the leading whitespace errors and commented line
>of codes, deleted those lines rebuild the module and it didn't produced
>any errors. one warning remains, fixing it breaks the logic.
>
>Signed-off-by: Aadarsh Mandal <aadarshmandal9354@xxxxxxxxx>
>---
>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 | 54 ++++++++++----------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>index e32f051ed415..70a5213ae52a 100644
>--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
>@@ -46,7 +46,7 @@ u8 _InitPowerOn_8723BS(struct adapter *padapter)
> 	u16 value16;
> 	u32 value32;
> 	u8 ret;
>-/* 	u8 bMacPwrCtrlOn; */
>+
By deleting this commented-out line and replacing it with a `+` on an empty line, you are introducing extra blank lines into the code. You should just delete the commented line entirely without leaving a gap.
> 
> 
> 	/*  all of these MUST be configured before power on */
>@@ -70,8 +70,6 @@ u8 _InitPowerOn_8723BS(struct adapter *padapter)
> 	rtw_write16(padapter, REG_APS_FSMCO, value16);
> 
> 	/*  Enable CMD53 R/W Operation */
>-/* 	bMacPwrCtrlOn = true; */
>-/* 	rtw_hal_set_hwreg(padapter, HW_VAR_APFM_ON_MAC, &bMacPwrCtrlOn); */
The standard practice in the kernel is to completely delete dead or commented-out code rather than re-indenting it. If the code is no longer used, it is better to just remove it entirely. Re-formatting dead code usually isn't accepted in staging cleanups.

>@@ -1147,6 +1146,7 @@ void SetHwReg8723BS(struct adapter *padapter, u8 variable, u8 *val)
> 	case HW_VAR_SET_REQ_FW_PS:
> 		{
> 			u8 req_fw_ps = 0;
>+
> 			req_fw_ps = rtw_read8(padapter, 0x8f);
> 			req_fw_ps |= 0x10;
> 			rtw_write8(padapter, 0x8f, req_fw_ps);
This is a good fix. Adding the blank line after the variable declaration improves readability and follows the kernel coding style.

Hope this helps with your v2!

Best regards,
Luka Gejak
Hi Aadarsh,

Thanks for submitting this patch, and good luck with your GSoC application!

I am also currently working on cleanups for
the rtl8723bs driver. I took a look at your
patch and noticed a few formatting and logic
issues that you might want to fix in a v2 before the maintainers review it.

First, your subject line does not match the standard format for this subsystem. It should start with "staging: rtl8723bs: " and be more descriptive of the actual changes rather than mentioning the tool used. Something like:
"staging: rtl8723bs: clean up coding style in sdio_halinit.c"

I have left a few specific inline comments below regarding the code changes.