[PATCH 0/8] staging: r8188eu: clean up rtw_pwr_wakeup

[PATCH 0/8] staging: r8188eu: clean up rtw_pwr_wakeup

[Martin Kaiser]

Clean up the rtw_pwr_wakeup function. Use in-kernel routines for
handling timeouts.

Martin Kaiser (8):
  staging: r8188eu: simplify delay conditions in rtw_pwr_wakeup
  staging: r8188eu: don't set _SUCCESS again
  staging: r8188eu: make return values consistent
  staging: r8188eu: simplify the ps_processing check
  staging: r8188eu: summarize two if statements
  staging: r8188eu: use kernel functions for timeout handling
  staging: r8188eu: clean up the code to set ips_deny_time
  staging: r8188eu: remove the bInSuspend loop

 drivers/staging/r8188eu/core/rtw_pwrctrl.c    | 48 ++++++-------------
 .../staging/r8188eu/include/osdep_service.h   |  1 -
 drivers/staging/r8188eu/include/rtw_pwrctrl.h |  2 +-
 .../staging/r8188eu/os_dep/osdep_service.c    |  5 --
 4 files changed, 15 insertions(+), 41 deletions(-)

-- 
2.30.2

[PATCH 1/8] staging: r8188eu: simplify delay conditions in rtw_pwr_wakeup

[Martin Kaiser]

Simplify the conditions for a loop in rtw_pwr_wakeup that waits while
the system is suspended.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index efdc7de49d49..7c1e79808087 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -378,12 +378,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	}
 
 	/* System suspend is not allowed to wakeup */
-	if (pwrpriv->bInSuspend) {
-		while (pwrpriv->bInSuspend &&
-		       (rtw_get_passing_time_ms(start) <= 3000 ||
-		       (rtw_get_passing_time_ms(start) <= 500)))
-				msleep(10);
-	}
+	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
+		msleep(10);
 
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
 	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-- 
2.30.2

[PATCH 2/8] staging: r8188eu: don't set _SUCCESS again

[Martin Kaiser]

ret is initialized to _SUCCESS, there's no need to set it again.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 7c1e79808087..c43759500ef9 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -382,10 +382,9 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 		msleep(10);
 
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		ret = _SUCCESS;
+	if (check_fwstate(pmlmepriv, _FW_LINKED))
 		goto exit;
-	}
+
 	if (rf_off == pwrpriv->rf_pwrstate) {
 		if (_FAIL ==  ips_leave(padapter)) {
 			ret = _FAIL;
-- 
2.30.2

[PATCH 3/8] staging: r8188eu: make return values consistent

[Martin Kaiser]

rtw_pwr_wakeup should return _SUCCESS or _FAIL.

Replace false with _FAIL in one place and reformat the if-statement.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index c43759500ef9..a7471468e2e2 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -392,10 +392,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 		}
 	}
 
-	/* TODO: the following checking need to be merged... */
-	if (padapter->bDriverStopped || !padapter->bup ||
-	    !padapter->hw_init_completed) {
-		ret = false;
+	if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
+		ret = _FAIL;
 		goto exit;
 	}
 
-- 
2.30.2

[PATCH 4/8] staging: r8188eu: simplify the ps_processing check

[Martin Kaiser]

It's sufficient to check pwrpriv->ps_processing as part of the while-loop.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index a7471468e2e2..49b2b4dd9faa 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -372,10 +372,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
 		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
-	if (pwrpriv->ps_processing) {
-		while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
-			msleep(10);
-	}
+	while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
+		msleep(10);
 
 	/* System suspend is not allowed to wakeup */
 	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
-- 
2.30.2

[PATCH 5/8] staging: r8188eu: summarize two if statements

[Martin Kaiser]

Summarize two if statements in rtw_pwr_wakeup and place the constants
on the right side of the comparison.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 49b2b4dd9faa..fa47dd24806b 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -383,11 +383,9 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	if (check_fwstate(pmlmepriv, _FW_LINKED))
 		goto exit;
 
-	if (rf_off == pwrpriv->rf_pwrstate) {
-		if (_FAIL ==  ips_leave(padapter)) {
-			ret = _FAIL;
-			goto exit;
-		}
+	if (pwrpriv->rf_pwrstate == rf_off && ips_leave(padapter) == _FAIL) {
+		ret = _FAIL;
+		goto exit;
 	}
 
 	if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
-- 
2.30.2

[PATCH 6/8] staging: r8188eu: use kernel functions for timeout handling

[Martin Kaiser]

Use the kernel functions to set a timeout and to check if it's expired.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index fa47dd24806b..1156a46b5de6 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -362,8 +362,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 {
 	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+	unsigned long timeout = jiffies + msecs_to_jiffies(3000);
 	int ret = _SUCCESS;
-	u32 start = jiffies;
 	u32 ips_deffer_ms;
 
 	/* the ms will prevent from falling into IPS after wakeup */
@@ -372,11 +372,11 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
 		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
-	while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
+	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
 
 	/* System suspend is not allowed to wakeup */
-	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
+	while (pwrpriv->bInSuspend && time_before(jiffies, timeout))
 		msleep(10);
 
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
-- 
2.30.2

[PATCH 7/8] staging: r8188eu: clean up the code to set ips_deny_time

[Martin Kaiser]

Clean up the code in rtw_pwr_wakeup that sets pwrpriv->ips_deny_time.

Make ips_deny_time an unsigned long, this type is used by the kernel
functions that process jiffies.

Remove the temporary variable ips_deffer_ms and use
RTW_PWR_STATE_CHK_INTERVAL directly.

There's no need to set ips_deny_time twice, it's sufficient to set it at
the end of rtw_pwr_wakeup.

Use time_before to check if ips_deny_time should be updated.

We can now remove rtw_ms_to_systime, this function is not used any more.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c      | 13 ++++---------
 drivers/staging/r8188eu/include/osdep_service.h |  1 -
 drivers/staging/r8188eu/include/rtw_pwrctrl.h   |  2 +-
 drivers/staging/r8188eu/os_dep/osdep_service.c  |  5 -----
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 1156a46b5de6..717a9bb26c19 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -363,14 +363,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	unsigned long timeout = jiffies + msecs_to_jiffies(3000);
+	unsigned long deny_time;
 	int ret = _SUCCESS;
-	u32 ips_deffer_ms;
-
-	/* the ms will prevent from falling into IPS after wakeup */
-	ips_deffer_ms = RTW_PWR_STATE_CHK_INTERVAL;
-
-	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
-		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
 	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
@@ -394,8 +388,9 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	}
 
 exit:
-	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
-		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
+	deny_time = jiffies + msecs_to_jiffies(RTW_PWR_STATE_CHK_INTERVAL);
+	if (time_before(pwrpriv->ips_deny_time, deny_time))
+		pwrpriv->ips_deny_time = deny_time;
 	return ret;
 }
 
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index f1f3e3ba5377..1e55a8008acc 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -78,7 +78,6 @@ void *rtw_malloc2d(int h, int w, int size);
 	} while (0)
 
 u32  rtw_systime_to_ms(u32 systime);
-u32  rtw_ms_to_systime(u32 ms);
 s32  rtw_get_passing_time_ms(u32 start);
 
 void rtw_usleep_os(int us);
diff --git a/drivers/staging/r8188eu/include/rtw_pwrctrl.h b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
index 1d7dba853c40..a5bc2f276024 100644
--- a/drivers/staging/r8188eu/include/rtw_pwrctrl.h
+++ b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
@@ -56,7 +56,7 @@ struct pwrctrl_priv {
 	u8	ips_mode_req;	/*  used to accept the mode setting request,
 				 *  will update to ipsmode later */
 	uint bips_processing;
-	u32 ips_deny_time; /* will deny IPS when system time less than this */
+	unsigned long ips_deny_time; /* will deny IPS when system time less than this */
 	u8 ps_processing; /* temp used to mark whether in rtw_ps_processor */
 
 	u8	bLeisurePs;
diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
index 6824a6ab2e36..7b177d50eee2 100644
--- a/drivers/staging/r8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
@@ -47,11 +47,6 @@ inline u32 rtw_systime_to_ms(u32 systime)
 	return systime * 1000 / HZ;
 }
 
-inline u32 rtw_ms_to_systime(u32 ms)
-{
-	return ms * HZ / 1000;
-}
-
 /*  the input parameter start use the same unit as jiffies */
 inline s32 rtw_get_passing_time_ms(u32 start)
 {
-- 
2.30.2

[PATCH 8/8] staging: r8188eu: remove the bInSuspend loop

[Martin Kaiser]

Remove the loop in rtw_pwr_wakeup that waits while the system is
suspended.

pwrpriv->bInSuspend is set in rtw_suspend and cleared in rtw_resume. These
functions are the .suspend and .resume functions of the struct usb_driver
for r8188eu.

A usb_driver's suspend and resume functions are called when the entire
system goes into suspend or runtime suspend.

All of the code paths for rtw_pwr_wakeup start at ioctl handlers.

We can remove the loop that checks bInSuspend. It's not possible to call
an ioctl while the entire system is suspended.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---

I tried to track down who calls suspend and resume of an usb_driver. My
understanding is that all of these calls come from the pm layer and that
the suspend and resume affects the whole system, not just the usb device.

 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 717a9bb26c19..093794414d67 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -369,10 +369,6 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
 
-	/* System suspend is not allowed to wakeup */
-	while (pwrpriv->bInSuspend && time_before(jiffies, timeout))
-		msleep(10);
-
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
 	if (check_fwstate(pmlmepriv, _FW_LINKED))
 		goto exit;
-- 
2.30.2

Re: [PATCH 2/8] staging: r8188eu: don't set _SUCCESS again

[Dan Carpenter]

On Sat, Apr 09, 2022 at 05:15:51PM +0200, Martin Kaiser wrote:
> ret is initialized to _SUCCESS, there's no need to set it again.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

I liked the original code better.  Otherwise you wonder, is it
intentional to return success on this path.  Smatch has a static
checker warning for these because most of the time it is a bug.
You can set "ret = _SUCCESS;" within 4 lines of the goto to silence
the warning.  A second, not as good option, is to add comment
"/* deliberatly returning success */" which saves a little time, but
also wastes a little time.

If you have to remove something, then remove the initializer.

The compiler is probably clever enough to remove these assignments so
it doesn't make sense to add an additional way to annotate success
paths.

regards,
dan carpenter

Re: [PATCH 2/8] staging: r8188eu: don't set _SUCCESS again

[Martin Kaiser]

Thus wrote Dan Carpenter (dan.carpenter@oracle.com):

> On Sat, Apr 09, 2022 at 05:15:51PM +0200, Martin Kaiser wrote:
> > ret is initialized to _SUCCESS, there's no need to set it again.

> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>

> I liked the original code better.  Otherwise you wonder, is it
> intentional to return success on this path. 

You're right. The original code is easier to understand. It's not
obvious that this check should return _SUCCESS and the remaining ones
return _FAIL.

Greg, could you drop this patch or should I resend the series without
this patch?

Thanks,
Martin

Re: [PATCH 2/8] staging: r8188eu: don't set _SUCCESS again

[Dan Carpenter]

On Mon, Apr 11, 2022 at 08:39:18PM +0200, Martin Kaiser wrote:
> Thus wrote Dan Carpenter (dan.carpenter@oracle.com):
> 
> > On Sat, Apr 09, 2022 at 05:15:51PM +0200, Martin Kaiser wrote:
> > > ret is initialized to _SUCCESS, there's no need to set it again.
> 
> > > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> 
> > I liked the original code better.  Otherwise you wonder, is it
> > intentional to return success on this path. 
> 
> You're right. The original code is easier to understand. It's not
> obvious that this check should return _SUCCESS and the remaining ones
> return _FAIL.
> 
> Greg, could you drop this patch or should I resend the series without
> this patch?

Martin, if Greg's already applied this patch then just leave it as-is.

We're going to have to go through and remove all the _SUCCESS/_FAIL
stuff anyway.  This problem will be cleaned up in end.  No need to worry
about it.

regards,
dan carpenter

Re: [PATCH 2/8] staging: r8188eu: don't set _SUCCESS again

[Greg Kroah-Hartman]

On Mon, Apr 11, 2022 at 08:39:18PM +0200, Martin Kaiser wrote:
> Thus wrote Dan Carpenter (dan.carpenter@oracle.com):
> 
> > On Sat, Apr 09, 2022 at 05:15:51PM +0200, Martin Kaiser wrote:
> > > ret is initialized to _SUCCESS, there's no need to set it again.
> 
> > > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> 
> > I liked the original code better.  Otherwise you wonder, is it
> > intentional to return success on this path. 
> 
> You're right. The original code is easier to understand. It's not
> obvious that this check should return _SUCCESS and the remaining ones
> return _FAIL.
> 
> Greg, could you drop this patch or should I resend the series without
> this patch?

Please resend without this one.

thanks,

greg k-h

[PATCH v2 0/8] staging: r8188eu: clean up rtw_pwr_wakeup

[Martin Kaiser]

Clean up the rtw_pwr_wakeup function. Use in-kernel routines for
handling timeouts.

v2:
 - drop the "don't set _SUCCESS again" patch
 - add another patch to remove unused timer functions

Martin Kaiser (8):
  staging: r8188eu: simplify delay conditions in rtw_pwr_wakeup
  staging: r8188eu: make return values consistent
  staging: r8188eu: simplify the ps_processing check
  staging: r8188eu: summarize two if statements
  staging: r8188eu: use kernel functions for timeout handling
  staging: r8188eu: clean up the code to set ips_deny_time
  staging: r8188eu: remove the bInSuspend loop
  staging: r8188eu: remove unused timer functions

 drivers/staging/r8188eu/core/rtw_pwrctrl.c    | 44 ++++++-------------
 .../staging/r8188eu/include/osdep_service.h   |  4 --
 drivers/staging/r8188eu/include/rtw_pwrctrl.h |  2 +-
 .../staging/r8188eu/os_dep/osdep_service.c    | 16 -------
 4 files changed, 14 insertions(+), 52 deletions(-)

-- 
2.30.2

[PATCH v2 1/8] staging: r8188eu: simplify delay conditions in rtw_pwr_wakeup

[Martin Kaiser]

Simplify the conditions for a loop in rtw_pwr_wakeup that waits while
the system is suspended.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 8c2e98361e47..8150894fba82 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -388,12 +388,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	}
 
 	/* System suspend is not allowed to wakeup */
-	if (pwrpriv->bInSuspend) {
-		while (pwrpriv->bInSuspend &&
-		       (rtw_get_passing_time_ms(start) <= 3000 ||
-		       (rtw_get_passing_time_ms(start) <= 500)))
-				msleep(10);
-	}
+	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
+		msleep(10);
 
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
 	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-- 
2.30.2

[PATCH v2 2/8] staging: r8188eu: make return values consistent

[Martin Kaiser]

rtw_pwr_wakeup should return _SUCCESS or _FAIL.

Replace false with _FAIL in one place and reformat the if-statement.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 8150894fba82..6a40f4a251c7 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -403,10 +403,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 		}
 	}
 
-	/* TODO: the following checking need to be merged... */
-	if (padapter->bDriverStopped || !padapter->bup ||
-	    !padapter->hw_init_completed) {
-		ret = false;
+	if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
+		ret = _FAIL;
 		goto exit;
 	}
 
-- 
2.30.2

[PATCH v2 3/8] staging: r8188eu: simplify the ps_processing check

[Martin Kaiser]

It's sufficient to check pwrpriv->ps_processing as part of the while-loop.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 6a40f4a251c7..fd7ea83968ed 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -382,10 +382,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
 		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
-	if (pwrpriv->ps_processing) {
-		while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
-			msleep(10);
-	}
+	while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
+		msleep(10);
 
 	/* System suspend is not allowed to wakeup */
 	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
-- 
2.30.2

[PATCH v2 4/8] staging: r8188eu: summarize two if statements

[Martin Kaiser]

Summarize two if statements in rtw_pwr_wakeup and place the constants
on the right side of the comparison.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index fd7ea83968ed..ff96e5229b52 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -394,11 +394,10 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 		ret = _SUCCESS;
 		goto exit;
 	}
-	if (rf_off == pwrpriv->rf_pwrstate) {
-		if (_FAIL ==  ips_leave(padapter)) {
-			ret = _FAIL;
-			goto exit;
-		}
+
+	if (pwrpriv->rf_pwrstate == rf_off && ips_leave(padapter) == _FAIL) {
+		ret = _FAIL;
+		goto exit;
 	}
 
 	if (padapter->bDriverStopped || !padapter->bup || !padapter->hw_init_completed) {
-- 
2.30.2

[PATCH v2 5/8] staging: r8188eu: use kernel functions for timeout handling

[Martin Kaiser]

Use the kernel functions to set a timeout and to check if it's expired.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index ff96e5229b52..2ad6105e6ec4 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -372,8 +372,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 {
 	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+	unsigned long timeout = jiffies + msecs_to_jiffies(3000);
 	int ret = _SUCCESS;
-	u32 start = jiffies;
 	u32 ips_deffer_ms;
 
 	/* the ms will prevent from falling into IPS after wakeup */
@@ -382,11 +382,11 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
 		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
-	while (pwrpriv->ps_processing && rtw_get_passing_time_ms(start) <= 3000)
+	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
 
 	/* System suspend is not allowed to wakeup */
-	while (pwrpriv->bInSuspend && rtw_get_passing_time_ms(start) <= 3000)
+	while (pwrpriv->bInSuspend && time_before(jiffies, timeout))
 		msleep(10);
 
 	/* I think this should be check in IPS, LPS, autosuspend functions... */
-- 
2.30.2

[PATCH v2 6/8] staging: r8188eu: clean up the code to set ips_deny_time

[Martin Kaiser]

Clean up the code in rtw_pwr_wakeup that sets pwrpriv->ips_deny_time.

Make ips_deny_time an unsigned long, this type is used by the kernel
functions that process jiffies.

Remove the temporary variable ips_deffer_ms and use
RTW_PWR_STATE_CHK_INTERVAL directly.

There's no need to set ips_deny_time twice, it's sufficient to set it at
the end of rtw_pwr_wakeup.

Use time_before to check if ips_deny_time should be updated.

We can now remove rtw_ms_to_systime, this function is not used any more.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c      | 13 ++++---------
 drivers/staging/r8188eu/include/osdep_service.h |  1 -
 drivers/staging/r8188eu/include/rtw_pwrctrl.h   |  2 +-
 drivers/staging/r8188eu/os_dep/osdep_service.c  |  5 -----
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 2ad6105e6ec4..605210d89f32 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -373,14 +373,8 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	unsigned long timeout = jiffies + msecs_to_jiffies(3000);
+	unsigned long deny_time;
 	int ret = _SUCCESS;
-	u32 ips_deffer_ms;
-
-	/* the ms will prevent from falling into IPS after wakeup */
-	ips_deffer_ms = RTW_PWR_STATE_CHK_INTERVAL;
-
-	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
-		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
 
 	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
@@ -406,8 +400,9 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	}
 
 exit:
-	if (pwrpriv->ips_deny_time < jiffies + rtw_ms_to_systime(ips_deffer_ms))
-		pwrpriv->ips_deny_time = jiffies + rtw_ms_to_systime(ips_deffer_ms);
+	deny_time = jiffies + msecs_to_jiffies(RTW_PWR_STATE_CHK_INTERVAL);
+	if (time_before(pwrpriv->ips_deny_time, deny_time))
+		pwrpriv->ips_deny_time = deny_time;
 	return ret;
 }
 
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index f1f3e3ba5377..1e55a8008acc 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -78,7 +78,6 @@ void *rtw_malloc2d(int h, int w, int size);
 	} while (0)
 
 u32  rtw_systime_to_ms(u32 systime);
-u32  rtw_ms_to_systime(u32 ms);
 s32  rtw_get_passing_time_ms(u32 start);
 
 void rtw_usleep_os(int us);
diff --git a/drivers/staging/r8188eu/include/rtw_pwrctrl.h b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
index 3084d00628bd..6e9fdd66fad1 100644
--- a/drivers/staging/r8188eu/include/rtw_pwrctrl.h
+++ b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
@@ -56,7 +56,7 @@ struct pwrctrl_priv {
 	u8	ips_mode_req;	/*  used to accept the mode setting request,
 				 *  will update to ipsmode later */
 	uint bips_processing;
-	u32 ips_deny_time; /* will deny IPS when system time less than this */
+	unsigned long ips_deny_time; /* will deny IPS when system time less than this */
 	u8 ps_processing; /* temp used to mark whether in rtw_ps_processor */
 
 	u8	bLeisurePs;
diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
index 6824a6ab2e36..7b177d50eee2 100644
--- a/drivers/staging/r8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
@@ -47,11 +47,6 @@ inline u32 rtw_systime_to_ms(u32 systime)
 	return systime * 1000 / HZ;
 }
 
-inline u32 rtw_ms_to_systime(u32 ms)
-{
-	return ms * HZ / 1000;
-}
-
 /*  the input parameter start use the same unit as jiffies */
 inline s32 rtw_get_passing_time_ms(u32 start)
 {
-- 
2.30.2

[PATCH v2 7/8] staging: r8188eu: remove the bInSuspend loop

[Martin Kaiser]

Remove the loop in rtw_pwr_wakeup that waits while the system is
suspended.

pwrpriv->bInSuspend is set in rtw_suspend and cleared in rtw_resume. These
functions are the .suspend and .resume functions of the struct usb_driver
for r8188eu.

A usb_driver's suspend and resume functions are called when the entire
system goes into suspend or runtime suspend.

All of the code paths for rtw_pwr_wakeup start at ioctl handlers.

We can remove the loop that checks bInSuspend. It's not possible to call
an ioctl while the entire system is suspended.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---

I tried to track down who calls suspend and resume of an usb_driver. My
understanding is that all of these calls come from the pm layer and that
the suspend and resume affect the whole system, not just the usb device.

 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 605210d89f32..6990808ef353 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -379,10 +379,6 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 	while (pwrpriv->ps_processing && time_before(jiffies, timeout))
 		msleep(10);
 
-	/* System suspend is not allowed to wakeup */
-	while (pwrpriv->bInSuspend && 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;
-- 
2.30.2

[PATCH v2 8/8] staging: r8188eu: remove unused timer functions

[Martin Kaiser]

rtw_get_passing_time_ms and rtw_systime_to_ms are not used any more.
Remove them.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/osdep_service.h |  3 ---
 drivers/staging/r8188eu/os_dep/osdep_service.c  | 11 -----------
 2 files changed, 14 deletions(-)

diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index 1e55a8008acc..f1a703643e74 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -77,9 +77,6 @@ void *rtw_malloc2d(int h, int w, int size);
 		spin_lock_init(&((q)->lock));			\
 	} while (0)
 
-u32  rtw_systime_to_ms(u32 systime);
-s32  rtw_get_passing_time_ms(u32 start);
-
 void rtw_usleep_os(int us);
 
 static inline unsigned char _cancel_timer_ex(struct timer_list *ptimer)
diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
index 7b177d50eee2..812acd59be79 100644
--- a/drivers/staging/r8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
@@ -42,17 +42,6 @@ Otherwise, there will be racing condition.
 Caller must check if the list is empty before calling rtw_list_delete
 */
 
-inline u32 rtw_systime_to_ms(u32 systime)
-{
-	return systime * 1000 / HZ;
-}
-
-/*  the input parameter start use the same unit as jiffies */
-inline s32 rtw_get_passing_time_ms(u32 start)
-{
-	return rtw_systime_to_ms(jiffies - start);
-}
-
 void rtw_usleep_os(int us)
 {
 	if (1 < (us / 1000))
-- 
2.30.2