[PATCH v3 0/2] staging: rtl8723bs: clean up rtw_joinbss_event_prehandle

[PATCH v3 0/2] staging: rtl8723bs: clean up rtw_joinbss_event_prehandle

[Jose A. Perez de Azpillaga]

This series cleans up the rtw_joinbss_event_prehandle function in the
rtl8723bs staging driver.

The first patch removes dead code guarded by the REJOIN macro, which is
never defined in the kernel. This simplifies the function before the
subsequent refactor.

The second patch refactors the function using guard clauses and early
returns to reduce deep indentation, improving overall readability.

v3:
- Removed the 'retry' variable which became dead after REJOIN
  removal.
- Remoed the 'ignore_joinbss_callback' label and use direct
  returns.
- Preserve the original comment for the join_res <= 0 case.
- Keep the original newline for the netdev_dbg call.
- Moved the 's5' comment after the spin_unlock_bh call to match
  original placement.

v2:
- Split the original patch into a two-patch series.
- Changed the subject line.
- Removed all REJOIN-related dead code.
- Fixed a typo in the ptarget_sta comment.
- Avoided unnecessary changes.

Jose A. Perez de Azpillaga (2):
  staging: rtl8723bs: remove dead REJOIN code
  staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce
    indentation

 drivers/staging/rtl8723bs/core/rtw_mlme.c | 156 ++++++++++------------
 1 file changed, 69 insertions(+), 87 deletions(-)

--
2.53.0

[PATCH v3 1/2] staging: rtl8723bs: remove dead REJOIN code

[Jose A. Perez de Azpillaga]

The REJOIN macro is not defined anywhere in the kernel tree. Remove this
dead code to simplify the function.

Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 28 ++---------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 2adc98c955fd..4bf6d1865391 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1153,10 +1153,8 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 /* if join_res > 0, for (fw_state ==WIFI_ADHOC_STATE), we only check if "ptarget_wlan" exist. */
 /* if join_res > 0, update "cur_network->network" from "pnetwork->network" if (ptarget_wlan != NULL). */
 /*  */
-/* define REJOIN */
 void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
 {
-	static u8 __maybe_unused retry;
 	struct sta_info *ptarget_sta = NULL, *pcur_sta = NULL;
 	struct	sta_priv *pstapriv = &adapter->stapriv;
 	struct	mlme_priv *pmlmepriv = &adapter->mlmepriv;
@@ -1178,7 +1176,6 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
 
 	if (pnetwork->join_res > 0) {
 		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
-		retry = 0;
 		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
 			/* s1. find ptarget_wlan */
 			if (check_fwstate(pmlmepriv, _FW_LINKED)) {
@@ -1250,29 +1247,8 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
 			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 
 	} else {/* if join_res < 0 (join fails), then try again */
-
-		#ifdef REJOIN
-		res = _FAIL;
-		if (retry < 2)
-			res = rtw_select_and_join_from_scanned_queue(pmlmepriv);
-
-		if (res == _SUCCESS) {
-			/* extend time of assoc_timer */
-			_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
-			retry++;
-		} else if (res == 2) {/* there is no need to wait for join */
-			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
-			rtw_indicate_connect(adapter);
-		} else {
-		#endif
-
-			_set_timer(&pmlmepriv->assoc_timer, 1);
-			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
-
-		#ifdef REJOIN
-			retry = 0;
-		}
-		#endif
+		_set_timer(&pmlmepriv->assoc_timer, 1);
+		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 	}
 
 ignore_joinbss_callback:
-- 
2.53.0

[PATCH v3 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation

[Jose A. Perez de Azpillaga]

The rtw_joinbss_event_prehandle function has excessive indentation due
to deeply nested if-statements.

Refactor the function using early returns and guard clauses for the
failure paths. This flattens the code and significantly improves
readability.

Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 136 +++++++++++-----------
 1 file changed, 71 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 4bf6d1865391..3b16d3fadd54 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1174,86 +1174,92 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
 	pmlmepriv->link_detect_info.traffic_transition_count = 0;
 	pmlmepriv->link_detect_info.low_power_transition_count = 0;
 
-	if (pnetwork->join_res > 0) {
-		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
-		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
-			/* s1. find ptarget_wlan */
-			if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-				if (the_same_macaddr) {
-					ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
-				} else {
-					pcur_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
-					if (pcur_wlan)
-						pcur_wlan->fixed = false;
-
-					pcur_sta = rtw_get_stainfo(pstapriv, cur_network->network.mac_address);
-					if (pcur_sta)
-						rtw_free_stainfo(adapter,  pcur_sta);
-
-					ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, pnetwork->network.mac_address);
-					if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
-						if (ptarget_wlan)
-							ptarget_wlan->fixed = true;
-					}
-				}
+	if (pnetwork->join_res == -4) {
+		rtw_reset_securitypriv(adapter);
+		_set_timer(&pmlmepriv->assoc_timer, 1);
 
-			} else {
-				ptarget_wlan = _rtw_find_same_network(&pmlmepriv->scanned_queue, pnetwork);
-				if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
-					if (ptarget_wlan)
-						ptarget_wlan->fixed = true;
-				}
-			}
+		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
+			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 
-			/* s2. update cur_network */
-			if (ptarget_wlan) {
-				rtw_joinbss_update_network(adapter, ptarget_wlan, pnetwork);
-			} else {
-				netdev_dbg(adapter->pnetdev,
-					   "Can't find ptarget_wlan when joinbss_event callback\n");
-				spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
-				goto ignore_joinbss_callback;
-			}
+		spin_unlock_bh(&pmlmepriv->lock);
+		return;
+	}
 
-			/* s3. find ptarget_sta & update ptarget_sta after update cur_network only for station mode */
-			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
-				ptarget_sta = rtw_joinbss_update_stainfo(adapter, pnetwork);
-				if (!ptarget_sta) {
-					spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
-					goto ignore_joinbss_callback;
-				}
-			}
+	if (pnetwork->join_res <= 0) { /* if join_res < 0 (join fails), then try again */
+		_set_timer(&pmlmepriv->assoc_timer, 1);
+		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
+		spin_unlock_bh(&pmlmepriv->lock);
+		return;
+	}
+
+	spin_lock_bh(&pmlmepriv->scanned_queue.lock);
+
+	if (!check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+		spin_unlock_bh(&pmlmepriv->lock);
+		return;
+	}
+
+	/* s1. find ptarget_wlan */
+	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
+		if (the_same_macaddr) {
+			ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
+		} else {
+			pcur_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
+			if (pcur_wlan)
+				pcur_wlan->fixed = false;
 
-			/* s4. indicate connect */
+			pcur_sta = rtw_get_stainfo(pstapriv, cur_network->network.mac_address);
+			if (pcur_sta)
+				rtw_free_stainfo(adapter, pcur_sta);
+
+			ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, pnetwork->network.mac_address);
 			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
-				pmlmepriv->cur_network_scanned = ptarget_wlan;
-				rtw_indicate_connect(adapter);
+				if (ptarget_wlan)
+					ptarget_wlan->fixed = true;
 			}
+		}
+	} else {
+		ptarget_wlan = _rtw_find_same_network(&pmlmepriv->scanned_queue, pnetwork);
+		if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
+			if (ptarget_wlan)
+				ptarget_wlan->fixed = true;
+		}
+	}
 
-			spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+	/* s2. update cur_network */
+	if (!ptarget_wlan) {
+		netdev_dbg(adapter->pnetdev,
+			   "Can't find ptarget_wlan when joinbss_event callback\n");
+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+		spin_unlock_bh(&pmlmepriv->lock);
+		return;
+	}
 
-			spin_unlock_bh(&pmlmepriv->lock);
-			/* s5. Cancel assoc_timer */
-			timer_delete_sync(&pmlmepriv->assoc_timer);
-			spin_lock_bh(&pmlmepriv->lock);
-		} else {
+	rtw_joinbss_update_network(adapter, ptarget_wlan, pnetwork);
+
+	/* s3. find ptarget_sta & update ptarget_sta after update cur_network only for station mode */
+	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
+		ptarget_sta = rtw_joinbss_update_stainfo(adapter, pnetwork);
+		if (!ptarget_sta) {
 			spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+			spin_unlock_bh(&pmlmepriv->lock);
+			return;
 		}
-	} else if (pnetwork->join_res == -4) {
-		rtw_reset_securitypriv(adapter);
-		_set_timer(&pmlmepriv->assoc_timer, 1);
-
-		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
-			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
+	}
 
-	} else {/* if join_res < 0 (join fails), then try again */
-		_set_timer(&pmlmepriv->assoc_timer, 1);
-		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
+	/* s4. indicate connect */
+	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
+		pmlmepriv->cur_network_scanned = ptarget_wlan;
+		rtw_indicate_connect(adapter);
 	}
 
-ignore_joinbss_callback:
+	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
 
 	spin_unlock_bh(&pmlmepriv->lock);
+	/* s5. Cancel assoc_timer */
+	timer_delete_sync(&pmlmepriv->assoc_timer);
+	return;
 }
 
 void rtw_joinbss_event_callback(struct adapter *adapter, u8 *pbuf)
-- 
2.53.0

Re: [PATCH v3 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation

[Dan Carpenter]

On Fri, Mar 20, 2026 at 04:47:17PM +0100, Jose A. Perez de Azpillaga wrote:
> -ignore_joinbss_callback:
> +	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>  
>  	spin_unlock_bh(&pmlmepriv->lock);
> +	/* s5. Cancel assoc_timer */
> +	timer_delete_sync(&pmlmepriv->assoc_timer);
> +	return;
>  }

So close...  But you can't end a void function with a return statement.
It causes a checkpatch warning.

KTODO: Fix checkpatch to catch extra void statements

Checkpatch will catch an unnecessary return statement like this when
people use the -f option, but why can't it catch it for normal patches?
Surely, if there is a line which is just "}" then we could just print
a warning if the previous line is "return;"?

Test this on commits in the git log to find if there is a non-obvious
reason why it only works for -f.

regards,
dan carpenter

Re: [PATCH v3 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation

[Dan Carpenter]

On Sat, Mar 21, 2026 at 10:46:48AM +0300, Dan Carpenter wrote:
> KTODO: Fix checkpatch to catch extra void statements
> 
> Checkpatch will catch an unnecessary return statement like this when
> people use the -f option, but why can't it catch it for normal patches?
> Surely, if there is a line which is just "}" then we could just print
> a warning if the previous line is "return;"?
> 
> Test this on commits in the git log to find if there is a non-obvious
> reason why it only works for -f.

(This was not intended for you Jose, I'm trying to create a TODO list
for kernel janitors to work on so I put KTODO in the email so people
looking for things to do can find it on lore).

regards,
dan carpenter

Re: [PATCH v3 0/2] staging: rtl8723bs: clean up rtw_joinbss_event_prehandle

[Luka Gejak]

On March 20, 2026 4:47:15 PM GMT+01:00, "Jose A. Perez de Azpillaga" <azpijr@gmail.com> wrote:
>This series cleans up the rtw_joinbss_event_prehandle function in the
>rtl8723bs staging driver.
>
>The first patch removes dead code guarded by the REJOIN macro, which is
>never defined in the kernel. This simplifies the function before the
>subsequent refactor.
>
>The second patch refactors the function using guard clauses and early
>returns to reduce deep indentation, improving overall readability.
>
>v3:
>- Removed the 'retry' variable which became dead after REJOIN
>  removal.
>- Remoed the 'ignore_joinbss_callback' label and use direct
>  returns.
>- Preserve the original comment for the join_res <= 0 case.
>- Keep the original newline for the netdev_dbg call.
>- Moved the 's5' comment after the spin_unlock_bh call to match
>  original placement.
>
>v2:
>- Split the original patch into a two-patch series.
>- Changed the subject line.
>- Removed all REJOIN-related dead code.
>- Fixed a typo in the ptarget_sta comment.
>- Avoided unnecessary changes.
>
>Jose A. Perez de Azpillaga (2):
>  staging: rtl8723bs: remove dead REJOIN code
>  staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce
>    indentation
>
> drivers/staging/rtl8723bs/core/rtw_mlme.c | 156 ++++++++++------------
> 1 file changed, 69 insertions(+), 87 deletions(-)
>
>--
>2.53.0
>

Small spelling mistake in cover letter ->Remoed instead of Removed, 
however this is in cover letter so it shouldn't be a problem. 
Other than that, it looks good to me.

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>