[PATCH v5 3/3] staging: wilc1000: Remove unused mutex cfg_values_lock
From: Adham Abozaeid
After removing cfg_values member, cfg_values_lock that was used to protect
it can also be removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 9 -
drivers/staging/wilc1000/host_interface.h | 2 --
2 files changed, 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 8d1142776310..2bf91df1ded1 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -369,11 +369,8 @@ static void handle_cfg_param(struct work_struct *work)
struct cfg_param_attr *param = >body.cfg_info;
int ret;
struct wid wid_list[32];
- struct host_if_drv *hif_drv = vif->hif_drv;
int i = 0;
- mutex_lock(_drv->cfg_values_lock);
-
if (param->flag & RETRY_SHORT) {
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
@@ -409,7 +406,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
- mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
@@ -3240,15 +3236,10 @@ int wilc_init(struct net_device *dev, struct
host_if_drv **hif_drv_handler)
timer_setup(_drv->connect_timer, timer_connect_cb, 0);
timer_setup(_drv->remain_on_ch_timer, listen_timer_cb, 0);
- mutex_init(_drv->cfg_values_lock);
- mutex_lock(_drv->cfg_values_lock);
-
hif_drv->hif_state = HOST_IF_IDLE;
hif_drv->p2p_timeout = 0;
- mutex_unlock(_drv->cfg_values_lock);
-
wilc->clients_count++;
return 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 1e2e50e91f76..523b46bfb488 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,8 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- /*lock to protect concurrent setting of cfg params*/
- struct mutex cfg_values_lock;
struct timer_list scan_timer;
struct wilc_vif *scan_timer_vif;
--
2.17.1
[PATCH v5 0/3] staging: wilc1000: validate input to set_wiphy_param and return proper
From: Adham Abozaeid Validate input parameters to set_wiphy_param before scheduling handle_cfg_param() to validate them. This way proper errors can be returned to caller. Also cleaned up unused code in handle_cfg_param. Changes since v1: - Correction spelling in subject of patch#2 - Added From: tag to have correct email from value Changes since v2: - Reverted unnecessary file permission changes Changes since v3: - Fixed 0-day bot warnings for always true conditions for retry_short and retry_long - Fixed typo in From: tag Changes since v4: - Fixed duplicate From: tag Adham Abozaeid (3): staging: wilc1000: validate cfg parameters before scheduling the work staging: wilc1000: Don't keep a copy of wiphy parameters in the driver staging: wilc1000: Remove unused mutex cfg_values_lock drivers/staging/wilc1000/host_interface.c | 75 --- drivers/staging/wilc1000/host_interface.h | 3 - .../staging/wilc1000/wilc_wfi_cfgoperations.c | 32 +++- 3 files changed, 44 insertions(+), 66 deletions(-) -- 2.17.1
[PATCH v5 2/3] staging: wilc1000: Don't keep a copy of wiphy parameters in the driver
From: Adham Abozaeid
host_if_drv.cfg_values is a write only member, and can be removed
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 13 -
drivers/staging/wilc1000/host_interface.h | 1 -
2 files changed, 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index c1215c194907..8d1142776310 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -375,43 +375,31 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
if (param->flag & RETRY_SHORT) {
- u16 retry_limit = param->short_retry_limit;
-
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
- u16 limit = param->long_retry_limit;
-
wid_list[i].id = WID_LONG_RETRY_LIMIT;
wid_list[i].val = (s8 *)>long_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
- u16 frag_th = param->frag_threshold;
-
wid_list[i].id = WID_FRAG_THRESHOLD;
wid_list[i].val = (s8 *)>frag_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
- u16 rts_th = param->rts_threshold;
-
wid_list[i].id = WID_RTS_THRESHOLD;
wid_list[i].val = (s8 *)>rts_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -3256,7 +3244,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv
**hif_drv_handler)
mutex_lock(_drv->cfg_values_lock);
hif_drv->hif_state = HOST_IF_IDLE;
- hif_drv->cfg_values.scan_source = DEFAULT_SCAN;
hif_drv->p2p_timeout = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index df9fc33be094..1e2e50e91f76 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,7 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- struct cfg_param_attr cfg_values;
/*lock to protect concurrent setting of cfg params*/
struct mutex cfg_values_lock;
--
2.17.1
[PATCH v5 1/3] staging: wilc1000: validate cfg parameters before scheduling the work
From: Adham Abozaeid
Validate cfg parameters after being called by cfg80211 in set_wiphy_params
before scheduling the work executed in handle_cfg_param
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 61 ++-
.../staging/wilc1000/wilc_wfi_cfgoperations.c | 32 --
2 files changed, 48 insertions(+), 45 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index b89116c57064..c1215c194907 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -377,61 +377,41 @@ static void handle_cfg_param(struct work_struct *work)
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
- if (retry_limit > 0 && retry_limit < 256) {
- wid_list[i].id = WID_SHORT_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>short_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_SHORT_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>short_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
u16 limit = param->long_retry_limit;
- if (limit > 0 && limit < 256) {
- wid_list[i].id = WID_LONG_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>long_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_LONG_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>long_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
u16 frag_th = param->frag_threshold;
- if (frag_th > 255 && frag_th < 7937) {
- wid_list[i].id = WID_FRAG_THRESHOLD;
- wid_list[i].val = (s8 *)>frag_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_FRAG_THRESHOLD;
+ wid_list[i].val = (s8 *)>frag_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
u16 rts_th = param->rts_threshold;
- if (rts_th > 255) {
- wid_list[i].id = WID_RTS_THRESHOLD;
- wid_list[i].val = (s8 *)>rts_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_RTS_THRESHOLD;
+ wid_list[i].val = (s8 *)>rts_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -441,7 +421,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
-unlock:
mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 4fd5a64b..a6f4fad43bf7 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1149,21 +1149,45 @@ static int set_wiphy_params(struct wiphy *wiphy, u32
changed)
cfg_param_val.flag = 0;
if (changed &
[PATCH v4 1/3] staging: wilc1000: validate cfg parameters before scheduling the work
From: Adham Abozaeid
From: Adham Abozaeid
Validate cfg parameters after being called by cfg80211 in set_wiphy_params
before scheduling the work executed in handle_cfg_param
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 61 ++-
.../staging/wilc1000/wilc_wfi_cfgoperations.c | 32 --
2 files changed, 48 insertions(+), 45 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index b89116c57064..c1215c194907 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -377,61 +377,41 @@ static void handle_cfg_param(struct work_struct *work)
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
- if (retry_limit > 0 && retry_limit < 256) {
- wid_list[i].id = WID_SHORT_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>short_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_SHORT_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>short_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
u16 limit = param->long_retry_limit;
- if (limit > 0 && limit < 256) {
- wid_list[i].id = WID_LONG_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>long_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_LONG_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>long_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
u16 frag_th = param->frag_threshold;
- if (frag_th > 255 && frag_th < 7937) {
- wid_list[i].id = WID_FRAG_THRESHOLD;
- wid_list[i].val = (s8 *)>frag_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_FRAG_THRESHOLD;
+ wid_list[i].val = (s8 *)>frag_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
u16 rts_th = param->rts_threshold;
- if (rts_th > 255) {
- wid_list[i].id = WID_RTS_THRESHOLD;
- wid_list[i].val = (s8 *)>rts_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_RTS_THRESHOLD;
+ wid_list[i].val = (s8 *)>rts_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -441,7 +421,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
-unlock:
mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 4fd5a64b..a6f4fad43bf7 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1149,21 +1149,45 @@ static int set_wiphy_params(struct wiphy *wiphy, u32
changed)
cfg_param_val.flag = 0;
if
[PATCH v4 2/3] staging: wilc1000: Don't keep a copy of wiphy parameters in the driver
From: Adham Abozaeid
From: Adham Abozaeid
host_if_drv.cfg_values is a write only member, and can be removed
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 13 -
drivers/staging/wilc1000/host_interface.h | 1 -
2 files changed, 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index c1215c194907..8d1142776310 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -375,43 +375,31 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
if (param->flag & RETRY_SHORT) {
- u16 retry_limit = param->short_retry_limit;
-
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
- u16 limit = param->long_retry_limit;
-
wid_list[i].id = WID_LONG_RETRY_LIMIT;
wid_list[i].val = (s8 *)>long_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
- u16 frag_th = param->frag_threshold;
-
wid_list[i].id = WID_FRAG_THRESHOLD;
wid_list[i].val = (s8 *)>frag_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
- u16 rts_th = param->rts_threshold;
-
wid_list[i].id = WID_RTS_THRESHOLD;
wid_list[i].val = (s8 *)>rts_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -3256,7 +3244,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv
**hif_drv_handler)
mutex_lock(_drv->cfg_values_lock);
hif_drv->hif_state = HOST_IF_IDLE;
- hif_drv->cfg_values.scan_source = DEFAULT_SCAN;
hif_drv->p2p_timeout = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index df9fc33be094..1e2e50e91f76 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,7 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- struct cfg_param_attr cfg_values;
/*lock to protect concurrent setting of cfg params*/
struct mutex cfg_values_lock;
--
2.17.1
[PATCH v4 3/3] staging: wilc1000: Remove unused mutex cfg_values_lock
From: Adham Abozaeid
From: Adham Abozaeid
After removing cfg_values member, cfg_values_lock that was used to protect
it can also be removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 9 -
drivers/staging/wilc1000/host_interface.h | 2 --
2 files changed, 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 8d1142776310..2bf91df1ded1 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -369,11 +369,8 @@ static void handle_cfg_param(struct work_struct *work)
struct cfg_param_attr *param = >body.cfg_info;
int ret;
struct wid wid_list[32];
- struct host_if_drv *hif_drv = vif->hif_drv;
int i = 0;
- mutex_lock(_drv->cfg_values_lock);
-
if (param->flag & RETRY_SHORT) {
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
@@ -409,7 +406,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
- mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
@@ -3240,15 +3236,10 @@ int wilc_init(struct net_device *dev, struct
host_if_drv **hif_drv_handler)
timer_setup(_drv->connect_timer, timer_connect_cb, 0);
timer_setup(_drv->remain_on_ch_timer, listen_timer_cb, 0);
- mutex_init(_drv->cfg_values_lock);
- mutex_lock(_drv->cfg_values_lock);
-
hif_drv->hif_state = HOST_IF_IDLE;
hif_drv->p2p_timeout = 0;
- mutex_unlock(_drv->cfg_values_lock);
-
wilc->clients_count++;
return 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 1e2e50e91f76..523b46bfb488 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,8 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- /*lock to protect concurrent setting of cfg params*/
- struct mutex cfg_values_lock;
struct timer_list scan_timer;
struct wilc_vif *scan_timer_vif;
--
2.17.1
[PATCH v4 0/3] staging: wilc1000: validate input to set_wiphy_param and return proper
From: Adham Abozaeid Validate input parameters to set_wiphy_param before scheduling handle_cfg_param() to validate them. This way proper errors can be returned to caller. Also cleaned up unused code in handle_cfg_param. Changes since v1: - Correction spelling in subject of patch#2 - Added From: tag to have correct email from value Changes since v2: - Reverted unnecessary file permission changes Changes since v3: - Fixed 0-day bot warnings for always true conditions for retry_short and retry_long - Fixed typo in From: tag Adham Abozaeid (3): staging: wilc1000: validate cfg parameters before scheduling the work staging: wilc1000: Don't keep a copy of wiphy parameters in the driver staging: wilc1000: Remove unused mutex cfg_values_lock drivers/staging/wilc1000/host_interface.c | 75 --- drivers/staging/wilc1000/host_interface.h | 3 - .../staging/wilc1000/wilc_wfi_cfgoperations.c | 32 +++- 3 files changed, 44 insertions(+), 66 deletions(-) -- 2.17.1
Re: [PATCH v3 2/4] staging: wilc1000: validate cfg parameters before scheduling the work
On 11/8/18 4:22 AM, Greg KH wrote: > On Tue, Nov 06, 2018 at 12:01:18AM +, adham.aboza...@microchip.com wrote: >> From: Adham Abozaeid >> >> Validate cfg parameters after being called by cfg80211 in set_wiphy_params >> before scheduling the work executed in handle_cfg_param >> >> Signed-off-by: Adham Abozaeid >> --- >> drivers/staging/wilc1000/host_interface.c | 61 ++- >> .../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 --- >> 2 files changed, 62 insertions(+), 49 deletions(-) > Please fix this up so that the 0-day bot does not complain about it. Sure Greg. I'll submit v4 with the fix for the bot warning. I assume I shouldn't include patch 1/4 in the new version of the series since I see it was applied already, so I'll only resend the other 3 patches Thanks, Adham
[PATCH v3 1/4] staging: wilc1000: remove unused flags in handle_cfg_param()
From: Adham Abozaeid
handle_cfg_param() receives a bit map that describes what to be changed.
Some of these bits flags aren't referred to from elsewhere and can be
removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 216 --
drivers/staging/wilc1000/host_interface.h | 29 ---
drivers/staging/wilc1000/wilc_wlan_if.h | 9 -
3 files changed, 254 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index c4f858b22914..b89116c57064 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -374,64 +374,6 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
- if (param->flag & BSS_TYPE) {
- u8 bss_type = param->bss_type;
-
- if (bss_type < 6) {
- wid_list[i].id = WID_BSS_TYPE;
- wid_list[i].val = (s8 *)>bss_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.bss_type = bss_type;
- } else {
- netdev_err(vif->ndev, "check value 6 over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTH_TYPE) {
- u8 auth_type = param->auth_type;
-
- if (auth_type == 1 || auth_type == 2 || auth_type == 5) {
- wid_list[i].id = WID_AUTH_TYPE;
- wid_list[i].val = (s8 *)>auth_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.auth_type = auth_type;
- } else {
- netdev_err(vif->ndev, "Impossible value\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTHEN_TIMEOUT) {
- if (param->auth_timeout > 0) {
- wid_list[i].id = WID_AUTH_TIMEOUT;
- wid_list[i].val = (s8 *)>auth_timeout;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.auth_timeout = param->auth_timeout;
- } else {
- netdev_err(vif->ndev, "Range(1 ~ 65535) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & POWER_MANAGEMENT) {
- u8 pm_mode = param->power_mgmt_mode;
-
- if (pm_mode < 5) {
- wid_list[i].id = WID_POWER_MANAGEMENT;
- wid_list[i].val = (s8 *)>power_mgmt_mode;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.power_mgmt_mode = pm_mode;
- } else {
- netdev_err(vif->ndev, "Invalid power mode\n");
- goto unlock;
- }
- i++;
- }
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
@@ -492,160 +434,6 @@ static void handle_cfg_param(struct work_struct *work)
}
i++;
}
- if (param->flag & PREAMBLE) {
- u16 preamble_type = param->preamble_type;
-
- if (param->preamble_type < 3) {
- wid_list[i].id = WID_PREAMBLE;
- wid_list[i].val = (s8 *)>preamble_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.preamble_type = preamble_type;
- } else {
- netdev_err(vif->ndev, "Preamble Range(0~2) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & SHORT_SLOT_ALLOWED) {
- u8 slot_allowed = param->short_slot_allowed;
-
- if (slot_allowed < 2) {
- wid_list[i].id = WID_SHORT_SLOT_ALLOWED;
- wid_list[i].val = (s8 *)>short_slot_allowed;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.short_slot_allowed = slot_allowed;
- } else {
- netdev_err(vif->ndev, "Short slot(2) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & TXOP_PROT_DISABLE) {
- u8 prot_disabled = param->txop_prot_disabled;
-
- if (param->txop_prot_disabled < 2) {
- wid_list[i].id = WID_11N_TXOP_PROT_DISABLE;
-
[PATCH v3 4/4] staging: wilc1000: Remove unused mutex cfg_values_lock
From: Adham Abozaeid
After removing cfg_values member, cfg_values_lock that was used to protect
it can also be removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 9 -
drivers/staging/wilc1000/host_interface.h | 2 --
2 files changed, 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 8d1142776310..2bf91df1ded1 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -369,11 +369,8 @@ static void handle_cfg_param(struct work_struct *work)
struct cfg_param_attr *param = >body.cfg_info;
int ret;
struct wid wid_list[32];
- struct host_if_drv *hif_drv = vif->hif_drv;
int i = 0;
- mutex_lock(_drv->cfg_values_lock);
-
if (param->flag & RETRY_SHORT) {
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
@@ -409,7 +406,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
- mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
@@ -3240,15 +3236,10 @@ int wilc_init(struct net_device *dev, struct
host_if_drv **hif_drv_handler)
timer_setup(_drv->connect_timer, timer_connect_cb, 0);
timer_setup(_drv->remain_on_ch_timer, listen_timer_cb, 0);
- mutex_init(_drv->cfg_values_lock);
- mutex_lock(_drv->cfg_values_lock);
-
hif_drv->hif_state = HOST_IF_IDLE;
hif_drv->p2p_timeout = 0;
- mutex_unlock(_drv->cfg_values_lock);
-
wilc->clients_count++;
return 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 1e2e50e91f76..523b46bfb488 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,8 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- /*lock to protect concurrent setting of cfg params*/
- struct mutex cfg_values_lock;
struct timer_list scan_timer;
struct wilc_vif *scan_timer_vif;
--
2.17.1
[PATCH v3 3/4] staging: wilc1000: Don't keep a copy of wiphy parameters in the driver
From: Adham Abozaeid
host_if_drv.cfg_values is a write only member, and can be removed
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 13 -
drivers/staging/wilc1000/host_interface.h | 1 -
2 files changed, 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index c1215c194907..8d1142776310 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -375,43 +375,31 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
if (param->flag & RETRY_SHORT) {
- u16 retry_limit = param->short_retry_limit;
-
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
- u16 limit = param->long_retry_limit;
-
wid_list[i].id = WID_LONG_RETRY_LIMIT;
wid_list[i].val = (s8 *)>long_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
- u16 frag_th = param->frag_threshold;
-
wid_list[i].id = WID_FRAG_THRESHOLD;
wid_list[i].val = (s8 *)>frag_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
- u16 rts_th = param->rts_threshold;
-
wid_list[i].id = WID_RTS_THRESHOLD;
wid_list[i].val = (s8 *)>rts_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -3256,7 +3244,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv
**hif_drv_handler)
mutex_lock(_drv->cfg_values_lock);
hif_drv->hif_state = HOST_IF_IDLE;
- hif_drv->cfg_values.scan_source = DEFAULT_SCAN;
hif_drv->p2p_timeout = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index df9fc33be094..1e2e50e91f76 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -293,7 +293,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- struct cfg_param_attr cfg_values;
/*lock to protect concurrent setting of cfg params*/
struct mutex cfg_values_lock;
--
2.17.1
[PATCH v3 2/4] staging: wilc1000: validate cfg parameters before scheduling the work
From: Adham Abozaeid
Validate cfg parameters after being called by cfg80211 in set_wiphy_params
before scheduling the work executed in handle_cfg_param
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 61 ++-
.../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ---
2 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index b89116c57064..c1215c194907 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -377,61 +377,41 @@ static void handle_cfg_param(struct work_struct *work)
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
- if (retry_limit > 0 && retry_limit < 256) {
- wid_list[i].id = WID_SHORT_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>short_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_SHORT_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>short_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
u16 limit = param->long_retry_limit;
- if (limit > 0 && limit < 256) {
- wid_list[i].id = WID_LONG_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>long_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_LONG_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>long_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
u16 frag_th = param->frag_threshold;
- if (frag_th > 255 && frag_th < 7937) {
- wid_list[i].id = WID_FRAG_THRESHOLD;
- wid_list[i].val = (s8 *)>frag_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_FRAG_THRESHOLD;
+ wid_list[i].val = (s8 *)>frag_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
u16 rts_th = param->rts_threshold;
- if (rts_th > 255) {
- wid_list[i].id = WID_RTS_THRESHOLD;
- wid_list[i].val = (s8 *)>rts_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_RTS_THRESHOLD;
+ wid_list[i].val = (s8 *)>rts_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -441,7 +421,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
-unlock:
mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 4fd5a64b..26bb78a49d81 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1149,21 +1149,55 @@ static int set_wiphy_params(struct wiphy *wiphy, u32
changed)
cfg_param_val.flag = 0;
if (changed &
[PATCH v3 0/4] staging: wilc1000: validate input to set_wiphy_param and return proper errors
From: Adham Abozaeid Validate input parameters to set_wiphy_param before scheduling handle_cfg_param() to validate them. This way proper errors can be returned to caller. Also cleaned up unused code in handle_cfg_param. Changes since v1: - Correction spelling in subject of patch#2 - Added From: tag to have correct email from value Changes since v2: - Reverted unnecessary file permission changes Adham Abozaeid (4): staging: wilc1000: remove unused flags in handle_cfg_param() staging: wilc1000: validate cfg parameters before scheduling the work staging: wilc1000: Don't keep a copy of wiphy parameters in the driver staging: wilc1000: Remove unused mutex cfg_values_lock drivers/staging/wilc1000/host_interface.c | 291 +- drivers/staging/wilc1000/host_interface.h | 32 -- .../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ++- drivers/staging/wilc1000/wilc_wlan_if.h | 9 - 4 files changed, 58 insertions(+), 324 deletions(-) -- 2.17.1
[PATCH v2 3/4] staging: wilc1000: Don't keep a copy of wiphy parameters in the driver
From: Adham Abozaeid
host_if_drv.cfg_values is a write only member, and can be removed
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 13 -
drivers/staging/wilc1000/host_interface.h | 1 -
2 files changed, 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 0bb3b4f6d04e..cdc495287949 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -375,43 +375,31 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
if (param->flag & RETRY_SHORT) {
- u16 retry_limit = param->short_retry_limit;
-
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
- u16 limit = param->long_retry_limit;
-
wid_list[i].id = WID_LONG_RETRY_LIMIT;
wid_list[i].val = (s8 *)>long_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
- u16 frag_th = param->frag_threshold;
-
wid_list[i].id = WID_FRAG_THRESHOLD;
wid_list[i].val = (s8 *)>frag_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
- u16 rts_th = param->rts_threshold;
-
wid_list[i].id = WID_RTS_THRESHOLD;
wid_list[i].val = (s8 *)>rts_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -3139,7 +3127,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv
**hif_drv_handler)
mutex_lock(_drv->cfg_values_lock);
hif_drv->hif_state = HOST_IF_IDLE;
- hif_drv->cfg_values.scan_source = DEFAULT_SCAN;
hif_drv->p2p_timeout = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 5f894fe7c896..60cb1a1fee1a 100755
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -238,7 +238,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- struct cfg_param_attr cfg_values;
/*lock to protect concurrent setting of cfg params*/
struct mutex cfg_values_lock;
--
2.17.1
[PATCH v2 4/4] staging: wilc1000: Remove unused mutex cfg_values_lock
From: Adham Abozaeid
After removing cfg_values member, cfg_values_lock that was used to protect
it can also be removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 9 -
drivers/staging/wilc1000/host_interface.h | 2 --
2 files changed, 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index cdc495287949..165330b494f5 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -369,11 +369,8 @@ static void handle_cfg_param(struct work_struct *work)
struct cfg_param_attr *param = >body.cfg_info;
int ret;
struct wid wid_list[32];
- struct host_if_drv *hif_drv = vif->hif_drv;
int i = 0;
- mutex_lock(_drv->cfg_values_lock);
-
if (param->flag & RETRY_SHORT) {
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
@@ -409,7 +406,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
- mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
@@ -3123,15 +3119,10 @@ int wilc_init(struct net_device *dev, struct
host_if_drv **hif_drv_handler)
timer_setup(_drv->connect_timer, timer_connect_cb, 0);
timer_setup(_drv->remain_on_ch_timer, listen_timer_cb, 0);
- mutex_init(_drv->cfg_values_lock);
- mutex_lock(_drv->cfg_values_lock);
-
hif_drv->hif_state = HOST_IF_IDLE;
hif_drv->p2p_timeout = 0;
- mutex_unlock(_drv->cfg_values_lock);
-
wilc->clients_count++;
return 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 60cb1a1fee1a..c78e0682a248 100755
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -238,8 +238,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- /*lock to protect concurrent setting of cfg params*/
- struct mutex cfg_values_lock;
struct timer_list scan_timer;
struct wilc_vif *scan_timer_vif;
--
2.17.1
[PATCH v2 2/4] staging: wilc1000: validate cfg parameters before scheduling the work
From: Adham Abozaeid
Validate cfg parameters after being called by cfg80211 in set_wiphy_params
before scheduling the work executed in handle_cfg_param
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 61 ++-
.../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ---
2 files changed, 62 insertions(+), 49 deletions(-)
mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 1a1ac5e936a7..0bb3b4f6d04e 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -377,61 +377,41 @@ static void handle_cfg_param(struct work_struct *work)
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
- if (retry_limit > 0 && retry_limit < 256) {
- wid_list[i].id = WID_SHORT_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>short_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_SHORT_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>short_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
u16 limit = param->long_retry_limit;
- if (limit > 0 && limit < 256) {
- wid_list[i].id = WID_LONG_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>long_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_LONG_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>long_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
u16 frag_th = param->frag_threshold;
- if (frag_th > 255 && frag_th < 7937) {
- wid_list[i].id = WID_FRAG_THRESHOLD;
- wid_list[i].val = (s8 *)>frag_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_FRAG_THRESHOLD;
+ wid_list[i].val = (s8 *)>frag_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
u16 rts_th = param->rts_threshold;
- if (rts_th > 255) {
- wid_list[i].id = WID_RTS_THRESHOLD;
- wid_list[i].val = (s8 *)>rts_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_RTS_THRESHOLD;
+ wid_list[i].val = (s8 *)>rts_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -441,7 +421,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
-unlock:
mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
old mode 100644
new mode 100755
index 4fd5a64b..26bb78a49d81
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1149,21 +1149,55 @@ static int
[PATCH v2 1/4] staging: wilc1000: remove unused flags in handle_cfg_param()
From: Adham Abozaeid
handle_cfg_param() receives a bit map that describes what to be changed.
Some of these bits flags aren't referred to from elsewhere and can be
removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 216 --
drivers/staging/wilc1000/host_interface.h | 29 ---
drivers/staging/wilc1000/wilc_wlan_if.h | 9 -
3 files changed, 254 deletions(-)
mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.c
mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.h
mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wlan_if.h
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
old mode 100644
new mode 100755
index 01db8999335e..1a1ac5e936a7
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -374,64 +374,6 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
- if (param->flag & BSS_TYPE) {
- u8 bss_type = param->bss_type;
-
- if (bss_type < 6) {
- wid_list[i].id = WID_BSS_TYPE;
- wid_list[i].val = (s8 *)>bss_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.bss_type = bss_type;
- } else {
- netdev_err(vif->ndev, "check value 6 over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTH_TYPE) {
- u8 auth_type = param->auth_type;
-
- if (auth_type == 1 || auth_type == 2 || auth_type == 5) {
- wid_list[i].id = WID_AUTH_TYPE;
- wid_list[i].val = (s8 *)>auth_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.auth_type = auth_type;
- } else {
- netdev_err(vif->ndev, "Impossible value\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTHEN_TIMEOUT) {
- if (param->auth_timeout > 0) {
- wid_list[i].id = WID_AUTH_TIMEOUT;
- wid_list[i].val = (s8 *)>auth_timeout;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.auth_timeout = param->auth_timeout;
- } else {
- netdev_err(vif->ndev, "Range(1 ~ 65535) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & POWER_MANAGEMENT) {
- u8 pm_mode = param->power_mgmt_mode;
-
- if (pm_mode < 5) {
- wid_list[i].id = WID_POWER_MANAGEMENT;
- wid_list[i].val = (s8 *)>power_mgmt_mode;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.power_mgmt_mode = pm_mode;
- } else {
- netdev_err(vif->ndev, "Invalid power mode\n");
- goto unlock;
- }
- i++;
- }
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
@@ -492,160 +434,6 @@ static void handle_cfg_param(struct work_struct *work)
}
i++;
}
- if (param->flag & PREAMBLE) {
- u16 preamble_type = param->preamble_type;
-
- if (param->preamble_type < 3) {
- wid_list[i].id = WID_PREAMBLE;
- wid_list[i].val = (s8 *)>preamble_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.preamble_type = preamble_type;
- } else {
- netdev_err(vif->ndev, "Preamble Range(0~2) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & SHORT_SLOT_ALLOWED) {
- u8 slot_allowed = param->short_slot_allowed;
-
- if (slot_allowed < 2) {
- wid_list[i].id = WID_SHORT_SLOT_ALLOWED;
- wid_list[i].val = (s8 *)>short_slot_allowed;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.short_slot_allowed = slot_allowed;
- } else {
- netdev_err(vif->ndev, "Short slot(2) over\n");
- goto unlock;
- }
- i++;
- }
-
[PATCH v2 0/4] staging: wilc1000: validate input to set_wiphy_param and return proper errors
From: Adham Abozaeid Validate input parameters to set_wiphy_param before scheduling handle_cfg_param() to validate them. This way proper errors can be returned to caller. Also cleaned up unused code in handle_cfg_param. Changes since v1: - Correction spelling in subject of patch#2 - Added From: tag to have correct email from value Adham Abozaeid (4): staging: wilc1000: remove unused flags in handle_cfg_param() staging: wilc1000: validate cfg parameters before scheduling the work staging: wilc1000: Don't keep a copy of wiphy parameters in the driver staging: wilc1000: Remove unused mutex cfg_values_lock drivers/staging/wilc1000/host_interface.c | 291 +- drivers/staging/wilc1000/host_interface.h | 32 -- .../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ++- drivers/staging/wilc1000/wilc_wlan_if.h | 9 - 4 files changed, 58 insertions(+), 324 deletions(-) mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.c mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.h mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wlan_if.h -- 2.17.1
Re: [PATCH 12/19] wilc: add wilc_wfi_cfgoperations.c
On 10/29/18 1:11 PM, Johannes Berg wrote: > Hi, > > Sorry for the late reply. No worries! Thanks for taking the time to review the driver. > > On Fri, 2018-10-19 at 21:47 +, adham.aboza...@microchip.com wrote: >> Johannes, shadow buffer has 2 more usage that I missed in my first email: >> 1- It keeps a copy of scan results to be able to auto-select from if >> the cfg80211 didn't supply a specific bssid in cfg's connect request >> (struct cfg80211_connect_params). >> In this case the driver will select a network that matches the ssid, >> while having the highest rssi. > You should be able to find this from cfg80211's BSS list as well. > > If we lack some API in this area, which is possible, then we can add it. Frankly, I wasn't aware that the cfg80211's BSS list is accessible from the driver! I'll do some research on that and let you know if these APIs didn't serve our purpose completely. >> 2- It keeps network parameters that the device will need to connect >> to a network, since the device doesn't keep the scan results >> internally. >> These parameters are stored in struct join_bss_param, and passed to >> the device when a connect request is received. >> Some of these parameters can be extracted from cfg's >> cfg80211_connect_params (like cap_info.. etc), but others (like bssid, >> beacon period.. etc) are still required. > Again though, you should be able to extract these from struct > cfg80211_bss, I'd argue? > > johannes >
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/29/18 1:10 PM, Johannes Berg wrote: >>> I'd argue for trying it though as it makes the code MUCH simpler. >> I totally agree that it will be simpler to do the work directly from >> the cfg context rather than scheduling it. >> On a cortex A5 MPU and SPI bus running on 48Mhz, this takes 20ms in >> the idle case, and 100 to 300ms in case of data being transferred in >> parallel. > That does sound pretty slow ... but OTOH, mostly wpa_s etc. would expect > this to actually complete before the next step, so if this is slow and > wpa_s/hostapd is much faster, it might send you a lot of things to do > and so you'd end up being slow anyway? Correct. The speed of executing the work will be the same in both cases (maybe a little slower in case of deferring the work) The intuition here was to do minimum work in the cfg's context, but since this isn't a concern, I can skip deferring the work. Thanks, Adham
Re: [PATCH 2/4] stagign: wilc1000: validate cfg parameters before scheduling the work
On 10/26/18 2:36 AM, Dan Carpenter wrote: > On Thu, Oct 25, 2018 at 09:11:00PM +, adham.aboza...@microchip.com wrote: > > You'll need to resend these because your email from header is in the > wrong format. Also there is a typo in the subject. s/stagign/staging/. > Apologies Dan for that! Will correct the subject and email from format and send v2. Regards, Adham
[PATCH 4/4] staging: wilc1000: Remove unused mutex cfg_values_lock
After removing cfg_values member, cfg_values_lock that was used to protect
it can also be removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 9 -
drivers/staging/wilc1000/host_interface.h | 2 --
2 files changed, 11 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index cdc495287949..165330b494f5 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -369,11 +369,8 @@ static void handle_cfg_param(struct work_struct *work)
struct cfg_param_attr *param = >body.cfg_info;
int ret;
struct wid wid_list[32];
- struct host_if_drv *hif_drv = vif->hif_drv;
int i = 0;
- mutex_lock(_drv->cfg_values_lock);
-
if (param->flag & RETRY_SHORT) {
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
@@ -409,7 +406,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
- mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
@@ -3123,15 +3119,10 @@ int wilc_init(struct net_device *dev, struct
host_if_drv **hif_drv_handler)
timer_setup(_drv->connect_timer, timer_connect_cb, 0);
timer_setup(_drv->remain_on_ch_timer, listen_timer_cb, 0);
- mutex_init(_drv->cfg_values_lock);
- mutex_lock(_drv->cfg_values_lock);
-
hif_drv->hif_state = HOST_IF_IDLE;
hif_drv->p2p_timeout = 0;
- mutex_unlock(_drv->cfg_values_lock);
-
wilc->clients_count++;
return 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 60cb1a1fee1a..c78e0682a248 100755
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -238,8 +238,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- /*lock to protect concurrent setting of cfg params*/
- struct mutex cfg_values_lock;
struct timer_list scan_timer;
struct wilc_vif *scan_timer_vif;
--
2.17.1
[PATCH 0/4] staging: wilc1000: validate input to set_wiphy_param and return proper errors
Validate input parameters to set_wiphy_param before scheduling handle_cfg_param() to validate them. This way proper errors can be returned to caller. Also cleaned up unused code in handle_cfg_param. Adham Abozaeid (4): staging: wilc1000: remove unused flags in handle_cfg_param() stagign: wilc1000: validate cfg parameters before scheduling the work staging: wilc1000: Don't keep a copy of wiphy parameters in the driver staging: wilc1000: Remove unused mutex cfg_values_lock drivers/staging/wilc1000/host_interface.c | 291 +- drivers/staging/wilc1000/host_interface.h | 32 -- .../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ++- drivers/staging/wilc1000/wilc_wlan_if.h | 9 - 4 files changed, 58 insertions(+), 324 deletions(-) mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.c mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.h mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wlan_if.h -- 2.17.1
[PATCH 2/4] stagign: wilc1000: validate cfg parameters before scheduling the work
Validate cfg parameters after being called by cfg80211 in set_wiphy_params
before scheduling the work executed in handle_cfg_param
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 61 ++-
.../staging/wilc1000/wilc_wfi_cfgoperations.c | 50 ---
2 files changed, 62 insertions(+), 49 deletions(-)
mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 1a1ac5e936a7..0bb3b4f6d04e 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -377,61 +377,41 @@ static void handle_cfg_param(struct work_struct *work)
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
- if (retry_limit > 0 && retry_limit < 256) {
- wid_list[i].id = WID_SHORT_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>short_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_SHORT_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>short_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
u16 limit = param->long_retry_limit;
- if (limit > 0 && limit < 256) {
- wid_list[i].id = WID_LONG_RETRY_LIMIT;
- wid_list[i].val = (s8 *)>long_retry_limit;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
- } else {
- netdev_err(vif->ndev, "Range(1~256) over\n");
- goto unlock;
- }
+ wid_list[i].id = WID_LONG_RETRY_LIMIT;
+ wid_list[i].val = (s8 *)>long_retry_limit;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
u16 frag_th = param->frag_threshold;
- if (frag_th > 255 && frag_th < 7937) {
- wid_list[i].id = WID_FRAG_THRESHOLD;
- wid_list[i].val = (s8 *)>frag_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_FRAG_THRESHOLD;
+ wid_list[i].val = (s8 *)>frag_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
u16 rts_th = param->rts_threshold;
- if (rts_th > 255) {
- wid_list[i].id = WID_RTS_THRESHOLD;
- wid_list[i].val = (s8 *)>rts_threshold;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
- } else {
- netdev_err(vif->ndev, "Threshold Range fail\n");
- goto unlock;
- }
+ wid_list[i].id = WID_RTS_THRESHOLD;
+ wid_list[i].val = (s8 *)>rts_threshold;
+ wid_list[i].type = WID_SHORT;
+ wid_list[i].size = sizeof(u16);
+ hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -441,7 +421,6 @@ static void handle_cfg_param(struct work_struct *work)
if (ret)
netdev_err(vif->ndev, "Error in setting CFG params\n");
-unlock:
mutex_unlock(_drv->cfg_values_lock);
kfree(msg);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
old mode 100644
new mode 100755
index 4fd5a64b..26bb78a49d81
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1149,21 +1149,55 @@ static int set_wiphy_params(struct wiphy
[PATCH 3/4] staging: wilc1000: Don't keep a copy of wiphy parameters in the driver
host_if_drv.cfg_values is a write only member, and can be removed
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 13 -
drivers/staging/wilc1000/host_interface.h | 1 -
2 files changed, 14 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 0bb3b4f6d04e..cdc495287949 100755
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -375,43 +375,31 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
if (param->flag & RETRY_SHORT) {
- u16 retry_limit = param->short_retry_limit;
-
wid_list[i].id = WID_SHORT_RETRY_LIMIT;
wid_list[i].val = (s8 *)>short_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.short_retry_limit = retry_limit;
i++;
}
if (param->flag & RETRY_LONG) {
- u16 limit = param->long_retry_limit;
-
wid_list[i].id = WID_LONG_RETRY_LIMIT;
wid_list[i].val = (s8 *)>long_retry_limit;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.long_retry_limit = limit;
i++;
}
if (param->flag & FRAG_THRESHOLD) {
- u16 frag_th = param->frag_threshold;
-
wid_list[i].id = WID_FRAG_THRESHOLD;
wid_list[i].val = (s8 *)>frag_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.frag_threshold = frag_th;
i++;
}
if (param->flag & RTS_THRESHOLD) {
- u16 rts_th = param->rts_threshold;
-
wid_list[i].id = WID_RTS_THRESHOLD;
wid_list[i].val = (s8 *)>rts_threshold;
wid_list[i].type = WID_SHORT;
wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.rts_threshold = rts_th;
i++;
}
@@ -3139,7 +3127,6 @@ int wilc_init(struct net_device *dev, struct host_if_drv
**hif_drv_handler)
mutex_lock(_drv->cfg_values_lock);
hif_drv->hif_state = HOST_IF_IDLE;
- hif_drv->cfg_values.scan_source = DEFAULT_SCAN;
hif_drv->p2p_timeout = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 5f894fe7c896..60cb1a1fee1a 100755
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -238,7 +238,6 @@ struct host_if_drv {
enum host_if_state hif_state;
u8 assoc_bssid[ETH_ALEN];
- struct cfg_param_attr cfg_values;
/*lock to protect concurrent setting of cfg params*/
struct mutex cfg_values_lock;
--
2.17.1
[PATCH 1/4] staging: wilc1000: remove unused flags in handle_cfg_param()
handle_cfg_param() receives a bit map that describes what to be changed.
Some of these bits flags aren't referred to from elsewhere and can be
removed.
Signed-off-by: Adham Abozaeid
---
drivers/staging/wilc1000/host_interface.c | 216 --
drivers/staging/wilc1000/host_interface.h | 29 ---
drivers/staging/wilc1000/wilc_wlan_if.h | 9 -
3 files changed, 254 deletions(-)
mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.c
mode change 100644 => 100755 drivers/staging/wilc1000/host_interface.h
mode change 100644 => 100755 drivers/staging/wilc1000/wilc_wlan_if.h
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
old mode 100644
new mode 100755
index 01db8999335e..1a1ac5e936a7
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -374,64 +374,6 @@ static void handle_cfg_param(struct work_struct *work)
mutex_lock(_drv->cfg_values_lock);
- if (param->flag & BSS_TYPE) {
- u8 bss_type = param->bss_type;
-
- if (bss_type < 6) {
- wid_list[i].id = WID_BSS_TYPE;
- wid_list[i].val = (s8 *)>bss_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.bss_type = bss_type;
- } else {
- netdev_err(vif->ndev, "check value 6 over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTH_TYPE) {
- u8 auth_type = param->auth_type;
-
- if (auth_type == 1 || auth_type == 2 || auth_type == 5) {
- wid_list[i].id = WID_AUTH_TYPE;
- wid_list[i].val = (s8 *)>auth_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.auth_type = auth_type;
- } else {
- netdev_err(vif->ndev, "Impossible value\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & AUTHEN_TIMEOUT) {
- if (param->auth_timeout > 0) {
- wid_list[i].id = WID_AUTH_TIMEOUT;
- wid_list[i].val = (s8 *)>auth_timeout;
- wid_list[i].type = WID_SHORT;
- wid_list[i].size = sizeof(u16);
- hif_drv->cfg_values.auth_timeout = param->auth_timeout;
- } else {
- netdev_err(vif->ndev, "Range(1 ~ 65535) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & POWER_MANAGEMENT) {
- u8 pm_mode = param->power_mgmt_mode;
-
- if (pm_mode < 5) {
- wid_list[i].id = WID_POWER_MANAGEMENT;
- wid_list[i].val = (s8 *)>power_mgmt_mode;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.power_mgmt_mode = pm_mode;
- } else {
- netdev_err(vif->ndev, "Invalid power mode\n");
- goto unlock;
- }
- i++;
- }
if (param->flag & RETRY_SHORT) {
u16 retry_limit = param->short_retry_limit;
@@ -492,160 +434,6 @@ static void handle_cfg_param(struct work_struct *work)
}
i++;
}
- if (param->flag & PREAMBLE) {
- u16 preamble_type = param->preamble_type;
-
- if (param->preamble_type < 3) {
- wid_list[i].id = WID_PREAMBLE;
- wid_list[i].val = (s8 *)>preamble_type;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.preamble_type = preamble_type;
- } else {
- netdev_err(vif->ndev, "Preamble Range(0~2) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag & SHORT_SLOT_ALLOWED) {
- u8 slot_allowed = param->short_slot_allowed;
-
- if (slot_allowed < 2) {
- wid_list[i].id = WID_SHORT_SLOT_ALLOWED;
- wid_list[i].val = (s8 *)>short_slot_allowed;
- wid_list[i].type = WID_CHAR;
- wid_list[i].size = sizeof(char);
- hif_drv->cfg_values.short_slot_allowed = slot_allowed;
- } else {
- netdev_err(vif->ndev, "Short slot(2) over\n");
- goto unlock;
- }
- i++;
- }
- if (param->flag &
Re: [PATCH 12/19] wilc: add wilc_wfi_cfgoperations.c
On 10/09/2018 10:15 AM, Adham Abozaeid wrote: > > > On 10/09/2018 12:55 AM, Johannes Berg wrote: >> On Tue, 2018-10-09 at 04:23 +, adham.aboza...@microchip.com wrote: >>> I don't know what you need the shadow stuff for, but you should remove it anyway, and use the cfg80211 functionality instead. If not sufficient, propose patches to improve it? >>> >>> The point behind using a shadow buffer was to keep the scan results >>> consistent between consecutive scans, and smooth out the cases where >>> an AP isn't found momentarily during scanning. >>> In this implementation, APs found during scanning are added to the >>> shadow list, and removed if not found again for a period of time. >>> >>> I'm not much in favour of this implementation neither since it >>> complicates the driver's logic, but it was serving the purpose. >> >> You really should remove it - cfg80211 *and* wpa_s already do this if >> required. >> >> johannes >> > > Thanks Johannes for the tip. I did some research, and I believe you are > referring to using bss_expire_age and bss_expire_count. > We'll go ahead and remove that. Johannes, shadow buffer has 2 more usage that I missed in my first email: 1- It keeps a copy of scan results to be able to auto-select from if the cfg80211 didn't supply a specific bssid in cfg's connect request (struct cfg80211_connect_params). In this case the driver will select a network that matches the ssid, while having the highest rssi. 2- It keeps network parameters that the device will need to connect to a network, since the device doesn't keep the scan results internally. These parameters are stored in struct join_bss_param, and passed to the device when a connect request is received. Some of these parameters can be extracted from cfg's cfg80211_connect_params (like cap_info.. etc), but others (like bssid, beacon period.. etc) are still required. I think I can remove the functionality where it keeps older networks that didn't appear in the latest scan results, and let cfg take care of that, but will still need it to keep a copy of the latest scan results. Let me know what you think. Thanks, Adham
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/19/2018 12:02 AM, Johannes Berg wrote: > Do you mean we should be doing the work from the cfg context? Note that this is called cfg80211_ops.set_wiphy_params(), and involves locking mutexes, packing the wids, bus operations, and waiting for the device to respond. >>> >>> That *should* be fine - how long do you expect that to take? >>> >> >> It depends on the IO bus used (SPI/SDIO), clock speed.. etc >> It would also vary according to the data packets being transferred >> with the device. >> If there's heavy data transfer, configuration packets would take >> longer to be sent, and for their response to be received. > > How much time do you think you're looking at? > > I'd argue for trying it though as it makes the code MUCH simpler. I totally agree that it will be simpler to do the work directly from the cfg context rather than scheduling it. On a cortex A5 MPU and SPI bus running on 48Mhz, this takes 20ms in the idle case, and 100 to 300ms in case of data being transferred in parallel. Thanks, Adham
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/18/2018 01:23 AM, Johannes Berg wrote: > On Fri, 2018-10-12 at 22:08 +, adham.aboza...@microchip.com wrote: >> >> On 10/11/2018 12:01 AM, Johannes Berg wrote: >>> Agree. parameter validation can be done before scheduling the work, and hence appropriate error can be returned to caller . >>> >>> If I got your point correctly, you are referring to the lines that stores the parameters into the hif_drv->cfg_values. >>> >>> Well, I was actually thinking that I'm not even sure why you schedule >>> work at all! >> >> Do you mean we should be doing the work from the cfg context? >> Note that this is called cfg80211_ops.set_wiphy_params(), and involves >> locking mutexes, packing the wids, bus operations, and waiting for the >> device to respond. > > That *should* be fine - how long do you expect that to take? > It depends on the IO bus used (SPI/SDIO), clock speed.. etc It would also vary according to the data packets being transferred with the device. If there's heavy data transfer, configuration packets would take longer to be sent, and for their response to be received. Thanks, Adham
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/11/2018 12:01 AM, Johannes Berg wrote: > >> Agree. parameter validation can be done before scheduling the work, >> and hence appropriate error can be returned to caller . > > >> If I got your point correctly, you are referring to the lines that >> stores the parameters into the hif_drv->cfg_values. > > Well, I was actually thinking that I'm not even sure why you schedule > work at all! Do you mean we should be doing the work from the cfg context? Note that this is called cfg80211_ops.set_wiphy_params(), and involves locking mutexes, packing the wids, bus operations, and waiting for the device to respond. >> Instead of packing the parameters in host structures like struct >> add_sta_param, then repacking it in the device format, it can use >> struct station_parameters and pack them directly into the device >> format > > Makes sense. Also note my other email on how there should be structs > involved, rather than open-coded WID entry lists. Thanks for the suggestion. I agree to that as well.
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/10/2018 01:14 PM, Johannes Berg wrote:
> Looking at all this wid_list stuff again,
>
>> +wid_list[wid_cnt].id = WID_SUCCESS_FRAME_COUNT;
>> +wid_list[wid_cnt].type = WID_INT;
>> +wid_list[wid_cnt].size = sizeof(u32);
>> +wid_list[wid_cnt].val = (s8 *)(&(dummyval));
>> +wid_cnt++;
>
> Doesn't that have endian issues?
>
>> +wid_list[wid_cnt].id = WID_RECEIVED_FRAGMENT_COUNT;
>> +wid_list[wid_cnt].type = WID_INT;
>> +wid_list[wid_cnt].size = sizeof(u32);
>> +wid_list[wid_cnt].val = (s8 *)(&(dummyval));
>> +wid_cnt++;
>
> But I'm not really sure what the pointer does, tbh.
Here the driver is configuring parameters in the device by sending a WID
command for each parameters.
The val pointer points to the value of the parameter to be set, and here all
parameters being set to 0 were sharing the dummyval variable.
Looking again at this, these constant parameters can be omitted from the driver
and done on the device instead.
>
>> +wid_list[wid_cnt].id = WID_JOIN_REQ_EXTENDED;
>> +wid_list[wid_cnt].type = WID_STR;
>> +wid_list[wid_cnt].size = 112;
>> +wid_list[wid_cnt].val = kmalloc(wid_list[wid_cnt].size, GFP_KERNEL);
>
> I think you should declare a structure for these 112 bytes, clearly it's
> something like
>
>> +if (conn_attr->ssid) {
>> +memcpy(cur_byte, conn_attr->ssid, conn_attr->ssid_len);
>> +cur_byte[conn_attr->ssid_len] = '\0';
>> +}
>> +cur_byte += MAX_SSID_LEN;
>
> u8 ssid[32];
>
>> +*(cur_byte++) = INFRASTRUCTURE;
>
> u8 type;
>
>> +
>> +if (conn_attr->ch >= 1 && conn_attr->ch <= 14) {
>> +*(cur_byte++) = conn_attr->ch;
>> +} else {
>> +netdev_err(vif->ndev, "Channel out of range\n");
>> +*(cur_byte++) = 0xFF;
>> +}
>
> u8 channel;
>
>> +*(cur_byte++) = (bss_param->cap_info) & 0xFF;
>> +*(cur_byte++) = ((bss_param->cap_info) >> 8) & 0xFF;
>
> __le16 cap_info;
>
>> +if (conn_attr->bssid)
>> +memcpy(cur_byte, conn_attr->bssid, 6);
>> +cur_byte += 6;
>
> u8 bssid[ETH_ALEN];
>
>> +if (conn_attr->bssid)
>> +memcpy(cur_byte, conn_attr->bssid, 6);
>> +cur_byte += 6;
>
> again?
Agree. Can be changed to avoid duplication. Requires a matching change on the
device.
>
>> +*(cur_byte++) = (bss_param->beacon_period) & 0xFF;
>> +*(cur_byte++) = ((bss_param->beacon_period) >> 8) & 0xFF;
>
> __le16 beacon_period;
>
>> +*(cur_byte++) = bss_param->dtim_period;
>
> u8 dtim_period;
>
> etc.
>
> Declaring it as a struct also means you don't have to do all the
> put_le16_unaligned() or whatever, but can just fill the struct properly.
>
Agree. The idea was of packing the parameters manually was to avoid struct
padding issues, but I can declare the struct as packed instead
Thanks,
Adham
Re: [PATCH 04/19] wilc: add host_interface.c
On 10/08/2018 07:31 AM, Johannes Berg wrote:
>
>> +*currbyte = (u32)0 & DRV_HANDLER_MASK;
>
> You do this a few times, not sure what it's supposed to achieve?
>
>> +if (param->flag & RETRY_LONG) {
>> +u16 limit = param->long_retry_limit;
>> +
>> +if (limit > 0 && limit < 256) {
>> +wid_list[i].id = WID_LONG_RETRY_LIMIT;
>> +wid_list[i].val = (s8 *)>long_retry_limit;
>> +wid_list[i].type = WID_SHORT;
>> +wid_list[i].size = sizeof(u16);
>> +hif_drv->cfg_values.long_retry_limit = limit;
>> +} else {
>> +netdev_err(vif->ndev, "Range(1~256) over\n");
>> +goto unlock;
>> +}
>> +i++;
>> +}
>
> So ... can anyone tell me why there's a complete driver-internal
> messaging infrastructure in this, that even suppresses errors like here
> (out of range just results in a message rather than returning an error
> to wherever it originated)?
>
Agree. parameter validation can be done before scheduling the work, and hence
appropriate error can be returned to caller .
> It almost *seems* like it's a to-device infrastructure, but it can't be
> since it uses host pointers everywhere?
>
> I think this code would be far better off without the "bounce in driver
> to resolve host pointers" step.
If I got your point correctly, you are referring to the lines that stores the
parameters into the hif_drv->cfg_values.
I agree, the cfg_values isn't read from anywhere in the driver, so can be
removed
>> +if (conn_attr->ssid) {
>> +memcpy(cur_byte, conn_attr->ssid, conn_attr->ssid_len);
>> +cur_byte[conn_attr->ssid_len] = '\0';
>> +}
>> +cur_byte += MAX_SSID_LEN;
>
> again, SSIDs are not 0-terminated strings
For this specific code, the device requires the ssid to be null terminated,
since it doesn't receive the ssid_len parameter.
For other ssid references in the driver, the null termination can be removed.
>
>> +static void host_int_fill_join_bss_param(struct join_bss_param *param, u8
>> *ies,
>> + u16 *out_index, u8 *pcipher_tc,
>> + u8 *auth_total_cnt, u32 tsf_lo,
>> + u8 *rates_no)
>> +{
>> +u8 ext_rates_no;
>> +u16 offset;
>> +u8 pcipher_cnt;
>> +u8 auth_cnt;
>> +u8 i, j;
>> +u16 index = *out_index;
>> +
>> +if (ies[index] == WLAN_EID_SUPP_RATES) {
>> +*rates_no = ies[index + 1];
>> +param->supp_rates[0] = *rates_no;
>> +index += 2;
>> +
>> +for (i = 0; i < *rates_no; i++)
>> +param->supp_rates[i + 1] = ies[index + i];
>> +
>> +index += *rates_no;
>> +} else if (ies[index] == WLAN_EID_EXT_SUPP_RATES) {
>> +ext_rates_no = ies[index + 1];
>> +if (ext_rates_no > (MAX_RATES_SUPPORTED - *rates_no))
>> +param->supp_rates[0] = MAX_RATES_SUPPORTED;
>> +else
>> +param->supp_rates[0] += ext_rates_no;
>> +index += 2;
>> +for (i = 0; i < (param->supp_rates[0] - *rates_no); i++)
>> +param->supp_rates[*rates_no + i + 1] = ies[index + i];
>> +
>> +index += ext_rates_no;
>> +} else if (ies[index] == WLAN_EID_HT_CAPABILITY) {
>> +param->ht_capable = true;
>> +index += ies[index + 1] + 2;
>> +} else if ((ies[index] == WLAN_EID_VENDOR_SPECIFIC) &&
>> + (ies[index + 2] == 0x00) && (ies[index + 3] == 0x50) &&
>> + (ies[index + 4] == 0xF2) && (ies[index + 5] == 0x02) &&
>> + ((ies[index + 6] == 0x00) || (ies[index + 6] == 0x01)) &&
>> + (ies[index + 7] == 0x01)) {
>> +param->wmm_cap = true;
>> +
>> +if (ies[index + 8] & BIT(7))
>> +param->uapsd_cap = true;
>> +index += ies[index + 1] + 2;
>> +} else if ((ies[index] == WLAN_EID_VENDOR_SPECIFIC) &&
>> + (ies[index + 2] == 0x50) && (ies[index + 3] == 0x6f) &&
>> + (ies[index + 4] == 0x9a) &&
>> + (ies[index + 5] == 0x09) && (ies[index + 6] == 0x0c)) {
>> +u16 p2p_cnt;
>> +
>> +param->tsf = tsf_lo;
>> +param->noa_enabled = 1;
>> +param->idx = ies[index + 9];
>> +
>> +if (ies[index + 10] & BIT(7)) {
>> +param->opp_enabled = 1;
>> +param->ct_window = ies[index + 10];
>> +} else {
>> +param->opp_enabled = 0;
>> +}
>> +
>> +param->cnt = ies[index + 11];
>> +p2p_cnt = index + 12;
>> +
>> +memcpy(param->duration, ies + p2p_cnt, 4);
>> +p2p_cnt += 4;
>> +
>> +memcpy(param->interval, ies + p2p_cnt, 4);
>> +p2p_cnt += 4;
Re: [PATCH 03/19] wilc: add host_interface.h
On 10/09/2018 01:02 PM, Johannes Berg wrote: > On Tue, 2018-10-09 at 20:01 +, adham.aboza...@microchip.com wrote: >> >> Do you mean that it can also be used for probing specific networks even if >> they are broadcasting their SSID? > > Yes. > >> I think this might be a possible case if the user is trying to limit the >> scan results to a specific network. > > No. Don't do any filtering based on this. > > Basically the only thing you should do with the (list of) SSIDs is to > send a probe request containing each of them. Userspace/cfg80211 will > take care that the usual empty (wildcard) SSID is included in the list, > so don't send one with that by yourself, just iterate the list. > > If you only support sending one probe request in each scan request, then > set the max # of SSIDs supported to 1, otherwise set it to the maximum > (or a reasonable limit like 20 if it's in software). > > johannes > Thanks Johannes. It's clear now.
Re: [PATCH 03/19] wilc: add host_interface.h
On 10/09/2018 12:14 PM, Johannes Berg wrote: > On Tue, 2018-10-09 at 18:36 +, adham.aboza...@microchip.com wrote: >> >> Johannes, is the cfg80211_scan_request.ssid used for something else >> other than specifying the hidden networks that the controller should >> scan for? > > Ahh, now I understand where you're coming from. > > Well, it's the SSID to include in the probe request(s), the most common > thing there is the 0-length wildcard, of course. > > So while it may be required for hidden networks, it's not strictly > related to that. Do you mean that it can also be used for probing specific networks even if they are broadcasting their SSID? I think this might be a possible case if the user is trying to limit the scan results to a specific network. > > johannes >
Re: [PATCH 03/19] wilc: add host_interface.h
On 10/09/2018 05:18 AM, Ajay Singh wrote:
>
> On 10/9/2018 5:16 PM, Johannes Berg wrote:
>> On Tue, 2018-10-09 at 17:14 +0530, Ajay Singh wrote:
>>> On 10/9/2018 4:06 PM, Johannes Berg wrote:
On Tue, 2018-10-09 at 16:04 +0530, Ajay Singh wrote:
>>> +typedef void (*wilc_remain_on_chan_expired)(void *, u32);
>>> +typedef void (*wilc_remain_on_chan_ready)(void *);
> I think as per coding style the typedef for function pointer are allowed.
True, I guess, but why do you need them?
>>> Actually these function pointer are used in multiple places i.e inside
>>> the struct and also for passing as the argument for the function. So i
>>> think its better to keep them as typedef to simplify and avoid any 'line
>>> over 80 chars' checkpatch issue. But anyway if you suggest we can modify
>>> to remove these typedefs .
>> I guess that must be part of the internal bounce buffer mechanism? I
>> guess leave them for now and see what falls out.
>>
>>> +struct hidden_network {
>>>
>>> Yes, its not related to hidden SSID. Suppose cfg80211 scan is called
>>> with SSID information(active scan) then SSID info will be maintained in
>>> this structure.
>> so maybe rename this?
>>
> Yes, sure I will rename this struct.
>
> Regards,
> Ajay
>
Johannes, is the cfg80211_scan_request.ssid used for something else other than
specifying the hidden networks that the controller should scan for?
Re: [PATCH 12/19] wilc: add wilc_wfi_cfgoperations.c
On 10/09/2018 12:55 AM, Johannes Berg wrote: > On Tue, 2018-10-09 at 04:23 +, adham.aboza...@microchip.com wrote: >> >>> I don't know what you need the shadow stuff for, but you should remove >>> it anyway, and use the cfg80211 functionality instead. If not >>> sufficient, propose patches to improve it? >> >> The point behind using a shadow buffer was to keep the scan results >> consistent between consecutive scans, and smooth out the cases where >> an AP isn't found momentarily during scanning. >> In this implementation, APs found during scanning are added to the >> shadow list, and removed if not found again for a period of time. >> >> I'm not much in favour of this implementation neither since it >> complicates the driver's logic, but it was serving the purpose. > > You really should remove it - cfg80211 *and* wpa_s already do this if > required. > > johannes > Thanks Johannes for the tip. I did some research, and I believe you are referring to using bss_expire_age and bss_expire_count. We'll go ahead and remove that. Thanks, Adham
Re: [PATCH 12/19] wilc: add wilc_wfi_cfgoperations.c
On 10/08/2018 07:57 AM, Johannes Berg wrote:
> On Wed, 2018-09-26 at 15:55 +0530, Ajay Singh wrote:
>>
>> +static void clear_shadow_scan(struct wilc_priv *priv)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < priv->scanned_cnt; i++) {
>> +kfree(priv->scanned_shadow[i].ies);
>> +priv->scanned_shadow[i].ies = NULL;
>> +
>> +kfree(priv->scanned_shadow[i].join_params);
>> +priv->scanned_shadow[i].join_params = NULL;
>> +}
>> +priv->scanned_cnt = 0;
>> +}
>
> This seems unlikely to be a good idea - why keep things around in the
> driver?
>
>
>> +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 < priv->scanned_cnt; i++) {
>> +struct network_info *network_info;
>> +s32 freq;
>> +struct ieee80211_channel *channel;
>> +int rssi;
>> +struct cfg80211_bss *bss;
>> +
>> +network_info = >scanned_shadow[i];
>> +
>> +if (!memcmp("DIRECT-", network_info->ssid, 7) && !direct_scan)
>> +continue;
>
> Err, no? Don't do that? What's the point?
>
> I don't know what you need the shadow stuff for, but you should remove
> it anyway, and use the cfg80211 functionality instead. If not
> sufficient, propose patches to improve it?
The point behind using a shadow buffer was to keep the scan results consistent
between consecutive scans, and smooth out the cases where an AP isn't found
momentarily during scanning.
In this implementation, APs found during scanning are added to the shadow list,
and removed if not found again for a period of time.
I'm not much in favour of this implementation neither since it complicates the
driver's logic, but it was serving the purpose.
>
>> +if (memcmp("DIRECT-", network_info->ssid, 7))
>> +return;
>
> same here
>
>
> johannes
>
Thanks,
Adham
Re: [RFC 00/19] wilc: added driver for wilc module
On 10/08/2018 12:38 AM, Kalle Valo wrote: > Ajay Singh writes: > >> On Sat, 6 Oct 2018 15:45:41 +0300 >> Kalle Valo wrote: >> >>> Ajay Singh writes: >>> This patch set contains the driver files from 'driver/staging/wilc1000'. Renamed the driver from 'wilc1000' to 'wilc' to have generic name, as the same driver will be used by other wilc family members. >>> >>> I'm worried that the name 'wilc' is just too generic, I liked the >>> original name wilc1000 much more. Quite often when we have a new >>> generation of wireless devices there's also a new driver, so in the >>> long run I'm worried that a generic name like 'wilc' could be a >>> source of confusion. I think it's much smaller problem if >>> 'wilc1000' (the driver) also supports wilc3000 (the device), people >>> are already used to that. > If I'm understanding correctly you are worried that 'wilc1000-spi.ko' > also supports wilc3000 devices, but I don't see that as a problem. I > think it's very common (not just in wireless) that the marketing names > don't always match with driver names. > It's highly unlikely that microchip will have a new generation of wilc devices other than wilc1000 and wilc3000, since a new family is already in development. And in case a new generation was developed, it will be best to support it in the current driver because of the similarities between wilc devices. I'm afraid that it might be more confusing for users to use wilc1000 naming while they are using wilc3000 hardware. It's not only that the name that is different from the marketing name, but it also refers to another existing product. Thanks, Adham
