* [PATCH 1/5] staging: ks7010: invert conditional, reduce indent
2017-02-27 3:14 [PATCH 0/5] staging: ks7010 refactor _ks_wlan_hw_power_save Tobin C. Harding
@ 2017-02-27 3:14 ` Tobin C. Harding
2017-02-27 3:14 ` [PATCH 2/5] staging: ks7010: fix checkpatch MULTILINE_DEREFERENCE Tobin C. Harding
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-02-27 3:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Function _ks_wlan_hw_power_save has 5 levels of indentation. Reducing
the amount of indentation may make code clearer to read. Also a number
of other checkpatch warnings may be removed if we first reduce the
level of indentation in this function.
Invert conditional and return from function if new conditional
evaluates to true. Repeat on subsequent conditional. Reduce
indentation without changing the program logic.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 122 ++++++++++++++++++-----------------
1 file changed, 62 insertions(+), 60 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 6f9f746..1cf8b12 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -176,70 +176,72 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
if (priv->reg.powermgt == POWMGT_ACTIVE_MODE)
return 0;
- if (priv->reg.operation_mode == MODE_INFRASTRUCTURE &&
- (priv->connect_status & CONNECT_STATUS_MASK) == CONNECT_STATUS) {
- if (priv->dev_state == DEVICE_STATE_SLEEP) {
- switch (atomic_read(&priv->psstatus.status)) {
- case PS_SNOOZE: /* 4 */
+ if (priv->reg.operation_mode != MODE_INFRASTRUCTURE ||
+ (priv->connect_status & CONNECT_STATUS_MASK) != CONNECT_STATUS)
+ return 0;
+
+ if (priv->dev_state != DEVICE_STATE_SLEEP)
+ return 0;
+
+ switch (atomic_read(&priv->psstatus.status)) {
+ case PS_SNOOZE: /* 4 */
+ break;
+ default:
+ DPRINTK(5, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
+ atomic_read(&priv->psstatus.status),
+ atomic_read(&priv->psstatus.confirm_wait),
+ atomic_read(&priv->psstatus.snooze_guard),
+ cnt_txqbody(priv));
+
+ if (!atomic_read(&priv->psstatus.confirm_wait)
+ && !atomic_read(&priv->psstatus.snooze_guard)
+ && !cnt_txqbody(priv)) {
+ retval =
+ ks7010_sdio_read(priv, INT_PENDING,
+ &rw_data,
+ sizeof(rw_data));
+ if (retval) {
+ DPRINTK(1,
+ " error : INT_PENDING=%02X\n",
+ rw_data);
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
break;
- default:
- DPRINTK(5, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
- atomic_read(&priv->psstatus.status),
- atomic_read(&priv->psstatus.confirm_wait),
- atomic_read(&priv->psstatus.snooze_guard),
- cnt_txqbody(priv));
-
- if (!atomic_read(&priv->psstatus.confirm_wait)
- && !atomic_read(&priv->psstatus.snooze_guard)
- && !cnt_txqbody(priv)) {
- retval =
- ks7010_sdio_read(priv, INT_PENDING,
- &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1,
- " error : INT_PENDING=%02X\n",
- rw_data);
- queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
- break;
- }
- if (!rw_data) {
- rw_data = GCR_B_DOZE;
- retval =
- ks7010_sdio_write(priv,
- GCR_B,
- &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1,
- " error : GCR_B=%02X\n",
- rw_data);
- queue_delayed_work
- (priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
- break;
- }
- DPRINTK(4,
- "PMG SET!! : GCR_B=%02X\n",
- rw_data);
- atomic_set(&priv->psstatus.
- status, PS_SNOOZE);
- DPRINTK(3,
- "psstatus.status=PS_SNOOZE\n");
- } else {
- queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
- }
- } else {
- queue_delayed_work(priv->ks_wlan_hw.
- ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq,
- 0);
+ }
+ if (!rw_data) {
+ rw_data = GCR_B_DOZE;
+ retval =
+ ks7010_sdio_write(priv,
+ GCR_B,
+ &rw_data,
+ sizeof(rw_data));
+ if (retval) {
+ DPRINTK(1,
+ " error : GCR_B=%02X\n",
+ rw_data);
+ queue_delayed_work
+ (priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
+ break;
}
- break;
+ DPRINTK(4,
+ "PMG SET!! : GCR_B=%02X\n",
+ rw_data);
+ atomic_set(&priv->psstatus.
+ status, PS_SNOOZE);
+ DPRINTK(3,
+ "psstatus.status=PS_SNOOZE\n");
+ } else {
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
}
+ } else {
+ queue_delayed_work(priv->ks_wlan_hw.
+ ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq,
+ 0);
}
+ break;
}
return 0;
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] staging: ks7010: fix checkpatch MULTILINE_DEREFERENCE
2017-02-27 3:14 [PATCH 0/5] staging: ks7010 refactor _ks_wlan_hw_power_save Tobin C. Harding
2017-02-27 3:14 ` [PATCH 1/5] staging: ks7010: invert conditional, reduce indent Tobin C. Harding
@ 2017-02-27 3:14 ` Tobin C. Harding
2017-02-27 3:14 ` [PATCH 3/5] staging: ks7010: move logic operator to end of line Tobin C. Harding
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-02-27 3:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Checkpatch emits WARNING: Avoid multiple line dereference.
Move dereference onto single line.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 1cf8b12..4ce5867 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -227,8 +227,7 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
DPRINTK(4,
"PMG SET!! : GCR_B=%02X\n",
rw_data);
- atomic_set(&priv->psstatus.
- status, PS_SNOOZE);
+ atomic_set(&priv->psstatus.status, PS_SNOOZE);
DPRINTK(3,
"psstatus.status=PS_SNOOZE\n");
} else {
@@ -236,8 +235,7 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
&priv->ks_wlan_hw.rw_wq, 1);
}
} else {
- queue_delayed_work(priv->ks_wlan_hw.
- ks7010sdio_wq,
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
&priv->ks_wlan_hw.rw_wq,
0);
}
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] staging: ks7010: move logic operator to end of line
2017-02-27 3:14 [PATCH 0/5] staging: ks7010 refactor _ks_wlan_hw_power_save Tobin C. Harding
2017-02-27 3:14 ` [PATCH 1/5] staging: ks7010: invert conditional, reduce indent Tobin C. Harding
2017-02-27 3:14 ` [PATCH 2/5] staging: ks7010: fix checkpatch MULTILINE_DEREFERENCE Tobin C. Harding
@ 2017-02-27 3:14 ` Tobin C. Harding
2017-02-27 3:14 ` [PATCH 4/5] staging: ks7010: remove switch statement Tobin C. Harding
2017-02-27 3:14 ` [PATCH 5/5] staging: ks7010: refactor function call parameters Tobin C. Harding
4 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-02-27 3:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Logic operator (&&) is place at the start of the line. Kernel
standards suggest that logical operators should be placed at the end
of the line.
Move logical operator to the end of the previous line.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 4ce5867..0e07b83 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -193,9 +193,9 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
atomic_read(&priv->psstatus.snooze_guard),
cnt_txqbody(priv));
- if (!atomic_read(&priv->psstatus.confirm_wait)
- && !atomic_read(&priv->psstatus.snooze_guard)
- && !cnt_txqbody(priv)) {
+ if (!atomic_read(&priv->psstatus.confirm_wait) &&
+ !atomic_read(&priv->psstatus.snooze_guard) &&
+ !cnt_txqbody(priv)) {
retval =
ks7010_sdio_read(priv, INT_PENDING,
&rw_data,
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] staging: ks7010: remove switch statement
2017-02-27 3:14 [PATCH 0/5] staging: ks7010 refactor _ks_wlan_hw_power_save Tobin C. Harding
` (2 preceding siblings ...)
2017-02-27 3:14 ` [PATCH 3/5] staging: ks7010: move logic operator to end of line Tobin C. Harding
@ 2017-02-27 3:14 ` Tobin C. Harding
2017-02-27 3:14 ` [PATCH 5/5] staging: ks7010: refactor function call parameters Tobin C. Harding
4 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-02-27 3:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Switch statement uses one [trivial] case and a default case holding
bulk of code. We can swap the switch statement with an if/return
statement as replacement for the original switch. This can be done
without changing the program logic.
Remove switch statement. Use original switch parameter as conditional
and return if conditional evaluates to true. Reduce level of
indentation. Do not change the program logic.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 89 +++++++++++++++++-------------------
1 file changed, 43 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 0e07b83..04130fb 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -183,63 +183,60 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
if (priv->dev_state != DEVICE_STATE_SLEEP)
return 0;
- switch (atomic_read(&priv->psstatus.status)) {
- case PS_SNOOZE: /* 4 */
- break;
- default:
- DPRINTK(5, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
- atomic_read(&priv->psstatus.status),
- atomic_read(&priv->psstatus.confirm_wait),
- atomic_read(&priv->psstatus.snooze_guard),
- cnt_txqbody(priv));
-
- if (!atomic_read(&priv->psstatus.confirm_wait) &&
- !atomic_read(&priv->psstatus.snooze_guard) &&
- !cnt_txqbody(priv)) {
+ if (atomic_read(&priv->psstatus.status) == PS_SNOOZE)
+ return 0;
+
+ DPRINTK(5, "\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
+ atomic_read(&priv->psstatus.status),
+ atomic_read(&priv->psstatus.confirm_wait),
+ atomic_read(&priv->psstatus.snooze_guard),
+ cnt_txqbody(priv));
+
+ if (!atomic_read(&priv->psstatus.confirm_wait) &&
+ !atomic_read(&priv->psstatus.snooze_guard) &&
+ !cnt_txqbody(priv)) {
+ retval =
+ ks7010_sdio_read(priv, INT_PENDING,
+ &rw_data,
+ sizeof(rw_data));
+ if (retval) {
+ DPRINTK(1,
+ " error : INT_PENDING=%02X\n",
+ rw_data);
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
+ return 0;
+ }
+ if (!rw_data) {
+ rw_data = GCR_B_DOZE;
retval =
- ks7010_sdio_read(priv, INT_PENDING,
+ ks7010_sdio_write(priv,
+ GCR_B,
&rw_data,
sizeof(rw_data));
if (retval) {
DPRINTK(1,
- " error : INT_PENDING=%02X\n",
+ " error : GCR_B=%02X\n",
rw_data);
- queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
- break;
- }
- if (!rw_data) {
- rw_data = GCR_B_DOZE;
- retval =
- ks7010_sdio_write(priv,
- GCR_B,
- &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1,
- " error : GCR_B=%02X\n",
- rw_data);
- queue_delayed_work
- (priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
- break;
- }
- DPRINTK(4,
- "PMG SET!! : GCR_B=%02X\n",
- rw_data);
- atomic_set(&priv->psstatus.status, PS_SNOOZE);
- DPRINTK(3,
- "psstatus.status=PS_SNOOZE\n");
- } else {
- queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ queue_delayed_work
+ (priv->ks_wlan_hw.ks7010sdio_wq,
&priv->ks_wlan_hw.rw_wq, 1);
+ return 0;
}
+ DPRINTK(4,
+ "PMG SET!! : GCR_B=%02X\n",
+ rw_data);
+ atomic_set(&priv->psstatus.status, PS_SNOOZE);
+ DPRINTK(3,
+ "psstatus.status=PS_SNOOZE\n");
} else {
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq,
- 0);
+ &priv->ks_wlan_hw.rw_wq, 1);
}
- break;
+ } else {
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq,
+ 0);
}
return 0;
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] staging: ks7010: refactor function call parameters
2017-02-27 3:14 [PATCH 0/5] staging: ks7010 refactor _ks_wlan_hw_power_save Tobin C. Harding
` (3 preceding siblings ...)
2017-02-27 3:14 ` [PATCH 4/5] staging: ks7010: remove switch statement Tobin C. Harding
@ 2017-02-27 3:14 ` Tobin C. Harding
4 siblings, 0 replies; 6+ messages in thread
From: Tobin C. Harding @ 2017-02-27 3:14 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Function call parameters are split over more lines than
necessary. Also assignment statements are split after the '=' sign.
This adds extra lines to the function and may also reduces
readability.
Refactor function call parameters and reduce the number of lines
used. Put assignment statements onto single line.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 40 ++++++++++++------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 04130fb..14580cb 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -195,48 +195,34 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
if (!atomic_read(&priv->psstatus.confirm_wait) &&
!atomic_read(&priv->psstatus.snooze_guard) &&
!cnt_txqbody(priv)) {
- retval =
- ks7010_sdio_read(priv, INT_PENDING,
- &rw_data,
- sizeof(rw_data));
+ retval = ks7010_sdio_read(priv, INT_PENDING, &rw_data,
+ sizeof(rw_data));
if (retval) {
- DPRINTK(1,
- " error : INT_PENDING=%02X\n",
- rw_data);
+ DPRINTK(1, " error : INT_PENDING=%02X\n", rw_data);
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
+ &priv->ks_wlan_hw.rw_wq, 1);
return 0;
}
if (!rw_data) {
rw_data = GCR_B_DOZE;
- retval =
- ks7010_sdio_write(priv,
- GCR_B,
- &rw_data,
- sizeof(rw_data));
+ retval = ks7010_sdio_write(priv, GCR_B, &rw_data,
+ sizeof(rw_data));
if (retval) {
- DPRINTK(1,
- " error : GCR_B=%02X\n",
- rw_data);
- queue_delayed_work
- (priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
+ DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
return 0;
}
- DPRINTK(4,
- "PMG SET!! : GCR_B=%02X\n",
- rw_data);
+ DPRINTK(4, "PMG SET!! : GCR_B=%02X\n", rw_data);
atomic_set(&priv->psstatus.status, PS_SNOOZE);
- DPRINTK(3,
- "psstatus.status=PS_SNOOZE\n");
+ DPRINTK(3, "psstatus.status=PS_SNOOZE\n");
} else {
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
+ &priv->ks_wlan_hw.rw_wq, 1);
}
} else {
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq,
- 0);
+ &priv->ks_wlan_hw.rw_wq, 0);
}
return 0;
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread