* [PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt()
@ 2017-03-27 2:06 Tobin C. Harding
2017-03-27 2:06 ` [PATCH 1/3] staging: ks7010: rename identifier retval to ret Tobin C. Harding
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Tobin C. Harding @ 2017-03-27 2:06 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Checkpatch emits several warnings when parsing the function
ks_sdio_interrupt(). Function lends itself to being refactored.
Patch 01 renames identifier 'retval' to 'ret' in line with the rest of
the driver.
Patch 02 inverts if statement conditional and reduces the level of
indentation in subsequent code by one level.
Patch 03 fixes white space issues, clearing a couple of checkpatch
warnings and trivial style issues.
Code is build-tested only (x86_64 and PowerPC).
Tobin C. Harding (3):
staging: ks7010: rename identifier retval to ret
staging: ks7010: invert conditional, reduce indentation
staging: ks7010: fix whitespace issues
drivers/staging/ks7010/ks7010_sdio.c | 118 ++++++++++++++++-------------------
1 file changed, 55 insertions(+), 63 deletions(-)
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] staging: ks7010: rename identifier retval to ret
2017-03-27 2:06 [PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt() Tobin C. Harding
@ 2017-03-27 2:06 ` Tobin C. Harding
2017-03-27 2:06 ` [PATCH 2/3] staging: ks7010: invert conditional, reduce indentation Tobin C. Harding
2017-03-27 2:06 ` [PATCH 3/3] staging: ks7010: fix whitespace issues Tobin C. Harding
2 siblings, 0 replies; 4+ messages in thread
From: Tobin C. Harding @ 2017-03-27 2:06 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Driver predominately uses the identifier 'ret' for the error return
code. Function uses 'retval' for this task. It is cleaner if we use a
single name for a single task.
Rename 'retval' to 'ret'.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index b16618b..08e89b3 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -534,7 +534,7 @@ static void ks7010_rw_function(struct work_struct *work)
static void ks_sdio_interrupt(struct sdio_func *func)
{
- int retval;
+ int ret;
struct ks_sdio_card *card;
struct ks_wlan_private *priv;
unsigned char status, rsize, rw_data;
@@ -544,11 +544,10 @@ static void ks_sdio_interrupt(struct sdio_func *func)
DPRINTK(4, "\n");
if (priv->dev_state >= DEVICE_STATE_BOOT) {
- retval =
- ks7010_sdio_read(priv, INT_PENDING, &status,
- sizeof(status));
- if (retval) {
- DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
+ ret = ks7010_sdio_read(priv, INT_PENDING, &status,
+ sizeof(status));
+ if (ret) {
+ DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret);
goto queue_delayed_work;
}
DPRINTK(4, "INT_PENDING=%02X\n", rw_data);
@@ -560,10 +559,9 @@ static void ks_sdio_interrupt(struct sdio_func *func)
/* bit2 -> Read Status Busy */
if (status & INT_GCR_B ||
atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
- retval =
- ks7010_sdio_read(priv, GCR_B, &rw_data,
- sizeof(rw_data));
- if (retval) {
+ ret = ks7010_sdio_read(priv, GCR_B, &rw_data,
+ sizeof(rw_data));
+ if (ret) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto queue_delayed_work;
}
@@ -581,10 +579,9 @@ static void ks_sdio_interrupt(struct sdio_func *func)
do {
/* read (WriteStatus/ReadDataSize FN1:00_0014) */
- retval =
- ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
- sizeof(rw_data));
- if (retval) {
+ ret = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
+ sizeof(rw_data));
+ if (ret) {
DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
rw_data);
goto queue_delayed_work;
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] staging: ks7010: invert conditional, reduce indentation
2017-03-27 2:06 [PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt() Tobin C. Harding
2017-03-27 2:06 ` [PATCH 1/3] staging: ks7010: rename identifier retval to ret Tobin C. Harding
@ 2017-03-27 2:06 ` Tobin C. Harding
2017-03-27 2:06 ` [PATCH 3/3] staging: ks7010: fix whitespace issues Tobin C. Harding
2 siblings, 0 replies; 4+ messages in thread
From: Tobin C. Harding @ 2017-03-27 2:06 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Bulk of function is guarded by an if statement. If we invert the
conditional and return, the subsequent code can be indented one level
less.
Invert if statement conditional. '>=' to '<'. Jump to goto label if
new conditional evaluates to true. Do not change program logic.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 117 ++++++++++++++++++-----------------
1 file changed, 59 insertions(+), 58 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 08e89b3..bbee02c 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -543,72 +543,73 @@ static void ks_sdio_interrupt(struct sdio_func *func)
priv = card->priv;
DPRINTK(4, "\n");
- if (priv->dev_state >= DEVICE_STATE_BOOT) {
- ret = ks7010_sdio_read(priv, INT_PENDING, &status,
- sizeof(status));
+ if (priv->dev_state < DEVICE_STATE_BOOT)
+ goto queue_delayed_work;
+
+ ret = ks7010_sdio_read(priv, INT_PENDING, &status,
+ sizeof(status));
+ if (ret) {
+ DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret);
+ goto queue_delayed_work;
+ }
+ DPRINTK(4, "INT_PENDING=%02X\n", rw_data);
+
+ /* schedule task for interrupt status */
+ /* bit7 -> Write General Communication B register */
+ /* read (General Communication B register) */
+ /* bit5 -> Write Status Idle */
+ /* bit2 -> Read Status Busy */
+ if (status & INT_GCR_B ||
+ atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
+ ret = ks7010_sdio_read(priv, GCR_B, &rw_data,
+ sizeof(rw_data));
if (ret) {
- DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", ret);
+ DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto queue_delayed_work;
}
- DPRINTK(4, "INT_PENDING=%02X\n", rw_data);
-
- /* schedule task for interrupt status */
- /* bit7 -> Write General Communication B register */
- /* read (General Communication B register) */
- /* bit5 -> Write Status Idle */
- /* bit2 -> Read Status Busy */
- if (status & INT_GCR_B ||
- atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
- ret = ks7010_sdio_read(priv, GCR_B, &rw_data,
- sizeof(rw_data));
- if (ret) {
- DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
- goto queue_delayed_work;
- }
- /* DPRINTK(1, "GCR_B=%02X\n", rw_data); */
- if (rw_data == GCR_B_ACTIVE) {
- if (atomic_read(&priv->psstatus.status) ==
- PS_SNOOZE) {
- atomic_set(&priv->psstatus.status,
- PS_WAKEUP);
- priv->wakeup_count = 0;
- }
- complete(&priv->psstatus.wakeup_wait);
+ /* DPRINTK(1, "GCR_B=%02X\n", rw_data); */
+ if (rw_data == GCR_B_ACTIVE) {
+ if (atomic_read(&priv->psstatus.status) ==
+ PS_SNOOZE) {
+ atomic_set(&priv->psstatus.status,
+ PS_WAKEUP);
+ priv->wakeup_count = 0;
}
+ complete(&priv->psstatus.wakeup_wait);
}
+ }
- do {
- /* read (WriteStatus/ReadDataSize FN1:00_0014) */
- ret = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
- sizeof(rw_data));
- if (ret) {
- DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
- rw_data);
- goto queue_delayed_work;
- }
- DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
- rsize = rw_data & RSIZE_MASK;
- if (rsize != 0) { /* Read schedule */
- ks_wlan_hw_rx((void *)priv,
- (uint16_t)(rsize << 4));
- }
- if (rw_data & WSTATUS_MASK) {
- if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
- if (cnt_txqbody(priv)) {
- ks_wlan_hw_wakeup_request(priv);
- queue_delayed_work
- (priv->ks_wlan_hw.
- ks7010sdio_wq,
- &priv->ks_wlan_hw.
- rw_wq, 1);
- return;
- }
- } else {
- tx_device_task((void *)priv);
+ do {
+ /* read (WriteStatus/ReadDataSize FN1:00_0014) */
+ ret = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
+ sizeof(rw_data));
+ if (ret) {
+ DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
+ rw_data);
+ goto queue_delayed_work;
+ }
+ DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
+ rsize = rw_data & RSIZE_MASK;
+ if (rsize != 0) { /* Read schedule */
+ ks_wlan_hw_rx((void *)priv,
+ (uint16_t)(rsize << 4));
+ }
+ if (rw_data & WSTATUS_MASK) {
+ if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
+ if (cnt_txqbody(priv)) {
+ ks_wlan_hw_wakeup_request(priv);
+ queue_delayed_work
+ (priv->ks_wlan_hw.
+ ks7010sdio_wq,
+ &priv->ks_wlan_hw.
+ rw_wq, 1);
+ return;
}
+ } else {
+ tx_device_task((void *)priv);
}
- } while (rsize);
- }
+ }
+ } while (rsize);
queue_delayed_work:
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] staging: ks7010: fix whitespace issues
2017-03-27 2:06 [PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt() Tobin C. Harding
2017-03-27 2:06 ` [PATCH 1/3] staging: ks7010: rename identifier retval to ret Tobin C. Harding
2017-03-27 2:06 ` [PATCH 2/3] staging: ks7010: invert conditional, reduce indentation Tobin C. Harding
@ 2017-03-27 2:06 ` Tobin C. Harding
2 siblings, 0 replies; 4+ messages in thread
From: Tobin C. Harding @ 2017-03-27 2:06 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: driverdev-devel, Wolfram Sang
Code is able to have white space refactoring after previous patch
reduced the level of indentation. Doing so fixes two checkpatch
multiple line dereference.
Concatenate multiple line function calls when doing so will not take
character count over 80 characters. Remove multiple line dereference
even though doing so introduces two new checkpatch line over 80
warnings, resulting code is cleaner.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index bbee02c..ab00e77 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -569,10 +569,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
}
/* DPRINTK(1, "GCR_B=%02X\n", rw_data); */
if (rw_data == GCR_B_ACTIVE) {
- if (atomic_read(&priv->psstatus.status) ==
- PS_SNOOZE) {
- atomic_set(&priv->psstatus.status,
- PS_WAKEUP);
+ if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
+ atomic_set(&priv->psstatus.status, PS_WAKEUP);
priv->wakeup_count = 0;
}
complete(&priv->psstatus.wakeup_wait);
@@ -590,19 +588,15 @@ static void ks_sdio_interrupt(struct sdio_func *func)
}
DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
rsize = rw_data & RSIZE_MASK;
- if (rsize != 0) { /* Read schedule */
- ks_wlan_hw_rx((void *)priv,
- (uint16_t)(rsize << 4));
- }
+ if (rsize != 0) /* Read schedule */
+ ks_wlan_hw_rx((void *)priv, (uint16_t)(rsize << 4));
+
if (rw_data & WSTATUS_MASK) {
if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
if (cnt_txqbody(priv)) {
ks_wlan_hw_wakeup_request(priv);
- queue_delayed_work
- (priv->ks_wlan_hw.
- ks7010sdio_wq,
- &priv->ks_wlan_hw.
- rw_wq, 1);
+ queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
+ &priv->ks_wlan_hw.rw_wq, 1);
return;
}
} else {
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-27 2:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 2:06 [PATCH 0/3] staging: ks7010: refactor ks_sdio_interrupt() Tobin C. Harding
2017-03-27 2:06 ` [PATCH 1/3] staging: ks7010: rename identifier retval to ret Tobin C. Harding
2017-03-27 2:06 ` [PATCH 2/3] staging: ks7010: invert conditional, reduce indentation Tobin C. Harding
2017-03-27 2:06 ` [PATCH 3/3] staging: ks7010: fix whitespace issues Tobin C. Harding
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.