[PATCH 00/10] staging: ks7010: audit return statements

[PATCH 00/10] staging: ks7010: audit return statements

[Tobin C. Harding]

Driver return statement code exhibits a degree of uncleanliness.

Driver return statement code is non-uniform in its use of
identifiers, both variables and goto labels.

goto statements occasionally are used with out clean up code instead
of returning directly.

Variables are defined (at declaration time) to zero unnecessarily.

Code could be cleaned up by an audit of all return statement code.

Initial patches do general checkpatch cleanups.

Patch 01, patch 02, and patch 03 are white space fixes, separated by
checkpatch type.

Patch 04 fixes unbalanced braces.

Patch 05 removes multiple line assignments.

Patch 06 removes usage of goto statements with out clean up code.

Patch 07 renames goto labels to be more informative and uniform.

Patch 08 removes not-zero comparison i.e if (foo != 0).

Patch 09 removes comparison to zero i.e if (foo == 0) if it does not
add information to the statement.

Patch 10 renames identifiers 'rc' and 'retval' to 'ret' in order to be
uniform across the driver.

Tobin C. Harding (10):
  staging: ks7010: fix checkpatch LINE_SPACING
  staging: ks7010: fix checkpatch SPACING
  staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT
  staging: ks7010: fix checkpatch BRACES
  staging: ks7010: fix checkpatch MULTIPLE_ASSIGNMENTS
  staging: ks7010: return directly on error
  staging: ks7010: make goto labels uniform
  staging: ks7010: remove non-zero comparison
  staging: ks7010: remove zero comparison
  staging: ks7010: rename return value identifier

 drivers/staging/ks7010/ks7010_sdio.c | 226 +++++++++++++++++------------------
 drivers/staging/ks7010/ks_hostif.c   |  47 ++++----
 drivers/staging/ks7010/ks_wlan_net.c |  38 +++---
 drivers/staging/ks7010/michael_mic.c |   4 +-
 4 files changed, 155 insertions(+), 160 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/10] staging: ks7010: fix checkpatch LINE_SPACING

[Tobin C. Harding]

Checkpatch emits CHECK: Please use a blank line after
function/struct/union/enum declarations.

Add blank line after function definition.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks_wlan_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 1b7027c..f6f7ffd 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2360,6 +2360,7 @@ static int ks_wlan_get_sleep_mode(struct net_device *dev,
 
 	return 0;
 }
+
 #ifdef WPS
 
 static int ks_wlan_set_wps_enable(struct net_device *dev,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/10] staging: ks7010: fix checkpatch SPACING

[Tobin C. Harding]

Checkpatch emits CHECK: No space is necessary after a cast.

Remove unnecessary space after cast.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/michael_mic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/michael_mic.c b/drivers/staging/ks7010/michael_mic.c
index de5e724..80497ef 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -141,8 +141,8 @@ void MichaelMICFunction(struct michael_mic_t *Mic, u8 *Key,
 	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
 	 */
 	MichaelInitializeFunction(Mic, Key);
-	MichaelAppend(Mic, (uint8_t *) Data, 12);	/* |DA|SA| */
+	MichaelAppend(Mic, (uint8_t *)Data, 12);	/* |DA|SA| */
 	MichaelAppend(Mic, pad_data, 4);	/* |Priority|0|0|0| */
-	MichaelAppend(Mic, (uint8_t *) (Data + 12), Len - 12);	/* |Data| */
+	MichaelAppend(Mic, (uint8_t *)(Data + 12), Len - 12);	/* |Data| */
 	MichaelGetMIC(Mic, Result);
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/10] staging: ks7010: fix checkpatch PARENTHESIS_ALIGNMENT

[Tobin C. Harding]

Checkpatch emits CHECK: Alignment should match open parenthesis.

Fix alignment to open parenthesis in line with kernel coding style.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c |  4 ++--
 drivers/staging/ks7010/ks_wlan_net.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 0fa13cd..1d3a15f 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -193,8 +193,8 @@ static int _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 		cnt_txqbody(priv));
 
 	if (!atomic_read(&priv->psstatus.confirm_wait) &&
-		!atomic_read(&priv->psstatus.snooze_guard) &&
-		!cnt_txqbody(priv)) {
+	    !atomic_read(&priv->psstatus.snooze_guard) &&
+	    !cnt_txqbody(priv)) {
 		retval = ks7010_sdio_read(priv, INT_PENDING, &rw_data,
 					  sizeof(rw_data));
 		if (retval) {
diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index f6f7ffd..f18ff56 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1806,11 +1806,11 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
 		commit |= SME_WEP_INDEX;
 	} else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) {
 		memcpy(&priv->wpa.key[index].rx_seq[0],
-			enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
+		       enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
 	}
 
 	memcpy(&priv->wpa.key[index].addr.sa_data[0],
-		&enc->addr.sa_data[0], ETH_ALEN);
+	       &enc->addr.sa_data[0], ETH_ALEN);
 
 	switch (enc->alg) {
 	case IW_ENCODE_ALG_NONE:
@@ -1829,7 +1829,7 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
 		}
 		if (enc->key_len) {
 			memcpy(&priv->wpa.key[index].key_val[0],
-				&enc->key[0], enc->key_len);
+			       &enc->key[0], enc->key_len);
 			priv->wpa.key[index].key_len = enc->key_len;
 			commit |= (SME_WEP_VAL1 << index);
 		}
@@ -1841,19 +1841,19 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
 		}
 		if (enc->key_len == 32) {
 			memcpy(&priv->wpa.key[index].key_val[0],
-				&enc->key[0], enc->key_len - 16);
+			       &enc->key[0], enc->key_len - 16);
 			priv->wpa.key[index].key_len =
 				enc->key_len - 16;
 			if (priv->wpa.key_mgmt_suite == 4) {	/* WPA_NONE */
 				memcpy(&priv->wpa.key[index].
-					tx_mic_key[0], &enc->key[16], 8);
+				       tx_mic_key[0], &enc->key[16], 8);
 				memcpy(&priv->wpa.key[index].
-					rx_mic_key[0], &enc->key[16], 8);
+				       rx_mic_key[0], &enc->key[16], 8);
 			} else {
 				memcpy(&priv->wpa.key[index].
-					tx_mic_key[0], &enc->key[16], 8);
+				       tx_mic_key[0], &enc->key[16], 8);
 				memcpy(&priv->wpa.key[index].
-					rx_mic_key[0], &enc->key[24], 8);
+				       rx_mic_key[0], &enc->key[24], 8);
 			}
 			commit |= (SME_WEP_VAL1 << index);
 		}
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/10] staging: ks7010: fix checkpatch BRACES

