* [PATCH 1/3] staging: vt6656: Remove the local variable "array"
2020-04-25 12:38 [PATCH 0/3] Refactor the vnt_rf_table_download function Oscar Carter
@ 2020-04-25 12:38 ` Oscar Carter
2020-04-25 12:50 ` Joe Perches
2020-04-25 12:38 ` [PATCH 2/3] staging: vt6656: Use return instead of goto Oscar Carter
2020-04-25 12:38 ` [PATCH 3/3] staging: vt6656: Remove duplicate code in vnt_rf_table_download Oscar Carter
2 siblings, 1 reply; 6+ messages in thread
From: Oscar Carter @ 2020-04-25 12:38 UTC (permalink / raw)
To: Forest Bond, Greg Kroah-Hartman
Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes, devel, linux-kernel
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 <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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] staging: vt6656: Remove the local variable "array"
2020-04-25 12:38 ` [PATCH 1/3] staging: vt6656: Remove the local variable "array" Oscar Carter
@ 2020-04-25 12:50 ` Joe Perches
2020-04-25 14:29 ` Oscar Carter
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-04-25 12:50 UTC (permalink / raw)
To: Oscar Carter, Forest Bond, Greg Kroah-Hartman
Cc: Malcolm Priestley, Quentin Deslandes, devel, linux-kernel
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 <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.
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] staging: vt6656: Remove the local variable "array"
2020-04-25 12:50 ` Joe Perches
@ 2020-04-25 14:29 ` Oscar Carter
0 siblings, 0 replies; 6+ messages in thread
From: Oscar Carter @ 2020-04-25 14:29 UTC (permalink / raw)
To: Joe Perches
Cc: Oscar Carter, Forest Bond, Greg Kroah-Hartman, Malcolm Priestley,
Quentin Deslandes, devel, linux-kernel
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 <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.
>
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 <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
> >
>
thanks,
oscar carter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] staging: vt6656: Use return instead of goto
2020-04-25 12:38 [PATCH 0/3] Refactor the vnt_rf_table_download function Oscar Carter
2020-04-25 12:38 ` [PATCH 1/3] staging: vt6656: Remove the local variable "array" Oscar Carter
@ 2020-04-25 12:38 ` Oscar Carter
2020-04-25 12:38 ` [PATCH 3/3] staging: vt6656: Remove duplicate code in vnt_rf_table_download Oscar Carter
2 siblings, 0 replies; 6+ messages in thread
From: Oscar Carter @ 2020-04-25 12:38 UTC (permalink / raw)
To: Forest Bond, Greg Kroah-Hartman
Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes, devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] staging: vt6656: Remove duplicate code in vnt_rf_table_download
2020-04-25 12:38 [PATCH 0/3] Refactor the vnt_rf_table_download function Oscar Carter
2020-04-25 12:38 ` [PATCH 1/3] staging: vt6656: Remove the local variable "array" Oscar Carter
2020-04-25 12:38 ` [PATCH 2/3] staging: vt6656: Use return instead of goto Oscar Carter
@ 2020-04-25 12:38 ` Oscar Carter
2 siblings, 0 replies; 6+ messages in thread
From: Oscar Carter @ 2020-04-25 12:38 UTC (permalink / raw)
To: Forest Bond, Greg Kroah-Hartman
Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes, devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread