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.
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
Remove the local variable "array" and all the memcpy function calls
because this copy operation from different arrays to this variable is
unnecessary.
The same result can be achieved using the arrays directly.
Signed-off-by: Oscar Carter <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Sat, 2020-04-25 at 14:38 +0200, Oscar Carter wrote:
> Remove the local variable "array" and all the memcpy function calls
> because this copy operation from different arrays to this variable is
> unnecessary.
You might write here that vnt_control_out already does
a kmemdup copy of its const char *buffer argument and
this was made unnecessary by:
commit 12ecd24ef93277e4e5feaf27b0b18f2d3828bc5e
Author: Malcolm Priestley <[email protected]>
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.
> The same result can be achieved using the arrays directly.
>
> Signed-off-by: Oscar Carter <[email protected]>
> ---
> 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
>
On Sat, Apr 25, 2020 at 05:50:39AM -0700, Joe Perches wrote:
> On Sat, 2020-04-25 at 14:38 +0200, Oscar Carter wrote:
> > Remove the local variable "array" and all the memcpy function calls
> > because this copy operation from different arrays to this variable is
> > unnecessary.
>
> You might write here that vnt_control_out already does
> a kmemdup copy of its const char *buffer argument and
> this was made unnecessary by:
>
> commit 12ecd24ef93277e4e5feaf27b0b18f2d3828bc5e
> Author: Malcolm Priestley <[email protected]>
> 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.
>
Great. I will add all this information to clarify the commit changelog.
>
> > The same result can be achieved using the arrays directly.
> >
> > Signed-off-by: Oscar Carter <[email protected]>
> > ---
> > 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
> >
>
thanks,
oscar carter