[PATCH v2 0/3] Refactor the vnt_rf_table_download function

[PATCH v2 0/3] Refactor the vnt_rf_table_download function

[Oscar Carter]

This patch series refactors the vnt_rf_table_download function through
tree patches.

The first one removes the local variable "array" and all the memcpy
function calls because this copy operation from different arrays to this
variable is unnecessary.

The second patch replaces the "goto" statements with a direct "return ret"
as the jump label only returns the ret variable.

The third patch replaces three while loops with three calls to the
vnt_control_out_blocks function. This way avoid repeat a functionality
that already exists.

Changelog v1 -> v2
- Modify the commit changelog of the first patch to clarify the change as
  Joe Perches suggested.

Oscar Carter (3):
  staging: vt6656: Remove the local variable "array"
  staging: vt6656: Use return instead of goto
  staging: vt6656: Remove duplicate code in vnt_rf_table_download

 drivers/staging/vt6656/rf.c | 85 +++++++------------------------------
 1 file changed, 16 insertions(+), 69 deletions(-)

--
2.20.1

[PATCH v2 1/3] staging: vt6656: Remove the local variable "array"

[Oscar Carter]

Remove the local variable "array" and all the memcpy function calls
because this copy operation from different arrays to this variable is
unnecessary.

The vnt_control_out function already does a kmemdup copy of its const
char *buffer argument and this was made unnecessary by:

commit 12ecd24ef932
("staging: vt6656: use off stack for out buffer USB transfers.")
Author: Malcolm Priestley <tvboxspy@gmail.com>
Date:   Sat Apr 22 11:14:57 2017 +0100

    staging: vt6656: use off stack for out buffer USB transfers.

    Since 4.9 mandated USB buffers be heap allocated this causes the driver
    to fail.

    Since there is a wide range of buffer sizes use kmemdup to create
    allocated buffer.

So, the same result can be achieved using the arrays directly.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/rf.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 06fa8867cfa3..82d3b6081b5b 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -770,7 +770,6 @@ int vnt_rf_table_download(struct vnt_private *priv)
 	u16 length1 = 0, length2 = 0, length3 = 0;
 	u8 *addr1 = NULL, *addr2 = NULL, *addr3 = NULL;
 	u16 length, value;
-	u8 array[256];

 	switch (priv->rf_type) {
 	case RF_AL2230:
@@ -817,10 +816,8 @@ int vnt_rf_table_download(struct vnt_private *priv)
 	}

 	/* Init Table */
-	memcpy(array, addr1, length1);
-
 	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-			      MESSAGE_REQUEST_RF_INIT, length1, array);
+			      MESSAGE_REQUEST_RF_INIT, length1, addr1);
 	if (ret)
 		goto end;

@@ -832,10 +829,8 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		else
 			length = length2;

-		memcpy(array, addr2, length);
-
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
-				      MESSAGE_REQUEST_RF_CH0, length, array);
+				      MESSAGE_REQUEST_RF_CH0, length, addr2);
 		if (ret)
 			goto end;

@@ -852,10 +847,8 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		else
 			length = length3;

-		memcpy(array, addr3, length);
-
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
-				      MESSAGE_REQUEST_RF_CH1, length, array);
+				      MESSAGE_REQUEST_RF_CH1, length, addr3);
 		if (ret)
 			goto end;

@@ -870,11 +863,9 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		addr1 = &al7230_init_table_amode[0][0];
 		addr2 = &al7230_channel_table2[0][0];

-		memcpy(array, addr1, length1);
-
 		/* Init Table 2 */
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-				      MESSAGE_REQUEST_RF_INIT2, length1, array);
+				      MESSAGE_REQUEST_RF_INIT2, length1, addr1);
 		if (ret)
 			goto end;

@@ -886,11 +877,9 @@ int vnt_rf_table_download(struct vnt_private *priv)
 			else
 				length = length2;

-			memcpy(array, addr2, length);
-
 			ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
 					      MESSAGE_REQUEST_RF_CH2, length,
-					      array);
+					      addr2);
 			if (ret)
 				goto end;

--
2.20.1

[PATCH v2 2/3] staging: vt6656: Use return instead of goto

[Oscar Carter]

Replace the "goto" statements with a direct "return ret" as the jump
label only returns the ret variable.

Also, remove the unnecessary variable initialization because the ret
variable is set a few lines later.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/rf.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 82d3b6081b5b..888b6fcb6e91 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -766,7 +766,7 @@ void vnt_rf_rssi_to_dbm(struct vnt_private *priv, u8 rssi, long *dbm)

 int vnt_rf_table_download(struct vnt_private *priv)
 {
-	int ret = 0;
+	int ret;
 	u16 length1 = 0, length2 = 0, length3 = 0;
 	u8 *addr1 = NULL, *addr2 = NULL, *addr3 = NULL;
 	u16 length, value;
@@ -819,7 +819,7 @@ int vnt_rf_table_download(struct vnt_private *priv)
 	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
 			      MESSAGE_REQUEST_RF_INIT, length1, addr1);
 	if (ret)
-		goto end;
+		return ret;

 	/* Channel Table 0 */
 	value = 0;
@@ -832,7 +832,7 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
 				      MESSAGE_REQUEST_RF_CH0, length, addr2);
 		if (ret)
-			goto end;
+			return ret;

 		length2 -= length;
 		value += length;
@@ -850,7 +850,7 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
 				      MESSAGE_REQUEST_RF_CH1, length, addr3);
 		if (ret)
-			goto end;
+			return ret;

 		length3 -= length;
 		value += length;
@@ -867,7 +867,7 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
 				      MESSAGE_REQUEST_RF_INIT2, length1, addr1);
 		if (ret)
-			goto end;
+			return ret;

 		/* Channel Table 0 */
 		value = 0;
@@ -881,7 +881,7 @@ int vnt_rf_table_download(struct vnt_private *priv)
 					      MESSAGE_REQUEST_RF_CH2, length,
 					      addr2);
 			if (ret)
-				goto end;
+				return ret;

 			length2 -= length;
 			value += length;
@@ -889,6 +889,5 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		}
 	}

-end:
-	return ret;
+	return 0;
 }
--
2.20.1

[PATCH v2 3/3] staging: vt6656: Remove duplicate code in vnt_rf_table_download

[Oscar Carter]

Replace three while loops with three calls to the vnt_control_out_blocks
function. This way avoid repeat a functionality that already exists.

Also remove the variables that now are not used.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/rf.c | 65 +++++++------------------------------
 1 file changed, 12 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 888b6fcb6e91..420e9869af76 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -769,7 +769,6 @@ int vnt_rf_table_download(struct vnt_private *priv)
 	int ret;
 	u16 length1 = 0, length2 = 0, length3 = 0;
 	u8 *addr1 = NULL, *addr2 = NULL, *addr3 = NULL;
-	u16 length, value;

 	switch (priv->rf_type) {
 	case RF_AL2230:
@@ -822,40 +821,14 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		return ret;

 	/* Channel Table 0 */
-	value = 0;
-	while (length2 > 0) {
-		if (length2 >= 64)
-			length = 64;
-		else
-			length = length2;
-
-		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
-				      MESSAGE_REQUEST_RF_CH0, length, addr2);
-		if (ret)
-			return ret;
-
-		length2 -= length;
-		value += length;
-		addr2 += length;
-	}
-
-	/* Channel table 1 */
-	value = 0;
-	while (length3 > 0) {
-		if (length3 >= 64)
-			length = 64;
-		else
-			length = length3;
-
-		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
-				      MESSAGE_REQUEST_RF_CH1, length, addr3);
-		if (ret)
-			return ret;
+	ret = vnt_control_out_blocks(priv, VNT_REG_BLOCK_SIZE,
+				     MESSAGE_REQUEST_RF_CH0, length2, addr2);
+	if (ret)
+		return ret;

-		length3 -= length;
-		value += length;
-		addr3 += length;
-	}
+	/* Channel Table 1 */
+	ret = vnt_control_out_blocks(priv, VNT_REG_BLOCK_SIZE,
+				     MESSAGE_REQUEST_RF_CH1, length3, addr3);

 	if (priv->rf_type == RF_AIROHA7230) {
 		length1 = CB_AL7230_INIT_SEQ * 3;
@@ -869,25 +842,11 @@ int vnt_rf_table_download(struct vnt_private *priv)
 		if (ret)
 			return ret;

-		/* Channel Table 0 */
-		value = 0;
-		while (length2 > 0) {
-			if (length2 >= 64)
-				length = 64;
-			else
-				length = length2;
-
-			ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
-					      MESSAGE_REQUEST_RF_CH2, length,
-					      addr2);
-			if (ret)
-				return ret;
-
-			length2 -= length;
-			value += length;
-			addr2 += length;
-		}
+		/* Channel Table 2 */
+		ret = vnt_control_out_blocks(priv, VNT_REG_BLOCK_SIZE,
+					     MESSAGE_REQUEST_RF_CH2, length2,
+					     addr2);
 	}

-	return 0;
+	return ret;
 }
--
2.20.1