[Tobin C. Harding]

Checkpatch emits CHECK: Unbalanced braces around else
statement. Statements in question are single statements so we do not
need braces. Checkpatch also warns about multiple line dereference for
this code.

Fix if/else/else if statement use of braces. Fix function argument layout
at the same time since it is the same statement.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks_hostif.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index db10e16..68e26f4 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2456,19 +2456,15 @@ void hostif_sme_execute(struct ks_wlan_private *priv, int event)
 		hostif_phy_information_request(priv);
 		break;
 	case SME_MIC_FAILURE_REQUEST:
-		if (priv->wpa.mic_failure.failure == 1) {
-			hostif_mic_failure_request(priv,
-						   priv->wpa.mic_failure.
-						   failure - 1, 0);
-		} else if (priv->wpa.mic_failure.failure == 2) {
-			hostif_mic_failure_request(priv,
-						   priv->wpa.mic_failure.
-						   failure - 1,
-						   priv->wpa.mic_failure.
-						   counter);
-		} else
-			DPRINTK(4,
-				"SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
+		if (priv->wpa.mic_failure.failure == 1)
+			hostif_mic_failure_request(
+				priv, priv->wpa.mic_failure.failure - 1, 0);
+		else if (priv->wpa.mic_failure.failure == 2)
+			hostif_mic_failure_request(
+				priv, priv->wpa.mic_failure.failure - 1,
+				priv->wpa.mic_failure.counter);
+		else
+			DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
 				priv->wpa.mic_failure.failure);
 		break;
 	case SME_MIC_FAILURE_CONFIRM:
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/10] staging: ks7010: fix checkpatch MULTIPLE_ASSIGNMENTS

[Tobin C. Harding]

Checkpatch emits CHECK: multiple assignments should be avoided.

Move multiple line assignment to individual lines.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks_hostif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 68e26f4..3eb0f2e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -2682,7 +2682,8 @@ int hostif_init(struct ks_wlan_private *priv)
 		INIT_LIST_HEAD(&priv->pmklist.pmk[i].list);
 
 	priv->sme_i.sme_status = SME_IDLE;
-	priv->sme_i.qhead = priv->sme_i.qtail = 0;
+	priv->sme_i.qhead = 0;
+	priv->sme_i.qtail = 0;
 #ifdef KS_WLAN_DEBUG
 	priv->sme_i.max_event_count = 0;
 #endif
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/10] staging: ks7010: return directly on error

[Tobin C. Harding]

Function uses goto label with no clean up code. In this case we
should just return directly.

Remove goto statement, return directly on error.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---

'/* length check fail */' comment added because preceding line is a
standard function return value check followed by single (un-braced)
statement. Following this with a return makes it look like there is a
bug (missing braces). Adding the comment will save the next (and
subsequent) developers from having to think through this.

 drivers/staging/ks7010/ks7010_sdio.c | 10 ++++------
 drivers/staging/ks7010/ks_wlan_net.c |  7 +++----
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 1d3a15f..3dab700 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -400,7 +400,7 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 	if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
 		/* in case of buffer overflow */
 		DPRINTK(1, "rx buffer overflow\n");
-		goto error_out;
+		return;
 	}
 	rx_buffer = &priv->rx_dev.rx_dev_buff[priv->rx_dev.qtail];
 
@@ -408,7 +408,7 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 	    ks7010_sdio_read(priv, DATA_WINDOW, &rx_buffer->data[0],
 			     hif_align_size(size));
 	if (retval)
-		goto error_out;
+		return;
 
 	/* length check */
 	if (size > 2046 || size == 0) {
@@ -426,7 +426,8 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 		if (retval)
 			DPRINTK(1, " error : READ_STATUS=%02X\n", read_status);
 
-		goto error_out;
+		/* length check fail */
+		return;
 	}
 
 	hdr = (struct hostif_hdr *)&rx_buffer->data[0];
@@ -453,9 +454,6 @@ static void ks_wlan_hw_rx(void *dev, uint16_t size)
 
 	/* rx_event_task((void *)priv); */
 	tasklet_schedule(&priv->ks_wlan_hw.rx_bh_task);
-
- error_out:
-	return;
 }
 
 static void ks7010_rw_function(struct work_struct *work)
diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index f18ff56..d96eed6 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -202,7 +202,6 @@ static int ks_wlan_set_freq(struct net_device *dev,
 {
 	struct ks_wlan_private *priv =
 	    (struct ks_wlan_private *)netdev_priv(dev);
-	int rc = -EINPROGRESS;	/* Call commit handler */
 
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
@@ -222,7 +221,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
 	}
 	/* Setting by channel number */
 	if ((fwrq->m > 1000) || (fwrq->e > 0)) {
-		rc = -EOPNOTSUPP;
+		return -EOPNOTSUPP;
 	} else {
 		int channel = fwrq->m;
 		/* We should do a better check than that,
@@ -232,7 +231,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
 			netdev_dbg(dev,
 				   "%s: New channel value of %d is invalid!\n",
 				   dev->name, fwrq->m);
-			rc = -EINVAL;
+			return -EINVAL;
 		} else {
 			/* Yes ! We can set it !!! */
 			priv->reg.channel = (u8)(channel);
@@ -240,7 +239,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
 		}
 	}
 
-	return rc;
+	return -EINPROGRESS;	/* Call commit handler */
 }
 
 static int ks_wlan_get_freq(struct net_device *dev,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/10] staging: ks7010: make goto labels uniform

[Tobin C. Harding]

Driver uses different label forms for similar purposes. It would be
more clear if single use case has uniform label. 'out' is generic and
adds no meaning to label.

Documentation/process/coding-style.rst:
Choose label names which say what the goto does or why the goto
exists.

Rename labels so as to better describe what they do. If an execution
path only exists for the label on an error, prefix the label with
'err_'. If a non-error execution path exist do not use prefix.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 70 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 3dab700..40ec028 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -102,7 +102,7 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
 		if (retval) {
 			DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
-			goto out;
+			goto set_sleep_mode;
 		}
 		DPRINTK(4, "PMG SET!! : GCR_B=%02X\n", rw_data);
 		DPRINTK(3, "sleep_mode=SLP_SLEEP\n");
