[PATCH] staging: vt6656: Declare a few variables as __read_mostly

2020-03-01 Thread Oscar Carter
These include module parameters.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/main_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..701300202b21 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);

 #define RX_DESC_DEF0 64
-static int vnt_rx_buffers = RX_DESC_DEF0;
+static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
 module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
 MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");

 #define TX_DESC_DEF0 64
-static int vnt_tx_buffers = TX_DESC_DEF0;
+static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
 module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
 MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");

--
2.20.1

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


Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly

2020-03-01 Thread Oscar Carter
On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > These include module parameters.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 5e48b3ddb94c..701300202b21 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> >
> >  #define RX_DESC_DEF0 64
> > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> >
> >  #define TX_DESC_DEF0 64
> > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> >
>
> Why?  What does this help with?

If we declare these variables __read_mostly we can improve the performance. If
these variables are read many more times than written, each core of a multicore
system can maintain a copy in a local cache and the time to access is less than
if they use the shared-cache.

>
> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: vt6656: Remove unnecessary local variables initialization

2020-03-01 Thread Oscar Carter
Don't initialize variables that are then set a few lines later.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/main_usb.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 701300202b21..bfeb5df896fe 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -109,7 +109,7 @@ static void vnt_set_options(struct vnt_private *priv)
  */
 static int vnt_init_registers(struct vnt_private *priv)
 {
-   int ret = 0;
+   int ret;
struct vnt_cmd_card_init *init_cmd = &priv->init_command;
struct vnt_rsp_card_init *init_rsp = &priv->init_response;
u8 antenna;
@@ -435,7 +435,7 @@ static void vnt_free_int_bufs(struct vnt_private *priv)

 static int vnt_alloc_bufs(struct vnt_private *priv)
 {
-   int ret = 0;
+   int ret;
struct vnt_usb_send_context *tx_context;
struct vnt_rcb *rcb;
int ii;
@@ -528,7 +528,7 @@ static void vnt_tx_80211(struct ieee80211_hw *hw,

 static int vnt_start(struct ieee80211_hw *hw)
 {
-   int ret = 0;
+   int ret;
struct vnt_private *priv = hw->priv;

priv->rx_buf_sz = MAX_TOTAL_SIZE_WITH_ALL_HEADERS;
@@ -798,7 +798,7 @@ static u64 vnt_prepare_multicast(struct ieee80211_hw *hw,
struct vnt_private *priv = hw->priv;
struct netdev_hw_addr *ha;
u64 mc_filter = 0;
-   u32 bit_nr = 0;
+   u32 bit_nr;

netdev_hw_addr_list_for_each(ha, mc_list) {
bit_nr = ether_crc(ETH_ALEN, ha->addr) >> 26;
@@ -973,7 +973,7 @@ vt6656_probe(struct usb_interface *intf, const struct 
usb_device_id *id)
struct vnt_private *priv;
struct ieee80211_hw *hw;
struct wiphy *wiphy;
-   int rc = 0;
+   int rc;

udev = usb_get_dev(interface_to_usbdev(intf));

--
2.20.1

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


Re: [PATCH] staging: vt6656: Declare a few variables as __read_mostly

2020-03-07 Thread Oscar Carter
On Sun, Mar 01, 2020 at 04:09:13PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 01, 2020 at 02:17:01PM +0100, Oscar Carter wrote:
> > On Sun, Mar 01, 2020 at 01:25:14PM +0100, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 01, 2020 at 12:26:20PM +0100, Oscar Carter wrote:
> > > > These include module parameters.
> > > >
> > > > Signed-off-by: Oscar Carter 
> > > > ---
> > > >  drivers/staging/vt6656/main_usb.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/main_usb.c 
> > > > b/drivers/staging/vt6656/main_usb.c
> > > > index 5e48b3ddb94c..701300202b21 100644
> > > > --- a/drivers/staging/vt6656/main_usb.c
> > > > +++ b/drivers/staging/vt6656/main_usb.c
> > > > @@ -49,12 +49,12 @@ MODULE_LICENSE("GPL");
> > > >  MODULE_DESCRIPTION(DEVICE_FULL_DRV_NAM);
> > > >
> > > >  #define RX_DESC_DEF0 64
> > > > -static int vnt_rx_buffers = RX_DESC_DEF0;
> > > > +static int __read_mostly vnt_rx_buffers = RX_DESC_DEF0;
> > > >  module_param_named(rx_buffers, vnt_rx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(rx_buffers, "Number of receive usb rx buffers");
> > > >
> > > >  #define TX_DESC_DEF0 64
> > > > -static int vnt_tx_buffers = TX_DESC_DEF0;
> > > > +static int __read_mostly vnt_tx_buffers = TX_DESC_DEF0;
> > > >  module_param_named(tx_buffers, vnt_tx_buffers, int, 0644);
> > > >  MODULE_PARM_DESC(tx_buffers, "Number of receive usb tx buffers");
> > > >
> > >
> > > Why?  What does this help with?
> >
> > If we declare these variables __read_mostly we can improve the performance. 
> > If
> > these variables are read many more times than written, each core of a 
> > multicore
> > system can maintain a copy in a local cache and the time to access is less 
> > than
> > if they use the shared-cache.
>
> This is a USB driver, performance is always limited to the hardware, not
> the CPU location of variables.

Thank you for the explanation.

>
> Please always benchmark things to see if it actually makes sense to make
> changes like this, before proposing them.

I'm sorry.

>
> thanks,
>
> greg k-h

thanks,

Oscar
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: vt6656: Use BIT_ULL() macro instead of bit shift operation

2020-03-07 Thread Oscar Carter
Replace the bit left shift operation with the BIT_ULL() macro and remove
the unnecessary "and" operation against the bit_nr variable.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/main_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..f7ca9e97594d 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -21,6 +21,7 @@
  */
 #undef __NO_VERSION__

+#include 
 #include 
 #include 
 #include "device.h"
@@ -802,8 +803,7 @@ static u64 vnt_prepare_multicast(struct ieee80211_hw *hw,

netdev_hw_addr_list_for_each(ha, mc_list) {
bit_nr = ether_crc(ETH_ALEN, ha->addr) >> 26;
-
-   mc_filter |= 1ULL << (bit_nr & 0x3f);
+   mc_filter |= BIT_ULL(bit_nr);
}

priv->mc_list_count = mc_list->count;
--
2.20.1

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


Re: [PATCH] staging: vt6656: Use BIT_ULL() macro instead of bit shift operation

2020-03-08 Thread Oscar Carter
On Sun, Mar 08, 2020 at 07:55:38AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 07, 2020 at 11:49:29AM +0100, Oscar Carter wrote:
> > Replace the bit left shift operation with the BIT_ULL() macro and remove
> > the unnecessary "and" operation against the bit_nr variable.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 5e48b3ddb94c..f7ca9e97594d 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -21,6 +21,7 @@
> >   */
> >  #undef __NO_VERSION__
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include "device.h"
> > @@ -802,8 +803,7 @@ static u64 vnt_prepare_multicast(struct ieee80211_hw 
> > *hw,
> >
> > netdev_hw_addr_list_for_each(ha, mc_list) {
> > bit_nr = ether_crc(ETH_ALEN, ha->addr) >> 26;
> > -
> > -   mc_filter |= 1ULL << (bit_nr & 0x3f);
> > +   mc_filter |= BIT_ULL(bit_nr);
>
> Are you sure this does the same thing?  You are not masking off bit_nr
> anymore, why not?

My reasons are exposed below:

The ether_crc function returns an u32 type (unsigned of 32 bits). Then the right
shift operand discards the 26 lsb bits (the bits shifted off the right side are
discarded). The 6 msb bits of the u32 returned by the ether_crc function are
positioned in bit 5 to bit 0 of the variable bit_nr. Due to the right shift
happens over an unsigned type, the 26 new bits added on the left side will be 0.

In summary, after the right bit shift operation we obtain in the variable bit_nr
(unsigned of 32 bits) the value represented by the 6 msb bits of the value
returned by the ether_crc function. So, only the 6 lsb bits of the variable
bit_nr are important. The 26 msb bits of this variable are 0.

In this situation, the "and" operation with the mask 0x3f (mask of 6 lsb bits)
is unnecessary due to its purpose is to reset (set to 0 value) the 26 msb bits
that are yet 0.
>
> thanks,
>
> greg k-h

thanks,

Oscar
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Use BIT_ULL() macro instead of bit shift operation

2020-03-14 Thread Oscar Carter
On Tue, Mar 10, 2020 at 10:50:11AM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 08, 2020 at 07:22:07PM +, Malcolm Priestley wrote:
> > >>>   */
> > >>>  #undef __NO_VERSION__
> > >>>
> > >>> +#include 
> > >>>  #include 
> > >>>  #include 
> > >>>  #include "device.h"
> > >>> @@ -802,8 +803,7 @@ static u64 vnt_prepare_multicast(struct 
> > >>> ieee80211_hw *hw,
> > >>>
> > >>> netdev_hw_addr_list_for_each(ha, mc_list) {
> > >>> bit_nr = ether_crc(ETH_ALEN, ha->addr) >> 26;
> > >>> -
> > >>> -   mc_filter |= 1ULL << (bit_nr & 0x3f);
> > >>> +   mc_filter |= BIT_ULL(bit_nr);
> > >>
> > >> Are you sure this does the same thing?  You are not masking off bit_nr
> > >> anymore, why not?
> > >
> > > My reasons are exposed below:
> > >
> > > The ether_crc function returns an u32 type (unsigned of 32 bits). Then 
> > > the right
> > > shift operand discards the 26 lsb bits (the bits shifted off the right 
> > > side are
> > > discarded). The 6 msb bits of the u32 returned by the ether_crc function 
> > > are
> > > positioned in bit 5 to bit 0 of the variable bit_nr. Due to the right 
> > > shift
> > > happens over an unsigned type, the 26 new bits added on the left side 
> > > will be 0.
> > >
> > > In summary, after the right bit shift operation we obtain in the variable 
> > > bit_nr
> > > (unsigned of 32 bits) the value represented by the 6 msb bits of the value
> > > returned by the ether_crc function. So, only the 6 lsb bits of the 
> > > variable
> > > bit_nr are important. The 26 msb bits of this variable are 0.
> > >
> > > In this situation, the "and" operation with the mask 0x3f (mask of 6 lsb 
> > > bits)
> > > is unnecessary due to its purpose is to reset (set to 0 value) the 26 msb 
> > > bits
> > > that are yet 0.
> >
> > The mask is only there out of legacy originally it was 31(0x1f) and the
> > bit_nr spread across two mc_filter u32 arrays.
> >
> > The mask is not needed now it is u64.
> >
> > The patch is fine.
>
> Ok, then the changelog needs to be fixed up to explain all of this and
> resent.

Ok, I will create a new version patch with all of this information and I will
resend it.

>
> thanks,
>
> greg k-h

thanks,

Oscar
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] staging: vt6656: Use BIT_ULL() macro instead of bit shift operation

2020-03-14 Thread Oscar Carter
Replace the bit left shift operation with the BIT_ULL() macro and remove
the unnecessary mask off operation against the bit_nr variable.

This mask is only there for legacy reasons. Originally it was 31 (0x1f)
and the bit_nr variable spread across an mc_filter array with two u32
elements. Now, this mask is not needed because its value is 0x3f and the
mc_filter variable is an u64 type.

In this situation, the 26 bit right shift operation against the value
returned by the ether_crc function set the bit_nr variable to the
following value:

bit 31 to bit 6 -> All 0 (due to the bit shift operation happens over an
unsigned type).

bit 5 to bit 0 -> The 6 msb bits of the value returned by the ether_crc
function.

The purpose of the 0x3f mask against the bit_nr variable is to reset
(set to 0) the 26 msb bits (bit 31 to bit 6), but these bits are yet 0.
So, the mask off operation is now unnecessary.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Add more information to clarify the patch.
- Add notes about the legacy provide by Malcolm Priestley.

 drivers/staging/vt6656/main_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..f7ca9e97594d 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -21,6 +21,7 @@
  */
 #undef __NO_VERSION__

+#include 
 #include 
 #include 
 #include "device.h"
@@ -802,8 +803,7 @@ static u64 vnt_prepare_multicast(struct ieee80211_hw *hw,

netdev_hw_addr_list_for_each(ha, mc_list) {
bit_nr = ether_crc(ETH_ALEN, ha->addr) >> 26;
-
-   mc_filter |= 1ULL << (bit_nr & 0x3f);
+   mc_filter |= BIT_ULL(bit_nr);
}

priv->mc_list_count = mc_list->count;
--
2.20.1

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


[PATCH] staging: vt6656: Use ARRAY_SIZE instead of hardcoded size

2020-03-14 Thread Oscar Carter
Use ARRAY_SIZE to replace the hardcoded size so we will never have a
mismatch.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/main_usb.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..4370941ffc04 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -23,6 +23,7 @@

 #include 
 #include 
+#include 
 #include "device.h"
 #include "card.h"
 #include "baseband.h"
@@ -116,6 +117,7 @@ static int vnt_init_registers(struct vnt_private *priv)
int ii;
u8 tmp;
u8 calib_tx_iq = 0, calib_tx_dc = 0, calib_rx_iq = 0;
+   const int n_cck_pwr_tbl = ARRAY_SIZE(priv->cck_pwr_tbl);

dev_dbg(&priv->usb->dev, ">INIbInitAdapter. [%d][%d]\n",
DEVICE_INIT_COLD, priv->packet_type);
@@ -145,7 +147,7 @@ static int vnt_init_registers(struct vnt_private *priv)

init_cmd->init_class = DEVICE_INIT_COLD;
init_cmd->exist_sw_net_addr = priv->exist_sw_net_addr;
-   for (ii = 0; ii < 6; ii++)
+   for (ii = 0; ii < ARRAY_SIZE(init_cmd->sw_net_addr); ii++)
init_cmd->sw_net_addr[ii] = priv->current_net_addr[ii];
init_cmd->short_retry_limit = priv->short_retry_limit;
init_cmd->long_retry_limit = priv->long_retry_limit;
@@ -184,7 +186,7 @@ static int vnt_init_registers(struct vnt_private *priv)
priv->cck_pwr = priv->eeprom[EEP_OFS_PWR_CCK];
priv->ofdm_pwr_g = priv->eeprom[EEP_OFS_PWR_OFDMG];
/* load power table */
-   for (ii = 0; ii < 14; ii++) {
+   for (ii = 0; ii < n_cck_pwr_tbl; ii++) {
priv->cck_pwr_tbl[ii] =
priv->eeprom[ii + EEP_OFS_CCK_PWR_TBL];
if (priv->cck_pwr_tbl[ii] == 0)
@@ -200,7 +202,7 @@ static int vnt_init_registers(struct vnt_private *priv)
 * original zonetype is USA, but custom zonetype is Europe,
 * then need to recover 12, 13, 14 channels with 11 channel
 */
-   for (ii = 11; ii < 14; ii++) {
+   for (ii = 11; ii < n_cck_pwr_tbl; ii++) {
priv->cck_pwr_tbl[ii] = priv->cck_pwr_tbl[10];
priv->ofdm_pwr_tbl[ii] = priv->ofdm_pwr_tbl[10];
}
--
2.20.1

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


Re: [PATCH] staging: vt6656: Use ARRAY_SIZE instead of hardcoded size

2020-03-18 Thread Oscar Carter
On Tue, Mar 17, 2020 at 01:45:06PM +0300, Dan Carpenter wrote:
> On Sat, Mar 14, 2020 at 05:47:54PM +0100, Oscar Carter wrote:
> > Use ARRAY_SIZE to replace the hardcoded size so we will never have a
> > mismatch.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 5e48b3ddb94c..4370941ffc04 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -23,6 +23,7 @@
> >
> >  #include 
> >  #include 
> > +#include 
> >  #include "device.h"
> >  #include "card.h"
> >  #include "baseband.h"
> > @@ -116,6 +117,7 @@ static int vnt_init_registers(struct vnt_private *priv)
> > int ii;
> > u8 tmp;
> > u8 calib_tx_iq = 0, calib_tx_dc = 0, calib_rx_iq = 0;
> > +   const int n_cck_pwr_tbl = ARRAY_SIZE(priv->cck_pwr_tbl);
>
> Please use ARRAY_SIZE(priv->cck_pwr_tbl) everywhere instead of
> introducing this new variable.
>
Ok, I create a new version of the patch and I resend it.

> regards,
> dan carpenter
>
thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] staging: vt6656: Use ARRAY_SIZE instead of hardcoded size

2020-03-18 Thread Oscar Carter
Use ARRAY_SIZE to replace the hardcoded size so we will never have a
mismatch.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Use ARRAY_SIZE(priv->cck_pwr_tbl) everywhere instead of introducing a new
  variable to hold its value.

 drivers/staging/vt6656/main_usb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..acfcc11c3b61 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -23,6 +23,7 @@

 #include 
 #include 
+#include 
 #include "device.h"
 #include "card.h"
 #include "baseband.h"
@@ -145,7 +146,7 @@ static int vnt_init_registers(struct vnt_private *priv)

init_cmd->init_class = DEVICE_INIT_COLD;
init_cmd->exist_sw_net_addr = priv->exist_sw_net_addr;
-   for (ii = 0; ii < 6; ii++)
+   for (ii = 0; ii < ARRAY_SIZE(init_cmd->sw_net_addr); ii++)
init_cmd->sw_net_addr[ii] = priv->current_net_addr[ii];
init_cmd->short_retry_limit = priv->short_retry_limit;
init_cmd->long_retry_limit = priv->long_retry_limit;
@@ -184,7 +185,7 @@ static int vnt_init_registers(struct vnt_private *priv)
priv->cck_pwr = priv->eeprom[EEP_OFS_PWR_CCK];
priv->ofdm_pwr_g = priv->eeprom[EEP_OFS_PWR_OFDMG];
/* load power table */
-   for (ii = 0; ii < 14; ii++) {
+   for (ii = 0; ii < ARRAY_SIZE(priv->cck_pwr_tbl); ii++) {
priv->cck_pwr_tbl[ii] =
priv->eeprom[ii + EEP_OFS_CCK_PWR_TBL];
if (priv->cck_pwr_tbl[ii] == 0)
@@ -200,7 +201,7 @@ static int vnt_init_registers(struct vnt_private *priv)
 * original zonetype is USA, but custom zonetype is Europe,
 * then need to recover 12, 13, 14 channels with 11 channel
 */
-   for (ii = 11; ii < 14; ii++) {
+   for (ii = 11; ii < ARRAY_SIZE(priv->cck_pwr_tbl); ii++) {
priv->cck_pwr_tbl[ii] = priv->cck_pwr_tbl[10];
priv->ofdm_pwr_tbl[ii] = priv->ofdm_pwr_tbl[10];
}
--
2.20.1

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


[PATCH] staging: vt6656: Use BIT() macro instead of hex value

2020-03-20 Thread Oscar Carter
Use the BIT() macro instead of the hexadecimal value to define the
different bits in registers.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/desc.h   |  35 ++---
 drivers/staging/vt6656/device.h |   9 +-
 drivers/staging/vt6656/mac.h| 263 
 3 files changed, 155 insertions(+), 152 deletions(-)

diff --git a/drivers/staging/vt6656/desc.h b/drivers/staging/vt6656/desc.h
index 3a83a9ea9a2a..703597a911f4 100644
--- a/drivers/staging/vt6656/desc.h
+++ b/drivers/staging/vt6656/desc.h
@@ -18,6 +18,7 @@
 #ifndef __DESC_H__
 #define __DESC_H__

+#include 
 #include 
 #include 

@@ -36,32 +37,32 @@
 /*
  * bits in the RSR register
  */
-#define RSR_ADDRBROAD   0x80
-#define RSR_ADDRMULTI   0x40
+#define RSR_ADDRBROAD   BIT(7)
+#define RSR_ADDRMULTI   BIT(6)
 #define RSR_ADDRUNI 0x00
-#define RSR_IVLDTYP 0x20/* invalid packet type */
-#define RSR_IVLDLEN 0x10/* invalid len (> 2312 byte) */
-#define RSR_BSSIDOK 0x08
-#define RSR_CRCOK   0x04
-#define RSR_BCNSSIDOK   0x02
-#define RSR_ADDROK  0x01
+#define RSR_IVLDTYP BIT(5) /* invalid packet type */
+#define RSR_IVLDLEN BIT(4) /* invalid len (> 2312 byte) */
+#define RSR_BSSIDOK BIT(3)
+#define RSR_CRCOK   BIT(2)
+#define RSR_BCNSSIDOK   BIT(1)
+#define RSR_ADDROK  BIT(0)

 /*
  * bits in the new RSR register
  */
-#define NEWRSR_DECRYPTOK0x10
-#define NEWRSR_CFPIND   0x08
-#define NEWRSR_HWUTSF   0x04
-#define NEWRSR_BCNHITAID0x02
-#define NEWRSR_BCNHITAID0   0x01
+#define NEWRSR_DECRYPTOKBIT(4)
+#define NEWRSR_CFPIND   BIT(3)
+#define NEWRSR_HWUTSF   BIT(2)
+#define NEWRSR_BCNHITAIDBIT(1)
+#define NEWRSR_BCNHITAID0   BIT(0)

 /*
  * bits in the TSR register
  */
-#define TSR_RETRYTMO0x08
-#define TSR_TMO 0x04
-#define TSR_ACKDATA 0x02
-#define TSR_VALID   0x01
+#define TSR_RETRYTMOBIT(3)
+#define TSR_TMO BIT(2)
+#define TSR_ACKDATA BIT(1)
+#define TSR_VALID   BIT(0)

 #define FIFOCTL_AUTO_FB_1   0x1000
 #define FIFOCTL_AUTO_FB_0   0x0800
diff --git a/drivers/staging/vt6656/device.h b/drivers/staging/vt6656/device.h
index fe6c11266123..45faf0ab9f62 100644
--- a/drivers/staging/vt6656/device.h
+++ b/drivers/staging/vt6656/device.h
@@ -16,6 +16,7 @@
 #ifndef __DEVICE_H__
 #define __DEVICE_H__

+#include 
 #include 
 #include 
 #include 
@@ -129,12 +130,12 @@
 #define EEP_OFS_OFDMA_PWR_TBL  0x50

 /* Bits in EEP_OFS_ANTENNA */
-#define EEP_ANTENNA_MAIN   0x1
-#define EEP_ANTENNA_AUX0x2
-#define EEP_ANTINV 0x4
+#define EEP_ANTENNA_MAIN   BIT(0)
+#define EEP_ANTENNA_AUXBIT(1)
+#define EEP_ANTINV BIT(2)

 /* Bits in EEP_OFS_RADIOCTL */
-#define EEP_RADIOCTL_ENABLE0x80
+#define EEP_RADIOCTL_ENABLEBIT(7)

 /* control commands */
 #define MESSAGE_TYPE_READ  0x1
diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
index 0a42308b81e9..c532b27de37f 100644
--- a/drivers/staging/vt6656/mac.h
+++ b/drivers/staging/vt6656/mac.h
@@ -20,6 +20,7 @@
 #ifndef __MAC_H__
 #define __MAC_H__

+#include 
 #include "device.h"

 #define REV_ID_VT3253_A0   0x00
@@ -142,109 +143,109 @@
 #define MAC_REG_RSPINF_A_720xfc

 /* Bits in the I2MCFG EEPROM register */
-#define I2MCFG_BOUNDCTL0x80
-#define I2MCFG_WAITCTL 0x20
-#define I2MCFG_SCLOECTL0x10
-#define I2MCFG_WBUSYCTL0x08
-#define I2MCFG_NORETRY 0x04
-#define I2MCFG_I2MLDSEQ0x02
-#define I2MCFG_I2CMFAST0x01
+#define I2MCFG_BOUNDCTLBIT(7)
+#define I2MCFG_WAITCTL BIT(5)
+#define I2MCFG_SCLOECTLBIT(4)
+#define I2MCFG_WBUSYCTLBIT(3)
+#define I2MCFG_NORETRY BIT(2)
+#define I2MCFG_I2MLDSEQBIT(1)
+#define I2MCFG_I2CMFASTBIT(0)

 /* Bits in the I2MCSR EEPROM register */
-#define I2MCSR_EEMW0x80
-#define I2MCSR_EEMR0x40
-#define I2MCSR_AUTOLD  0x08
-#define I2MCSR_NACK0x02
-#define I2MCSR_DONE0x01
+#define I2MCSR_EEMWBIT(7)
+#define I2MCSR_EEMRBIT(6)
+#define I2MCSR_AUTOLD  BIT(3)
+#define I2MCSR_NACKBIT(1)
+#define I2MCSR_DONEBIT(0)

 /* Bits in the TMCTL register */
-#define TMCTL_TSUSP0x04
-#define TMCTL_TMD  0x02
-#define TMCTL_TE   0x01
+#define TMCTL_TSUSPBIT(2)
+#define TMCTL_TMD  BIT(1)
+#define TMCTL_TE   BIT(0)

 /* Bits in the TFTCTL register */
-#define TFTCTL_HWUTSF  0x80
-#define TFTCTL_TBTTSYNC0x40
-#define TFTCTL_HWUTSFEN0x20
-#define TFTCTL_TSFCNTRRD   0x10
-#define TFTCTL_TBTTSYNCEN  0x08
-#define TFTCTL_TSFS

[PATCH] staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_* functions

2020-03-20 Thread Oscar Carter
The last parameter in the functions vnt_mac_reg_bits_on and
vnt_mac_reg_bits_off defines the bits to set or unset. So, it's more
clear to use the BIT() macro instead of an hexadecimal value.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 5 +++--
 drivers/staging/vt6656/card.c | 4 +++-
 drivers/staging/vt6656/main_usb.c | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index f18e059ce66b..a29d9f4f575e 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -22,6 +22,7 @@
  *
  */

+#include 
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -468,7 +469,7 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
if (ret)
goto end;
} else if (priv->rf_type == RF_VT3226D0) {
@@ -477,7 +478,7 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
if (ret)
goto end;
}
diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 7958fc165462..dc3ab10eb630 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -26,6 +26,7 @@
  *
  */

+#include 
 #include "device.h"
 #include "card.h"
 #include "baseband.h"
@@ -63,7 +64,8 @@ void vnt_set_channel(struct vnt_private *priv, u32 
connection_channel)
vnt_mac_reg_bits_on(priv, MAC_REG_MACCR, MACCR_CLRNAV);

/* Set Channel[7] = 0 to tell H/W channel is changing now. */
-   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL, 0xb0);
+   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL,
+(BIT(7) | BIT(5) | BIT(4)));

vnt_control_out(priv, MESSAGE_TYPE_SELECT_CHANNEL,
connection_channel, 0, 0, NULL);
diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 5e48b3ddb94c..1196b6e28c22 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -21,6 +21,7 @@
  */
 #undef __NO_VERSION__

+#include 
 #include 
 #include 
 #include "device.h"
@@ -370,7 +371,7 @@ static int vnt_init_registers(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, 0x01);
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, BIT(0));
if (ret)
goto end;

--
2.20.1

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


[PATCH] staging: vt6656: Use DIV_ROUND_UP macro instead of specific code

2020-03-22 Thread Oscar Carter
Use DIV_ROUND_UP macro instead of specific code with the same purpose.
Also, remove the unused variables.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 21 -
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index f18e059ce66b..e2eb2b98a73d 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -22,6 +22,7 @@
  *
  */

+#include 
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -132,7 +133,6 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
 {
unsigned int frame_time;
unsigned int preamble;
-   unsigned int tmp;
unsigned int rate = 0;

if (tx_rate > RATE_54M)
@@ -146,20 +146,11 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
else
preamble = 192;

-   frame_time = (frame_length * 80) / rate;
-   tmp = (frame_time * rate) / 80;
-
-   if (frame_length != tmp)
-   frame_time++;
-
+   frame_time = DIV_ROUND_UP(frame_length * 80, rate);
return preamble + frame_time;
}
-   frame_time = (frame_length * 8 + 22) / rate;
-   tmp = ((frame_time * rate) - 22) / 8;
-
-   if (frame_length != tmp)
-   frame_time++;

+   frame_time = DIV_ROUND_UP(frame_length * 8 + 22, rate);
frame_time = frame_time * 4;

if (pkt_type != PK_TYPE_11A)
@@ -213,11 +204,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,

break;
case RATE_5M:
-   count = (bit_count * 10) / 55;
-   tmp = (count * 55) / 10;
-
-   if (tmp != bit_count)
-   count++;
+   count = DIV_ROUND_UP(bit_count * 10, 55);

if (preamble_type == 1)
phy->signal = 0x0a;
--
2.20.1

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


Re: [PATCH] staging: vt6656: Use DIV_ROUND_UP macro instead of specific code

2020-03-24 Thread Oscar Carter
On Mon, Mar 23, 2020 at 11:42:00AM +0100, Greg Kroah-Hartman wrote:
> On Sun, Mar 22, 2020 at 12:23:42PM +0100, Oscar Carter wrote:
> > Use DIV_ROUND_UP macro instead of specific code with the same purpose.
> > Also, remove the unused variables.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/baseband.c | 21 -
> >  1 file changed, 4 insertions(+), 17 deletions(-)
>
> Please rebase this against my staging-next branch of my staging.git tree
> and resend it as it does not apply to it at the moment at all.
>
Ok, I rebase against your staging-next branch and I resend the patch as a new
version.

> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Use BIT() macro instead of hex value

2020-03-26 Thread Oscar Carter
On Mon, Mar 23, 2020 at 10:35:18AM +0300, Dan Carpenter wrote:
> On Fri, Mar 20, 2020 at 06:10:56PM +0100, Oscar Carter wrote:
> > -#define RSR_ADDRBROAD   0x80
> > -#define RSR_ADDRMULTI   0x40
> > +#define RSR_ADDRBROAD   BIT(7)
> > +#define RSR_ADDRMULTI   BIT(6)
> >  #define RSR_ADDRUNI 0x00
> > -#define RSR_IVLDTYP 0x20/* invalid packet type */
> > -#define RSR_IVLDLEN 0x10/* invalid len (> 2312 byte) */
> > -#define RSR_BSSIDOK 0x08
> > -#define RSR_CRCOK   0x04
> > -#define RSR_BCNSSIDOK   0x02
> > -#define RSR_ADDROK  0x01
> > +#define RSR_IVLDTYP BIT(5) /* invalid packet type */
> > +#define RSR_IVLDLEN BIT(4) /* invalid len (> 2312 byte) */
> > +#define RSR_BSSIDOK BIT(3)
> > +#define RSR_CRCOK   BIT(2)
> > +#define RSR_BCNSSIDOK   BIT(1)
> > +#define RSR_ADDROK  BIT(0)
>
> I like these ones because I do think the new version is more clear
> now.
>
> >  /* Bits in the EnhanceCFG_0 register */
> >  #define EnCFG_BBType_a 0x00
> > -#define EnCFG_BBType_b 0x01
> > -#define EnCFG_BBType_g 0x02
> > -#define EnCFG_BBType_MASK  0x03
> > -#define EnCFG_ProtectMd0x20
> > +#define EnCFG_BBType_b BIT(0)
> > +#define EnCFG_BBType_g BIT(1)
> > +#define EnCFG_BBType_MASK  (BIT(0) | BIT(1))
> > +#define EnCFG_ProtectMdBIT(5)
>
> Probably EnCFG_BBType_MASK should be defined using the other defines.
>
> #define EnCFG_BBType_MASK (EnCFG_BBType_b | EnCFG_BBType_g)
>
> Otherwise it looks good.  Can you change that one thing and then add
> my Reviewed-by: Dan Carpenter 
>
Ok, i will make this change and i will send and incremental patch with the
"Fixes:" tag due to the this patch has already been added to the staging-next
branch of the greg staging tree.

> regards,
> dan carpenter
>
thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_* functions

2020-03-26 Thread Oscar Carter
On Mon, Mar 23, 2020 at 10:32:14AM +0300, Dan Carpenter wrote:
> On Fri, Mar 20, 2020 at 07:13:26PM +0100, Oscar Carter wrote:
> > +#include 
> >  #include "mac.h"
> >  #include "baseband.h"
> >  #include "rf.h"
> > @@ -468,7 +469,7 @@ int vnt_vt3184_init(struct vnt_private *priv)
> > if (ret)
> > goto end;
> >
> > -   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
> > +   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
>
> Everyone knows 0x01 is bit(0) already.  This isn't more clear.  It
> should be a define instead of a magic number.
>
I agree. I create a new define for this case.

> > @@ -63,7 +64,8 @@ void vnt_set_channel(struct vnt_private *priv, u32 
> > connection_channel)
> > vnt_mac_reg_bits_on(priv, MAC_REG_MACCR, MACCR_CLRNAV);
> >
> > /* Set Channel[7] = 0 to tell H/W channel is changing now. */
> > -   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL, 0xb0);
> > +   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL,
> > +(BIT(7) | BIT(5) | BIT(4)));
>
> This one especially is just a lot longer now but still not clear.
>
Like the previous one, i create a define. In this case to avoid the magic
number or the OR operation between BIT macros.

> regards,
> dan carpenter
>
I will make these changes and i will send and incremental patch with the
"Fixes:" tag due to the this patch has already been added to the staging-next
branch of the greg staging tree.

thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] staging: vt6656: Use ARRAY_SIZE instead of hardcoded size

2020-03-26 Thread Oscar Carter
On Wed, Mar 25, 2020 at 09:19:24AM +, Quentin Deslandes wrote:
> On 03/24/20 16:18:30, Dan Carpenter wrote:
> > That's a bit over engineering something which is pretty trivial.
> > Normally, we would just make the size a define instead of a magic number
> > 14.
>
> My bad, I meant "define", not "macro".
>
> > If people change the size in the future (unlikely) and it causes a bug
> > then they kind of deserve it because they need to ensure all the new
> > stuff is initialized, right?  If they change it and it results in a
> > buffer overflow then static checkers would complain.  If they changed it
> > and it resulted in uninitialized data being used then it would be zero
> > so that's okay.
>
> I wasn't sure where I should stand on this, that's clearer now.
>
> Thanks,
> Quentin

Dan and Quentin, thanks for your time to review my work, and make comments.

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] staging: vt6656: Use DIV_ROUND_UP macro instead of specific code

2020-03-26 Thread Oscar Carter
Use DIV_ROUND_UP macro instead of specific code with the same purpose.
Also, remove the unused variables.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Rebase the original patch [1] against the staging-next branch of the greg's
  staging.git tree.

  [1] https://lore.kernel.org/lkml/20200322112342.9040-1-oscar.car...@gmx.com/

 drivers/staging/vt6656/baseband.c | 21 -
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 0b5729abcbcd..a19a563d8bcc 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -133,7 +134,6 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
 {
unsigned int frame_time;
unsigned int preamble;
-   unsigned int tmp;
unsigned int rate = 0;

if (tx_rate > RATE_54M)
@@ -147,20 +147,11 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
else
preamble = 192;

-   frame_time = (frame_length * 80) / rate;
-   tmp = (frame_time * rate) / 80;
-
-   if (frame_length != tmp)
-   frame_time++;
-
+   frame_time = DIV_ROUND_UP(frame_length * 80, rate);
return preamble + frame_time;
}
-   frame_time = (frame_length * 8 + 22) / rate;
-   tmp = ((frame_time * rate) - 22) / 8;
-
-   if (frame_length != tmp)
-   frame_time++;

+   frame_time = DIV_ROUND_UP(frame_length * 8 + 22, rate);
frame_time = frame_time * 4;

if (pkt_type != PK_TYPE_11A)
@@ -214,11 +205,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,

break;
case RATE_5M:
-   count = (bit_count * 10) / 55;
-   tmp = (count * 55) / 10;
-
-   if (tmp != bit_count)
-   count++;
+   count = DIV_ROUND_UP(bit_count * 10, 55);

if (preamble_type == 1)
phy->signal = 0x0a;
--
2.20.1

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


[PATCH] staging: vt6656: Define EnCFG_BBType_MASK as OR between previous defines

2020-03-27 Thread Oscar Carter
Define the EnCFG_BBType_MASK bit as an OR operation between two previous
defines instead of using the OR between two new BIT macros. Thus, the
code is more clear.

Fixes: a74081b44291 ("staging: vt6656: Use BIT() macro instead of hex value")
Signed-off-by: Oscar Carter 
Reviewed-by: Dan Carpenter 
---
 drivers/staging/vt6656/mac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
index c532b27de37f..b01d9ee8677e 100644
--- a/drivers/staging/vt6656/mac.h
+++ b/drivers/staging/vt6656/mac.h
@@ -177,7 +177,7 @@
 #define EnCFG_BBType_a 0x00
 #define EnCFG_BBType_b BIT(0)
 #define EnCFG_BBType_g BIT(1)
-#define EnCFG_BBType_MASK  (BIT(0) | BIT(1))
+#define EnCFG_BBType_MASK  (EnCFG_BBType_b | EnCFG_BBType_g)
 #define EnCFG_ProtectMdBIT(5)

 /* Bits in the EnhanceCFG_1 register */
--
2.20.1

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


[PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions

2020-03-28 Thread Oscar Carter
Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
registers to can use them in the calls to vnt_mac_reg_bits_on and
vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
macros and clarify the code.

Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_* 
functions")
Suggested-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c |  6 --
 drivers/staging/vt6656/card.c |  3 +--
 drivers/staging/vt6656/mac.h  | 12 
 drivers/staging/vt6656/main_usb.c |  2 +-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..dd3c3bf5e8b5 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
+ PAPEDELAY_B0);
if (ret)
goto end;
} else if (priv->rf_type == RF_VT3226D0) {
@@ -451,7 +452,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
+ PAPEDELAY_B0);
if (ret)
goto end;
}
diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index dc3ab10eb630..b88de0042b9d 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -64,8 +64,7 @@ void vnt_set_channel(struct vnt_private *priv, u32 
connection_channel)
vnt_mac_reg_bits_on(priv, MAC_REG_MACCR, MACCR_CLRNAV);

/* Set Channel[7] = 0 to tell H/W channel is changing now. */
-   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL,
-(BIT(7) | BIT(5) | BIT(4)));
+   vnt_mac_reg_bits_off(priv, MAC_REG_CHANNEL, CHANNEL_B7_B5_B4);

vnt_control_out(priv, MESSAGE_TYPE_SELECT_CHANNEL,
connection_channel, 0, 0, NULL);
diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
index c532b27de37f..f61b6595defb 100644
--- a/drivers/staging/vt6656/mac.h
+++ b/drivers/staging/vt6656/mac.h
@@ -295,11 +295,20 @@
 #define BBREGCTL_REGR  BIT(1)
 #define BBREGCTL_REGW  BIT(0)

+/* Bits in the CHANNEL register */
+#define CHANNEL_B7 BIT(7)
+#define CHANNEL_B5 BIT(5)
+#define CHANNEL_B4 BIT(4)
+#define CHANNEL_B7_B5_B4   (CHANNEL_B7 | CHANNEL_B5 | CHANNEL_B4)
+
 /* Bits in the IFREGCTL register */
 #define IFREGCTL_DONE  BIT(2)
 #define IFREGCTL_IFRF  BIT(1)
 #define IFREGCTL_REGW  BIT(0)

+/* Bits in the PAPEDELAY register */
+#define PAPEDELAY_B0   BIT(0)
+
 /* Bits in the SOFTPWRCTL register */
 #define SOFTPWRCTL_RFLEOPT BIT(3)
 #define SOFTPWRCTL_TXPEINV BIT(1)
@@ -311,6 +320,9 @@
 #define SOFTPWRCTL_SWPE1   BIT(1)
 #define SOFTPWRCTL_SWPE3   BIT(0)

+/* Bits in the GPIOCTL0 register */
+#define GPIOCTL0_B0BIT(0)
+
 /* Bits in the GPIOCTL1 register */
 #define GPIO3_MD   BIT(5)
 #define GPIO3_DATA BIT(6)
diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 8e7269c87ea9..aa9c1fccc134 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -366,7 +366,7 @@ static int vnt_init_registers(struct vnt_private *priv)
if (ret)
goto end;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, BIT(0));
+   ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, GPIOCTL0_B0);
if (ret)
goto end;

--
2.20.1

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


[PATCH] staging: vt6656: Use defines in preamble_type variables

2020-03-28 Thread Oscar Carter
Use the PREAMBLE_SHORT and PREAMBLE_LONG defines present in the file
"baseband.h" to assign values to preamble_type variables. Also, use the
same defines to make comparisons against this variables.

In this way, avoid the use of numerical literals or boolean values and
make the code more clear.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 8 
 drivers/staging/vt6656/main_usb.c | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..9bbafa7fff61 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -142,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
rate = (unsigned int)vnt_frame_time[tx_rate];

if (tx_rate <= 3) {
-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
preamble = 96;
else
preamble = 192;
@@ -198,7 +198,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
case RATE_2M:
count = bit_count / 2;

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x09;
else
phy->signal = 0x01;
@@ -207,7 +207,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x0a;
else
phy->signal = 0x02;
@@ -224,7 +224,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
ext_bit = true;
}

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x0b;
else
phy->signal = 0x03;
diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 8e7269c87ea9..dd89f98cc18c 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -99,7 +99,7 @@ static void vnt_set_options(struct vnt_private *priv)
priv->op_mode = NL80211_IFTYPE_UNSPECIFIED;
priv->bb_type = BBP_TYPE_DEF;
priv->packet_type = priv->bb_type;
-   priv->preamble_type = 0;
+   priv->preamble_type = PREAMBLE_LONG;
priv->exist_sw_net_addr = false;
 }

@@ -721,10 +721,10 @@ static void vnt_bss_info_changed(struct ieee80211_hw *hw,
if (changed & BSS_CHANGED_ERP_PREAMBLE) {
if (conf->use_short_preamble) {
vnt_mac_enable_barker_preamble_mode(priv);
-   priv->preamble_type = true;
+   priv->preamble_type = PREAMBLE_SHORT;
} else {
vnt_mac_disable_barker_preamble_mode(priv);
-   priv->preamble_type = false;
+   priv->preamble_type = PREAMBLE_LONG;
}
}

--
2.20.1

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


[PATCH v2] staging: vt6656: Use defines in preamble_type variables

2020-03-28 Thread Oscar Carter
Use the PREAMBLE_SHORT and PREAMBLE_LONG defines present in the file
"baseband.h" to assign values to preamble_type variables. Also, use the
same defines to make comparisons against these variables.

In this way, avoid the use of numerical literals or boolean values and
make the code more clear.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Spell checking correction in the changelog.

 drivers/staging/vt6656/baseband.c | 8 
 drivers/staging/vt6656/main_usb.c | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..9bbafa7fff61 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -142,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
rate = (unsigned int)vnt_frame_time[tx_rate];

if (tx_rate <= 3) {
-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
preamble = 96;
else
preamble = 192;
@@ -198,7 +198,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
case RATE_2M:
count = bit_count / 2;

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x09;
else
phy->signal = 0x01;
@@ -207,7 +207,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x0a;
else
phy->signal = 0x02;
@@ -224,7 +224,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
ext_bit = true;
}

-   if (preamble_type == 1)
+   if (preamble_type == PREAMBLE_SHORT)
phy->signal = 0x0b;
else
phy->signal = 0x03;
diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 8e7269c87ea9..dd89f98cc18c 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -99,7 +99,7 @@ static void vnt_set_options(struct vnt_private *priv)
priv->op_mode = NL80211_IFTYPE_UNSPECIFIED;
priv->bb_type = BBP_TYPE_DEF;
priv->packet_type = priv->bb_type;
-   priv->preamble_type = 0;
+   priv->preamble_type = PREAMBLE_LONG;
priv->exist_sw_net_addr = false;
 }

@@ -721,10 +721,10 @@ static void vnt_bss_info_changed(struct ieee80211_hw *hw,
if (changed & BSS_CHANGED_ERP_PREAMBLE) {
if (conf->use_short_preamble) {
vnt_mac_enable_barker_preamble_mode(priv);
-   priv->preamble_type = true;
+   priv->preamble_type = PREAMBLE_SHORT;
} else {
vnt_mac_disable_barker_preamble_mode(priv);
-   priv->preamble_type = false;
+   priv->preamble_type = PREAMBLE_LONG;
}
}

--
2.20.1

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


[PATCH] staging: vt6656: Refactor the vnt_update_pre_ed_threshold function

2020-03-28 Thread Oscar Carter
Create three arrays with the threshold data use in the switch statement
of the vnt_update_pre_ed_threshold function. These three arrays contains
elements of struct vnt_threshold new type.

Create a for loop in the vnt_update_pre_ed_threshold function to do
exactly the same that the if-elseif-else statements in the switch
statement.

Also, remove the if check against the !cr_201 && !cr_206 due to now it
is replace by the NULL check against the threshold pointer. When this
pointer is NULL means that the cr_201 and cr_206 variables have not been
assigned, that is the same that the old comparison against cr_201 and
cr_206 due to these variables were initialized with 0.

The statistics of the old baseband object file are:

section  size   addr
.text3415  0
.data 576  0
.bss0  0
.rodata   120  0
.comment   45  0
.note.GNU-stack 0  0
.note.gnu.property 28  0
Total4184

The statistics of the new baseband object file are:

section  size   addr
.text2209  0
.data 576  0
.bss0  0
.rodata   344  0
.comment   45  0
.note.GNU-stack 0  0
.note.gnu.property 28  0
Total3202

With this refactoring it increase a little the readonly data but it
decrease much more the .text section. This refactoring decrease the
footprint and makes the code more clear.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 335 +-
 1 file changed, 100 insertions(+), 235 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..e03f83e1c394 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -115,6 +115,86 @@ static const u16 vnt_frame_time[MAX_RATE] = {
10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
 };

+struct vnt_threshold {
+   u8 bb_pre_ed_rssi;
+   u8 cr_201;
+   u8 cr_206;
+};
+
+static const struct vnt_threshold al2230_vnt_threshold[] = {
+   {0, 0x00, 0x30},/* Max sensitivity */
+   {68, 0x00, 0x36},
+   {67, 0x00, 0x43},
+   {66, 0x00, 0x51},
+   {65, 0x00, 0x62},
+   {64, 0x00, 0x79},
+   {63, 0x00, 0x93},
+   {62, 0x00, 0xb9},
+   {61, 0x00, 0xe3},
+   {60, 0x01, 0x18},
+   {59, 0x01, 0x54},
+   {58, 0x01, 0xa0},
+   {57, 0x02, 0x20},
+   {56, 0x02, 0xa0},
+   {55, 0x03, 0x00},
+   {53, 0x06, 0x00},
+   {51, 0x09, 0x00},
+   {49, 0x0e, 0x00},
+   {47, 0x15, 0x00},
+   {46, 0x1a, 0x00},
+   {45, 0xff, 0x00}
+};
+
+static const struct vnt_threshold vt3226_vnt_threshold[] = {
+   {0, 0x00, 0x24},/* Max sensitivity */
+   {68, 0x00, 0x2d},
+   {67, 0x00, 0x36},
+   {66, 0x00, 0x43},
+   {65, 0x00, 0x52},
+   {64, 0x00, 0x68},
+   {63, 0x00, 0x80},
+   {62, 0x00, 0x9c},
+   {61, 0x00, 0xc0},
+   {60, 0x00, 0xea},
+   {59, 0x01, 0x30},
+   {58, 0x01, 0x70},
+   {57, 0x01, 0xb0},
+   {56, 0x02, 0x30},
+   {55, 0x02, 0xc0},
+   {53, 0x04, 0x00},
+   {51, 0x07, 0x00},
+   {49, 0x0a, 0x00},
+   {47, 0x11, 0x00},
+   {45, 0x18, 0x00},
+   {43, 0x26, 0x00},
+   {42, 0x36, 0x00},
+   {41, 0xff, 0x00}
+};
+
+static const struct vnt_threshold vt3342_vnt_threshold[] = {
+   {0, 0x00, 0x38},/* Max sensitivity */
+   {66, 0x00, 0x43},
+   {65, 0x00, 0x52},
+   {64, 0x00, 0x68},
+   {63, 0x00, 0x80},
+   {62, 0x00, 0x9c},
+   {61, 0x00, 0xc0},
+   {60, 0x00, 0xea},
+   {59, 0x01, 0x30},
+   {58, 0x01, 0x70},
+   {57, 0x01, 0xb0},
+   {56, 0x02, 0x30},
+   {55, 0x02, 0xc0},
+   {53, 0x04, 0x00},
+   {51, 0x07, 0x00},
+   {49, 0x0a, 0x00},
+   {47, 0x11, 0x00},
+   {45, 0x18, 0x00},
+   {43, 0x26, 0x00},
+   {42, 0x36, 0x00},
+   {41, 0xff, 0x00}
+};
+
 /*
  * Description: Calculate data frame transmitting time
  *
@@ -572,254 +652,42 @@ int vnt_exit_deep_sleep(struct vnt_private *priv)

 void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
 {
-   u8 cr_201 = 0x0, cr_206 = 0x0;
+   const struct vnt_threshold *threshold = NULL;
+   u8 length;
+   u8 cr_201, cr_206;
u8 ed_inx = priv->bb_pre_ed_index;

switch (priv->rf_type) {
case RF_AL2230:
case RF_AL2230S:
case RF_AIROHA7230:
-   if (scanning) { /* Max sensitivity */
-   ed_inx = 0;
-   cr_206 = 0x30;
-   break;
-   }
-
-   if (priv->bb_pre_ed_rssi <= 45) {
-   ed_inx = 20;
-   cr_201 = 0xff;
-   } else if (priv-&

Re: [PATCH] staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_* functions

2020-04-01 Thread Oscar Carter
On Tue, Mar 31, 2020 at 01:41:30PM +0300, Dan Carpenter wrote:
> On Thu, Mar 26, 2020 at 06:10:43PM +0100, Oscar Carter wrote:
> > I will make these changes and i will send and incremental patch with the
> > "Fixes:" tag due to the this patch has already been added to the 
> > staging-next
> > branch of the greg staging tree.
>
> Fixes is only for bug fixes, not style issues.
>
Ok, thanks for the explanation.

> regards,
> dan carpenter
>
regards,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Define EnCFG_BBType_MASK as OR between previous defines

2020-04-01 Thread Oscar Carter
On Mon, Mar 30, 2020 at 02:27:14PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Mar 27, 2020 at 05:58:02PM +0100, Oscar Carter wrote:
> > Define the EnCFG_BBType_MASK bit as an OR operation between two previous
> > defines instead of using the OR between two new BIT macros. Thus, the
> > code is more clear.
> >
> > Fixes: a74081b44291 ("staging: vt6656: Use BIT() macro instead of hex 
> > value")
> > Signed-off-by: Oscar Carter 
> > Reviewed-by: Dan Carpenter 
> > ---
> >  drivers/staging/vt6656/mac.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
> > index c532b27de37f..b01d9ee8677e 100644
> > --- a/drivers/staging/vt6656/mac.h
> > +++ b/drivers/staging/vt6656/mac.h
> > @@ -177,7 +177,7 @@
> >  #define EnCFG_BBType_a 0x00
> >  #define EnCFG_BBType_b BIT(0)
> >  #define EnCFG_BBType_g BIT(1)
> > -#define EnCFG_BBType_MASK  (BIT(0) | BIT(1))
> > +#define EnCFG_BBType_MASK  (EnCFG_BBType_b | EnCFG_BBType_g)
>
> This does not "fix" anything, like your "Fixes:" tag implies.  It just
> cleans up the code some more.  Only use Fixes: if it actually fixes a
> problem introduced by a previous patch.
>
Ok, thanks for the explanation.

> Can you remove that line and resend?
>
Yes, I will remove that line, I will create a new version of this patch and I
will resend it.

> thanks.
>
> greg k-h

Thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions

2020-04-01 Thread Oscar Carter
On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
> On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
> > Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
> > registers to can use them in the calls to vnt_mac_reg_bits_on and
> > vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
> > macros and clarify the code.
> >
> > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in 
> > vnt_mac_reg_bits_* functions")
> > Suggested-by: Dan Carpenter 
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/baseband.c |  6 --
> >  drivers/staging/vt6656/card.c |  3 +--
> >  drivers/staging/vt6656/mac.h  | 12 
> >  drivers/staging/vt6656/main_usb.c |  2 +-
> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/baseband.c 
> > b/drivers/staging/vt6656/baseband.c
> > index a19a563d8bcc..dd3c3bf5e8b5 100644
> > --- a/drivers/staging/vt6656/baseband.c
> > +++ b/drivers/staging/vt6656/baseband.c
> > @@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
> > if (ret)
> > goto end;
> >
> > -   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
> > +   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > + PAPEDELAY_B0);
>
> This doesn't clarify anything.  It makes it less clear because someone
> would assume B0 means something but it's just hiding a magic number
> behind a meaningless define.  B0 means BIT(0) which means nothing.  So
> now we have to jump through two hoops to find out that we don't know
> anything.
>
I created this names due to the lack of information about the hardware. I
searched but I did not find anything.

> Just leave it as-is.  Same for the rest.
Ok.

>
> There problem is a hardware spec which explains what this stuff is.
>
It's possible to find a datasheet of this hardware to make this modification
accordingly to the correct bit names of registers ?

> regards,
> dan carpenter
>
Thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_* functions

2020-04-02 Thread Oscar Carter
On Thu, Apr 02, 2020 at 11:58:07AM +0100, Malcolm Priestley wrote:
>
>
> On 02/04/2020 10:19, Quentin Deslandes wrote:
> > On 04/01/20 18:55:38, Oscar Carter wrote:
> > > On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
> > > > On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
> > > > > Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
> > > > > registers to can use them in the calls to vnt_mac_reg_bits_on and
> > > > > vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT()
> > > > > macros and clarify the code.
> > > > >
> > > > > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in 
> > > > > vnt_mac_reg_bits_* functions")
> > > > > Suggested-by: Dan Carpenter 
> > > > > Signed-off-by: Oscar Carter 
> > > > > ---
> > > > >   drivers/staging/vt6656/baseband.c |  6 --
> > > > >   drivers/staging/vt6656/card.c |  3 +--
> > > > >   drivers/staging/vt6656/mac.h  | 12 
> > > > >   drivers/staging/vt6656/main_usb.c |  2 +-
> > > > >   4 files changed, 18 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vt6656/baseband.c 
> > > > > b/drivers/staging/vt6656/baseband.c
> > > > > index a19a563d8bcc..dd3c3bf5e8b5 100644
> > > > > --- a/drivers/staging/vt6656/baseband.c
> > > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > > @@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
> > > > >   if (ret)
> > > > >   goto end;
> > > > >
> > > > > - ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 
> > > > > BIT(0));
> > > > > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > > > > +   PAPEDELAY_B0);
> > > >
> > > > This doesn't clarify anything.  It makes it less clear because someone
> > > > would assume B0 means something but it's just hiding a magic number
> > > > behind a meaningless define.  B0 means BIT(0) which means nothing.  So
> > > > now we have to jump through two hoops to find out that we don't know
> > > > anything.
> > > >
> > > I created this names due to the lack of information about the hardware. I
> > > searched but I did not find anything.
> > >
> > > > Just leave it as-is.  Same for the rest.
> > > Ok.
> > >
> > > >
> > > > There problem is a hardware spec which explains what this stuff is.
> > > >
> > > It's possible to find a datasheet of this hardware to make this 
> > > modification
> > > accordingly to the correct bit names of registers ?
> >
> > I haven't found any so far, if your researches are luckier than mine, I
> > would be interested too. Even getting your hands on the actual device is
> > complicated.
>
> All I can tell you is it related to command above it MAC_REG_ITRTMSET
> without it the device will not associate with access point is probably TX
> timing/power rated.
>
> Other bits in MAC_REG_PAPEDELAY are related to LED function and defined in
> LEDSTS_* in mac.h.
>
> I think it is some kind of enable so something like ITRTMSET_ENABLE would
> do.
>
I think the best for now is leave it as-is due to the lack of information about
bit names of registers.

> Regards
>
> Malcolm

Thanks,

Oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] staging: vt6656: Define EnCFG_BBType_MASK as OR between previous defines

2020-04-02 Thread Oscar Carter
Define the EnCFG_BBType_MASK bit as an OR operation between two previous
defines instead of using the OR between two new BIT macros. Thus, the
code is more clear.

Signed-off-by: Oscar Carter 
Reviewed-by: Dan Carpenter 
Reviewed-by: Quentin Deslandes 
---
Changelog v1 -> v2
- Remove the "Fixes:" tag line.
- Add "Reviewed-by: Quentin Deslandes"

 drivers/staging/vt6656/mac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
index c532b27de37f..b01d9ee8677e 100644
--- a/drivers/staging/vt6656/mac.h
+++ b/drivers/staging/vt6656/mac.h
@@ -177,7 +177,7 @@
 #define EnCFG_BBType_a 0x00
 #define EnCFG_BBType_b BIT(0)
 #define EnCFG_BBType_g BIT(1)
-#define EnCFG_BBType_MASK  (BIT(0) | BIT(1))
+#define EnCFG_BBType_MASK  (EnCFG_BBType_b | EnCFG_BBType_g)
 #define EnCFG_ProtectMdBIT(5)

 /* Bits in the EnhanceCFG_1 register */
--
2.20.1

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


[PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-04 Thread Oscar Carter
Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 3e4bd637849a..a785f91c1566 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include 
 #include 
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,

rate = (unsigned int)vnt_frame_time[tx_rate];

-   if (tx_rate <= 3) {
+   if (tx_rate <= RATE_11M) {
if (preamble_type == 1)
preamble = 96;
else
--
2.20.1

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


[PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M

2020-04-04 Thread Oscar Carter
Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
mismatch. In this way, avoid the possibility of a buffer overflow if
this define is changed in the future to a greater value.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..3e4bd637849a 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -136,7 +136,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
unsigned int preamble;
unsigned int rate = 0;

-   if (tx_rate > RATE_54M)
+   if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
return 0;

rate = (unsigned int)vnt_frame_time[tx_rate];
--
2.20.1

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


[PATCH 0/3] staging: vt6656: Cleanup of the vnt_get_frame_time function

2020-04-04 Thread Oscar Carter
This patch series makes a cleanup of the vnt_get_frame_time function.

The first patch removes the define RATE_54M and changes it for the
ARRAY_SIZE macro. This way avoid possibles issues if the size of the
vnt_frame_time array change in the future but not change accordingly the
RATE_54M constant.

The second patch makes use of the define RATE_11M instead of a magic
number.

The third patch remove unnecessary local variable initialization.

Oscar Carter (3):
  staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

--
2.20.1

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


[PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization

2020-04-04 Thread Oscar Carter
Don't initialize the rate variable as it is set a few lines later.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a785f91c1566..04393ea6287d 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -135,7 +135,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
 {
unsigned int frame_time;
unsigned int preamble;
-   unsigned int rate = 0;
+   unsigned int rate;

if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
return 0;
--
2.20.1

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


Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M

2020-04-06 Thread Oscar Carter
On Mon, Apr 06, 2020 at 02:13:23PM +0300, Dan Carpenter wrote:
> On Sat, Apr 04, 2020 at 04:13:58PM +0200, Oscar Carter wrote:
> > Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
> > mismatch. In this way, avoid the possibility of a buffer overflow if
> > this define is changed in the future to a greater value.
> >
>
> Future proofing is not really a valid reason to change this.

Ok, then I leave it as is.

> We have to assume that future programmers are not idiots.
>
That was not my intention. I'm sorry.

> The only valid reason to do this is readability, but I'm not convinced
> the new version is more readable.
>
Ok.

> regards,
> dan carpenter
>
Thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-06 Thread Oscar Carter
On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > Use the define RATE_11M present in the file "device.h" instead of the
> > magic number 3. So the code is more clear.
> >
> > Signed-off-by: Oscar Carter 
> > Reviewed-by: Dan Carpenter 
> > ---
> >  drivers/staging/vt6656/baseband.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vt6656/baseband.c 
> > b/drivers/staging/vt6656/baseband.c
> > index 3e4bd637849a..a785f91c1566 100644
> > --- a/drivers/staging/vt6656/baseband.c
> > +++ b/drivers/staging/vt6656/baseband.c
> > @@ -24,6 +24,7 @@
> >
> >  #include 
> >  #include 
> > +#include "device.h"
> >  #include "mac.h"
> >  #include "baseband.h"
> >  #include "rf.h"
> > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
> > pkt_type,
> >
> > rate = (unsigned int)vnt_frame_time[tx_rate];
> >
> > -   if (tx_rate <= 3) {
> > +   if (tx_rate <= RATE_11M) {
> > if (preamble_type == 1)
> > preamble = 96;
> >     else
> > --
> > 2.20.1
>
> This doesn't apply to my tree :(
>
Sorry, but I don't understand what it means. This meant that I need to rebase
this patch against your staging-next branch of your staging tree ? Or it means
something else ?

Thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-07 Thread Oscar Carter
On Mon, Apr 06, 2020 at 07:58:08PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 06, 2020 at 06:38:36PM +0200, Oscar Carter wrote:
> > On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > > > Use the define RATE_11M present in the file "device.h" instead of the
> > > > magic number 3. So the code is more clear.
> > > >
> > > > Signed-off-by: Oscar Carter 
> > > > Reviewed-by: Dan Carpenter 
> > > > ---
> > > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/baseband.c 
> > > > b/drivers/staging/vt6656/baseband.c
> > > > index 3e4bd637849a..a785f91c1566 100644
> > > > --- a/drivers/staging/vt6656/baseband.c
> > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > @@ -24,6 +24,7 @@
> > > >
> > > >  #include 
> > > >  #include 
> > > > +#include "device.h"
> > > >  #include "mac.h"
> > > >  #include "baseband.h"
> > > >  #include "rf.h"
> > > > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, 
> > > > u8 pkt_type,
> > > >
> > > > rate = (unsigned int)vnt_frame_time[tx_rate];
> > > >
> > > > -   if (tx_rate <= 3) {
> > > > +   if (tx_rate <= RATE_11M) {
> > > > if (preamble_type == 1)
> > > > preamble = 96;
> > > >     else
> > > > --
> > > > 2.20.1
> > >
> > > This doesn't apply to my tree :(
> > >
> > Sorry, but I don't understand what it means. This meant that I need to 
> > rebase
> > this patch against your staging-next branch of your staging tree ?
>
> Yes, and 3/3 as well, because I dropped the 1/3 patch here.
>
Ok, I will create a new patch series version rebased against your staging-next
branch and I will send it.

> thanks,
>
> greg k-h

thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/2] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-07 Thread Oscar Carter
Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Reviewed-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..092e56668a09 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include 
 #include 
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,

rate = (unsigned int)vnt_frame_time[tx_rate];

-   if (tx_rate <= 3) {
+   if (tx_rate <= RATE_11M) {
if (preamble_type == 1)
preamble = 96;
else
--
2.20.1

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


[PATCH 2/2] staging: vt6656: Remove unnecessary local variable initialization

2020-04-07 Thread Oscar Carter
Don't initialize the rate variable as it is set a few lines later.

Reviewed-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 092e56668a09..5d9bc97916a5 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -135,7 +135,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
 {
unsigned int frame_time;
unsigned int preamble;
-   unsigned int rate = 0;
+   unsigned int rate;

if (tx_rate > RATE_54M)
return 0;
--
2.20.1

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


[PATCH v2 0/2] staging: vt6656: Cleanup of the vnt_get_frame_time function

2020-04-07 Thread Oscar Carter
This patch series makes a cleanup of the vnt_get_frame_time function.

The first patch makes use of the define RATE_11M instead of a magic
number. The second patch remove unnecessary local variable initialization.

Changelog v1 -> v2
- Not use the ARRAY_SIZE macro to compare against the tx_rate variable.

Oscar Carter (2):
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--
2.20.1

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


[PATCH v3 0/2] staging: vt6656: Cleanup of the vnt_get_frame_time function

2020-04-07 Thread Oscar Carter
This patch series makes a cleanup of the vnt_get_frame_time function.

The first patch makes use of the define RATE_11M instead of a magic
number. The second patch remove unnecessary local variable initialization.

Changelog v1 -> v2
- Not use the ARRAY_SIZE macro to compare against the tx_rate variable.

Changelog v2 -> v3
- Use the version number in the subject line of patch 1/2 and 2/2.

Oscar Carter (2):
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--
2.20.1

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


[PATCH v3 1/2] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-07 Thread Oscar Carter
Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Reviewed-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..092e56668a09 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include 
 #include 
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,

rate = (unsigned int)vnt_frame_time[tx_rate];

-   if (tx_rate <= 3) {
+   if (tx_rate <= RATE_11M) {
if (preamble_type == 1)
preamble = 96;
else
--
2.20.1

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


[PATCH v3 2/2] staging: vt6656: Remove unnecessary local variable initialization

2020-04-07 Thread Oscar Carter
Don't initialize the rate variable as it is set a few lines later.

Reviewed-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 092e56668a09..5d9bc97916a5 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -135,7 +135,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,
 {
unsigned int frame_time;
unsigned int preamble;
-   unsigned int rate = 0;
+   unsigned int rate;

if (tx_rate > RATE_54M)
return 0;
--
2.20.1

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


Re: [PATCH v2 0/2] staging: vt6656: Cleanup of the vnt_get_frame_time function

2020-04-07 Thread Oscar Carter
On Tue, Apr 07, 2020 at 06:29:57PM +0200, Oscar Carter wrote:
> This patch series makes a cleanup of the vnt_get_frame_time function.
>
> The first patch makes use of the define RATE_11M instead of a magic
> number. The second patch remove unnecessary local variable initialization.
>
> Changelog v1 -> v2
> - Not use the ARRAY_SIZE macro to compare against the tx_rate variable.
>
> Oscar Carter (2):
>   staging: vt6656: Use define instead of magic number for tx_rate
>   staging: vt6656: Remove unnecessary local variable initialization
>
>  drivers/staging/vt6656/baseband.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> --
> 2.20.1
>
Don't review this patch series as I have sent a new version. Sorry.

Thanks
Oscar Carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] staging: vt6656: Remove duplicate code for the phy->service assignment

2020-04-10 Thread Oscar Carter
Take out the "phy->service" assignment from the if-else statement due to
it's the same for the two branches.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 47f93bf6e07b..b0054f6c07e6 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -233,14 +233,13 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
   : preamble_type == PREAMBLE_SHORT;

phy->signal = vnt_phy_signal[i][j];
+   phy->service = 0x00;

if (pkt_type == PK_TYPE_11B) {
-   phy->service = 0x00;
if (ext_bit)
phy->service |= 0x80;
phy->len = cpu_to_le16((u16)count);
} else {
-   phy->service = 0x00;
phy->len = cpu_to_le16((u16)frame_length);
}
 }
--
2.20.1

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


[PATCH 0/2] staging: vt6656: Refactor the vnt_get_phy_field function

2020-04-10 Thread Oscar Carter
This patch series makes a refactor of the vnt_get_phy_field function
through two patches.

The first one refactors the assignment of the "phy->signal" variable
using a constant array with the correct values for every rate.

The second patch removes duplicate code for the assignment of the
"phy->service" variable by putting it outside the if-else statement due
to it's the same for the two branches.

Oscar Carter (2):
  staging: vt6656: Refactor the assignment of the phy->signal variable
  staging: vt6656: Remove duplicate code for the phy->service assignment

 drivers/staging/vt6656/baseband.c | 104 +++---
 1 file changed, 22 insertions(+), 82 deletions(-)

--
2.20.1

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


[PATCH 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-10 Thread Oscar Carter
Create a constant array with the values of the "phy->signal" for every
rate. Remove all "phy->signal" assignments inside the switch statement
and replace these with a single reading from the new vnt_phy_signal
array.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 101 +++---
 1 file changed, 21 insertions(+), 80 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..47f93bf6e07b 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -115,6 +115,21 @@ static const u16 vnt_frame_time[MAX_RATE] = {
10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
 };

+static const u8 vnt_phy_signal[][2] = {
+   {0x00, 0x00},   /* RATE_1M  */
+   {0x01, 0x09},   /* RATE_2M  */
+   {0x02, 0x0a},   /* RATE_5M  */
+   {0x03, 0x0b},   /* RATE_11M */
+   {0x8b, 0x9b},   /* RATE_6M  */
+   {0x8f, 0x9f},   /* RATE_9M  */
+   {0x8a, 0x9a},   /* RATE_12M */
+   {0x8e, 0x9e},   /* RATE_18M */
+   {0x89, 0x99},   /* RATE_24M */
+   {0x8d, 0x9d},   /* RATE_36M */
+   {0x88, 0x98},   /* RATE_48M */
+   {0x8c, 0x9c}/* RATE_54M */
+};
+
 /*
  * Description: Calculate data frame transmitting time
  *
@@ -183,6 +198,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
u32 count = 0;
u32 tmp;
int ext_bit;
+   int i, j;
u8 preamble_type = priv->preamble_type;

bit_count = frame_length * 8;
@@ -191,27 +207,12 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
switch (tx_rate) {
case RATE_1M:
count = bit_count;
-
-   phy->signal = 0x00;
-
break;
case RATE_2M:
count = bit_count / 2;
-
-   if (preamble_type == 1)
-   phy->signal = 0x09;
-   else
-   phy->signal = 0x01;
-
break;
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);
-
-   if (preamble_type == 1)
-   phy->signal = 0x0a;
-   else
-   phy->signal = 0x02;
-
break;
case RATE_11M:
count = bit_count / 11;
@@ -224,74 +225,14 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
ext_bit = true;
}

-   if (preamble_type == 1)
-   phy->signal = 0x0b;
-   else
-   phy->signal = 0x03;
-
-   break;
-   case RATE_6M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9b;
-   else
-   phy->signal = 0x8b;
-
-   break;
-   case RATE_9M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9f;
-   else
-   phy->signal = 0x8f;
-
-   break;
-   case RATE_12M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9a;
-   else
-   phy->signal = 0x8a;
-
-   break;
-   case RATE_18M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9e;
-   else
-   phy->signal = 0x8e;
-
-   break;
-   case RATE_24M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x99;
-   else
-   phy->signal = 0x89;
-
break;
-   case RATE_36M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9d;
-   else
-   phy->signal = 0x8d;
+   }

-   break;
-   case RATE_48M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x98;
-   else
-   phy->signal = 0x88;
+   i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
+   j = tx_rate > RATE_11M ? pkt_type == PK_TYPE_11A
+  : preamble_type == PREAMBLE_SHORT;

-   break;
-   case RATE_54M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
-   default:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
-   }
+   phy->signal = vnt_phy_signal[i][j];

if (pkt_type == PK_TYPE_11B) {
phy->service = 0x00;
--
2.20.1

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


Re: [PATCH 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-10 Thread Oscar Carter
On Fri, Apr 10, 2020 at 04:37:59PM +0100, Malcolm Priestley wrote:
>
>
> On 10/04/2020 12:28, Oscar Carter wrote:
> > Create a constant array with the values of the "phy->signal" for every
> > rate. Remove all "phy->signal" assignments inside the switch statement
> > and replace these with a single reading from the new vnt_phy_signal
> > array.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >   drivers/staging/vt6656/baseband.c | 101 +++---
> >   1 file changed, 21 insertions(+), 80 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/baseband.c 
> > b/drivers/staging/vt6656/baseband.c
> > index a19a563d8bcc..47f93bf6e07b 100644
> > --- a/drivers/staging/vt6656/baseband.c
> > +++ b/drivers/staging/vt6656/baseband.c
> > @@ -115,6 +115,21 @@ static const u16 vnt_frame_time[MAX_RATE] = {
> > 10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
> >   };
>
> Actually you don't need the second values

Great.
> >
> > +static const u8 vnt_phy_signal[][2] = {
> > +   {0x00, 0x00},   /* RATE_1M  */
> The driver would never attempt use preamble at this rate
> so it's safe to include in with the next 3 rates
>
> > +   {0x01, 0x09},   /* RATE_2M  */
> > +   {0x02, 0x0a},   /* RATE_5M  */
> > +   {0x03, 0x0b},   /* RATE_11M */
> just |= BIT(3) for preamble.
>
Ok, I apply this OR operation.

> > +   {0x8b, 0x9b},   /* RATE_6M  */
> > +   {0x8f, 0x9f},   /* RATE_9M  */
> > +   {0x8a, 0x9a},   /* RATE_12M */
> > +   {0x8e, 0x9e},   /* RATE_18M */
> > +   {0x89, 0x99},   /* RATE_24M */
> > +   {0x8d, 0x9d},   /* RATE_36M */
> > +   {0x88, 0x98},   /* RATE_48M */
> > +   {0x8c, 0x9c}/* RATE_54M */
>
> Again just |= BIT(4) for PK_TYPE_11A
>
And this one.

> Regards
>
> Malcolm

I will create a new version of this patch and I will resend it.

Thanks,

Oscar Carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-10 Thread Oscar Carter
On Fri, Apr 10, 2020 at 05:40:52PM +0100, Malcolm Priestley wrote:
>
>
> On 10/04/2020 16:59, Oscar Carter wrote:
> > On Fri, Apr 10, 2020 at 04:37:59PM +0100, Malcolm Priestley wrote:
> > >
> > >
> > > On 10/04/2020 12:28, Oscar Carter wrote:
> > > > Create a constant array with the values of the "phy->signal" for every
> > > > rate. Remove all "phy->signal" assignments inside the switch statement
> > > > and replace these with a single reading from the new vnt_phy_signal
> > > > array.
> > > >
> > > > Signed-off-by: Oscar Carter 
> > > > ---
> > > >drivers/staging/vt6656/baseband.c | 101 
> > > > +++---
> > > >1 file changed, 21 insertions(+), 80 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/baseband.c 
> > > > b/drivers/staging/vt6656/baseband.c
> > > > index a19a563d8bcc..47f93bf6e07b 100644
> > > > --- a/drivers/staging/vt6656/baseband.c
> > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > @@ -115,6 +115,21 @@ static const u16 vnt_frame_time[MAX_RATE] = {
> > > > 10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
> > > >};
> > >
> > > Actually you don't need the second values
> >
> > Great.
> > > >
> > > > +static const u8 vnt_phy_signal[][2] = {
> > > > +   {0x00, 0x00},   /* RATE_1M  */
> > > The driver would never attempt use preamble at this rate
> > > so it's safe to include in with the next 3 rates
> Sorry got this wrong the driver is trying to do preamble (short)
> at this rate and it is not working.
>
> So don't apply it to RATE_1M rate.

Ok, I take it into account.
>
> Regards
>
> Malcolm
>
Thanks,

Oscar Carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-11 Thread Oscar Carter
Create a constant array with the values of the "phy->signal" for every
rate. Remove all "phy->signal" assignments inside the switch statement
and replace these with a single reading from the new vnt_phy_signal
array.

The constant array can be of one dimension because the OR mask with
BIT(3) or BIT(4) allow obtain a second value according to the rate,
the preamble_type and the pkt_type.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 105 --
 1 file changed, 26 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..05cc4797df52 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -115,6 +115,21 @@ static const u16 vnt_frame_time[MAX_RATE] = {
10, 20, 55, 110, 24, 36, 48, 72, 96, 144, 192, 216
 };

+static const u8 vnt_phy_signal[] = {
+   0x00,   /* RATE_1M  */
+   0x01,   /* RATE_2M  */
+   0x02,   /* RATE_5M  */
+   0x03,   /* RATE_11M */
+   0x8b,   /* RATE_6M  */
+   0x8f,   /* RATE_9M  */
+   0x8a,   /* RATE_12M */
+   0x8e,   /* RATE_18M */
+   0x89,   /* RATE_24M */
+   0x8d,   /* RATE_36M */
+   0x88,   /* RATE_48M */
+   0x8c/* RATE_54M */
+};
+
 /*
  * Description: Calculate data frame transmitting time
  *
@@ -183,6 +198,8 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
u32 count = 0;
u32 tmp;
int ext_bit;
+   int i;
+   u8 mask = 0;
u8 preamble_type = priv->preamble_type;

bit_count = frame_length * 8;
@@ -191,27 +208,12 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
switch (tx_rate) {
case RATE_1M:
count = bit_count;
-
-   phy->signal = 0x00;
-
break;
case RATE_2M:
count = bit_count / 2;
-
-   if (preamble_type == 1)
-   phy->signal = 0x09;
-   else
-   phy->signal = 0x01;
-
break;
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);
-
-   if (preamble_type == 1)
-   phy->signal = 0x0a;
-   else
-   phy->signal = 0x02;
-
break;
case RATE_11M:
count = bit_count / 11;
@@ -224,75 +226,20 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
ext_bit = true;
}

-   if (preamble_type == 1)
-   phy->signal = 0x0b;
-   else
-   phy->signal = 0x03;
-
-   break;
-   case RATE_6M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9b;
-   else
-   phy->signal = 0x8b;
-
break;
-   case RATE_9M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9f;
-   else
-   phy->signal = 0x8f;
-
-   break;
-   case RATE_12M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9a;
-   else
-   phy->signal = 0x8a;
-
-   break;
-   case RATE_18M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9e;
-   else
-   phy->signal = 0x8e;
-
-   break;
-   case RATE_24M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x99;
-   else
-   phy->signal = 0x89;
-
-   break;
-   case RATE_36M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9d;
-   else
-   phy->signal = 0x8d;
-
-   break;
-   case RATE_48M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x98;
-   else
-   phy->signal = 0x88;
+   }

-   break;
-   case RATE_54M:
+   if (tx_rate > RATE_11M) {
if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
-   default:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
+   mask = BIT(4);
+   } else if (tx_rate > RATE_1M) {
+   if (preamble_type == PREAMBLE_SHORT)
+   mask = BIT(3);
}

+   i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
+   phy->signal = vnt_phy_signal[

[PATCH v2 0/2] staging: vt6656: Refactor the vnt_get_phy_field function

2020-04-11 Thread Oscar Carter
This patch series makes a refactor of the vnt_get_phy_field function
through two patches.

The first one refactors the assignment of the "phy->signal" variable
using a constant array with the correct values for every rate.

The second patch removes duplicate code for the assignment of the
"phy->service" variable by putting it outside the if-else statement due
to it's the same for the two branches.

Changelog v1 -> v2:
- Remove one dimension from the constant array for the "phy->signal"
  values and use an OR mask instead of the second array dimension as
  Malcolm Priestley has suggested.

Oscar Carter (2):
  staging: vt6656: Refactor the assignment of the phy->signal variable
  staging: vt6656: Remove duplicate code for the phy->service assignment

 drivers/staging/vt6656/baseband.c | 108 --
 1 file changed, 27 insertions(+), 81 deletions(-)

--
2.20.1

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


[PATCH v2 2/2] staging: vt6656: Remove duplicate code for the phy->service assignment

2020-04-11 Thread Oscar Carter
Take out the "phy->service" assignment from the if-else statement due to
it's the same for the two branches.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 05cc4797df52..c8b3cc84da6c 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -239,14 +239,13 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,

i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
phy->signal = vnt_phy_signal[i] | mask;
+   phy->service = 0x00;

if (pkt_type == PK_TYPE_11B) {
-   phy->service = 0x00;
if (ext_bit)
phy->service |= 0x80;
phy->len = cpu_to_le16((u16)count);
} else {
-   phy->service = 0x00;
phy->len = cpu_to_le16((u16)frame_length);
}
 }
--
2.20.1

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


[PATCH 0/2] staging: vt6656: Refactor the vnt_vt3184_init function

2020-04-11 Thread Oscar Carter
This patch series makes a refactor of the vnt_vt3184_init function through
two patches.

The first one removes duplicate code in the if statements because
different branches are almost the same.

The second patch remove unnecessary local variable initialization.

Oscar Carter (2):
  staging: vt6656: Remove duplicate code in vnt_vt3184_init function
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 54 ---
 1 file changed, 13 insertions(+), 41 deletions(-)

--
2.20.1

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


[PATCH 2/2] staging: vt6656: Remove unnecessary local variable initialization

2020-04-11 Thread Oscar Carter
Don't initialize the ret variable as it is set a few lines later.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 276210a7284e..10d1f2cbb3d9 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -352,7 +352,7 @@ int vnt_set_antenna_mode(struct vnt_private *priv, u8 
antenna_mode)

 int vnt_vt3184_init(struct vnt_private *priv)
 {
-   int ret = 0;
+   int ret;
u16 length;
u8 *addr;
u8 data;
--
2.20.1

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


[PATCH 1/2] staging: vt6656: Remove duplicate code in vnt_vt3184_init function

2020-04-11 Thread Oscar Carter
Remove duplicate code in "if" statements because different branches are
almost the same.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 52 +++
 1 file changed, 12 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..276210a7284e 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -366,23 +366,15 @@ int vnt_vt3184_init(struct vnt_private *priv)

dev_dbg(&priv->usb->dev, "RF Type %d\n", priv->rf_type);

-   if (priv->rf_type == RF_AL2230 ||
-   priv->rf_type == RF_AL2230S) {
+   if ((priv->rf_type == RF_AL2230) ||
+   (priv->rf_type == RF_AL2230S) ||
+   (priv->rf_type == RF_AIROHA7230)) {
priv->bb_rx_conf = vnt_vt3184_al2230[10];
length = sizeof(vnt_vt3184_al2230);
addr = vnt_vt3184_al2230;

-   priv->bb_vga[0] = 0x1C;
-   priv->bb_vga[1] = 0x10;
-   priv->bb_vga[2] = 0x0;
-   priv->bb_vga[3] = 0x0;
-
-   } else if (priv->rf_type == RF_AIROHA7230) {
-   priv->bb_rx_conf = vnt_vt3184_al2230[10];
-   length = sizeof(vnt_vt3184_al2230);
-   addr = vnt_vt3184_al2230;
-
-   addr[0xd7] = 0x06;
+   if (priv->rf_type == RF_AIROHA7230)
+   addr[0xd7] = 0x06;

priv->bb_vga[0] = 0x1c;
priv->bb_vga[1] = 0x10;
@@ -390,22 +382,8 @@ int vnt_vt3184_init(struct vnt_private *priv)
priv->bb_vga[3] = 0x0;

} else if ((priv->rf_type == RF_VT3226) ||
-   (priv->rf_type == RF_VT3226D0)) {
-   priv->bb_rx_conf = vnt_vt3184_vt3226d0[10];
-   length = sizeof(vnt_vt3184_vt3226d0);
-   addr = vnt_vt3184_vt3226d0;
-
-   priv->bb_vga[0] = 0x20;
-   priv->bb_vga[1] = 0x10;
-   priv->bb_vga[2] = 0x0;
-   priv->bb_vga[3] = 0x0;
-
-   /* Fix VT3226 DFC system timing issue */
-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL2,
- SOFTPWRCTL_RFLEOPT);
-   if (ret)
-   goto end;
-   } else if (priv->rf_type == RF_VT3342A0) {
+  (priv->rf_type == RF_VT3226D0) ||
+  (priv->rf_type == RF_VT3342A0)) {
priv->bb_rx_conf = vnt_vt3184_vt3226d0[10];
length = sizeof(vnt_vt3184_vt3226d0);
addr = vnt_vt3184_vt3226d0;
@@ -435,19 +413,13 @@ int vnt_vt3184_init(struct vnt_private *priv)
if (ret)
goto end;

-   if (priv->rf_type == RF_VT3226 ||
-   priv->rf_type == RF_VT3342A0) {
-   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
-MAC_REG_ITRTMSET, 0x23);
-   if (ret)
-   goto end;
+   if ((priv->rf_type == RF_VT3226) ||
+   (priv->rf_type == RF_VT3342A0) ||
+   (priv->rf_type == RF_VT3226D0)) {
+   data = (priv->rf_type == RF_VT3226D0) ? 0x11 : 0x23;

-   ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
-   if (ret)
-   goto end;
-   } else if (priv->rf_type == RF_VT3226D0) {
ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
-MAC_REG_ITRTMSET, 0x11);
+MAC_REG_ITRTMSET, data);
if (ret)
goto end;

--
2.20.1

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


[PATCH] staging: vt6656: Use BIT() macro instead of bit shift operator

2020-04-12 Thread Oscar Carter
Use the BIT() macro instead of the bit left shift operator. So the code
is more clear.

It's safe to remove the casting to u16 type because the value obtained
never exceeds 16 bits. So the casting is unnecessary.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index dc3ab10eb630..14e6c71f122c 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -92,7 +92,7 @@ static u16 vnt_get_cck_rate(struct vnt_private *priv, u16 
rate_idx)
u16 ui = rate_idx;

while (ui > RATE_1M) {
-   if (priv->basic_rates & (1 << ui))
+   if (priv->basic_rates & BIT(ui))
return ui;
ui--;
}
@@ -129,7 +129,7 @@ static u16 vnt_get_ofdm_rate(struct vnt_private *priv, u16 
rate_idx)
}

while (ui > RATE_11M) {
-   if (priv->basic_rates & (1 << ui)) {
+   if (priv->basic_rates & BIT(ui)) {
dev_dbg(&priv->usb->dev, "%s rate: %d\n",
__func__, ui);
return ui;
@@ -420,7 +420,7 @@ void vnt_update_top_rates(struct vnt_private *priv)

/*Determines the highest basic rate.*/
for (i = RATE_54M; i >= RATE_6M; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
+   if (priv->basic_rates & BIT(i)) {
top_ofdm = i;
break;
}
@@ -429,7 +429,7 @@ void vnt_update_top_rates(struct vnt_private *priv)
priv->top_ofdm_basic_rate = top_ofdm;

for (i = RATE_11M;; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
+   if (priv->basic_rates & BIT(i)) {
top_cck = i;
break;
}
--
2.20.1

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


[PATCH] staging: vt6656: Return error code in vnt_rf_write_embedded function

2020-04-13 Thread Oscar Carter
Use the error code returned by the vnt_control_out function as the
returned value of the vnt_rf_write_embedded function instead of a
boolean value.

Then, fix all vnt_rf_write_embedded calls removing the "and" operations
and replace with a direct assignment to the ret variable and add a check
condition after every call.

Also replace the boolean values true or false in the vnt_rf_set_txpower
function to 0 or error code EINVAL to follow the coding style guide.

The vnt_rf_set_txpower function is called only in the vnt_rf_setpower
function that already returns error codes. The calls to this function
(vnt_rf_set_txpower) not use the returned values, so they not need to be
fixed.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/rf.c | 99 -
 1 file changed, 64 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 4f9aba0f21b0..5270ef511af9 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -21,6 +21,7 @@
  *
  */

+#include 
 #include "mac.h"
 #include "rf.h"
 #include "baseband.h"
@@ -531,10 +532,8 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 
data)
reg_data[2] = (u8)(data >> 16);
reg_data[3] = (u8)(data >> 24);

-   vnt_control_out(priv, MESSAGE_TYPE_WRITE_IFRF,
-   0, 0, ARRAY_SIZE(reg_data), reg_data);
-
-   return true;
+   return vnt_control_out(priv, MESSAGE_TYPE_WRITE_IFRF, 0, 0,
+  ARRAY_SIZE(reg_data), reg_data);
 }

 /* Set Tx power by rate and channel number */
@@ -603,14 +602,14 @@ static u8 vnt_rf_addpower(struct vnt_private *priv)
 int vnt_rf_set_txpower(struct vnt_private *priv, u8 power, u32 rate)
 {
u32 power_setting = 0;
-   int ret = true;
+   int ret = 0;

power += vnt_rf_addpower(priv);
if (power > VNT_RF_MAX_POWER)
power = VNT_RF_MAX_POWER;

if (priv->power == power)
-   return true;
+   return 0;

priv->power = power;

@@ -618,35 +617,50 @@ int vnt_rf_set_txpower(struct vnt_private *priv, u8 
power, u32 rate)
case RF_AL2230:
power_setting = 0x0404090 | (power << 12);

-   ret &= vnt_rf_write_embedded(priv, power_setting);
+   ret = vnt_rf_write_embedded(priv, power_setting);
+   if (ret)
+   return ret;

if (rate <= RATE_11M)
-   ret &= vnt_rf_write_embedded(priv, 0x0001b400);
+   ret = vnt_rf_write_embedded(priv, 0x0001b400);
else
-   ret &= vnt_rf_write_embedded(priv, 0x0005a400);
+   ret = vnt_rf_write_embedded(priv, 0x0005a400);
+
break;
case RF_AL2230S:
power_setting = 0x0404090 | (power << 12);

-   ret &= vnt_rf_write_embedded(priv, power_setting);
+   ret = vnt_rf_write_embedded(priv, power_setting);
+   if (ret)
+   return ret;

if (rate <= RATE_11M) {
-   ret &= vnt_rf_write_embedded(priv, 0x040c1400);
-   ret &= vnt_rf_write_embedded(priv, 0x00299b00);
+   ret = vnt_rf_write_embedded(priv, 0x040c1400);
+   if (ret)
+   return ret;
+
+   ret = vnt_rf_write_embedded(priv, 0x00299b00);
} else {
-   ret &= vnt_rf_write_embedded(priv, 0x0005a400);
-   ret &= vnt_rf_write_embedded(priv, 0x00099b00);
+   ret = vnt_rf_write_embedded(priv, 0x0005a400);
+   if (ret)
+   return ret;
+
+   ret = vnt_rf_write_embedded(priv, 0x00099b00);
}
+
break;

case RF_AIROHA7230:
if (rate <= RATE_11M)
-   ret &= vnt_rf_write_embedded(priv, 0x111bb900);
+   ret = vnt_rf_write_embedded(priv, 0x111bb900);
else
-   ret &= vnt_rf_write_embedded(priv, 0x221bb900);
+   ret = vnt_rf_write_embedded(priv, 0x221bb900);
+
+   if (ret)
+   return ret;

if (power >= AL7230_PWR_IDX_LEN)
-   return false;
+   return -EINVAL;

/*
 * 0x080F1B00 for 3 wire control TxGain(D10)
@@ -654,61 +668,76 @@ int vnt_rf_set_txpower(struct vnt_private *priv, u8 
power, u32 rate)
 */
power_setting = 0x080c0b00 | (power << 12);

-   ret &= vnt_rf_write_embedded(priv, power_setting);
-
+   ret = vnt_

[RFC] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-13 Thread Oscar Carter
Use a formula to calculate the return value of the vnt_rf_addpower
function instead of the "if" statement with literal values for every
case.

Signed-off-by: Oscar Carter 
---
What is the better approach for this function ? Leave it as is or use
a formula although it is less clear.

I prefer the formula as it is a more compact function.
What do you think ? Feedback wellcome.

Thanks,
Oscar Carter

 drivers/staging/vt6656/rf.c | 20 +++-
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 4f9aba0f21b0..3b200d7290a5 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -575,28 +575,14 @@ int vnt_rf_setpower(struct vnt_private *priv, u32 rate, 
u32 channel)

 static u8 vnt_rf_addpower(struct vnt_private *priv)
 {
+   s32 base;
s32 rssi = -priv->current_rssi;

if (!rssi)
return 7;

-   if (priv->rf_type == RF_VT3226D0) {
-   if (rssi < -70)
-   return 9;
-   else if (rssi < -65)
-   return 7;
-   else if (rssi < -60)
-   return 5;
-   } else {
-   if (rssi < -80)
-   return 9;
-   else if (rssi < -75)
-   return 7;
-   else if (rssi < -70)
-   return 5;
-   }
-
-   return 0;
+   base = (priv->rf_type == RF_VT3226D0) ? -60 : -70;
+   return (rssi < base--) ? ((rssi - base) / -5) * 2 + 5 : 0;
 }

 /* Set Tx power by power level and rate */
--
2.20.1

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


Re: [PATCH v3 1/2] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-13 Thread Oscar Carter
On Mon, Apr 13, 2020 at 02:56:33PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 07, 2020 at 06:39:14PM +0200, Oscar Carter wrote:
> > Use the define RATE_11M present in the file "device.h" instead of the
> > magic number 3. So the code is more clear.
> >
> > Reviewed-by: Dan Carpenter 
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/baseband.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
>
> This patch did not apply to my tree, please rebase and resend.
>
I need to rebase only this patch for this serie so, it's necessary to send all
the serie or only this patch?

If it's only this patch I need to indicate v4 in the subject or a v2 due it's
related only with this patch?

> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-13 Thread Oscar Carter
On Mon, Apr 13, 2020 at 02:56:16PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> > Create a constant array with the values of the "phy->signal" for every
> > rate. Remove all "phy->signal" assignments inside the switch statement
> > and replace these with a single reading from the new vnt_phy_signal
> > array.
> >
> > The constant array can be of one dimension because the OR mask with
> > BIT(3) or BIT(4) allow obtain a second value according to the rate,
> > the preamble_type and the pkt_type.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> >  drivers/staging/vt6656/baseband.c | 105 --
> >  1 file changed, 26 insertions(+), 79 deletions(-)
>
> This series did not apply to my tree, please rebase and resend.

Rebase the patchs is a normal process in the development or am I doing something
wrong ?

>
> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 1/2] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-13 Thread Oscar Carter
On Mon, Apr 13, 2020 at 04:29:07PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 13, 2020 at 04:13:15PM +0200, Oscar Carter wrote:
> > On Mon, Apr 13, 2020 at 02:56:33PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Apr 07, 2020 at 06:39:14PM +0200, Oscar Carter wrote:
> > > > Use the define RATE_11M present in the file "device.h" instead of the
> > > > magic number 3. So the code is more clear.
> > > >
> > > > Reviewed-by: Dan Carpenter 
> > > > Signed-off-by: Oscar Carter 
> > > > ---
> > > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > This patch did not apply to my tree, please rebase and resend.
> > >
> > I need to rebase only this patch for this serie so, it's necessary to send 
> > all
> > the serie or only this patch?
>
> If I applied the other one, just this patch.
>
> > If it's only this patch I need to indicate v4 in the subject or a v2 due 
> > it's
> > related only with this patch?
>
> As so many of your patches were rejected because of this, rebase them
> all, and resend them all as a single patch series, so that I know what
> order to apply them in and have a chance to get it right :)

Ok, I will create a patch series with all the rejected patches rebased.
>
> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 1/2] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-13 Thread Oscar Carter
On Mon, Apr 13, 2020 at 04:32:58PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 13, 2020 at 04:25:17PM +0200, Oscar Carter wrote:
> > On Mon, Apr 13, 2020 at 02:56:16PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Apr 11, 2020 at 02:26:09PM +0200, Oscar Carter wrote:
> > > > Create a constant array with the values of the "phy->signal" for every
> > > > rate. Remove all "phy->signal" assignments inside the switch statement
> > > > and replace these with a single reading from the new vnt_phy_signal
> > > > array.
> > > >
> > > > The constant array can be of one dimension because the OR mask with
> > > > BIT(3) or BIT(4) allow obtain a second value according to the rate,
> > > > the preamble_type and the pkt_type.
> > > >
> > > > Signed-off-by: Oscar Carter 
> > > > ---
> > > >  drivers/staging/vt6656/baseband.c | 105 --
> > > >  1 file changed, 26 insertions(+), 79 deletions(-)
> > >
> > > This series did not apply to my tree, please rebase and resend.
> >
> > Rebase the patchs is a normal process in the development or am I doing 
> > something
> > wrong ?
>
> It's normal when multiple people are working on the same area with lots
> of patches flying around.  Not a problem, it doesn't bother me at all :)

Thanks for the clarification about this theme.

>
> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 5/5] staging: vt6656: Remove duplicate code for the phy->service assignment

2020-04-14 Thread Oscar Carter
Take out the "phy->service" assignment from the if-else statement due to
it's the same for the two branches.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 3b6f2bcf91a7..13b91d7fc6db 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -320,14 +320,13 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,

i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
phy->signal = vnt_phy_signal[i] | mask;
+   phy->service = 0x00;

if (pkt_type == PK_TYPE_11B) {
-   phy->service = 0x00;
if (ext_bit)
phy->service |= 0x80;
phy->len = cpu_to_le16((u16)count);
} else {
-   phy->service = 0x00;
phy->len = cpu_to_le16((u16)frame_length);
}
 }
--
2.20.1

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


[PATCH 1/5] staging: vt6656: Return error code in vnt_rf_write_embedded function

2020-04-14 Thread Oscar Carter
Use the error code returned by the vnt_control_out function as the
returned value of the vnt_rf_write_embedded function instead of a
boolean value.

Then, fix all vnt_rf_write_embedded calls removing the "and" operations
and replace with a direct assignment to the ret variable and add a check
condition after every call.

Also replace the boolean values true or false in the vnt_rf_set_txpower
function to 0 or error code EINVAL to follow the coding style guide.

The vnt_rf_set_txpower function is called only in the vnt_rf_setpower
function that already returns error codes. The calls to this function
(vnt_rf_set_txpower) not use the returned values, so they not need to be
fixed.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/rf.c | 99 -
 1 file changed, 64 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index a00179bd4c2e..06fa8867cfa3 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -21,6 +21,7 @@
  *
  */

+#include 
 #include "mac.h"
 #include "rf.h"
 #include "baseband.h"
@@ -531,10 +532,8 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 
data)
reg_data[2] = (u8)(data >> 16);
reg_data[3] = (u8)(data >> 24);

-   vnt_control_out(priv, MESSAGE_TYPE_WRITE_IFRF,
-   0, 0, ARRAY_SIZE(reg_data), reg_data);
-
-   return true;
+   return vnt_control_out(priv, MESSAGE_TYPE_WRITE_IFRF, 0, 0,
+  ARRAY_SIZE(reg_data), reg_data);
 }

 static u8 vnt_rf_addpower(struct vnt_private *priv)
@@ -568,14 +567,14 @@ static int vnt_rf_set_txpower(struct vnt_private *priv, 
u8 power,
  struct ieee80211_channel *ch)
 {
u32 power_setting = 0;
-   int ret = true;
+   int ret = 0;

power += vnt_rf_addpower(priv);
if (power > VNT_RF_MAX_POWER)
power = VNT_RF_MAX_POWER;

if (priv->power == power)
-   return true;
+   return 0;

priv->power = power;

@@ -583,35 +582,50 @@ static int vnt_rf_set_txpower(struct vnt_private *priv, 
u8 power,
case RF_AL2230:
power_setting = 0x0404090 | (power << 12);

-   ret &= vnt_rf_write_embedded(priv, power_setting);
+   ret = vnt_rf_write_embedded(priv, power_setting);
+   if (ret)
+   return ret;

if (ch->flags & IEEE80211_CHAN_NO_OFDM)
-   ret &= vnt_rf_write_embedded(priv, 0x0001b400);
+   ret = vnt_rf_write_embedded(priv, 0x0001b400);
else
-   ret &= vnt_rf_write_embedded(priv, 0x0005a400);
+   ret = vnt_rf_write_embedded(priv, 0x0005a400);
+
break;
case RF_AL2230S:
power_setting = 0x0404090 | (power << 12);

-   ret &= vnt_rf_write_embedded(priv, power_setting);
+   ret = vnt_rf_write_embedded(priv, power_setting);
+   if (ret)
+   return ret;

if (ch->flags & IEEE80211_CHAN_NO_OFDM) {
-   ret &= vnt_rf_write_embedded(priv, 0x040c1400);
-   ret &= vnt_rf_write_embedded(priv, 0x00299b00);
+   ret = vnt_rf_write_embedded(priv, 0x040c1400);
+   if (ret)
+   return ret;
+
+   ret = vnt_rf_write_embedded(priv, 0x00299b00);
} else {
-   ret &= vnt_rf_write_embedded(priv, 0x0005a400);
-   ret &= vnt_rf_write_embedded(priv, 0x00099b00);
+   ret = vnt_rf_write_embedded(priv, 0x0005a400);
+   if (ret)
+   return ret;
+
+   ret = vnt_rf_write_embedded(priv, 0x00099b00);
}
+
break;

case RF_AIROHA7230:
if (ch->flags & IEEE80211_CHAN_NO_OFDM)
-   ret &= vnt_rf_write_embedded(priv, 0x111bb900);
+   ret = vnt_rf_write_embedded(priv, 0x111bb900);
else
-   ret &= vnt_rf_write_embedded(priv, 0x221bb900);
+   ret = vnt_rf_write_embedded(priv, 0x221bb900);
+
+   if (ret)
+   return ret;

if (power >= AL7230_PWR_IDX_LEN)
-   return false;
+   return -EINVAL;

/*
 * 0x080F1B00 for 3 wire control TxGain(D10)
@@ -619,61 +633,76 @@ static int vnt_rf_set_txpower(struct vnt_private *priv, 
u8 power,
 */
power_setting = 0x080c0b00 | (power << 12);

-   ret &= vnt_rf_writ

[PATCH 2/5] staging: vt6656: Use BIT() macro instead of bit shift operator

2020-04-14 Thread Oscar Carter
Use the BIT() macro instead of the bit left shift operator. So the code
is more clear.

It's safe to remove the casting to u16 type because the value obtained
never exceeds 16 bits. So the casting is unnecessary.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 9bd37e57c727..f8bfadd4b506 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -228,7 +228,7 @@ void vnt_update_top_rates(struct vnt_private *priv)

/*Determines the highest basic rate.*/
for (i = RATE_54M; i >= RATE_6M; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
+   if (priv->basic_rates & BIT(i)) {
top_ofdm = i;
break;
}
@@ -237,7 +237,7 @@ void vnt_update_top_rates(struct vnt_private *priv)
priv->top_ofdm_basic_rate = top_ofdm;

for (i = RATE_11M;; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
+   if (priv->basic_rates & BIT(i)) {
top_cck = i;
break;
}
--
2.20.1

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


[PATCH 3/5] staging: vt6656: Use define instead of magic number for tx_rate

2020-04-14 Thread Oscar Carter
Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Reviewed-by: Dan Carpenter 
Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index e0352405e4cf..149c9bba7108 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include 
 #include 
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -221,7 +222,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 
pkt_type,

rate = (unsigned int)vnt_frame_time[tx_rate];

-   if (tx_rate <= 3) {
+   if (tx_rate <= RATE_11M) {
if (preamble_type == PREAMBLE_SHORT)
preamble = 96;
else
--
2.20.1

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


[PATCH 4/5] staging: vt6656: Refactor the assignment of the phy->signal variable

2020-04-14 Thread Oscar Carter
Create a constant array with the values of the "phy->signal" for every
rate. Remove all "phy->signal" assignments inside the switch statement
and replace these with a single reading from the new vnt_phy_signal
array.

The constant array can be of one dimension because the OR mask with
BIT(3) or BIT(4) allow obtain a second value according to the rate,
the preamble_type and the pkt_type.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 105 --
 1 file changed, 26 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index 149c9bba7108..3b6f2bcf91a7 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -196,6 +196,21 @@ static const struct vnt_threshold vt3342_vnt_threshold[] = 
{
{41, 0xff, 0x00}
 };

+static const u8 vnt_phy_signal[] = {
+   0x00,   /* RATE_1M  */
+   0x01,   /* RATE_2M  */
+   0x02,   /* RATE_5M  */
+   0x03,   /* RATE_11M */
+   0x8b,   /* RATE_6M  */
+   0x8f,   /* RATE_9M  */
+   0x8a,   /* RATE_12M */
+   0x8e,   /* RATE_18M */
+   0x89,   /* RATE_24M */
+   0x8d,   /* RATE_36M */
+   0x88,   /* RATE_48M */
+   0x8c/* RATE_54M */
+};
+
 /*
  * Description: Calculate data frame transmitting time
  *
@@ -264,6 +279,8 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
u32 count = 0;
u32 tmp;
int ext_bit;
+   int i;
+   u8 mask = 0;
u8 preamble_type = priv->preamble_type;

bit_count = frame_length * 8;
@@ -272,27 +289,12 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
switch (tx_rate) {
case RATE_1M:
count = bit_count;
-
-   phy->signal = 0x00;
-
break;
case RATE_2M:
count = bit_count / 2;
-
-   if (preamble_type == PREAMBLE_SHORT)
-   phy->signal = 0x09;
-   else
-   phy->signal = 0x01;
-
break;
case RATE_5M:
count = DIV_ROUND_UP(bit_count * 10, 55);
-
-   if (preamble_type == PREAMBLE_SHORT)
-   phy->signal = 0x0a;
-   else
-   phy->signal = 0x02;
-
break;
case RATE_11M:
count = bit_count / 11;
@@ -305,75 +307,20 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
ext_bit = true;
}

-   if (preamble_type == PREAMBLE_SHORT)
-   phy->signal = 0x0b;
-   else
-   phy->signal = 0x03;
-
-   break;
-   case RATE_6M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9b;
-   else
-   phy->signal = 0x8b;
-
-   break;
-   case RATE_9M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9f;
-   else
-   phy->signal = 0x8f;
-
-   break;
-   case RATE_12M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9a;
-   else
-   phy->signal = 0x8a;
-
break;
-   case RATE_18M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9e;
-   else
-   phy->signal = 0x8e;
-
-   break;
-   case RATE_24M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x99;
-   else
-   phy->signal = 0x89;
-
-   break;
-   case RATE_36M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9d;
-   else
-   phy->signal = 0x8d;
-
-   break;
-   case RATE_48M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x98;
-   else
-   phy->signal = 0x88;
+   }

-   break;
-   case RATE_54M:
-   if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
-   default:
+   if (tx_rate > RATE_11M) {
if (pkt_type == PK_TYPE_11A)
-   phy->signal = 0x9c;
-   else
-   phy->signal = 0x8c;
-   break;
+   mask = BIT(4);
+   } else if (tx_rate > RATE_1M) {
+   if (preamble_type == PREAMBLE_SHORT)
+   mask = BIT(3);
}

+   i = tx_rate > RATE_54M ? RATE_54M : tx_rate;
+   phy

[PATCH 0/5] staging: vt6656: Rebase all rejected patches

2020-04-14 Thread Oscar Carter
This patch series makes a "rebase" of all the patches that were
previously rejected because they did not apply.

The first patch uses the error code returned by the vnt_control_out
function as the returned value of the vnt_rf_write_embedded function
instead of a boolean value. This way, the return value is an error code.
Also, fix all the vnt_rf_write_embedded calls accordingly.

The second patch uses the BIT() macro instead of the bit left shift
operator.

The third patch use the define RATE_11M present in the file "device.h"
instead of the magic number 3.

The fourth patch creates a constant array with the values of the
"phy->signal" variable for every rate and makes a refactor of the
assignment of this variable.

The fifth path takes out the "phy->service" assignment from the if-else
statement due to it's the same for the two branches.

Oscar Carter (5):
  staging: vt6656: Return error code in vnt_rf_write_embedded function
  staging: vt6656: Use BIT() macro instead of bit shift operator
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Refactor the assignment of the phy->signal variable
  staging: vt6656: Remove duplicate code for the phy->service assignment

 drivers/staging/vt6656/baseband.c | 111 --
 drivers/staging/vt6656/card.c |   4 +-
 drivers/staging/vt6656/rf.c   |  99 --
 3 files changed, 95 insertions(+), 119 deletions(-)

--
2.20.1

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


Re: [RFC] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-15 Thread Oscar Carter
On Tue, Apr 14, 2020 at 04:12:14PM +0300, Dan Carpenter wrote:
> On Mon, Apr 13, 2020 at 04:02:09PM +0200, Oscar Carter wrote:
> > diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
> > index 4f9aba0f21b0..3b200d7290a5 100644
> > --- a/drivers/staging/vt6656/rf.c
> > +++ b/drivers/staging/vt6656/rf.c
> > @@ -575,28 +575,14 @@ int vnt_rf_setpower(struct vnt_private *priv, u32 
> > rate, u32 channel)
> >
> >  static u8 vnt_rf_addpower(struct vnt_private *priv)
> >  {
> > +   s32 base;
>
> Just use "int".  s32 is for when signed 32 bit is specified in the
> hardware.  I realize that it's done in this file, but if all your
> friends jumped off a bridge doesn't mean you should drink their kool-aid.

Ok, lesson learned and thanks for the aclaration about when use every type.

> > s32 rssi = -priv->current_rssi;
> >
> > if (!rssi)
> > return 7;
> >
> > -   if (priv->rf_type == RF_VT3226D0) {
> > -   if (rssi < -70)
> > -   return 9;
> > -   else if (rssi < -65)
> > -   return 7;
> > -   else if (rssi < -60)
> > -   return 5;
> > -   } else {
> > -   if (rssi < -80)
> > -   return 9;
> > -   else if (rssi < -75)
> > -   return 7;
> > -   else if (rssi < -70)
> > -   return 5;
> > -   }
> > -
> > -   return 0;
> > +   base = (priv->rf_type == RF_VT3226D0) ? -60 : -70;
> > +   return (rssi < base--) ? ((rssi - base) / -5) * 2 + 5 : 0;
>^^
> I quite hate this postop.  It would have been cleaner to write it like:
>
>   return (rssi < base) ? ((rssi - (base - 1)) / -5) * 2 + 5 : 0
^^
Now, if we apply the minus operator one parentheses can be removed. The
same expression is now:

return (rssi < base) ? ((rssi - base + 1) / -5) * 2 + 5 : 0

I think it's clear enought.

> I'm sorry, I'm not clever enough to figure out the potential values of
> "rssi".

The IEEE 802.11 standard specifies that RSSI can be on a scale of 0 to
up to 255, and that each chipset manufacturer can define their own max
RSSI value.  It's all up to the manufacturer.

> How did you work out this formula?  It feels like it came from
> a standard or something?

I realized that the two branches of the if statement return the same
values (5, 7, 9) and that each value has a difference of 2 units from
the previous one. Also, every branch has 3 ranges, and every range has
an interval of 5. The only difference in this case is the "base" value
of each branch.

So, the solution was obtain the range index --> (rssi - base) / -5
Then, we need two units for every range index -> * 2
Now, the return value starts with five ---> + 5

The base-- was to obtain the range index the same that the orignal
function.

> Do we not have a function already which implements the standard?

I have been searching but I have not found anything that relates the
RSSI value with the amount of power to add. I have found

struct station_parameters -> member txpwr (struct sta_txpwr type)

but all the functions related to this doesn't set the tx power
depending on the RSSI value.

> regards,
> dan carpenter
>

thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 0/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-18 Thread Oscar Carter
This patch series checks the return value of vnt_control_out_* function
calls.

The first patch checks the return value and when necessary modify the
function prototype to be able to return the new checked error code.

The second patch replaces the documentation of functions that their
prototype has changed by the kernel-doc style, fixing the parameters and
return value.

Oscar Carter (2):
  staging: vt6656: Check the return value of vnt_control_out_* calls
  staging: vt6656: Fix functions' documentation

 drivers/staging/vt6656/baseband.c |  35 +++---
 drivers/staging/vt6656/baseband.h |   4 +-
 drivers/staging/vt6656/card.c | 198 +++---
 drivers/staging/vt6656/card.h |  18 +--
 drivers/staging/vt6656/mac.c  | 143 ++---
 drivers/staging/vt6656/mac.h  |  26 ++--
 drivers/staging/vt6656/power.c|  24 ++--
 drivers/staging/vt6656/power.h|   2 +-
 8 files changed, 217 insertions(+), 233 deletions(-)

--
2.20.1

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


[PATCH 2/2] staging: vt6656: Fix functions' documentation

2020-04-18 Thread Oscar Carter
Replace the functions' documentation by the kernel-doc style fixing the
parameters and return value.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c  | 101 +++--
 drivers/staging/vt6656/mac.c   |  67 --
 drivers/staging/vt6656/power.c |  12 ++--
 3 files changed, 61 insertions(+), 119 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 99ad56b7617d..51acf2212577 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -46,15 +46,12 @@ static const u16 cw_rxbcntsf_off[MAX_RATE] = {
192, 96, 34, 17, 34, 23, 17, 11, 8, 5, 4, 3
 };

-/*
- * Description: Set NIC media channel
+/**
+ * vnt_set_channel - Set NIC media channel.
+ * @priv: The adapter to be set.
+ * @connection_channel: The channel to be set.
  *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  connection_channel  - Channel to be set
- *  Out:
- *  none
+ * Return: 0 if successful, or a negative error code on failure.
  */
 int vnt_set_channel(struct vnt_private *priv, u32 connection_channel)
 {
@@ -99,19 +96,13 @@ static const u8 vnt_rspinf_gb_table[] = {
0x0e, 0x8d, 0x0a, 0x88, 0x0a, 0x8c, 0x0a, 0x8c, 0x0a
 };

-/*
- * Description: Set RSPINF
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  Out:
- *  none
- *
- * Return Value: None.
+/**
+ * vnt_set_rspinf - Set RSPINF.
+ * @priv: The adapter to be set.
+ * @bb_type: The base band type.
  *
+ * Return: 0 if successful, or a negative error code on failure.
  */
-
 int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
 {
const u8 *data;
@@ -145,17 +136,11 @@ int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
   MESSAGE_REQUEST_MACREG, len, data);
 }

-/*
- * Description: Update IFS
- *
- * Parameters:
- *  In:
- * priv - The adapter to be set
- * Out:
- * none
- *
- * Return Value: None.
+/**
+ * vnt_update_ifs - Update IFS.
+ * @priv: The adapter to be set.
  *
+ * Return: 0 if successful, or a negative error code on failure.
  */
 int vnt_update_ifs(struct vnt_private *priv)
 {
@@ -300,20 +285,14 @@ u64 vnt_get_tsf_offset(u8 rx_rate, u64 tsf1, u64 tsf2)
return tsf1 - tsf2 - (u64)cw_rxbcntsf_off[rx_rate % MAX_RATE];
 }

-/*
- * Description: Sync. TSF counter to BSS
- *  Get TSF offset and write to HW
- *
- * Parameters:
- *  In:
- *  priv   - The adapter to be sync.
- *  time_stamp - Rx BCN's TSF
- *  local_tsf  - Local TSF
- *  Out:
- *  none
- *
- * Return Value: none
+/**
+ * vnt_adjust_tsf - Sync TSF counter to BSS. Get TSF offset and write to HW.
+ * @priv: The adapter to be set.
+ * @rx_rate: The Rx rate.
+ * @time_stamp: The Rx BCN's TSF.
+ * @local_tsf: The local TSF.
  *
+ * Return: 0 if successful, or a negative error code on failure.
  */
 int vnt_adjust_tsf(struct vnt_private *priv, u8 rx_rate,
   u64 time_stamp, u64 local_tsf)
@@ -408,19 +387,13 @@ u64 vnt_get_next_tbtt(u64 tsf, u16 beacon_interval)
return tsf;
 }

-/*
- * Description: Set NIC TSF counter for first Beacon time
- *  Get NEXTTBTT from adjusted TSF and Beacon Interval
- *
- * Parameters:
- *  In:
- *  dwIoBase- IO Base
- * beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
+/**
+ * vnt_reset_next_tbtt - Set NIC TSF counter for first beacon time. Get
+ *  NEXTTBTT from adjusted TSF and beacon interval.
+ * @priv: The adapter to be set.
+ * @beacon_interval: The beacon interval.
  *
+ * Return: 0 if successful, or a negative error code on failure.
  */
 int vnt_reset_next_tbtt(struct vnt_private *priv, u16 beacon_interval)
 {
@@ -444,20 +417,14 @@ int vnt_reset_next_tbtt(struct vnt_private *priv, u16 
beacon_interval)
   MESSAGE_REQUEST_TBTT, 0, 8, data);
 }

-/*
- * Description: Sync NIC TSF counter for Beacon time
- *  Get NEXTTBTT and write to HW
- *
- * Parameters:
- *  In:
- * priv- The adapter to be set
- *  tsf- Current TSF counter
- *  beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
+/**
+ * vnt_update_next_tbtt - Sync NIC TSF counter for beacon time. Get NEXTTBTT
+ *   and write to HW.
+ * @priv: The adapter to be set.
+ * @tsf: The current TSF counter.
+ * @beacon_interval: The beacon interval.
  *
+ * Return: 0 if successful, or a negative error code on failure.
  */
 int vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
 u16 beacon_interval)
diff --git a/drivers/staging/vt6656/mac.c b/drivers/staging/vt6656/mac.c
index 639172fad0f3..c4b541178109 100644
--- a/drivers/staging/vt6656/mac.c
+++ b/drivers/staging/vt6656/mac.c
@@ -22,18 +22,12 @@
 #include "mac.h"
 #include "u

[PATCH 1/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-18 Thread Oscar Carter
Check the return value of vnt_control_out_* function calls. When
necessary modify the function prototype to be able to return the new
checked error code.

It's safe to modify all the function prototypes without fix the call
because the only change is the return value from void to int. If before
the call didn't check the return value, now neither.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 35 ++-
 drivers/staging/vt6656/baseband.h |  4 +-
 drivers/staging/vt6656/card.c | 97 ---
 drivers/staging/vt6656/card.h | 18 +++---
 drivers/staging/vt6656/mac.c  | 76 
 drivers/staging/vt6656/mac.h  | 26 -
 drivers/staging/vt6656/power.c| 12 +++-
 drivers/staging/vt6656/power.h|  2 +-
 8 files changed, 156 insertions(+), 114 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index e0352405e4cf..91cf00615ef3 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 
 #include 
 #include "mac.h"
 #include "baseband.h"
@@ -559,21 +560,22 @@ int vnt_set_short_slot_time(struct vnt_private *priv)

ret = vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
if (ret)
-   goto end;
+   return ret;

if (bb_vga == priv->bb_vga[0])
priv->bb_rx_conf |= 0x20;

-   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
-priv->bb_rx_conf);
-
-end:
-   return ret;
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
 {
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   int ret;
+
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   if (ret)
+   return ret;

/* patch for 3253B0 Baseband with Cardbus module */
if (priv->short_slot_time)
@@ -581,7 +583,8 @@ void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 
data)
else
priv->bb_rx_conf |= 0x20; /* 0010  */

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a, priv->bb_rx_conf);
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

 /*
@@ -622,12 +625,13 @@ int vnt_exit_deep_sleep(struct vnt_private *priv)
return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
 }

-void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
+int vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
 {
const struct vnt_threshold *threshold = NULL;
u8 length;
u8 cr_201, cr_206;
u8 ed_inx = priv->bb_pre_ed_index;
+   int ret;

switch (priv->rf_type) {
case RF_AL2230:
@@ -650,7 +654,7 @@ void vnt_update_pre_ed_threshold(struct vnt_private *priv, 
int scanning)
}

if (!threshold)
-   return;
+   return -EINVAL;

for (ed_inx = scanning ? 0 : length - 1; ed_inx > 0; ed_inx--) {
if (priv->bb_pre_ed_rssi <= threshold[ed_inx].bb_pre_ed_rssi)
@@ -661,14 +665,17 @@ void vnt_update_pre_ed_threshold(struct vnt_private 
*priv, int scanning)
cr_206 = threshold[ed_inx].cr_206;

if (ed_inx == priv->bb_pre_ed_index && !scanning)
-   return;
+   return 0;

priv->bb_pre_ed_index = ed_inx;

dev_dbg(&priv->usb->dev, "%s bb_pre_ed_rssi %d\n",
__func__, priv->bb_pre_ed_rssi);

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
+   if (ret)
+   return ret;
+
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
 }

diff --git a/drivers/staging/vt6656/baseband.h 
b/drivers/staging/vt6656/baseband.h
index dc42aa6ae1d9..8739988bf9e8 100644
--- a/drivers/staging/vt6656/baseband.h
+++ b/drivers/staging/vt6656/baseband.h
@@ -80,11 +80,11 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
   u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy);

 int vnt_set_short_slot_time(struct vnt_private *priv);
-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
 int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
 int vnt_vt3184_init(struct vnt_private *priv);
 int vnt_set_deep_sleep(struct vnt_private *priv);
 int vnt_exit_

[PATCH] staging: vt6656: Refactor the vnt_ofdm_min_rate function

2020-04-18 Thread Oscar Carter
Replace the for loop by a ternary operator whose condition is an AND
bitmask against the priv->basic_rates variable.

The purpose of the for loop was to check if any of bits from RATE_54M to
RATE_6M was set, but it's not necessary to check every individual bit.
The same result can be achieved using only one single mask which
comprises all the commented bits.

This way avoid the iteration over an unnecessary for loop.

Also change the return type to bool because it's the type that this
function returns.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c | 11 ++-
 drivers/staging/vt6656/card.h |  2 +-
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 9bd37e57c727..adaf93cf653a 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -248,16 +248,9 @@ void vnt_update_top_rates(struct vnt_private *priv)
priv->top_cck_basic_rate = top_cck;
 }

-int vnt_ofdm_min_rate(struct vnt_private *priv)
+bool vnt_ofdm_min_rate(struct vnt_private *priv)
 {
-   int ii;
-
-   for (ii = RATE_54M; ii >= RATE_6M; ii--) {
-   if ((priv->basic_rates) & ((u16)BIT(ii)))
-   return true;
-   }
-
-   return false;
+   return priv->basic_rates & GENMASK(RATE_54M, RATE_6M) ? true : false;
 }

 u8 vnt_get_pkt_type(struct vnt_private *priv)
diff --git a/drivers/staging/vt6656/card.h b/drivers/staging/vt6656/card.h
index 75cd340c0cce..eaa15d0c291a 100644
--- a/drivers/staging/vt6656/card.h
+++ b/drivers/staging/vt6656/card.h
@@ -29,7 +29,7 @@ void vnt_set_channel(struct vnt_private *priv, u32 
connection_channel);
 void vnt_set_rspinf(struct vnt_private *priv, u8 bb_type);
 void vnt_update_ifs(struct vnt_private *priv);
 void vnt_update_top_rates(struct vnt_private *priv);
-int vnt_ofdm_min_rate(struct vnt_private *priv);
+bool vnt_ofdm_min_rate(struct vnt_private *priv);
 void vnt_adjust_tsf(struct vnt_private *priv, u8 rx_rate,
u64 time_stamp, u64 local_tsf);
 bool vnt_get_current_tsf(struct vnt_private *priv, u64 *current_tsf);
--
2.20.1

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


Re: [PATCH 2/2] staging: vt6656: Fix functions' documentation

2020-04-19 Thread Oscar Carter
On Sat, Apr 18, 2020 at 07:05:53PM +0100, Malcolm Priestley wrote:
> Actually I don't really think the function descriptions are needed at all the
> names of the functions are enough.
>
Then, it would be better leave the documentation as it was before or remove it?

> card.c needs to be removed the bss callers to baseband.c, the tbtt's to 
> power.c
> and the rest to mac.c
>
> Regards
>
> Malcolm

Thanks,
Oscar Carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: vt6656: Use fls instead of for loop in vnt_update_top_rates

2020-04-19 Thread Oscar Carter
Replace the for loops of the vnt_update_top_rates function by the fls
function.

The purpose of the two for loops is to find the most significant bit set
in a range of bits. So, they can be replace by the fls function (find
last set) with a previous mask to define the range.

This way avoid the iteration over unnecessary for loops.

The header "linux/bits.h" can be remove as it is included in the header
"linux/bitops.h".

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 9bd37e57c727..952a7726fdd3 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -26,7 +26,7 @@
  *
  */

-#include 
+#include 
 #include "device.h"
 #include "card.h"
 #include "baseband.h"
@@ -223,29 +223,13 @@ void vnt_update_ifs(struct vnt_private *priv)

 void vnt_update_top_rates(struct vnt_private *priv)
 {
-   u8 top_ofdm = RATE_24M, top_cck = RATE_1M;
-   u8 i;
+   int pos;

-   /*Determines the highest basic rate.*/
-   for (i = RATE_54M; i >= RATE_6M; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
-   top_ofdm = i;
-   break;
-   }
-   }
-
-   priv->top_ofdm_basic_rate = top_ofdm;
-
-   for (i = RATE_11M;; i--) {
-   if (priv->basic_rates & (u16)(1 << i)) {
-   top_cck = i;
-   break;
-   }
-   if (i == RATE_1M)
-   break;
-   }
+   pos = fls(priv->basic_rates & GENMASK(RATE_54M, RATE_6M));
+   priv->top_ofdm_basic_rate = pos ? pos-- : RATE_24M;

-   priv->top_cck_basic_rate = top_cck;
+   pos = fls(priv->basic_rates & GENMASK(RATE_11M, RATE_1M));
+   priv->top_cck_basic_rate = pos ? pos-- : RATE_1M;
 }

 int vnt_ofdm_min_rate(struct vnt_private *priv)
--
2.20.1

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


Re: [PATCH 2/2] staging: vt6656: Fix functions' documentation

2020-04-19 Thread Oscar Carter
On Sun, Apr 19, 2020 at 10:22:50AM +0100, Malcolm Priestley wrote:
>
>
> On 19/04/2020 08:47, Oscar Carter wrote:
> > On Sat, Apr 18, 2020 at 07:05:53PM +0100, Malcolm Priestley wrote:
> >> Actually I don't really think the function descriptions are needed at all 
> >> the
> >> names of the functions are enough.
> >>
> > Then, it would be better leave the documentation as it was before or remove 
> > it?
> >
>
> I would remove them all except for comments inside functions.
>
Ok, then I make the suggested changes and send a new version serie.

> Regards
>
> Malcolm

Thanks,
Oscar Carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 2/2] staging: vt6656: Remove functions' documentation

2020-04-19 Thread Oscar Carter
Remove the functions' documentation as the names of the functions are
clear enought. Also, the actual documentation it's not correct in all
cases.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c  | 79 --
 drivers/staging/vt6656/mac.c   | 52 --
 drivers/staging/vt6656/power.c | 10 -
 3 files changed, 141 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 99ad56b7617d..e5e44d0a07ff 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -46,16 +46,6 @@ static const u16 cw_rxbcntsf_off[MAX_RATE] = {
192, 96, 34, 17, 34, 23, 17, 11, 8, 5, 4, 3
 };

-/*
- * Description: Set NIC media channel
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  connection_channel  - Channel to be set
- *  Out:
- *  none
- */
 int vnt_set_channel(struct vnt_private *priv, u32 connection_channel)
 {
int ret;
@@ -99,19 +89,6 @@ static const u8 vnt_rspinf_gb_table[] = {
0x0e, 0x8d, 0x0a, 0x88, 0x0a, 0x8c, 0x0a, 0x8c, 0x0a
 };

-/*
- * Description: Set RSPINF
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  Out:
- *  none
- *
- * Return Value: None.
- *
- */
-
 int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
 {
const u8 *data;
@@ -145,18 +122,6 @@ int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
   MESSAGE_REQUEST_MACREG, len, data);
 }

-/*
- * Description: Update IFS
- *
- * Parameters:
- *  In:
- * priv - The adapter to be set
- * Out:
- * none
- *
- * Return Value: None.
- *
- */
 int vnt_update_ifs(struct vnt_private *priv)
 {
u8 max_min = 0;
@@ -300,21 +265,6 @@ u64 vnt_get_tsf_offset(u8 rx_rate, u64 tsf1, u64 tsf2)
return tsf1 - tsf2 - (u64)cw_rxbcntsf_off[rx_rate % MAX_RATE];
 }

-/*
- * Description: Sync. TSF counter to BSS
- *  Get TSF offset and write to HW
- *
- * Parameters:
- *  In:
- *  priv   - The adapter to be sync.
- *  time_stamp - Rx BCN's TSF
- *  local_tsf  - Local TSF
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_adjust_tsf(struct vnt_private *priv, u8 rx_rate,
   u64 time_stamp, u64 local_tsf)
 {
@@ -408,20 +358,6 @@ u64 vnt_get_next_tbtt(u64 tsf, u16 beacon_interval)
return tsf;
 }

-/*
- * Description: Set NIC TSF counter for first Beacon time
- *  Get NEXTTBTT from adjusted TSF and Beacon Interval
- *
- * Parameters:
- *  In:
- *  dwIoBase- IO Base
- * beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_reset_next_tbtt(struct vnt_private *priv, u16 beacon_interval)
 {
u64 next_tbtt = 0;
@@ -444,21 +380,6 @@ int vnt_reset_next_tbtt(struct vnt_private *priv, u16 
beacon_interval)
   MESSAGE_REQUEST_TBTT, 0, 8, data);
 }

-/*
- * Description: Sync NIC TSF counter for Beacon time
- *  Get NEXTTBTT and write to HW
- *
- * Parameters:
- *  In:
- * priv- The adapter to be set
- *  tsf- Current TSF counter
- *  beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
 u16 beacon_interval)
 {
diff --git a/drivers/staging/vt6656/mac.c b/drivers/staging/vt6656/mac.c
index 639172fad0f3..da7067c34643 100644
--- a/drivers/staging/vt6656/mac.c
+++ b/drivers/staging/vt6656/mac.c
@@ -22,19 +22,6 @@
 #include "mac.h"
 #include "usbpipe.h"

-/*
- * Description:
- *  Write MAC Multicast Address Mask
- *
- * Parameters:
- *  In:
- * mc_filter (mac filter)
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_set_filter(struct vnt_private *priv, u64 mc_filter)
 {
__le64 le_mc = cpu_to_le64(mc_filter);
@@ -44,17 +31,6 @@ int vnt_mac_set_filter(struct vnt_private *priv, u64 
mc_filter)
   (u8 *)&le_mc);
 }

-/*
- * Description:
- *  Shut Down MAC
- *
- * Parameters:
- *  In:
- *  Out:
- *  none
- *
- *
- */
 int vnt_mac_shutdown(struct vnt_private *priv)
 {
return vnt_control_out(priv, MESSAGE_TYPE_MACSHUTDOWN, 0, 0, 0, NULL);
@@ -72,40 +48,12 @@ int vnt_mac_set_bb_type(struct vnt_private *priv, u8 type)
   data);
 }

-/*
- * Description:
- *  Disable the Key Entry by MISCFIFO
- *
- * Parameters:
- *  In:
- *  dwIoBase- Base Address for MAC
- *
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_disable_keyentry(struct vnt_private *priv, u8 entry_idx)
 {
return vnt_control_out(priv, MESSAGE_TYPE_CLRKEYENTRY, 0, 0,
   sizeof(entry_idx), &entry_idx);
 }

-/*
- * Description:
- *  Set the Key by MISCFIFO
- *
- * Parameter

[PATCH v2 0/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-19 Thread Oscar Carter
This patch series checks the return value of vnt_control_out_* function
calls.

The first patch checks the return value and when necessary modify the
function prototype to be able to return the new checked error code.

The second patch removes the documentation of functions that their
prototype has changed as the function names are clear enought. Also,
the actual documentation is not correct in all cases.

Changelog v1 -> v2
- Remove the function's documentation instead of fix them as suggested
  Malcolm Priestley.

Oscar Carter (2):
  staging: vt6656: Check the return value of vnt_control_out_* calls
  staging: vt6656: Remove functions' documentation

 drivers/staging/vt6656/baseband.c |  35 +++---
 drivers/staging/vt6656/baseband.h |   4 +-
 drivers/staging/vt6656/card.c | 176 +++---
 drivers/staging/vt6656/card.h |  18 +--
 drivers/staging/vt6656/mac.c  | 128 +++---
 drivers/staging/vt6656/mac.h  |  26 ++---
 drivers/staging/vt6656/power.c|  22 ++--
 drivers/staging/vt6656/power.h|   2 +-
 8 files changed, 156 insertions(+), 255 deletions(-)

--
2.20.1

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


[PATCH v2 1/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-19 Thread Oscar Carter
Check the return value of vnt_control_out_* function calls. When
necessary modify the function prototype to be able to return the new
checked error code.

It's safe to modify all the function prototypes without fix the call
because the only change is the return value from void to int. If before
the call didn't check the return value, now neither.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 35 ++-
 drivers/staging/vt6656/baseband.h |  4 +-
 drivers/staging/vt6656/card.c | 97 ---
 drivers/staging/vt6656/card.h | 18 +++---
 drivers/staging/vt6656/mac.c  | 76 
 drivers/staging/vt6656/mac.h  | 26 -
 drivers/staging/vt6656/power.c| 12 +++-
 drivers/staging/vt6656/power.h|  2 +-
 8 files changed, 156 insertions(+), 114 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index e0352405e4cf..91cf00615ef3 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 
 #include 
 #include "mac.h"
 #include "baseband.h"
@@ -559,21 +560,22 @@ int vnt_set_short_slot_time(struct vnt_private *priv)

ret = vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
if (ret)
-   goto end;
+   return ret;

if (bb_vga == priv->bb_vga[0])
priv->bb_rx_conf |= 0x20;

-   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
-priv->bb_rx_conf);
-
-end:
-   return ret;
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
 {
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   int ret;
+
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   if (ret)
+   return ret;

/* patch for 3253B0 Baseband with Cardbus module */
if (priv->short_slot_time)
@@ -581,7 +583,8 @@ void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 
data)
else
priv->bb_rx_conf |= 0x20; /* 0010  */

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a, priv->bb_rx_conf);
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

 /*
@@ -622,12 +625,13 @@ int vnt_exit_deep_sleep(struct vnt_private *priv)
return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
 }

-void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
+int vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
 {
const struct vnt_threshold *threshold = NULL;
u8 length;
u8 cr_201, cr_206;
u8 ed_inx = priv->bb_pre_ed_index;
+   int ret;

switch (priv->rf_type) {
case RF_AL2230:
@@ -650,7 +654,7 @@ void vnt_update_pre_ed_threshold(struct vnt_private *priv, 
int scanning)
}

if (!threshold)
-   return;
+   return -EINVAL;

for (ed_inx = scanning ? 0 : length - 1; ed_inx > 0; ed_inx--) {
if (priv->bb_pre_ed_rssi <= threshold[ed_inx].bb_pre_ed_rssi)
@@ -661,14 +665,17 @@ void vnt_update_pre_ed_threshold(struct vnt_private 
*priv, int scanning)
cr_206 = threshold[ed_inx].cr_206;

if (ed_inx == priv->bb_pre_ed_index && !scanning)
-   return;
+   return 0;

priv->bb_pre_ed_index = ed_inx;

dev_dbg(&priv->usb->dev, "%s bb_pre_ed_rssi %d\n",
__func__, priv->bb_pre_ed_rssi);

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
+   if (ret)
+   return ret;
+
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
 }

diff --git a/drivers/staging/vt6656/baseband.h 
b/drivers/staging/vt6656/baseband.h
index dc42aa6ae1d9..8739988bf9e8 100644
--- a/drivers/staging/vt6656/baseband.h
+++ b/drivers/staging/vt6656/baseband.h
@@ -80,11 +80,11 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
   u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy);

 int vnt_set_short_slot_time(struct vnt_private *priv);
-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
 int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
 int vnt_vt3184_init(struct vnt_private *priv);
 int vnt_set_deep_sleep(struct vnt_private *priv);
 int vnt_exit_

Re: [PATCH] staging: vt6656: Use fls instead of for loop in vnt_update_top_rates

2020-04-20 Thread Oscar Carter
On Mon, Apr 20, 2020 at 03:10:59PM +0300, Dan Carpenter wrote:
> On Sun, Apr 19, 2020 at 12:09:21PM +0200, Oscar Carter wrote:
> > -   for (i = RATE_11M;; i--) {
> > -   if (priv->basic_rates & (u16)(1 << i)) {
> > -   top_cck = i;
> > -   break;
> > -   }
> > -   if (i == RATE_1M)
> > -   break;
> > -   }
> > +   pos = fls(priv->basic_rates & GENMASK(RATE_54M, RATE_6M));
> > +   priv->top_ofdm_basic_rate = pos ? pos-- : RATE_24M;
>   ^
> Argh...  Come on.  I don't want to have to break out the C standard to
> see if this is defined behavior and where the sequence points are.  A
> pre-op would be clear but the most clear thing is to write it like this:
>
>   priv->top_ofdm_basic_rate = pos ? (pos - 1) : RATE_24M;
>
Ok, I do the modification as you suggested and resend a new version.
>
> >
> > -   priv->top_cck_basic_rate = top_cck;
> > +   pos = fls(priv->basic_rates & GENMASK(RATE_11M, RATE_1M));
> > +   priv->top_cck_basic_rate = pos ? pos-- : RATE_1M;
>  ^
> Same.
>
> regards,
> dan carpenter
>
thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] staging: vt6656: Use fls instead of for loop in vnt_update_top_rates

2020-04-20 Thread Oscar Carter
Replace the for loops of the vnt_update_top_rates function by the fls
function.

The purpose of the two for loops is to find the most significant bit set
in a range of bits. So, they can be replace by the fls function (find
last set) with a previous mask to define the range.

This way avoid the iteration over unnecessary for loops.

The header "linux/bits.h" can be remove as it is included in the header
"linux/bitops.h".

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Replace the expression pos-- with the expresion (pos - 1) as Dan
  Carpenter suggested.

 drivers/staging/vt6656/card.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index f8bfadd4b506..2478edee756a 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -26,7 +26,7 @@
  *
  */

-#include 
+#include 
 #include "device.h"
 #include "card.h"
 #include "baseband.h"
@@ -223,29 +223,13 @@ void vnt_update_ifs(struct vnt_private *priv)

 void vnt_update_top_rates(struct vnt_private *priv)
 {
-   u8 top_ofdm = RATE_24M, top_cck = RATE_1M;
-   u8 i;
+   int pos;

-   /*Determines the highest basic rate.*/
-   for (i = RATE_54M; i >= RATE_6M; i--) {
-   if (priv->basic_rates & BIT(i)) {
-   top_ofdm = i;
-   break;
-   }
-   }
-
-   priv->top_ofdm_basic_rate = top_ofdm;
-
-   for (i = RATE_11M;; i--) {
-   if (priv->basic_rates & BIT(i)) {
-   top_cck = i;
-   break;
-   }
-   if (i == RATE_1M)
-   break;
-   }
+   pos = fls(priv->basic_rates & GENMASK(RATE_54M, RATE_6M));
+   priv->top_ofdm_basic_rate = pos ? (pos - 1) : RATE_24M;

-   priv->top_cck_basic_rate = top_cck;
+   pos = fls(priv->basic_rates & GENMASK(RATE_11M, RATE_1M));
+   priv->top_cck_basic_rate = pos ? (pos - 1) : RATE_1M;
 }

 int vnt_ofdm_min_rate(struct vnt_private *priv)
--
2.20.1

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


Re: [RFC] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-20 Thread Oscar Carter
On Wed, Apr 15, 2020 at 06:25:41PM +0200, Oscar Carter wrote:
> On Tue, Apr 14, 2020 at 04:12:14PM +0300, Dan Carpenter wrote:
> > On Mon, Apr 13, 2020 at 04:02:09PM +0200, Oscar Carter wrote:
> > > diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
> > > index 4f9aba0f21b0..3b200d7290a5 100644
> > > --- a/drivers/staging/vt6656/rf.c
> > > +++ b/drivers/staging/vt6656/rf.c
> > > @@ -575,28 +575,14 @@ int vnt_rf_setpower(struct vnt_private *priv, u32 
> > > rate, u32 channel)
> > >
> > >  static u8 vnt_rf_addpower(struct vnt_private *priv)
> > >  {
> > > + s32 base;
> >
> > Just use "int".  s32 is for when signed 32 bit is specified in the
> > hardware.  I realize that it's done in this file, but if all your
> > friends jumped off a bridge doesn't mean you should drink their kool-aid.
>
> Ok, lesson learned and thanks for the aclaration about when use every type.
>
> > >   s32 rssi = -priv->current_rssi;
> > >
> > >   if (!rssi)
> > >   return 7;
> > >
> > > - if (priv->rf_type == RF_VT3226D0) {
> > > - if (rssi < -70)
> > > - return 9;
> > > - else if (rssi < -65)
> > > - return 7;
> > > - else if (rssi < -60)
> > > - return 5;
> > > - } else {
> > > - if (rssi < -80)
> > > - return 9;
> > > - else if (rssi < -75)
> > > - return 7;
> > > - else if (rssi < -70)
> > > - return 5;
> > > - }
> > > -
> > > - return 0;
> > > + base = (priv->rf_type == RF_VT3226D0) ? -60 : -70;
> > > + return (rssi < base--) ? ((rssi - base) / -5) * 2 + 5 : 0;
> >^^
> > I quite hate this postop.  It would have been cleaner to write it like:
> >
> > return (rssi < base) ? ((rssi - (base - 1)) / -5) * 2 + 5 : 0
> ^^
> Now, if we apply the minus operator one parentheses can be removed. The
> same expression is now:
>
>   return (rssi < base) ? ((rssi - base + 1) / -5) * 2 + 5 : 0
>
> I think it's clear enought.
>
> > I'm sorry, I'm not clever enough to figure out the potential values of
> > "rssi".
>
> The IEEE 802.11 standard specifies that RSSI can be on a scale of 0 to
> up to 255, and that each chipset manufacturer can define their own max
> RSSI value.  It's all up to the manufacturer.
>
> > How did you work out this formula?  It feels like it came from
> > a standard or something?
>
> I realized that the two branches of the if statement return the same
> values (5, 7, 9) and that each value has a difference of 2 units from
> the previous one. Also, every branch has 3 ranges, and every range has
> an interval of 5. The only difference in this case is the "base" value
> of each branch.
>
> So, the solution was obtain the range index --> (rssi - base) / -5
> Then, we need two units for every range index -> * 2
> Now, the return value starts with five ---> + 5
>
> The base-- was to obtain the range index the same that the orignal
> function.
>
> > Do we not have a function already which implements the standard?
>
> I have been searching but I have not found anything that relates the
> RSSI value with the amount of power to add. I have found
>
> struct station_parameters -> member txpwr (struct sta_txpwr type)
>
> but all the functions related to this doesn't set the tx power
> depending on the RSSI value.
>
I will create a new version with the previous comments (only change the
type of "base" variable to "int"), but what's the correct process for
an RFC patch. I need to send an email with the subject RFC v2 or now I
can send an email with the subject PATCH v2.

> > regards,
> > dan carpenter
> >
>
> thanks,
> oscar carter

thanks,
oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 1/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-23 Thread Oscar Carter
On Thu, Apr 23, 2020 at 01:40:32PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Apr 19, 2020 at 12:48:20PM +0200, Oscar Carter wrote:
> > Check the return value of vnt_control_out_* function calls. When
> > necessary modify the function prototype to be able to return the new
> > checked error code.
> >
> > It's safe to modify all the function prototypes without fix the call
> > because the only change is the return value from void to int. If before
> > the call didn't check the return value, now neither.
> >
> > Signed-off-by: Oscar Carter 
>
> This patch, and the 2/2 patch did not apply to my tree.  Can you please
> rebase and resend?
>
Yes. I will rebase and I will resend a new patch series.

> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 0/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-23 Thread Oscar Carter
This patch series checks the return value of vnt_control_out_* function
calls.

The first patch checks the return value and when necessary modify the
function prototype to be able to return the new checked error code.

The second patch removes the documentation of functions that their
prototype has changed as the function names are clear enought. Also,
the actual documentation is not correct in all cases.

Changelog v1 -> v2
- Remove the function's documentation instead of fix them as suggested
  Malcolm Priestley.

Changelog v2 -> v3
- Rebase against the staging-next branch of Greg's staging tree.

Oscar Carter (2):
  staging: vt6656: Check the return value of vnt_control_out_* calls
  staging: vt6656: Remove functions' documentation

 drivers/staging/vt6656/baseband.c |  35 +++---
 drivers/staging/vt6656/baseband.h |   4 +-
 drivers/staging/vt6656/card.c | 176 +++---
 drivers/staging/vt6656/card.h |  18 +--
 drivers/staging/vt6656/mac.c  | 128 +++---
 drivers/staging/vt6656/mac.h  |  26 ++---
 drivers/staging/vt6656/power.c|  22 ++--
 drivers/staging/vt6656/power.h|   2 +-
 8 files changed, 156 insertions(+), 255 deletions(-)

--
2.20.1

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


[PATCH v3 1/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-23 Thread Oscar Carter
Check the return value of vnt_control_out_* function calls. When
necessary modify the function prototype to be able to return the new
checked error code.

It's safe to modify all the function prototypes without fix the call
because the only change is the return value from void to int. If before
the call didn't check the return value, now neither.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 35 ++-
 drivers/staging/vt6656/baseband.h |  4 +-
 drivers/staging/vt6656/card.c | 97 ---
 drivers/staging/vt6656/card.h | 18 +++---
 drivers/staging/vt6656/mac.c  | 76 
 drivers/staging/vt6656/mac.h  | 26 -
 drivers/staging/vt6656/power.c| 12 +++-
 drivers/staging/vt6656/power.h|  2 +-
 8 files changed, 156 insertions(+), 114 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index d21a9cf0afe5..3d2d20e6f055 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 
 #include 
 #include "device.h"
 #include "mac.h"
@@ -506,21 +507,22 @@ int vnt_set_short_slot_time(struct vnt_private *priv)

ret = vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
if (ret)
-   goto end;
+   return ret;

if (bb_vga == priv->bb_vga[0])
priv->bb_rx_conf |= 0x20;

-   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
-priv->bb_rx_conf);
-
-end:
-   return ret;
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
 {
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   int ret;
+
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   if (ret)
+   return ret;

/* patch for 3253B0 Baseband with Cardbus module */
if (priv->short_slot_time)
@@ -528,7 +530,8 @@ void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 
data)
else
priv->bb_rx_conf |= 0x20; /* 0010  */

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a, priv->bb_rx_conf);
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

 /*
@@ -569,12 +572,13 @@ int vnt_exit_deep_sleep(struct vnt_private *priv)
return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
 }

-void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
+int vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
 {
const struct vnt_threshold *threshold = NULL;
u8 length;
u8 cr_201, cr_206;
u8 ed_inx;
+   int ret;

switch (priv->rf_type) {
case RF_AL2230:
@@ -597,7 +601,7 @@ void vnt_update_pre_ed_threshold(struct vnt_private *priv, 
int scanning)
}

if (!threshold)
-   return;
+   return -EINVAL;

for (ed_inx = scanning ? 0 : length - 1; ed_inx > 0; ed_inx--) {
if (priv->bb_pre_ed_rssi <= threshold[ed_inx].bb_pre_ed_rssi)
@@ -608,14 +612,17 @@ void vnt_update_pre_ed_threshold(struct vnt_private 
*priv, int scanning)
cr_206 = threshold[ed_inx].cr_206;

if (ed_inx == priv->bb_pre_ed_index && !scanning)
-   return;
+   return 0;

priv->bb_pre_ed_index = ed_inx;

dev_dbg(&priv->usb->dev, "%s bb_pre_ed_rssi %d\n",
__func__, priv->bb_pre_ed_rssi);

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
+   if (ret)
+   return ret;
+
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
 }

diff --git a/drivers/staging/vt6656/baseband.h 
b/drivers/staging/vt6656/baseband.h
index dc42aa6ae1d9..8739988bf9e8 100644
--- a/drivers/staging/vt6656/baseband.h
+++ b/drivers/staging/vt6656/baseband.h
@@ -80,11 +80,11 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 
frame_length,
   u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy);

 int vnt_set_short_slot_time(struct vnt_private *priv);
-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
 int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
 int vnt_vt3184_init(struct vnt_private *priv);
 int vnt_set_deep_sleep(struct vnt_private *priv);
 int vnt_exit_deep_sleep

[PATCH v3 2/2] staging: vt6656: Remove functions' documentation

2020-04-23 Thread Oscar Carter
Remove the functions' documentation as the names of the functions are
clear enought. Also, the actual documentation it's not correct in all
cases.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c  | 79 --
 drivers/staging/vt6656/mac.c   | 52 --
 drivers/staging/vt6656/power.c | 10 -
 3 files changed, 141 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index c29f7072c742..370ae4253a3f 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -46,16 +46,6 @@ static const u16 cw_rxbcntsf_off[MAX_RATE] = {
192, 96, 34, 17, 34, 23, 17, 11, 8, 5, 4, 3
 };

-/*
- * Description: Set NIC media channel
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  connection_channel  - Channel to be set
- *  Out:
- *  none
- */
 int vnt_set_channel(struct vnt_private *priv, u32 connection_channel)
 {
int ret;
@@ -99,19 +89,6 @@ static const u8 vnt_rspinf_gb_table[] = {
0x0e, 0x8d, 0x0a, 0x88, 0x0a, 0x8c, 0x0a, 0x8c, 0x0a
 };

-/*
- * Description: Set RSPINF
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  Out:
- *  none
- *
- * Return Value: None.
- *
- */
-
 int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
 {
const u8 *data;
@@ -145,18 +122,6 @@ int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
   MESSAGE_REQUEST_MACREG, len, data);
 }

-/*
- * Description: Update IFS
- *
- * Parameters:
- *  In:
- * priv - The adapter to be set
- * Out:
- * none
- *
- * Return Value: None.
- *
- */
 int vnt_update_ifs(struct vnt_private *priv)
 {
u8 max_min = 0;
@@ -300,21 +265,6 @@ u64 vnt_get_tsf_offset(u8 rx_rate, u64 tsf1, u64 tsf2)
return tsf1 - tsf2 - (u64)cw_rxbcntsf_off[rx_rate % MAX_RATE];
 }

-/*
- * Description: Sync. TSF counter to BSS
- *  Get TSF offset and write to HW
- *
- * Parameters:
- *  In:
- *  priv   - The adapter to be sync.
- *  time_stamp - Rx BCN's TSF
- *  local_tsf  - Local TSF
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_adjust_tsf(struct vnt_private *priv, u8 rx_rate,
   u64 time_stamp, u64 local_tsf)
 {
@@ -408,20 +358,6 @@ u64 vnt_get_next_tbtt(u64 tsf, u16 beacon_interval)
return tsf;
 }

-/*
- * Description: Set NIC TSF counter for first Beacon time
- *  Get NEXTTBTT from adjusted TSF and Beacon Interval
- *
- * Parameters:
- *  In:
- *  dwIoBase- IO Base
- * beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_reset_next_tbtt(struct vnt_private *priv, u16 beacon_interval)
 {
u64 next_tbtt = 0;
@@ -444,21 +380,6 @@ int vnt_reset_next_tbtt(struct vnt_private *priv, u16 
beacon_interval)
   MESSAGE_REQUEST_TBTT, 0, 8, data);
 }

-/*
- * Description: Sync NIC TSF counter for Beacon time
- *  Get NEXTTBTT and write to HW
- *
- * Parameters:
- *  In:
- * priv- The adapter to be set
- *  tsf- Current TSF counter
- *  beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
 u16 beacon_interval)
 {
diff --git a/drivers/staging/vt6656/mac.c b/drivers/staging/vt6656/mac.c
index 639172fad0f3..da7067c34643 100644
--- a/drivers/staging/vt6656/mac.c
+++ b/drivers/staging/vt6656/mac.c
@@ -22,19 +22,6 @@
 #include "mac.h"
 #include "usbpipe.h"

-/*
- * Description:
- *  Write MAC Multicast Address Mask
- *
- * Parameters:
- *  In:
- * mc_filter (mac filter)
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_set_filter(struct vnt_private *priv, u64 mc_filter)
 {
__le64 le_mc = cpu_to_le64(mc_filter);
@@ -44,17 +31,6 @@ int vnt_mac_set_filter(struct vnt_private *priv, u64 
mc_filter)
   (u8 *)&le_mc);
 }

-/*
- * Description:
- *  Shut Down MAC
- *
- * Parameters:
- *  In:
- *  Out:
- *  none
- *
- *
- */
 int vnt_mac_shutdown(struct vnt_private *priv)
 {
return vnt_control_out(priv, MESSAGE_TYPE_MACSHUTDOWN, 0, 0, 0, NULL);
@@ -72,40 +48,12 @@ int vnt_mac_set_bb_type(struct vnt_private *priv, u8 type)
   data);
 }

-/*
- * Description:
- *  Disable the Key Entry by MISCFIFO
- *
- * Parameters:
- *  In:
- *  dwIoBase- Base Address for MAC
- *
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_disable_keyentry(struct vnt_private *priv, u8 entry_idx)
 {
return vnt_control_out(priv, MESSAGE_TYPE_CLRKEYENTRY, 0, 0,
   sizeof(entry_idx), &entry_idx);
 }

-/*
- * Description:
- *  Set the Key by MISCFIFO
- *
- * Parameter

[PATCH v2] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-23 Thread Oscar Carter
Use a formula to calculate the return value of the vnt_rf_addpower
function instead of the "if" statement with literal values for every
case.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Change the type of "base" variable from s32 to int as Dan Carpenter
  suggested.
- Remove the "--" postoperator and replace with (base - 1) as Dan
  Carpenter suggested. Also, as this expression has a minus before the
  parenthesis, remove it an apply the minus operator changing the sign of
  "base" and literal "1".

 drivers/staging/vt6656/rf.c | 20 +++-
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 06fa8867cfa3..612fd4a59f8a 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -538,28 +538,14 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 
data)

 static u8 vnt_rf_addpower(struct vnt_private *priv)
 {
+   int base;
s32 rssi = -priv->current_rssi;

if (!rssi)
return 7;

-   if (priv->rf_type == RF_VT3226D0) {
-   if (rssi < -70)
-   return 9;
-   else if (rssi < -65)
-   return 7;
-   else if (rssi < -60)
-   return 5;
-   } else {
-   if (rssi < -80)
-   return 9;
-   else if (rssi < -75)
-   return 7;
-   else if (rssi < -70)
-   return 5;
-   }
-
-   return 0;
+   base = (priv->rf_type == RF_VT3226D0) ? -60 : -70;
+   return (rssi < base) ? ((rssi - base + 1) / -5) * 2 + 5 : 0;
 }

 /* Set Tx power by power level and rate */
--
2.20.1

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


[PATCH 0/3] Refactor the vnt_rf_table_download function

2020-04-25 Thread 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.

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

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


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

2020-04-25 Thread 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 
---
 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

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


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

2020-04-25 Thread 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 
---
 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

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


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

2020-04-25 Thread 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 same result can be achieved using the arrays directly.

Signed-off-by: Oscar Carter 
---
 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

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


Re: [PATCH v3 0/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-25 Thread Oscar Carter
On Sat, Apr 25, 2020 at 12:56:26PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 23, 2020 at 05:38:34PM +0200, Oscar Carter wrote:
> > This patch series checks the return value of vnt_control_out_* function
> > calls.
> >
> > The first patch checks the return value and when necessary modify the
> > function prototype to be able to return the new checked error code.
> >
> > The second patch removes the documentation of functions that their
> > prototype has changed as the function names are clear enought. Also,
> > the actual documentation is not correct in all cases.
> >
> > Changelog v1 -> v2
> > - Remove the function's documentation instead of fix them as suggested
> >   Malcolm Priestley.
> >
> > Changelog v2 -> v3
> > - Rebase against the staging-next branch of Greg's staging tree.
>
> Are you sure?  It still doesn't apply :(
>
> Please try again.
>
Ok, I will try again.

> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4 0/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-25 Thread Oscar Carter
This patch series checks the return value of vnt_control_out_* function
calls.

The first patch checks the return value and when necessary modify the
function prototype to be able to return the new checked error code.

The second patch removes the documentation of functions that their
prototype has changed as the function names are clear enought. Also,
the actual documentation is not correct in all cases.

Changelog v1 -> v2
- Remove the function's documentation instead of fix them as suggested
  Malcolm Priestley.

Changelog v2 -> v3
- Rebase against the staging-next branch of Greg's staging tree.

Changelog v3 -> v4
- Rebase again.

Oscar Carter (2):
  staging: vt6656: Check the return value of vnt_control_out_* calls
  staging: vt6656: Remove functions' documentation

 drivers/staging/vt6656/baseband.c |  35 +++---
 drivers/staging/vt6656/baseband.h |   4 +-
 drivers/staging/vt6656/card.c | 176 +++---
 drivers/staging/vt6656/card.h |  18 +--
 drivers/staging/vt6656/mac.c  | 128 +++---
 drivers/staging/vt6656/mac.h  |  26 ++---
 drivers/staging/vt6656/power.c|  22 ++--
 drivers/staging/vt6656/power.h|   2 +-
 8 files changed, 156 insertions(+), 255 deletions(-)

--
2.20.1

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


[PATCH v4 2/2] staging: vt6656: Remove functions' documentation

2020-04-25 Thread Oscar Carter
Remove the functions' documentation as the names of the functions are
clear enought. Also, the actual documentation it's not correct in all
cases.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/card.c  | 79 --
 drivers/staging/vt6656/mac.c   | 52 --
 drivers/staging/vt6656/power.c | 10 -
 3 files changed, 141 deletions(-)

diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 341796da2561..872717f9df49 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -46,16 +46,6 @@ static const u16 cw_rxbcntsf_off[MAX_RATE] = {
192, 96, 34, 17, 34, 23, 17, 11, 8, 5, 4, 3
 };

-/*
- * Description: Set NIC media channel
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  connection_channel  - Channel to be set
- *  Out:
- *  none
- */
 int vnt_set_channel(struct vnt_private *priv, u32 connection_channel)
 {
int ret;
@@ -99,19 +89,6 @@ static const u8 vnt_rspinf_gb_table[] = {
0x0e, 0x8d, 0x0a, 0x88, 0x0a, 0x8c, 0x0a, 0x8c, 0x0a
 };

-/*
- * Description: Set RSPINF
- *
- * Parameters:
- *  In:
- *  pDevice - The adapter to be set
- *  Out:
- *  none
- *
- * Return Value: None.
- *
- */
-
 int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
 {
const u8 *data;
@@ -145,18 +122,6 @@ int vnt_set_rspinf(struct vnt_private *priv, u8 bb_type)
   MESSAGE_REQUEST_MACREG, len, data);
 }

-/*
- * Description: Update IFS
- *
- * Parameters:
- *  In:
- * priv - The adapter to be set
- * Out:
- * none
- *
- * Return Value: None.
- *
- */
 int vnt_update_ifs(struct vnt_private *priv)
 {
u8 max_min = 0;
@@ -277,21 +242,6 @@ u64 vnt_get_tsf_offset(u8 rx_rate, u64 tsf1, u64 tsf2)
return tsf1 - tsf2 - (u64)cw_rxbcntsf_off[rx_rate % MAX_RATE];
 }

-/*
- * Description: Sync. TSF counter to BSS
- *  Get TSF offset and write to HW
- *
- * Parameters:
- *  In:
- *  priv   - The adapter to be sync.
- *  time_stamp - Rx BCN's TSF
- *  local_tsf  - Local TSF
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_adjust_tsf(struct vnt_private *priv, u8 rx_rate,
   u64 time_stamp, u64 local_tsf)
 {
@@ -385,20 +335,6 @@ u64 vnt_get_next_tbtt(u64 tsf, u16 beacon_interval)
return tsf;
 }

-/*
- * Description: Set NIC TSF counter for first Beacon time
- *  Get NEXTTBTT from adjusted TSF and Beacon Interval
- *
- * Parameters:
- *  In:
- *  dwIoBase- IO Base
- * beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_reset_next_tbtt(struct vnt_private *priv, u16 beacon_interval)
 {
u64 next_tbtt = 0;
@@ -421,21 +357,6 @@ int vnt_reset_next_tbtt(struct vnt_private *priv, u16 
beacon_interval)
   MESSAGE_REQUEST_TBTT, 0, 8, data);
 }

-/*
- * Description: Sync NIC TSF counter for Beacon time
- *  Get NEXTTBTT and write to HW
- *
- * Parameters:
- *  In:
- * priv- The adapter to be set
- *  tsf- Current TSF counter
- *  beacon_interval - Beacon Interval
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
 u16 beacon_interval)
 {
diff --git a/drivers/staging/vt6656/mac.c b/drivers/staging/vt6656/mac.c
index 639172fad0f3..da7067c34643 100644
--- a/drivers/staging/vt6656/mac.c
+++ b/drivers/staging/vt6656/mac.c
@@ -22,19 +22,6 @@
 #include "mac.h"
 #include "usbpipe.h"

-/*
- * Description:
- *  Write MAC Multicast Address Mask
- *
- * Parameters:
- *  In:
- * mc_filter (mac filter)
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_set_filter(struct vnt_private *priv, u64 mc_filter)
 {
__le64 le_mc = cpu_to_le64(mc_filter);
@@ -44,17 +31,6 @@ int vnt_mac_set_filter(struct vnt_private *priv, u64 
mc_filter)
   (u8 *)&le_mc);
 }

-/*
- * Description:
- *  Shut Down MAC
- *
- * Parameters:
- *  In:
- *  Out:
- *  none
- *
- *
- */
 int vnt_mac_shutdown(struct vnt_private *priv)
 {
return vnt_control_out(priv, MESSAGE_TYPE_MACSHUTDOWN, 0, 0, 0, NULL);
@@ -72,40 +48,12 @@ int vnt_mac_set_bb_type(struct vnt_private *priv, u8 type)
   data);
 }

-/*
- * Description:
- *  Disable the Key Entry by MISCFIFO
- *
- * Parameters:
- *  In:
- *  dwIoBase- Base Address for MAC
- *
- *  Out:
- *  none
- *
- * Return Value: none
- *
- */
 int vnt_mac_disable_keyentry(struct vnt_private *priv, u8 entry_idx)
 {
return vnt_control_out(priv, MESSAGE_TYPE_CLRKEYENTRY, 0, 0,
   sizeof(entry_idx), &entry_idx);
 }

-/*
- * Description:
- *  Set the Key by MISCFIFO
- *
- * Parameter

[PATCH v4 1/2] staging: vt6656: Check the return value of vnt_control_out_* calls

2020-04-25 Thread Oscar Carter
Check the return value of vnt_control_out_* function calls. When
necessary modify the function prototype to be able to return the new
checked error code.

It's safe to modify all the function prototypes without fix the call
because the only change is the return value from void to int. If before
the call didn't check the return value, now neither.

Signed-off-by: Oscar Carter 
---
 drivers/staging/vt6656/baseband.c | 35 ++-
 drivers/staging/vt6656/baseband.h |  4 +-
 drivers/staging/vt6656/card.c | 97 ---
 drivers/staging/vt6656/card.h | 18 +++---
 drivers/staging/vt6656/mac.c  | 76 
 drivers/staging/vt6656/mac.h  | 26 -
 drivers/staging/vt6656/power.c| 12 +++-
 drivers/staging/vt6656/power.h|  2 +-
 8 files changed, 156 insertions(+), 114 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c 
b/drivers/staging/vt6656/baseband.c
index e7000bba644a..1d75acaec8f3 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -23,6 +23,7 @@
  */

 #include 
+#include 
 #include 
 #include "device.h"
 #include "mac.h"
@@ -367,21 +368,22 @@ int vnt_set_short_slot_time(struct vnt_private *priv)

ret = vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
if (ret)
-   goto end;
+   return ret;

if (bb_vga == priv->bb_vga[0])
priv->bb_rx_conf |= 0x20;

-   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
-priv->bb_rx_conf);
-
-end:
-   return ret;
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
 {
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   int ret;
+
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xE7, data);
+   if (ret)
+   return ret;

/* patch for 3253B0 Baseband with Cardbus module */
if (priv->short_slot_time)
@@ -389,7 +391,8 @@ void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 
data)
else
priv->bb_rx_conf |= 0x20; /* 0010  */

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a, priv->bb_rx_conf);
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+ priv->bb_rx_conf);
 }

 /*
@@ -430,12 +433,13 @@ int vnt_exit_deep_sleep(struct vnt_private *priv)
return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
 }

-void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
+int vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
 {
const struct vnt_threshold *threshold = NULL;
u8 length;
u8 cr_201, cr_206;
u8 ed_inx;
+   int ret;

switch (priv->rf_type) {
case RF_AL2230:
@@ -458,7 +462,7 @@ void vnt_update_pre_ed_threshold(struct vnt_private *priv, 
int scanning)
}

if (!threshold)
-   return;
+   return -EINVAL;

for (ed_inx = scanning ? 0 : length - 1; ed_inx > 0; ed_inx--) {
if (priv->bb_pre_ed_rssi <= threshold[ed_inx].bb_pre_ed_rssi)
@@ -469,14 +473,17 @@ void vnt_update_pre_ed_threshold(struct vnt_private 
*priv, int scanning)
cr_206 = threshold[ed_inx].cr_206;

if (ed_inx == priv->bb_pre_ed_index && !scanning)
-   return;
+   return 0;

priv->bb_pre_ed_index = ed_inx;

dev_dbg(&priv->usb->dev, "%s bb_pre_ed_rssi %d\n",
__func__, priv->bb_pre_ed_rssi);

-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
-   vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
+   ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xc9, cr_201);
+   if (ret)
+   return ret;
+
+   return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0xce, cr_206);
 }

diff --git a/drivers/staging/vt6656/baseband.h 
b/drivers/staging/vt6656/baseband.h
index ee7325d942fe..12456ebc23ec 100644
--- a/drivers/staging/vt6656/baseband.h
+++ b/drivers/staging/vt6656/baseband.h
@@ -67,11 +67,11 @@
 #define TOP_RATE_1M 0x0010

 int vnt_set_short_slot_time(struct vnt_private *priv);
-void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
+int vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
 int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
 int vnt_vt3184_init(struct vnt_private *priv);
 int vnt_set_deep_sleep(struct vnt_private *priv);
 int vnt_exit_deep_sleep(struct vnt_private *priv);
-void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning);
+int vnt_update_p

Re: [PATCH v2] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-25 Thread Oscar Carter
On Sat, Apr 25, 2020 at 12:57:14PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 23, 2020 at 07:05:57PM +0200, Oscar Carter wrote:
> > Use a formula to calculate the return value of the vnt_rf_addpower
> > function instead of the "if" statement with literal values for every
> > case.
> >
> > Signed-off-by: Oscar Carter 
> > ---
> > Changelog v1 -> v2
> > - Change the type of "base" variable from s32 to int as Dan Carpenter
> >   suggested.
> > - Remove the "--" postoperator and replace with (base - 1) as Dan
> >   Carpenter suggested. Also, as this expression has a minus before the
> >   parenthesis, remove it an apply the minus operator changing the sign of
> >   "base" and literal "1".
> >
> >  drivers/staging/vt6656/rf.c | 20 +++-
> >  1 file changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
> > index 06fa8867cfa3..612fd4a59f8a 100644
> > --- a/drivers/staging/vt6656/rf.c
> > +++ b/drivers/staging/vt6656/rf.c
> > @@ -538,28 +538,14 @@ int vnt_rf_write_embedded(struct vnt_private *priv, 
> > u32 data)
> >
> >  static u8 vnt_rf_addpower(struct vnt_private *priv)
> >  {
> > +   int base;
> > s32 rssi = -priv->current_rssi;
> >
> > if (!rssi)
> > return 7;
> >
> > -   if (priv->rf_type == RF_VT3226D0) {
> > -   if (rssi < -70)
> > -   return 9;
> > -   else if (rssi < -65)
> > -   return 7;
> > -   else if (rssi < -60)
> > -   return 5;
> > -   } else {
> > -   if (rssi < -80)
> > -   return 9;
> > -   else if (rssi < -75)
> > -   return 7;
> > -   else if (rssi < -70)
> > -   return 5;
> > -   }
> > -
> > -   return 0;
> > +   base = (priv->rf_type == RF_VT3226D0) ? -60 : -70;
> > +   return (rssi < base) ? ((rssi - base + 1) / -5) * 2 + 5 : 0;
>
> I _hate_ ? : functions, just spell this out please as a real if()
> statement.
>
Ok, I will do the modification and I will resend a new version patch.

> thanks,
>
> greg k-h

thanks,

oscar carter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3] staging: vt6656: Add formula to the vnt_rf_addpower function

2020-04-25 Thread Oscar Carter
Use a formula to calculate the return value of the vnt_rf_addpower
function instead of the "if" statement with literal values for every
case.

Signed-off-by: Oscar Carter 
---
Changelog v1 -> v2
- Change the type of "base" variable from s32 to int as Dan Carpenter
  suggested.
- Remove the "--" postoperator and replace with (base - 1) as Dan
  Carpenter suggested. Also, as this expression has a minus before the
  parenthesis, remove it an apply the minus operator changing the sign of
  "base" and literal "1".

Changelog v2 -> v3
- Replace ternary operators with the equivalents "if" statements as Greg
  Kroah-Hartman suggested.

 drivers/staging/vt6656/rf.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 06fa8867cfa3..05c9d06f84dd 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -538,26 +538,19 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 
data)

 static u8 vnt_rf_addpower(struct vnt_private *priv)
 {
+   int base;
s32 rssi = -priv->current_rssi;

if (!rssi)
return 7;

-   if (priv->rf_type == RF_VT3226D0) {
-   if (rssi < -70)
-   return 9;
-   else if (rssi < -65)
-   return 7;
-   else if (rssi < -60)
-   return 5;
-   } else {
-   if (rssi < -80)
-   return 9;
-   else if (rssi < -75)
-   return 7;
-   else if (rssi < -70)
-   return 5;
-   }
+   if (priv->rf_type == RF_VT3226D0)
+   base = -60;
+   else
+   base = -70;
+
+   if (rssi < base)
+   return ((rssi - base + 1) / -5) * 2 + 5;

return 0;
 }
--
2.20.1

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


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

2020-04-25 Thread Oscar Carter
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 
> 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 
> > ---
> >  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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2020-04-25 Thread 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 
---
 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

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


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

2020-04-25 Thread 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 
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 
---
 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

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


[PATCH v2 0/3] Refactor the vnt_rf_table_download function

2020-04-25 Thread 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

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


  1   2   >