staging: rtl8723bs: modernize locking with cleanup.h

PATCH In review Samrudh Nelli 2 participants 4 messages Aug 22 – Aug 24 Staging

Share & tools

Samrudh Nelli

author

2026-08-22 18:24 UTC #0 lore raw

cover staging: rtl8723bs: modernize locking with cleanup.h

This series modernizes the mutex locking in rtw_pwrctrl.c by utilizing the guard() and scoped_guard() macros introduced in cleanup.h.

Patch 1 converts functions where the lock is held until returning. Patch 2 converts functions where the lock is dropped mid-execution.

Samrudh Nelli (2):
staging: rtl8723bs: use guard(mutex) to simplify locking
staging: rtl8723bs: use scoped_guard() for mid-function locking

 drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 154 ++++++++-----------
 1 file changed, 64 insertions(+), 90 deletions(-)

Signature · 1 line

2.55.0

Samrudh Nelli

author

2026-08-22 18:24 UTC #1 lore raw

PATCH 1/2 staging: rtl8723bs: use guard(mutex) to simplify locking

Replace classic mutex_lock() / mutex_unlock() pairs with the guard(mutex) macro in functions where the lock is held for the remainder of the execution scope.

This simplifies the code, eliminates the need for goto-based error handling (e.g., in cpwm_int_hdl()), and ensures that locks are always released gracefully when the function returns, preventing future deadlocks if new exit paths are added.

Functions where the lock is intentionally dropped before the function exits have been left untouched.

Signed-off-by Samrudh Nelli <[email protected]>

---
 drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 33 ++++++--------------
 1 file changed, 10 insertions(+), 23 deletions(-)

drivers/staging/rtl8723bs/core/rtw_pwrctrl.c +10 ~~-23~~

index 448611ad8237..18163a3dbade 100644
@@ -7,6 +7,7 @@
 #include <drv_types.h>
 #include <hal_data.h>
 #include <linux/jiffies.h>
+#include <linux/cleanup.h>
 
 void _ips_enter(struct adapter *padapter)
 {

@@ -37,9 +38,8 @@ void ips_enter(struct adapter *padapter)
 
 	hal_btcoex_IpsNotify(padapter, pwrpriv->ips_mode_req);
 
-	mutex_lock(&pwrpriv->lock);
+	guard(mutex)(&pwrpriv->lock);
 	_ips_enter(padapter);
-	mutex_unlock(&pwrpriv->lock);
 }
 
 int _ips_leave(struct adapter *padapter)

@@ -337,7 +337,7 @@ void rtw_set_ps_mode(struct adapter *padapter, u8 ps_mode,
 		if (ps_mode == PS_MODE_ACTIVE)
 			return;
 
-	mutex_lock(&pwrpriv->lock);
+	guard(mutex)(&pwrpriv->lock);
 
 	/* if (pwrpriv->pwr_mode == PS_MODE_ACTIVE) */
 	if (ps_mode == PS_MODE_ACTIVE) {

@@ -383,8 +383,6 @@ void rtw_set_ps_mode(struct adapter *padapter, u8 ps_mode,
 			rtw_set_rpwm(padapter, pslv);
 		}
 	}
-
-	mutex_unlock(&pwrpriv->lock);
 }
 
 /*

@@ -578,10 +576,10 @@ void cpwm_int_hdl(struct adapter *padapter, struct reportpwrstate_parm *preportp
 
 	pwrpriv = adapter_to_pwrctl(padapter);
 
-	mutex_lock(&pwrpriv->lock);
+	guard(mutex)(&pwrpriv->lock);
 
 	if (pwrpriv->rpwm < PS_STATE_S2)
-		goto exit;
+		return;
 
 	pwrpriv->cpwm = PS_STATE(preportpwrstate->state);
 	pwrpriv->cpwm_tog = preportpwrstate->state & PS_TOGGLE;

@@ -593,9 +591,6 @@ void cpwm_int_hdl(struct adapter *padapter, struct reportpwrstate_parm *preportp
 		if (pwrpriv->alives & XMIT_ALIVE)
 			complete(&padapter->xmitpriv.xmit_comp);
 	}
-
-exit:
-	mutex_unlock(&pwrpriv->lock);
 }
 
 static void cpwm_event_callback(struct work_struct *work)

@@ -744,7 +739,7 @@ void rtw_unregister_task_alive(struct adapter *padapter, u32 task)
 			pslv = PS_STATE_S2;
 	}
 
-	mutex_lock(&pwrctrl->lock);
+	guard(mutex)(&pwrctrl->lock);
 
 	unregister_task_alive(pwrctrl, task);

@@ -754,8 +749,6 @@ void rtw_unregister_task_alive(struct adapter *padapter, u32 task)
 				rtw_set_rpwm(padapter, pslv);
 		}
 	}
-
-	mutex_unlock(&pwrctrl->lock);
 }
 
 /*

@@ -871,7 +864,7 @@ void rtw_unregister_tx_alive(struct adapter *padapter)
 			pslv = PS_STATE_S2;
 	}
 
-	mutex_lock(&pwrctrl->lock);
+	guard(mutex)(&pwrctrl->lock);
 
 	unregister_task_alive(pwrctrl, XMIT_ALIVE);

@@ -880,8 +873,6 @@ void rtw_unregister_tx_alive(struct adapter *padapter)
 			if ((pslv >= PS_STATE_S2) || (pwrctrl->alives == 0))
 				rtw_set_rpwm(padapter, pslv);
 	}
-
-	mutex_unlock(&pwrctrl->lock);
 }
 
 /*

@@ -907,7 +898,7 @@ void rtw_unregister_cmd_alive(struct adapter *padapter)
 			pslv = PS_STATE_S2;
 	}
 
-	mutex_lock(&pwrctrl->lock);
+	guard(mutex)(&pwrctrl->lock);
 
 	unregister_task_alive(pwrctrl, CMD_ALIVE);

@@ -917,8 +908,6 @@ void rtw_unregister_cmd_alive(struct adapter *padapter)
 				rtw_set_rpwm(padapter, pslv);
 		}
 	}
-
-	mutex_unlock(&pwrctrl->lock);
 }
 
 void rtw_init_pwrctrl_priv(struct adapter *padapter)

@@ -1108,9 +1097,8 @@ void rtw_ps_deny(struct adapter *padapter, enum ps_deny_reason reason)
 
 	pwrpriv = adapter_to_pwrctl(padapter);
 
-	mutex_lock(&pwrpriv->lock);
+	guard(mutex)(&pwrpriv->lock);
 	pwrpriv->ps_deny |= BIT(reason);
-	mutex_unlock(&pwrpriv->lock);
 }
 
 /*

@@ -1123,9 +1111,8 @@ void rtw_ps_deny_cancel(struct adapter *padapter, enum ps_deny_reason reason)
 
 	pwrpriv = adapter_to_pwrctl(padapter);
 
-	mutex_lock(&pwrpriv->lock);
+	guard(mutex)(&pwrpriv->lock);
 	pwrpriv->ps_deny &= ~BIT(reason);
-	mutex_unlock(&pwrpriv->lock);
 }
 
 /*

Signature · 1 line

2.55.0

Samrudh Nelli

author

2026-08-22 18:24 UTC #2 lore raw

PATCH 2/2 staging: rtl8723bs: use scoped_guard() for mid-function locking

In reply to Samrudh Nelli

Replace classic mutex_lock() / mutex_unlock() pairs with the scoped_guard(mutex) macro in functions where the lock is intentionally dropped before the function returns.

This isolates the critical sections explicitly within block scopes, eliminating the need for goto-based error handling (e.g., in rpwmtimeout_workitem_callback()) while ensuring the lock duration remains strictly identical to the original implementation.

Signed-off-by Samrudh Nelli <[email protected]>

---
 drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 121 +++++++++----------
 1 file changed, 54 insertions(+), 67 deletions(-)

drivers/staging/rtl8723bs/core/rtw_pwrctrl.c +54 ~~-67~~

index 18163a3dbade..913d77351725 100644
@@ -69,9 +69,9 @@ int ips_leave(struct adapter *padapter)
 	struct pwrctrl_priv *pwrpriv = adapter_to_pwrctl(padapter);
 	int ret;
 
-	mutex_lock(&pwrpriv->lock);
-	ret = _ips_leave(padapter);
-	mutex_unlock(&pwrpriv->lock);
+	scoped_guard(mutex, &pwrpriv->lock) {
+		ret = _ips_leave(padapter);
+	}
 
 	if (ret == _SUCCESS)
 		hal_btcoex_IpsNotify(padapter, IPS_NONE);

@@ -138,9 +138,10 @@ void rtw_ps_processor(struct adapter *padapter)
 	struct pwrctrl_priv *pwrpriv = adapter_to_pwrctl(padapter);
 	u32 ps_deny = 0;
 
-	mutex_lock(&adapter_to_pwrctl(padapter)->lock);
-	ps_deny = rtw_ps_deny_get(padapter);
-	mutex_unlock(&adapter_to_pwrctl(padapter)->lock);
+	scoped_guard(mutex, &adapter_to_pwrctl(padapter)->lock) {
+		ps_deny = rtw_ps_deny_get(padapter);
+	}
+
 	if (ps_deny != 0)
 		goto exit;

@@ -486,11 +487,9 @@ void LeaveAllPowerSaveModeDirect(struct adapter *Adapter)
 		if (pwrpriv->pwr_mode == PS_MODE_ACTIVE)
 			return;
 
-		mutex_lock(&pwrpriv->lock);
-
-		rtw_set_rpwm(Adapter, PS_STATE_S4);
-
-		mutex_unlock(&pwrpriv->lock);
+		scoped_guard(mutex, &pwrpriv->lock) {
+			rtw_set_rpwm(Adapter, PS_STATE_S4);
+		}
 
 		rtw_lps_ctrl_wk_cmd(pri_padapter, LPS_CTRL_LEAVE, 0);
 	} else {

@@ -544,14 +543,12 @@ void LPS_Leave_check(struct adapter *padapter)
 	cond_resched();
 
 	while (1) {
-		mutex_lock(&pwrpriv->lock);
-
-		if (padapter->bSurpriseRemoved ||
-		    !(padapter->hw_init_completed) ||
-		    (pwrpriv->pwr_mode == PS_MODE_ACTIVE))
-			bReady = true;
-
-		mutex_unlock(&pwrpriv->lock);
+		scoped_guard(mutex, &pwrpriv->lock) {
+			if (padapter->bSurpriseRemoved ||
+			    !(padapter->hw_init_completed) ||
+			    (pwrpriv->pwr_mode == PS_MODE_ACTIVE))
+				bReady = true;
+		}
 
 		if (bReady)
 			break;

@@ -614,11 +611,10 @@ static void rpwmtimeout_workitem_callback(struct work_struct *work)
 	dvobj = pwrctl_to_dvobj(pwrpriv);
 	padapter = dvobj->if1;
 
-	mutex_lock(&pwrpriv->lock);
-	if ((pwrpriv->rpwm == pwrpriv->cpwm) || (pwrpriv->cpwm >= PS_STATE_S2))
-		goto exit;
-
-	mutex_unlock(&pwrpriv->lock);
+	scoped_guard(mutex, &pwrpriv->lock) {
+		if ((pwrpriv->rpwm == pwrpriv->cpwm) || (pwrpriv->cpwm >= PS_STATE_S2))
+			return;
+	}
 
 	if (rtw_read8(padapter, 0x100) != 0xEA) {
 		struct reportpwrstate_parm report;

@@ -629,17 +625,14 @@ static void rpwmtimeout_workitem_callback(struct work_struct *work)
 		return;
 	}
 
-	mutex_lock(&pwrpriv->lock);
+	scoped_guard(mutex, &pwrpriv->lock) {
+		if ((pwrpriv->rpwm == pwrpriv->cpwm) || (pwrpriv->cpwm >= PS_STATE_S2))
+			return;
 
-	if ((pwrpriv->rpwm == pwrpriv->cpwm) || (pwrpriv->cpwm >= PS_STATE_S2))
-		goto exit;
-
-	pwrpriv->brpwmtimeout = true;
-	rtw_set_rpwm(padapter, pwrpriv->rpwm);
-	pwrpriv->brpwmtimeout = false;
-
-exit:
-	mutex_unlock(&pwrpriv->lock);
+		pwrpriv->brpwmtimeout = true;
+		rtw_set_rpwm(padapter, pwrpriv->rpwm);
+		pwrpriv->brpwmtimeout = false;
+	}
 }
 
 /*

@@ -691,21 +684,19 @@ s32 rtw_register_task_alive(struct adapter *padapter, u32 task)
 	pwrctrl = adapter_to_pwrctl(padapter);
 	pslv = PS_STATE_S2;
 
-	mutex_lock(&pwrctrl->lock);
+	scoped_guard(mutex, &pwrctrl->lock) {
+		register_task_alive(pwrctrl, task);
 
-	register_task_alive(pwrctrl, task);
-
-	if (pwrctrl->fw_current_in_ps_mode) {
-		if (pwrctrl->cpwm < pslv) {
-			if (pwrctrl->cpwm < PS_STATE_S2)
-				res = _FAIL;
-			if (pwrctrl->rpwm < pslv)
-				rtw_set_rpwm(padapter, pslv);
+		if (pwrctrl->fw_current_in_ps_mode) {
+			if (pwrctrl->cpwm < pslv) {
+				if (pwrctrl->cpwm < PS_STATE_S2)
+					res = _FAIL;
+				if (pwrctrl->rpwm < pslv)
+					rtw_set_rpwm(padapter, pslv);
+			}
 		}
 	}
 
-	mutex_unlock(&pwrctrl->lock);
-
 	if (res == _FAIL)
 		if (pwrctrl->cpwm >= PS_STATE_S2)
 			res = _SUCCESS;

@@ -774,21 +765,19 @@ s32 rtw_register_tx_alive(struct adapter *padapter)
 	pwrctrl = adapter_to_pwrctl(padapter);
 	pslv = PS_STATE_S2;
 
-	mutex_lock(&pwrctrl->lock);
+	scoped_guard(mutex, &pwrctrl->lock) {
+		register_task_alive(pwrctrl, XMIT_ALIVE);
 
-	register_task_alive(pwrctrl, XMIT_ALIVE);
-
-	if (pwrctrl->fw_current_in_ps_mode) {
-		if (pwrctrl->cpwm < pslv) {
-			if (pwrctrl->cpwm < PS_STATE_S2)
-				res = _FAIL;
-			if (pwrctrl->rpwm < pslv)
-				rtw_set_rpwm(padapter, pslv);
+		if (pwrctrl->fw_current_in_ps_mode) {
+			if (pwrctrl->cpwm < pslv) {
+				if (pwrctrl->cpwm < PS_STATE_S2)
+					res = _FAIL;
+				if (pwrctrl->rpwm < pslv)
+					rtw_set_rpwm(padapter, pslv);
+			}
 		}
 	}
 
-	mutex_unlock(&pwrctrl->lock);
-
 	if (res == _FAIL)
 		if (pwrctrl->cpwm >= PS_STATE_S2)
 			res = _SUCCESS;

@@ -819,21 +808,19 @@ s32 rtw_register_cmd_alive(struct adapter *padapter)
 	pwrctrl = adapter_to_pwrctl(padapter);
 	pslv = PS_STATE_S2;
 
-	mutex_lock(&pwrctrl->lock);
+	scoped_guard(mutex, &pwrctrl->lock) {
+		register_task_alive(pwrctrl, CMD_ALIVE);
 
-	register_task_alive(pwrctrl, CMD_ALIVE);
-
-	if (pwrctrl->fw_current_in_ps_mode) {
-		if (pwrctrl->cpwm < pslv) {
-			if (pwrctrl->cpwm < PS_STATE_S2)
-				res = _FAIL;
-			if (pwrctrl->rpwm < pslv)
-				rtw_set_rpwm(padapter, pslv);
+		if (pwrctrl->fw_current_in_ps_mode) {
+			if (pwrctrl->cpwm < pslv) {
+				if (pwrctrl->cpwm < PS_STATE_S2)
+					res = _FAIL;
+				if (pwrctrl->rpwm < pslv)
+					rtw_set_rpwm(padapter, pslv);
+			}
 		}
 	}
 
-	mutex_unlock(&pwrctrl->lock);
-
 	if (res == _FAIL)
 		if (pwrctrl->cpwm >= PS_STATE_S2)
 			res = _SUCCESS;

Signature · 1 line

2.55.0

Dan Carpenter

2026-08-24 06:53 UTC #3 lore raw

In reply to Samrudh Nelli

Quoted 9 lines · Samrudh Nelli

This series modernizes the mutex locking in rtw_pwrctrl.c by utilizing the guard() and scoped_guard() macros introduced in cleanup.h.
Patch 1 converts functions where the lock is held until returning. Patch 2 converts functions where the lock is dropped mid-execution.
Samrudh Nelli (2):
staging: rtl8723bs: use guard(mutex) to simplify locking
staging: rtl8723bs: use scoped_guard() for mid-function locking

We're not doing this sort of change to existing code unless it's part of a bugfix. It's just a lot of churn.

regards,
dan carpenter