@@ -112,7 +112,7 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 		DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
 	}
 
- out:
+set_sleep_mode:
 	priv->sleep_mode = atomic_read(&priv->sleepstatus.status);
 }
 
@@ -132,7 +132,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
 		if (retval) {
 			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
-			goto out;
+			goto set_sleep_mode;
 		}
 		DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
 		atomic_set(&priv->sleepstatus.status, 0);
@@ -142,7 +142,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 		DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
 	}
 
- out:
+set_sleep_mode:
 	priv->sleep_mode = atomic_read(&priv->sleepstatus.status);
 }
 
@@ -495,18 +495,18 @@ static void ks7010_rw_function(struct work_struct *work)
 			queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
 					   &priv->ks_wlan_hw.rw_wq, 1);
 		}
-		goto err_out;
+		goto err_release_host;
 	}
 
 	/* sleep mode doze */
 	if (atomic_read(&priv->sleepstatus.doze_request) == 1) {
 		ks_wlan_hw_sleep_doze_request(priv);
-		goto err_out;
+		goto err_release_host;
 	}
 	/* sleep mode wakeup */
 	if (atomic_read(&priv->sleepstatus.wakeup_request) == 1) {
 		ks_wlan_hw_sleep_wakeup_request(priv);
-		goto err_out;
+		goto err_release_host;
 	}
 
 	/* read (WriteStatus/ReadDataSize FN1:00_0014) */
@@ -515,7 +515,7 @@ static void ks7010_rw_function(struct work_struct *work)
 	if (retval) {
 		DPRINTK(1, " error : WSTATUS_RSIZE=%02X psstatus=%d\n", rw_data,
 			atomic_read(&priv->psstatus.status));
-		goto err_out;
+		goto err_release_host;
 	}
 	DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
 
@@ -528,7 +528,7 @@ static void ks7010_rw_function(struct work_struct *work)
 
 	_ks_wlan_hw_power_save(priv);
 
- err_out:
+err_release_host:
 	sdio_release_host(priv->ks_wlan_hw.sdio_card->func);
 }
 
@@ -549,7 +549,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 				     sizeof(status));
 		if (retval) {
 			DPRINTK(1, "read INT_PENDING Failed!!(%d)\n", retval);
-			goto intr_out;
+			goto queue_delayed_work;
 		}
 		DPRINTK(4, "INT_PENDING=%02X\n", rw_data);
 
@@ -565,7 +565,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 					     sizeof(rw_data));
 			if (retval) {
 				DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
-				goto intr_out;
+				goto queue_delayed_work;
 			}
 			/* DPRINTK(1, "GCR_B=%02X\n", rw_data); */
 			if (rw_data == GCR_B_ACTIVE) {
@@ -587,7 +587,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 			if (retval) {
 				DPRINTK(1, " error : WSTATUS_RSIZE=%02X\n",
 					rw_data);
-				goto intr_out;
+				goto queue_delayed_work;
 			}
 			DPRINTK(4, "WSTATUS_RSIZE=%02X\n", rw_data);
 			rsize = rw_data & RSIZE_MASK;
@@ -613,7 +613,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 		} while (rsize);
 	}
 
- intr_out:
+queue_delayed_work:
 	queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
 			   &priv->ks_wlan_hw.rw_wq, 0);
 }
@@ -666,15 +666,15 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 	memcpy(data_buf, &index, sizeof(index));
 	rc = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
 	if (rc)
-		goto error_out;
+		goto err_free_data_buf;
 
 	rc = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
 	if (rc)
-		goto error_out;
+		goto err_free_data_buf;
 
 	return 0;
 
- error_out:
+err_free_data_buf:
 	kfree(data_buf);
 
 	return rc;
@@ -693,17 +693,17 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 
 	rc = ks7010_sdio_read(priv, address, read_buf, size);
 	if (rc)
-		goto error_out;
+		goto err_free_read_buf;
 
 	rc = memcmp(data, read_buf, size);
 	if (rc) {
 		DPRINTK(0, "data compare error (%d)\n", rc);
-		goto error_out;
+		goto err_free_read_buf;
 	}
 
 	return 0;
 
- error_out:
+err_free_read_buf:
 	kfree(read_buf);
 
 	return rc;
@@ -930,20 +930,20 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	ret = sdio_enable_func(func);
 	DPRINTK(5, "sdio_enable_func() %d\n", ret);
 	if (ret)
-		goto error_free_card;
+		goto err_free_card;
 
 	/* interrupt disable */
 	sdio_writeb(func, 0, INT_ENABLE, &ret);
 	if (ret)
-		goto error_free_card;
+		goto err_free_card;
 	sdio_writeb(func, 0xff, INT_PENDING, &ret);
 	if (ret)
-		goto error_disable_func;
+		goto err_disable_func;
 
 	/* setup interrupt handler */
 	ret = sdio_claim_irq(func, ks_sdio_interrupt);
 	if (ret)
-		goto error_disable_func;
+		goto err_disable_func;
 
 	sdio_release_host(func);
 
@@ -956,12 +956,12 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	netdev = alloc_etherdev(sizeof(*priv));
 	if (!netdev) {
 		dev_err(&card->func->dev, "ks7010 : Unable to alloc new net device\n");
-		goto error_release_irq;
+		goto err_release_irq;
 	}
 	if (dev_alloc_name(netdev, "wlan%d") < 0) {
 		dev_err(&card->func->dev,
 			"ks7010 :  Couldn't get name!\n");
-		goto error_free_netdev;
+		goto err_free_netdev;
 	}
 
 	priv = netdev_priv(netdev);
@@ -975,7 +975,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	priv->ks_wlan_hw.read_buf = NULL;
 	priv->ks_wlan_hw.read_buf = kmalloc(RX_DATA_SIZE, GFP_KERNEL);
 	if (!priv->ks_wlan_hw.read_buf)
-		goto error_free_netdev;
+		goto err_free_netdev;
 
 	priv->dev_state = DEVICE_STATE_PREBOOT;
 	priv->net_dev = netdev;
@@ -1003,7 +1003,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 		dev_err(&card->func->dev,
 			"ks7010: firmware load failed !! return code = %d\n",
 			 ret);
