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 <ajay.kathat@xxxxxxxxxxxxx>
> ---
> 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 {
>
