* [PATCH 0/9] staging: ks7010: Checkpatch fixes
@ 2017-02-20 4:23 Tobin C. Harding
2017-02-20 4:23 ` [PATCH 1/9] staging: ks7010: Fix checkpatch warnings Tobin C. Harding
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Checkpatch emits numerous warnings and checks. Line over 80 warnings
are not touched. Each patch addresses an individual checkpatch warning
(or check). Code is refactored over a series of patches in an effort
to modularize the changes. When a patch reduces the indentation no
other changes are made in the same patch. Decision to bundle
refactoring along with checkpatch fixes was made because the
motivating force is checkpatch.
Refactoring is limited to the functions surrounding checkpatch
warnings, if these changes are deemed satisfactory the rest of the
file could be refactored in a similar manner, perhaps that should be
done in this series also? 9 patches for the series seemed like it was
enough.
After application of this series 3 types of checkpatch message remain
(line over 80, macro argument reuse, and code compiled out with
ifdef).
Tobin C. Harding (9):
staging: ks7010: Fix checkpatch warnings
staging: ks7010: Remove dead code
staging: ks7010: Fix checkpatch warning
staging: ks7010: Fix checkpatch warning
staging: ks7010: refactor, change retval to ret
staging: ks7010: Refactor code
staging: ks7010: Remove level of indentation
staging: ks7010: Refactor code
staging: ks7010: Fix checkpatch whitespace checks
drivers/staging/ks7010/ks7010_sdio.c | 274 ++++++++++++++++-------------------
1 file changed, 123 insertions(+), 151 deletions(-)
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/9] staging: ks7010: Fix checkpatch warnings
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 2/9] staging: ks7010: Remove dead code Tobin C. Harding
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
There are multiple line dereferences. This leaves a '.' as the
last character of the line and makes the code hard to read.
Move variable dereference onto single line. Even if this causes line
over 80 warning.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index a604c83..129702c 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -223,8 +223,8 @@ 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 {
@@ -232,8 +232,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);
}
@@ -334,8 +333,7 @@ static void tx_device_task(void *dev)
if (rc) {
DPRINTK(1, "write_to_device error !!(%d)\n",
rc);
- 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;
}
@@ -635,10 +633,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
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);
+ (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] 11+ messages in thread
* [PATCH 2/9] staging: ks7010: Remove dead code
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
2017-02-20 4:23 ` [PATCH 1/9] staging: ks7010: Fix checkpatch warnings Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 3/9] staging: ks7010: Fix checkpatch warning Tobin C. Harding
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
There are blocks of code that are commented out.
Remove commented out code. Fix two small spelling typo's.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 129702c..488213e 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -482,7 +482,7 @@ static void ks7010_rw_function(struct work_struct *work)
DPRINTK(4, "\n");
- /* wiat after DOZE */
+ /* wait after DOZE */
if (time_after(priv->last_doze + ((30 * HZ) / 1000), jiffies)) {
DPRINTK(4, "wait after DOZE\n");
queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
@@ -490,11 +490,9 @@ static void ks7010_rw_function(struct work_struct *work)
return;
}
- /* wiat after WAKEUP */
+ /* wait after WAKEUP */
while (time_after(priv->last_wakeup + ((30 * HZ) / 1000), jiffies)) {
DPRINTK(4, "wait after WAKEUP\n");
-/* queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,&priv->ks_wlan_hw.rw_wq,
- (priv->last_wakeup + ((30*HZ)/1000) - jiffies));*/
dev_info(&priv->ks_wlan_hw.sdio_card->func->dev,
"wake: %lu %lu\n",
priv->last_wakeup + (30 * HZ) / 1000,
@@ -634,7 +632,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
ks_wlan_hw_wakeup_request(priv);
queue_delayed_work
(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
+ &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] 11+ messages in thread
* [PATCH 3/9] staging: ks7010: Fix checkpatch warning
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
2017-02-20 4:23 ` [PATCH 1/9] staging: ks7010: Fix checkpatch warnings Tobin C. Harding
2017-02-20 4:23 ` [PATCH 2/9] staging: ks7010: Remove dead code Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 4/9] " Tobin C. Harding
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Checkpatch emits warning because string is split over two lines.
Concatenate string onto single line.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 488213e..2992eca 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -996,8 +996,8 @@ static int ks7010_sdio_probe(struct sdio_func *func,
sdio_set_drvdata(func, card);
- DPRINTK(5, "class = 0x%X, vendor = 0x%X, "
- "device = 0x%X\n", func->class, func->vendor, func->device);
+ DPRINTK(5, "class = 0x%X, vendor = 0x%X, device = 0x%X\n",
+ func->class, func->vendor, func->device);
/* private memory allocate */
netdev = alloc_etherdev(sizeof(*priv));
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/9] staging: ks7010: Fix checkpatch warning
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (2 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 3/9] staging: ks7010: Fix checkpatch warning Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 5/9] staging: ks7010: refactor, change retval to ret Tobin C. Harding
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Checkpatch emits WARNING: Too many leading tabs - consider code
refactoring. Code in question is nested under two if statements.
Outside of if statements there is the return statement. If statement
conditionals can be inverted and function made to return immediately
without changing program logic.
Invert conditionals. Also change && to ||. Return immediately if new
conditionals are true. Do not touch remaining code except to remove
indentation.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 122 +++++++++++++++++------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 2992eca..06d1037 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -176,71 +176,71 @@ 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;
}
@@ -632,7 +632,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
ks_wlan_hw_wakeup_request(priv);
queue_delayed_work
(priv->ks_wlan_hw.ks7010sdio_wq,
- &priv->ks_wlan_hw.rw_wq, 1);
+ &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] 11+ messages in thread
* [PATCH 5/9] staging: ks7010: refactor, change retval to ret
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (3 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 4/9] " Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 6/9] staging: ks7010: Refactor code Tobin C. Harding
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Code can be refactored to take advantage of indentation removal in
previous commit. Code is still nested quite deeply. Variable name
retval takes up six characters, this is significant because 80
character line limit is being hit frequently.
Change variable name retval to ret to get an extra three characters to
fit on each line.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 06d1037..29d332d 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -171,7 +171,7 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
{
unsigned char rw_data;
- int retval;
+ int ret;
if (priv->reg.powermgt == POWMGT_ACTIVE_MODE)
return 0;
@@ -196,11 +196,11 @@ 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 =
+ ret =
ks7010_sdio_read(priv, INT_PENDING,
&rw_data,
sizeof(rw_data));
- if (retval) {
+ if (ret) {
DPRINTK(1,
" error : INT_PENDING=%02X\n",
rw_data);
@@ -210,12 +210,12 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
}
if (!rw_data) {
rw_data = GCR_B_DOZE;
- retval =
+ ret =
ks7010_sdio_write(priv,
GCR_B,
&rw_data,
sizeof(rw_data));
- if (retval) {
+ if (ret) {
DPRINTK(1,
" error : GCR_B=%02X\n",
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] 11+ messages in thread
* [PATCH 6/9] staging: ks7010: Refactor code
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (4 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 5/9] staging: ks7010: refactor, change retval to ret Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-24 17:10 ` Greg Kroah-Hartman
2017-02-20 4:23 ` [PATCH 7/9] staging: ks7010: Remove level of indentation Tobin C. Harding
` (2 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Code may be refactored to take advantage of previous commit.
Refactor code. Introduce four new line over 80 warnings. Make code
more legible.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 31 +++++++++----------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 29d332d..4842b2d 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -196,40 +196,27 @@ 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)) {
- ret =
- ks7010_sdio_read(priv, INT_PENDING,
- &rw_data,
- sizeof(rw_data));
+ ret = ks7010_sdio_read(priv, INT_PENDING, &rw_data,
+ sizeof(rw_data));
if (ret) {
- 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);
break;
}
if (!rw_data) {
rw_data = GCR_B_DOZE;
- ret =
- ks7010_sdio_write(priv,
- GCR_B,
- &rw_data,
+ ret = ks7010_sdio_write(priv, GCR_B, &rw_data,
sizeof(rw_data));
if (ret) {
- 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);
break;
}
- 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);
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 7/9] staging: ks7010: Remove level of indentation
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (5 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 6/9] staging: ks7010: Refactor code Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 8/9] staging: ks7010: Refactor code Tobin C. Harding
2017-02-20 4:23 ` [PATCH 9/9] staging: ks7010: Fix checkpatch whitespace checks Tobin C. Harding
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Checkpatch emits warning that code is too deeply nested.
Remove one level of nesting by inverting if statement conditional
and using goto to maintain program logic.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 151 ++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 75 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 4842b2d..5a69468 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -544,93 +544,94 @@ static void ks_sdio_interrupt(struct sdio_func *func)
priv = card->priv;
DPRINTK(4, "\n");
- if (priv->dev_state >= DEVICE_STATE_BOOT) {
+ if (priv->dev_state < DEVICE_STATE_BOOT)
+ goto intr_out;
+
+ retval =
+ ks7010_sdio_read(priv, INT_PENDING, &status,
+ sizeof(status));
+ if (retval) {
+ DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
+ goto intr_out;
+ }
+ 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) {
retval =
- ks7010_sdio_read(priv, INT_PENDING, &status,
- sizeof(status));
+ ks7010_sdio_read(priv, GCR_B, &rw_data,
+ sizeof(rw_data));
if (retval) {
- DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
+ DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto intr_out;
}
- 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) {
- retval =
- ks7010_sdio_read(priv, GCR_B, &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
- goto intr_out;
- }
- /* 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) */
- retval =
- ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
- rw_data);
- goto intr_out;
- }
- DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
- rsize = rw_data & RSIZE_MASK;
- if (rsize) { /* Read schedule */
- ks_wlan_hw_rx((void *)priv,
- (uint16_t)(rsize << 4));
- }
- if (rw_data & WSTATUS_MASK) {
+ do {
+ /* read (WriteStatus/ReadDataSize FN1:00_0014) */
+ retval =
+ ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
+ sizeof(rw_data));
+ if (retval) {
+ DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
+ rw_data);
+ goto intr_out;
+ }
+ DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
+ rsize = rw_data & RSIZE_MASK;
+ if (rsize) { /* Read schedule */
+ ks_wlan_hw_rx((void *)priv,
+ (uint16_t)(rsize << 4));
+ }
+ if (rw_data & WSTATUS_MASK) {
#if 0
- if (status & INT_WRITE_STATUS
- && !cnt_txqbody(priv)) {
- /* dummy write for interrupt clear */
- rw_data = 0;
- retval =
- ks7010_sdio_write(priv, DATA_WINDOW,
- &rw_data,
- sizeof(rw_data));
- if (retval) {
- DPRINTK(1,
- "write DATA_WINDOW Failed!!(%d)\n",
- retval);
- }
- status &= ~INT_WRITE_STATUS;
- } else {
+ if (status & INT_WRITE_STATUS
+ && !cnt_txqbody(priv)) {
+ /* dummy write for interrupt clear */
+ rw_data = 0;
+ retval =
+ ks7010_sdio_write(priv, DATA_WINDOW,
+ &rw_data,
+ sizeof(rw_data));
+ if (retval) {
+ DPRINTK(1,
+ "write DATA_WINDOW Failed!!(%d)\n",
+ retval);
+ }
+ status &= ~INT_WRITE_STATUS;
+ } else {
#endif
- 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);
+ 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;
}
-#if 0
+ } else {
+ tx_device_task((void *)priv);
}
-#endif
+#if 0
}
- } while (rsize);
- }
+#endif
+ }
+ } while (rsize);
intr_out:
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] 11+ messages in thread
* [PATCH 8/9] staging: ks7010: Refactor code
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (6 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 7/9] staging: ks7010: Remove level of indentation Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
2017-02-20 4:23 ` [PATCH 9/9] staging: ks7010: Fix checkpatch whitespace checks Tobin C. Harding
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Code is able to be refactored. Various assignments have the assigning
value on a separate line.
Refactor code bring it closer to kernel standards.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 5a69468..3119ee2 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -547,9 +547,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
if (priv->dev_state < DEVICE_STATE_BOOT)
goto intr_out;
- retval =
- ks7010_sdio_read(priv, INT_PENDING, &status,
- sizeof(status));
+ retval = ks7010_sdio_read(priv, INT_PENDING, &status, sizeof(status));
if (retval) {
DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
goto intr_out;
@@ -563,19 +561,15 @@ 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));
+ retval = ks7010_sdio_read(priv, GCR_B, &rw_data, sizeof(rw_data));
if (retval) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto intr_out;
}
/* 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);
@@ -584,28 +578,23 @@ 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,
+ retval = ks7010_sdio_read(priv, WSTATUS_RSIZE, &rw_data,
sizeof(rw_data));
if (retval) {
- DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
- rw_data);
+ DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n", rw_data);
goto intr_out;
}
DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
rsize = rw_data & RSIZE_MASK;
if (rsize) { /* Read schedule */
- ks_wlan_hw_rx((void *)priv,
- (uint16_t)(rsize << 4));
+ ks_wlan_hw_rx((void *)priv, (uint16_t)(rsize << 4));
}
if (rw_data & WSTATUS_MASK) {
#if 0
- if (status & INT_WRITE_STATUS
- && !cnt_txqbody(priv)) {
+ if (status & INT_WRITE_STATUS && !cnt_txqbody(priv)) {
/* dummy write for interrupt clear */
rw_data = 0;
- retval =
- ks7010_sdio_write(priv, DATA_WINDOW,
+ retval = ks7010_sdio_write(priv, DATA_WINDOW,
&rw_data,
sizeof(rw_data));
if (retval) {
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 9/9] staging: ks7010: Fix checkpatch whitespace checks
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
` (7 preceding siblings ...)
2017-02-20 4:23 ` [PATCH 8/9] staging: ks7010: Refactor code Tobin C. Harding
@ 2017-02-20 4:23 ` Tobin C. Harding
8 siblings, 0 replies; 11+ messages in thread
From: Tobin C. Harding @ 2017-02-20 4:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Greg Kroah-Hartman, driverdev-devel
Checkpatch emits two CHECK messages;
CHECK: Logical continuations should be on the previous line
CHECK: Alignment should match open parenthesis
Move logical continuations onto the previous line. Add whitespace to
align code with parenthesis.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
drivers/staging/ks7010/ks7010_sdio.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 3119ee2..a1e863f 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -193,15 +193,15 @@ 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)) {
ret = ks7010_sdio_read(priv, INT_PENDING, &rw_data,
sizeof(rw_data));
if (ret) {
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);
break;
}
if (!rw_data) {
@@ -219,12 +219,12 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
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);
}
break;
}
@@ -312,8 +312,8 @@ static void tx_device_task(void *dev)
int rc = 0;
DPRINTK(4, "\n");
- if (cnt_txqbody(priv) > 0
- && atomic_read(&priv->psstatus.status) != PS_SNOOZE) {
+ if (cnt_txqbody(priv) > 0 &&
+ atomic_read(&priv->psstatus.status) != PS_SNOOZE) {
sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead];
if (priv->dev_state >= DEVICE_STATE_BOOT) {
rc = write_to_device(priv, sp->sendp, sp->size);
@@ -559,9 +559,11 @@ static void ks_sdio_interrupt(struct sdio_func *func)
/* 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) {
- retval = ks7010_sdio_read(priv, GCR_B, &rw_data, sizeof(rw_data));
+ 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) {
DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
goto intr_out;
@@ -579,7 +581,7 @@ 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));
+ sizeof(rw_data));
if (retval) {
DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n", rw_data);
goto intr_out;
@@ -595,8 +597,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
/* dummy write for interrupt clear */
rw_data = 0;
retval = ks7010_sdio_write(priv, DATA_WINDOW,
- &rw_data,
- sizeof(rw_data));
+ &rw_data,
+ sizeof(rw_data));
if (retval) {
DPRINTK(1,
"write DATA_WINDOW Failed!!(%d)\n",
@@ -855,7 +857,6 @@ static void ks7010_card_init(struct ks_wlan_private *priv)
if (priv->mac_address_valid && priv->version_size)
priv->dev_state = DEVICE_STATE_PREINIT;
-
hostif_sme_enqueue(priv, SME_GET_EEPROM_CKSUM);
/* load initial wireless parameter */
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6/9] staging: ks7010: Refactor code
2017-02-20 4:23 ` [PATCH 6/9] staging: ks7010: Refactor code Tobin C. Harding
@ 2017-02-24 17:10 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2017-02-24 17:10 UTC (permalink / raw)
To: Tobin C. Harding; +Cc: driverdev-devel, Wolfram Sang
On Mon, Feb 20, 2017 at 03:23:31PM +1100, Tobin C. Harding wrote:
> Code may be refactored to take advantage of previous commit.
>
> Refactor code. Introduce four new line over 80 warnings. Make code
> more legible.
>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
> drivers/staging/ks7010/ks7010_sdio.c | 31 +++++++++----------------------
> 1 file changed, 9 insertions(+), 22 deletions(-)
You have at least two different commits in this patch series with the
same identical subject line :(
Please fix them all up and make them unique, and a bit more explicit as
to what is going on. "Refactor code" is not good...
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-02-24 17:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 4:23 [PATCH 0/9] staging: ks7010: Checkpatch fixes Tobin C. Harding
2017-02-20 4:23 ` [PATCH 1/9] staging: ks7010: Fix checkpatch warnings Tobin C. Harding
2017-02-20 4:23 ` [PATCH 2/9] staging: ks7010: Remove dead code Tobin C. Harding
2017-02-20 4:23 ` [PATCH 3/9] staging: ks7010: Fix checkpatch warning Tobin C. Harding
2017-02-20 4:23 ` [PATCH 4/9] " Tobin C. Harding
2017-02-20 4:23 ` [PATCH 5/9] staging: ks7010: refactor, change retval to ret Tobin C. Harding
2017-02-20 4:23 ` [PATCH 6/9] staging: ks7010: Refactor code Tobin C. Harding
2017-02-24 17:10 ` Greg Kroah-Hartman
2017-02-20 4:23 ` [PATCH 7/9] staging: ks7010: Remove level of indentation Tobin C. Harding
2017-02-20 4:23 ` [PATCH 8/9] staging: ks7010: Refactor code Tobin C. Harding
2017-02-20 4:23 ` [PATCH 9/9] staging: ks7010: Fix checkpatch whitespace checks 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.