mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 1/2] staging: rtl8723bs: remove dead REJOIN code
       [not found] <20260319231344.569418-1-azpijr@gmail.com>
@ 2026-03-19 23:13 ` Jose A. Perez de Azpillaga
  2026-03-19 23:13 ` [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation Jose A. Perez de Azpillaga
  1 sibling, 0 replies; 4+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-03-19 23:13 UTC (permalink / raw)
  To: linux-staging
  Cc: Greg Kroah-Hartman, Michael Straube, Hans de Goede,
	Khushal Chitturi, Dan Carpenter, Vivek BalachandharTN,
	Ethan Tidmore, Luka Gejak, Artur Stupa, Zhuoheng Li, Nino Zhang,
	linux-kernel

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 | 26 ++---------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 2adc98c955fd..805427353aa6 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1153,7 +1153,6 @@ 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;
@@ -1250,29 +1249,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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation
       [not found] <20260319231344.569418-1-azpijr@gmail.com>
  2026-03-19 23:13 ` [PATCH v2 1/2] staging: rtl8723bs: remove dead REJOIN code Jose A. Perez de Azpillaga
@ 2026-03-19 23:13 ` Jose A. Perez de Azpillaga
  2026-03-20  8:05   ` Luka Gejak
  2026-03-20  9:40   ` Dan Carpenter
  1 sibling, 2 replies; 4+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-03-19 23:13 UTC (permalink / raw)
  To: linux-staging
  Cc: Greg Kroah-Hartman, Michael Straube, Hans de Goede,
	Khushal Chitturi, Dan Carpenter, Ethan Tidmore,
	Vivek BalachandharTN, Luka Gejak, Artur Stupa, Zhuoheng Li,
	Nino Zhang, linux-kernel

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 | 134 +++++++++++-----------
 1 file changed, 69 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 805427353aa6..2a192c1dd038 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1175,84 +1175,88 @@ 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);
-		retry = 0;
-		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;
-			}
+		goto ignore_joinbss_callback;
+	}
 
-			/* 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) {
+		_set_timer(&pmlmepriv->assoc_timer, 1);
+		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
+		goto ignore_joinbss_callback;
+	}
+
+	spin_lock_bh(&pmlmepriv->scanned_queue.lock);
+	retry = 0;
+
+	if (!check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+		goto ignore_joinbss_callback;
+	}
+
+	/* 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);
 
-			/* s4. indicate connect */
+			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);
+		goto ignore_joinbss_callback;
+	}
 
-			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);
+			goto ignore_joinbss_callback;
 		}
-	} 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);
 	}
 
+	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
+
+	/* s5. Cancel assoc_timer */
+	spin_unlock_bh(&pmlmepriv->lock);
+	timer_delete_sync(&pmlmepriv->assoc_timer);
+	return;
+
 ignore_joinbss_callback:
 
 	spin_unlock_bh(&pmlmepriv->lock);
-- 
2.53.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation
  2026-03-19 23:13 ` [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation Jose A. Perez de Azpillaga
@ 2026-03-20  8:05   ` Luka Gejak
  2026-03-20  9:40   ` Dan Carpenter
  1 sibling, 0 replies; 4+ messages in thread
From: Luka Gejak @ 2026-03-20  8:05 UTC (permalink / raw)
  To: Jose A. Perez de Azpillaga, linux-staging
  Cc: Greg Kroah-Hartman, Michael Straube, Hans de Goede,
	Khushal Chitturi, Dan Carpenter, Ethan Tidmore,
	Vivek BalachandharTN, Artur Stupa, Zhuoheng Li, Nino Zhang,
	linux-kernel

On March 20, 2026 12:13:35 AM GMT+01:00, "Jose A. Perez de Azpillaga" <azpijr@gmail.com> wrote:
>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 | 134 +++++++++++-----------
> 1 file changed, 69 insertions(+), 65 deletions(-)
>
>diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
>index 805427353aa6..2a192c1dd038 100644
>--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
>+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
>@@ -1175,84 +1175,88 @@ 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);
>-		retry = 0;
>-		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;
>-			}
>+		goto ignore_joinbss_callback;
>+	}
> 
>-			/* 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) {
>+		_set_timer(&pmlmepriv->assoc_timer, 1);
>+		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
>+		goto ignore_joinbss_callback;
>+	}
>+
>+	spin_lock_bh(&pmlmepriv->scanned_queue.lock);
>+	retry = 0;
>+
>+	if (!check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
>+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+		goto ignore_joinbss_callback;
>+	}
>+
>+	/* 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);
> 
>-			/* s4. indicate connect */
>+			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);
>+		goto ignore_joinbss_callback;
>+	}
> 
>-			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);
>+			goto ignore_joinbss_callback;
> 		}
>-	} 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);
> 	}
> 
>+	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+
>+	/* s5. Cancel assoc_timer */
>+	spin_unlock_bh(&pmlmepriv->lock);
>+	timer_delete_sync(&pmlmepriv->assoc_timer);
>+	return;
>+
> ignore_joinbss_callback:
> 
> 	spin_unlock_bh(&pmlmepriv->lock);

One minor note: Since the REJOIN logic is now gone, the static u8 retry
 variable at line 1150 is now only assigned to 0 and never used. It 
doesn't break the build thanks to __maybe_unused, but it should be 
removed in a v3 of the series. Also include everyone from To and Cc for
 cover letter email as well.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation
  2026-03-19 23:13 ` [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation Jose A. Perez de Azpillaga
  2026-03-20  8:05   ` Luka Gejak
@ 2026-03-20  9:40   ` Dan Carpenter
  1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2026-03-20  9:40 UTC (permalink / raw)
  To: Jose A. Perez de Azpillaga
  Cc: linux-staging, Greg Kroah-Hartman, Michael Straube,
	Hans de Goede, Khushal Chitturi, Ethan Tidmore,
	Vivek BalachandharTN, Luka Gejak, Artur Stupa, Zhuoheng Li,
	Nino Zhang, linux-kernel

On Fri, Mar 20, 2026 at 12:13:35AM +0100, Jose A. Perez de Azpillaga wrote:
> 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 | 134 +++++++++++-----------
>  1 file changed, 69 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 805427353aa6..2a192c1dd038 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -1175,84 +1175,88 @@ 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);
> -		retry = 0;
> -		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;
> -			}
> +		goto ignore_joinbss_callback;
> +	}
>  
> -			/* 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) {

This preserves how the existing code works, but originally there was
a comment here which said that join_res could only be < 0.  Emotionally,
I would prefer if zero were treated as success.  Can join_res actually
be zero?

Update: I looked and join_res can be zero.  It comes from:

	res = pmlmeinfo->aid = (int)(le16_to_cpu(*(__le16 *)(pframe + WLAN_HDR_A3_LEN + 4))&0x3fff);

in the OnAssocRsp() function.  I guess preserve the <= 0, but also
preserve the comment.

> +		_set_timer(&pmlmepriv->assoc_timer, 1);
> +		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
> +		goto ignore_joinbss_callback;
> +	}
> +
> +	spin_lock_bh(&pmlmepriv->scanned_queue.lock);
> +	retry = 0;
> +
> +	if (!check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
> +		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
> +		goto ignore_joinbss_callback;
> +	}
> +
> +	/* 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);
>  
> -			/* s4. indicate connect */
> +			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");

This line is too long.  In the original code there was a newline after
the comma so it would have been 84 characters.  Try to make as few
unrelated changes as possible.


> +		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
> +		goto ignore_joinbss_callback;
> +	}
>  
> -			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);
> +			goto ignore_joinbss_callback;
>  		}
> -	} 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);
>  	}
>  
> +	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
> +
> +	/* s5. Cancel assoc_timer */

Put this comment after the spinlock (the way it was in the original
code).

> +	spin_unlock_bh(&pmlmepriv->lock);
> +	timer_delete_sync(&pmlmepriv->assoc_timer);
> +	return;

Originally, we took a lock and then fell through to the unlock after
the ignore_joinbss_callback label.  Which is obviously bogus so your
return is better.  I also think it might be even better yet to get
rid of the ignore_joinbss_callback label entirely.  A simple

	spin_unlock_bh(&pmlmepriv->lock);
	return;

Is only one more line of code, but it's much more readable.

regards,
dan carpenter

> +
>  ignore_joinbss_callback:
>  
>  	spin_unlock_bh(&pmlmepriv->lock);


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-20  9:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20260319231344.569418-1-azpijr@gmail.com>
2026-03-19 23:13 ` [PATCH v2 1/2] staging: rtl8723bs: remove dead REJOIN code Jose A. Perez de Azpillaga
2026-03-19 23:13 ` [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation Jose A. Perez de Azpillaga
2026-03-20  8:05   ` Luka Gejak
2026-03-20  9:40   ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome