Re: [PATCH v2] staging: r8188eu: convert rtw_pwr_wakeup to correct error code semantics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/26/22 00:07, Phillip Potter wrote:
Convert the rtw_pwr_wakeup function to use 0 on success and an appropriate
error code on error. For the first failure block where ips_leave is
invoked, use -ENOMEM as this is the main cause of failure here anyway.
For the second failure block, use -EBUSY, as it seems the most
appropriate.

Finally, within the functions rtw_wx_set_mode, rtw_wx_set_wap,
rtw_wx_set_scan and rtw_wx_set_essid, pass the error code on from
rtw_pwr_wakeup as appropriate now that it is converted.

This gets the driver closer to removal of the non-standard _SUCCESS and
_FAIL definitions, which are inverted compared to the standard in-kernel
error code mechanism.

Signed-off-by: Phillip Potter <phil@xxxxxxxxxxxxxxxx>
---

Changes from V1: Act on feedback from Dan Carpenter:
* Try to use more appropriate error codes than -EPERM.
* Revert the places where existing -1 was converted as they are out of
   scope.
* Preserve error codes in places where calling function already uses
   proper negative semantics, so that they can be passed through to the
   caller.

---
  drivers/staging/r8188eu/core/rtw_p2p.c       |  4 ++--
  drivers/staging/r8188eu/core/rtw_pwrctrl.c   | 10 ++++----
  drivers/staging/r8188eu/os_dep/ioctl_linux.c | 24 +++++++-------------
  3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
index c306aafa183b..bd654d4ff8b4 100644
--- a/drivers/staging/r8188eu/core/rtw_p2p.c
+++ b/drivers/staging/r8188eu/core/rtw_p2p.c
@@ -1888,7 +1888,7 @@ int rtw_p2p_enable(struct adapter *padapter, enum P2P_ROLE role)
if (role == P2P_ROLE_DEVICE || role == P2P_ROLE_CLIENT || role == P2P_ROLE_GO) {
  		/* leave IPS/Autosuspend */
-		if (rtw_pwr_wakeup(padapter) == _FAIL) {
+		if (rtw_pwr_wakeup(padapter)) {
  			ret = _FAIL;
  			goto exit;
  		}
@@ -1902,7 +1902,7 @@ int rtw_p2p_enable(struct adapter *padapter, enum P2P_ROLE role)
  		init_wifidirect_info(padapter, role);
} else if (role == P2P_ROLE_DISABLE) {
-		if (rtw_pwr_wakeup(padapter) == _FAIL) {
+		if (rtw_pwr_wakeup(padapter)) {
  			ret = _FAIL;
  			goto exit;
  		}
diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index cf9020a73933..8b1c50668dfe 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -381,24 +381,24 @@ int rtw_pwr_wakeup(struct adapter *padapter)
  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
  	unsigned long timeout = jiffies + msecs_to_jiffies(3000);
  	unsigned long deny_time;
-	int ret = _SUCCESS;
+	int ret = 0;
while (pwrpriv->ps_processing && time_before(jiffies, timeout))
  		msleep(10);
/* I think this should be check in IPS, LPS, autosuspend functions... */
  	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		ret = _SUCCESS;
+		ret = 0;
  		goto exit;
  	}
if (pwrpriv->rf_pwrstate == rf_off && ips_leave(padapter) == _FAIL) {
-		ret = _FAIL;
+		ret = -ENOMEM;
  		goto exit;
  	}
if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
-		ret = _FAIL;
+		ret = -EBUSY;
  		goto exit;
  	}
@@ -439,7 +439,7 @@ int rtw_pm_set_ips(struct adapter *padapter, u8 mode)
  		return 0;
  	} else if (mode == IPS_NONE) {
  		rtw_ips_mode_req(pwrctrlpriv, mode);
-		if ((padapter->bSurpriseRemoved == 0) && (rtw_pwr_wakeup(padapter) == _FAIL))
+		if ((padapter->bSurpriseRemoved == 0) && rtw_pwr_wakeup(padapter))
  			return -EFAULT;
  	} else {
  		return -EINVAL;
diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 930bb4aea435..7f91dac2e41b 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -687,12 +687,9 @@ static int rtw_wx_set_mode(struct net_device *dev, struct iw_request_info *a,
  	enum ndis_802_11_network_infra networkType;
  	int ret = 0;
-
-
-	if (_FAIL == rtw_pwr_wakeup(padapter)) {
-		ret = -EPERM;
+	ret = rtw_pwr_wakeup(padapter);
+	if (ret)
  		goto exit;
-	}
if (!padapter->hw_init_completed) {
  		ret = -EPERM;
@@ -931,12 +928,9 @@ static int rtw_wx_set_wap(struct net_device *dev,
  	struct	wlan_network	*pnetwork = NULL;
  	enum ndis_802_11_auth_mode	authmode;
-
-
-	if (_FAIL == rtw_pwr_wakeup(padapter)) {
-		ret = -1;
+	ret = rtw_pwr_wakeup(padapter);
+	if (ret)
  		goto exit;
-	}
if (!padapter->bup) {
  		ret = -1;
@@ -1049,10 +1043,9 @@ static int rtw_wx_set_scan(struct net_device *dev, struct iw_request_info *a,
  	struct ndis_802_11_ssid ssid[RTW_SSID_SCAN_AMOUNT];
  	struct wifidirect_info *pwdinfo = &padapter->wdinfo;
- if (_FAIL == rtw_pwr_wakeup(padapter)) {
-		ret = -1;
+	ret = rtw_pwr_wakeup(padapter);
+	if (ret)
  		goto exit;
-	}
if (padapter->bDriverStopped) {
  		ret = -1;
@@ -1252,10 +1245,9 @@ static int rtw_wx_set_essid(struct net_device *dev,
uint ret = 0, len; - if (_FAIL == rtw_pwr_wakeup(padapter)) {
-		ret = -1;
+	ret = rtw_pwr_wakeup(padapter);
+	if (ret)
  		goto exit;
-	}
if (!padapter->bup) {
  		ret = -1;

Tested-by: Philipp Hortmann <philipp.g.hortmann@xxxxxxxxx> # Edimax N150




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux