This patch set mainly contains changes to avoid the use of static
and global variables. Also contains few patch to avoid the checkpatch
warning arise due to code refactor.
Ajay Singh (24):
staging: wilc1000: move 'wilc_enable_ps' global variable into 'wilc'
struct
staging: wilc1000: move 'aging_timer' static variable to wilc_priv
struct
staging: wilc1000: fix to use correct index to free scanned info in
clear_shadow_scan()
staging: wilc1000: remove unnecessary NULL check in
clear_shadow_scan()
staging: wilc1000: moved last_scanned_shadow & last_scanned_cnt to
wilc_priv struct
staging: wilc1000: move during_ip_timer & wilc_optaining_ip to
'wilc_vif' struct
staging: wilc1000: remove unused variable 'op_ifcs'
staging: wilc1000: avoid use of extra 'if' condition in wilc_init()
staging: wilc1000: move static variable clients_count to 'wilc'
structure
staging: wilc1000: move wilc_multicast_mac_addr_list to 'wilc_vif'
struct
staging: wilc1000: move hif specific static variables to 'wilc'
structure
staging: wilc1000: move static variable 'terminated_handle' to
wilc_vif struct
staging: wilc1000: move 'periodic_rssi' as part of 'wilc_vif' struct
staging: wilc1000: rename 'dummy_statistics' variable to
'periodic_stat'
staging: wilc1000: move 'rcv_assoc_resp' as part of hif_drv
staging: wilc1000: refactor tcp_process() to avoid extra leading tabs
staging: wilc1000: use lowercase for get_BSSID() and HIL variable
staging: wilc1000: move tcp_ack_filter algo related variables to
'wilc_vif' struct
staging: wilc1000: avoid line over 80 chars in
wilc_wlan_txq_filter_dup_tcp_ack()
staging: wilc1000: avoid line over 80 chars in tcp_process()
staging: wilc1000: remove unused code to set and get IP address
staging: wilc1000: move 'chip_ps_state' static variable as part of
'wilc' struct
staging: wilc1000: move 'wilc_connecting' static variable to
'wilc_vif' struct
staging: wilc1000: remove unnecessary static variable
'p2p_listen_state'
drivers/staging/wilc1000/coreconfigurator.c | 4 +-
drivers/staging/wilc1000/host_interface.c | 227 +++++-----------------
drivers/staging/wilc1000/host_interface.h | 9 +-
drivers/staging/wilc1000/linux_wlan.c | 29 ++-
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 191 +++++++++---------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 47 ++++-
drivers/staging/wilc1000/wilc_wlan.c | 167 ++++++++--------
drivers/staging/wilc1000/wilc_wlan.h | 3 +-
8 files changed, 284 insertions(+), 393 deletions(-)
--
2.7.4
Hi Dan,
On Thu, 23 Aug 2018 15:37:40 +0300
Dan Carpenter <[email protected]> wrote:
> On Thu, Aug 23, 2018 at 04:57:48PM +0530, Ajay Singh wrote:
> > Hi Greg,
> >
> > On Thu, 23 Aug 2018 12:55:27 +0200
> > Greg KH <[email protected]> wrote:
> >
> > > On Tue, Aug 14, 2018 at 12:20:15PM +0530, Ajay Singh wrote:
> > > > --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > > > +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > > > @@ -151,6 +151,7 @@ struct wilc_vif {
> > > > struct timer_list periodic_rssi;
> > > > struct rf_info periodic_stat;
> > > > struct tcp_ack_filter ack_filter;
> > > > + int connecting;
> > >
> > > Shouldn't this be a boolean?
> > >
> >
> > Yes, 'connecting' only have value as 0 or 1. I will change it to
> > bool and rename it to 'is_connecting'.
>
> I think just the name "connecting" implies bool so there is no need
> for the "is_".
>
Ok. So, I will keep the same name and only change its type to
boolean.
Regards,
Ajay
Hi Adham,
On Fri, 24 Aug 2018 17:32:46 -0700
Adham Abozaeid <[email protected]> wrote:
> On Fri, 24 Aug 2018 11:47:14 +0300
> Claudiu Beznea <[email protected]> wrote:
>
> >
> >
> > On 23.08.2018 13:00, Ajay Singh wrote:
> > > Unless ndo_set_rx_mode() gets called quickly I don't think there
> > > is any issue here.
> >
> > I don't agree with this.
>
> It would be safer that the mcast list be passed to
> wilc_setup_multicast_filter() to be copied to the msg structure then
> handled by the worker thread. In this case vif->mc_mac_addr_list can
> be removed all together.
Thanks for your suggestion.
Yes, I agree we can allocate and pass 'mac_addr_list' to worker thread
so it can be handled safely. And this solution would be better than
changing handle_set_mcast_filter() to a sync call, which i have
suggested earlier.
Regards,
Ajay
Fixes to use correct index to free the allocated memory for ies
information. The check was done using 'last_scanned_cnt' index and its
not correct, so use the correct index ('i') to check for before freeing
the allocated memory.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index d853508..ede9134 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -165,9 +165,9 @@ static void clear_shadow_scan(void)
return;
for (i = 0; i < last_scanned_cnt; i++) {
- if (last_scanned_shadow[last_scanned_cnt].ies) {
+ if (last_scanned_shadow[i].ies) {
kfree(last_scanned_shadow[i].ies);
- last_scanned_shadow[last_scanned_cnt].ies = NULL;
+ last_scanned_shadow[i].ies = NULL;
}
kfree(last_scanned_shadow[i].join_params);
--
2.7.4
Avoid use of static variables and moved the varibles as part of private
data. last_scanned_shadow & last_scanned_cnt variable is moved to
'wilc_priv' to maintain for each interface. After moving static
variable, clear_shadow_scan() doesn't require check 'op_ifcs'
count as now for each interface the againg timer is initiated.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 105 +++++++++++-----------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +
2 files changed, 52 insertions(+), 55 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 00a167b..1eac244 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -82,8 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
.flags = WIPHY_WOWLAN_ANY
};
-static struct network_info last_scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
-static u32 last_scanned_cnt;
struct timer_list wilc_during_ip_timer;
static u8 op_ifcs;
@@ -157,21 +155,18 @@ static struct ieee80211_supported_band wilc_band_2ghz = {
#define AGING_TIME (9 * 1000)
#define DURING_IP_TIME_OUT 15000
-static void clear_shadow_scan(void)
+static void clear_shadow_scan(struct wilc_priv *priv)
{
int i;
- if (op_ifcs != 0)
- return;
-
- for (i = 0; i < last_scanned_cnt; i++) {
- kfree(last_scanned_shadow[i].ies);
- last_scanned_shadow[i].ies = NULL;
+ for (i = 0; i < priv->scanned_cnt; i++) {
+ kfree(priv->scanned_shadow[i].ies);
+ priv->scanned_shadow[i].ies = NULL;
- kfree(last_scanned_shadow[i].join_params);
- last_scanned_shadow[i].join_params = NULL;
+ kfree(priv->scanned_shadow[i].join_params);
+ priv->scanned_shadow[i].join_params = NULL;
}
- last_scanned_cnt = 0;
+ priv->scanned_cnt = 0;
}
static u32 get_rssi_avg(struct network_info *network_info)
@@ -193,14 +188,14 @@ static void refresh_scan(struct wilc_priv *priv, bool direct_scan)
struct wiphy *wiphy = priv->dev->ieee80211_ptr->wiphy;
int i;
- for (i = 0; i < last_scanned_cnt; i++) {
+ for (i = 0; i < priv->scanned_cnt; i++) {
struct network_info *network_info;
s32 freq;
struct ieee80211_channel *channel;
int rssi;
struct cfg80211_bss *bss;
- network_info = &last_scanned_shadow[i];
+ network_info = &priv->scanned_shadow[i];
if (!memcmp("DIRECT-", network_info->ssid, 7) && !direct_scan)
continue;
@@ -224,20 +219,20 @@ static void refresh_scan(struct wilc_priv *priv, bool direct_scan)
}
}
-static void reset_shadow_found(void)
+static void reset_shadow_found(struct wilc_priv *priv)
{
int i;
- for (i = 0; i < last_scanned_cnt; i++)
- last_scanned_shadow[i].found = 0;
+ for (i = 0; i < priv->scanned_cnt; i++)
+ priv->scanned_shadow[i].found = 0;
}
-static void update_scan_time(void)
+static void update_scan_time(struct wilc_priv *priv)
{
int i;
- for (i = 0; i < last_scanned_cnt; i++)
- last_scanned_shadow[i].time_scan = jiffies;
+ for (i = 0; i < priv->scanned_cnt; i++)
+ priv->scanned_shadow[i].time_scan = jiffies;
}
static void remove_network_from_shadow(struct timer_list *t)
@@ -246,22 +241,22 @@ static void remove_network_from_shadow(struct timer_list *t)
unsigned long now = jiffies;
int i, j;
- for (i = 0; i < last_scanned_cnt; i++) {
- if (!time_after(now, last_scanned_shadow[i].time_scan +
+ for (i = 0; i < priv->scanned_cnt; i++) {
+ if (!time_after(now, priv->scanned_shadow[i].time_scan +
(unsigned long)(SCAN_RESULT_EXPIRE)))
continue;
- kfree(last_scanned_shadow[i].ies);
- last_scanned_shadow[i].ies = NULL;
+ kfree(priv->scanned_shadow[i].ies);
+ priv->scanned_shadow[i].ies = NULL;
- kfree(last_scanned_shadow[i].join_params);
+ kfree(priv->scanned_shadow[i].join_params);
- for (j = i; (j < last_scanned_cnt - 1); j++)
- last_scanned_shadow[j] = last_scanned_shadow[j + 1];
+ for (j = i; (j < priv->scanned_cnt - 1); j++)
+ priv->scanned_shadow[j] = priv->scanned_shadow[j + 1];
- last_scanned_cnt--;
+ priv->scanned_cnt--;
}
- if (last_scanned_cnt != 0)
+ if (priv->scanned_cnt != 0)
mod_timer(&priv->aging_timer,
jiffies + msecs_to_jiffies(AGING_TIME));
}
@@ -277,13 +272,13 @@ static int is_network_in_shadow(struct network_info *nw_info,
int state = -1;
int i;
- if (last_scanned_cnt == 0) {
+ if (priv->scanned_cnt == 0) {
mod_timer(&priv->aging_timer,
jiffies + msecs_to_jiffies(AGING_TIME));
state = -1;
} else {
- for (i = 0; i < last_scanned_cnt; i++) {
- if (memcmp(last_scanned_shadow[i].bssid,
+ for (i = 0; i < priv->scanned_cnt; i++) {
+ if (memcmp(priv->scanned_shadow[i].bssid,
nw_info->bssid, 6) == 0) {
state = i;
break;
@@ -301,16 +296,16 @@ static void add_network_to_shadow(struct network_info *nw_info,
u8 rssi_index = 0;
struct network_info *shadow_nw_info;
- if (last_scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW)
+ if (priv->scanned_cnt >= MAX_NUM_SCANNED_NETWORKS_SHADOW)
return;
if (ap_found == -1) {
- ap_index = last_scanned_cnt;
- last_scanned_cnt++;
+ ap_index = priv->scanned_cnt;
+ priv->scanned_cnt++;
} else {
ap_index = ap_found;
}
- shadow_nw_info = &last_scanned_shadow[ap_index];
+ shadow_nw_info = &priv->scanned_shadow[ap_index];
rssi_index = shadow_nw_info->rssi_history.index;
shadow_nw_info->rssi_history.samples[rssi_index++] = nw_info->rssi;
if (rssi_index == NUM_RSSI) {
@@ -402,7 +397,7 @@ static void cfg_scan_result(enum scan_event scan_event,
u32 i;
for (i = 0; i < priv->rcvd_ch_cnt; i++) {
- if (memcmp(last_scanned_shadow[i].bssid,
+ if (memcmp(priv->scanned_shadow[i].bssid,
network_info->bssid, 6) == 0)
break;
}
@@ -410,8 +405,8 @@ static void cfg_scan_result(enum scan_event scan_event,
if (i >= priv->rcvd_ch_cnt)
return;
- last_scanned_shadow[i].rssi = network_info->rssi;
- last_scanned_shadow[i].time_scan = jiffies;
+ priv->scanned_shadow[i].rssi = network_info->rssi;
+ priv->scanned_shadow[i].time_scan = jiffies;
}
} else if (scan_event == SCAN_EVENT_DONE) {
refresh_scan(priv, false);
@@ -437,7 +432,7 @@ static void cfg_scan_result(enum scan_event scan_event,
.aborted = false,
};
- update_scan_time();
+ update_scan_time(priv);
refresh_scan(priv, false);
cfg80211_scan_done(priv->scan_req, &info);
@@ -448,11 +443,11 @@ static void cfg_scan_result(enum scan_event scan_event,
}
}
-static inline bool wilc_wfi_cfg_scan_time_expired(int i)
+static inline bool wilc_cfg_scan_time_expired(struct wilc_priv *priv, int i)
{
unsigned long now = jiffies;
- if (time_after(now, last_scanned_shadow[i].time_scan_cached +
+ if (time_after(now, priv->scanned_shadow[i].time_scan_cached +
(unsigned long)(nl80211_SCAN_RESULT_EXPIRE - (1 * HZ))))
return true;
else
@@ -501,11 +496,11 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
memcpy(priv->associated_bss, conn_info->bssid,
ETH_ALEN);
- for (i = 0; i < last_scanned_cnt; i++) {
- if (memcmp(last_scanned_shadow[i].bssid,
+ for (i = 0; i < priv->scanned_cnt; i++) {
+ if (memcmp(priv->scanned_shadow[i].bssid,
conn_info->bssid,
ETH_ALEN) == 0) {
- if (wilc_wfi_cfg_scan_time_expired(i))
+ if (wilc_cfg_scan_time_expired(priv, i))
scan_refresh = true;
break;
@@ -619,7 +614,7 @@ static int scan(struct wiphy *wiphy, struct cfg80211_scan_request *request)
priv->rcvd_ch_cnt = 0;
- reset_shadow_found();
+ reset_shadow_found(priv);
priv->cfg_scanning = true;
if (request->n_channels <= MAX_NUM_SCANNED_NETWORKS) {
@@ -679,18 +674,18 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
else
wfi_drv->p2p_connect = 0;
- for (i = 0; i < last_scanned_cnt; i++) {
- if (sme->ssid_len == last_scanned_shadow[i].ssid_len &&
- memcmp(last_scanned_shadow[i].ssid,
+ for (i = 0; i < priv->scanned_cnt; i++) {
+ if (sme->ssid_len == priv->scanned_shadow[i].ssid_len &&
+ memcmp(priv->scanned_shadow[i].ssid,
sme->ssid,
sme->ssid_len) == 0) {
if (!sme->bssid) {
if (sel_bssi_idx == UINT_MAX ||
- last_scanned_shadow[i].rssi >
- last_scanned_shadow[sel_bssi_idx].rssi)
+ priv->scanned_shadow[i].rssi >
+ priv->scanned_shadow[sel_bssi_idx].rssi)
sel_bssi_idx = i;
} else {
- if (memcmp(last_scanned_shadow[i].bssid,
+ if (memcmp(priv->scanned_shadow[i].bssid,
sme->bssid,
ETH_ALEN) == 0) {
sel_bssi_idx = i;
@@ -700,8 +695,8 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
}
}
- if (sel_bssi_idx < last_scanned_cnt) {
- nw_info = &last_scanned_shadow[sel_bssi_idx];
+ if (sel_bssi_idx < priv->scanned_cnt) {
+ nw_info = &priv->scanned_shadow[sel_bssi_idx];
} else {
ret = -ENOENT;
wilc_connecting = 0;
@@ -2194,7 +2189,7 @@ int wilc_deinit_host_int(struct net_device *net)
ret = wilc_deinit(vif);
del_timer_sync(&priv->aging_timer);
- clear_shadow_scan();
+ clear_shadow_scan(priv);
if (op_ifcs == 0)
del_timer_sync(&wilc_during_ip_timer);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index a76b68c..3767e31 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -95,6 +95,8 @@ struct wilc_priv {
struct mutex scan_req_lock;
bool p2p_listen_state;
struct timer_list aging_timer;
+ struct network_info scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
+ int scanned_cnt;
};
struct frame_reg {
--
2.7.4
On Thu, 23 Aug 2018 11:11:28 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Cleanup patch to avoid line over 80 chars checkpatch issue
> > introduced in previous code refactor commit.
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/wilc_wlan.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> > b/drivers/staging/wilc1000/wilc_wlan.c index 52402c3..041c9dd 100644
> > --- a/drivers/staging/wilc1000/wilc_wlan.c
> > +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > @@ -199,19 +199,20 @@ static int
> > wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev) unsigned
> > long flags;
> > spin_lock_irqsave(&wilc->txq_spinlock, flags);
> > - for (i = f->pending_base; i < (f->pending_base +
> > f->pending_acks); i++) {
> > - u32 session_index;
> > + i = f->pending_base;
> You could do it like this to avoid line over 80 chars checkpatch.pl
> warnings:
>
> for (i = f->pending_base;
> i < (f->pending_base + f->pending_acks); i++) {
>
Actually, I was not fully sure about the best approach to
handle this. :)
If the suggested is preferred approach our the over
previous one. I will include the suggestion and submit the updated
patch.
> > + for (; i < (f->pending_base + f->pending_acks); i++) {
> > + u32 index;
> > u32 bigger_ack_num;
> >
> > if (i >= MAX_PENDING_ACKS)
> > break;
> >
> > - session_index =
> > f->pending_acks_info[i].session_index;
> > + index = f->pending_acks_info[i].session_index;
> >
> > - if (session_index >= 2 * MAX_TCP_SESSION)
> > + if (index >= 2 * MAX_TCP_SESSION)
> > break;
> >
> > - bigger_ack_num =
> > f->ack_session_info[session_index].bigger_ack_num;
> > + bigger_ack_num =
> > f->ack_session_info[index].bigger_ack_num;
> > if (f->pending_acks_info[i].ack_num <
> > bigger_ack_num) { struct txq_entry_t *tqe;
> >
On 23.08.2018 12:43, Ajay Singh wrote:
> The idea was to keep private data related to 'wiphy priv'
> in 'wilc_priv' struct and 'netdev priv' related data in 'wilc_vif'
> struct.
OK, I see, agree!
Instead of using 'wilc_multicast_mac_addr_list' as global variable move
it part of wilc_vif struct. Rename 'wilc_multicast_mac_addr_list'
variable to 'mc_mac_addr_list' as its now part of 'wilc_vif' struct.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 4 +---
drivers/staging/wilc1000/host_interface.h | 1 -
drivers/staging/wilc1000/linux_wlan.c | 14 +++++++-------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index d930f06..642c314 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -193,8 +193,6 @@ static struct mutex hif_deinit_lock;
static struct timer_list periodic_rssi;
static struct wilc_vif *periodic_rssi_vif;
-u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
-
static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
static u8 set_ip[2][4];
@@ -2588,7 +2586,7 @@ static void handle_set_mcast_filter(struct work_struct *work)
*cur_byte++ = ((hif_set_mc->cnt >> 24) & 0xFF);
if (hif_set_mc->cnt > 0)
- memcpy(cur_byte, wilc_multicast_mac_addr_list,
+ memcpy(cur_byte, vif->mc_mac_addr_list,
((hif_set_mc->cnt) * ETH_ALEN));
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index d026f44..4a84dd2 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -362,7 +362,6 @@ int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power);
int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
extern u8 wilc_connected_ssid[6];
-extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
extern int wilc_connecting;
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 283bb74..bbaa653 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -822,14 +822,14 @@ static void wilc_set_multicast_list(struct net_device *dev)
}
netdev_for_each_mc_addr(ha, dev) {
- memcpy(wilc_multicast_mac_addr_list[i], ha->addr, ETH_ALEN);
+ memcpy(vif->mc_mac_addr_list[i], ha->addr, ETH_ALEN);
netdev_dbg(dev, "Entry[%d]: %x:%x:%x:%x:%x:%x\n", i,
- wilc_multicast_mac_addr_list[i][0],
- wilc_multicast_mac_addr_list[i][1],
- wilc_multicast_mac_addr_list[i][2],
- wilc_multicast_mac_addr_list[i][3],
- wilc_multicast_mac_addr_list[i][4],
- wilc_multicast_mac_addr_list[i][5]);
+ vif->mc_mac_addr_list[i][0],
+ vif->mc_mac_addr_list[i][1],
+ vif->mc_mac_addr_list[i][2],
+ vif->mc_mac_addr_list[i][3],
+ vif->mc_mac_addr_list[i][4],
+ vif->mc_mac_addr_list[i][5]);
i++;
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8cccbbc..ee8eda7 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -120,6 +120,7 @@ struct wilc_vif {
u8 ifc_id;
struct timer_list during_ip_timer;
bool obtaining_ip;
+ u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
};
struct wilc {
--
2.7.4
Refactor code to move 'periodic_rssi' as part of wilc_vif struct.Move
'dummy_statistics' from 'wilc' struct to 'wilc_vif' to maintain for
each interface separatly.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 19 ++++++++-----------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 3 ++-
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index f71f451f..a440f84 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -186,8 +186,6 @@ struct join_bss_param {
};
static u8 p2p_listen_state;
-static struct timer_list periodic_rssi;
-static struct wilc_vif *periodic_rssi_vif;
static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
@@ -3417,9 +3415,9 @@ int wilc_hif_set_cfg(struct wilc_vif *vif,
return result;
}
-static void get_periodic_rssi(struct timer_list *unused)
+static void get_periodic_rssi(struct timer_list *t)
{
- struct wilc_vif *vif = periodic_rssi_vif;
+ struct wilc_vif *vif = from_timer(vif, t, periodic_rssi);
if (!vif->hif_drv) {
netdev_err(vif->ndev, "%s: hif driver is NULL", __func__);
@@ -3427,9 +3425,9 @@ static void get_periodic_rssi(struct timer_list *unused)
}
if (vif->hif_drv->hif_state == HOST_IF_CONNECTED)
- wilc_get_statistics(vif, &vif->wilc->dummy_statistics, false);
+ wilc_get_statistics(vif, &vif->dummy_statistics, false);
- mod_timer(&periodic_rssi, jiffies + msecs_to_jiffies(5000));
+ mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
}
int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
@@ -3463,12 +3461,11 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
kfree(hif_drv);
return -ENOMEM;
}
-
- periodic_rssi_vif = vif;
- timer_setup(&periodic_rssi, get_periodic_rssi, 0);
- mod_timer(&periodic_rssi, jiffies + msecs_to_jiffies(5000));
}
+ timer_setup(&vif->periodic_rssi, get_periodic_rssi, 0);
+ mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
+
timer_setup(&hif_drv->scan_timer, timer_scan_cb, 0);
timer_setup(&hif_drv->connect_timer, timer_connect_cb, 0);
timer_setup(&hif_drv->remain_on_ch_timer, listen_timer_cb, 0);
@@ -3508,7 +3505,7 @@ int wilc_deinit(struct wilc_vif *vif)
del_timer_sync(&hif_drv->scan_timer);
del_timer_sync(&hif_drv->connect_timer);
- del_timer_sync(&periodic_rssi);
+ del_timer_sync(&vif->periodic_rssi);
del_timer_sync(&hif_drv->remain_on_ch_timer);
wilc_set_wfi_drv_handler(vif, 0, 0, 0);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index ba606d0..cdb90c3 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -122,6 +122,8 @@ struct wilc_vif {
bool obtaining_ip;
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
bool is_termination_progress;
+ struct timer_list periodic_rssi;
+ struct rf_info dummy_statistics;
};
struct wilc {
@@ -171,7 +173,6 @@ struct wilc {
struct device *dev;
bool suspend_event;
- struct rf_info dummy_statistics;
bool enable_ps;
int clients_count;
struct workqueue_struct *hif_workqueue;
--
2.7.4
Avoid use of static variable and move it in 'wilc' structure related to
hif and added NULL before accessing hif_workqueue in wilc_enqueue_work().
Below variables are moved to 'wilc' struct:
struct workqueue_struct *hif_workqueue;
struct mutex hif_deinit_lock;
struct completion hif_driver_comp;
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 43 ++++++++++++++-------------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 4 +++
2 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 642c314..d5d81843 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,9 +187,6 @@ struct join_bss_param {
static struct host_if_drv *terminated_handle;
static u8 p2p_listen_state;
-static struct workqueue_struct *hif_workqueue;
-static struct completion hif_driver_comp;
-static struct mutex hif_deinit_lock;
static struct timer_list periodic_rssi;
static struct wilc_vif *periodic_rssi_vif;
@@ -225,7 +222,11 @@ wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
static int wilc_enqueue_work(struct host_if_msg *msg)
{
INIT_WORK(&msg->work, msg->fn);
- if (!hif_workqueue || !queue_work(hif_workqueue, &msg->work))
+
+ if (!msg->vif || !msg->vif->wilc || !msg->vif->wilc->hif_workqueue)
+ return -EINVAL;
+
+ if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
return -EINVAL;
return 0;
@@ -316,7 +317,7 @@ static void handle_set_wfi_drv_handler(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Failed to set driver handler\n");
- complete(&hif_driver_comp);
+ complete(&vif->wilc->hif_driver_comp);
kfree(buffer);
free_msg:
@@ -340,7 +341,7 @@ static void handle_set_operation_mode(struct work_struct *work)
wilc_get_vif_idx(vif));
if (hif_op_mode->mode == IDLE_MODE)
- complete(&hif_driver_comp);
+ complete(&vif->wilc->hif_driver_comp);
if (ret)
netdev_err(vif->ndev, "Failed to set operation mode\n");
@@ -3454,11 +3455,11 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
vif->obtaining_ip = false;
if (wilc->clients_count == 0) {
- init_completion(&hif_driver_comp);
- mutex_init(&hif_deinit_lock);
+ init_completion(&wilc->hif_driver_comp);
+ mutex_init(&wilc->hif_deinit_lock);
- hif_workqueue = create_singlethread_workqueue("WILC_wq");
- if (!hif_workqueue) {
+ wilc->hif_workqueue = create_singlethread_workqueue("WILC_wq");
+ if (!wilc->hif_workqueue) {
netdev_err(vif->ndev, "Failed to create workqueue\n");
kfree(hif_drv);
return -ENOMEM;
@@ -3502,7 +3503,7 @@ int wilc_deinit(struct wilc_vif *vif)
return -EFAULT;
}
- mutex_lock(&hif_deinit_lock);
+ mutex_lock(&vif->wilc->hif_deinit_lock);
terminated_handle = hif_drv;
@@ -3512,7 +3513,7 @@ int wilc_deinit(struct wilc_vif *vif)
del_timer_sync(&hif_drv->remain_on_ch_timer);
wilc_set_wfi_drv_handler(vif, 0, 0, 0);
- wait_for_completion(&hif_driver_comp);
+ wait_for_completion(&vif->wilc->hif_driver_comp);
if (hif_drv->usr_scan_req.scan_result) {
hif_drv->usr_scan_req.scan_result(SCAN_EVENT_ABORTED, NULL,
@@ -3536,14 +3537,14 @@ int wilc_deinit(struct wilc_vif *vif)
wait_for_completion(&msg->work_comp);
kfree(msg);
}
- destroy_workqueue(hif_workqueue);
+ destroy_workqueue(vif->wilc->hif_workqueue);
}
kfree(hif_drv);
vif->wilc->clients_count--;
terminated_handle = NULL;
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&vif->wilc->hif_deinit_lock);
return result;
}
@@ -3596,7 +3597,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
struct host_if_drv *hif_drv;
struct wilc_vif *vif;
- mutex_lock(&hif_deinit_lock);
+ mutex_lock(&wilc->hif_deinit_lock);
id = buffer[length - 4];
id |= (buffer[length - 3] << 8);
@@ -3604,26 +3605,26 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
id |= (buffer[length - 1] << 24);
vif = wilc_get_vif_from_idx(wilc, id);
if (!vif) {
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
return;
}
hif_drv = vif->hif_drv;
if (!hif_drv || hif_drv == terminated_handle) {
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
return;
}
if (!hif_drv->usr_conn_req.conn_result) {
netdev_err(vif->ndev, "%s: conn_result is NULL\n", __func__);
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
return;
}
msg = wilc_alloc_work(vif, handle_rcvd_gnrl_async_info, false);
if (IS_ERR(msg)) {
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
return;
}
@@ -3631,7 +3632,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
msg->body.async_info.buffer = kmemdup(buffer, length, GFP_KERNEL);
if (!msg->body.async_info.buffer) {
kfree(msg);
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
return;
}
@@ -3642,7 +3643,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
kfree(msg);
}
- mutex_unlock(&hif_deinit_lock);
+ mutex_unlock(&wilc->hif_deinit_lock);
}
void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length)
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index ee8eda7..eb00e42 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -173,6 +173,10 @@ struct wilc {
struct rf_info dummy_statistics;
bool enable_ps;
int clients_count;
+ struct workqueue_struct *hif_workqueue;
+ /* deinitialization lock */
+ struct mutex hif_deinit_lock;
+ struct completion hif_driver_comp;
};
struct wilc_wfi_mon_priv {
--
2.7.4
On 14.08.2018 09:50, Ajay Singh wrote:
> Cleanup patch to avoid line over 80 chars issue reported by
> checkpatch.pl script.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 041c9dd..f0743d9 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct wilc_vif *vif, u32 ack,
> return 0;
> }
>
> +static inline void clear_tcp_session_txq(struct wilc_vif *vif, int index)
> +{
> + vif->ack_filter.pending_acks_info[index].txqe = NULL;
> +}
> +
This seems useless to me...
> static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
> {
> void *buffer = tqe->buffer;
> @@ -670,7 +675,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
> tqe->tx_complete_func(tqe->priv, tqe->status);
> if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
> tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
> - vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
> + clear_tcp_session_txq(vif, tqe->tcp_pending_ack_idx);
> kfree(tqe);
> } while (--entries);
>
>
Instead of having 'wilc_enable_ps' as global variable moved it to 'wilc'
structure. Rename 'wilc_enable_ps' to 'enable_ps' as its already part of
'wilc' structure
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/linux_wlan.c | 5 ++---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
drivers/staging/wilc1000/wilc_wlan.h | 1 -
4 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 01cf4bd..57e3176 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -12,8 +12,6 @@
#include "wilc_wfi_cfgoperations.h"
-bool wilc_enable_ps = true;
-
static int dev_state_ev_handler(struct notifier_block *this,
unsigned long event, void *ptr)
{
@@ -54,7 +52,7 @@ static int dev_state_ev_handler(struct notifier_block *this,
del_timer(&wilc_during_ip_timer);
}
- if (wilc_enable_ps)
+ if (vif->wilc->enable_ps)
wilc_set_power_mgmt(vif, 1, 0);
netdev_dbg(dev, "[%s] Up IP\n", dev_iface->ifa_label);
@@ -1066,6 +1064,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
*wilc = wl;
wl->io_type = io_type;
wl->hif_func = ops;
+ wl->enable_ps = true;
INIT_LIST_HEAD(&wl->txq_head.list);
INIT_LIST_HEAD(&wl->rxq_head.list);
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 7cd0330..4727a8a 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1732,7 +1732,7 @@ static int set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
if (!priv->hif_drv)
return -EIO;
- if (wilc_enable_ps)
+ if (vif->wilc->enable_ps)
wilc_set_power_mgmt(vif, enabled, timeout);
return 0;
@@ -1764,7 +1764,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
memset(priv->assoc_stainfo.sta_associated_bss, 0,
MAX_NUM_STA * ETH_ALEN);
- wilc_enable_ps = true;
+ wl->enable_ps = true;
wilc_set_power_mgmt(vif, 1, 0);
break;
@@ -1776,12 +1776,12 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
vif->iftype = CLIENT_MODE;
wilc_set_operation_mode(vif, STATION_MODE);
- wilc_enable_ps = false;
+ wl->enable_ps = false;
wilc_set_power_mgmt(vif, 0, 0);
break;
case NL80211_IFTYPE_AP:
- wilc_enable_ps = false;
+ wl->enable_ps = false;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->iftype = AP_MODE;
@@ -1803,7 +1803,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
priv->wdev->iftype = type;
vif->iftype = GO_MODE;
- wilc_enable_ps = false;
+ wl->enable_ps = false;
wilc_set_power_mgmt(vif, 0, 0);
break;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index b7eee77..8b74d61 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -166,6 +166,7 @@ struct wilc {
bool suspend_event;
struct rf_info dummy_statistics;
+ bool enable_ps;
};
struct wilc_wfi_mon_priv {
diff --git a/drivers/staging/wilc1000/wilc_wlan.h b/drivers/staging/wilc1000/wilc_wlan.h
index 7467188..1f874d1 100644
--- a/drivers/staging/wilc1000/wilc_wlan.h
+++ b/drivers/staging/wilc1000/wilc_wlan.h
@@ -289,7 +289,6 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size);
void host_wakeup_notify(struct wilc *wilc);
void host_sleep_notify(struct wilc *wilc);
-extern bool wilc_enable_ps;
void chip_allow_sleep(struct wilc *wilc);
void chip_wakeup(struct wilc *wilc);
int wilc_send_config_pkt(struct wilc_vif *vif, u8 mode, struct wid *wids,
--
2.7.4
On Fri, 24 Aug 2018 11:46:39 +0300
Claudiu Beznea <[email protected]> wrote:
> On 23.08.2018 17:36, Ajay Singh wrote:
> > On Thu, 23 Aug 2018 11:11:18 +0300
> > Claudiu Beznea <[email protected]> wrote:
> >
> >> On 14.08.2018 09:50, Ajay Singh wrote:
> >>> Remove the use of static variable 'terminated_handle' and instead
> >>> move in wilc_vif struct.
> >>> After moving this variable to wilc_vif struct its not required to
> >>> keep 'terminated_handle', so changed it to boolean type.
> >>
> >> You can remove it at all and use wilc->hif_deinit_lock mutex also
> >> in wilc_scan_complete_received() and wilc_network_info_received()
> >> it is used in wilc_gnrl_async_info_received().
> >
> > In my understanding, 'terminated_handle' is used to know the
> > status when interface is getting deinit(). During deinitialization
> > of an interface if any async event received for the interface(from
> > firmware) should be ignored.
>
> 'terminated_handle' true only inside mutex. So, outside of it it will
> be false, so *mostly* it will tell you when mutex is locked for
> deinit. *Mostly*, because context switches may happen while a mutex
> is locked.
>
Yes, outside the mutex 'terminated_handle' would be false.
I already agreed that while fixing the bug using 'terminated_handle'
all the scenarios are not handled but as you suggested before
to remove 'terminated_handle' and only use 'mutex' will also not
help to address all corner scenarios. So, I suggest to keep the
current status by making use of this flag and handle all scenario in
later patch to de-initialization graceful.
> With the current approach you have this code:
>
> int wilc_deinit(struct wilc_vif *vif)
> {
> // ...
> mutex_lock(&vif->wilc->hif_deinit_lock);
>
> // (A)
>
> vif->is_termination_progress = true;
> // ...
> vif->is_termination_progress = false;
>
> mutex_unlokc(&vif->wilc->hif_deinit_lock);
> }
>
> And:
>
> void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32
> length) {
> // ...
> if (!hif_drv || vif->is_termination_progress) {
> netdev_err(vif->ndev, "driver not init[%p]\n",
> hif_drv); return;
> }
>
> // ...
>
> // (B)
> result = wilc_enqueue_work(msg);
> // ...
> }
>
> And:
>
> static int wilc_enqueue_work(struct host_if_msg *msg)
>
> {
>
> INIT_WORK(&msg->work, msg->fn);
>
>
>
> if (!msg->vif || !msg->vif->wilc
> || !msg->vif->wilc->hif_workqueue)
>
> return -EINVAL;
>
>
> // (C)
> if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
>
> return -EINVAL;
>
>
>
> return 0;
>
> }
>
>
> You may have the following scenario:
> 1. context switch in wilc_deinit() just after the mutex is taken
> (point A above). vif->is_termination_progress = false at this point.
>
> 2. a new packet is received and wilc_network_info_received() gets
> executed and execution reaches point B, goes to wilc_enqueue_work()
> and suspend at point C then context switch.
>
Thanks for explaining the scenario with an example.
For the given scenario, I think not only here it can even suspend
after posting the work queue and start the execution of handler
function eg. handle_recvd_gnrl_async_info()(there is no protection in
handle function)
There are some combination of these scenario, I will relook into these
cases and check how to handle them separately.
> 3. wilc_deinit() resumes and finish its execution.
>
> 4. wilc_enqueue_work() resumes and queue_work() is executed but you
> already freed the hif_workqueue. You will have a crash here.
>
> Notice that hif_drv is not set to NULL on wilc_deinit() after it is
> kfree().
>
> >
> > In my opinion its not right to only wait for the mutex in any of
> > callback e.g wilc_scan_complete_received() because it will delay the
> > handling of callback and try to process the event once lock is
> > available for the interface which is already de-initialized.
>
> But this is already done for wilc_gnrl_async_info_received().
>
> >
> > Based on my understand 'mutex' alone is not enough to
> > handle this and we some extra check to know if interface is down.
>
> terminated_handle will *mostly* tell you when the mutex is locked,
> nothing more.
>
> I will
> > check more about this patch how to handle the extra scenario for
> > this case.
> > Please suggest if someone has better idea on how to handle this.
> >
Cleanup code to remove the variables related to setting and getting IP
address as this case was not handled from firmware side.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 105 ------------------------------
drivers/staging/wilc1000/host_interface.h | 3 -
drivers/staging/wilc1000/linux_wlan.c | 3 -
3 files changed, 111 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index bffe0c8..f37ba64 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,11 +187,6 @@ struct join_bss_param {
static u8 p2p_listen_state;
-static u8 set_ip[2][4];
-static u8 get_ip[2][4];
-
-static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
-
/* 'msg' should be free by the caller for syc */
static struct host_if_msg*
wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
@@ -344,64 +339,6 @@ static void handle_set_operation_mode(struct work_struct *work)
kfree(msg);
}
-static void handle_set_ip_address(struct work_struct *work)
-{
- struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
- struct wilc_vif *vif = msg->vif;
- u8 *ip_addr = msg->body.ip_info.ip_addr;
- u8 idx = msg->body.ip_info.idx;
- int ret;
- struct wid wid;
- char firmware_ip_addr[4] = {0};
-
- if (ip_addr[0] < 192)
- ip_addr[0] = 0;
-
- memcpy(set_ip[idx], ip_addr, IP_ALEN);
-
- wid.id = WID_IP_ADDRESS;
- wid.type = WID_STR;
- wid.val = ip_addr;
- wid.size = IP_ALEN;
-
- ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
- wilc_get_vif_idx(vif));
-
- host_int_get_ipaddress(vif, firmware_ip_addr, idx);
-
- if (ret)
- netdev_err(vif->ndev, "Failed to set IP address\n");
- kfree(msg);
-}
-
-static void handle_get_ip_address(struct work_struct *work)
-{
- struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
- struct wilc_vif *vif = msg->vif;
- u8 idx = msg->body.ip_info.idx;
- int ret;
- struct wid wid;
-
- wid.id = WID_IP_ADDRESS;
- wid.type = WID_STR;
- wid.val = kmalloc(IP_ALEN, GFP_KERNEL);
- wid.size = IP_ALEN;
-
- ret = wilc_send_config_pkt(vif, GET_CFG, &wid, 1,
- wilc_get_vif_idx(vif));
-
- memcpy(get_ip[idx], wid.val, IP_ALEN);
-
- kfree(wid.val);
-
- if (memcmp(get_ip[idx], set_ip[idx], IP_ALEN) != 0)
- wilc_setup_ipaddress(vif, set_ip[idx], idx);
-
- if (ret)
- netdev_err(vif->ndev, "Failed to get IP address\n");
- kfree(msg);
-}
-
static void handle_get_mac_address(struct work_struct *work)
{
struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
@@ -4002,48 +3939,6 @@ int wilc_setup_multicast_filter(struct wilc_vif *vif, bool enabled,
return result;
}
-int wilc_setup_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
-{
- int result;
- struct host_if_msg *msg;
-
- msg = wilc_alloc_work(vif, handle_set_ip_address, false);
- if (IS_ERR(msg))
- return PTR_ERR(msg);
-
- msg->body.ip_info.ip_addr = ip_addr;
- msg->body.ip_info.idx = idx;
-
- result = wilc_enqueue_work(msg);
- if (result) {
- netdev_err(vif->ndev, "%s: enqueue work failed\n", __func__);
- kfree(msg);
- }
-
- return result;
-}
-
-static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
-{
- int result;
- struct host_if_msg *msg;
-
- msg = wilc_alloc_work(vif, handle_get_ip_address, false);
- if (IS_ERR(msg))
- return PTR_ERR(msg);
-
- msg->body.ip_info.ip_addr = ip_addr;
- msg->body.ip_info.idx = idx;
-
- result = wilc_enqueue_work(msg);
- if (result) {
- netdev_err(vif->ndev, "%s: enqueue work failed\n", __func__);
- kfree(msg);
- }
-
- return result;
-}
-
int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power)
{
int ret;
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 80cb298..0f0d509 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -9,8 +9,6 @@
#include <linux/ieee80211.h>
#include "coreconfigurator.h"
-#define IP_ALEN 4
-
#define IDLE_MODE 0x00
#define AP_MODE 0x01
#define STATION_MODE 0x02
@@ -344,7 +342,6 @@ int wilc_edit_station(struct wilc_vif *vif,
int wilc_set_power_mgmt(struct wilc_vif *vif, bool enabled, u32 timeout);
int wilc_setup_multicast_filter(struct wilc_vif *vif, bool enabled,
u32 count);
-int wilc_setup_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
int wilc_remain_on_channel(struct wilc_vif *vif, u32 session_id,
u32 duration, u16 chan,
wilc_remain_on_chan_expired expired,
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index bbaa653..74f8166 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -61,7 +61,6 @@ static int dev_state_ev_handler(struct notifier_block *this,
netdev_dbg(dev, "IP add=%d:%d:%d:%d\n",
ip_addr_buf[0], ip_addr_buf[1],
ip_addr_buf[2], ip_addr_buf[3]);
- wilc_setup_ipaddress(vif, ip_addr_buf, vif->idx);
break;
@@ -83,8 +82,6 @@ static int dev_state_ev_handler(struct notifier_block *this,
ip_addr_buf[0], ip_addr_buf[1],
ip_addr_buf[2], ip_addr_buf[3]);
- wilc_setup_ipaddress(vif, ip_addr_buf, vif->idx);
-
break;
default:
--
2.7.4
On 14.08.2018 09:50, Ajay Singh wrote:
> Remove the use of static variable 'terminated_handle' and instead move
> in wilc_vif struct.
> After moving this variable to wilc_vif struct its not required to keep
> 'terminated_handle', so changed it to boolean type.
You can remove it at all and use wilc->hif_deinit_lock mutex also in
wilc_scan_complete_received() and wilc_network_info_received() it is used
in wilc_gnrl_async_info_received().
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 11 +++++------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index d5d81843..f71f451f 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -185,7 +185,6 @@ struct join_bss_param {
> u8 start_time[4];
> };
>
> -static struct host_if_drv *terminated_handle;
> static u8 p2p_listen_state;
> static struct timer_list periodic_rssi;
> static struct wilc_vif *periodic_rssi_vif;
> @@ -3505,7 +3504,7 @@ int wilc_deinit(struct wilc_vif *vif)
>
> mutex_lock(&vif->wilc->hif_deinit_lock);
>
> - terminated_handle = hif_drv;
> + vif->is_termination_progress = true;
>
> del_timer_sync(&hif_drv->scan_timer);
> del_timer_sync(&hif_drv->connect_timer);
> @@ -3543,7 +3542,7 @@ int wilc_deinit(struct wilc_vif *vif)
> kfree(hif_drv);
>
> vif->wilc->clients_count--;
> - terminated_handle = NULL;
> + vif->is_termination_progress = false;
> mutex_unlock(&vif->wilc->hif_deinit_lock);
> return result;
> }
> @@ -3565,7 +3564,7 @@ void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length)
> return;
> hif_drv = vif->hif_drv;
>
> - if (!hif_drv || hif_drv == terminated_handle) {
> + if (!hif_drv || vif->is_termination_progress) {
> netdev_err(vif->ndev, "driver not init[%p]\n", hif_drv);
> return;
> }
> @@ -3611,7 +3610,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
>
> hif_drv = vif->hif_drv;
>
> - if (!hif_drv || hif_drv == terminated_handle) {
> + if (!hif_drv || vif->is_termination_progress) {
> mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
> @@ -3662,7 +3661,7 @@ void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length)
> return;
> hif_drv = vif->hif_drv;
>
> - if (!hif_drv || hif_drv == terminated_handle)
> + if (!hif_drv || vif->is_termination_progress)
> return;
>
> if (hif_drv->usr_scan_req.scan_result) {
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index eb00e42..ba606d0 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -121,6 +121,7 @@ struct wilc_vif {
> struct timer_list during_ip_timer;
> bool obtaining_ip;
> u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> + bool is_termination_progress;
> };
>
> struct wilc {
>
On Tue, Aug 14, 2018 at 12:20:15PM +0530, Ajay Singh wrote:
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -151,6 +151,7 @@ struct wilc_vif {
> struct timer_list periodic_rssi;
> struct rf_info periodic_stat;
> struct tcp_ack_filter ack_filter;
> + int connecting;
Shouldn't this be a boolean?
thanks,
greg k-h
Hi Ajay,
Few comments on this series. See per patch replies.
Thank you,
Claudiu Beznea
On 14.08.2018 09:49, Ajay Singh wrote:
> This patch set mainly contains changes to avoid the use of static
> and global variables. Also contains few patch to avoid the checkpatch
> warning arise due to code refactor.
>
> Ajay Singh (24):
> staging: wilc1000: move 'wilc_enable_ps' global variable into 'wilc'
> struct
> staging: wilc1000: move 'aging_timer' static variable to wilc_priv
> struct
> staging: wilc1000: fix to use correct index to free scanned info in
> clear_shadow_scan()
> staging: wilc1000: remove unnecessary NULL check in
> clear_shadow_scan()
> staging: wilc1000: moved last_scanned_shadow & last_scanned_cnt to
> wilc_priv struct
> staging: wilc1000: move during_ip_timer & wilc_optaining_ip to
> 'wilc_vif' struct
> staging: wilc1000: remove unused variable 'op_ifcs'
> staging: wilc1000: avoid use of extra 'if' condition in wilc_init()
> staging: wilc1000: move static variable clients_count to 'wilc'
> structure
> staging: wilc1000: move wilc_multicast_mac_addr_list to 'wilc_vif'
> struct
> staging: wilc1000: move hif specific static variables to 'wilc'
> structure
> staging: wilc1000: move static variable 'terminated_handle' to
> wilc_vif struct
> staging: wilc1000: move 'periodic_rssi' as part of 'wilc_vif' struct
> staging: wilc1000: rename 'dummy_statistics' variable to
> 'periodic_stat'
> staging: wilc1000: move 'rcv_assoc_resp' as part of hif_drv
> staging: wilc1000: refactor tcp_process() to avoid extra leading tabs
> staging: wilc1000: use lowercase for get_BSSID() and HIL variable
> staging: wilc1000: move tcp_ack_filter algo related variables to
> 'wilc_vif' struct
> staging: wilc1000: avoid line over 80 chars in
> wilc_wlan_txq_filter_dup_tcp_ack()
> staging: wilc1000: avoid line over 80 chars in tcp_process()
> staging: wilc1000: remove unused code to set and get IP address
> staging: wilc1000: move 'chip_ps_state' static variable as part of
> 'wilc' struct
> staging: wilc1000: move 'wilc_connecting' static variable to
> 'wilc_vif' struct
> staging: wilc1000: remove unnecessary static variable
> 'p2p_listen_state'
>
> drivers/staging/wilc1000/coreconfigurator.c | 4 +-
> drivers/staging/wilc1000/host_interface.c | 227 +++++-----------------
> drivers/staging/wilc1000/host_interface.h | 9 +-
> drivers/staging/wilc1000/linux_wlan.c | 29 ++-
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 191 +++++++++---------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 47 ++++-
> drivers/staging/wilc1000/wilc_wlan.c | 167 ++++++++--------
> drivers/staging/wilc1000/wilc_wlan.h | 3 +-
> 8 files changed, 284 insertions(+), 393 deletions(-)
>
Avoid use of static variable 'clients_count' and move it part of 'wilc'
structure.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 9 ++++-----
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 6225e67..d930f06 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -199,7 +199,6 @@ static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
static u8 set_ip[2][4];
static u8 get_ip[2][4];
-static u32 clients_count;
static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
@@ -3456,7 +3455,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
vif->obtaining_ip = false;
- if (clients_count == 0) {
+ if (wilc->clients_count == 0) {
init_completion(&hif_driver_comp);
mutex_init(&hif_deinit_lock);
@@ -3490,7 +3489,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
mutex_unlock(&hif_drv->cfg_values_lock);
- clients_count++;
+ wilc->clients_count++;
return 0;
}
@@ -3526,7 +3525,7 @@ int wilc_deinit(struct wilc_vif *vif)
hif_drv->hif_state = HOST_IF_IDLE;
- if (clients_count == 1) {
+ if (vif->wilc->clients_count == 1) {
struct host_if_msg *msg;
msg = wilc_alloc_work(vif, handle_hif_exit_work, true);
@@ -3544,7 +3543,7 @@ int wilc_deinit(struct wilc_vif *vif)
kfree(hif_drv);
- clients_count--;
+ vif->wilc->clients_count--;
terminated_handle = NULL;
mutex_unlock(&hif_deinit_lock);
return result;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8e56a28..8cccbbc 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -171,6 +171,7 @@ struct wilc {
struct rf_info dummy_statistics;
bool enable_ps;
+ int clients_count;
};
struct wilc_wfi_mon_priv {
--
2.7.4
Remove the use of static variable 'terminated_handle' and instead move
in wilc_vif struct.
After moving this variable to wilc_vif struct its not required to keep
'terminated_handle', so changed it to boolean type.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 11 +++++------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index d5d81843..f71f451f 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -185,7 +185,6 @@ struct join_bss_param {
u8 start_time[4];
};
-static struct host_if_drv *terminated_handle;
static u8 p2p_listen_state;
static struct timer_list periodic_rssi;
static struct wilc_vif *periodic_rssi_vif;
@@ -3505,7 +3504,7 @@ int wilc_deinit(struct wilc_vif *vif)
mutex_lock(&vif->wilc->hif_deinit_lock);
- terminated_handle = hif_drv;
+ vif->is_termination_progress = true;
del_timer_sync(&hif_drv->scan_timer);
del_timer_sync(&hif_drv->connect_timer);
@@ -3543,7 +3542,7 @@ int wilc_deinit(struct wilc_vif *vif)
kfree(hif_drv);
vif->wilc->clients_count--;
- terminated_handle = NULL;
+ vif->is_termination_progress = false;
mutex_unlock(&vif->wilc->hif_deinit_lock);
return result;
}
@@ -3565,7 +3564,7 @@ void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length)
return;
hif_drv = vif->hif_drv;
- if (!hif_drv || hif_drv == terminated_handle) {
+ if (!hif_drv || vif->is_termination_progress) {
netdev_err(vif->ndev, "driver not init[%p]\n", hif_drv);
return;
}
@@ -3611,7 +3610,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
hif_drv = vif->hif_drv;
- if (!hif_drv || hif_drv == terminated_handle) {
+ if (!hif_drv || vif->is_termination_progress) {
mutex_unlock(&wilc->hif_deinit_lock);
return;
}
@@ -3662,7 +3661,7 @@ void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length)
return;
hif_drv = vif->hif_drv;
- if (!hif_drv || hif_drv == terminated_handle)
+ if (!hif_drv || vif->is_termination_progress)
return;
if (hif_drv->usr_scan_req.scan_result) {
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index eb00e42..ba606d0 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -121,6 +121,7 @@ struct wilc_vif {
struct timer_list during_ip_timer;
bool obtaining_ip;
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
+ bool is_termination_progress;
};
struct wilc {
--
2.7.4
Cleanup patch to avoid the avoid extra 'if' condition and clubbed the
same condition in single 'if' block.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 11eb632..6225e67 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3459,9 +3459,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
if (clients_count == 0) {
init_completion(&hif_driver_comp);
mutex_init(&hif_deinit_lock);
- }
- if (clients_count == 0) {
hif_workqueue = create_singlethread_workqueue("WILC_wq");
if (!hif_workqueue) {
netdev_err(vif->ndev, "Failed to create workqueue\n");
--
2.7.4
Move the static variable as part of 'wilc' priv struct.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/linux_wlan.c | 1 +
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 ++
drivers/staging/wilc1000/wilc_wlan.c | 10 ++++------
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 74f8166..4b99de2 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1062,6 +1062,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
wl->io_type = io_type;
wl->hif_func = ops;
wl->enable_ps = true;
+ wl->chip_ps_state = CHIP_WAKEDUP;
INIT_LIST_HEAD(&wl->txq_head.list);
INIT_LIST_HEAD(&wl->rxq_head.list);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 54ce1ff..9d57adb 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -206,6 +206,8 @@ struct wilc {
/* deinitialization lock */
struct mutex hif_deinit_lock;
struct completion hif_driver_comp;
+
+ enum chip_ps_states chip_ps_state;
};
struct wilc_wfi_mon_priv {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index f0743d9..fb8a1e4 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -9,8 +9,6 @@
#include "wilc_wfi_netdevice.h"
#include "wilc_wlan_cfg.h"
-static enum chip_ps_states chip_ps_state = CHIP_WAKEDUP;
-
static inline bool is_wilc1000(u32 id)
{
return ((id & 0xfffff000) == 0x100000 ? true : false);
@@ -449,7 +447,7 @@ void chip_wakeup(struct wilc *wilc)
} while ((clk_status_reg & 0x1) == 0);
}
- if (chip_ps_state == CHIP_SLEEPING_MANUAL) {
+ if (wilc->chip_ps_state == CHIP_SLEEPING_MANUAL) {
if (wilc_get_chipid(wilc, false) < 0x1002b0) {
u32 val32;
@@ -462,19 +460,19 @@ void chip_wakeup(struct wilc *wilc)
wilc->hif_func->hif_write_reg(wilc, 0x1e9c, val32);
}
}
- chip_ps_state = CHIP_WAKEDUP;
+ wilc->chip_ps_state = CHIP_WAKEDUP;
}
void wilc_chip_sleep_manually(struct wilc *wilc)
{
- if (chip_ps_state != CHIP_WAKEDUP)
+ if (wilc->chip_ps_state != CHIP_WAKEDUP)
return;
acquire_bus(wilc, ACQUIRE_ONLY);
chip_allow_sleep(wilc);
wilc->hif_func->hif_write_reg(wilc, 0x10a8, 1);
- chip_ps_state = CHIP_SLEEPING_MANUAL;
+ wilc->chip_ps_state = CHIP_SLEEPING_MANUAL;
release_bus(wilc, RELEASE_ONLY);
}
--
2.7.4
On Thu, 23 Aug 2018 11:11:18 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Remove the use of static variable 'terminated_handle' and instead
> > move in wilc_vif struct.
> > After moving this variable to wilc_vif struct its not required to
> > keep 'terminated_handle', so changed it to boolean type.
>
> You can remove it at all and use wilc->hif_deinit_lock mutex also in
> wilc_scan_complete_received() and wilc_network_info_received() it is
> used in wilc_gnrl_async_info_received().
In my understanding, 'terminated_handle' is used to know the
status when interface is getting deinit(). During deinitialization
of an interface if any async event received for the interface(from
firmware) should be ignored.
In my opinion its not right to only wait for the mutex in any of
callback e.g wilc_scan_complete_received() because it will delay the
handling of callback and try to process the event once lock is
available for the interface which is already de-initialized.
Based on my understand 'mutex' alone is not enough to
handle this and we some extra check to know if interface is down.I will
check more about this patch how to handle the extra scenario for this
case.
Please suggest if someone has better idea on how to handle this.
>
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 11 +++++------
> > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index
> > d5d81843..f71f451f 100644 ---
> > a/drivers/staging/wilc1000/host_interface.c +++
> > b/drivers/staging/wilc1000/host_interface.c @@ -185,7 +185,6 @@
> > struct join_bss_param { u8 start_time[4];
> > };
> >
> > -static struct host_if_drv *terminated_handle;
> > static u8 p2p_listen_state;
> > static struct timer_list periodic_rssi;
> > static struct wilc_vif *periodic_rssi_vif;
> > @@ -3505,7 +3504,7 @@ int wilc_deinit(struct wilc_vif *vif)
> >
> > mutex_lock(&vif->wilc->hif_deinit_lock);
> >
> > - terminated_handle = hif_drv;
> > + vif->is_termination_progress = true;
> >
> > del_timer_sync(&hif_drv->scan_timer);
> > del_timer_sync(&hif_drv->connect_timer);
> > @@ -3543,7 +3542,7 @@ int wilc_deinit(struct wilc_vif *vif)
> > kfree(hif_drv);
> >
> > vif->wilc->clients_count--;
> > - terminated_handle = NULL;
> > + vif->is_termination_progress = false;
> > mutex_unlock(&vif->wilc->hif_deinit_lock);
> > return result;
> > }
> > @@ -3565,7 +3564,7 @@ void wilc_network_info_received(struct wilc
> > *wilc, u8 *buffer, u32 length) return;
> > hif_drv = vif->hif_drv;
> >
> > - if (!hif_drv || hif_drv == terminated_handle) {
> > + if (!hif_drv || vif->is_termination_progress) {
> > netdev_err(vif->ndev, "driver not init[%p]\n",
> > hif_drv); return;
> > }
> > @@ -3611,7 +3610,7 @@ void wilc_gnrl_async_info_received(struct
> > wilc *wilc, u8 *buffer, u32 length)
> > hif_drv = vif->hif_drv;
> >
> > - if (!hif_drv || hif_drv == terminated_handle) {
> > + if (!hif_drv || vif->is_termination_progress) {
> > mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> > @@ -3662,7 +3661,7 @@ void wilc_scan_complete_received(struct wilc
> > *wilc, u8 *buffer, u32 length) return;
> > hif_drv = vif->hif_drv;
> >
> > - if (!hif_drv || hif_drv == terminated_handle)
> > + if (!hif_drv || vif->is_termination_progress)
> > return;
> >
> > if (hif_drv->usr_scan_req.scan_result) {
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
> > eb00e42..ba606d0 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -121,6 +121,7 @@
> > struct wilc_vif { struct timer_list during_ip_timer;
> > bool obtaining_ip;
> > u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> > + bool is_termination_progress;
> > };
> >
> > struct wilc {
> >
On 14.08.2018 09:50, Ajay Singh wrote:
> Move static variable 'wilc_connecting' as part of 'wilc_vif' private
> struct. Remove "wilc_" prefix from name as its already part of wilc_vif
> struct.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 4 ++--
> drivers/staging/wilc1000/host_interface.h | 2 --
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 ++++++++----------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> 4 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index f37ba64..d8cc08b 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -720,7 +720,7 @@ static void handle_scan(struct work_struct *work)
> goto error;
> }
>
> - if (vif->obtaining_ip || wilc_connecting) {
> + if (vif->obtaining_ip || vif->connecting) {
As far as I can tell this is also set/read from different contexts, so, it
should also be protected by a locking mechanism. If not in this patch then
in a future one...
> netdev_err(vif->ndev, "Don't do obss scan\n");
> result = -EBUSY;
> goto error;
> @@ -2326,7 +2326,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
> goto error;
> }
>
> - if (vif->obtaining_ip || wilc_connecting) {
> + if (vif->obtaining_ip || vif->connecting) {
> result = -EBUSY;
> goto error;
> }
> diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
> index 0f0d509..4048eab 100644
> --- a/drivers/staging/wilc1000/host_interface.h
> +++ b/drivers/staging/wilc1000/host_interface.h
> @@ -361,6 +361,4 @@ int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
>
> extern u8 wilc_connected_ssid[6];
>
> -extern int wilc_connecting;
> -
> #endif
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index 35a83d4..cc44640 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -453,8 +453,6 @@ static inline bool wilc_cfg_scan_time_expired(struct wilc_priv *priv, int i)
> return false;
> }
>
> -int wilc_connecting;
> -
> static void cfg_connect_result(enum conn_event conn_disconn_evt,
> struct connect_info *conn_info,
> u8 mac_status,
> @@ -468,7 +466,7 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
> struct host_if_drv *wfi_drv = priv->hif_drv;
> u8 null_bssid[ETH_ALEN] = {0};
>
> - wilc_connecting = 0;
> + vif->connecting = 0;
>
> if (conn_disconn_evt == CONN_DISCONN_EVENT_CONN_RESP) {
> u16 connect_status;
> @@ -666,7 +664,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
> enum authtype auth_type = ANY;
> u32 cipher_group;
>
> - wilc_connecting = 1;
> + vif->connecting = 1;
>
> if (!(strncmp(sme->ssid, "DIRECT-", 7)))
> wfi_drv->p2p_connect = 1;
> @@ -698,7 +696,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
> nw_info = &priv->scanned_shadow[sel_bssi_idx];
> } else {
> ret = -ENOENT;
> - wilc_connecting = 0;
> + vif->connecting = 0;
> return ret;
> }
>
> @@ -741,7 +739,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
> ret = -ENOTSUPP;
> netdev_err(dev, "%s: Unsupported cipher\n",
> __func__);
> - wilc_connecting = 0;
> + vif->connecting = 0;
> return ret;
> }
> }
> @@ -792,7 +790,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
> if (ret != 0) {
> netdev_err(dev, "wilc_set_join_req(): Error\n");
> ret = -ENOENT;
> - wilc_connecting = 0;
> + vif->connecting = 0;
> return ret;
> }
>
> @@ -809,7 +807,7 @@ static int disconnect(struct wiphy *wiphy, struct net_device *dev,
> int ret;
> u8 null_bssid[ETH_ALEN] = {0};
>
> - wilc_connecting = 0;
> + vif->connecting = 0;
>
> if (!wilc)
> return -EIO;
> @@ -1747,7 +1745,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
>
> switch (type) {
> case NL80211_IFTYPE_STATION:
> - wilc_connecting = 0;
> + vif->connecting = 0;
> dev->ieee80211_ptr->iftype = type;
> priv->wdev->iftype = type;
> vif->monitor_flag = 0;
> @@ -1762,7 +1760,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
> break;
>
> case NL80211_IFTYPE_P2P_CLIENT:
> - wilc_connecting = 0;
> + vif->connecting = 0;
> dev->ieee80211_ptr->iftype = type;
> priv->wdev->iftype = type;
> vif->monitor_flag = 0;
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index 9d57adb..fd3e69e 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -151,6 +151,7 @@ struct wilc_vif {
> struct timer_list periodic_rssi;
> struct rf_info periodic_stat;
> struct tcp_ack_filter ack_filter;
> + int connecting;
> };
>
> struct wilc {
>
Moved 'aging_timer' to wilc_priv struct instead of having it as static
variable.
As 'aging_timer' is maintained for each interfaces so 'op_ifcs' check is
not required before the timer_setup() and del_timer_sync() call.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 25 ++++++++++++-----------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 4727a8a..d853508 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -85,7 +85,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
static struct network_info last_scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
static u32 last_scanned_cnt;
struct timer_list wilc_during_ip_timer;
-static struct timer_list aging_timer;
static u8 op_ifcs;
#define CHAN2G(_channel, _freq, _flags) { \
@@ -165,8 +164,6 @@ static void clear_shadow_scan(void)
if (op_ifcs != 0)
return;
- del_timer_sync(&aging_timer);
-
for (i = 0; i < last_scanned_cnt; i++) {
if (last_scanned_shadow[last_scanned_cnt].ies) {
kfree(last_scanned_shadow[i].ies);
@@ -245,8 +242,9 @@ static void update_scan_time(void)
last_scanned_shadow[i].time_scan = jiffies;
}
-static void remove_network_from_shadow(struct timer_list *unused)
+static void remove_network_from_shadow(struct timer_list *t)
{
+ struct wilc_priv *priv = from_timer(priv, t, aging_timer);
unsigned long now = jiffies;
int i, j;
@@ -266,7 +264,8 @@ static void remove_network_from_shadow(struct timer_list *unused)
}
if (last_scanned_cnt != 0)
- mod_timer(&aging_timer, jiffies + msecs_to_jiffies(AGING_TIME));
+ mod_timer(&priv->aging_timer,
+ jiffies + msecs_to_jiffies(AGING_TIME));
}
static void clear_during_ip(struct timer_list *unused)
@@ -274,13 +273,15 @@ static void clear_during_ip(struct timer_list *unused)
wilc_optaining_ip = false;
}
-static int is_network_in_shadow(struct network_info *nw_info, void *user_void)
+static int is_network_in_shadow(struct network_info *nw_info,
+ struct wilc_priv *priv)
{
int state = -1;
int i;
if (last_scanned_cnt == 0) {
- mod_timer(&aging_timer, jiffies + msecs_to_jiffies(AGING_TIME));
+ mod_timer(&priv->aging_timer,
+ jiffies + msecs_to_jiffies(AGING_TIME));
state = -1;
} else {
for (i = 0; i < last_scanned_cnt; i++) {
@@ -295,9 +296,9 @@ static int is_network_in_shadow(struct network_info *nw_info, void *user_void)
}
static void add_network_to_shadow(struct network_info *nw_info,
- void *user_void, void *join_params)
+ struct wilc_priv *priv, void *join_params)
{
- int ap_found = is_network_in_shadow(nw_info, user_void);
+ int ap_found = is_network_in_shadow(nw_info, priv);
u32 ap_index = 0;
u8 rssi_index = 0;
struct network_info *shadow_nw_info;
@@ -2166,10 +2167,9 @@ int wilc_init_host_int(struct net_device *net)
int ret;
struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
- if (op_ifcs == 0) {
- timer_setup(&aging_timer, remove_network_from_shadow, 0);
+ timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
+ if (op_ifcs == 0)
timer_setup(&wilc_during_ip_timer, clear_during_ip, 0);
- }
op_ifcs++;
priv->p2p_listen_state = false;
@@ -2195,6 +2195,7 @@ int wilc_deinit_host_int(struct net_device *net)
mutex_destroy(&priv->scan_req_lock);
ret = wilc_deinit(vif);
+ del_timer_sync(&priv->aging_timer);
clear_shadow_scan();
if (op_ifcs == 0)
del_timer_sync(&wilc_during_ip_timer);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8b74d61..a76b68c 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -94,7 +94,7 @@ struct wilc_priv {
/* mutexes */
struct mutex scan_req_lock;
bool p2p_listen_state;
-
+ struct timer_list aging_timer;
};
struct frame_reg {
--
2.7.4
Cleanup patch to avoid line over 80 chars checkpatch issue introduced in
previous code refactor commit.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 52402c3..041c9dd 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -199,19 +199,20 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
unsigned long flags;
spin_lock_irqsave(&wilc->txq_spinlock, flags);
- for (i = f->pending_base; i < (f->pending_base + f->pending_acks); i++) {
- u32 session_index;
+ i = f->pending_base;
+ for (; i < (f->pending_base + f->pending_acks); i++) {
+ u32 index;
u32 bigger_ack_num;
if (i >= MAX_PENDING_ACKS)
break;
- session_index = f->pending_acks_info[i].session_index;
+ index = f->pending_acks_info[i].session_index;
- if (session_index >= 2 * MAX_TCP_SESSION)
+ if (index >= 2 * MAX_TCP_SESSION)
break;
- bigger_ack_num = f->ack_session_info[session_index].bigger_ack_num;
+ bigger_ack_num = f->ack_session_info[index].bigger_ack_num;
if (f->pending_acks_info[i].ack_num < bigger_ack_num) {
struct txq_entry_t *tqe;
--
2.7.4
On Thu, Aug 23, 2018 at 04:57:48PM +0530, Ajay Singh wrote:
> Hi Greg,
>
> On Thu, 23 Aug 2018 12:55:27 +0200
> Greg KH <[email protected]> wrote:
>
> > On Tue, Aug 14, 2018 at 12:20:15PM +0530, Ajay Singh wrote:
> > > --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > > +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > > @@ -151,6 +151,7 @@ struct wilc_vif {
> > > struct timer_list periodic_rssi;
> > > struct rf_info periodic_stat;
> > > struct tcp_ack_filter ack_filter;
> > > + int connecting;
> >
> > Shouldn't this be a boolean?
> >
>
> Yes, 'connecting' only have value as 0 or 1. I will change it to
> bool and rename it to 'is_connecting'.
I think just the name "connecting" implies bool so there is no need for
the "is_".
regards,
dan carpenter
Hi Claudiu,
On Thu, 23 Aug 2018 11:09:29 +0300
Claudiu Beznea <[email protected]> wrote:
> Hi Ajay,
>
> Few comments on this series. See per patch replies.
>
Thank you for your time to review.
I will check and update my replies on the patches.
Regards,
Ajay
On 23.08.2018 13:09, Ajay Singh wrote:
> On Thu, 23 Aug 2018 11:11:09 +0300
> Claudiu Beznea <[email protected]> wrote:
>
>> On 14.08.2018 09:50, Ajay Singh wrote:
>>> Avoid use of static variable and move it in 'wilc' structure
>>> related to hif and added NULL before accessing hif_workqueue in
>>> wilc_enqueue_work().
>>>
>>> Below variables are moved to 'wilc' struct:
>>> struct workqueue_struct *hif_workqueue;
>>> struct mutex hif_deinit_lock;
>>> struct completion hif_driver_comp;
>>>
>>> Signed-off-by: Ajay Singh <[email protected]>
>>> ---
>>> drivers/staging/wilc1000/host_interface.c | 43
>>> ++++++++++++++-------------
>>> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 4 +++ 2 files
>>> changed, 26 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/host_interface.c
>>> b/drivers/staging/wilc1000/host_interface.c index 642c314..d5d81843
>>> 100644 --- a/drivers/staging/wilc1000/host_interface.c
>>> +++ b/drivers/staging/wilc1000/host_interface.c
>>> @@ -187,9 +187,6 @@ struct join_bss_param {
[...]
>>> void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer,
>>> u32 length) diff --git
>>> a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
>>> b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
>>> ee8eda7..eb00e42 100644 ---
>>> a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
>>> b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -173,6 +173,10
>>> @@ struct wilc { struct rf_info dummy_statistics; bool enable_ps;
>>> int clients_count;
>>> + struct workqueue_struct *hif_workqueue;
>>> + /* deinitialization lock */
>>> + struct mutex hif_deinit_lock;
>>> + struct completion hif_driver_comp;
>>
>> I'm thinking now... wouldn't fit these better to struct host_if_drv?
>
> No, In my opinion it should be part of 'wilc' struct as only the single
> instance of there variables is required. 'host_if_drv' is maintained
> separately for each interface.
>
Ok, agree on this.
>>
>>> };
>>>
>>> struct wilc_wfi_mon_priv {
>>>
>
>
Avoid use of static variable and move 'rcv_assoc_resp' as part of
'hif_drv' struct. Rename from 'rcv_assoc_resp' to 'assoc_resp'.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 8 +++-----
drivers/staging/wilc1000/host_interface.h | 1 +
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 98098fb..4c148bc 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,8 +187,6 @@ struct join_bss_param {
static u8 p2p_listen_state;
-static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
-
static u8 set_ip[2][4];
static u8 get_ip[2][4];
@@ -1498,16 +1496,16 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
if (mac_status == MAC_STATUS_CONNECTED) {
u32 assoc_resp_info_len;
- memset(rcv_assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
+ memset(hif_drv->assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
- host_int_get_assoc_res_info(vif, rcv_assoc_resp,
+ host_int_get_assoc_res_info(vif, hif_drv->assoc_resp,
MAX_ASSOC_RESP_FRAME_SIZE,
&assoc_resp_info_len);
if (assoc_resp_info_len != 0) {
s32 err = 0;
- err = wilc_parse_assoc_resp_info(rcv_assoc_resp,
+ err = wilc_parse_assoc_resp_info(hif_drv->assoc_resp,
assoc_resp_info_len,
&conn_info);
if (err)
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 4a84dd2..80cb298 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -284,6 +284,7 @@ struct host_if_drv {
bool ifc_up;
int driver_handler_id;
+ u8 assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
};
struct add_sta_param {
--
2.7.4
On 14.08.2018 09:49, Ajay Singh wrote:
> Move global variable 'wilc_during_ip_timer' and 'wilc_optaining_ip' to
> 'wilc_vif' structure.
>
> Rename these variables like below
>
> wilc_during_ip_timer -> during_ip_timer
> wilc_optaining_ip -> obtaining_ip.
Is there any reason you choose to have these in struct wilc_vif and not in
struct wilc_priv as you did for aging timer?
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 15 +++++++-------
> drivers/staging/wilc1000/host_interface.h | 2 --
> drivers/staging/wilc1000/linux_wlan.c | 6 +++---
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 24 +++++++++++------------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 ++
> 5 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 42d8acc..11eb632 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -186,7 +186,6 @@ struct join_bss_param {
> };
>
> static struct host_if_drv *terminated_handle;
> -bool wilc_optaining_ip;
> static u8 p2p_listen_state;
> static struct workqueue_struct *hif_workqueue;
> static struct completion hif_driver_comp;
> @@ -791,7 +790,7 @@ static void handle_scan(struct work_struct *work)
> goto error;
> }
>
> - if (wilc_optaining_ip || wilc_connecting) {
> + if (vif->obtaining_ip || wilc_connecting) {
> netdev_err(vif->ndev, "Don't do obss scan\n");
> result = -EBUSY;
> goto error;
> @@ -1562,8 +1561,8 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
>
> hif_drv->hif_state = HOST_IF_CONNECTED;
>
> - wilc_optaining_ip = true;
> - mod_timer(&wilc_during_ip_timer,
> + vif->obtaining_ip = true;
> + mod_timer(&vif->during_ip_timer,
> jiffies + msecs_to_jiffies(10000));
> } else {
> hif_drv->hif_state = HOST_IF_IDLE;
> @@ -1595,7 +1594,7 @@ static inline void host_int_handle_disconnect(struct wilc_vif *vif)
> disconn_info.ie_len = 0;
>
> if (conn_result) {
> - wilc_optaining_ip = false;
> + vif->obtaining_ip = false;
> wilc_set_power_mgmt(vif, 0, 0);
>
> conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF, NULL, 0,
> @@ -1942,7 +1941,7 @@ static void handle_disconnect(struct work_struct *work)
> wid.val = (s8 *)&dummy_reason_code;
> wid.size = sizeof(char);
>
> - wilc_optaining_ip = false;
> + vif->obtaining_ip = false;
> wilc_set_power_mgmt(vif, 0, 0);
>
> eth_zero_addr(wilc_connected_ssid);
> @@ -2397,7 +2396,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
> goto error;
> }
>
> - if (wilc_optaining_ip || wilc_connecting) {
> + if (vif->obtaining_ip || wilc_connecting) {
> result = -EBUSY;
> goto error;
> }
> @@ -3455,7 +3454,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
> break;
> }
>
> - wilc_optaining_ip = false;
> + vif->obtaining_ip = false;
>
> if (clients_count == 0) {
> init_completion(&hif_driver_comp);
> diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
> index 84866a6..d026f44 100644
> --- a/drivers/staging/wilc1000/host_interface.h
> +++ b/drivers/staging/wilc1000/host_interface.h
> @@ -361,11 +361,9 @@ 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);
>
> -extern bool wilc_optaining_ip;
> extern u8 wilc_connected_ssid[6];
> extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
>
> extern int wilc_connecting;
> -extern struct timer_list wilc_during_ip_timer;
>
> #endif
> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
> index 57e3176..283bb74 100644
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -48,8 +48,8 @@ static int dev_state_ev_handler(struct notifier_block *this,
> case NETDEV_UP:
> if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
> hif_drv->ifc_up = 1;
> - wilc_optaining_ip = false;
> - del_timer(&wilc_during_ip_timer);
> + vif->obtaining_ip = false;
> + del_timer(&vif->during_ip_timer);
> }
>
> if (vif->wilc->enable_ps)
> @@ -68,7 +68,7 @@ static int dev_state_ev_handler(struct notifier_block *this,
> case NETDEV_DOWN:
> if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
> hif_drv->ifc_up = 0;
> - wilc_optaining_ip = false;
> + vif->obtaining_ip = false;
> }
>
> if (memcmp(dev_iface->ifa_label, wlan_dev_name, 5) == 0)
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index 1eac244..1690890 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -82,7 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
> .flags = WIPHY_WOWLAN_ANY
> };
>
> -struct timer_list wilc_during_ip_timer;
> static u8 op_ifcs;
>
> #define CHAN2G(_channel, _freq, _flags) { \
> @@ -261,9 +260,11 @@ static void remove_network_from_shadow(struct timer_list *t)
> jiffies + msecs_to_jiffies(AGING_TIME));
> }
>
> -static void clear_during_ip(struct timer_list *unused)
> +static void clear_during_ip(struct timer_list *t)
> {
> - wilc_optaining_ip = false;
> + struct wilc_vif *vif = from_timer(vif, t, during_ip_timer);
> +
> + vif->obtaining_ip = false;
> }
>
> static int is_network_in_shadow(struct network_info *nw_info,
> @@ -518,7 +519,7 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
> conn_info->resp_ies_len, connect_status,
> GFP_KERNEL);
> } else if (conn_disconn_evt == CONN_DISCONN_EVENT_DISCONN_NOTIF) {
> - wilc_optaining_ip = false;
> + vif->obtaining_ip = false;
> p2p_local_random = 0x01;
> p2p_recv_random = 0x00;
> wilc_ie = false;
> @@ -1743,8 +1744,8 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
> p2p_local_random = 0x01;
> p2p_recv_random = 0x00;
> wilc_ie = false;
> - wilc_optaining_ip = false;
> - del_timer(&wilc_during_ip_timer);
> + vif->obtaining_ip = false;
> + del_timer(&vif->during_ip_timer);
>
> switch (type) {
> case NL80211_IFTYPE_STATION:
> @@ -1789,8 +1790,8 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
> break;
>
> case NL80211_IFTYPE_P2P_GO:
> - wilc_optaining_ip = true;
> - mod_timer(&wilc_during_ip_timer,
> + vif->obtaining_ip = true;
> + mod_timer(&vif->during_ip_timer,
> jiffies + msecs_to_jiffies(DURING_IP_TIME_OUT));
> wilc_set_operation_mode(vif, AP_MODE);
> dev->ieee80211_ptr->iftype = type;
> @@ -2159,10 +2160,10 @@ int wilc_init_host_int(struct net_device *net)
> {
> int ret;
> struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
> + struct wilc_vif *vif = netdev_priv(priv->dev);
>
> timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
> - if (op_ifcs == 0)
> - timer_setup(&wilc_during_ip_timer, clear_during_ip, 0);
> + timer_setup(&vif->during_ip_timer, clear_during_ip, 0);
> op_ifcs++;
>
> priv->p2p_listen_state = false;
> @@ -2190,8 +2191,7 @@ int wilc_deinit_host_int(struct net_device *net)
>
> del_timer_sync(&priv->aging_timer);
> clear_shadow_scan(priv);
> - if (op_ifcs == 0)
> - del_timer_sync(&wilc_during_ip_timer);
> + del_timer_sync(&vif->during_ip_timer);
>
> if (ret)
> netdev_err(net, "Error while deinitializing host interface\n");
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index 3767e31..8e56a28 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -118,6 +118,8 @@ struct wilc_vif {
> struct net_device *ndev;
> u8 mode;
> u8 ifc_id;
> + struct timer_list during_ip_timer;
> + bool obtaining_ip;
> };
>
> struct wilc {
>
On Thu, 23 Aug 2018 11:13:04 +0300
Claudiu Beznea <[email protected]> wrote:
>
>
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Remove the use of unnecessary static variable 'p2p_listen_state'.
> > Already 'p2p_listen_state' is present in 'wilc_priv' struct. So
> > making use of that variable as its getting set in channel ready and
> > remain on channel expired callback.
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index d8cc08b..cf7ead5
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -185,8 +185,6 @@ struct join_bss_param {
> > u8 start_time[4];
> > };
> >
> > -static u8 p2p_listen_state;
> > -
> > /* 'msg' should be free by the caller for syc */
> > static struct host_if_msg*
> > wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct
> > work_struct *), @@ -2351,7 +2349,6 @@ static int
> > handle_remain_on_chan(struct wilc_vif *vif, netdev_err(vif->ndev,
> > "Failed to set remain on channel\n");
> > error:
> > - p2p_listen_state = 1;
> > hif_drv->remain_on_ch_timer_vif = vif;
> > mod_timer(&hif_drv->remain_on_ch_timer,
> > jiffies +
> > msecs_to_jiffies(hif_remain_ch->duration)); @@ -2407,8 +2404,9 @@
> > static void handle_listen_state_expired(struct work_struct *work)
> > struct wid wid; int result;
> > struct host_if_drv *hif_drv = vif->hif_drv;
> > + struct wilc_priv *priv =
> > wdev_priv(vif->ndev->ieee80211_ptr);
> > - if (p2p_listen_state) {
> > + if (priv->p2p_listen_state) {
> > remain_on_chan_flag = false;
> > wid.id = WID_REMAIN_ON_CHAN;
> > wid.type = WID_STR;
> > @@ -2433,7 +2431,6 @@ static void
> > handle_listen_state_expired(struct work_struct *work)
> > hif_drv->remain_on_ch.expired(hif_drv->remain_on_ch.arg,
> > hif_remain_ch->id); }
> > - p2p_listen_state = 0;
>
> Is this useless at all?
Instead of using the static variable now making use of
'p2p_listen_state' variable already maintained in priv data.
'p2p_listen_state' variable is already getting set in the
wilc_wfi_remain_on_channel_ready() and
wilc_wfi_remain_on_channel_expired(). So I think setting it explicitly
not required as the callbacks will take care to set the flag.
>
> > } else {
> > netdev_dbg(vif->ndev, "Not in listen state\n");
> > }
> >
On Thu, 23 Aug 2018 11:10:48 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Instead of using 'wilc_multicast_mac_addr_list' as global variable
> > move it part of wilc_vif struct. Rename
> > 'wilc_multicast_mac_addr_list' variable to 'mc_mac_addr_list' as
> > its now part of 'wilc_vif' struct.
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 4 +---
> > drivers/staging/wilc1000/host_interface.h | 1 -
> > drivers/staging/wilc1000/linux_wlan.c | 14 +++++++-------
> > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> > 4 files changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index d930f06..642c314
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -193,8 +193,6 @@ static struct mutex hif_deinit_lock;
> > static struct timer_list periodic_rssi;
> > static struct wilc_vif *periodic_rssi_vif;
> >
> > -u8
> > wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN]; -
> > static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
> >
> > static u8 set_ip[2][4];
> > @@ -2588,7 +2586,7 @@ static void handle_set_mcast_filter(struct
> > work_struct *work) *cur_byte++ = ((hif_set_mc->cnt >> 24) & 0xFF);
> >
> > if (hif_set_mc->cnt > 0)
> > - memcpy(cur_byte, wilc_multicast_mac_addr_list,
> > + memcpy(cur_byte, vif->mc_mac_addr_list,
>
> A locking mechanism should be used for vif->mc_mac_addr_list. It is
> read here.
Below is my understanding.
wilc_set_multicast_list() is called from 'ndo_set_rx_mode' callback.
wilc_set_multicast_list() calls wilc_setup_multicast_filter() which
only read the value from the array and don't write to it when below
if conditions pass.
if (dev->flags &
IFF_ALLMULTI || dev->mc.count > WILC_MULTICAST_TABLE_SIZE)
if (dev->mc.count == 0)
For the scenario when above 'if' conditions fails then value will be
write to array first followed by read operation which will happen
in different context(worker thread).
Unless ndo_set_rx_mode() gets called quickly I don't think there is any
issue here.
If required instead of adding the lock how about making
handle_set_mcast_filter() a sync call in further patches, so that it can
complete handle_set_mcast_filter() operation before coming out of
ndo_set_rx_mode() callback.
Actually, I am also worried if its right to add 'lock' at this
point without reproducing the issue and only based on the code
observation.
Regards,
Ajay
>
> > ((hif_set_mc->cnt) * ETH_ALEN));
> >
> > result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
> > diff --git a/drivers/staging/wilc1000/host_interface.h
> > b/drivers/staging/wilc1000/host_interface.h index d026f44..4a84dd2
> > 100644 --- a/drivers/staging/wilc1000/host_interface.h
> > +++ b/drivers/staging/wilc1000/host_interface.h
> > @@ -362,7 +362,6 @@ int wilc_set_tx_power(struct wilc_vif *vif, u8
> > tx_power); int wilc_get_tx_power(struct wilc_vif *vif, u8
> > *tx_power);
> > extern u8 wilc_connected_ssid[6];
> > -extern u8
> > wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> > extern int wilc_connecting;
> >
> > diff --git a/drivers/staging/wilc1000/linux_wlan.c
> > b/drivers/staging/wilc1000/linux_wlan.c index 283bb74..bbaa653
> > 100644 --- a/drivers/staging/wilc1000/linux_wlan.c
> > +++ b/drivers/staging/wilc1000/linux_wlan.c
> > @@ -822,14 +822,14 @@ static void wilc_set_multicast_list(struct
> > net_device *dev) }
> >
> > netdev_for_each_mc_addr(ha, dev) {
> > - memcpy(wilc_multicast_mac_addr_list[i], ha->addr,
> > ETH_ALEN);
> > + memcpy(vif->mc_mac_addr_list[i], ha->addr,
> > ETH_ALEN);
>
> and set here. The contexts are different. If not in this patch, then
> in a future one.
>
> > netdev_dbg(dev, "Entry[%d]: %x:%x:%x:%x:%x:%x\n",
> > i,
> > - wilc_multicast_mac_addr_list[i][0],
> > - wilc_multicast_mac_addr_list[i][1],
> > - wilc_multicast_mac_addr_list[i][2],
> > - wilc_multicast_mac_addr_list[i][3],
> > - wilc_multicast_mac_addr_list[i][4],
> > - wilc_multicast_mac_addr_list[i][5]);
> > + vif->mc_mac_addr_list[i][0],
> > + vif->mc_mac_addr_list[i][1],
> > + vif->mc_mac_addr_list[i][2],
> > + vif->mc_mac_addr_list[i][3],
> > + vif->mc_mac_addr_list[i][4],
> > + vif->mc_mac_addr_list[i][5]);
> > i++;
> > }
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
> > 8cccbbc..ee8eda7 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -120,6 +120,7 @@
> > struct wilc_vif { u8 ifc_id;
> > struct timer_list during_ip_timer;
> > bool obtaining_ip;
> > + u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> > };
> >
> > struct wilc {
> >
On Tue, Aug 14, 2018 at 12:19:52PM +0530, Ajay Singh wrote:
> This patch set mainly contains changes to avoid the use of static
> and global variables. Also contains few patch to avoid the checkpatch
> warning arise due to code refactor.
I'm dropping this whole series from my review queue as I have no idea
what is going on with it anymore :)
Please fix up and resend.
thanks,
greg k-h
On 23.08.2018 13:00, Ajay Singh wrote:
> Unless ndo_set_rx_mode() gets called quickly I don't think there is any
> issue here.
I don't agree with this.
Remove the use of unnecessary static variable 'p2p_listen_state'.
Already 'p2p_listen_state' is present in 'wilc_priv' struct. So making
use of that variable as its getting set in channel ready and
remain on channel expired callback.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index d8cc08b..cf7ead5 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -185,8 +185,6 @@ struct join_bss_param {
u8 start_time[4];
};
-static u8 p2p_listen_state;
-
/* 'msg' should be free by the caller for syc */
static struct host_if_msg*
wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
@@ -2351,7 +2349,6 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
netdev_err(vif->ndev, "Failed to set remain on channel\n");
error:
- p2p_listen_state = 1;
hif_drv->remain_on_ch_timer_vif = vif;
mod_timer(&hif_drv->remain_on_ch_timer,
jiffies + msecs_to_jiffies(hif_remain_ch->duration));
@@ -2407,8 +2404,9 @@ static void handle_listen_state_expired(struct work_struct *work)
struct wid wid;
int result;
struct host_if_drv *hif_drv = vif->hif_drv;
+ struct wilc_priv *priv = wdev_priv(vif->ndev->ieee80211_ptr);
- if (p2p_listen_state) {
+ if (priv->p2p_listen_state) {
remain_on_chan_flag = false;
wid.id = WID_REMAIN_ON_CHAN;
wid.type = WID_STR;
@@ -2433,7 +2431,6 @@ static void handle_listen_state_expired(struct work_struct *work)
hif_drv->remain_on_ch.expired(hif_drv->remain_on_ch.arg,
hif_remain_ch->id);
}
- p2p_listen_state = 0;
} else {
netdev_dbg(vif->ndev, "Not in listen state\n");
}
--
2.7.4
On Thu, 23 Aug 2018 11:09:42 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:49, Ajay Singh wrote:
> > Move global variable 'wilc_during_ip_timer' and 'wilc_optaining_ip'
> > to 'wilc_vif' structure.
> >
> > Rename these variables like below
> >
> > wilc_during_ip_timer -> during_ip_timer
> > wilc_optaining_ip -> obtaining_ip.
>
> Is there any reason you choose to have these in struct wilc_vif and
> not in struct wilc_priv as you did for aging timer?
The idea was to keep private data related to 'wiphy priv'
in 'wilc_priv' struct and 'netdev priv' related data in 'wilc_vif'
struct.
I tried to follow this approach while preparing this patches series to
avoid use of these static variables mostly.
If you look at aging timer its related to deletion of wifi scan
results from the shadow list. So I moved it as part of wilc_priv. And
variable related to ip were more specific to netdev.
Actually for each interface a separate copy is maintained for wiphy_priv
and netdev_priv struct.
Anyways, if you have any suggestion let me know I try to include them
in future patches.
>
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 15
> > +++++++------- drivers/staging/wilc1000/host_interface.h |
> > 2 -- drivers/staging/wilc1000/linux_wlan.c | 6 +++---
> > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 24
> > +++++++++++------------
> > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 ++ 5 files
> > changed, 24 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 42d8acc..11eb632
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -186,7 +186,6 @@ struct join_bss_param {
> > };
> >
> > static struct host_if_drv *terminated_handle;
> > -bool wilc_optaining_ip;
> > static u8 p2p_listen_state;
> > static struct workqueue_struct *hif_workqueue;
> > static struct completion hif_driver_comp;
> > @@ -791,7 +790,7 @@ static void handle_scan(struct work_struct
> > *work) goto error;
> > }
> >
> > - if (wilc_optaining_ip || wilc_connecting) {
> > + if (vif->obtaining_ip || wilc_connecting) {
> > netdev_err(vif->ndev, "Don't do obss scan\n");
> > result = -EBUSY;
> > goto error;
> > @@ -1562,8 +1561,8 @@ static inline void
> > host_int_parse_assoc_resp_info(struct wilc_vif *vif,
> > hif_drv->hif_state = HOST_IF_CONNECTED;
> >
> > - wilc_optaining_ip = true;
> > - mod_timer(&wilc_during_ip_timer,
> > + vif->obtaining_ip = true;
> > + mod_timer(&vif->during_ip_timer,
> > jiffies + msecs_to_jiffies(10000));
> > } else {
> > hif_drv->hif_state = HOST_IF_IDLE;
> > @@ -1595,7 +1594,7 @@ static inline void
> > host_int_handle_disconnect(struct wilc_vif *vif)
> > disconn_info.ie_len = 0;
> > if (conn_result) {
> > - wilc_optaining_ip = false;
> > + vif->obtaining_ip = false;
> > wilc_set_power_mgmt(vif, 0, 0);
> >
> > conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
> > NULL, 0, @@ -1942,7 +1941,7 @@ static void handle_disconnect(struct
> > work_struct *work) wid.val = (s8 *)&dummy_reason_code;
> > wid.size = sizeof(char);
> >
> > - wilc_optaining_ip = false;
> > + vif->obtaining_ip = false;
> > wilc_set_power_mgmt(vif, 0, 0);
> >
> > eth_zero_addr(wilc_connected_ssid);
> > @@ -2397,7 +2396,7 @@ static int handle_remain_on_chan(struct
> > wilc_vif *vif, goto error;
> > }
> >
> > - if (wilc_optaining_ip || wilc_connecting) {
> > + if (vif->obtaining_ip || wilc_connecting) {
> > result = -EBUSY;
> > goto error;
> > }
> > @@ -3455,7 +3454,7 @@ int wilc_init(struct net_device *dev, struct
> > host_if_drv **hif_drv_handler) break;
> > }
> >
> > - wilc_optaining_ip = false;
> > + vif->obtaining_ip = false;
> >
> > if (clients_count == 0) {
> > init_completion(&hif_driver_comp);
> > diff --git a/drivers/staging/wilc1000/host_interface.h
> > b/drivers/staging/wilc1000/host_interface.h index 84866a6..d026f44
> > 100644 --- a/drivers/staging/wilc1000/host_interface.h
> > +++ b/drivers/staging/wilc1000/host_interface.h
> > @@ -361,11 +361,9 @@ 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);
> >
> > -extern bool wilc_optaining_ip;
> > extern u8 wilc_connected_ssid[6];
> > extern u8
> > wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> > extern int wilc_connecting;
> > -extern struct timer_list wilc_during_ip_timer;
> >
> > #endif
> > diff --git a/drivers/staging/wilc1000/linux_wlan.c
> > b/drivers/staging/wilc1000/linux_wlan.c index 57e3176..283bb74
> > 100644 --- a/drivers/staging/wilc1000/linux_wlan.c
> > +++ b/drivers/staging/wilc1000/linux_wlan.c
> > @@ -48,8 +48,8 @@ static int dev_state_ev_handler(struct
> > notifier_block *this, case NETDEV_UP:
> > if (vif->iftype == STATION_MODE || vif->iftype ==
> > CLIENT_MODE) { hif_drv->ifc_up = 1;
> > - wilc_optaining_ip = false;
> > - del_timer(&wilc_during_ip_timer);
> > + vif->obtaining_ip = false;
> > + del_timer(&vif->during_ip_timer);
> > }
> >
> > if (vif->wilc->enable_ps)
> > @@ -68,7 +68,7 @@ static int dev_state_ev_handler(struct
> > notifier_block *this, case NETDEV_DOWN:
> > if (vif->iftype == STATION_MODE || vif->iftype ==
> > CLIENT_MODE) { hif_drv->ifc_up = 0;
> > - wilc_optaining_ip = false;
> > + vif->obtaining_ip = false;
> > }
> >
> > if (memcmp(dev_iface->ifa_label, wlan_dev_name, 5)
> > == 0) diff --git
> > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index
> > 1eac244..1690890 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++
> > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -82,7 +82,6
> > @@ static const struct wiphy_wowlan_support wowlan_support =
> > { .flags = WIPHY_WOWLAN_ANY };
> > -struct timer_list wilc_during_ip_timer;
> > static u8 op_ifcs;
> >
> > #define CHAN2G(_channel, _freq, _flags) { \
> > @@ -261,9 +260,11 @@ static void remove_network_from_shadow(struct
> > timer_list *t) jiffies + msecs_to_jiffies(AGING_TIME));
> > }
> >
> > -static void clear_during_ip(struct timer_list *unused)
> > +static void clear_during_ip(struct timer_list *t)
> > {
> > - wilc_optaining_ip = false;
> > + struct wilc_vif *vif = from_timer(vif, t, during_ip_timer);
> > +
> > + vif->obtaining_ip = false;
> > }
> >
> > static int is_network_in_shadow(struct network_info *nw_info,
> > @@ -518,7 +519,7 @@ static void cfg_connect_result(enum conn_event
> > conn_disconn_evt, conn_info->resp_ies_len, connect_status,
> > GFP_KERNEL);
> > } else if (conn_disconn_evt ==
> > CONN_DISCONN_EVENT_DISCONN_NOTIF) {
> > - wilc_optaining_ip = false;
> > + vif->obtaining_ip = false;
> > p2p_local_random = 0x01;
> > p2p_recv_random = 0x00;
> > wilc_ie = false;
> > @@ -1743,8 +1744,8 @@ static int change_virtual_intf(struct wiphy
> > *wiphy, struct net_device *dev, p2p_local_random = 0x01;
> > p2p_recv_random = 0x00;
> > wilc_ie = false;
> > - wilc_optaining_ip = false;
> > - del_timer(&wilc_during_ip_timer);
> > + vif->obtaining_ip = false;
> > + del_timer(&vif->during_ip_timer);
> >
> > switch (type) {
> > case NL80211_IFTYPE_STATION:
> > @@ -1789,8 +1790,8 @@ static int change_virtual_intf(struct wiphy
> > *wiphy, struct net_device *dev, break;
> >
> > case NL80211_IFTYPE_P2P_GO:
> > - wilc_optaining_ip = true;
> > - mod_timer(&wilc_during_ip_timer,
> > + vif->obtaining_ip = true;
> > + mod_timer(&vif->during_ip_timer,
> > jiffies +
> > msecs_to_jiffies(DURING_IP_TIME_OUT)); wilc_set_operation_mode(vif,
> > AP_MODE); dev->ieee80211_ptr->iftype = type;
> > @@ -2159,10 +2160,10 @@ int wilc_init_host_int(struct net_device
> > *net) {
> > int ret;
> > struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
> > + struct wilc_vif *vif = netdev_priv(priv->dev);
> >
> > timer_setup(&priv->aging_timer,
> > remove_network_from_shadow, 0);
> > - if (op_ifcs == 0)
> > - timer_setup(&wilc_during_ip_timer,
> > clear_during_ip, 0);
> > + timer_setup(&vif->during_ip_timer, clear_during_ip, 0);
> > op_ifcs++;
> >
> > priv->p2p_listen_state = false;
> > @@ -2190,8 +2191,7 @@ int wilc_deinit_host_int(struct net_device
> > *net)
> > del_timer_sync(&priv->aging_timer);
> > clear_shadow_scan(priv);
> > - if (op_ifcs == 0)
> > - del_timer_sync(&wilc_during_ip_timer);
> > + del_timer_sync(&vif->during_ip_timer);
> >
> > if (ret)
> > netdev_err(net, "Error while deinitializing host
> > interface\n"); diff --git
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
> > 3767e31..8e56a28 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -118,6 +118,8 @@
> > struct wilc_vif { struct net_device *ndev; u8 mode;
> > u8 ifc_id;
> > + struct timer_list during_ip_timer;
> > + bool obtaining_ip;
> > };
> >
> > struct wilc {
> >
Hi Greg,
On Mon, 27 Aug 2018 19:10:47 +0200
Greg KH <[email protected]> wrote:
> On Tue, Aug 14, 2018 at 12:19:52PM +0530, Ajay Singh wrote:
> > This patch set mainly contains changes to avoid the use of static
> > and global variables. Also contains few patch to avoid the
> > checkpatch warning arise due to code refactor.
>
> I'm dropping this whole series from my review queue as I have no idea
> what is going on with it anymore :)
Sorry for the trouble.
Please ignore this patch set. I am working on v2 for this patch series
to include the review comments.
I will send the updated patch set soon.
Regards,
Ajay
Cleanup patch to use lowercase name for get_BSSID() and HIL variable.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/coreconfigurator.c | 4 ++--
drivers/staging/wilc1000/wilc_wlan.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index e542067..d6d3a97 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -116,7 +116,7 @@ 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(u8 *data, u8 *bssid)
{
if (get_from_ds(data) == 1)
get_address2(data, bssid);
@@ -233,7 +233,7 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
network_info->tsf_hi = tsf_lo | ((u64)tsf_hi << 32);
get_ssid(msa, network_info->ssid, &network_info->ssid_len);
- get_BSSID(msa, network_info->bssid);
+ get_bssid(msa, network_info->bssid);
network_info->ch = get_current_channel_802_11n(msa, rx_len
+ FCS_LEN);
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index bf45b4c..d397c27 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -164,7 +164,7 @@ static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
struct wilc *wilc = vif->wilc;
const struct iphdr *ip_hdr_ptr;
const struct tcphdr *tcp_hdr_ptr;
- u32 IHL, total_length, data_offset;
+ u32 ihl, total_length, data_offset;
spin_lock_irqsave(&wilc->txq_spinlock, flags);
@@ -176,12 +176,12 @@ static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
if (ip_hdr_ptr->protocol != IPPROTO_TCP)
goto out;
- IHL = ip_hdr_ptr->ihl << 2;
- tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
+ ihl = ip_hdr_ptr->ihl << 2;
+ tcp_hdr_ptr = buffer + ETH_HLEN + ihl;
total_length = ntohs(ip_hdr_ptr->tot_len);
data_offset = tcp_hdr_ptr->doff << 2;
- if (total_length == (IHL + data_offset)) {
+ if (total_length == (ihl + data_offset)) {
u32 seq_no, ack_no;
seq_no = ntohl(tcp_hdr_ptr->seq);
--
2.7.4
Move static variable 'wilc_connecting' as part of 'wilc_vif' private
struct. Remove "wilc_" prefix from name as its already part of wilc_vif
struct.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 4 ++--
drivers/staging/wilc1000/host_interface.h | 2 --
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 ++++++++----------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
4 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index f37ba64..d8cc08b 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -720,7 +720,7 @@ static void handle_scan(struct work_struct *work)
goto error;
}
- if (vif->obtaining_ip || wilc_connecting) {
+ if (vif->obtaining_ip || vif->connecting) {
netdev_err(vif->ndev, "Don't do obss scan\n");
result = -EBUSY;
goto error;
@@ -2326,7 +2326,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
goto error;
}
- if (vif->obtaining_ip || wilc_connecting) {
+ if (vif->obtaining_ip || vif->connecting) {
result = -EBUSY;
goto error;
}
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 0f0d509..4048eab 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -361,6 +361,4 @@ int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
extern u8 wilc_connected_ssid[6];
-extern int wilc_connecting;
-
#endif
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 35a83d4..cc44640 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -453,8 +453,6 @@ static inline bool wilc_cfg_scan_time_expired(struct wilc_priv *priv, int i)
return false;
}
-int wilc_connecting;
-
static void cfg_connect_result(enum conn_event conn_disconn_evt,
struct connect_info *conn_info,
u8 mac_status,
@@ -468,7 +466,7 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
struct host_if_drv *wfi_drv = priv->hif_drv;
u8 null_bssid[ETH_ALEN] = {0};
- wilc_connecting = 0;
+ vif->connecting = 0;
if (conn_disconn_evt == CONN_DISCONN_EVENT_CONN_RESP) {
u16 connect_status;
@@ -666,7 +664,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
enum authtype auth_type = ANY;
u32 cipher_group;
- wilc_connecting = 1;
+ vif->connecting = 1;
if (!(strncmp(sme->ssid, "DIRECT-", 7)))
wfi_drv->p2p_connect = 1;
@@ -698,7 +696,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
nw_info = &priv->scanned_shadow[sel_bssi_idx];
} else {
ret = -ENOENT;
- wilc_connecting = 0;
+ vif->connecting = 0;
return ret;
}
@@ -741,7 +739,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
ret = -ENOTSUPP;
netdev_err(dev, "%s: Unsupported cipher\n",
__func__);
- wilc_connecting = 0;
+ vif->connecting = 0;
return ret;
}
}
@@ -792,7 +790,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
if (ret != 0) {
netdev_err(dev, "wilc_set_join_req(): Error\n");
ret = -ENOENT;
- wilc_connecting = 0;
+ vif->connecting = 0;
return ret;
}
@@ -809,7 +807,7 @@ static int disconnect(struct wiphy *wiphy, struct net_device *dev,
int ret;
u8 null_bssid[ETH_ALEN] = {0};
- wilc_connecting = 0;
+ vif->connecting = 0;
if (!wilc)
return -EIO;
@@ -1747,7 +1745,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
switch (type) {
case NL80211_IFTYPE_STATION:
- wilc_connecting = 0;
+ vif->connecting = 0;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->monitor_flag = 0;
@@ -1762,7 +1760,7 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
break;
case NL80211_IFTYPE_P2P_CLIENT:
- wilc_connecting = 0;
+ vif->connecting = 0;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->monitor_flag = 0;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 9d57adb..fd3e69e 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -151,6 +151,7 @@ struct wilc_vif {
struct timer_list periodic_rssi;
struct rf_info periodic_stat;
struct tcp_ack_filter ack_filter;
+ int connecting;
};
struct wilc {
--
2.7.4
On Mon, Aug 27, 2018 at 10:54:38AM +0530, Ajay Singh wrote:
> Hi Claudiu,
>
> On Fri, 24 Aug 2018 12:31:28 +0300
> Claudiu Beznea <[email protected]> wrote:
>
> > On 23.08.2018 13:33, Ajay Singh wrote:
> > > On Thu, 23 Aug 2018 11:12:08 +0300
> > > Claudiu Beznea <[email protected]> wrote:
> > >
> > >> On 14.08.2018 09:50, Ajay Singh wrote:
> > >>> Cleanup patch to avoid line over 80 chars issue reported by
> > >>> checkpatch.pl script.
> > >>>
> > >>> Signed-off-by: Ajay Singh <[email protected]>
> > >>> ---
> > >>> drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> > >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> > >>> b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9
> > >>> 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c
> > >>> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > >>> @@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct
> > >>> wilc_vif *vif, u32 ack, return 0;
> > >>> }
> > >>>
> > >>> +static inline void clear_tcp_session_txq(struct wilc_vif *vif,
> > >>> int index) +{
> > >>> + vif->ack_filter.pending_acks_info[index].txqe = NULL;
> > >>> +}
> > >>> +
> > >>
> > >> This seems useless to me...
> > >
> > > Sorry, this point is not fully clear to me.
> > >
> > > Did you mean setting of 'NULL' to 'pending_acks_info[index].txqe' is
> > > not required?
> > >
> >
> > No, having a new function that sets a variable just to avoid line
> > over 80 warning.
>
> Okay got it.
> How about using a temp variable to hold the value of
> 'tqe->tcp_pending_ack_idx'. It can also help to overcome the
> checkpatch warning.
Just ignore the checkpatch warning... Don't add a temporary variable
just to please checkpatch... It's good to fix checkpatch warnings if
it makes the code cleaner, but I hate when people do:
tmp = some_long_variable_name;
some_other_long_variable = tmp;
The tmp variable is only used one time and a simple statement becomes
two statements and it breaks grep.
You could consider renaming some of the variables. I think the "_info"
doesn't add anything, because obviously it's information. Maybe
"tcp_pending_ack_index" could just become "->ack_idx".
vif->ack_filter.pending_ack[tqe->ack_idx].txqe = NULL;
It's still very searchable. If you grep for "ack_idx" then the pending
and TCP are right there so no information is lost.
regards,
dan carpenter
Move global variable 'wilc_during_ip_timer' and 'wilc_optaining_ip' to
'wilc_vif' structure.
Rename these variables like below
wilc_during_ip_timer -> during_ip_timer
wilc_optaining_ip -> obtaining_ip.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 15 +++++++-------
drivers/staging/wilc1000/host_interface.h | 2 --
drivers/staging/wilc1000/linux_wlan.c | 6 +++---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 24 +++++++++++------------
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 ++
5 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 42d8acc..11eb632 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -186,7 +186,6 @@ struct join_bss_param {
};
static struct host_if_drv *terminated_handle;
-bool wilc_optaining_ip;
static u8 p2p_listen_state;
static struct workqueue_struct *hif_workqueue;
static struct completion hif_driver_comp;
@@ -791,7 +790,7 @@ static void handle_scan(struct work_struct *work)
goto error;
}
- if (wilc_optaining_ip || wilc_connecting) {
+ if (vif->obtaining_ip || wilc_connecting) {
netdev_err(vif->ndev, "Don't do obss scan\n");
result = -EBUSY;
goto error;
@@ -1562,8 +1561,8 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
hif_drv->hif_state = HOST_IF_CONNECTED;
- wilc_optaining_ip = true;
- mod_timer(&wilc_during_ip_timer,
+ vif->obtaining_ip = true;
+ mod_timer(&vif->during_ip_timer,
jiffies + msecs_to_jiffies(10000));
} else {
hif_drv->hif_state = HOST_IF_IDLE;
@@ -1595,7 +1594,7 @@ static inline void host_int_handle_disconnect(struct wilc_vif *vif)
disconn_info.ie_len = 0;
if (conn_result) {
- wilc_optaining_ip = false;
+ vif->obtaining_ip = false;
wilc_set_power_mgmt(vif, 0, 0);
conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF, NULL, 0,
@@ -1942,7 +1941,7 @@ static void handle_disconnect(struct work_struct *work)
wid.val = (s8 *)&dummy_reason_code;
wid.size = sizeof(char);
- wilc_optaining_ip = false;
+ vif->obtaining_ip = false;
wilc_set_power_mgmt(vif, 0, 0);
eth_zero_addr(wilc_connected_ssid);
@@ -2397,7 +2396,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
goto error;
}
- if (wilc_optaining_ip || wilc_connecting) {
+ if (vif->obtaining_ip || wilc_connecting) {
result = -EBUSY;
goto error;
}
@@ -3455,7 +3454,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
break;
}
- wilc_optaining_ip = false;
+ vif->obtaining_ip = false;
if (clients_count == 0) {
init_completion(&hif_driver_comp);
diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
index 84866a6..d026f44 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -361,11 +361,9 @@ 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);
-extern bool wilc_optaining_ip;
extern u8 wilc_connected_ssid[6];
extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
extern int wilc_connecting;
-extern struct timer_list wilc_during_ip_timer;
#endif
diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 57e3176..283bb74 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -48,8 +48,8 @@ static int dev_state_ev_handler(struct notifier_block *this,
case NETDEV_UP:
if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
hif_drv->ifc_up = 1;
- wilc_optaining_ip = false;
- del_timer(&wilc_during_ip_timer);
+ vif->obtaining_ip = false;
+ del_timer(&vif->during_ip_timer);
}
if (vif->wilc->enable_ps)
@@ -68,7 +68,7 @@ static int dev_state_ev_handler(struct notifier_block *this,
case NETDEV_DOWN:
if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
hif_drv->ifc_up = 0;
- wilc_optaining_ip = false;
+ vif->obtaining_ip = false;
}
if (memcmp(dev_iface->ifa_label, wlan_dev_name, 5) == 0)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 1eac244..1690890 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -82,7 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
.flags = WIPHY_WOWLAN_ANY
};
-struct timer_list wilc_during_ip_timer;
static u8 op_ifcs;
#define CHAN2G(_channel, _freq, _flags) { \
@@ -261,9 +260,11 @@ static void remove_network_from_shadow(struct timer_list *t)
jiffies + msecs_to_jiffies(AGING_TIME));
}
-static void clear_during_ip(struct timer_list *unused)
+static void clear_during_ip(struct timer_list *t)
{
- wilc_optaining_ip = false;
+ struct wilc_vif *vif = from_timer(vif, t, during_ip_timer);
+
+ vif->obtaining_ip = false;
}
static int is_network_in_shadow(struct network_info *nw_info,
@@ -518,7 +519,7 @@ static void cfg_connect_result(enum conn_event conn_disconn_evt,
conn_info->resp_ies_len, connect_status,
GFP_KERNEL);
} else if (conn_disconn_evt == CONN_DISCONN_EVENT_DISCONN_NOTIF) {
- wilc_optaining_ip = false;
+ vif->obtaining_ip = false;
p2p_local_random = 0x01;
p2p_recv_random = 0x00;
wilc_ie = false;
@@ -1743,8 +1744,8 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
p2p_local_random = 0x01;
p2p_recv_random = 0x00;
wilc_ie = false;
- wilc_optaining_ip = false;
- del_timer(&wilc_during_ip_timer);
+ vif->obtaining_ip = false;
+ del_timer(&vif->during_ip_timer);
switch (type) {
case NL80211_IFTYPE_STATION:
@@ -1789,8 +1790,8 @@ static int change_virtual_intf(struct wiphy *wiphy, struct net_device *dev,
break;
case NL80211_IFTYPE_P2P_GO:
- wilc_optaining_ip = true;
- mod_timer(&wilc_during_ip_timer,
+ vif->obtaining_ip = true;
+ mod_timer(&vif->during_ip_timer,
jiffies + msecs_to_jiffies(DURING_IP_TIME_OUT));
wilc_set_operation_mode(vif, AP_MODE);
dev->ieee80211_ptr->iftype = type;
@@ -2159,10 +2160,10 @@ int wilc_init_host_int(struct net_device *net)
{
int ret;
struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
+ struct wilc_vif *vif = netdev_priv(priv->dev);
timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
- if (op_ifcs == 0)
- timer_setup(&wilc_during_ip_timer, clear_during_ip, 0);
+ timer_setup(&vif->during_ip_timer, clear_during_ip, 0);
op_ifcs++;
priv->p2p_listen_state = false;
@@ -2190,8 +2191,7 @@ int wilc_deinit_host_int(struct net_device *net)
del_timer_sync(&priv->aging_timer);
clear_shadow_scan(priv);
- if (op_ifcs == 0)
- del_timer_sync(&wilc_during_ip_timer);
+ del_timer_sync(&vif->during_ip_timer);
if (ret)
netdev_err(net, "Error while deinitializing host interface\n");
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 3767e31..8e56a28 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -118,6 +118,8 @@ struct wilc_vif {
struct net_device *ndev;
u8 mode;
u8 ifc_id;
+ struct timer_list during_ip_timer;
+ bool obtaining_ip;
};
struct wilc {
--
2.7.4
On Thu, 23 Aug 2018 11:09:56 +0300
Claudiu Beznea <[email protected]> wrote:
>
>
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Avoid use of static variable 'clients_count' and move it part of 'wilc'
> > structure.
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 9 ++++-----
> > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> > index 6225e67..d930f06 100644
> > --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -199,7 +199,6 @@ static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
> >
> > static u8 set_ip[2][4];
> > static u8 get_ip[2][4];
> > -static u32 clients_count;
> >
> > static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
> >
> > @@ -3456,7 +3455,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
> >
> > vif->obtaining_ip = false;
> >
> > - if (clients_count == 0) {
> > + if (wilc->clients_count == 0) {
>
> The check of this should be mutex protected or a hif init/deinit should be
> done in probe/remove code not not in open (I'm for the second approach).
> Imagine this scenario:
>
> vif 1: is down
> vif 2: is down. "ifconfig vif2 up" is executed and the execution reach this
> point and then suspends after "if (wilc->clients_count == 0)" check
> vif 1: "ifconfig vif1 up" and execution reach the end of this function and
> then the execution for vif 2 resumes. This will init twice the hif
> mutex, completion etc.
>
> > init_completion(&hif_driver_comp);
> > mutex_init(&hif_deinit_lock);
> >
Instead of using mutex protection, it would be better to move this block to
wilc_netdev_init() where wilc structure is initialized
> > @@ -3490,7 +3489,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
> >
> > mutex_unlock(&hif_drv->cfg_values_lock);
> >
> > - clients_count++;
> > + wilc->clients_count++;
> >
> > return 0;
> > }
> > @@ -3526,7 +3525,7 @@ int wilc_deinit(struct wilc_vif *vif)
> >
> > hif_drv->hif_state = HOST_IF_IDLE;
> >
> > - if (clients_count == 1) {
> > + if (vif->wilc->clients_count == 1) {
> > struct host_if_msg *msg;
> >
> > msg = wilc_alloc_work(vif, handle_hif_exit_work, true);
> > @@ -3544,7 +3543,7 @@ int wilc_deinit(struct wilc_vif *vif)
> >
> > kfree(hif_drv);
> >
> > - clients_count--;
> > + vif->wilc->clients_count--;
> > terminated_handle = NULL;
> > mutex_unlock(&hif_deinit_lock);
> > return result;
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > index 8e56a28..8cccbbc 100644
> > --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > @@ -171,6 +171,7 @@ struct wilc {
> >
> > struct rf_info dummy_statistics;
> > bool enable_ps;
> > + int clients_count;
> > };
> >
> > struct wilc_wfi_mon_priv {
> >
Cleanup patch to use appropriate variable name to fetch the periodic
statistics.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 2 +-
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index a440f84..98098fb 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3425,7 +3425,7 @@ static void get_periodic_rssi(struct timer_list *t)
}
if (vif->hif_drv->hif_state == HOST_IF_CONNECTED)
- wilc_get_statistics(vif, &vif->dummy_statistics, false);
+ wilc_get_statistics(vif, &vif->periodic_stat, false);
mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index cdb90c3..c53d38f 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -123,7 +123,7 @@ struct wilc_vif {
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
bool is_termination_progress;
struct timer_list periodic_rssi;
- struct rf_info dummy_statistics;
+ struct rf_info periodic_stat;
};
struct wilc {
--
2.7.4
On 23.08.2018 13:33, Ajay Singh wrote:
> On Thu, 23 Aug 2018 11:12:08 +0300
> Claudiu Beznea <[email protected]> wrote:
>
>> On 14.08.2018 09:50, Ajay Singh wrote:
>>> Cleanup patch to avoid line over 80 chars issue reported by
>>> checkpatch.pl script.
>>>
>>> Signed-off-by: Ajay Singh <[email protected]>
>>> ---
>>> drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/wilc_wlan.c
>>> b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9 100644
>>> --- a/drivers/staging/wilc1000/wilc_wlan.c
>>> +++ b/drivers/staging/wilc1000/wilc_wlan.c
>>> @@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct
>>> wilc_vif *vif, u32 ack, return 0;
>>> }
>>>
>>> +static inline void clear_tcp_session_txq(struct wilc_vif *vif, int
>>> index) +{
>>> + vif->ack_filter.pending_acks_info[index].txqe = NULL;
>>> +}
>>> +
>>
>> This seems useless to me...
>
> Sorry, this point is not fully clear to me.
>
> Did you mean setting of 'NULL' to 'pending_acks_info[index].txqe' is
> not required?
>
No, having a new function that sets a variable just to avoid line over 80
warning.
>
>>
>>> static inline void tcp_process(struct net_device *dev, struct
>>> txq_entry_t *tqe) {
>>> void *buffer = tqe->buffer;
>>> @@ -670,7 +675,7 @@ int wilc_wlan_handle_txq(struct net_device
>>> *dev, u32 *txq_count) tqe->tx_complete_func(tqe->priv, tqe->status);
>>> if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
>>> tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
>>> -
>>> vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe =
>>> NULL;
>>> + clear_tcp_session_txq(vif,
>>> tqe->tcp_pending_ack_idx); kfree(tqe);
>>> } while (--entries);
>>>
>>>
>
>
On 14.08.2018 09:50, Ajay Singh wrote:
> Instead of using 'wilc_multicast_mac_addr_list' as global variable move
> it part of wilc_vif struct. Rename 'wilc_multicast_mac_addr_list'
> variable to 'mc_mac_addr_list' as its now part of 'wilc_vif' struct.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 4 +---
> drivers/staging/wilc1000/host_interface.h | 1 -
> drivers/staging/wilc1000/linux_wlan.c | 14 +++++++-------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> 4 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index d930f06..642c314 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -193,8 +193,6 @@ static struct mutex hif_deinit_lock;
> static struct timer_list periodic_rssi;
> static struct wilc_vif *periodic_rssi_vif;
>
> -u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> -
> static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
>
> static u8 set_ip[2][4];
> @@ -2588,7 +2586,7 @@ static void handle_set_mcast_filter(struct work_struct *work)
> *cur_byte++ = ((hif_set_mc->cnt >> 24) & 0xFF);
>
> if (hif_set_mc->cnt > 0)
> - memcpy(cur_byte, wilc_multicast_mac_addr_list,
> + memcpy(cur_byte, vif->mc_mac_addr_list,
A locking mechanism should be used for vif->mc_mac_addr_list. It is read here.
> ((hif_set_mc->cnt) * ETH_ALEN));
>
> result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
> diff --git a/drivers/staging/wilc1000/host_interface.h b/drivers/staging/wilc1000/host_interface.h
> index d026f44..4a84dd2 100644
> --- a/drivers/staging/wilc1000/host_interface.h
> +++ b/drivers/staging/wilc1000/host_interface.h
> @@ -362,7 +362,6 @@ int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power);
> int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
>
> extern u8 wilc_connected_ssid[6];
> -extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
>
> extern int wilc_connecting;
>
> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
> index 283bb74..bbaa653 100644
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -822,14 +822,14 @@ static void wilc_set_multicast_list(struct net_device *dev)
> }
>
> netdev_for_each_mc_addr(ha, dev) {
> - memcpy(wilc_multicast_mac_addr_list[i], ha->addr, ETH_ALEN);
> + memcpy(vif->mc_mac_addr_list[i], ha->addr, ETH_ALEN);
and set here. The contexts are different. If not in this patch, then in a
future one.
> netdev_dbg(dev, "Entry[%d]: %x:%x:%x:%x:%x:%x\n", i,
> - wilc_multicast_mac_addr_list[i][0],
> - wilc_multicast_mac_addr_list[i][1],
> - wilc_multicast_mac_addr_list[i][2],
> - wilc_multicast_mac_addr_list[i][3],
> - wilc_multicast_mac_addr_list[i][4],
> - wilc_multicast_mac_addr_list[i][5]);
> + vif->mc_mac_addr_list[i][0],
> + vif->mc_mac_addr_list[i][1],
> + vif->mc_mac_addr_list[i][2],
> + vif->mc_mac_addr_list[i][3],
> + vif->mc_mac_addr_list[i][4],
> + vif->mc_mac_addr_list[i][5]);
> i++;
> }
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index 8cccbbc..ee8eda7 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -120,6 +120,7 @@ struct wilc_vif {
> u8 ifc_id;
> struct timer_list during_ip_timer;
> bool obtaining_ip;
> + u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
> };
>
> struct wilc {
>
On Fri, 24 Aug 2018 11:47:14 +0300
Claudiu Beznea <[email protected]> wrote:
>
>
> On 23.08.2018 13:00, Ajay Singh wrote:
> > Unless ndo_set_rx_mode() gets called quickly I don't think there is any
> > issue here.
>
> I don't agree with this.
It would be safer that the mcast list be passed to wilc_setup_multicast_filter()
to be copied to the msg structure then handled by the worker thread.
In this case vif->mc_mac_addr_list can be removed all together.
Cleanup patch to remove the unnecessary NULL check before freeing up ies
information in clear_shadow_scan().
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index ede9134..00a167b 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -165,10 +165,8 @@ static void clear_shadow_scan(void)
return;
for (i = 0; i < last_scanned_cnt; i++) {
- if (last_scanned_shadow[i].ies) {
- kfree(last_scanned_shadow[i].ies);
- last_scanned_shadow[i].ies = NULL;
- }
+ kfree(last_scanned_shadow[i].ies);
+ last_scanned_shadow[i].ies = NULL;
kfree(last_scanned_shadow[i].join_params);
last_scanned_shadow[i].join_params = NULL;
--
2.7.4
Cleanup patch to avoid line over 80 chars issue reported by
checkpatch.pl script.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 041c9dd..f0743d9 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct wilc_vif *vif, u32 ack,
return 0;
}
+static inline void clear_tcp_session_txq(struct wilc_vif *vif, int index)
+{
+ vif->ack_filter.pending_acks_info[index].txqe = NULL;
+}
+
static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
{
void *buffer = tqe->buffer;
@@ -670,7 +675,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
tqe->tx_complete_func(tqe->priv, tqe->status);
if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
- vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
+ clear_tcp_session_txq(vif, tqe->tcp_pending_ack_idx);
kfree(tqe);
} while (--entries);
--
2.7.4
On 14.08.2018 09:50, Ajay Singh wrote:
> Avoid use of static variable 'clients_count' and move it part of 'wilc'
> structure.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 9 ++++-----
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 6225e67..d930f06 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -199,7 +199,6 @@ static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
>
> static u8 set_ip[2][4];
> static u8 get_ip[2][4];
> -static u32 clients_count;
>
> static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
>
> @@ -3456,7 +3455,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
>
> vif->obtaining_ip = false;
>
> - if (clients_count == 0) {
> + if (wilc->clients_count == 0) {
The check of this should be mutex protected or a hif init/deinit should be
done in probe/remove code not not in open (I'm for the second approach).
Imagine this scenario:
vif 1: is down
vif 2: is down. "ifconfig vif2 up" is executed and the execution reach this
point and then suspends after "if (wilc->clients_count == 0)" check
vif 1: "ifconfig vif1 up" and execution reach the end of this function and
then the execution for vif 2 resumes. This will init twice the hif
mutex, completion etc.
> init_completion(&hif_driver_comp);
> mutex_init(&hif_deinit_lock);
>
> @@ -3490,7 +3489,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
>
> mutex_unlock(&hif_drv->cfg_values_lock);
>
> - clients_count++;
> + wilc->clients_count++;
>
> return 0;
> }
> @@ -3526,7 +3525,7 @@ int wilc_deinit(struct wilc_vif *vif)
>
> hif_drv->hif_state = HOST_IF_IDLE;
>
> - if (clients_count == 1) {
> + if (vif->wilc->clients_count == 1) {
> struct host_if_msg *msg;
>
> msg = wilc_alloc_work(vif, handle_hif_exit_work, true);
> @@ -3544,7 +3543,7 @@ int wilc_deinit(struct wilc_vif *vif)
>
> kfree(hif_drv);
>
> - clients_count--;
> + vif->wilc->clients_count--;
> terminated_handle = NULL;
> mutex_unlock(&hif_deinit_lock);
> return result;
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index 8e56a28..8cccbbc 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -171,6 +171,7 @@ struct wilc {
>
> struct rf_info dummy_statistics;
> bool enable_ps;
> + int clients_count;
> };
>
> struct wilc_wfi_mon_priv {
>
On Thu, 23 Aug 2018 11:12:08 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Cleanup patch to avoid line over 80 chars issue reported by
> > checkpatch.pl script.
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> > b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9 100644
> > --- a/drivers/staging/wilc1000/wilc_wlan.c
> > +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > @@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct
> > wilc_vif *vif, u32 ack, return 0;
> > }
> >
> > +static inline void clear_tcp_session_txq(struct wilc_vif *vif, int
> > index) +{
> > + vif->ack_filter.pending_acks_info[index].txqe = NULL;
> > +}
> > +
>
> This seems useless to me...
Sorry, this point is not fully clear to me.
Did you mean setting of 'NULL' to 'pending_acks_info[index].txqe' is
not required?
>
> > static inline void tcp_process(struct net_device *dev, struct
> > txq_entry_t *tqe) {
> > void *buffer = tqe->buffer;
> > @@ -670,7 +675,7 @@ int wilc_wlan_handle_txq(struct net_device
> > *dev, u32 *txq_count) tqe->tx_complete_func(tqe->priv, tqe->status);
> > if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
> > tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
> > -
> > vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe =
> > NULL;
> > + clear_tcp_session_txq(vif,
> > tqe->tcp_pending_ack_idx); kfree(tqe);
> > } while (--entries);
> >
> >
On 14.08.2018 09:50, Ajay Singh wrote:
> Avoid use of static variable and move it in 'wilc' structure related to
> hif and added NULL before accessing hif_workqueue in wilc_enqueue_work().
>
> Below variables are moved to 'wilc' struct:
> struct workqueue_struct *hif_workqueue;
> struct mutex hif_deinit_lock;
> struct completion hif_driver_comp;
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 43 ++++++++++++++-------------
> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 4 +++
> 2 files changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 642c314..d5d81843 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -187,9 +187,6 @@ struct join_bss_param {
>
> static struct host_if_drv *terminated_handle;
> static u8 p2p_listen_state;
> -static struct workqueue_struct *hif_workqueue;
> -static struct completion hif_driver_comp;
> -static struct mutex hif_deinit_lock;
> static struct timer_list periodic_rssi;
> static struct wilc_vif *periodic_rssi_vif;
>
> @@ -225,7 +222,11 @@ wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
> static int wilc_enqueue_work(struct host_if_msg *msg)
> {
> INIT_WORK(&msg->work, msg->fn);
> - if (!hif_workqueue || !queue_work(hif_workqueue, &msg->work))
> +
> + if (!msg->vif || !msg->vif->wilc || !msg->vif->wilc->hif_workqueue)
> + return -EINVAL;
> +
> + if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
> return -EINVAL;
>
> return 0;
> @@ -316,7 +317,7 @@ static void handle_set_wfi_drv_handler(struct work_struct *work)
> if (ret)
> netdev_err(vif->ndev, "Failed to set driver handler\n");
>
> - complete(&hif_driver_comp);
> + complete(&vif->wilc->hif_driver_comp);
> kfree(buffer);
>
> free_msg:
> @@ -340,7 +341,7 @@ static void handle_set_operation_mode(struct work_struct *work)
> wilc_get_vif_idx(vif));
>
> if (hif_op_mode->mode == IDLE_MODE)
> - complete(&hif_driver_comp);
> + complete(&vif->wilc->hif_driver_comp);
>
> if (ret)
> netdev_err(vif->ndev, "Failed to set operation mode\n");
> @@ -3454,11 +3455,11 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
> vif->obtaining_ip = false;
>
> if (wilc->clients_count == 0) {
> - init_completion(&hif_driver_comp);
> - mutex_init(&hif_deinit_lock);
> + init_completion(&wilc->hif_driver_comp);
> + mutex_init(&wilc->hif_deinit_lock);
>
> - hif_workqueue = create_singlethread_workqueue("WILC_wq");
> - if (!hif_workqueue) {
> + wilc->hif_workqueue = create_singlethread_workqueue("WILC_wq");
> + if (!wilc->hif_workqueue) {
> netdev_err(vif->ndev, "Failed to create workqueue\n");
> kfree(hif_drv);
> return -ENOMEM;
> @@ -3502,7 +3503,7 @@ int wilc_deinit(struct wilc_vif *vif)
> return -EFAULT;
> }
>
> - mutex_lock(&hif_deinit_lock);
> + mutex_lock(&vif->wilc->hif_deinit_lock);
>
> terminated_handle = hif_drv;
>
> @@ -3512,7 +3513,7 @@ int wilc_deinit(struct wilc_vif *vif)
> del_timer_sync(&hif_drv->remain_on_ch_timer);
>
> wilc_set_wfi_drv_handler(vif, 0, 0, 0);
> - wait_for_completion(&hif_driver_comp);
> + wait_for_completion(&vif->wilc->hif_driver_comp);
>
> if (hif_drv->usr_scan_req.scan_result) {
> hif_drv->usr_scan_req.scan_result(SCAN_EVENT_ABORTED, NULL,
> @@ -3536,14 +3537,14 @@ int wilc_deinit(struct wilc_vif *vif)
> wait_for_completion(&msg->work_comp);
> kfree(msg);
> }
> - destroy_workqueue(hif_workqueue);
> + destroy_workqueue(vif->wilc->hif_workqueue);
> }
>
> kfree(hif_drv);
>
> vif->wilc->clients_count--;
> terminated_handle = NULL;
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&vif->wilc->hif_deinit_lock);
> return result;
> }
>
> @@ -3596,7 +3597,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
> struct host_if_drv *hif_drv;
> struct wilc_vif *vif;
>
> - mutex_lock(&hif_deinit_lock);
> + mutex_lock(&wilc->hif_deinit_lock);
>
> id = buffer[length - 4];
> id |= (buffer[length - 3] << 8);
> @@ -3604,26 +3605,26 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
> id |= (buffer[length - 1] << 24);
> vif = wilc_get_vif_from_idx(wilc, id);
> if (!vif) {
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
>
> hif_drv = vif->hif_drv;
>
> if (!hif_drv || hif_drv == terminated_handle) {
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
>
> if (!hif_drv->usr_conn_req.conn_result) {
> netdev_err(vif->ndev, "%s: conn_result is NULL\n", __func__);
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
>
> msg = wilc_alloc_work(vif, handle_rcvd_gnrl_async_info, false);
> if (IS_ERR(msg)) {
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
>
> @@ -3631,7 +3632,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
> msg->body.async_info.buffer = kmemdup(buffer, length, GFP_KERNEL);
> if (!msg->body.async_info.buffer) {
> kfree(msg);
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> return;
> }
>
> @@ -3642,7 +3643,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
> kfree(msg);
> }
>
> - mutex_unlock(&hif_deinit_lock);
> + mutex_unlock(&wilc->hif_deinit_lock);
> }
>
> void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer, u32 length)
> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> index ee8eda7..eb00e42 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> @@ -173,6 +173,10 @@ struct wilc {
> struct rf_info dummy_statistics;
> bool enable_ps;
> int clients_count;
> + struct workqueue_struct *hif_workqueue;
> + /* deinitialization lock */
> + struct mutex hif_deinit_lock;
> + struct completion hif_driver_comp;
I'm thinking now... wouldn't fit these better to struct host_if_drv?
> };
>
> struct wilc_wfi_mon_priv {
>
On 14.08.2018 09:50, Ajay Singh wrote:
> Remove the use of unnecessary static variable 'p2p_listen_state'.
> Already 'p2p_listen_state' is present in 'wilc_priv' struct. So making
> use of that variable as its getting set in channel ready and
> remain on channel expired callback.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index d8cc08b..cf7ead5 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -185,8 +185,6 @@ struct join_bss_param {
> u8 start_time[4];
> };
>
> -static u8 p2p_listen_state;
> -
> /* 'msg' should be free by the caller for syc */
> static struct host_if_msg*
> wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
> @@ -2351,7 +2349,6 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
> netdev_err(vif->ndev, "Failed to set remain on channel\n");
>
> error:
> - p2p_listen_state = 1;
> hif_drv->remain_on_ch_timer_vif = vif;
> mod_timer(&hif_drv->remain_on_ch_timer,
> jiffies + msecs_to_jiffies(hif_remain_ch->duration));
> @@ -2407,8 +2404,9 @@ static void handle_listen_state_expired(struct work_struct *work)
> struct wid wid;
> int result;
> struct host_if_drv *hif_drv = vif->hif_drv;
> + struct wilc_priv *priv = wdev_priv(vif->ndev->ieee80211_ptr);
>
> - if (p2p_listen_state) {
> + if (priv->p2p_listen_state) {
> remain_on_chan_flag = false;
> wid.id = WID_REMAIN_ON_CHAN;
> wid.type = WID_STR;
> @@ -2433,7 +2431,6 @@ static void handle_listen_state_expired(struct work_struct *work)
> hif_drv->remain_on_ch.expired(hif_drv->remain_on_ch.arg,
> hif_remain_ch->id);
> }
> - p2p_listen_state = 0;
Is this useless at all?
> } else {
> netdev_dbg(vif->ndev, "Not in listen state\n");
> }
>
Refactor tcp_process() to avoid unnecessary leading tabs in the
function.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wlan.c | 52 +++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 6787b6e..bf45b4c 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -162,42 +162,46 @@ static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
unsigned long flags;
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc = vif->wilc;
+ const struct iphdr *ip_hdr_ptr;
+ const struct tcphdr *tcp_hdr_ptr;
+ u32 IHL, total_length, data_offset;
spin_lock_irqsave(&wilc->txq_spinlock, flags);
- if (eth_hdr_ptr->h_proto == htons(ETH_P_IP)) {
- const struct iphdr *ip_hdr_ptr = buffer + ETH_HLEN;
+ if (eth_hdr_ptr->h_proto != htons(ETH_P_IP))
+ goto out;
- if (ip_hdr_ptr->protocol == IPPROTO_TCP) {
- const struct tcphdr *tcp_hdr_ptr;
- u32 IHL, total_length, data_offset;
+ ip_hdr_ptr = buffer + ETH_HLEN;
- IHL = ip_hdr_ptr->ihl << 2;
- tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
- total_length = ntohs(ip_hdr_ptr->tot_len);
+ if (ip_hdr_ptr->protocol != IPPROTO_TCP)
+ goto out;
- data_offset = tcp_hdr_ptr->doff << 2;
- if (total_length == (IHL + data_offset)) {
- u32 seq_no, ack_no;
+ IHL = ip_hdr_ptr->ihl << 2;
+ tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
+ total_length = ntohs(ip_hdr_ptr->tot_len);
- seq_no = ntohl(tcp_hdr_ptr->seq);
- ack_no = ntohl(tcp_hdr_ptr->ack_seq);
- for (i = 0; i < tcp_session; i++) {
- u32 j = ack_session_info[i].seq_num;
+ data_offset = tcp_hdr_ptr->doff << 2;
+ if (total_length == (IHL + data_offset)) {
+ u32 seq_no, ack_no;
- if (i < 2 * MAX_TCP_SESSION &&
- j == seq_no) {
- update_tcp_session(i, ack_no);
- break;
- }
- }
- if (i == tcp_session)
- add_tcp_session(0, 0, seq_no);
+ seq_no = ntohl(tcp_hdr_ptr->seq);
+ ack_no = ntohl(tcp_hdr_ptr->ack_seq);
+ for (i = 0; i < tcp_session; i++) {
+ u32 j = ack_session_info[i].seq_num;
- add_tcp_pending_ack(ack_no, i, tqe);
+ if (i < 2 * MAX_TCP_SESSION &&
+ j == seq_no) {
+ update_tcp_session(i, ack_no);
+ break;
}
}
+ if (i == tcp_session)
+ add_tcp_session(0, 0, seq_no);
+
+ add_tcp_pending_ack(ack_no, i, tqe);
}
+
+out:
spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
}
--
2.7.4
Hi Dan,
On Mon, 27 Aug 2018 15:00:50 +0300
Dan Carpenter <[email protected]> wrote:
> On Mon, Aug 27, 2018 at 10:54:38AM +0530, Ajay Singh wrote:
> > Hi Claudiu,
> >
> > On Fri, 24 Aug 2018 12:31:28 +0300
> > Claudiu Beznea <[email protected]> wrote:
> >
> > > On 23.08.2018 13:33, Ajay Singh wrote:
> > > > On Thu, 23 Aug 2018 11:12:08 +0300
> > > > Claudiu Beznea <[email protected]> wrote:
> > > >
> > > >> On 14.08.2018 09:50, Ajay Singh wrote:
> > > >>> Cleanup patch to avoid line over 80 chars issue reported by
> > > >>> checkpatch.pl script.
> > > >>>
> > > >>> Signed-off-by: Ajay Singh <[email protected]>
> > > >>> ---
> > > >>> drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> > > >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9
> > > >>> 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> > > >>> @@ -137,6 +137,11 @@ static inline int
> > > >>> add_tcp_pending_ack(struct wilc_vif *vif, u32 ack, return 0;
> > > >>> }
> > > >>>
> > > >>> +static inline void clear_tcp_session_txq(struct wilc_vif
> > > >>> *vif, int index) +{
> > > >>> + vif->ack_filter.pending_acks_info[index].txqe = NULL;
> > > >>> +}
> > > >>> +
> > > >>
> > > >> This seems useless to me...
> > > >
> > > > Sorry, this point is not fully clear to me.
> > > >
> > > > Did you mean setting of 'NULL' to
> > > > 'pending_acks_info[index].txqe' is not required?
> > > >
> > >
> > > No, having a new function that sets a variable just to avoid line
> > > over 80 warning.
> >
> > Okay got it.
> > How about using a temp variable to hold the value of
> > 'tqe->tcp_pending_ack_idx'. It can also help to overcome the
> > checkpatch warning.
>
> Just ignore the checkpatch warning... Don't add a temporary variable
> just to please checkpatch... It's good to fix checkpatch warnings if
> it makes the code cleaner, but I hate when people do:
>
> tmp = some_long_variable_name;
> some_other_long_variable = tmp;
>
> The tmp variable is only used one time and a simple statement becomes
> two statements and it breaks grep.
>
> You could consider renaming some of the variables. I think the
> "_info" doesn't add anything, because obviously it's information.
> Maybe "tcp_pending_ack_index" could just become "->ack_idx".
Thanks for providing the inputs.
I will include the changes by using shorter name for
'tcp_pending_ack_index' in next version.
Regards,
Ajay
After code refactor in previous commit now 'op_ifcs' is not require any
more, so remove it.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 1690890..3418d2d 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -82,8 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
.flags = WIPHY_WOWLAN_ANY
};
-static u8 op_ifcs;
-
#define CHAN2G(_channel, _freq, _flags) { \
.band = NL80211_BAND_2GHZ, \
.center_freq = (_freq), \
@@ -2164,7 +2162,6 @@ int wilc_init_host_int(struct net_device *net)
timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
timer_setup(&vif->during_ip_timer, clear_during_ip, 0);
- op_ifcs++;
priv->p2p_listen_state = false;
@@ -2184,8 +2181,6 @@ int wilc_deinit_host_int(struct net_device *net)
priv->p2p_listen_state = false;
- op_ifcs--;
-
mutex_destroy(&priv->scan_req_lock);
ret = wilc_deinit(vif);
--
2.7.4
Hi Claudiu,
On Fri, 24 Aug 2018 12:31:28 +0300
Claudiu Beznea <[email protected]> wrote:
> On 23.08.2018 13:33, Ajay Singh wrote:
> > On Thu, 23 Aug 2018 11:12:08 +0300
> > Claudiu Beznea <[email protected]> wrote:
> >
> >> On 14.08.2018 09:50, Ajay Singh wrote:
> >>> Cleanup patch to avoid line over 80 chars issue reported by
> >>> checkpatch.pl script.
> >>>
> >>> Signed-off-by: Ajay Singh <[email protected]>
> >>> ---
> >>> drivers/staging/wilc1000/wilc_wlan.c | 7 ++++++-
> >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/staging/wilc1000/wilc_wlan.c
> >>> b/drivers/staging/wilc1000/wilc_wlan.c index 041c9dd..f0743d9
> >>> 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c
> >>> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> >>> @@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct
> >>> wilc_vif *vif, u32 ack, return 0;
> >>> }
> >>>
> >>> +static inline void clear_tcp_session_txq(struct wilc_vif *vif,
> >>> int index) +{
> >>> + vif->ack_filter.pending_acks_info[index].txqe = NULL;
> >>> +}
> >>> +
> >>
> >> This seems useless to me...
> >
> > Sorry, this point is not fully clear to me.
> >
> > Did you mean setting of 'NULL' to 'pending_acks_info[index].txqe' is
> > not required?
> >
>
> No, having a new function that sets a variable just to avoid line
> over 80 warning.
Okay got it.
How about using a temp variable to hold the value of
'tqe->tcp_pending_ack_idx'. It can also help to overcome the
checkpatch warning.
Regards,
Ajay
On Thu, 23 Aug 2018 11:11:09 +0300
Claudiu Beznea <[email protected]> wrote:
> On 14.08.2018 09:50, Ajay Singh wrote:
> > Avoid use of static variable and move it in 'wilc' structure
> > related to hif and added NULL before accessing hif_workqueue in
> > wilc_enqueue_work().
> >
> > Below variables are moved to 'wilc' struct:
> > struct workqueue_struct *hif_workqueue;
> > struct mutex hif_deinit_lock;
> > struct completion hif_driver_comp;
> >
> > Signed-off-by: Ajay Singh <[email protected]>
> > ---
> > drivers/staging/wilc1000/host_interface.c | 43
> > ++++++++++++++-------------
> > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 4 +++ 2 files
> > changed, 26 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/host_interface.c
> > b/drivers/staging/wilc1000/host_interface.c index 642c314..d5d81843
> > 100644 --- a/drivers/staging/wilc1000/host_interface.c
> > +++ b/drivers/staging/wilc1000/host_interface.c
> > @@ -187,9 +187,6 @@ struct join_bss_param {
> >
> > static struct host_if_drv *terminated_handle;
> > static u8 p2p_listen_state;
> > -static struct workqueue_struct *hif_workqueue;
> > -static struct completion hif_driver_comp;
> > -static struct mutex hif_deinit_lock;
> > static struct timer_list periodic_rssi;
> > static struct wilc_vif *periodic_rssi_vif;
> >
> > @@ -225,7 +222,11 @@ wilc_alloc_work(struct wilc_vif *vif, void
> > (*work_fun)(struct work_struct *), static int
> > wilc_enqueue_work(struct host_if_msg *msg) {
> > INIT_WORK(&msg->work, msg->fn);
> > - if (!hif_workqueue || !queue_work(hif_workqueue,
> > &msg->work)) +
> > + if (!msg->vif || !msg->vif->wilc
> > || !msg->vif->wilc->hif_workqueue)
> > + return -EINVAL;
> > +
> > + if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
> > return -EINVAL;
> >
> > return 0;
> > @@ -316,7 +317,7 @@ static void handle_set_wfi_drv_handler(struct
> > work_struct *work) if (ret)
> > netdev_err(vif->ndev, "Failed to set driver
> > handler\n");
> > - complete(&hif_driver_comp);
> > + complete(&vif->wilc->hif_driver_comp);
> > kfree(buffer);
> >
> > free_msg:
> > @@ -340,7 +341,7 @@ static void handle_set_operation_mode(struct
> > work_struct *work) wilc_get_vif_idx(vif));
> >
> > if (hif_op_mode->mode == IDLE_MODE)
> > - complete(&hif_driver_comp);
> > + complete(&vif->wilc->hif_driver_comp);
> >
> > if (ret)
> > netdev_err(vif->ndev, "Failed to set operation
> > mode\n"); @@ -3454,11 +3455,11 @@ int wilc_init(struct net_device
> > *dev, struct host_if_drv **hif_drv_handler) vif->obtaining_ip =
> > false;
> > if (wilc->clients_count == 0) {
> > - init_completion(&hif_driver_comp);
> > - mutex_init(&hif_deinit_lock);
> > + init_completion(&wilc->hif_driver_comp);
> > + mutex_init(&wilc->hif_deinit_lock);
> >
> > - hif_workqueue =
> > create_singlethread_workqueue("WILC_wq");
> > - if (!hif_workqueue) {
> > + wilc->hif_workqueue =
> > create_singlethread_workqueue("WILC_wq");
> > + if (!wilc->hif_workqueue) {
> > netdev_err(vif->ndev, "Failed to create
> > workqueue\n"); kfree(hif_drv);
> > return -ENOMEM;
> > @@ -3502,7 +3503,7 @@ int wilc_deinit(struct wilc_vif *vif)
> > return -EFAULT;
> > }
> >
> > - mutex_lock(&hif_deinit_lock);
> > + mutex_lock(&vif->wilc->hif_deinit_lock);
> >
> > terminated_handle = hif_drv;
> >
> > @@ -3512,7 +3513,7 @@ int wilc_deinit(struct wilc_vif *vif)
> > del_timer_sync(&hif_drv->remain_on_ch_timer);
> >
> > wilc_set_wfi_drv_handler(vif, 0, 0, 0);
> > - wait_for_completion(&hif_driver_comp);
> > + wait_for_completion(&vif->wilc->hif_driver_comp);
> >
> > if (hif_drv->usr_scan_req.scan_result) {
> > hif_drv->usr_scan_req.scan_result(SCAN_EVENT_ABORTED,
> > NULL, @@ -3536,14 +3537,14 @@ int wilc_deinit(struct wilc_vif *vif)
> > wait_for_completion(&msg->work_comp);
> > kfree(msg);
> > }
> > - destroy_workqueue(hif_workqueue);
> > + destroy_workqueue(vif->wilc->hif_workqueue);
> > }
> >
> > kfree(hif_drv);
> >
> > vif->wilc->clients_count--;
> > terminated_handle = NULL;
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&vif->wilc->hif_deinit_lock);
> > return result;
> > }
> >
> > @@ -3596,7 +3597,7 @@ void wilc_gnrl_async_info_received(struct
> > wilc *wilc, u8 *buffer, u32 length) struct host_if_drv *hif_drv;
> > struct wilc_vif *vif;
> >
> > - mutex_lock(&hif_deinit_lock);
> > + mutex_lock(&wilc->hif_deinit_lock);
> >
> > id = buffer[length - 4];
> > id |= (buffer[length - 3] << 8);
> > @@ -3604,26 +3605,26 @@ void wilc_gnrl_async_info_received(struct
> > wilc *wilc, u8 *buffer, u32 length) id |= (buffer[length - 1] <<
> > 24); vif = wilc_get_vif_from_idx(wilc, id);
> > if (!vif) {
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> >
> > hif_drv = vif->hif_drv;
> >
> > if (!hif_drv || hif_drv == terminated_handle) {
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> >
> > if (!hif_drv->usr_conn_req.conn_result) {
> > netdev_err(vif->ndev, "%s: conn_result is NULL\n",
> > __func__);
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> >
> > msg = wilc_alloc_work(vif, handle_rcvd_gnrl_async_info,
> > false); if (IS_ERR(msg)) {
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> >
> > @@ -3631,7 +3632,7 @@ void wilc_gnrl_async_info_received(struct
> > wilc *wilc, u8 *buffer, u32 length) msg->body.async_info.buffer =
> > kmemdup(buffer, length, GFP_KERNEL); if
> > (!msg->body.async_info.buffer) { kfree(msg);
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > return;
> > }
> >
> > @@ -3642,7 +3643,7 @@ void wilc_gnrl_async_info_received(struct
> > wilc *wilc, u8 *buffer, u32 length) kfree(msg);
> > }
> >
> > - mutex_unlock(&hif_deinit_lock);
> > + mutex_unlock(&wilc->hif_deinit_lock);
> > }
> >
> > void wilc_scan_complete_received(struct wilc *wilc, u8 *buffer,
> > u32 length) diff --git
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
> > ee8eda7..eb00e42 100644 ---
> > a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
> > b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -173,6 +173,10
> > @@ struct wilc { struct rf_info dummy_statistics; bool enable_ps;
> > int clients_count;
> > + struct workqueue_struct *hif_workqueue;
> > + /* deinitialization lock */
> > + struct mutex hif_deinit_lock;
> > + struct completion hif_driver_comp;
>
> I'm thinking now... wouldn't fit these better to struct host_if_drv?
No, In my opinion it should be part of 'wilc' struct as only the single
instance of there variables is required. 'host_if_drv' is maintained
separately for each interface.
>
> > };
> >
> > struct wilc_wfi_mon_priv {
> >
On 14.08.2018 09:50, Ajay Singh wrote:
> Cleanup patch to avoid line over 80 chars checkpatch issue introduced in
> previous code refactor commit.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/wilc_wlan.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
> index 52402c3..041c9dd 100644
> --- a/drivers/staging/wilc1000/wilc_wlan.c
> +++ b/drivers/staging/wilc1000/wilc_wlan.c
> @@ -199,19 +199,20 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
> unsigned long flags;
>
> spin_lock_irqsave(&wilc->txq_spinlock, flags);
> - for (i = f->pending_base; i < (f->pending_base + f->pending_acks); i++) {
> - u32 session_index;
> + i = f->pending_base;
You could do it like this to avoid line over 80 chars checkpatch.pl warnings:
for (i = f->pending_base;
i < (f->pending_base + f->pending_acks); i++) {
> + for (; i < (f->pending_base + f->pending_acks); i++) {
> + u32 index;
> u32 bigger_ack_num;
>
> if (i >= MAX_PENDING_ACKS)
> break;
>
> - session_index = f->pending_acks_info[i].session_index;
> + index = f->pending_acks_info[i].session_index;
>
> - if (session_index >= 2 * MAX_TCP_SESSION)
> + if (index >= 2 * MAX_TCP_SESSION)
> break;
>
> - bigger_ack_num = f->ack_session_info[session_index].bigger_ack_num;
> + bigger_ack_num = f->ack_session_info[index].bigger_ack_num;
>
> if (f->pending_acks_info[i].ack_num < bigger_ack_num) {
> struct txq_entry_t *tqe;
>
On 23.08.2018 17:36, Ajay Singh wrote:
> On Thu, 23 Aug 2018 11:11:18 +0300
> Claudiu Beznea <[email protected]> wrote:
>
>> On 14.08.2018 09:50, Ajay Singh wrote:
>>> Remove the use of static variable 'terminated_handle' and instead
>>> move in wilc_vif struct.
>>> After moving this variable to wilc_vif struct its not required to
>>> keep 'terminated_handle', so changed it to boolean type.
>>
>> You can remove it at all and use wilc->hif_deinit_lock mutex also in
>> wilc_scan_complete_received() and wilc_network_info_received() it is
>> used in wilc_gnrl_async_info_received().
>
> In my understanding, 'terminated_handle' is used to know the
> status when interface is getting deinit(). During deinitialization
> of an interface if any async event received for the interface(from
> firmware) should be ignored.
'terminated_handle' true only inside mutex. So, outside of it it will be
false, so *mostly* it will tell you when mutex is locked for deinit.
*Mostly*, because context switches may happen while a mutex is locked.
With the current approach you have this code:
int wilc_deinit(struct wilc_vif *vif)
{
// ...
mutex_lock(&vif->wilc->hif_deinit_lock);
// (A)
vif->is_termination_progress = true;
// ...
vif->is_termination_progress = false;
mutex_unlokc(&vif->wilc->hif_deinit_lock);
}
And:
void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length)
{
// ...
if (!hif_drv || vif->is_termination_progress) {
netdev_err(vif->ndev, "driver not init[%p]\n", hif_drv);
return;
}
// ...
// (B)
result = wilc_enqueue_work(msg);
// ...
}
And:
static int wilc_enqueue_work(struct host_if_msg *msg)
{
INIT_WORK(&msg->work, msg->fn);
if (!msg->vif || !msg->vif->wilc || !msg->vif->wilc->hif_workqueue)
return -EINVAL;
// (C)
if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
return -EINVAL;
return 0;
}
You may have the following scenario:
1. context switch in wilc_deinit() just after the mutex is taken (point A
above). vif->is_termination_progress = false at this point.
2. a new packet is received and wilc_network_info_received() gets executed
and execution reaches point B, goes to wilc_enqueue_work() and suspend at
point C then context switch.
3. wilc_deinit() resumes and finish its execution.
4. wilc_enqueue_work() resumes and queue_work() is executed but you already
freed the hif_workqueue. You will have a crash here.
Notice that hif_drv is not set to NULL on wilc_deinit() after it is kfree().
>
> In my opinion its not right to only wait for the mutex in any of
> callback e.g wilc_scan_complete_received() because it will delay the
> handling of callback and try to process the event once lock is
> available for the interface which is already de-initialized.
But this is already done for wilc_gnrl_async_info_received().
>
> Based on my understand 'mutex' alone is not enough to
> handle this and we some extra check to know if interface is down.
terminated_handle will *mostly* tell you when the mutex is locked, nothing
more.
I will
> check more about this patch how to handle the extra scenario for this
> case.
> Please suggest if someone has better idea on how to handle this.
>
>>
>>>
>>> Signed-off-by: Ajay Singh <[email protected]>
>>> ---
>>> drivers/staging/wilc1000/host_interface.c | 11 +++++------
>>> drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/host_interface.c
>>> b/drivers/staging/wilc1000/host_interface.c index
>>> d5d81843..f71f451f 100644 ---
>>> a/drivers/staging/wilc1000/host_interface.c +++
>>> b/drivers/staging/wilc1000/host_interface.c @@ -185,7 +185,6 @@
>>> struct join_bss_param { u8 start_time[4];
>>> };
>>>
>>> -static struct host_if_drv *terminated_handle;
>>> static u8 p2p_listen_state;
>>> static struct timer_list periodic_rssi;
>>> static struct wilc_vif *periodic_rssi_vif;
>>> @@ -3505,7 +3504,7 @@ int wilc_deinit(struct wilc_vif *vif)
>>>
>>> mutex_lock(&vif->wilc->hif_deinit_lock);
>>>
>>> - terminated_handle = hif_drv;
>>> + vif->is_termination_progress = true;
>>>
>>> del_timer_sync(&hif_drv->scan_timer);
>>> del_timer_sync(&hif_drv->connect_timer);
>>> @@ -3543,7 +3542,7 @@ int wilc_deinit(struct wilc_vif *vif)
>>> kfree(hif_drv);
>>>
>>> vif->wilc->clients_count--;
>>> - terminated_handle = NULL;
>>> + vif->is_termination_progress = false;
>>> mutex_unlock(&vif->wilc->hif_deinit_lock);
>>> return result;
>>> }
>>> @@ -3565,7 +3564,7 @@ void wilc_network_info_received(struct wilc
>>> *wilc, u8 *buffer, u32 length) return;
>>> hif_drv = vif->hif_drv;
>>>
>>> - if (!hif_drv || hif_drv == terminated_handle) {
>>> + if (!hif_drv || vif->is_termination_progress) {
>>> netdev_err(vif->ndev, "driver not init[%p]\n",
>>> hif_drv); return;
>>> }
>>> @@ -3611,7 +3610,7 @@ void wilc_gnrl_async_info_received(struct
>>> wilc *wilc, u8 *buffer, u32 length)
>>> hif_drv = vif->hif_drv;
>>>
>>> - if (!hif_drv || hif_drv == terminated_handle) {
>>> + if (!hif_drv || vif->is_termination_progress) {
>>> mutex_unlock(&wilc->hif_deinit_lock);
>>> return;
>>> }
>>> @@ -3662,7 +3661,7 @@ void wilc_scan_complete_received(struct wilc
>>> *wilc, u8 *buffer, u32 length) return;
>>> hif_drv = vif->hif_drv;
>>>
>>> - if (!hif_drv || hif_drv == terminated_handle)
>>> + if (!hif_drv || vif->is_termination_progress)
>>> return;
>>>
>>> if (hif_drv->usr_scan_req.scan_result) {
>>> diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
>>> b/drivers/staging/wilc1000/wilc_wfi_netdevice.h index
>>> eb00e42..ba606d0 100644 ---
>>> a/drivers/staging/wilc1000/wilc_wfi_netdevice.h +++
>>> b/drivers/staging/wilc1000/wilc_wfi_netdevice.h @@ -121,6 +121,7 @@
>>> struct wilc_vif { struct timer_list during_ip_timer;
>>> bool obtaining_ip;
>>> u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
>>> + bool is_termination_progress;
>>> };
>>>
>>> struct wilc {
>>>
>
>
Hi Greg,
On Thu, 23 Aug 2018 12:55:27 +0200
Greg KH <[email protected]> wrote:
> On Tue, Aug 14, 2018 at 12:20:15PM +0530, Ajay Singh wrote:
> > --- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > +++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
> > @@ -151,6 +151,7 @@ struct wilc_vif {
> > struct timer_list periodic_rssi;
> > struct rf_info periodic_stat;
> > struct tcp_ack_filter ack_filter;
> > + int connecting;
>
> Shouldn't this be a boolean?
>
Yes, 'connecting' only have value as 0 or 1. I will change it to
bool and rename it to 'is_connecting'.
Regards,
Ajay
Avoid use of static variables and move them as part of wilc_vif struct.
Move all the parameters related to tcp_ack_filter algo to wilc_vif
struct.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 4 +-
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 +-
drivers/staging/wilc1000/wilc_wfi_netdevice.h | 27 ++++++
drivers/staging/wilc1000/wilc_wlan.c | 107 +++++++++-------------
drivers/staging/wilc1000/wilc_wlan.h | 2 +-
5 files changed, 77 insertions(+), 67 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 4c148bc..bffe0c8 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2068,9 +2068,9 @@ static void handle_get_statistics(struct work_struct *work)
if (stats->link_speed > TCP_ACK_FILTER_LINK_SPEED_THRESH &&
stats->link_speed != DEFAULT_LINK_SPEED)
- wilc_enable_tcp_ack_filter(true);
+ wilc_enable_tcp_ack_filter(vif, true);
else if (stats->link_speed != DEFAULT_LINK_SPEED)
- wilc_enable_tcp_ack_filter(false);
+ wilc_enable_tcp_ack_filter(vif, false);
/* free 'msg' for async command, for sync caller will free it */
if (msg->is_sync)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 3418d2d..35a83d4 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1125,9 +1125,9 @@ static int get_station(struct wiphy *wiphy, struct net_device *dev,
if (stats.link_speed > TCP_ACK_FILTER_LINK_SPEED_THRESH &&
stats.link_speed != DEFAULT_LINK_SPEED)
- wilc_enable_tcp_ack_filter(true);
+ wilc_enable_tcp_ack_filter(vif, true);
else if (stats.link_speed != DEFAULT_LINK_SPEED)
- wilc_enable_tcp_ack_filter(false);
+ wilc_enable_tcp_ack_filter(vif, false);
}
return 0;
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index c53d38f..54ce1ff 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -104,6 +104,32 @@ struct frame_reg {
bool reg;
};
+#define MAX_TCP_SESSION 25
+#define MAX_PENDING_ACKS 256
+
+struct ack_session_info {
+ u32 seq_num;
+ u32 bigger_ack_num;
+ u16 src_port;
+ u16 dst_port;
+ u16 status;
+};
+
+struct pending_acks_info {
+ u32 ack_num;
+ u32 session_index;
+ struct txq_entry_t *txqe;
+};
+
+struct tcp_ack_filter {
+ struct ack_session_info ack_session_info[2 * MAX_TCP_SESSION];
+ struct pending_acks_info pending_acks_info[MAX_PENDING_ACKS];
+ u32 pending_base;
+ u32 tcp_session;
+ u32 pending_acks;
+ bool enabled;
+};
+
struct wilc_vif {
u8 idx;
u8 iftype;
@@ -124,6 +150,7 @@ struct wilc_vif {
bool is_termination_progress;
struct timer_list periodic_rssi;
struct rf_info periodic_stat;
+ struct tcp_ack_filter ack_filter;
};
struct wilc {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index d397c27..52402c3 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -93,63 +93,46 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
return 0;
}
-struct ack_session_info;
-struct ack_session_info {
- u32 seq_num;
- u32 bigger_ack_num;
- u16 src_port;
- u16 dst_port;
- u16 status;
-};
-
-struct pending_acks_info {
- u32 ack_num;
- u32 session_index;
- struct txq_entry_t *txqe;
-};
-
#define NOT_TCP_ACK (-1)
-#define MAX_TCP_SESSION 25
-#define MAX_PENDING_ACKS 256
-static struct ack_session_info ack_session_info[2 * MAX_TCP_SESSION];
-static struct pending_acks_info pending_acks_info[MAX_PENDING_ACKS];
-
-static u32 pending_base;
-static u32 tcp_session;
-static u32 pending_acks;
-
-static inline int add_tcp_session(u32 src_prt, u32 dst_prt, u32 seq)
+static inline int add_tcp_session(struct wilc_vif *vif, u32 src_prt,
+ u32 dst_prt, u32 seq)
{
- if (tcp_session < 2 * MAX_TCP_SESSION) {
- ack_session_info[tcp_session].seq_num = seq;
- ack_session_info[tcp_session].bigger_ack_num = 0;
- ack_session_info[tcp_session].src_port = src_prt;
- ack_session_info[tcp_session].dst_port = dst_prt;
- tcp_session++;
+ struct tcp_ack_filter *f = &vif->ack_filter;
+
+ if (f->tcp_session < 2 * MAX_TCP_SESSION) {
+ f->ack_session_info[f->tcp_session].seq_num = seq;
+ f->ack_session_info[f->tcp_session].bigger_ack_num = 0;
+ f->ack_session_info[f->tcp_session].src_port = src_prt;
+ f->ack_session_info[f->tcp_session].dst_port = dst_prt;
+ f->tcp_session++;
}
return 0;
}
-static inline int update_tcp_session(u32 index, u32 ack)
+static inline int update_tcp_session(struct wilc_vif *vif, u32 index, u32 ack)
{
+ struct tcp_ack_filter *f = &vif->ack_filter;
+
if (index < 2 * MAX_TCP_SESSION &&
- ack > ack_session_info[index].bigger_ack_num)
- ack_session_info[index].bigger_ack_num = ack;
+ ack > f->ack_session_info[index].bigger_ack_num)
+ f->ack_session_info[index].bigger_ack_num = ack;
return 0;
}
-static inline int add_tcp_pending_ack(u32 ack, u32 session_index,
+static inline int add_tcp_pending_ack(struct wilc_vif *vif, u32 ack,
+ u32 session_index,
struct txq_entry_t *txqe)
{
- u32 i = pending_base + pending_acks;
+ struct tcp_ack_filter *f = &vif->ack_filter;
+ u32 i = f->pending_base + f->pending_acks;
if (i < MAX_PENDING_ACKS) {
- pending_acks_info[i].ack_num = ack;
- pending_acks_info[i].txqe = txqe;
- pending_acks_info[i].session_index = session_index;
+ f->pending_acks_info[i].ack_num = ack;
+ f->pending_acks_info[i].txqe = txqe;
+ f->pending_acks_info[i].session_index = session_index;
txqe->tcp_pending_ack_idx = i;
- pending_acks++;
+ f->pending_acks++;
}
return 0;
}
@@ -162,6 +145,7 @@ static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
unsigned long flags;
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc = vif->wilc;
+ struct tcp_ack_filter *f = &vif->ack_filter;
const struct iphdr *ip_hdr_ptr;
const struct tcphdr *tcp_hdr_ptr;
u32 ihl, total_length, data_offset;
@@ -186,19 +170,19 @@ static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
seq_no = ntohl(tcp_hdr_ptr->seq);
ack_no = ntohl(tcp_hdr_ptr->ack_seq);
- for (i = 0; i < tcp_session; i++) {
- u32 j = ack_session_info[i].seq_num;
+ for (i = 0; i < f->tcp_session; i++) {
+ u32 j = f->ack_session_info[i].seq_num;
if (i < 2 * MAX_TCP_SESSION &&
j == seq_no) {
- update_tcp_session(i, ack_no);
+ update_tcp_session(vif, i, ack_no);
break;
}
}
- if (i == tcp_session)
- add_tcp_session(0, 0, seq_no);
+ if (i == f->tcp_session)
+ add_tcp_session(vif, 0, 0, seq_no);
- add_tcp_pending_ack(ack_no, i, tqe);
+ add_tcp_pending_ack(vif, ack_no, i, tqe);
}
out:
@@ -209,29 +193,30 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
{
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc = vif->wilc;
+ struct tcp_ack_filter *f = &vif->ack_filter;
u32 i = 0;
u32 dropped = 0;
unsigned long flags;
spin_lock_irqsave(&wilc->txq_spinlock, flags);
- for (i = pending_base; i < (pending_base + pending_acks); i++) {
+ for (i = f->pending_base; i < (f->pending_base + f->pending_acks); i++) {
u32 session_index;
u32 bigger_ack_num;
if (i >= MAX_PENDING_ACKS)
break;
- session_index = pending_acks_info[i].session_index;
+ session_index = f->pending_acks_info[i].session_index;
if (session_index >= 2 * MAX_TCP_SESSION)
break;
- bigger_ack_num = ack_session_info[session_index].bigger_ack_num;
+ bigger_ack_num = f->ack_session_info[session_index].bigger_ack_num;
- if (pending_acks_info[i].ack_num < bigger_ack_num) {
+ if (f->pending_acks_info[i].ack_num < bigger_ack_num) {
struct txq_entry_t *tqe;
- tqe = pending_acks_info[i].txqe;
+ tqe = f->pending_acks_info[i].txqe;
if (tqe) {
wilc_wlan_txq_remove(wilc, tqe);
tqe->status = 1;
@@ -243,13 +228,13 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
}
}
}
- pending_acks = 0;
- tcp_session = 0;
+ f->pending_acks = 0;
+ f->tcp_session = 0;
- if (pending_base == 0)
- pending_base = MAX_TCP_SESSION;
+ if (f->pending_base == 0)
+ f->pending_base = MAX_TCP_SESSION;
else
- pending_base = 0;
+ f->pending_base = 0;
spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
@@ -262,11 +247,9 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
return 1;
}
-static bool enabled;
-
-void wilc_enable_tcp_ack_filter(bool value)
+void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value)
{
- enabled = value;
+ vif->ack_filter.enabled = value;
}
static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
@@ -324,7 +307,7 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev, void *priv, u8 *buffer,
tqe->priv = priv;
tqe->tcp_pending_ack_idx = NOT_TCP_ACK;
- if (enabled)
+ if (vif->ack_filter.enabled)
tcp_process(dev, tqe);
wilc_wlan_txq_add_to_tail(dev, tqe);
return wilc->txq_entries;
@@ -686,7 +669,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
tqe->tx_complete_func(tqe->priv, tqe->status);
if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
- pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
+ vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
kfree(tqe);
} while (--entries);
diff --git a/drivers/staging/wilc1000/wilc_wlan.h b/drivers/staging/wilc1000/wilc_wlan.h
index 1f874d1..0fdffdd 100644
--- a/drivers/staging/wilc1000/wilc_wlan.h
+++ b/drivers/staging/wilc1000/wilc_wlan.h
@@ -282,7 +282,7 @@ int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
u32 buffer_size, wilc_tx_complete_func_t func);
void wilc_chip_sleep_manually(struct wilc *wilc);
-void wilc_enable_tcp_ack_filter(bool value);
+void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
--
2.7.4