Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420698 > unrolled thread

[PATCH 0/7] *** staging: wilc1000: Replace semaphores with mutexes or completions ***

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] *** staging: wilc1000: Replace semaphores with mutexes or completions *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-13 12:40 +0200
    [PATCH 1/7] staging: wilc1000: Replace semaphore txq_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-13 12:40 +0200
    [PATCH 2/7] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-13 12:40 +0200
      Re: [PATCH 2/7] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex Arnd Bergmann <arnd@arndb.de> - 2016-06-13 15:20 +0200
    [PATCH 3/7] staging: wilc1000: Replace semaphore cfg_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-13 12:40 +0200
      Re: [PATCH 3/7] staging: wilc1000: Replace semaphore cfg_event with completion Arnd Bergmann <arnd@arndb.de> - 2016-06-13 16:30 +0200
    Re: [PATCH 0/7] *** staging: wilc1000: Replace semaphores with mutexes or completions *** Arnd Bergmann <arnd@arndb.de> - 2016-06-13 16:30 +0200
      Re: [PATCH 0/7] *** staging: wilc1000: Replace semaphores with  mutexes or completions *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-13 16:50 +0200
    [PATCH v2 0/5] *** staging: wilc1000: Replace semaphores with mutexes or completions *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:30 +0200
      [PATCH v2 1/5] staging: wilc1000: Replace semaphore txq_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:30 +0200
    [PATCH v3 4/5] staging: wilc1000: Replace semaphore sync_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
    [PATCH v3 0/5] *** staging: wilc1000: Replace semaphores with mutexes or completions *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
      [PATCH v3 5/5] staging: wilc1000: Remove semaphore close_exit_sync Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
      [PATCH v3 3/5] staging: wilc1000: Replace semaphore cfg_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
      [PATCH v3 1/5] staging: wilc1000: Replace semaphore txq_event with completion Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
      [PATCH v3 2/5] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-15 07:40 +0200
    [PATCH v2 1/2] staging: wilc1000: message_queue: Move code to host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-20 12:20 +0200
    [PATCH v2 0/2] *** staging: wilc1000: Replace semaphores *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-20 12:20 +0200
      [PATCH v2 2/2] staging: wilc1000: Replace kthread with workqueue for host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-20 12:20 +0200
        Re: [PATCH v2 2/2] staging: wilc1000: Replace kthread with workqueue for host interface Arnd Bergmann <arnd@arndb.de> - 2016-06-21 18:10 +0200
      [PATCH v3 2/3] staging: wilc1000: Replace kthread with workqueue for host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-22 12:10 +0200
      [PATCH v3 0/3] *** staging: wilc1000: Replace semaphores *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-22 12:10 +0200
        [PATCH v3 1/3] staging: wilc1000: message_queue: Move code to host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-22 12:10 +0200
        [PATCH v3 3/3] staging: wilc1000: Change interface wilc_mq_send to wilc_enqueue_cmd Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-22 12:10 +0200
      [PATCH v4 2/3] staging: wilc1000: Replace kthread with workqueue for host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-23 07:50 +0200
      [PATCH v4 0/3] *** staging: wilc1000: Replace semaphores *** Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-23 07:50 +0200
        [PATCH v4 3/3] staging: wilc1000: Change interface wilc_mq_send to wilc_enqueue_cmd Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-23 07:50 +0200
        [PATCH v4 1/3] staging: wilc1000: message_queue: Move code to host interface Binoy Jayan <binoy.jayan@linaro.org> - 2016-06-23 07:50 +0200
        Re: [PATCH v4 0/3] *** staging: wilc1000: Replace semaphores *** Arnd Bergmann <arnd@arndb.de> - 2016-06-23 11:40 +0200

Page 1 of 2  [1] 2  Next page →

#1420698 — [PATCH 0/7] *** staging: wilc1000: Replace semaphores with mutexes or completions ***

Hi,

These are a set of patches which removes semaphores from:

drivers/staging/wilc1000

These are part of a bigger effort to eliminate all semaphores
from the linux kernel.

They build correctly (individually and as a whole).

NB: The changes are untested

Thanks,
Binoy

Binoy Jayan (7):
  staging: wilc1000: Replace semaphore txq_event with completion
  staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex
  staging: wilc1000: Replace semaphore cfg_event with completion
  staging: wilc1000: Replace semaphore sync_event with completion
  staging: wilc1000: Replace semaphore close_exit_sync with completion
  staging: wilc1000: message_queue: Replace semaphore sem with
    completion
  staging: wilc1000: Remove unused inclusion of semaphore header

 drivers/staging/wilc1000/TODO                 |  1 -
 drivers/staging/wilc1000/linux_wlan.c         | 45 ++++++++++-----------------
 drivers/staging/wilc1000/wilc_msgqueue.c      | 13 ++++----
 drivers/staging/wilc1000/wilc_msgqueue.h      |  4 +--
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 13 ++++----
 drivers/staging/wilc1000/wilc_wlan.c          | 35 +++++++++++----------
 drivers/staging/wilc1000/wilc_wlan.h          |  2 +-
 drivers/staging/wilc1000/wilc_wlan_if.h       |  1 -
 8 files changed, 52 insertions(+), 62 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1420699 — [PATCH 1/7] staging: wilc1000: Replace semaphore txq_event with completion

The semaphore 'txq_event' is used as completion, so convert it
to a struct completion type.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/staging/wilc1000/linux_wlan.c         | 8 ++++----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 3 ++-
 drivers/staging/wilc1000/wilc_wlan.c          | 8 +++++---
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 4f93c11..90f906d 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -316,7 +316,7 @@ static int linux_wlan_txq_task(void *vp)
 
 	complete(&wl->txq_thread_started);
 	while (1) {
-		down(&wl->txq_event);
+		wait_for_completion(&wl->txq_event);
 
 		if (wl->close) {
 			complete(&wl->txq_thread_started);
@@ -650,7 +650,7 @@ void wilc1000_wlan_deinit(struct net_device *dev)
 			mutex_unlock(&wl->hif_cs);
 		}
 		if (&wl->txq_event)
-			up(&wl->txq_event);
+			wait_for_completion(&wl->txq_event);
 
 		wlan_deinitialize_threads(dev);
 		deinit_irq(dev);
@@ -681,7 +681,7 @@ static int wlan_init_locks(struct net_device *dev)
 	spin_lock_init(&wl->txq_spinlock);
 	sema_init(&wl->txq_add_to_head_cs, 1);
 
-	sema_init(&wl->txq_event, 0);
+	init_completion(&wl->txq_event);
 
 	sema_init(&wl->cfg_event, 0);
 	sema_init(&wl->sync_event, 0);
@@ -738,7 +738,7 @@ static void wlan_deinitialize_threads(struct net_device *dev)
 	wl->close = 1;
 
 	if (&wl->txq_event)
-		up(&wl->txq_event);
+		complete(&wl->txq_event);
 
 	if (wl->txq_thread) {
 		kthread_stop(wl->txq_thread);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 3a561df6..12d7c7b 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -42,6 +42,7 @@
 #include "host_interface.h"
 #include "wilc_wlan.h"
 #include <linux/wireless.h>
+#include <linux/completion.h>
 
 #define FLOW_CONTROL_LOWER_THRESHOLD	128
 #define FLOW_CONTROL_UPPER_THRESHOLD	256
@@ -178,7 +179,7 @@ struct wilc {
 
 	struct semaphore cfg_event;
 	struct semaphore sync_event;
-	struct semaphore txq_event;
+	struct completion txq_event;
 	struct completion txq_thread_started;
 
 	struct task_struct *txq_thread;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 11e16d5..1a57135 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1,3 +1,4 @@
+#include <linux/completion.h>
 #include "wilc_wlan_if.h"
 #include "wilc_wlan.h"
 #include "wilc_wfi_netdevice.h"
@@ -89,7 +90,7 @@ static void wilc_wlan_txq_add_to_tail(struct net_device *dev,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 }
 
 static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
@@ -119,7 +120,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 	up(&wilc->txq_add_to_head_cs);
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 
 	return 0;
 }
@@ -287,7 +288,8 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
 	spin_unlock_irqrestore(&wilc->txq_spinlock, wilc->txq_spinlock_flags);
 
 	while (dropped > 0) {
-		wilc_lock_timeout(wilc, &wilc->txq_event, 1);
+		wait_for_completion_timeout(&wilc->txq_event,
+						msecs_to_jiffies(1));
 		dropped--;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1420701 — [PATCH 2/7] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex

The semaphore 'txq_add_to_head_cs' is a simple mutex, so it should be
written as one. Semaphores are going away in the future. Also, removing
the timeout scenario as the error handling code does not propagate the
timeout properly.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/staging/wilc1000/linux_wlan.c         |  4 ++--
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  3 ++-
 drivers/staging/wilc1000/wilc_wlan.c          | 11 ++++-------
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 90f906d..a933551 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -20,7 +20,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/skbuff.h>
-
+#include <linux/mutex.h>
 #include <linux/semaphore.h>
 #include <linux/completion.h>
 
@@ -679,7 +679,7 @@ static int wlan_init_locks(struct net_device *dev)
 	mutex_init(&wl->rxq_cs);
 
 	spin_lock_init(&wl->txq_spinlock);
-	sema_init(&wl->txq_add_to_head_cs, 1);
+	mutex_init(&wl->txq_add_to_head_cs);
 
 	init_completion(&wl->txq_event);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 12d7c7b..239cd43 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -43,6 +43,7 @@
 #include "wilc_wlan.h"
 #include <linux/wireless.h>
 #include <linux/completion.h>
+#include <linux/mutex.h>
 
 #define FLOW_CONTROL_LOWER_THRESHOLD	128
 #define FLOW_CONTROL_UPPER_THRESHOLD	256
@@ -171,7 +172,7 @@ struct wilc {
 	struct wilc_vif *vif[NUM_CONCURRENT_IFC];
 	u8 open_ifcs;
 
-	struct semaphore txq_add_to_head_cs;
+	struct mutex txq_add_to_head_cs;
 	spinlock_t txq_spinlock;
 
 	struct mutex rxq_cs;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 1a57135..9afbe8d 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -99,9 +99,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 	unsigned long flags;
 	struct wilc *wilc = vif->wilc;
 
-	if (wilc_lock_timeout(wilc, &wilc->txq_add_to_head_cs,
-				    CFG_PKTS_TIMEOUT))
-		return -1;
+	mutex_lock(&wilc->txq_add_to_head_cs);
 
 	spin_lock_irqsave(&wilc->txq_spinlock, flags);
 
@@ -119,7 +117,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 	wilc->txq_entries += 1;
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
-	up(&wilc->txq_add_to_head_cs);
+	mutex_unlock(&wilc->txq_add_to_head_cs);
 	complete(&wilc->txq_event);
 
 	return 0;
@@ -573,8 +571,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 		if (wilc->quit)
 			break;
 
-		wilc_lock_timeout(wilc, &wilc->txq_add_to_head_cs,
-					CFG_PKTS_TIMEOUT);
+		mutex_lock(&wilc->txq_add_to_head_cs);
 		wilc_wlan_txq_filter_dup_tcp_ack(dev);
 		tqe = wilc_wlan_txq_get_first(wilc);
 		i = 0;
@@ -755,7 +752,7 @@ _end_:
 		if (ret != 1)
 			break;
 	} while (0);
-	up(&wilc->txq_add_to_head_cs);
+	mutex_unlock(&wilc->txq_add_to_head_cs);
 
 	wilc->txq_exit = 1;
 	*txq_count = wilc->txq_entries;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1420819 — Re: [PATCH 2/7] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex

On Monday, June 13, 2016 4:07:34 PM CEST Binoy Jayan wrote:
> The semaphore 'txq_add_to_head_cs' is a simple mutex, so it should be
> written as one. Semaphores are going away in the future.

Looks good to me.

> Also, removing
> the timeout scenario as the error handling code does not propagate the
> timeout properly.

Good catch! I guess wilc_wlan_handle_txq() running into a timeout 
would end putting the semaphore in a state in which we never again
block on it because the count is now one higher than it should be.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1420704 — [PATCH 3/7] staging: wilc1000: Replace semaphore cfg_event with completion

The semaphore 'cfg_event' is used as completion, so convert
it to a struct completion type.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/staging/wilc1000/linux_wlan.c         |  2 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 +-
 drivers/staging/wilc1000/wilc_wlan.c          | 16 +++++++++-------
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index a933551..81a469a 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -683,7 +683,7 @@ static int wlan_init_locks(struct net_device *dev)
 
 	init_completion(&wl->txq_event);
 
-	sema_init(&wl->cfg_event, 0);
+	init_completion(&wl->cfg_event);
 	sema_init(&wl->sync_event, 0);
 	init_completion(&wl->txq_thread_started);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 239cd43..5fbc07c 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -178,7 +178,7 @@ struct wilc {
 	struct mutex rxq_cs;
 	struct mutex hif_cs;
 
-	struct semaphore cfg_event;
+	struct completion cfg_event;
 	struct semaphore sync_event;
 	struct completion txq_event;
 	struct completion txq_thread_started;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 9afbe8d..2d6eb43 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -310,7 +310,7 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
 	netdev_dbg(vif->ndev, "Adding config packet ...\n");
 	if (wilc->quit) {
 		netdev_dbg(vif->ndev, "Return due to clear function\n");
-		up(&wilc->cfg_event);
+		complete(&wilc->cfg_event);
 		return 0;
 	}
 
@@ -769,7 +769,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
 
 	do {
 		if (wilc->quit) {
-			up(&wilc->cfg_event);
+			complete(&wilc->cfg_event);
 			break;
 		}
 		rqe = wilc_wlan_rxq_remove(wilc);
@@ -820,7 +820,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
 					wilc_wlan_cfg_indicate_rx(wilc, &buffer[pkt_offset + offset], pkt_len, &rsp);
 					if (rsp.type == WILC_CFG_RSP) {
 						if (wilc->cfg_seq_no == rsp.seq_no)
-							up(&wilc->cfg_event);
+							complete(&wilc->cfg_event);
 					} else if (rsp.type == WILC_CFG_RSP_STATUS) {
 						wilc_mac_indicate(wilc, WILC_MAC_INDICATE_STATUS);
 
@@ -1228,11 +1228,12 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
 		if (wilc_wlan_cfg_commit(vif, WILC_CFG_SET, drv_handler))
 			ret_size = 0;
 
-		if (wilc_lock_timeout(wilc, &wilc->cfg_event,
-					    CFG_PKTS_TIMEOUT)) {
+		if (!wait_for_completion_timeout(&wilc->cfg_event,
+					msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
 			netdev_dbg(vif->ndev, "Set Timed Out\n");
 			ret_size = 0;
 		}
+
 		wilc->cfg_frame_in_use = 0;
 		wilc->cfg_frame_offset = 0;
 		wilc->cfg_seq_no += 1;
@@ -1246,6 +1247,7 @@ int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
 {
 	u32 offset;
 	int ret_size;
+
 	struct wilc *wilc = vif->wilc;
 
 	if (wilc->cfg_frame_in_use)
@@ -1265,8 +1267,8 @@ int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
 		if (wilc_wlan_cfg_commit(vif, WILC_CFG_QUERY, drv_handler))
 			ret_size = 0;
 
-		if (wilc_lock_timeout(wilc, &wilc->cfg_event,
-					    CFG_PKTS_TIMEOUT)) {
+		if (!wait_for_completion_timeout(&wilc->cfg_event,
+					msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
 			netdev_dbg(vif->ndev, "Get Timed Out\n");
 			ret_size = 0;
 		}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1420913 — Re: [PATCH 3/7] staging: wilc1000: Replace semaphore cfg_event with completion

On Monday, June 13, 2016 4:07:35 PM CEST Binoy Jayan wrote:
> The semaphore 'cfg_event' is used as completion, so convert
> it to a struct completion type.
> 
> Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>

The change looks good, but


>                         netdev_dbg(vif->ndev, "Set Timed Out\n");
>                         ret_size = 0;
>                 }
> +
>                 wilc->cfg_frame_in_use = 0;
>                 wilc->cfg_frame_offset = 0;
>                 wilc->cfg_seq_no += 1;
> @@ -1246,6 +1247,7 @@ int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
>  {
>         u32 offset;
>         int ret_size;
> +
>         struct wilc *wilc = vif->wilc;
>  
>         if (wilc->cfg_frame_in_use)
> 

These two extra newlines seem unrelated to the other changes and probably
slipped in by accident. The first one is actually ok according to
normal coding style, while the second one looks misplaced.

In general, it's better not to touch whitespace when doing other changes,
except when you are fixing the same code you modify anyway.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1420916

On Monday, June 13, 2016 4:07:32 PM CEST Binoy Jayan wrote:
> Hi,
> 
> These are a set of patches which removes semaphores from:
> 
> drivers/staging/wilc1000
> 
> These are part of a bigger effort to eliminate all semaphores
> from the linux kernel.
> 
> They build correctly (individually and as a whole).
> 
> NB: The changes are untested
> 

Most of these look really good, but I've commented on what I think
should be done differently, most importantly using a work_queue
rather than a completion to replace the semaphore in patch 6.

Maybe you can resend the first five patches after addressing
my comments so we can get those tested and queued already while
you look into reworking patch 6. Let me know if that one ends up
being more complicated than I thought so we can come up with
a different approach.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1420944 — Re: [PATCH 0/7] *** staging: wilc1000: Replace semaphores with mutexes or completions ***

On 13 June 2016 at 19:59, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, June 13, 2016 4:07:32 PM CEST Binoy Jayan wrote:
>> Hi,
>>
>> These are a set of patches which removes semaphores from:
>>
>> drivers/staging/wilc1000
>>
>> These are part of a bigger effort to eliminate all semaphores
>> from the linux kernel.
>>
>> They build correctly (individually and as a whole).
>>
>> NB: The changes are untested
>>
>
> Most of these look really good, but I've commented on what I think
> should be done differently, most importantly using a work_queue
> rather than a completion to replace the semaphore in patch 6.
>
> Maybe you can resend the first five patches after addressing
> my comments so we can get those tested and queued already while
> you look into reworking patch 6. Let me know if that one ends up
> being more complicated than I thought so we can come up with
> a different approach.
>
>         Arnd

Thank you for reviewing the patches thoroughly. I overlooked the
problem with using
completion directly. Thank you for pointing that out. As you have
suggested, I'll rework
on the first 5 patches and work on the last one.

Binoy

[toc] | [prev] | [next] | [standalone]

#1422593 — [PATCH v2 0/5] *** staging: wilc1000: Replace semaphores with mutexes or completions ***

These are a set of patches [v2] which removes semaphores from:

drivers/staging/wilc1000

These are part of a bigger effort to eliminate all semaphores
from the linux kernel.

They build correctly (individually and as a whole).

NB: The changes are untested

Changes w.r.t. review comments on v1

1. Whitespace removed in patch 3
2. Removed semaphore 'close_exit_sync'
3. To rework on patch 6 and send in a seperate patch series

Binoy Jayan (5):
  staging: wilc1000: Replace semaphore txq_event with completion
  staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex
  staging: wilc1000: Replace semaphore cfg_event with completion
  staging: wilc1000: Replace semaphore sync_event with completion
  staging: wilc1000: Remove semaphore close_exit_sync

 drivers/staging/wilc1000/linux_wlan.c         | 31 ++++++++++--------------
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 ++++----
 drivers/staging/wilc1000/wilc_wlan.c          | 34 +++++++++++++--------------
 3 files changed, 35 insertions(+), 40 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422595 — [PATCH v2 1/5] staging: wilc1000: Replace semaphore txq_event with completion

The semaphore 'txq_event' is used as completion, so convert it
to a struct completion type.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
 drivers/staging/wilc1000/linux_wlan.c         | 8 ++++----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 3 ++-
 drivers/staging/wilc1000/wilc_wlan.c          | 8 +++++---
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 4f93c11..90f906d 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -316,7 +316,7 @@ static int linux_wlan_txq_task(void *vp)
 
 	complete(&wl->txq_thread_started);
 	while (1) {
-		down(&wl->txq_event);
+		wait_for_completion(&wl->txq_event);
 
 		if (wl->close) {
 			complete(&wl->txq_thread_started);
@@ -650,7 +650,7 @@ void wilc1000_wlan_deinit(struct net_device *dev)
 			mutex_unlock(&wl->hif_cs);
 		}
 		if (&wl->txq_event)
-			up(&wl->txq_event);
+			wait_for_completion(&wl->txq_event);
 
 		wlan_deinitialize_threads(dev);
 		deinit_irq(dev);
@@ -681,7 +681,7 @@ static int wlan_init_locks(struct net_device *dev)
 	spin_lock_init(&wl->txq_spinlock);
 	sema_init(&wl->txq_add_to_head_cs, 1);
 
-	sema_init(&wl->txq_event, 0);
+	init_completion(&wl->txq_event);
 
 	sema_init(&wl->cfg_event, 0);
 	sema_init(&wl->sync_event, 0);
@@ -738,7 +738,7 @@ static void wlan_deinitialize_threads(struct net_device *dev)
 	wl->close = 1;
 
 	if (&wl->txq_event)
-		up(&wl->txq_event);
+		complete(&wl->txq_event);
 
 	if (wl->txq_thread) {
 		kthread_stop(wl->txq_thread);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 3a561df6..12d7c7b 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -42,6 +42,7 @@
 #include "host_interface.h"
 #include "wilc_wlan.h"
 #include <linux/wireless.h>
+#include <linux/completion.h>
 
 #define FLOW_CONTROL_LOWER_THRESHOLD	128
 #define FLOW_CONTROL_UPPER_THRESHOLD	256
@@ -178,7 +179,7 @@ struct wilc {
 
 	struct semaphore cfg_event;
 	struct semaphore sync_event;
-	struct semaphore txq_event;
+	struct completion txq_event;
 	struct completion txq_thread_started;
 
 	struct task_struct *txq_thread;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 11e16d5..1a57135 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1,3 +1,4 @@
+#include <linux/completion.h>
 #include "wilc_wlan_if.h"
 #include "wilc_wlan.h"
 #include "wilc_wfi_netdevice.h"
@@ -89,7 +90,7 @@ static void wilc_wlan_txq_add_to_tail(struct net_device *dev,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 }
 
 static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
@@ -119,7 +120,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 	up(&wilc->txq_add_to_head_cs);
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 
 	return 0;
 }
@@ -287,7 +288,8 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
 	spin_unlock_irqrestore(&wilc->txq_spinlock, wilc->txq_spinlock_flags);
 
 	while (dropped > 0) {
-		wilc_lock_timeout(wilc, &wilc->txq_event, 1);
+		wait_for_completion_timeout(&wilc->txq_event,
+						msecs_to_jiffies(1));
 		dropped--;
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422600 — [PATCH v3 4/5] staging: wilc1000: Replace semaphore sync_event with completion

The semaphore 'sync_event' is used as completion, so convert
it to a struct completion type. Also, return -ETIME if the return
value of wait_for_completion_timeout is 0.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/linux_wlan.c         | 10 +++++-----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 81a469a..39fe350 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -241,7 +241,7 @@ void wilc_mac_indicate(struct wilc *wilc, int flag)
 				      (unsigned char *)&status, 4);
 		if (wilc->mac_status == WILC_MAC_STATUS_INIT) {
 			wilc->mac_status = status;
-			up(&wilc->sync_event);
+			complete(&wilc->sync_event);
 		} else {
 			wilc->mac_status = status;
 		}
@@ -386,9 +386,9 @@ static int linux_wlan_start_firmware(struct net_device *dev)
 	if (ret < 0)
 		return ret;
 
-	ret = wilc_lock_timeout(wilc, &wilc->sync_event, 5000);
-	if (ret)
-		return ret;
+	if (!wait_for_completion_timeout(&wilc->sync_event,
+					msecs_to_jiffies(5000)))
+		return -ETIME;
 
 	return 0;
 }
@@ -684,7 +684,7 @@ static int wlan_init_locks(struct net_device *dev)
 	init_completion(&wl->txq_event);
 
 	init_completion(&wl->cfg_event);
-	sema_init(&wl->sync_event, 0);
+	init_completion(&wl->sync_event);
 	init_completion(&wl->txq_thread_started);
 
 	return 0;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 5fbc07c..5cc6a82 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -179,7 +179,7 @@ struct wilc {
 	struct mutex hif_cs;
 
 	struct completion cfg_event;
-	struct semaphore sync_event;
+	struct completion sync_event;
 	struct completion txq_event;
 	struct completion txq_thread_started;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422601 — [PATCH v3 0/5] *** staging: wilc1000: Replace semaphores with mutexes or completions ***

These are a set of patches [v3] which removes semaphores from:

drivers/staging/wilc1000

These are part of a bigger effort to eliminate all semaphores
from the linux kernel.

They build correctly (individually and as a whole).

NB: The changes are untested

Changes w.r.t. review comments on v1

1. Whitespace removed in patch 3
2. Removed semaphore 'close_exit_sync'
3. To rework on patch 6 and send in a seperate patch series

Binoy Jayan (5):
  staging: wilc1000: Replace semaphore txq_event with completion
  staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex
  staging: wilc1000: Replace semaphore cfg_event with completion
  staging: wilc1000: Replace semaphore sync_event with completion
  staging: wilc1000: Remove semaphore close_exit_sync

 drivers/staging/wilc1000/linux_wlan.c         | 31 ++++++++++--------------
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 10 ++++----
 drivers/staging/wilc1000/wilc_wlan.c          | 34 +++++++++++++--------------
 3 files changed, 35 insertions(+), 40 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422603 — [PATCH v3 5/5] staging: wilc1000: Remove semaphore close_exit_sync

The semaphore 'close_exit_sync' does not serve any purpose other
than delaying the deregistration of the device which it is trying
to protect from shared access. 'up' is called only when a subdevice
is closed and not when it is opened. So, the semaphore count only
goes up when the device is used.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/linux_wlan.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 39fe350..f87a30f 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -31,8 +31,6 @@ static struct notifier_block g_dev_notifier = {
 	.notifier_call = dev_state_ev_handler
 };
 
-static struct semaphore close_exit_sync;
-
 static int wlan_deinit_locks(struct net_device *dev);
 static void wlan_deinitialize_threads(struct net_device *dev);
 
@@ -1088,7 +1086,6 @@ int wilc_mac_close(struct net_device *ndev)
 		WILC_WFI_deinit_mon_interface();
 	}
 
-	up(&close_exit_sync);
 	vif->mac_opened = 0;
 
 	return 0;
@@ -1232,8 +1229,6 @@ void wilc_netdev_cleanup(struct wilc *wilc)
 	}
 
 	if (wilc && (wilc->vif[0]->ndev || wilc->vif[1]->ndev)) {
-		wilc_lock_timeout(wilc, &close_exit_sync, 5 * 1000);
-
 		for (i = 0; i < NUM_CONCURRENT_IFC; i++)
 			if (wilc->vif[i]->ndev)
 				if (vif[i]->mac_opened)
@@ -1258,8 +1253,6 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
 	struct net_device *ndev;
 	struct wilc *wl;
 
-	sema_init(&close_exit_sync, 0);
-
 	wl = kzalloc(sizeof(*wl), GFP_KERNEL);
 	if (!wl)
 		return -ENOMEM;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422604 — [PATCH v3 3/5] staging: wilc1000: Replace semaphore cfg_event with completion

The semaphore 'cfg_event' is used as completion, so convert
it to a struct completion type.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/linux_wlan.c         |  2 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 +-
 drivers/staging/wilc1000/wilc_wlan.c          | 15 ++++++++-------
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index a933551..81a469a 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -683,7 +683,7 @@ static int wlan_init_locks(struct net_device *dev)
 
 	init_completion(&wl->txq_event);
 
-	sema_init(&wl->cfg_event, 0);
+	init_completion(&wl->cfg_event);
 	sema_init(&wl->sync_event, 0);
 	init_completion(&wl->txq_thread_started);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 239cd43..5fbc07c 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -178,7 +178,7 @@ struct wilc {
 	struct mutex rxq_cs;
 	struct mutex hif_cs;
 
-	struct semaphore cfg_event;
+	struct completion cfg_event;
 	struct semaphore sync_event;
 	struct completion txq_event;
 	struct completion txq_thread_started;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 9afbe8d..19a5809 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -310,7 +310,7 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
 	netdev_dbg(vif->ndev, "Adding config packet ...\n");
 	if (wilc->quit) {
 		netdev_dbg(vif->ndev, "Return due to clear function\n");
-		up(&wilc->cfg_event);
+		complete(&wilc->cfg_event);
 		return 0;
 	}
 
@@ -769,7 +769,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
 
 	do {
 		if (wilc->quit) {
-			up(&wilc->cfg_event);
+			complete(&wilc->cfg_event);
 			break;
 		}
 		rqe = wilc_wlan_rxq_remove(wilc);
@@ -820,7 +820,7 @@ static void wilc_wlan_handle_rxq(struct wilc *wilc)
 					wilc_wlan_cfg_indicate_rx(wilc, &buffer[pkt_offset + offset], pkt_len, &rsp);
 					if (rsp.type == WILC_CFG_RSP) {
 						if (wilc->cfg_seq_no == rsp.seq_no)
-							up(&wilc->cfg_event);
+							complete(&wilc->cfg_event);
 					} else if (rsp.type == WILC_CFG_RSP_STATUS) {
 						wilc_mac_indicate(wilc, WILC_MAC_INDICATE_STATUS);
 
@@ -1228,11 +1228,12 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
 		if (wilc_wlan_cfg_commit(vif, WILC_CFG_SET, drv_handler))
 			ret_size = 0;
 
-		if (wilc_lock_timeout(wilc, &wilc->cfg_event,
-					    CFG_PKTS_TIMEOUT)) {
+		if (!wait_for_completion_timeout(&wilc->cfg_event,
+					msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
 			netdev_dbg(vif->ndev, "Set Timed Out\n");
 			ret_size = 0;
 		}
+
 		wilc->cfg_frame_in_use = 0;
 		wilc->cfg_frame_offset = 0;
 		wilc->cfg_seq_no += 1;
@@ -1265,8 +1266,8 @@ int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
 		if (wilc_wlan_cfg_commit(vif, WILC_CFG_QUERY, drv_handler))
 			ret_size = 0;
 
-		if (wilc_lock_timeout(wilc, &wilc->cfg_event,
-					    CFG_PKTS_TIMEOUT)) {
+		if (!wait_for_completion_timeout(&wilc->cfg_event,
+					msecs_to_jiffies(CFG_PKTS_TIMEOUT))) {
 			netdev_dbg(vif->ndev, "Get Timed Out\n");
 			ret_size = 0;
 		}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422606 — [PATCH v3 1/5] staging: wilc1000: Replace semaphore txq_event with completion

The semaphore 'txq_event' is used as completion, so convert it
to a struct completion type.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/linux_wlan.c         | 8 ++++----
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 3 ++-
 drivers/staging/wilc1000/wilc_wlan.c          | 8 +++++---
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 4f93c11..90f906d 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -316,7 +316,7 @@ static int linux_wlan_txq_task(void *vp)
 
 	complete(&wl->txq_thread_started);
 	while (1) {
-		down(&wl->txq_event);
+		wait_for_completion(&wl->txq_event);
 
 		if (wl->close) {
 			complete(&wl->txq_thread_started);
@@ -650,7 +650,7 @@ void wilc1000_wlan_deinit(struct net_device *dev)
 			mutex_unlock(&wl->hif_cs);
 		}
 		if (&wl->txq_event)
-			up(&wl->txq_event);
+			wait_for_completion(&wl->txq_event);
 
 		wlan_deinitialize_threads(dev);
 		deinit_irq(dev);
@@ -681,7 +681,7 @@ static int wlan_init_locks(struct net_device *dev)
 	spin_lock_init(&wl->txq_spinlock);
 	sema_init(&wl->txq_add_to_head_cs, 1);
 
-	sema_init(&wl->txq_event, 0);
+	init_completion(&wl->txq_event);
 
 	sema_init(&wl->cfg_event, 0);
 	sema_init(&wl->sync_event, 0);
@@ -738,7 +738,7 @@ static void wlan_deinitialize_threads(struct net_device *dev)
 	wl->close = 1;
 
 	if (&wl->txq_event)
-		up(&wl->txq_event);
+		complete(&wl->txq_event);
 
 	if (wl->txq_thread) {
 		kthread_stop(wl->txq_thread);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 3a561df6..12d7c7b 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -42,6 +42,7 @@
 #include "host_interface.h"
 #include "wilc_wlan.h"
 #include <linux/wireless.h>
+#include <linux/completion.h>
 
 #define FLOW_CONTROL_LOWER_THRESHOLD	128
 #define FLOW_CONTROL_UPPER_THRESHOLD	256
@@ -178,7 +179,7 @@ struct wilc {
 
 	struct semaphore cfg_event;
 	struct semaphore sync_event;
-	struct semaphore txq_event;
+	struct completion txq_event;
 	struct completion txq_thread_started;
 
 	struct task_struct *txq_thread;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 11e16d5..1a57135 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1,3 +1,4 @@
+#include <linux/completion.h>
 #include "wilc_wlan_if.h"
 #include "wilc_wlan.h"
 #include "wilc_wfi_netdevice.h"
@@ -89,7 +90,7 @@ static void wilc_wlan_txq_add_to_tail(struct net_device *dev,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 }
 
 static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
@@ -119,7 +120,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 	up(&wilc->txq_add_to_head_cs);
-	up(&wilc->txq_event);
+	complete(&wilc->txq_event);
 
 	return 0;
 }
@@ -287,7 +288,8 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
 	spin_unlock_irqrestore(&wilc->txq_spinlock, wilc->txq_spinlock_flags);
 
 	while (dropped > 0) {
-		wilc_lock_timeout(wilc, &wilc->txq_event, 1);
+		wait_for_completion_timeout(&wilc->txq_event,
+						msecs_to_jiffies(1));
 		dropped--;
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1422607 — [PATCH v3 2/5] staging: wilc1000: Replace semaphore txq_add_to_head_cs with mutex

The semaphore 'txq_add_to_head_cs' is a simple mutex, so it should be
written as one. Semaphores are going away in the future. Also, removing
the timeout scenario as the error handling code does not propagate the
timeout properly.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/linux_wlan.c         |  4 ++--
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  3 ++-
 drivers/staging/wilc1000/wilc_wlan.c          | 11 ++++-------
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 90f906d..a933551 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -20,7 +20,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/skbuff.h>
-
+#include <linux/mutex.h>
 #include <linux/semaphore.h>
 #include <linux/completion.h>
 
@@ -679,7 +679,7 @@ static int wlan_init_locks(struct net_device *dev)
 	mutex_init(&wl->rxq_cs);
 
 	spin_lock_init(&wl->txq_spinlock);
-	sema_init(&wl->txq_add_to_head_cs, 1);
+	mutex_init(&wl->txq_add_to_head_cs);
 
 	init_completion(&wl->txq_event);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 12d7c7b..239cd43 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -43,6 +43,7 @@
 #include "wilc_wlan.h"
 #include <linux/wireless.h>
 #include <linux/completion.h>
+#include <linux/mutex.h>
 
 #define FLOW_CONTROL_LOWER_THRESHOLD	128
 #define FLOW_CONTROL_UPPER_THRESHOLD	256
@@ -171,7 +172,7 @@ struct wilc {
 	struct wilc_vif *vif[NUM_CONCURRENT_IFC];
 	u8 open_ifcs;
 
-	struct semaphore txq_add_to_head_cs;
+	struct mutex txq_add_to_head_cs;
 	spinlock_t txq_spinlock;
 
 	struct mutex rxq_cs;
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index 1a57135..9afbe8d 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -99,9 +99,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 	unsigned long flags;
 	struct wilc *wilc = vif->wilc;
 
-	if (wilc_lock_timeout(wilc, &wilc->txq_add_to_head_cs,
-				    CFG_PKTS_TIMEOUT))
-		return -1;
+	mutex_lock(&wilc->txq_add_to_head_cs);
 
 	spin_lock_irqsave(&wilc->txq_spinlock, flags);
 
@@ -119,7 +117,7 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
 	wilc->txq_entries += 1;
 
 	spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
-	up(&wilc->txq_add_to_head_cs);
+	mutex_unlock(&wilc->txq_add_to_head_cs);
 	complete(&wilc->txq_event);
 
 	return 0;
@@ -573,8 +571,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 		if (wilc->quit)
 			break;
 
-		wilc_lock_timeout(wilc, &wilc->txq_add_to_head_cs,
-					CFG_PKTS_TIMEOUT);
+		mutex_lock(&wilc->txq_add_to_head_cs);
 		wilc_wlan_txq_filter_dup_tcp_ack(dev);
 		tqe = wilc_wlan_txq_get_first(wilc);
 		i = 0;
@@ -755,7 +752,7 @@ _end_:
 		if (ret != 1)
 			break;
 	} while (0);
-	up(&wilc->txq_add_to_head_cs);
+	mutex_unlock(&wilc->txq_add_to_head_cs);
 
 	wilc->txq_exit = 1;
 	*txq_count = wilc->txq_entries;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1426459 — [PATCH v2 1/2] staging: wilc1000: message_queue: Move code to host interface

Move the contents of wilc_msgqueue.c and wilc_msgqueue.h into
host_interface.c, remove 'wilc_msgqueue.c' and 'wilc_msgqueue.h'.
This is done so as to restructure the implementation of the kthread
'hostIFthread' using a work queue.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/Makefile         |   1 -
 drivers/staging/wilc1000/host_interface.c | 163 +++++++++++++++++++++++++++++-
 drivers/staging/wilc1000/wilc_msgqueue.c  | 144 --------------------------
 drivers/staging/wilc1000/wilc_msgqueue.h  |  28 -----
 4 files changed, 162 insertions(+), 174 deletions(-)
 delete mode 100644 drivers/staging/wilc1000/wilc_msgqueue.c
 delete mode 100644 drivers/staging/wilc1000/wilc_msgqueue.h

diff --git a/drivers/staging/wilc1000/Makefile b/drivers/staging/wilc1000/Makefile
index acc3f3e..d226283 100644
--- a/drivers/staging/wilc1000/Makefile
+++ b/drivers/staging/wilc1000/Makefile
@@ -6,7 +6,6 @@ ccflags-y += -DFIRMWARE_1002=\"atmel/wilc1002_firmware.bin\" \
 ccflags-y += -I$(src)/ -DWILC_ASIC_A0 -DWILC_DEBUGFS
 
 wilc1000-objs := wilc_wfi_cfgoperations.o linux_wlan.o linux_mon.o \
-			wilc_msgqueue.o \
 			coreconfigurator.o host_interface.o \
 			wilc_wlan_cfg.o wilc_debugfs.o \
 			wilc_wlan.o
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 9535842..494345b 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3,11 +3,13 @@
 #include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/completion.h>
+#include <linux/list.h>
 #include "host_interface.h"
+#include <linux/spinlock.h>
+#include <linux/errno.h>
 #include "coreconfigurator.h"
 #include "wilc_wlan.h"
 #include "wilc_wlan_if.h"
-#include "wilc_msgqueue.h"
 #include <linux/etherdevice.h>
 #include "wilc_wfi_netdevice.h"
 
@@ -57,6 +59,20 @@
 #define TCP_ACK_FILTER_LINK_SPEED_THRESH	54
 #define DEFAULT_LINK_SPEED			72
 
+struct message {
+	void *buf;
+	u32 len;
+	struct list_head list;
+};
+
+struct message_queue {
+	struct semaphore sem;
+	spinlock_t lock;
+	bool exiting;
+	u32 recv_count;
+	struct list_head msg_list;
+};
+
 struct host_if_wpa_attr {
 	u8 *key;
 	const u8 *mac_addr;
@@ -264,6 +280,151 @@ static struct wilc_vif *join_req_vif;
 static void *host_int_ParseJoinBssParam(struct network_info *ptstrNetworkInfo);
 static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
 static s32 Handle_ScanDone(struct wilc_vif *vif, enum scan_event enuEvent);
+static int wilc_mq_create(struct message_queue *mq);
+static int wilc_mq_send(struct message_queue *mq,
+		 const void *send_buf, u32 send_buf_size);
+static int wilc_mq_recv(struct message_queue *mq,
+		 void *recv_buf, u32 recv_buf_size, u32 *recv_len);
+static int wilc_mq_destroy(struct message_queue *mq);
+
+/*!
+ *  @author		syounan
+ *  @date		1 Sep 2010
+ *  @note		copied from FLO glue implementatuion
+ *  @version		1.0
+ */
+static int wilc_mq_create(struct message_queue *mq)
+{
+	spin_lock_init(&mq->lock);
+	sema_init(&mq->sem, 0);
+	INIT_LIST_HEAD(&mq->msg_list);
+	mq->recv_count = 0;
+	mq->exiting = false;
+	return 0;
+}
+
+/*!
+ *  @author		syounan
+ *  @date		1 Sep 2010
+ *  @note		copied from FLO glue implementatuion
+ *  @version		1.0
+ */
+static int wilc_mq_destroy(struct message_queue *mq)
+{
+	struct message *msg;
+
+	mq->exiting = true;
+
+	/* Release any waiting receiver thread. */
+	while (mq->recv_count > 0) {
+		up(&mq->sem);
+		mq->recv_count--;
+	}
+
+	while (!list_empty(&mq->msg_list)) {
+		msg = list_first_entry(&mq->msg_list, struct message, list);
+		list_del(&msg->list);
+		kfree(msg->buf);
+	}
+
+	return 0;
+}
+
+/*!
+ *  @author		syounan
+ *  @date		1 Sep 2010
+ *  @note		copied from FLO glue implementatuion
+ *  @version		1.0
+ */
+static int wilc_mq_send(struct message_queue *mq,
+		 const void *send_buf, u32 send_buf_size)
+{
+	unsigned long flags;
+	struct message *new_msg = NULL;
+
+	if (!mq || (send_buf_size == 0) || !send_buf)
+		return -EINVAL;
+
+	if (mq->exiting)
+		return -EFAULT;
+
+	/* construct a new message */
+	new_msg = kmalloc(sizeof(*new_msg), GFP_ATOMIC);
+	if (!new_msg)
+		return -ENOMEM;
+
+	new_msg->len = send_buf_size;
+	INIT_LIST_HEAD(&new_msg->list);
+	new_msg->buf = kmemdup(send_buf, send_buf_size, GFP_ATOMIC);
+	if (!new_msg->buf) {
+		kfree(new_msg);
+		return -ENOMEM;
+	}
+
+	spin_lock_irqsave(&mq->lock, flags);
+
+	/* add it to the message queue */
+	list_add_tail(&new_msg->list, &mq->msg_list);
+
+	spin_unlock_irqrestore(&mq->lock, flags);
+
+	up(&mq->sem);
+
+	return 0;
+}
+
+/*!
+ *  @author		syounan
+ *  @date		1 Sep 2010
+ *  @note		copied from FLO glue implementatuion
+ *  @version		1.0
+ */
+static int wilc_mq_recv(struct message_queue *mq,
+		 void *recv_buf, u32 recv_buf_size, u32 *recv_len)
+{
+	struct message *msg;
+	unsigned long flags;
+
+	if (!mq || (recv_buf_size == 0) || !recv_buf || !recv_len)
+		return -EINVAL;
+
+	if (mq->exiting)
+		return -EFAULT;
+
+	spin_lock_irqsave(&mq->lock, flags);
+	mq->recv_count++;
+	spin_unlock_irqrestore(&mq->lock, flags);
+
+	down(&mq->sem);
+	spin_lock_irqsave(&mq->lock, flags);
+
+	if (list_empty(&mq->msg_list)) {
+		spin_unlock_irqrestore(&mq->lock, flags);
+		up(&mq->sem);
+		return -EFAULT;
+	}
+	/* check buffer size */
+	msg = list_first_entry(&mq->msg_list, struct message, list);
+	if (recv_buf_size < msg->len) {
+		spin_unlock_irqrestore(&mq->lock, flags);
+		up(&mq->sem);
+		return -EOVERFLOW;
+	}
+
+	/* consume the message */
+	mq->recv_count--;
+	memcpy(recv_buf, msg->buf, msg->len);
+	*recv_len = msg->len;
+
+	list_del(&msg->list);
+
+	kfree(msg->buf);
+	kfree(msg);
+
+	spin_unlock_irqrestore(&mq->lock, flags);
+
+	return 0;
+}
 
 /* The u8IfIdx starts from 0 to NUM_CONCURRENT_IFC -1, but 0 index used as
  * special purpose in wilc device, so we add 1 to the index to starts from 1.
diff --git a/drivers/staging/wilc1000/wilc_msgqueue.c b/drivers/staging/wilc1000/wilc_msgqueue.c
deleted file mode 100644
index 6cb894e..0000000
--- a/drivers/staging/wilc1000/wilc_msgqueue.c
+++ /dev/null
@@ -1,144 +0,0 @@
-
-#include "wilc_msgqueue.h"
-#include <linux/spinlock.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-int wilc_mq_create(struct message_queue *mq)
-{
-	spin_lock_init(&mq->lock);
-	sema_init(&mq->sem, 0);
-	INIT_LIST_HEAD(&mq->msg_list);
-	mq->recv_count = 0;
-	mq->exiting = false;
-	return 0;
-}
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-int wilc_mq_destroy(struct message_queue *mq)
-{
-	struct message *msg;
-
-	mq->exiting = true;
-
-	/* Release any waiting receiver thread. */
-	while (mq->recv_count > 0) {
-		up(&mq->sem);
-		mq->recv_count--;
-	}
-
-	while (!list_empty(&mq->msg_list)) {
-		msg = list_first_entry(&mq->msg_list, struct message, list);
-		list_del(&msg->list);
-		kfree(msg->buf);
-	}
-
-	return 0;
-}
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-int wilc_mq_send(struct message_queue *mq,
-		 const void *send_buf, u32 send_buf_size)
-{
-	unsigned long flags;
-	struct message *new_msg = NULL;
-
-	if (!mq || (send_buf_size == 0) || !send_buf)
-		return -EINVAL;
-
-	if (mq->exiting)
-		return -EFAULT;
-
-	/* construct a new message */
-	new_msg = kmalloc(sizeof(*new_msg), GFP_ATOMIC);
-	if (!new_msg)
-		return -ENOMEM;
-
-	new_msg->len = send_buf_size;
-	INIT_LIST_HEAD(&new_msg->list);
-	new_msg->buf = kmemdup(send_buf, send_buf_size, GFP_ATOMIC);
-	if (!new_msg->buf) {
-		kfree(new_msg);
-		return -ENOMEM;
-	}
-
-	spin_lock_irqsave(&mq->lock, flags);
-
-	/* add it to the message queue */
-	list_add_tail(&new_msg->list, &mq->msg_list);
-
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	up(&mq->sem);
-
-	return 0;
-}
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-int wilc_mq_recv(struct message_queue *mq,
-		 void *recv_buf, u32 recv_buf_size, u32 *recv_len)
-{
-	struct message *msg;
-	unsigned long flags;
-
-	if (!mq || (recv_buf_size == 0) || !recv_buf || !recv_len)
-		return -EINVAL;
-
-	if (mq->exiting)
-		return -EFAULT;
-
-	spin_lock_irqsave(&mq->lock, flags);
-	mq->recv_count++;
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	down(&mq->sem);
-	spin_lock_irqsave(&mq->lock, flags);
-
-	if (list_empty(&mq->msg_list)) {
-		spin_unlock_irqrestore(&mq->lock, flags);
-		up(&mq->sem);
-		return -EFAULT;
-	}
-	/* check buffer size */
-	msg = list_first_entry(&mq->msg_list, struct message, list);
-	if (recv_buf_size < msg->len) {
-		spin_unlock_irqrestore(&mq->lock, flags);
-		up(&mq->sem);
-		return -EOVERFLOW;
-	}
-
-	/* consume the message */
-	mq->recv_count--;
-	memcpy(recv_buf, msg->buf, msg->len);
-	*recv_len = msg->len;
-
-	list_del(&msg->list);
-
-	kfree(msg->buf);
-	kfree(msg);
-
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	return 0;
-}
diff --git a/drivers/staging/wilc1000/wilc_msgqueue.h b/drivers/staging/wilc1000/wilc_msgqueue.h
deleted file mode 100644
index 846a484..0000000
--- a/drivers/staging/wilc1000/wilc_msgqueue.h
+++ /dev/null
@@ -1,28 +0,0 @@
-#ifndef __WILC_MSG_QUEUE_H__
-#define __WILC_MSG_QUEUE_H__
-
-#include <linux/semaphore.h>
-#include <linux/list.h>
-
-struct message {
-	void *buf;
-	u32 len;
-	struct list_head list;
-};
-
-struct message_queue {
-	struct semaphore sem;
-	spinlock_t lock;
-	bool exiting;
-	u32 recv_count;
-	struct list_head msg_list;
-};
-
-int wilc_mq_create(struct message_queue *mq);
-int wilc_mq_send(struct message_queue *mq,
-		 const void *send_buf, u32 send_buf_size);
-int wilc_mq_recv(struct message_queue *mq,
-		 void *recv_buf, u32 recv_buf_size, u32 *recv_len);
-int wilc_mq_destroy(struct message_queue *mq);
-
-#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1426463 — [PATCH v2 0/2] *** staging: wilc1000: Replace semaphores ***

This is the second patch series for 'wilc1000'. The original patch series
consisted 7 patches of which only the first 5 are good. The patch 6 and 7
are being worked on in this series in a different way.

This patch series removes the semaphore 'sem' in 'wilc1000' and also
restructures the implementation of kthread / message_queue logic with
a create_singlethread_workqueue() / queue_work() setup.

These are part of a bigger effort to eliminate all semaphores
from the linux kernel.

They build correctly (individually and as a whole).

NB: The changes are untested

Rework on the review comments by Arnd w.r.t. v1 for patch 2:

struct message_queue can be removed since
 - after the workqueue conversion, mq->sem is no longer needed
 - recv_count is not needed, it just counts the number of entries in the list
 - struct wilc' pointer can be retrieved from the host_if_msg, (vif->wilc)
 - the message list is not needed because we always look only at the
   first entry, except in wilc_mq_destroy(), but it would be better
   to just call destroy_workqueue(), which also drains the remaining work.
 - the exiting flag is also handled by destroy_workqueue()   
 - with everything else gone, the spinlock is also not needed any more.

Do 'kfree' only at the end of 'host_if_work' 

wilc_initialized is always '1' so the conditional 'wilc_mq_send'
in 'hostIFthread' can be removed.

A connect command (HOST_IF_MSG_CONNECT) does not complete while scan is ongoing. 
So, the special handling of this command needs to be preserved.

Use create_singlethread_workqueue() instead of alloc_workqueue(), so that
we stay closer to the current behavior by having the thread run only
on one CPU at a time and not having a 'dedicated' thread for each.

Split the patch to seperate interface changes to 'wilc_mq_send'
    No easy way found to split the patch to change the interface
    'wilc_mq_send' and to 'wilc_enqueue_cmd' as the parameters 
    'mq' 'send_buf' and 'send_buf_size' itself are part of the message
    queue implementation.


Binoy Jayan (2):
  staging: wilc1000: message_queue: Move code to host interface
  staging: wilc1000: Replace kthread with workqueue for host interface

 drivers/staging/wilc1000/Makefile         |   1 -
 drivers/staging/wilc1000/TODO             |   5 +
 drivers/staging/wilc1000/host_interface.c | 417 +++++++++++++++---------------
 drivers/staging/wilc1000/wilc_msgqueue.c  | 144 -----------
 drivers/staging/wilc1000/wilc_msgqueue.h  |  28 --
 5 files changed, 216 insertions(+), 379 deletions(-)
 delete mode 100644 drivers/staging/wilc1000/wilc_msgqueue.c
 delete mode 100644 drivers/staging/wilc1000/wilc_msgqueue.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1426465 — [PATCH v2 2/2] staging: wilc1000: Replace kthread with workqueue for host interface

Deconstruct the kthread / message_queue logic, replacing it with
create_singlethread_workqueue() / queue_work() setup, by adding a
'struct work_struct' to 'struct host_if_msg'. The current kthread
hostIFthread() is converted to a work queue helper with the name
'host_if_work'.

Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/wilc1000/TODO             |   5 +
 drivers/staging/wilc1000/host_interface.c | 542 +++++++++++-------------------
 2 files changed, 198 insertions(+), 349 deletions(-)

diff --git a/drivers/staging/wilc1000/TODO b/drivers/staging/wilc1000/TODO
index 95199d8..ec93b2e 100644
--- a/drivers/staging/wilc1000/TODO
+++ b/drivers/staging/wilc1000/TODO
@@ -4,6 +4,11 @@ TODO:
 - remove custom debug and tracing functions
 - rework comments and function headers(also coding style)
 - replace all semaphores with mutexes or completions
+- Move handling for each individual members of 'union message_body' out
+  into a separate 'struct work_struct' and completely remove the multiplexer
+  that is currently part of host_if_work(), allowing movement of the
+  implementation of each message handler into the callsite of the function
+  that currently queues the 'host_if_msg'.
 - make spi and sdio components coexist in one build
 - turn compile-time platform configuration (BEAGLE_BOARD,
   PANDA_BOARD, PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 494345b..92d4561 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -4,6 +4,7 @@
 #include <linux/delay.h>
 #include <linux/completion.h>
 #include <linux/list.h>
+#include <linux/workqueue.h>
 #include "host_interface.h"
 #include <linux/spinlock.h>
 #include <linux/errno.h>
@@ -59,20 +60,6 @@
 #define TCP_ACK_FILTER_LINK_SPEED_THRESH	54
 #define DEFAULT_LINK_SPEED			72
 
-struct message {
-	void *buf;
-	u32 len;
-	struct list_head list;
-};
-
-struct message_queue {
-	struct semaphore sem;
-	spinlock_t lock;
-	bool exiting;
-	u32 recv_count;
-	struct list_head msg_list;
-};
-
 struct host_if_wpa_attr {
 	u8 *key;
 	const u8 *mac_addr;
@@ -211,6 +198,7 @@ struct host_if_msg {
 	u16 id;
 	union message_body body;
 	struct wilc_vif *vif;
+	struct work_struct work;
 };
 
 struct join_bss_param {
@@ -245,8 +233,7 @@ struct join_bss_param {
 static struct host_if_drv *terminated_handle;
 bool wilc_optaining_ip;
 static u8 P2P_LISTEN_STATE;
-static struct task_struct *hif_thread_handler;
-static struct message_queue hif_msg_q;
+static struct workqueue_struct *hif_workqueue;
 static struct completion hif_thread_comp;
 static struct completion hif_driver_comp;
 static struct completion hif_wait_response;
@@ -280,55 +267,8 @@ static struct wilc_vif *join_req_vif;
 static void *host_int_ParseJoinBssParam(struct network_info *ptstrNetworkInfo);
 static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
 static s32 Handle_ScanDone(struct wilc_vif *vif, enum scan_event enuEvent);
-static int wilc_mq_create(struct message_queue *mq);
-static int wilc_mq_send(struct message_queue *mq,
-		 const void *send_buf, u32 send_buf_size);
-static int wilc_mq_recv(struct message_queue *mq,
-		 void *recv_buf, u32 recv_buf_size, u32 *recv_len);
-static int wilc_mq_destroy(struct message_queue *mq);
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-static int wilc_mq_create(struct message_queue *mq)
-{
-	spin_lock_init(&mq->lock);
-	sema_init(&mq->sem, 0);
-	INIT_LIST_HEAD(&mq->msg_list);
-	mq->recv_count = 0;
-	mq->exiting = false;
-	return 0;
-}
-
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-static int wilc_mq_destroy(struct message_queue *mq)
-{
-	struct message *msg;
-
-	mq->exiting = true;
-
-	/* Release any waiting receiver thread. */
-	while (mq->recv_count > 0) {
-		up(&mq->sem);
-		mq->recv_count--;
-	}
-
-	while (!list_empty(&mq->msg_list)) {
-		msg = list_first_entry(&mq->msg_list, struct message, list);
-		list_del(&msg->list);
-		kfree(msg->buf);
-	}
-
-	return 0;
-}
+static int wilc_enqueue_cmd(struct host_if_msg *msg);
+static void host_if_work(struct work_struct *work);
 
 /*!
  *  @author		syounan
@@ -336,95 +276,19 @@ static int wilc_mq_destroy(struct message_queue *mq)
  *  @note		copied from FLO glue implementatuion
  *  @version		1.0
  */
-static int wilc_mq_send(struct message_queue *mq,
-		 const void *send_buf, u32 send_buf_size)
+static int wilc_enqueue_cmd(struct host_if_msg *msg)
 {
-	unsigned long flags;
-	struct message *new_msg = NULL;
-
-	if (!mq || (send_buf_size == 0) || !send_buf)
-		return -EINVAL;
-
-	if (mq->exiting)
-		return -EFAULT;
+	struct host_if_msg *new_msg;
 
-	/* construct a new message */
-	new_msg = kmalloc(sizeof(*new_msg), GFP_ATOMIC);
+	new_msg = kmemdup(msg, sizeof(*new_msg), GFP_ATOMIC);
 	if (!new_msg)
 		return -ENOMEM;
 
-	new_msg->len = send_buf_size;
-	INIT_LIST_HEAD(&new_msg->list);
-	new_msg->buf = kmemdup(send_buf, send_buf_size, GFP_ATOMIC);
-	if (!new_msg->buf) {
-		kfree(new_msg);
-		return -ENOMEM;
-	}
-
-	spin_lock_irqsave(&mq->lock, flags);
-
-	/* add it to the message queue */
-	list_add_tail(&new_msg->list, &mq->msg_list);
-
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	up(&mq->sem);
-
+	INIT_WORK(&new_msg->work, host_if_work);
+	queue_work(hif_workqueue, &new_msg->work);
 	return 0;
 }
 
-/*!
- *  @author		syounan
- *  @date		1 Sep 2010
- *  @note		copied from FLO glue implementatuion
- *  @version		1.0
- */
-static int wilc_mq_recv(struct message_queue *mq,
-		 void *recv_buf, u32 recv_buf_size, u32 *recv_len)
-{
-	struct message *msg;
-	unsigned long flags;
-
-	if (!mq || (recv_buf_size == 0) || !recv_buf || !recv_len)
-		return -EINVAL;
-
-	if (mq->exiting)
-		return -EFAULT;
-
-	spin_lock_irqsave(&mq->lock, flags);
-	mq->recv_count++;
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	down(&mq->sem);
-	spin_lock_irqsave(&mq->lock, flags);
-
-	if (list_empty(&mq->msg_list)) {
-		spin_unlock_irqrestore(&mq->lock, flags);
-		up(&mq->sem);
-		return -EFAULT;
-	}
-	/* check buffer size */
-	msg = list_first_entry(&mq->msg_list, struct message, list);
-	if (recv_buf_size < msg->len) {
-		spin_unlock_irqrestore(&mq->lock, flags);
-		up(&mq->sem);
-		return -EOVERFLOW;
-	}
-
-	/* consume the message */
-	mq->recv_count--;
-	memcpy(recv_buf, msg->buf, msg->len);
-	*recv_len = msg->len;
-
-	list_del(&msg->list);
-
-	kfree(msg->buf);
-	kfree(msg);
-
-	spin_unlock_irqrestore(&mq->lock, flags);
-
-	return 0;
-}
 
 /* The u8IfIdx starts from 0 to NUM_CONCURRENT_IFC -1, but 0 index used as
  * special purpose in wilc device, so we add 1 to the index to starts from 1.
@@ -2525,9 +2389,9 @@ static void ListenTimerCB(unsigned long arg)
 	msg.vif = vif;
 	msg.body.remain_on_ch.id = vif->hif_drv->remain_on_ch.id;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 }
 
 static void Handle_PowerManagement(struct wilc_vif *vif,
@@ -2625,187 +2489,173 @@ static void handle_get_tx_pwr(struct wilc_vif *vif, u8 *tx_pwr)
 	complete(&hif_wait_response);
 }
 
-static int hostIFthread(void *pvArg)
+static void host_if_work(struct work_struct *work)
 {
-	u32 u32Ret;
-	struct host_if_msg msg;
-	struct wilc *wilc = pvArg;
-	struct wilc_vif *vif;
-
-	memset(&msg, 0, sizeof(struct host_if_msg));
+	struct host_if_msg *msg;
+	struct wilc *wilc;
 
-	while (1) {
-		wilc_mq_recv(&hif_msg_q, &msg, sizeof(struct host_if_msg), &u32Ret);
-		vif = msg.vif;
-		if (msg.id == HOST_IF_MSG_EXIT)
-			break;
+	msg = container_of(work, struct host_if_msg, work);
+	wilc = msg->vif->wilc;
 
-		if ((!wilc_initialized)) {
-			usleep_range(200 * 1000, 200 * 1000);
-			wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
-			continue;
-		}
-
-		if (msg.id == HOST_IF_MSG_CONNECT &&
-		    vif->hif_drv->usr_scan_req.scan_result) {
-			wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
-			usleep_range(2 * 1000, 2 * 1000);
-			continue;
-		}
+	if (msg->id == HOST_IF_MSG_CONNECT &&
+	    msg->vif->hif_drv->usr_scan_req.scan_result) {
+		wilc_enqueue_cmd(msg);
+		usleep_range(2 * 1000, 2 * 1000);
+		goto end;
+	}
 
-		switch (msg.id) {
-		case HOST_IF_MSG_SCAN:
-			handle_scan(msg.vif, &msg.body.scan_info);
-			break;
+	switch (msg->id) {
+	case HOST_IF_MSG_SCAN:
+		handle_scan(msg->vif, &msg->body.scan_info);
+		break;
 
-		case HOST_IF_MSG_CONNECT:
-			Handle_Connect(msg.vif, &msg.body.con_info);
-			break;
+	case HOST_IF_MSG_CONNECT:
+		Handle_Connect(msg->vif, &msg->body.con_info);
+		break;
 
-		case HOST_IF_MSG_RCVD_NTWRK_INFO:
-			Handle_RcvdNtwrkInfo(msg.vif, &msg.body.net_info);
-			break;
+	case HOST_IF_MSG_RCVD_NTWRK_INFO:
+		Handle_RcvdNtwrkInfo(msg->vif, &msg->body.net_info);
+		break;
 
-		case HOST_IF_MSG_RCVD_GNRL_ASYNC_INFO:
-			Handle_RcvdGnrlAsyncInfo(vif,
-						 &msg.body.async_info);
-			break;
+	case HOST_IF_MSG_RCVD_GNRL_ASYNC_INFO:
+		Handle_RcvdGnrlAsyncInfo(msg->vif,
+					 &msg->body.async_info);
+		break;
 
-		case HOST_IF_MSG_KEY:
-			Handle_Key(msg.vif, &msg.body.key_info);
-			break;
+	case HOST_IF_MSG_KEY:
+		Handle_Key(msg->vif, &msg->body.key_info);
+		break;
 
-		case HOST_IF_MSG_CFG_PARAMS:
-			handle_cfg_param(msg.vif, &msg.body.cfg_info);
-			break;
+	case HOST_IF_MSG_CFG_PARAMS:
+		handle_cfg_param(msg->vif, &msg->body.cfg_info);
+		break;
 
-		case HOST_IF_MSG_SET_CHANNEL:
-			handle_set_channel(msg.vif, &msg.body.channel_info);
-			break;
+	case HOST_IF_MSG_SET_CHANNEL:
+		handle_set_channel(msg->vif, &msg->body.channel_info);
+		break;
 
-		case HOST_IF_MSG_DISCONNECT:
-			Handle_Disconnect(msg.vif);
-			break;
+	case HOST_IF_MSG_DISCONNECT:
+		Handle_Disconnect(msg->vif);
+		break;
 
-		case HOST_IF_MSG_RCVD_SCAN_COMPLETE:
-			del_timer(&vif->hif_drv->scan_timer);
+	case HOST_IF_MSG_RCVD_SCAN_COMPLETE:
+		del_timer(&msg->vif->hif_drv->scan_timer);
 
-			if (!wilc_wlan_get_num_conn_ifcs(wilc))
-				wilc_chip_sleep_manually(wilc);
+		if (!wilc_wlan_get_num_conn_ifcs(wilc))
+			wilc_chip_sleep_manually(wilc);
 
-			Handle_ScanDone(msg.vif, SCAN_EVENT_DONE);
+		Handle_ScanDone(msg->vif, SCAN_EVENT_DONE);
 
-			if (vif->hif_drv->remain_on_ch_pending)
-				Handle_RemainOnChan(msg.vif,
-						    &msg.body.remain_on_ch);
+		if (msg->vif->hif_drv->remain_on_ch_pending)
+			Handle_RemainOnChan(msg->vif,
+					    &msg->body.remain_on_ch);
 
-			break;
+		break;
 
-		case HOST_IF_MSG_GET_RSSI:
-			Handle_GetRssi(msg.vif);
-			break;
+	case HOST_IF_MSG_GET_RSSI:
+		Handle_GetRssi(msg->vif);
+		break;
 
-		case HOST_IF_MSG_GET_STATISTICS:
-			Handle_GetStatistics(msg.vif,
-					     (struct rf_info *)msg.body.data);
-			break;
+	case HOST_IF_MSG_GET_STATISTICS:
+		Handle_GetStatistics(msg->vif,
+				     (struct rf_info *)msg->body.data);
+		break;
 
-		case HOST_IF_MSG_ADD_BEACON:
-			Handle_AddBeacon(msg.vif, &msg.body.beacon_info);
-			break;
+	case HOST_IF_MSG_ADD_BEACON:
+		Handle_AddBeacon(msg->vif, &msg->body.beacon_info);
+		break;
 
-		case HOST_IF_MSG_DEL_BEACON:
-			Handle_DelBeacon(msg.vif);
-			break;
+	case HOST_IF_MSG_DEL_BEACON:
+		Handle_DelBeacon(msg->vif);
+		break;
 
-		case HOST_IF_MSG_ADD_STATION:
-			Handle_AddStation(msg.vif, &msg.body.add_sta_info);
-			break;
+	case HOST_IF_MSG_ADD_STATION:
+		Handle_AddStation(msg->vif, &msg->body.add_sta_info);
+		break;
 
-		case HOST_IF_MSG_DEL_STATION:
-			Handle_DelStation(msg.vif, &msg.body.del_sta_info);
-			break;
+	case HOST_IF_MSG_DEL_STATION:
+		Handle_DelStation(msg->vif, &msg->body.del_sta_info);
+		break;
 
-		case HOST_IF_MSG_EDIT_STATION:
-			Handle_EditStation(msg.vif, &msg.body.edit_sta_info);
-			break;
+	case HOST_IF_MSG_EDIT_STATION:
+		Handle_EditStation(msg->vif, &msg->body.edit_sta_info);
+		break;
 
-		case HOST_IF_MSG_GET_INACTIVETIME:
-			Handle_Get_InActiveTime(msg.vif, &msg.body.mac_info);
-			break;
+	case HOST_IF_MSG_GET_INACTIVETIME:
+		Handle_Get_InActiveTime(msg->vif, &msg->body.mac_info);
+		break;
 
-		case HOST_IF_MSG_SCAN_TIMER_FIRED:
+	case HOST_IF_MSG_SCAN_TIMER_FIRED:
 
-			Handle_ScanDone(msg.vif, SCAN_EVENT_ABORTED);
-			break;
+		Handle_ScanDone(msg->vif, SCAN_EVENT_ABORTED);
+		break;
 
-		case HOST_IF_MSG_CONNECT_TIMER_FIRED:
-			Handle_ConnectTimeout(msg.vif);
-			break;
+	case HOST_IF_MSG_CONNECT_TIMER_FIRED:
+		Handle_ConnectTimeout(msg->vif);
+		break;
 
-		case HOST_IF_MSG_POWER_MGMT:
-			Handle_PowerManagement(msg.vif,
-					       &msg.body.pwr_mgmt_info);
-			break;
+	case HOST_IF_MSG_POWER_MGMT:
+		Handle_PowerManagement(msg->vif,
+				       &msg->body.pwr_mgmt_info);
+		break;
 
-		case HOST_IF_MSG_SET_WFIDRV_HANDLER:
-			handle_set_wfi_drv_handler(msg.vif, &msg.body.drv);
-			break;
+	case HOST_IF_MSG_SET_WFIDRV_HANDLER:
+		handle_set_wfi_drv_handler(msg->vif, &msg->body.drv);
+		break;
 
-		case HOST_IF_MSG_SET_OPERATION_MODE:
-			handle_set_operation_mode(msg.vif, &msg.body.mode);
-			break;
+	case HOST_IF_MSG_SET_OPERATION_MODE:
+		handle_set_operation_mode(msg->vif, &msg->body.mode);
+		break;
 
-		case HOST_IF_MSG_SET_IPADDRESS:
-			handle_set_ip_address(vif,
-					      msg.body.ip_info.ip_addr,
-					      msg.body.ip_info.idx);
-			break;
+	case HOST_IF_MSG_SET_IPADDRESS:
+		handle_set_ip_address(msg->vif,
+				      msg->body.ip_info.ip_addr,
+				      msg->body.ip_info.idx);
+		break;
 
-		case HOST_IF_MSG_GET_IPADDRESS:
-			handle_get_ip_address(vif, msg.body.ip_info.idx);
-			break;
+	case HOST_IF_MSG_GET_IPADDRESS:
+		handle_get_ip_address(msg->vif, msg->body.ip_info.idx);
+		break;
 
-		case HOST_IF_MSG_GET_MAC_ADDRESS:
-			handle_get_mac_address(msg.vif,
-					       &msg.body.get_mac_info);
-			break;
+	case HOST_IF_MSG_GET_MAC_ADDRESS:
+		handle_get_mac_address(msg->vif,
+				       &msg->body.get_mac_info);
+		break;
 
-		case HOST_IF_MSG_REMAIN_ON_CHAN:
-			Handle_RemainOnChan(msg.vif, &msg.body.remain_on_ch);
-			break;
+	case HOST_IF_MSG_REMAIN_ON_CHAN:
+		Handle_RemainOnChan(msg->vif, &msg->body.remain_on_ch);
+		break;
 
-		case HOST_IF_MSG_REGISTER_FRAME:
-			Handle_RegisterFrame(msg.vif, &msg.body.reg_frame);
-			break;
+	case HOST_IF_MSG_REGISTER_FRAME:
+		Handle_RegisterFrame(msg->vif, &msg->body.reg_frame);
+		break;
 
-		case HOST_IF_MSG_LISTEN_TIMER_FIRED:
-			Handle_ListenStateExpired(msg.vif, &msg.body.remain_on_ch);
-			break;
+	case HOST_IF_MSG_LISTEN_TIMER_FIRED:
+		Handle_ListenStateExpired(msg->vif, &msg->body.remain_on_ch);
+		break;
 
-		case HOST_IF_MSG_SET_MULTICAST_FILTER:
-			Handle_SetMulticastFilter(msg.vif, &msg.body.multicast_info);
-			break;
+	case HOST_IF_MSG_SET_MULTICAST_FILTER:
+		Handle_SetMulticastFilter(msg->vif, &msg->body.multicast_info);
+		break;
 
-		case HOST_IF_MSG_DEL_ALL_STA:
-			Handle_DelAllSta(msg.vif, &msg.body.del_all_sta_info);
-			break;
+	case HOST_IF_MSG_DEL_ALL_STA:
+		Handle_DelAllSta(msg->vif, &msg->body.del_all_sta_info);
+		break;
 
-		case HOST_IF_MSG_SET_TX_POWER:
-			handle_set_tx_pwr(msg.vif, msg.body.tx_power.tx_pwr);
-			break;
+	case HOST_IF_MSG_SET_TX_POWER:
+		handle_set_tx_pwr(msg->vif, msg->body.tx_power.tx_pwr);
+		break;
 
-		case HOST_IF_MSG_GET_TX_POWER:
-			handle_get_tx_pwr(msg.vif, &msg.body.tx_power.tx_pwr);
-			break;
-		default:
-			netdev_err(vif->ndev, "[Host Interface] undefined\n");
-			break;
-		}
+	case HOST_IF_MSG_GET_TX_POWER:
+		handle_get_tx_pwr(msg->vif, &msg->body.tx_power.tx_pwr);
+		break;
+	default:
+		netdev_err(msg->vif->ndev, "[Host Interface] undefined\n");
+		break;
 	}
-
+end:
+	kfree(msg);
 	complete(&hif_thread_comp);
-	return 0;
 }
 
 static void TimerCB_Scan(unsigned long arg)
@@ -2817,7 +2667,7 @@ static void TimerCB_Scan(unsigned long arg)
 	msg.vif = vif;
 	msg.id = HOST_IF_MSG_SCAN_TIMER_FIRED;
 
-	wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	wilc_enqueue_cmd(&msg);
 }
 
 static void TimerCB_Connect(unsigned long arg)
@@ -2829,7 +2679,7 @@ static void TimerCB_Connect(unsigned long arg)
 	msg.vif = vif;
 	msg.id = HOST_IF_MSG_CONNECT_TIMER_FIRED;
 
-	wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	wilc_enqueue_cmd(&msg);
 }
 
 s32 wilc_remove_key(struct host_if_drv *hif_drv, const u8 *pu8StaAddress)
@@ -2864,7 +2714,7 @@ int wilc_remove_wep_key(struct wilc_vif *vif, u8 index)
 	msg.vif = vif;
 	msg.body.key_info.attr.wep.index = index;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "Request to remove WEP key\n");
 	else
@@ -2893,7 +2743,7 @@ int wilc_set_wep_default_keyid(struct wilc_vif *vif, u8 index)
 	msg.vif = vif;
 	msg.body.key_info.attr.wep.index = index;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "Default key index\n");
 	else
@@ -2927,7 +2777,7 @@ int wilc_add_wep_key_bss_sta(struct wilc_vif *vif, const u8 *key, u8 len,
 	msg.body.key_info.attr.wep.key_len = len;
 	msg.body.key_info.attr.wep.index = index;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "STA - WEP Key\n");
 	wait_for_completion(&hif_drv->comp_test_key_block);
@@ -2962,7 +2812,7 @@ int wilc_add_wep_key_bss_ap(struct wilc_vif *vif, const u8 *key, u8 len,
 	msg.body.key_info.attr.wep.mode = mode;
 	msg.body.key_info.attr.wep.auth_type = auth_type;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 
 	if (result)
 		netdev_err(vif->ndev, "AP - WEP Key\n");
@@ -3018,7 +2868,7 @@ int wilc_add_ptk(struct wilc_vif *vif, const u8 *ptk, u8 ptk_key_len,
 	msg.body.key_info.attr.wpa.mode = cipher_mode;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 
 	if (result)
 		netdev_err(vif->ndev, "PTK Key\n");
@@ -3087,7 +2937,7 @@ int wilc_add_rx_gtk(struct wilc_vif *vif, const u8 *rx_gtk, u8 gtk_key_len,
 	msg.body.key_info.attr.wpa.key_len = key_len;
 	msg.body.key_info.attr.wpa.seq_len = key_rsc_len;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "RX GTK\n");
 	else
@@ -3117,7 +2967,7 @@ int wilc_set_pmkid_info(struct wilc_vif *vif,
 		       &pmkid->pmkidlist[i].pmkid, PMKID_LEN);
 	}
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "PMKID Info\n");
 
@@ -3135,7 +2985,7 @@ int wilc_get_mac_address(struct wilc_vif *vif, u8 *mac_addr)
 	msg.body.get_mac_info.mac_addr = mac_addr;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "Failed to send get mac address\n");
 		return -EFAULT;
@@ -3199,7 +3049,7 @@ int wilc_set_join_req(struct wilc_vif *vif, u8 *bssid, const u8 *ssid,
 	if (hif_drv->hif_state < HOST_IF_CONNECTING)
 		hif_drv->hif_state = HOST_IF_CONNECTING;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "send message: Set join request\n");
 		return -EFAULT;
@@ -3228,7 +3078,7 @@ int wilc_disconnect(struct wilc_vif *vif, u16 reason_code)
 	msg.id = HOST_IF_MSG_DISCONNECT;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "Failed to send message: disconnect\n");
 	else
@@ -3272,7 +3122,7 @@ int wilc_set_mac_chnl_num(struct wilc_vif *vif, u8 channel)
 	msg.body.channel_info.set_ch = channel;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 		return -EINVAL;
@@ -3292,7 +3142,7 @@ int wilc_set_wfi_drv_handler(struct wilc_vif *vif, int index, u8 mac_idx)
 	msg.body.drv.mac_idx = mac_idx;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 		result = -EINVAL;
@@ -3311,7 +3161,7 @@ int wilc_set_operation_mode(struct wilc_vif *vif, u32 mode)
 	msg.body.mode.mode = mode;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 		result = -EINVAL;
@@ -3338,7 +3188,7 @@ s32 wilc_get_inactive_time(struct wilc_vif *vif, const u8 *mac,
 	msg.id = HOST_IF_MSG_GET_INACTIVETIME;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "Failed to send get host ch param\n");
 	else
@@ -3359,7 +3209,7 @@ int wilc_get_rssi(struct wilc_vif *vif, s8 *rssi_level)
 	msg.id = HOST_IF_MSG_GET_RSSI;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "Failed to send get host ch param\n");
 		return -EFAULT;
@@ -3387,7 +3237,7 @@ int wilc_get_statistics(struct wilc_vif *vif, struct rf_info *stats)
 	msg.body.data = (char *)stats;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "Failed to send get host channel\n");
 		return -EFAULT;
@@ -3440,7 +3290,7 @@ int wilc_scan(struct wilc_vif *vif, u8 scan_source, u8 scan_type,
 	if (!scan_info->ies)
 		return -ENOMEM;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result) {
 		netdev_err(vif->ndev, "Error in sending message queue\n");
 		return -EINVAL;
@@ -3470,7 +3320,7 @@ int wilc_hif_set_cfg(struct wilc_vif *vif,
 	msg.body.cfg_info = *cfg_param;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 
 	return result;
 }
@@ -3532,21 +3382,17 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
 	init_completion(&hif_drv->comp_inactive_time);
 
 	if (clients_count == 0)	{
-		result = wilc_mq_create(&hif_msg_q);
-
 		if (result < 0) {
 			netdev_err(vif->ndev, "Failed to creat MQ\n");
 			goto _fail_;
 		}
-
-		hif_thread_handler = kthread_run(hostIFthread, wilc,
-						 "WILC_kthread");
-
-		if (IS_ERR(hif_thread_handler)) {
-			netdev_err(vif->ndev, "Failed to creat Thread\n");
-			result = -EFAULT;
+		hif_workqueue = create_singlethread_workqueue("WILC_wq");
+		if (!hif_workqueue) {
+			netdev_err(vif->ndev, "Failed to create workqueue\n");
+			result = -ENOMEM;
 			goto _fail_mq_;
 		}
+
 		setup_timer(&periodic_rssi, GetPeriodicRSSI,
 			    (unsigned long)vif);
 		mod_timer(&periodic_rssi, jiffies + msecs_to_jiffies(5000));
@@ -3572,10 +3418,8 @@ int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
 
 	clients_count++;
 
-	return result;
-
 _fail_mq_:
-	wilc_mq_destroy(&hif_msg_q);
+	destroy_workqueue(hif_workqueue);
 _fail_:
 	return result;
 }
@@ -3619,13 +3463,13 @@ int wilc_deinit(struct wilc_vif *vif)
 		msg.id = HOST_IF_MSG_EXIT;
 		msg.vif = vif;
 
-		result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+		result = wilc_enqueue_cmd(&msg);
 		if (result != 0)
 			netdev_err(vif->ndev, "deinit : Error(%d)\n", result);
 		else
 			wait_for_completion(&hif_thread_comp);
 
-		wilc_mq_destroy(&hif_msg_q);
+		destroy_workqueue(hif_workqueue);
 	}
 
 	kfree(hif_drv);
@@ -3665,7 +3509,7 @@ void wilc_network_info_received(struct wilc *wilc, u8 *pu8Buffer,
 	msg.body.net_info.buffer = kmalloc(u32Length, GFP_KERNEL);
 	memcpy(msg.body.net_info.buffer, pu8Buffer, u32Length);
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "message parameters (%d)\n", result);
 }
@@ -3710,7 +3554,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *pu8Buffer,
 	msg.body.async_info.buffer = kmalloc(u32Length, GFP_KERNEL);
 	memcpy(msg.body.async_info.buffer, pu8Buffer, u32Length);
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "synchronous info (%d)\n", result);
 
@@ -3741,7 +3585,7 @@ void wilc_scan_complete_received(struct wilc *wilc, u8 *pu8Buffer,
 		msg.id = HOST_IF_MSG_RCVD_SCAN_COMPLETE;
 		msg.vif = vif;
 
-		result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+		result = wilc_enqueue_cmd(&msg);
 		if (result)
 			netdev_err(vif->ndev, "complete param (%d)\n", result);
 	}
@@ -3767,7 +3611,7 @@ int wilc_remain_on_channel(struct wilc_vif *vif, u32 session_id,
 	msg.body.remain_on_ch.id = session_id;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 
@@ -3792,7 +3636,7 @@ int wilc_listen_state_expired(struct wilc_vif *vif, u32 session_id)
 	msg.vif = vif;
 	msg.body.remain_on_ch.id = session_id;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 
@@ -3823,7 +3667,7 @@ int wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg)
 	msg.body.reg_frame.reg = reg;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 
@@ -3861,7 +3705,7 @@ int wilc_add_beacon(struct wilc_vif *vif, u32 interval, u32 dtim_period,
 		beacon_info->tail = NULL;
 	}
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
 		netdev_err(vif->ndev, "wilc mq send fail\n");
 
@@ -3883,9 +3727,9 @@ int wilc_del_beacon(struct wilc_vif *vif)
 	msg.id = HOST_IF_MSG_DEL_BEACON;
 	msg.vif = vif;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 
 	return result;
 }
@@ -3910,9 +3754,9 @@ int wilc_add_station(struct wilc_vif *vif, struct add_sta_param *sta_param)
 			return -ENOMEM;
 	}
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 	return result;
 }
 
@@ -3932,9 +3776,9 @@ int wilc_del_station(struct wilc_vif *vif, const u8 *mac_addr)
 	else
 		memcpy(del_sta_info->mac_addr, mac_addr, ETH_ALEN);
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 	return result;
 }
 
@@ -3962,10 +3806,10 @@ int wilc_del_allstation(struct wilc_vif *vif, u8 mac_addr[][ETH_ALEN])
 		return result;
 
 	del_all_sta_info->assoc_sta = assoc_sta;
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 	else
 		wait_for_completion(&hif_wait_response);
 
@@ -3993,9 +3837,9 @@ int wilc_edit_station(struct wilc_vif *vif,
 			return -ENOMEM;
 	}
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 
 	return result;
 }
@@ -4017,9 +3861,9 @@ int wilc_set_power_mgmt(struct wilc_vif *vif, bool enabled, u32 timeout)
 	pwr_mgmt_info->enabled = enabled;
 	pwr_mgmt_info->timeout = timeout;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 	return result;
 }
 
@@ -4038,9 +3882,9 @@ int wilc_setup_multicast_filter(struct wilc_vif *vif, bool enabled,
 	multicast_filter_param->enabled = enabled;
 	multicast_filter_param->cnt = count;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 	return result;
 }
 
@@ -4211,9 +4055,9 @@ int wilc_setup_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
 	msg.vif = vif;
 	msg.body.ip_info.idx = idx;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 
 	return result;
 }
@@ -4231,9 +4075,9 @@ static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
 	msg.vif = vif;
 	msg.body.ip_info.idx = idx;
 
-	result = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	result = wilc_enqueue_cmd(&msg);
 	if (result)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 
 	return result;
 }
@@ -4249,9 +4093,9 @@ int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power)
 	msg.body.tx_power.tx_pwr = tx_power;
 	msg.vif = vif;
 
-	ret = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	ret = wilc_enqueue_cmd(&msg);
 	if (ret)
-		netdev_err(vif->ndev, "wilc_mq_send fail\n");
+		netdev_err(vif->ndev, "wilc_enqueue_cmd fail\n");
 
 	return ret;
 }
@@ -4266,7 +4110,7 @@ int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power)
 	msg.id = HOST_IF_MSG_GET_TX_POWER;
 	msg.vif = vif;
 
-	ret = wilc_mq_send(&hif_msg_q, &msg, sizeof(struct host_if_msg));
+	ret = wilc_enqueue_cmd(&msg);
 	if (ret)
 		netdev_err(vif->ndev, "Failed to get TX PWR\n");
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1427913 — Re: [PATCH v2 2/2] staging: wilc1000: Replace kthread with workqueue for host interface

On Monday, June 20, 2016 3:40:19 PM CEST Binoy Jayan wrote:
> Deconstruct the kthread / message_queue logic, replacing it with
> create_singlethread_workqueue() / queue_work() setup, by adding a
> 'struct work_struct' to 'struct host_if_msg'. The current kthread
> hostIFthread() is converted to a work queue helper with the name
> 'host_if_work'.
> 
> Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Hi Binoy,

You made a mistake here in adding the 'Reviewed-by:' tag before I replied with
that tag. I did a review of an earlier version and you addressed the comments
that I had for that, but you can only add the tag after I send that reply.

That said, the patch looks very good to me, and I see no remaining
problems that would prevent it from getting merged (after Atmel have tested
it), just a couple of things that would make it easier to review:

> +static int wilc_enqueue_cmd(struct host_if_msg *msg);
> +static void host_if_work(struct work_struct *work);

A small comment on coding style: we try to avoid forward declaration for local
functions. Instead, you can reorder the code to have the callee first. This
is the order which most readers will expect, and having no forward declarations
in the code makes it more likely that there are no recursions that would
be problematic for stack overflow.

>  /*!
>   *  @author		syounan
> @@ -336,95 +276,19 @@ static int wilc_mq_destroy(struct message_queue *mq)
>   *  @note		copied from FLO glue implementatuion
>   *  @version		1.0
>   */
> -static int wilc_mq_send(struct message_queue *mq,
> -		 const void *send_buf, u32 send_buf_size)
> +static int wilc_enqueue_cmd(struct host_if_msg *msg)

I think this API change can be done as a separate patch: the mq and send_buf_size
arguments are both constant for all callers, so you can have one patch just
removes them and renames the function, to make the patch that does the
tricky rework smaller.


> -		}
> +	if (msg->id == HOST_IF_MSG_CONNECT &&
> +	    msg->vif->hif_drv->usr_scan_req.scan_result) {
> +		wilc_enqueue_cmd(msg);
> +		usleep_range(2 * 1000, 2 * 1000);
> +		goto end;
> +	}
>  
> -		switch (msg.id) {
> -		case HOST_IF_MSG_SCAN:

A similar trick could apply here: we can leave the switch()
indented at the original level by putting it in an 'else'
clause. This again makes the patch shorter and easier to
review, and the optional reformatting can be done as a follow-up.

	Arnd

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web