-		goto error_free_read_buf;
+		goto err_free_read_buf;
 	}
 
 	/* interrupt setting */
@@ -1023,7 +1023,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	ret = ks7010_sdio_write(priv, INT_ENABLE, &rw_data, sizeof(rw_data));
 	sdio_release_host(func);
 	if (ret)
-		DPRINTK(1, " error : INT_ENABLE=%02X\n", rw_data);
+		DPRINTK(1, " err : INT_ENABLE=%02X\n", rw_data);
 
 	DPRINTK(4, " enable Interrupt : INT_ENABLE=%02X\n", rw_data);
 	priv->dev_state = DEVICE_STATE_BOOT;
@@ -1031,7 +1031,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	priv->ks_wlan_hw.ks7010sdio_wq = create_workqueue("ks7010sdio_wq");
 	if (!priv->ks_wlan_hw.ks7010sdio_wq) {
 		DPRINTK(1, "create_workqueue failed !!\n");
-		goto error_free_read_buf;
+		goto err_free_read_buf;
 	}
 
 	INIT_DELAYED_WORK(&priv->ks_wlan_hw.rw_wq, ks7010_rw_function);
@@ -1039,22 +1039,22 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 
 	ret = register_netdev(priv->net_dev);
 	if (ret)
-		goto error_free_read_buf;
+		goto err_free_read_buf;
 
 	return 0;
 
- error_free_read_buf:
+ err_free_read_buf:
 	kfree(priv->ks_wlan_hw.read_buf);
 	priv->ks_wlan_hw.read_buf = NULL;
- error_free_netdev:
+ err_free_netdev:
 	free_netdev(priv->net_dev);
 	card->priv = NULL;
- error_release_irq:
+ err_release_irq:
 	sdio_claim_host(func);
 	sdio_release_irq(func);
- error_disable_func:
+ err_disable_func:
 	sdio_disable_func(func);
- error_free_card:
+ err_free_card:
 	sdio_release_host(func);
 	sdio_set_drvdata(func, NULL);
 	kfree(card);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/10] staging: ks7010: remove non-zero comparison

[Tobin C. Harding]

Comparison, does not equal zero, is redundant

'if (foo != 0)'  is equal to  'if (foo)'

Typical kernel coding style is to use the shorter form.

Remove unnecessary non-zero comparison.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 4 ++--
 drivers/staging/ks7010/ks_hostif.c   | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 40ec028..8823e93 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -61,7 +61,7 @@ static int ks7010_sdio_read(struct ks_wlan_private *priv, unsigned int address,
 	else	/* CMD53 multi-block transfer */
 		rc = sdio_memcpy_fromio(card->func, buffer, address, length);
 
-	if (rc != 0)
+	if (rc)
 		DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
 
 	return rc;
@@ -80,7 +80,7 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
 	else	/* CMD53 */
 		rc = sdio_memcpy_toio(card->func, address, buffer, length);
 
-	if (rc != 0)
+	if (rc)
 		DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
 
 	return rc;
diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 3eb0f2e..83cda1f 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -528,7 +528,7 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
 	mib_val_size = get_WORD(priv);	/* MIB value size */
 	mib_val_type = get_WORD(priv);	/* MIB value type */
 
-	if (mib_status != 0) {
+	if (mib_status) {
 		/* in case of error */
 		DPRINTK(1, "attribute=%08X, status=%08X\n", mib_attribute,
 			mib_status);
@@ -604,7 +604,7 @@ void hostif_mib_set_confirm(struct ks_wlan_private *priv)
 	mib_status = get_DWORD(priv);	/* MIB Status */
 	mib_attribute = get_DWORD(priv);	/* MIB attribute */
 
-	if (mib_status != 0) {
+	if (mib_status) {
 		/* in case of error */
 		DPRINTK(1, "error :: attribute=%08X, status=%08X\n",
 			mib_attribute, mib_status);
@@ -834,7 +834,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
 	DPRINTK(3, "scan_ind_count = %d\n", priv->scan_ind_count);
 	ap_info = (struct ap_info_t *)(priv->rxp);
 
-	if (priv->scan_ind_count != 0) {
+	if (priv->scan_ind_count) {
 		for (i = 0; i < priv->aplist.size; i++) {	/* bssid check */
 			if (!memcmp
 			    (ap_info->bssid,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/10] staging: ks7010: remove zero comparison

[Tobin C. Harding]

Comparison, equal to zero, is redundant

'if (foo == 0)'  is equal to  'if (!foo)'

Typical kernel coding style is to use the shorter form.

Remove unnecessary zero comparison.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks_wlan_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index d96eed6..7d4c7f3 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -279,7 +279,7 @@ static int ks_wlan_set_essid(struct net_device *dev,
 
 	/* for SLEEP MODE */
 	/* Check if we asked for `any' */
-	if (dwrq->flags == 0) {
+	if (!dwrq->flags) {
 		/* Just send an empty SSID list */
 		memset(priv->reg.ssid.body, 0, sizeof(priv->reg.ssid.body));
 		priv->reg.ssid.size = 0;
@@ -1531,7 +1531,7 @@ static int ks_wlan_get_scan(struct net_device *dev,
 		return -EAGAIN;
 	}
 
-	if (priv->aplist.size == 0) {
+	if (!priv->aplist.size) {
 		/* Client error, no scan results...
 		 * The caller need to restart the scan.
 		 */
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/10] staging: ks7010: rename return value identifier

[Tobin C. Harding]

Driver uses multiple identifier names for the same task (retval, ret,
rc). It would be easier to read the code if a single task is
identified with a single name. 'ret' is the most common return value
identifier name found in the kernel tree, following the principle of
least surprise using 'ret' is a decent choice.

Rename rc -> ret
Rename retval -> ret

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 142 +++++++++++++++++------------------
 drivers/staging/ks7010/ks_hostif.c   |  16 ++--
 drivers/staging/ks7010/ks_wlan_net.c |  10 +--
 3 files changed, 84 insertions(+), 84 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 8823e93..28b91be 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -52,44 +52,48 @@ static int ks7010_sdio_read(struct ks_wlan_private *priv, unsigned int address,
 			    unsigned char *buffer, int length)
 {
 	struct ks_sdio_card *card;
-	int rc;
+	int ret;
 
 	card = priv->ks_wlan_hw.sdio_card;
 
 	if (length == 1)	/* CMD52 */
-		*buffer = sdio_readb(card->func, address, &rc);
+		*buffer = sdio_readb(card->func, address, &ret);
 	else	/* CMD53 multi-block transfer */
-		rc = sdio_memcpy_fromio(card->func, buffer, address, length);
+		ret = sdio_memcpy_fromio(card->func, buffer, address, length);
 
-	if (rc)
-		DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
+	if (ret) {
+		DPRINTK(1, "sdio error=%d size=%d\n", ret, length);
+		return ret;
+	}
 
-	return rc;
+	return 0;
 }
 
 static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
 			     unsigned char *buffer, int length)
 {
 	struct ks_sdio_card *card;
-	int rc;
+	int ret;
 
 	card = priv->ks_wlan_hw.sdio_card;
 
 	if (length == 1)	/* CMD52 */
-		sdio_writeb(card->func, *buffer, address, &rc);
+		sdio_writeb(card->func, *buffer, address, &ret);
 	else	/* CMD53 */
-		rc = sdio_memcpy_toio(card->func, address, buffer, length);
+		ret = sdio_memcpy_toio(card->func, address, buffer, length);
 
-	if (rc)
-		DPRINTK(1, "sdio error=%d size=%d\n", rc, length);
+	if (ret) {
+		DPRINTK(1, "sdio error=%d size=%d\n", ret, length);
+		return ret;
+	}
 
-	return rc;
+	return 0;
 }
 
 static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
+	int ret;
 
 	DPRINTK(4, "\n");
 
@@ -98,9 +102,8 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 
 	if (atomic_read(&priv->sleepstatus.status) == 0) {
 		rw_data = GCR_B_DOZE;
-		retval =
-		    ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
-		if (retval) {
+		ret = ks7010_sdio_write(priv, GCR_B, &rw_data, sizeof(rw_data));
+		if (ret) {
 			DPRINTK(1, " error : GCR_B=%02X\n", rw_data);
 			goto set_sleep_mode;
 		}
@@ -119,7 +122,7 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
+	int ret;
 
 	DPRINTK(4, "\n");
 
@@ -128,9 +131,8 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 
 	if (atomic_read(&priv->sleepstatus.status) == 1) {
 		rw_data = WAKEUP_REQ;
-		retval =
-		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
-		if (retval) {
+		ret = ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
+		if (ret) {
 			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
 			goto set_sleep_mode;
 		}
@@ -149,14 +151,13 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
 {
 	unsigned char rw_data;
-	int retval;
+	int ret;
 
 	DPRINTK(4, "\n");
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 		rw_data = WAKEUP_REQ;
-		retval =
-		    ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
-		if (retval)
+		ret = ks7010_sdio_write(priv, WAKEUP, &rw_data, sizeof(rw_data));
+		if (ret)
 			DPRINTK(1, " error : WAKEUP=%02X\n", rw_data);
 
 		DPRINTK(4, "wake up : WAKEUP=%02X\n", rw_data);
@@ -241,17 +242,17 @@ static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p,
 			 void *arg1, void *arg2)
 {
 	struct tx_device_buffer *sp;
-	int rc;
+	int ret;
 
 	if (priv->dev_state < DEVICE_STATE_BOOT) {
-		rc = -EPERM;
+		ret = -EPERM;
 		goto err_complete;
 	}
 
 	if ((TX_DEVICE_BUFF_SIZE - 1) <= cnt_txqbody(priv)) {
 		/* in case of buffer overflow */
 		DPRINTK(1, "tx buffer overflow\n");
-		rc = -EOVERFLOW;
+		ret = -EOVERFLOW;
 		goto err_complete;
 	}
 
@@ -270,7 +271,7 @@ static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p,
 	if (complete_handler)
 		(*complete_handler) (arg1, arg2);
 
-	return rc;
+	return ret;
 }
 
 /* write data */
@@ -279,7 +280,7 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer,
 {
 	unsigned char rw_data;
 	struct hostif_hdr *hdr;
-	int rc;
+	int ret;
 
 	hdr = (struct hostif_hdr *)buffer;
 
@@ -289,17 +290,17 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer,
 		return 0;
 	}
 
-	rc = ks7010_sdio_write(priv, DATA_WINDOW, buffer, size);
-	if (rc) {
-		DPRINTK(1, " write error : retval=%d\n", rc);
-		return rc;
+	ret = ks7010_sdio_write(priv, DATA_WINDOW, buffer, size);
+	if (ret) {
+		DPRINTK(1, " write error : retval=%d\n", ret);
+		return ret;
 	}
 
 	rw_data = WRITE_STATUS_BUSY;
-	rc = ks7010_sdio_write(priv, WRITE_STATUS, &rw_data, sizeof(rw_data));
-	if (rc) {
+	ret = ks7010_sdio_write(priv, WRITE_STATUS, &rw_data, sizeof(rw_data));
+	if (ret) {
 		DPRINTK(1, " error : WRITE_STATUS=%02X\n", rw_data);
-		return rc;
+		return ret;
 	}
 
 	return 0;
@@ -461,7 +462,7 @@ static void ks7010_rw_function(struct work_struct *work)
 	struct hw_info_t *hw;
 	struct ks_wlan_private *priv;
 	unsigned char rw_data;
-	int retval;
+	int ret;
 
 	hw = container_of(work, struct hw_info_t, rw_wq.work);
 	priv = container_of(hw, struct ks_wlan_private, ks_wlan_hw);
@@ -510,9 +511,8 @@ static void ks7010_rw_function(struct work_struct *work)
 	}
 
 	/* 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 psstatus=%d\n", rw_data,
 			atomic_read(&priv->psstatus.status));
 		goto err_release_host;
@@ -656,7 +656,7 @@ static void trx_device_exit(struct ks_wlan_private *priv)
 
 static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 {
-	int rc;
+	int ret;
 	unsigned char *data_buf;
 
 	data_buf = kmalloc(sizeof(u32), GFP_KERNEL);
@@ -664,12 +664,12 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 		return -ENOMEM;
 
 	memcpy(data_buf, &index, sizeof(index));
-	rc = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
-	if (rc)
+	ret = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
+	if (ret)
 		goto err_free_data_buf;
 
-	rc = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
-	if (rc)
+	ret = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
+	if (ret)
 		goto err_free_data_buf;
 
 	return 0;
@@ -677,27 +677,27 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 err_free_data_buf:
 	kfree(data_buf);
 
-	return rc;
+	return ret;
 }
 
 #define ROM_BUFF_SIZE (64 * 1024)
 static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 				    unsigned char *data, unsigned int size)
 {
-	int rc;
+	int ret;
 	unsigned char *read_buf;
 
 	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
 	if (!read_buf)
 		return -ENOMEM;
 
-	rc = ks7010_sdio_read(priv, address, read_buf, size);
-	if (rc)
+	ret = ks7010_sdio_read(priv, address, read_buf, size);
+	if (ret)
 		goto err_free_read_buf;
 
-	rc = memcmp(data, read_buf, size);
-	if (rc) {
-		DPRINTK(0, "data compare error (%d)\n", rc);
+	ret = memcmp(data, read_buf, size);
+	if (ret) {
+		DPRINTK(0, "data compare error (%d)\n", ret);
 		goto err_free_read_buf;
 	}
 
@@ -706,7 +706,7 @@ static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
 err_free_read_buf:
 	kfree(read_buf);
 
-	return rc;
+	return ret;
 }
 
 static int ks7010_upload_firmware(struct ks_wlan_private *priv,
@@ -715,7 +715,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	unsigned int size, offset, n = 0;
 	unsigned char *rom_buf;
 	unsigned char rw_data = 0;
-	int rc;
+	int ret;
 	int length;
 	const struct firmware *fw_entry = NULL;
 
@@ -727,14 +727,15 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	sdio_claim_host(card->func);
 
 	/* Firmware running ? */
-	rc = ks7010_sdio_read(priv, GCR_A, &rw_data, sizeof(rw_data));
+	ret = ks7010_sdio_read(priv, GCR_A, &rw_data, sizeof(rw_data));
 	if (rw_data == GCR_A_RUN) {
 		DPRINTK(0, "MAC firmware running ...\n");
 		goto release_host_and_free;
 	}
 
-	rc = request_firmware(&fw_entry, ROM_FILE, &priv->ks_wlan_hw.sdio_card->func->dev);
-	if (rc)
+	ret = request_firmware(&fw_entry, ROM_FILE,
+			       &priv->ks_wlan_hw.sdio_card->func->dev);
+	if (ret)
 		goto release_host_and_free;
 
 	length = fw_entry->size;
@@ -755,19 +756,18 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 		memcpy(rom_buf, fw_entry->data + n, size);
 		/* Update write index */
 		offset = n;
-		rc = ks7010_sdio_update_index(priv,
-					      KS7010_IRAM_ADDRESS + offset);
-		if (rc)
+		ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + offset);
+		if (ret)
 			goto release_firmware;
 
 		/* Write data */
-		rc = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
-		if (rc)
+		ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
+		if (ret)
 			goto release_firmware;
 
 		/* compare */
-		rc = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, size);
-		if (rc)
+		ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, size);
+		if (ret)
 			goto release_firmware;
 
 		n += size;
@@ -776,8 +776,8 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 
 	/* Remap request */
 	rw_data = GCR_A_REMAP;
-	rc = ks7010_sdio_write(priv, GCR_A, &rw_data, sizeof(rw_data));
-	if (rc)
+	ret = ks7010_sdio_write(priv, GCR_A, &rw_data, sizeof(rw_data));
+	if (ret)
 		goto release_firmware;
 
 	DPRINTK(4, " REMAP Request : GCR_A=%02X\n", rw_data);
@@ -785,8 +785,8 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	/* Firmware running check */
 	for (n = 0; n < 50; ++n) {
 		mdelay(10);	/* wait_ms(10); */
-		rc = ks7010_sdio_read(priv, GCR_A, &rw_data, sizeof(rw_data));
-		if (rc)
+		ret = ks7010_sdio_read(priv, GCR_A, &rw_data, sizeof(rw_data));
+		if (ret)
 			goto release_firmware;
 
 		if (rw_data == GCR_A_RUN)
@@ -795,11 +795,11 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	DPRINTK(4, "firmware wakeup (%d)!!!!\n", n);
 	if ((50) <= n) {
 		DPRINTK(1, "firmware can't start\n");
-		rc = -EIO;
+		ret = -EIO;
 		goto release_firmware;
 	}
 
-	rc = 0;
+	ret = 0;
 
  release_firmware:
 	release_firmware(fw_entry);
@@ -807,7 +807,7 @@ static int ks7010_upload_firmware(struct ks_wlan_private *priv,
 	sdio_release_host(card->func);
 	kfree(rom_buf);
 
-	return rc;
+	return ret;
 }
 
 static void ks7010_card_init(struct ks_wlan_private *priv)
diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 83cda1f..460ab13 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -402,7 +402,7 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 	unsigned short eth_proto;
 	struct ieee802_1x_hdr *aa1x_hdr;
 	struct wpa_eapol_key *eap_key;
-	int rc;
+	int ret;
 
 	DPRINTK(3, "\n");
 
@@ -434,8 +434,8 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 
 	/*  for WPA */
 	if (auth_type != TYPE_DATA && priv->wpa.rsn_enabled) {
-		rc = hostif_data_indication_wpa(priv, auth_type);
-		if (rc)
+		ret = hostif_data_indication_wpa(priv, auth_type);
+		if (ret)
 			return;
 	}
 
@@ -1124,12 +1124,12 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 	struct ieee802_1x_hdr *aa1x_hdr;
 	struct wpa_eapol_key *eap_key;
 	struct ethhdr *eth;
-	int rc;
+	int ret;
 
 	packet_len = packet->len;
 	if (packet_len > ETH_FRAME_LEN) {
 		DPRINTK(1, "bad length packet_len=%d\n", packet_len);
-		rc = -EOVERFLOW;
+		ret = -EOVERFLOW;
 		goto err_kfree_skb;
 	}
 
@@ -1157,7 +1157,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 
 	if (!pp) {
 		DPRINTK(3, "allocate memory failed..\n");
-		rc = -ENOMEM;
+		ret = -ENOMEM;
 		goto err_kfree_skb;
 	}
 
@@ -1171,7 +1171,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 	if (memcmp(&priv->eth_addr[0], eth->h_source, ETH_ALEN)) {
 		DPRINTK(1, "invalid mac address !!\n");
 		DPRINTK(1, "ethernet->h_source=%pM\n", eth->h_source);
-		rc = -ENXIO;
+		ret = -ENXIO;
 		goto err_kfree;
 	}
 
@@ -1281,7 +1281,7 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 err_kfree_skb:
 	dev_kfree_skb(packet);
 
-	return rc;
+	return ret;
 }
 
 #define ps_confirm_wait_inc(priv) do { \
diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index 7d4c7f3..3f9eba4 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2842,20 +2842,20 @@ static const struct iw_handler_def ks_wlan_handler_def = {
 static int ks_wlan_netdev_ioctl(struct net_device *dev, struct ifreq *rq,
 				int cmd)
 {
-	int rc = 0;
+	int ret;
 	struct iwreq *wrq = (struct iwreq *)rq;
 
 	switch (cmd) {
 	case SIOCIWFIRSTPRIV + 20:	/* KS_WLAN_SET_STOP_REQ */
-		rc = ks_wlan_set_stop_request(dev, NULL, &wrq->u.mode, NULL);
+		ret = ks_wlan_set_stop_request(dev, NULL, &wrq->u.mode, NULL);
 		break;
 		// All other calls are currently unsupported
 	default:
-		rc = -EOPNOTSUPP;
+		ret = -EOPNOTSUPP;
 	}
 
-	DPRINTK(5, "return=%d\n", rc);
-	return rc;
+	DPRINTK(5, "return=%d\n", ret);
+	return ret;
 }
 
 static
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/10] staging: ks7010: fix checkpatch BRACES

[Dan Carpenter]

On Tue, Mar 21, 2017 at 01:37:06PM +1100, Tobin C. Harding wrote:
> Checkpatch emits CHECK: Unbalanced braces around else
> statement. Statements in question are single statements so we do not
> need braces. Checkpatch also warns about multiple line dereference for
> this code.
> 
> Fix if/else/else if statement use of braces. Fix function argument layout
> at the same time since it is the same statement.
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  drivers/staging/ks7010/ks_hostif.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> index db10e16..68e26f4 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -2456,19 +2456,15 @@ void hostif_sme_execute(struct ks_wlan_private *priv, int event)
>  		hostif_phy_information_request(priv);
>  		break;
>  	case SME_MIC_FAILURE_REQUEST:
> -		if (priv->wpa.mic_failure.failure == 1) {
> -			hostif_mic_failure_request(priv,
> -						   priv->wpa.mic_failure.
> -						   failure - 1, 0);
> -		} else if (priv->wpa.mic_failure.failure == 2) {
> -			hostif_mic_failure_request(priv,
> -						   priv->wpa.mic_failure.
> -						   failure - 1,
> -						   priv->wpa.mic_failure.
> -						   counter);
> -		} else
> -			DPRINTK(4,
> -				"SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
> +		if (priv->wpa.mic_failure.failure == 1)
> +			hostif_mic_failure_request(
> +				priv, priv->wpa.mic_failure.failure - 1, 0);
> +		else if (priv->wpa.mic_failure.failure == 2)
> +			hostif_mic_failure_request(
> +				priv, priv->wpa.mic_failure.failure - 1,
> +				priv->wpa.mic_failure.counter);
> +		else
> +			DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
>  				priv->wpa.mic_failure.failure);

No.  This isn't nice.

Multi-line indents get curly braces generally for readability.  It's
better to go over the 80 character limit here.


		if (priv->wpa.mic_failure.failure == 1) {
			hostif_mic_failure_request(priv,
						   priv->wpa.mic_failure.failure - 1,
						   0);
		} else if priv->wpa.mic_failure.failure == 2) {
			hostif_mic_failure_request(priv,
						   priv->wpa.mic_failure.failure - 1,
						   priv->wpa.mic_failure.counter);
		} else {
			DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
				priv->wpa.mic_failure.failure);
		}

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/10] staging: ks7010: remove zero comparison

[Dan Carpenter]

On Tue, Mar 21, 2017 at 01:37:11PM +1100, Tobin C. Harding wrote:
> Comparison, equal to zero, is redundant
> 
> 'if (foo == 0)'  is equal to  'if (!foo)'
> 
> Typical kernel coding style is to use the shorter form.
> 
> Remove unnecessary zero comparison.

Not exactly.  If you're talking about the number zero then == 0 and != 0
are good style.  "if (size == 0) ".  Other times we're talking about
error codes or whatever and not the number zero so it should be
"if (ret) ".  Also for strcmp() functions, please use != 0 and == 0.

	if (strcmp(foo, bar) != 0)  <-- read this as "foo != bar"
	if (strcmp(foo, bar) == 0)  <-- read this as "foo == bar"
	if (strcmp(foo, bar) < 0)  <-- read this as "foo < bar"

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/10] staging: ks7010: rename return value identifier

[Dan Carpenter]

On Tue, Mar 21, 2017 at 01:37:12PM +1100, Tobin C. Harding wrote:
>  static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
>  				    unsigned char *data, unsigned int size)
>  {
> -	int rc;
> +	int ret;
>  	unsigned char *read_buf;
>  
>  	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
>  	if (!read_buf)
>  		return -ENOMEM;
>  
> -	rc = ks7010_sdio_read(priv, address, read_buf, size);
> -	if (rc)
> +	ret = ks7010_sdio_read(priv, address, read_buf, size);
> +	if (ret)
>  		goto err_free_read_buf;
>  
> -	rc = memcmp(data, read_buf, size);
> -	if (rc) {
> -		DPRINTK(0, "data compare error (%d)\n", rc);
> +	ret = memcmp(data, read_buf, size);
> +	if (ret) {

You didn't introduce this, but this is a bug.  memcpy() doesn't return
error codes.  Could you fix it in a follow on patch?

regards,
dan carpenter

Re: [PATCH 04/10] staging: ks7010: fix checkpatch BRACES

[Tobin C. Harding]

On Tue, Mar 21, 2017 at 03:36:34PM +0300, Dan Carpenter wrote:
> On Tue, Mar 21, 2017 at 01:37:06PM +1100, Tobin C. Harding wrote:
> > Checkpatch emits CHECK: Unbalanced braces around else
> > statement. Statements in question are single statements so we do not
> > need braces. Checkpatch also warns about multiple line dereference for
> > this code.
> > 
> > Fix if/else/else if statement use of braces. Fix function argument layout
> > at the same time since it is the same statement.
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  drivers/staging/ks7010/ks_hostif.c | 22 +++++++++-------------
> >  1 file changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> > index db10e16..68e26f4 100644
> > --- a/drivers/staging/ks7010/ks_hostif.c
> > +++ b/drivers/staging/ks7010/ks_hostif.c
> > @@ -2456,19 +2456,15 @@ void hostif_sme_execute(struct ks_wlan_private *priv, int event)
> >  		hostif_phy_information_request(priv);
> >  		break;
> >  	case SME_MIC_FAILURE_REQUEST:
> > -		if (priv->wpa.mic_failure.failure == 1) {
> > -			hostif_mic_failure_request(priv,
> > -						   priv->wpa.mic_failure.
> > -						   failure - 1, 0);
> > -		} else if (priv->wpa.mic_failure.failure == 2) {
> > -			hostif_mic_failure_request(priv,
> > -						   priv->wpa.mic_failure.
> > -						   failure - 1,
> > -						   priv->wpa.mic_failure.
> > -						   counter);
> > -		} else
> > -			DPRINTK(4,
> > -				"SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
> > +		if (priv->wpa.mic_failure.failure == 1)
> > +			hostif_mic_failure_request(
> > +				priv, priv->wpa.mic_failure.failure - 1, 0);
> > +		else if (priv->wpa.mic_failure.failure == 2)
> > +			hostif_mic_failure_request(
> > +				priv, priv->wpa.mic_failure.failure - 1,
> > +				priv->wpa.mic_failure.counter);
> > +		else
> > +			DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
> >  				priv->wpa.mic_failure.failure);
> 
> No.  This isn't nice.
> 
> Multi-line indents get curly braces generally for readability.  It's
> better to go over the 80 character limit here.
> 
> 
> 		if (priv->wpa.mic_failure.failure == 1) {
> 			hostif_mic_failure_request(priv,
> 						   priv->wpa.mic_failure.failure - 1,
> 						   0);
> 		} else if priv->wpa.mic_failure.failure == 2) {
> 			hostif_mic_failure_request(priv,
> 						   priv->wpa.mic_failure.failure - 1,
> 						   priv->wpa.mic_failure.counter);
> 		} else {
> 			DPRINTK(4, "SME_MIC_FAILURE_REQUEST: failure count=%u error?\n",
> 				priv->wpa.mic_failure.failure);
> 		}
> 
> regards,
> dan carpenter
> 

Ok, point noted, thank you.

Tobin

Re: [PATCH 09/10] staging: ks7010: remove zero comparison

[Tobin C. Harding]

On Tue, Mar 21, 2017 at 03:49:22PM +0300, Dan Carpenter wrote:
> On Tue, Mar 21, 2017 at 01:37:11PM +1100, Tobin C. Harding wrote:
> > Comparison, equal to zero, is redundant
> > 
> > 'if (foo == 0)'  is equal to  'if (!foo)'
> > 
> > Typical kernel coding style is to use the shorter form.
> > 
> > Remove unnecessary zero comparison.
> 
> Not exactly.  If you're talking about the number zero then == 0 and != 0
> are good style.  "if (size == 0) ".  Other times we're talking about
> error codes or whatever and not the number zero so it should be
> "if (ret) ".  Also for strcmp() functions, please use != 0 and == 0.
> 
> 	if (strcmp(foo, bar) != 0)  <-- read this as "foo != bar"
> 	if (strcmp(foo, bar) == 0)  <-- read this as "foo == bar"
> 	if (strcmp(foo, bar) < 0)  <-- read this as "foo < bar"
> 
> regards,
> dan carpenter
> 
> 

What is best for flags please?

-       if (dwrq->flags == 0) {
+       if (!dwrq->flags) {


thanks,
Tobin.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/10] staging: ks7010: rename return value identifier

[Tobin C. Harding]

On Tue, Mar 21, 2017 at 03:53:46PM +0300, Dan Carpenter wrote:
> On Tue, Mar 21, 2017 at 01:37:12PM +1100, Tobin C. Harding wrote:
> >  static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
> >  				    unsigned char *data, unsigned int size)
> >  {
> > -	int rc;
> > +	int ret;
> >  	unsigned char *read_buf;
> >  
> >  	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
> >  	if (!read_buf)
> >  		return -ENOMEM;
> >  
> > -	rc = ks7010_sdio_read(priv, address, read_buf, size);
> > -	if (rc)
> > +	ret = ks7010_sdio_read(priv, address, read_buf, size);
> > +	if (ret)
> >  		goto err_free_read_buf;
> >  
> > -	rc = memcmp(data, read_buf, size);
> > -	if (rc) {
> > -		DPRINTK(0, "data compare error (%d)\n", rc);
> > +	ret = memcmp(data, read_buf, size);
> > +	if (ret) {
> 
> You didn't introduce this, but this is a bug.  memcpy() doesn't return
> error codes.  Could you fix it in a follow on patch?

I have a sneaking suspicion that I may have introduced this in an
earlier patch. I'll fix it. Good spot, cheers.

thanks,
Tobin.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/10] staging: ks7010: remove zero comparison

[Dan Carpenter]

On Wed, Mar 22, 2017 at 10:22:24AM +1100, Tobin C. Harding wrote:
> On Tue, Mar 21, 2017 at 03:49:22PM +0300, Dan Carpenter wrote:
> > On Tue, Mar 21, 2017 at 01:37:11PM +1100, Tobin C. Harding wrote:
> > > Comparison, equal to zero, is redundant
> > > 
> > > 'if (foo == 0)'  is equal to  'if (!foo)'
> > > 
> > > Typical kernel coding style is to use the shorter form.
> > > 
> > > Remove unnecessary zero comparison.
> > 
> > Not exactly.  If you're talking about the number zero then == 0 and != 0
> > are good style.  "if (size == 0) ".  Other times we're talking about
> > error codes or whatever and not the number zero so it should be
> > "if (ret) ".  Also for strcmp() functions, please use != 0 and == 0.
> > 
> > 	if (strcmp(foo, bar) != 0)  <-- read this as "foo != bar"
> > 	if (strcmp(foo, bar) == 0)  <-- read this as "foo == bar"
> > 	if (strcmp(foo, bar) < 0)  <-- read this as "foo < bar"
> > 
> > regards,
> > dan carpenter
> > 
> > 
> 
> What is best for flags please?
> 
> -       if (dwrq->flags == 0) {
> +       if (!dwrq->flags) {

The second one.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel