2018-11-01 16:45:06

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 0/8] staging: wilc1000: make use of cfg80211 provided API's

From: Ajay Singh <[email protected]>

This series mainly contains the changes to make use of cfg80211 and
framework provided API's instead of having own implementation.
After refactoring of coreconfigurator functions, most of the redundant code
is removed, hence deleted the extra file.

Ajay Singh (8):
staging: wilc1000: refactor wilc_parse_network_info() using kernel
framework api's
staging: wilc1000: remove unused code in coreconfigurator
staging: wilc1000: refactor get_bssid() function
staging: wilc1000: avoid line over 80 chars in
wilc_parse_network_info()
staging: wilc1000: refactor wilc_parse_assoc_resp_info()
staging: wilc1000: remove unnecessary MAX_STRING_LEN macro
staging: wilc1000: remove coreconfigurator.c file
staging: wilc1000: remove coreconfigurator.h file

drivers/staging/wilc1000/Makefile | 3 +-
drivers/staging/wilc1000/coreconfigurator.c | 287 ----------------------------
drivers/staging/wilc1000/coreconfigurator.h | 81 --------
drivers/staging/wilc1000/host_interface.c | 117 ++++++++++++
drivers/staging/wilc1000/host_interface.h | 61 +++++-
drivers/staging/wilc1000/wilc_wlan_cfg.c | 1 -
6 files changed, 177 insertions(+), 373 deletions(-)
delete mode 100644 drivers/staging/wilc1000/coreconfigurator.c
delete mode 100644 drivers/staging/wilc1000/coreconfigurator.h

--
2.7.4



2018-11-01 16:45:09

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 1/8] staging: wilc1000: refactor wilc_parse_network_info() using kernel framework api's

From: Ajay Singh <[email protected]>

Refactor wilc_parse_network_info() by making use of cfg80211.h provided
API.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 90 ++++++++++++++++++-----------
1 file changed, 55 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index d6d3a97..4dfa658 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -4,7 +4,7 @@
* All rights reserved.
*/

-#include <linux/ieee80211.h>
+#include <net/cfg80211.h>

#include "coreconfigurator.h"

@@ -116,11 +116,11 @@ static inline void get_address3(u8 *msa, u8 *addr)
memcpy(addr, msa + 16, 6);
}

-static inline void get_bssid(u8 *data, u8 *bssid)
+static inline void get_bssid(__le16 fc, u8 *data, u8 *bssid)
{
- if (get_from_ds(data) == 1)
+ if (ieee80211_has_fromds(fc))
get_address2(data, bssid);
- else if (get_to_ds(data) == 1)
+ else if (ieee80211_has_tods(fc))
get_address1(data, bssid);
else
get_address3(data, bssid);
@@ -202,17 +202,18 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
struct network_info **ret_network_info)
{
struct network_info *network_info;
- u8 *wid_val, *msa, *tim_elm, *ies;
- u32 tsf_lo, tsf_hi;
+ struct ieee80211_mgmt *mgt;
+ u8 *wid_val, *msa, *ies;
u16 wid_len, rx_len, ies_len;
- u8 msg_type, index;
+ u8 msg_type;
+ size_t offset;
+ const u8 *ch_elm, *tim_elm, *ssid_elm;

msg_type = msg_buffer[0];
-
if ('N' != msg_type)
return -EFAULT;

- wid_len = MAKE_WORD16(msg_buffer[6], msg_buffer[7]);
+ wid_len = get_unaligned_le16(&msg_buffer[6]);
wid_val = &msg_buffer[8];

network_info = kzalloc(sizeof(*network_info), GFP_KERNEL);
@@ -222,42 +223,61 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
network_info->rssi = wid_val[0];

msa = &wid_val[1];
-
+ mgt = (struct ieee80211_mgmt *)&wid_val[1];
rx_len = wid_len - 1;
- network_info->cap_info = get_cap_info(msa);
- network_info->tsf_lo = get_beacon_timestamp_lo(msa);

- tsf_lo = get_beacon_timestamp_lo(msa);
- tsf_hi = get_beacon_timestamp_hi(msa);
+ if (ieee80211_is_probe_resp(mgt->frame_control)) {
+ network_info->cap_info = le16_to_cpu(mgt->u.probe_resp.capab_info);
+ network_info->beacon_period = le16_to_cpu(mgt->u.probe_resp.beacon_int);
+ network_info->tsf_hi = le64_to_cpu(mgt->u.probe_resp.timestamp);
+ network_info->tsf_lo = (u32)network_info->tsf_hi;
+ offset = offsetof(struct ieee80211_mgmt, u.probe_resp.variable);
+ } else if (ieee80211_is_beacon(mgt->frame_control)) {
+ network_info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
+ network_info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
+ network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
+ network_info->tsf_lo = (u32)network_info->tsf_hi;
+ offset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
+ } else {
+ /* only process probe response and beacon frame */
+ kfree(network_info);
+ return -EIO;
+ }

- network_info->tsf_hi = tsf_lo | ((u64)tsf_hi << 32);
+ get_bssid(mgt->frame_control, msa, network_info->bssid);

- get_ssid(msa, network_info->ssid, &network_info->ssid_len);
- get_bssid(msa, network_info->bssid);
+ ies = mgt->u.beacon.variable;
+ ies_len = rx_len - offset;
+ if (ies_len <= 0) {
+ kfree(network_info);
+ return -EIO;
+ }

- network_info->ch = get_current_channel_802_11n(msa, rx_len
- + FCS_LEN);
+ network_info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
+ if (!network_info->ies) {
+ kfree(network_info);
+ return -ENOMEM;
+ }

- index = MAC_HDR_LEN + TIME_STAMP_LEN;
+ network_info->ies_len = ies_len;

- network_info->beacon_period = get_beacon_period(msa + index);
+ ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies, ies_len);
+ if (ssid_elm) {
+ network_info->ssid_len = ssid_elm[1];
+ if (network_info->ssid_len <= IEEE80211_MAX_SSID_LEN)
+ memcpy(network_info->ssid, ssid_elm + 2,
+ network_info->ssid_len);
+ else
+ network_info->ssid_len = 0;
+ }

- index += BEACON_INTERVAL_LEN + CAP_INFO_LEN;
+ ch_elm = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ies, ies_len);
+ if (ch_elm && ch_elm[1] > 0)
+ network_info->ch = ch_elm[2];

- tim_elm = get_tim_elm(msa, rx_len + FCS_LEN, index);
- if (tim_elm)
+ tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies, ies_len);
+ if (tim_elm && tim_elm[1] >= 2)
network_info->dtim_period = tim_elm[3];
- ies = &msa[TAG_PARAM_OFFSET];
- ies_len = rx_len - TAG_PARAM_OFFSET;
-
- if (ies_len > 0) {
- network_info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
- if (!network_info->ies) {
- kfree(network_info);
- return -ENOMEM;
- }
- }
- network_info->ies_len = ies_len;

*ret_network_info = network_info;

--
2.7.4


2018-11-01 16:45:16

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 3/8] staging: wilc1000: refactor get_bssid() function

From: Ajay Singh <[email protected]>

Refactor get_bssid() by making use of 'ieee80211_mgmt' struct. Instead
of passing the memory offset now using structure element to fetch the
bssid information.
Returning the pointer to bssid from get_bssid() instead of filing the
input argument.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index 166443d..ac44846 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -8,29 +8,14 @@

#include "coreconfigurator.h"

-static inline void get_address1(u8 *msa, u8 *addr)
+static inline u8 *get_bssid(struct ieee80211_mgmt *mgmt)
{
- memcpy(addr, msa + 4, 6);
-}
-
-static inline void get_address2(u8 *msa, u8 *addr)
-{
- memcpy(addr, msa + 10, 6);
-}
-
-static inline void get_address3(u8 *msa, u8 *addr)
-{
- memcpy(addr, msa + 16, 6);
-}
-
-static inline void get_bssid(__le16 fc, u8 *data, u8 *bssid)
-{
- if (ieee80211_has_fromds(fc))
- get_address2(data, bssid);
- else if (ieee80211_has_tods(fc))
- get_address1(data, bssid);
+ if (ieee80211_has_fromds(mgmt->frame_control))
+ return mgmt->sa;
+ else if (ieee80211_has_tods(mgmt->frame_control))
+ return mgmt->da;
else
- get_address3(data, bssid);
+ return mgmt->bssid;
}

static inline u16 get_asoc_status(u8 *data)
@@ -87,7 +72,7 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
return -EIO;
}

- get_bssid(mgt->frame_control, msa, network_info->bssid);
+ ether_addr_copy(network_info->bssid, get_bssid(mgt));

ies = mgt->u.beacon.variable;
ies_len = rx_len - offset;
--
2.7.4


2018-11-01 16:45:17

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 2/8] staging: wilc1000: remove unused code in coreconfigurator

From: Ajay Singh <[email protected]>

After refactoring of wilc_parse_network_info(), some of the functions and
macro are not required, so removed the unused code.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 157 ----------------------------
drivers/staging/wilc1000/coreconfigurator.h | 8 --
2 files changed, 165 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index 4dfa658..166443d 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -8,99 +8,6 @@

#include "coreconfigurator.h"

-#define TAG_PARAM_OFFSET (MAC_HDR_LEN + TIME_STAMP_LEN + \
- BEACON_INTERVAL_LEN + CAP_INFO_LEN)
-
-enum sub_frame_type {
- ASSOC_REQ = 0x00,
- ASSOC_RSP = 0x10,
- REASSOC_REQ = 0x20,
- REASSOC_RSP = 0x30,
- PROBE_REQ = 0x40,
- PROBE_RSP = 0x50,
- BEACON = 0x80,
- ATIM = 0x90,
- DISASOC = 0xA0,
- AUTH = 0xB0,
- DEAUTH = 0xC0,
- ACTION = 0xD0,
- PS_POLL = 0xA4,
- RTS = 0xB4,
- CTS = 0xC4,
- ACK = 0xD4,
- CFEND = 0xE4,
- CFEND_ACK = 0xF4,
- DATA = 0x08,
- DATA_ACK = 0x18,
- DATA_POLL = 0x28,
- DATA_POLL_ACK = 0x38,
- NULL_FRAME = 0x48,
- CFACK = 0x58,
- CFPOLL = 0x68,
- CFPOLL_ACK = 0x78,
- QOS_DATA = 0x88,
- QOS_DATA_ACK = 0x98,
- QOS_DATA_POLL = 0xA8,
- QOS_DATA_POLL_ACK = 0xB8,
- QOS_NULL_FRAME = 0xC8,
- QOS_CFPOLL = 0xE8,
- QOS_CFPOLL_ACK = 0xF8,
- BLOCKACK_REQ = 0x84,
- BLOCKACK = 0x94,
- FRAME_SUBTYPE_FORCE_32BIT = 0xFFFFFFFF
-};
-
-static inline u16 get_beacon_period(u8 *data)
-{
- u16 bcn_per;
-
- bcn_per = data[0];
- bcn_per |= (data[1] << 8);
-
- return bcn_per;
-}
-
-static inline u32 get_beacon_timestamp_lo(u8 *data)
-{
- u32 time_stamp = 0;
- u32 index = MAC_HDR_LEN;
-
- time_stamp |= data[index++];
- time_stamp |= (data[index++] << 8);
- time_stamp |= (data[index++] << 16);
- time_stamp |= (data[index] << 24);
-
- return time_stamp;
-}
-
-static inline u32 get_beacon_timestamp_hi(u8 *data)
-{
- u32 time_stamp = 0;
- u32 index = (MAC_HDR_LEN + 4);
-
- time_stamp |= data[index++];
- time_stamp |= (data[index++] << 8);
- time_stamp |= (data[index++] << 16);
- time_stamp |= (data[index] << 24);
-
- return time_stamp;
-}
-
-static inline enum sub_frame_type get_sub_type(u8 *header)
-{
- return ((enum sub_frame_type)(header[0] & 0xFC));
-}
-
-static inline u8 get_to_ds(u8 *header)
-{
- return (header[1] & 0x01);
-}
-
-static inline u8 get_from_ds(u8 *header)
-{
- return ((header[1] & 0x02) >> 1);
-}
-
static inline void get_address1(u8 *msa, u8 *addr)
{
memcpy(addr, msa + 4, 6);
@@ -126,41 +33,6 @@ static inline void get_bssid(__le16 fc, u8 *data, u8 *bssid)
get_address3(data, bssid);
}

-static inline void get_ssid(u8 *data, u8 *ssid, u8 *p_ssid_len)
-{
- u8 i, j, len;
-
- len = data[TAG_PARAM_OFFSET + 1];
- j = TAG_PARAM_OFFSET + 2;
-
- if (len >= MAX_SSID_LEN)
- len = 0;
-
- for (i = 0; i < len; i++, j++)
- ssid[i] = data[j];
-
- ssid[len] = '\0';
-
- *p_ssid_len = len;
-}
-
-static inline u16 get_cap_info(u8 *data)
-{
- u16 cap_info = 0;
- u16 index = MAC_HDR_LEN;
- enum sub_frame_type st;
-
- st = get_sub_type(data);
-
- if (st == BEACON || st == PROBE_RSP)
- index += TIME_STAMP_LEN + BEACON_INTERVAL_LEN;
-
- cap_info = data[index];
- cap_info |= (data[index + 1] << 8);
-
- return cap_info;
-}
-
static inline u16 get_asoc_status(u8 *data)
{
u16 asoc_status;
@@ -169,35 +41,6 @@ static inline u16 get_asoc_status(u8 *data)
return (asoc_status << 8) | data[2];
}

-static u8 *get_tim_elm(u8 *msa, u16 rx_len, u16 tag_param_offset)
-{
- u16 index;
-
- index = tag_param_offset;
-
- while (index < (rx_len - FCS_LEN)) {
- if (msa[index] == WLAN_EID_TIM)
- return &msa[index];
- index += (IE_HDR_LEN + msa[index + 1]);
- }
-
- return NULL;
-}
-
-static u8 get_current_channel_802_11n(u8 *msa, u16 rx_len)
-{
- u16 index;
-
- index = TAG_PARAM_OFFSET;
- while (index < (rx_len - FCS_LEN)) {
- if (msa[index] == WLAN_EID_DS_PARAMS)
- return msa[index + 2];
- index += msa[index + 1] + IE_HDR_LEN;
- }
-
- return 0;
-}
-
s32 wilc_parse_network_info(u8 *msg_buffer,
struct network_info **ret_network_info)
{
diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index b62acb4..0d40c77 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -11,14 +11,9 @@

#define NUM_RSSI 5

-#define MAC_HDR_LEN 24
-#define FCS_LEN 4
-#define TIME_STAMP_LEN 8
-#define BEACON_INTERVAL_LEN 2
#define CAP_INFO_LEN 2
#define STATUS_CODE_LEN 2
#define AID_LEN 2
-#define IE_HDR_LEN 2

#define SET_CFG 0
#define GET_CFG 1
@@ -26,9 +21,6 @@
#define MAX_STRING_LEN 256
#define MAX_ASSOC_RESP_FRAME_SIZE MAX_STRING_LEN

-#define MAKE_WORD16(lsb, msb) ((((u16)(msb) << 8) & 0xFF00) | (lsb))
-#define MAKE_WORD32(lsw, msw) ((((u32)(msw) << 16) & 0xFFFF0000) | (lsw))
-
struct rssi_history_buffer {
bool full;
u8 index;
--
2.7.4


2018-11-01 16:45:18

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

From: Ajay Singh <[email protected]>

Use shorter name for 'network_info' variable to avoid line over 80 chars
issue.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 53 ++++++++++++++---------------
1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index ac44846..2c77e5a 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -29,7 +29,7 @@ static inline u16 get_asoc_status(u8 *data)
s32 wilc_parse_network_info(u8 *msg_buffer,
struct network_info **ret_network_info)
{
- struct network_info *network_info;
+ struct network_info *info;
struct ieee80211_mgmt *mgt;
u8 *wid_val, *msa, *ies;
u16 wid_len, rx_len, ies_len;
@@ -44,70 +44,69 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
wid_len = get_unaligned_le16(&msg_buffer[6]);
wid_val = &msg_buffer[8];

- network_info = kzalloc(sizeof(*network_info), GFP_KERNEL);
- if (!network_info)
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
return -ENOMEM;

- network_info->rssi = wid_val[0];
+ info->rssi = wid_val[0];

msa = &wid_val[1];
mgt = (struct ieee80211_mgmt *)&wid_val[1];
rx_len = wid_len - 1;

if (ieee80211_is_probe_resp(mgt->frame_control)) {
- network_info->cap_info = le16_to_cpu(mgt->u.probe_resp.capab_info);
- network_info->beacon_period = le16_to_cpu(mgt->u.probe_resp.beacon_int);
- network_info->tsf_hi = le64_to_cpu(mgt->u.probe_resp.timestamp);
- network_info->tsf_lo = (u32)network_info->tsf_hi;
+ info->cap_info = le16_to_cpu(mgt->u.probe_resp.capab_info);
+ info->beacon_period = le16_to_cpu(mgt->u.probe_resp.beacon_int);
+ info->tsf_hi = le64_to_cpu(mgt->u.probe_resp.timestamp);
+ info->tsf_lo = (u32)info->tsf_hi;
offset = offsetof(struct ieee80211_mgmt, u.probe_resp.variable);
} else if (ieee80211_is_beacon(mgt->frame_control)) {
- network_info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
- network_info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
- network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
- network_info->tsf_lo = (u32)network_info->tsf_hi;
+ info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
+ info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
+ info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
+ info->tsf_lo = (u32)info->tsf_hi;
offset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
} else {
/* only process probe response and beacon frame */
- kfree(network_info);
+ kfree(info);
return -EIO;
}

- ether_addr_copy(network_info->bssid, get_bssid(mgt));
+ ether_addr_copy(info->bssid, get_bssid(mgt));

ies = mgt->u.beacon.variable;
ies_len = rx_len - offset;
if (ies_len <= 0) {
- kfree(network_info);
+ kfree(info);
return -EIO;
}

- network_info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
- if (!network_info->ies) {
- kfree(network_info);
+ info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
+ if (!info->ies) {
+ kfree(info);
return -ENOMEM;
}

- network_info->ies_len = ies_len;
+ info->ies_len = ies_len;

ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies, ies_len);
if (ssid_elm) {
- network_info->ssid_len = ssid_elm[1];
- if (network_info->ssid_len <= IEEE80211_MAX_SSID_LEN)
- memcpy(network_info->ssid, ssid_elm + 2,
- network_info->ssid_len);
+ info->ssid_len = ssid_elm[1];
+ if (info->ssid_len <= IEEE80211_MAX_SSID_LEN)
+ memcpy(info->ssid, ssid_elm + 2, info->ssid_len);
else
- network_info->ssid_len = 0;
+ info->ssid_len = 0;
}

ch_elm = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ies, ies_len);
if (ch_elm && ch_elm[1] > 0)
- network_info->ch = ch_elm[2];
+ info->ch = ch_elm[2];

tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies, ies_len);
if (tim_elm && tim_elm[1] >= 2)
- network_info->dtim_period = tim_elm[3];
+ info->dtim_period = tim_elm[3];

- *ret_network_info = network_info;
+ *ret_network_info = info;

return 0;
}
--
2.7.4


2018-11-01 16:45:21

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 5/8] staging: wilc1000: refactor wilc_parse_assoc_resp_info()

From: Ajay Singh <[email protected]>

Refactor wilc_parse_assoc_resp_info() function by removing the use of
get_asoc_status() API. For parsing assoc response use the struct and
avoided the use of offset macros to extract the ies information.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 16 ++++------------
drivers/staging/wilc1000/coreconfigurator.h | 10 ++++++----
2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index 2c77e5a..2bd62fd 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -18,14 +18,6 @@ static inline u8 *get_bssid(struct ieee80211_mgmt *mgmt)
return mgmt->bssid;
}

-static inline u16 get_asoc_status(u8 *data)
-{
- u16 asoc_status;
-
- asoc_status = data[3];
- return (asoc_status << 8) | data[2];
-}
-
s32 wilc_parse_network_info(u8 *msg_buffer,
struct network_info **ret_network_info)
{
@@ -116,12 +108,12 @@ s32 wilc_parse_assoc_resp_info(u8 *buffer, u32 buffer_len,
{
u8 *ies;
u16 ies_len;
+ struct assoc_resp *res = (struct assoc_resp *)buffer;

- ret_conn_info->status = get_asoc_status(buffer);
+ ret_conn_info->status = le16_to_cpu(res->status_code);
if (ret_conn_info->status == WLAN_STATUS_SUCCESS) {
- ies = &buffer[CAP_INFO_LEN + STATUS_CODE_LEN + AID_LEN];
- ies_len = buffer_len - (CAP_INFO_LEN + STATUS_CODE_LEN +
- AID_LEN);
+ ies = &buffer[sizeof(*res)];
+ ies_len = buffer_len - sizeof(*res);

ret_conn_info->resp_ies = kmemdup(ies, ies_len, GFP_KERNEL);
if (!ret_conn_info->resp_ies)
diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index 0d40c77..71a9f27 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -11,10 +11,6 @@

#define NUM_RSSI 5

-#define CAP_INFO_LEN 2
-#define STATUS_CODE_LEN 2
-#define AID_LEN 2
-
#define SET_CFG 0
#define GET_CFG 1

@@ -63,6 +59,12 @@ struct disconnect_info {
size_t ie_len;
};

+struct assoc_resp {
+ __le16 capab_info;
+ __le16 status_code;
+ __le16 aid;
+} __packed;
+
s32 wilc_parse_network_info(u8 *msg_buffer,
struct network_info **ret_network_info);
s32 wilc_parse_assoc_resp_info(u8 *buffer, u32 buffer_len,
--
2.7.4


2018-11-01 16:45:23

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 6/8] staging: wilc1000: remove unnecessary MAX_STRING_LEN macro

From: Ajay Singh <[email protected]>

Cleanup patch to remove the use of unnecessary 'MAX_STRING_LEN' macro.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index 71a9f27..a1347f7 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -14,8 +14,7 @@
#define SET_CFG 0
#define GET_CFG 1

-#define MAX_STRING_LEN 256
-#define MAX_ASSOC_RESP_FRAME_SIZE MAX_STRING_LEN
+#define MAX_ASSOC_RESP_FRAME_SIZE 256

struct rssi_history_buffer {
bool full;
--
2.7.4


2018-11-01 16:45:28

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 7/8] staging: wilc1000: remove coreconfigurator.c file

From: Ajay Singh <[email protected]>

After use of framework API's most of the redundant functions are removed
in coreconfigurator.c file. Now moved left over function to
host_interface file and deleted the coreconfigurator.c file.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/Makefile | 3 +-
drivers/staging/wilc1000/coreconfigurator.c | 126 ----------------------------
drivers/staging/wilc1000/coreconfigurator.h | 4 -
drivers/staging/wilc1000/host_interface.c | 117 ++++++++++++++++++++++++++
4 files changed, 118 insertions(+), 132 deletions(-)
delete mode 100644 drivers/staging/wilc1000/coreconfigurator.c

diff --git a/drivers/staging/wilc1000/Makefile b/drivers/staging/wilc1000/Makefile
index 37e8560..72a4daa 100644
--- a/drivers/staging/wilc1000/Makefile
+++ b/drivers/staging/wilc1000/Makefile
@@ -5,8 +5,7 @@ ccflags-y += -DFIRMWARE_1002=\"atmel/wilc1002_firmware.bin\" \
-DFIRMWARE_1003=\"atmel/wilc1003_firmware.bin\"

wilc1000-objs := wilc_wfi_cfgoperations.o linux_wlan.o linux_mon.o \
- coreconfigurator.o host_interface.o \
- wilc_wlan_cfg.o wilc_wlan.o
+ host_interface.o wilc_wlan_cfg.o wilc_wlan.o

obj-$(CONFIG_WILC1000_SDIO) += wilc1000-sdio.o
wilc1000-sdio-objs += wilc_sdio.o
diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
deleted file mode 100644
index 2bd62fd..0000000
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ /dev/null
@@ -1,126 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2012 - 2018 Microchip Technology Inc., and its subsidiaries.
- * All rights reserved.
- */
-
-#include <net/cfg80211.h>
-
-#include "coreconfigurator.h"
-
-static inline u8 *get_bssid(struct ieee80211_mgmt *mgmt)
-{
- if (ieee80211_has_fromds(mgmt->frame_control))
- return mgmt->sa;
- else if (ieee80211_has_tods(mgmt->frame_control))
- return mgmt->da;
- else
- return mgmt->bssid;
-}
-
-s32 wilc_parse_network_info(u8 *msg_buffer,
- struct network_info **ret_network_info)
-{
- struct network_info *info;
- struct ieee80211_mgmt *mgt;
- u8 *wid_val, *msa, *ies;
- u16 wid_len, rx_len, ies_len;
- u8 msg_type;
- size_t offset;
- const u8 *ch_elm, *tim_elm, *ssid_elm;
-
- msg_type = msg_buffer[0];
- if ('N' != msg_type)
- return -EFAULT;
-
- wid_len = get_unaligned_le16(&msg_buffer[6]);
- wid_val = &msg_buffer[8];
-
- info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info->rssi = wid_val[0];
-
- msa = &wid_val[1];
- mgt = (struct ieee80211_mgmt *)&wid_val[1];
- rx_len = wid_len - 1;
-
- if (ieee80211_is_probe_resp(mgt->frame_control)) {
- info->cap_info = le16_to_cpu(mgt->u.probe_resp.capab_info);
- info->beacon_period = le16_to_cpu(mgt->u.probe_resp.beacon_int);
- info->tsf_hi = le64_to_cpu(mgt->u.probe_resp.timestamp);
- info->tsf_lo = (u32)info->tsf_hi;
- offset = offsetof(struct ieee80211_mgmt, u.probe_resp.variable);
- } else if (ieee80211_is_beacon(mgt->frame_control)) {
- info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
- info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
- info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
- info->tsf_lo = (u32)info->tsf_hi;
- offset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
- } else {
- /* only process probe response and beacon frame */
- kfree(info);
- return -EIO;
- }
-
- ether_addr_copy(info->bssid, get_bssid(mgt));
-
- ies = mgt->u.beacon.variable;
- ies_len = rx_len - offset;
- if (ies_len <= 0) {
- kfree(info);
- return -EIO;
- }
-
- info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
- if (!info->ies) {
- kfree(info);
- return -ENOMEM;
- }
-
- info->ies_len = ies_len;
-
- ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies, ies_len);
- if (ssid_elm) {
- info->ssid_len = ssid_elm[1];
- if (info->ssid_len <= IEEE80211_MAX_SSID_LEN)
- memcpy(info->ssid, ssid_elm + 2, info->ssid_len);
- else
- info->ssid_len = 0;
- }
-
- ch_elm = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ies, ies_len);
- if (ch_elm && ch_elm[1] > 0)
- info->ch = ch_elm[2];
-
- tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies, ies_len);
- if (tim_elm && tim_elm[1] >= 2)
- info->dtim_period = tim_elm[3];
-
- *ret_network_info = info;
-
- return 0;
-}
-
-s32 wilc_parse_assoc_resp_info(u8 *buffer, u32 buffer_len,
- struct connect_info *ret_conn_info)
-{
- u8 *ies;
- u16 ies_len;
- struct assoc_resp *res = (struct assoc_resp *)buffer;
-
- ret_conn_info->status = le16_to_cpu(res->status_code);
- if (ret_conn_info->status == WLAN_STATUS_SUCCESS) {
- ies = &buffer[sizeof(*res)];
- ies_len = buffer_len - sizeof(*res);
-
- ret_conn_info->resp_ies = kmemdup(ies, ies_len, GFP_KERNEL);
- if (!ret_conn_info->resp_ies)
- return -ENOMEM;
-
- ret_conn_info->resp_ies_len = ies_len;
- }
-
- return 0;
-}
diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index a1347f7..67f6855 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -64,10 +64,6 @@ struct assoc_resp {
__le16 aid;
} __packed;

-s32 wilc_parse_network_info(u8 *msg_buffer,
- struct network_info **ret_network_info);
-s32 wilc_parse_assoc_resp_info(u8 *buffer, u32 buffer_len,
- struct connect_info *ret_conn_info);
void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length);
void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length);
void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length);
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 01db899..c4f858b 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1305,6 +1305,101 @@ static void *host_int_parse_join_bss_param(struct network_info *info)
return (void *)param;
}

+static inline u8 *get_bssid(struct ieee80211_mgmt *mgmt)
+{
+ if (ieee80211_has_fromds(mgmt->frame_control))
+ return mgmt->sa;
+ else if (ieee80211_has_tods(mgmt->frame_control))
+ return mgmt->da;
+ else
+ return mgmt->bssid;
+}
+
+static s32 wilc_parse_network_info(u8 *msg_buffer,
+ struct network_info **ret_network_info)
+{
+ struct network_info *info;
+ struct ieee80211_mgmt *mgt;
+ u8 *wid_val, *msa, *ies;
+ u16 wid_len, rx_len, ies_len;
+ u8 msg_type;
+ size_t offset;
+ const u8 *ch_elm, *tim_elm, *ssid_elm;
+
+ msg_type = msg_buffer[0];
+ if ('N' != msg_type)
+ return -EFAULT;
+
+ wid_len = get_unaligned_le16(&msg_buffer[6]);
+ wid_val = &msg_buffer[8];
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->rssi = wid_val[0];
+
+ msa = &wid_val[1];
+ mgt = (struct ieee80211_mgmt *)&wid_val[1];
+ rx_len = wid_len - 1;
+
+ if (ieee80211_is_probe_resp(mgt->frame_control)) {
+ info->cap_info = le16_to_cpu(mgt->u.probe_resp.capab_info);
+ info->beacon_period = le16_to_cpu(mgt->u.probe_resp.beacon_int);
+ info->tsf_hi = le64_to_cpu(mgt->u.probe_resp.timestamp);
+ info->tsf_lo = (u32)info->tsf_hi;
+ offset = offsetof(struct ieee80211_mgmt, u.probe_resp.variable);
+ } else if (ieee80211_is_beacon(mgt->frame_control)) {
+ info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
+ info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
+ info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
+ info->tsf_lo = (u32)info->tsf_hi;
+ offset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
+ } else {
+ /* only process probe response and beacon frame */
+ kfree(info);
+ return -EIO;
+ }
+
+ ether_addr_copy(info->bssid, get_bssid(mgt));
+
+ ies = mgt->u.beacon.variable;
+ ies_len = rx_len - offset;
+ if (ies_len <= 0) {
+ kfree(info);
+ return -EIO;
+ }
+
+ info->ies = kmemdup(ies, ies_len, GFP_KERNEL);
+ if (!info->ies) {
+ kfree(info);
+ return -ENOMEM;
+ }
+
+ info->ies_len = ies_len;
+
+ ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies, ies_len);
+ if (ssid_elm) {
+ info->ssid_len = ssid_elm[1];
+ if (info->ssid_len <= IEEE80211_MAX_SSID_LEN)
+ memcpy(info->ssid, ssid_elm + 2, info->ssid_len);
+ else
+ info->ssid_len = 0;
+ }
+
+ ch_elm = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ies, ies_len);
+ if (ch_elm && ch_elm[1] > 0)
+ info->ch = ch_elm[2];
+
+ tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies, ies_len);
+ if (tim_elm && tim_elm[1] >= 2)
+ info->dtim_period = tim_elm[3];
+
+ *ret_network_info = info;
+
+ return 0;
+}
+
static void handle_rcvd_ntwrk_info(struct work_struct *work)
{
struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
@@ -1410,6 +1505,28 @@ static inline void host_int_free_user_conn_req(struct host_if_drv *hif_drv)
hif_drv->usr_conn_req.ies = NULL;
}

+static s32 wilc_parse_assoc_resp_info(u8 *buffer, u32 buffer_len,
+ struct connect_info *ret_conn_info)
+{
+ u8 *ies;
+ u16 ies_len;
+ struct assoc_resp *res = (struct assoc_resp *)buffer;
+
+ ret_conn_info->status = le16_to_cpu(res->status_code);
+ if (ret_conn_info->status == WLAN_STATUS_SUCCESS) {
+ ies = &buffer[sizeof(*res)];
+ ies_len = buffer_len - sizeof(*res);
+
+ ret_conn_info->resp_ies = kmemdup(ies, ies_len, GFP_KERNEL);
+ if (!ret_conn_info->resp_ies)
+ return -ENOMEM;
+
+ ret_conn_info->resp_ies_len = ies_len;
+ }
+
+ return 0;
+}
+
static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
u8 mac_status)
{
--
2.7.4


2018-11-01 16:45:39

by Ajay Singh

[permalink] [raw]
Subject: [PATCH 8/8] staging: wilc1000: remove coreconfigurator.h file

From: Ajay Singh <[email protected]>

Remove the coreconfigurator header file, as its source file is deleted
after code refactor. Moved the required structure and prototypes to
hostinterface header.

Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.h | 70 -----------------------------
drivers/staging/wilc1000/host_interface.h | 61 ++++++++++++++++++++++++-
drivers/staging/wilc1000/wilc_wlan_cfg.c | 1 -
3 files changed, 59 insertions(+), 73 deletions(-)
delete mode 100644 drivers/staging/wilc1000/coreconfigurator.h

diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
deleted file mode 100644
index 67f6855..0000000
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ /dev/null
@@ -1,70 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (c) 2012 - 2018 Microchip Technology Inc., and its subsidiaries.
- * All rights reserved.
- */
-
-#ifndef CORECONFIGURATOR_H
-#define CORECONFIGURATOR_H
-
-#include "wilc_wlan_if.h"
-
-#define NUM_RSSI 5
-
-#define SET_CFG 0
-#define GET_CFG 1
-
-#define MAX_ASSOC_RESP_FRAME_SIZE 256
-
-struct rssi_history_buffer {
- bool full;
- u8 index;
- s8 samples[NUM_RSSI];
-};
-
-struct network_info {
- s8 rssi;
- u16 cap_info;
- u8 ssid[MAX_SSID_LEN];
- u8 ssid_len;
- u8 bssid[6];
- u16 beacon_period;
- u8 dtim_period;
- u8 ch;
- unsigned long time_scan_cached;
- unsigned long time_scan;
- bool new_network;
- u8 found;
- u32 tsf_lo;
- u8 *ies;
- u16 ies_len;
- void *join_params;
- struct rssi_history_buffer rssi_history;
- u64 tsf_hi;
-};
-
-struct connect_info {
- u8 bssid[6];
- u8 *req_ies;
- size_t req_ies_len;
- u8 *resp_ies;
- u16 resp_ies_len;
- u16 status;
-};
-
-struct disconnect_info {
- u16 reason;
- u8 *ie;
- size_t ie_len;
-};
-
-struct assoc_resp {
- __le16 capab_info;
- __le16 status_code;
- __le16 aid;
-} __packed;
-
-void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length);
-void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length);
-void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length);
-#endif
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 33fb731..5f8d30f 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -7,7 +7,7 @@
#ifndef HOST_INT_H
#define HOST_INT_H
#include <linux/ieee80211.h>
-#include "coreconfigurator.h"
+#include "wilc_wlan_if.h"

#define IDLE_MODE 0x00
#define AP_MODE 0x01
@@ -56,6 +56,61 @@
#define DRV_HANDLER_SIZE 5
#define DRV_HANDLER_MASK 0x000000FF

+#define NUM_RSSI 5
+
+#define SET_CFG 0
+#define GET_CFG 1
+
+#define MAX_ASSOC_RESP_FRAME_SIZE 256
+
+struct rssi_history_buffer {
+ bool full;
+ u8 index;
+ s8 samples[NUM_RSSI];
+};
+
+struct network_info {
+ s8 rssi;
+ u16 cap_info;
+ u8 ssid[MAX_SSID_LEN];
+ u8 ssid_len;
+ u8 bssid[6];
+ u16 beacon_period;
+ u8 dtim_period;
+ u8 ch;
+ unsigned long time_scan_cached;
+ unsigned long time_scan;
+ bool new_network;
+ u8 found;
+ u32 tsf_lo;
+ u8 *ies;
+ u16 ies_len;
+ void *join_params;
+ struct rssi_history_buffer rssi_history;
+ u64 tsf_hi;
+};
+
+struct connect_info {
+ u8 bssid[6];
+ u8 *req_ies;
+ size_t req_ies_len;
+ u8 *resp_ies;
+ u16 resp_ies_len;
+ u16 status;
+};
+
+struct disconnect_info {
+ u16 reason;
+ u8 *ie;
+ size_t ie_len;
+};
+
+struct assoc_resp {
+ __le16 capab_info;
+ __le16 status_code;
+ __le16 aid;
+} __packed;
+
struct rf_info {
u8 link_speed;
s8 rssi;
@@ -358,5 +413,7 @@ void wilc_resolve_disconnect_aberration(struct wilc_vif *vif);
int wilc_get_vif_idx(struct wilc_vif *vif);
int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power);
int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
-
+void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length);
+void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length);
+void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length);
#endif
diff --git a/drivers/staging/wilc1000/wilc_wlan_cfg.c b/drivers/staging/wilc1000/wilc_wlan_cfg.c
index faa001c..8390766 100644
--- a/drivers/staging/wilc1000/wilc_wlan_cfg.c
+++ b/drivers/staging/wilc1000/wilc_wlan_cfg.c
@@ -7,7 +7,6 @@
#include "wilc_wlan_if.h"
#include "wilc_wlan.h"
#include "wilc_wlan_cfg.h"
-#include "coreconfigurator.h"
#include "wilc_wfi_netdevice.h"

