[PATCH] staging: vt6656: Declare a few variables as __read_mostly
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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"
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
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
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
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
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
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
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"
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
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"
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
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