enum cfg_cmd_type {
--
2.7.4


2018-11-04 19:18:06

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
> From: Ajay Singh <[email protected]>
>
> Use shorter name for 'network_info' variable to avoid line over 80 chars
> issue.

This seems completely unnecessary as patch 7/8 and 8/8
removes the file.



2018-11-05 04:42:57

by Ajay Singh

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

Hi Joe,

On 11/5/2018 12:47 AM, Joe Perches wrote:
> On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
>> From: Ajay Singh <[email protected]>
>>
>> Use shorter name for 'network_info' variable to avoid line over 80 chars
>> issue.
> This seems completely unnecessary as patch 7/8 and 8/8
> removes the file.
>
As wilc_parse_network_info() is moved to a different file in patch#7.
So to separate the variable name changes from the function movement I
have divided them into 2 different patches.

Regards,
Ajay

2018-11-05 10:57:59

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
> From: Ajay Singh <[email protected]>
>
> Use shorter name for 'network_info' variable to avoid line over 80 chars
> issue.


I suppose this is OK, though perhaps unnecessary.

As well, perhaps there are defects in the original code
in a couple places.

> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
[]
> - network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
> - network_info->tsf_lo = (u32)network_info->tsf_hi;

Perhaps there is a defect for both tsf_hi assignments
as it appears as if both are missing ">> 32"

> + info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
> + info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
> + info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
> + info->tsf_lo = (u32)info->tsf_hi;

Perhaps this should be

network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp) >> 32;

or

network_info->tsf_hi = upper_32_bits(le64_to_cpu(...));
network_info->tsf_lo = lower_32_bits(le64_to_cpu(...));



2018-11-05 12:18:08

by Ajay Singh

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

Hi Joe,

On 11/5/2018 4:27 PM, Joe Perches wrote:
> On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
>> From: Ajay Singh <[email protected]>
>>
>> Use shorter name for 'network_info' variable to avoid line over 80 chars
>> issue.
>
> I suppose this is OK, though perhaps unnecessary.
>
> As well, perhaps there are defects in the original code
> in a couple places.
>
>> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> []
>> - network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
>> - network_info->tsf_lo = (u32)network_info->tsf_hi;
> Perhaps there is a defect for both tsf_hi assignments
> as it appears as if both are missing ">> 32"

Actually, 'tsf_hi' is used to store the complete tsf value as its data
type is u64.  This value is pass to the cfg80211_inform_bss() as it is.
So the conversion of 'tsf_hi' to 32 bit  is not required in this case.

>> + info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
>> + info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
>> + info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
>> + info->tsf_lo = (u32)info->tsf_hi;
> Perhaps this should be
>
> network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp) >> 32;
>
> or
>
> network_info->tsf_hi = upper_32_bits(le64_to_cpu(...));
> network_info->tsf_lo = lower_32_bits(le64_to_cpu(...));
>
>

2018-11-05 15:57:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()

On Mon, 2018-11-05 at 12:18 +0000, [email protected] wrote:
> Hi Joe,
>
> On 11/5/2018 4:27 PM, Joe Perches wrote:
> > On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
> > > From: Ajay Singh <[email protected]>
> > >
> > > Use shorter name for 'network_info' variable to avoid line over 80 chars
> > > issue.
> >
> > I suppose this is OK, though perhaps unnecessary.
> >
> > As well, perhaps there are defects in the original code
> > in a couple places.
> >
> > > diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> > []
> > > - network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
> > > - network_info->tsf_lo = (u32)network_info->tsf_hi;
> > Perhaps there is a defect for both tsf_hi assignments
> > as it appears as if both are missing ">> 32"
>
> Actually, 'tsf_hi' is used to store the complete tsf value as its data
> type is u64. This value is pass to the cfg80211_inform_bss() as it is.
> So the conversion of 'tsf_hi' to 32 bit is not required in this case.

Antipattern naming generally warrants a rename.



2018-11-06 06:01:31

by Ajay Singh

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in wilc_parse_network_info()


On 11/5/2018 9:27 PM, Joe Perches wrote:
> On Mon, 2018-11-05 at 12:18 +0000, [email protected] wrote:
>> Hi Joe,
>>
>> On 11/5/2018 4:27 PM, Joe Perches wrote:
>>> On Thu, 2018-11-01 at 16:45 +0000, [email protected] wrote:
>>>> From: Ajay Singh <[email protected]>
>>>>
>>>> Use shorter name for 'network_info' variable to avoid line over 80 chars
>>>> issue.
>>> I suppose this is OK, though perhaps unnecessary.
>>>
>>> As well, perhaps there are defects in the original code
>>> in a couple places.
>>>
>>>> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
>>> []
>>>> - network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
>>>> - network_info->tsf_lo = (u32)network_info->tsf_hi;
>>> Perhaps there is a defect for both tsf_hi assignments
>>> as it appears as if both are missing ">> 32"
>> Actually, 'tsf_hi' is used to store the complete tsf value as its data
>> type is u64. This value is pass to the cfg80211_inform_bss() as it is.
>> So the conversion of 'tsf_hi' to 32 bit is not required in this case.
> Antipattern naming generally warrants a rename.
>
Thanks Joe.
Sure, I will rename tsf_hi in future patch to have clear meaning.

Regards,
Ajay