[PATCH] libertas: make a handy lbs_cmd_async() command

[PATCH] libertas: make a handy lbs_cmd_async() command

[Holger Schurig]

This uses a static lbs_cmd_async_callback function, which is a noop. Just
setting the callback argument to __lbs_cmd_async() to NULL won't work,
because then the cmdnode wouldn't be released.

This also makes __lbs_cmd_async() a static method, which is now only used by
lbs_cmd() and lbs_cmd_async().

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

---

This patch is needed to fix the CMD_MAC_CONTROL bug correcly. Patch for this
follows in the next mail.

Index: wireless-testing/drivers/net/wireless/libertas/cmd.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/cmd.c	2008-03-18 10:07:11.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/cmd.c	2008-03-18 10:14:23.000000000 +0100
@@ -2040,10 +2040,27 @@ int lbs_cmd_copyback(struct lbs_private 
 }
 EXPORT_SYMBOL_GPL(lbs_cmd_copyback);
 
-struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
-				      struct cmd_header *in_cmd, int in_cmd_size,
-				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
-				      unsigned long callback_arg)
+/**
+ *  @brief Simple callback that ignores the result. Used if
+ *  we just want to send a command to the hardware, but don't
+ *  care for the result.
+ *
+ *  @param priv         ignored
+ *  @param extra        ignored
+ *  @param resp         ignored
+ *
+ *  @return 	   	0 for success
+ */
+static int lbs_cmd_async_callback(struct lbs_private *priv, unsigned long extra,
+		     struct cmd_header *resp)
+{
+	return 0;
+}
+
+static struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,
+	uint16_t command, struct cmd_header *in_cmd, int in_cmd_size,
+	int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
+	unsigned long callback_arg)
 {
 	struct cmd_ctrl_node *cmdnode;
 
@@ -2080,9 +2097,6 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
 
 	lbs_deb_host("PREP_CMD: command 0x%04x\n", command);
 
-	/* here was the big old switch() statement, which is now obsolete,
-	 * because the caller of lbs_cmd() sets up all of *cmd for us. */
-
 	cmdnode->cmdwaitqwoken = 0;
 	lbs_queue_cmd(priv, cmdnode);
 	wake_up_interruptible(&priv->waitq);
@@ -2092,6 +2106,15 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
 	return cmdnode;
 }
 
+void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
+	struct cmd_header *in_cmd, int in_cmd_size)
+{
+	lbs_deb_enter(LBS_DEB_CMD);
+	__lbs_cmd_async(priv, command, in_cmd, in_cmd_size,
+		lbs_cmd_async_callback, 0);
+	lbs_deb_leave(LBS_DEB_CMD);
+}
+
 int __lbs_cmd(struct lbs_private *priv, uint16_t command,
 	      struct cmd_header *in_cmd, int in_cmd_size,
 	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
Index: wireless-testing/drivers/net/wireless/libertas/cmd.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/cmd.h	2008-03-18 10:07:11.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/cmd.h	2008-03-18 10:09:21.000000000 +0100
@@ -18,12 +18,9 @@
 #define lbs_cmd_with_response(priv, cmdnr, cmd)	\
 	lbs_cmd(priv, cmdnr, cmd, lbs_cmd_copyback, (unsigned long) (cmd))
 
-/* __lbs_cmd() will free the cmdnode and return success/failure.
-   __lbs_cmd_async() requires that the callback free the cmdnode */
-struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
-				      struct cmd_header *in_cmd, int in_cmd_size,
-				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
-				      unsigned long callback_arg);
+void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
+	struct cmd_header *in_cmd, int in_cmd_size);
+
 int __lbs_cmd(struct lbs_private *priv, uint16_t command,
 	      struct cmd_header *in_cmd, int in_cmd_size,
 	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),

Re: [PATCH, take 2] libertas: make a handy lbs_cmd_async() command

[Holger Schurig]

This uses a static lbs_cmd_async_callback function, which is a
noop. Just setting the callback argument to __lbs_cmd_async()
to NULL won't work, because then the cmdnode wouldn't be
released.

This also makes __lbs_cmd_async() a static method, which is
now only used by lbs_cmd() and lbs_cmd_async().

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>


---

This patch is needed to fix the CMD_MAC_CONTROL bug correcly.

Compared with "take 1", this patch doesn't leak cmd_ctrl_node's
anymore.


[PATCH] libertas: make a handy lbs_cmd_async() command

This uses a static lbs_cmd_async_callback function, which is a noop. Just
setting the callback argument to __lbs_cmd_async() to NULL won't work,
because then the cmdnode wouldn't be released.

This also makes __lbs_cmd_async() a static method, which is now only used by
lbs_cmd() and lbs_cmd_async().

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

---

This patch is needed to fix the CMD_MAC_CONTROL bug correcly. Patch for this
follows in the next mail.

Index: wireless-testing/drivers/net/wireless/libertas/cmd.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/cmd.c	2008-03-19 09:04:44.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/cmd.c	2008-03-19 09:05:25.000000000 +0100
@@ -20,6 +20,46 @@ static void lbs_set_cmd_ctrl_node(struct
 
 
 /**
+ *  @brief Simple callback that copies response back into command
+ *
+ *  @param priv    	A pointer to struct lbs_private structure
+ *  @param extra  	A pointer to the original command structure for which
+ *                      'resp' is a response
+ *  @param resp         A pointer to the command response
+ *
+ *  @return 	   	0 on success, error on failure
+ */
+int lbs_cmd_copyback(struct lbs_private *priv, unsigned long extra,
+		     struct cmd_header *resp)
+{
+	struct cmd_header *buf = (void *)extra;
+	uint16_t copy_len;
+
+	copy_len = min(le16_to_cpu(buf->size), le16_to_cpu(resp->size));
+	memcpy(buf, resp, copy_len);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lbs_cmd_copyback);
+
+/**
+ *  @brief Simple callback that ignores the result. Use this if
+ *  you just want to send a command to the hardware, but don't
+ *  care for the result.
+ *
+ *  @param priv         ignored
+ *  @param extra        ignored
+ *  @param resp         ignored
+ *
+ *  @return 	   	0 for success
+ */
+static int lbs_cmd_async_callback(struct lbs_private *priv, unsigned long extra,
+		     struct cmd_header *resp)
+{
+	return 0;
+}
+
+
+/**
  *  @brief Checks whether a command is allowed in Power Save mode
  *
  *  @param command the command ID
@@ -1242,7 +1282,7 @@ void lbs_complete_command(struct lbs_pri
 	cmd->cmdwaitqwoken = 1;
 	wake_up_interruptible(&cmd->cmdwait_q);
 
-	if (!cmd->callback)
+	if (!cmd->callback || cmd->callback == lbs_cmd_async_callback)
 		__lbs_cleanup_and_insert_cmd(priv, cmd);
 	priv->cur_cmd = NULL;
 }
@@ -2018,32 +2058,10 @@ void lbs_ps_confirm_sleep(struct lbs_pri
 }
 
 
-/**
- *  @brief Simple callback that copies response back into command
- *
- *  @param priv    	A pointer to struct lbs_private structure
- *  @param extra  	A pointer to the original command structure for which
- *                      'resp' is a response
- *  @param resp         A pointer to the command response
- *
- *  @return 	   	0 on success, error on failure
- */
-int lbs_cmd_copyback(struct lbs_private *priv, unsigned long extra,
-		     struct cmd_header *resp)
-{
-	struct cmd_header *buf = (void *)extra;
-	uint16_t copy_len;
-
-	copy_len = min(le16_to_cpu(buf->size), le16_to_cpu(resp->size));
-	memcpy(buf, resp, copy_len);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(lbs_cmd_copyback);
-
-struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
-				      struct cmd_header *in_cmd, int in_cmd_size,
-				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
-				      unsigned long callback_arg)
+static struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,
+	uint16_t command, struct cmd_header *in_cmd, int in_cmd_size,
+	int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
+	unsigned long callback_arg)
 {
 	struct cmd_ctrl_node *cmdnode;
 
@@ -2080,9 +2098,6 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
 
 	lbs_deb_host("PREP_CMD: command 0x%04x\n", command);
 
-	/* here was the big old switch() statement, which is now obsolete,
-	 * because the caller of lbs_cmd() sets up all of *cmd for us. */
-
 	cmdnode->cmdwaitqwoken = 0;
 	lbs_queue_cmd(priv, cmdnode);
 	wake_up_interruptible(&priv->waitq);
@@ -2092,6 +2107,15 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
 	return cmdnode;
 }
 
+void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
+	struct cmd_header *in_cmd, int in_cmd_size)
+{
+	lbs_deb_enter(LBS_DEB_CMD);
+	__lbs_cmd_async(priv, command, in_cmd, in_cmd_size,
+		lbs_cmd_async_callback, 0);
+	lbs_deb_leave(LBS_DEB_CMD);
+}
+
 int __lbs_cmd(struct lbs_private *priv, uint16_t command,
 	      struct cmd_header *in_cmd, int in_cmd_size,
 	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
Index: wireless-testing/drivers/net/wireless/libertas/cmd.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/cmd.h	2008-03-19 09:04:44.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/cmd.h	2008-03-19 09:04:46.000000000 +0100
@@ -18,12 +18,9 @@
 #define lbs_cmd_with_response(priv, cmdnr, cmd)	\
 	lbs_cmd(priv, cmdnr, cmd, lbs_cmd_copyback, (unsigned long) (cmd))
 
-/* __lbs_cmd() will free the cmdnode and return success/failure.
-   __lbs_cmd_async() requires that the callback free the cmdnode */
-struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
-				      struct cmd_header *in_cmd, int in_cmd_size,
-				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
-				      unsigned long callback_arg);
+void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
+	struct cmd_header *in_cmd, int in_cmd_size);
+
 int __lbs_cmd(struct lbs_private *priv, uint16_t command,
 	      struct cmd_header *in_cmd, int in_cmd_size,
 	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),

Re: [PATCH, take 2] libertas: make a handy lbs_cmd_async() command

[Dan Williams]

On Wed, 2008-03-19 at 10:11 +0100, Holger Schurig wrote:
> This uses a static lbs_cmd_async_callback function, which is a
> noop. Just setting the callback argument to __lbs_cmd_async()
> to NULL won't work, because then the cmdnode wouldn't be
> released.
> 
> This also makes __lbs_cmd_async() a static method, which is
> now only used by lbs_cmd() and lbs_cmd_async().
> 
> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Acked-by: Dan Williams <dcbw@redhat.com>

> 
> ---
> 
> This patch is needed to fix the CMD_MAC_CONTROL bug correcly.
> 
> Compared with "take 1", this patch doesn't leak cmd_ctrl_node's
> anymore.
> 
> 
> [PATCH] libertas: make a handy lbs_cmd_async() command
> 
> This uses a static lbs_cmd_async_callback function, which is a noop. Just
> setting the callback argument to __lbs_cmd_async() to NULL won't work,
> because then the cmdnode wouldn't be released.
> 
> This also makes __lbs_cmd_async() a static method, which is now only used by
> lbs_cmd() and lbs_cmd_async().
> 
> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>
> 
> ---
> 
> This patch is needed to fix the CMD_MAC_CONTROL bug correcly. Patch for this
> follows in the next mail.
> 
> Index: wireless-testing/drivers/net/wireless/libertas/cmd.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/cmd.c	2008-03-19 09:04:44.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/cmd.c	2008-03-19 09:05:25.000000000 +0100
> @@ -20,6 +20,46 @@ static void lbs_set_cmd_ctrl_node(struct
>  
> 
>  /**
> + *  @brief Simple callback that copies response back into command
> + *
> + *  @param priv    	A pointer to struct lbs_private structure
> + *  @param extra  	A pointer to the original command structure for which
> + *                      'resp' is a response
> + *  @param resp         A pointer to the command response
> + *
> + *  @return 	   	0 on success, error on failure
> + */
> +int lbs_cmd_copyback(struct lbs_private *priv, unsigned long extra,
> +		     struct cmd_header *resp)
> +{
> +	struct cmd_header *buf = (void *)extra;
> +	uint16_t copy_len;
> +
> +	copy_len = min(le16_to_cpu(buf->size), le16_to_cpu(resp->size));
> +	memcpy(buf, resp, copy_len);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lbs_cmd_copyback);
> +
> +/**
> + *  @brief Simple callback that ignores the result. Use this if
> + *  you just want to send a command to the hardware, but don't
> + *  care for the result.
> + *
> + *  @param priv         ignored
> + *  @param extra        ignored
> + *  @param resp         ignored
> + *
> + *  @return 	   	0 for success
> + */
> +static int lbs_cmd_async_callback(struct lbs_private *priv, unsigned long extra,
> +		     struct cmd_header *resp)
> +{
> +	return 0;
> +}
> +
> +
> +/**
>   *  @brief Checks whether a command is allowed in Power Save mode
>   *
>   *  @param command the command ID
> @@ -1242,7 +1282,7 @@ void lbs_complete_command(struct lbs_pri
>  	cmd->cmdwaitqwoken = 1;
>  	wake_up_interruptible(&cmd->cmdwait_q);
>  
> -	if (!cmd->callback)
> +	if (!cmd->callback || cmd->callback == lbs_cmd_async_callback)
>  		__lbs_cleanup_and_insert_cmd(priv, cmd);
>  	priv->cur_cmd = NULL;
>  }
> @@ -2018,32 +2058,10 @@ void lbs_ps_confirm_sleep(struct lbs_pri
>  }
>  
> 
> -/**
> - *  @brief Simple callback that copies response back into command
> - *
> - *  @param priv    	A pointer to struct lbs_private structure
> - *  @param extra  	A pointer to the original command structure for which
> - *                      'resp' is a response
> - *  @param resp         A pointer to the command response
> - *
> - *  @return 	   	0 on success, error on failure
> - */
> -int lbs_cmd_copyback(struct lbs_private *priv, unsigned long extra,
> -		     struct cmd_header *resp)
> -{
> -	struct cmd_header *buf = (void *)extra;
> -	uint16_t copy_len;
> -
> -	copy_len = min(le16_to_cpu(buf->size), le16_to_cpu(resp->size));
> -	memcpy(buf, resp, copy_len);
> -	return 0;
> -}
> -EXPORT_SYMBOL_GPL(lbs_cmd_copyback);
> -
> -struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
> -				      struct cmd_header *in_cmd, int in_cmd_size,
> -				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
> -				      unsigned long callback_arg)
> +static struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,
> +	uint16_t command, struct cmd_header *in_cmd, int in_cmd_size,
> +	int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
> +	unsigned long callback_arg)
>  {
>  	struct cmd_ctrl_node *cmdnode;
>  
> @@ -2080,9 +2098,6 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
>  
>  	lbs_deb_host("PREP_CMD: command 0x%04x\n", command);
>  
> -	/* here was the big old switch() statement, which is now obsolete,
> -	 * because the caller of lbs_cmd() sets up all of *cmd for us. */
> -
>  	cmdnode->cmdwaitqwoken = 0;
>  	lbs_queue_cmd(priv, cmdnode);
>  	wake_up_interruptible(&priv->waitq);
> @@ -2092,6 +2107,15 @@ struct cmd_ctrl_node *__lbs_cmd_async(st
>  	return cmdnode;
>  }
>  
> +void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
> +	struct cmd_header *in_cmd, int in_cmd_size)
> +{
> +	lbs_deb_enter(LBS_DEB_CMD);
> +	__lbs_cmd_async(priv, command, in_cmd, in_cmd_size,
> +		lbs_cmd_async_callback, 0);
> +	lbs_deb_leave(LBS_DEB_CMD);
> +}
> +
>  int __lbs_cmd(struct lbs_private *priv, uint16_t command,
>  	      struct cmd_header *in_cmd, int in_cmd_size,
>  	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
> Index: wireless-testing/drivers/net/wireless/libertas/cmd.h
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/cmd.h	2008-03-19 09:04:44.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/cmd.h	2008-03-19 09:04:46.000000000 +0100
> @@ -18,12 +18,9 @@
>  #define lbs_cmd_with_response(priv, cmdnr, cmd)	\
>  	lbs_cmd(priv, cmdnr, cmd, lbs_cmd_copyback, (unsigned long) (cmd))
>  
> -/* __lbs_cmd() will free the cmdnode and return success/failure.
> -   __lbs_cmd_async() requires that the callback free the cmdnode */
> -struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv, uint16_t command,
> -				      struct cmd_header *in_cmd, int in_cmd_size,
> -				      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),
> -				      unsigned long callback_arg);
> +void lbs_cmd_async(struct lbs_private *priv, uint16_t command,
> +	struct cmd_header *in_cmd, int in_cmd_size);
> +
>  int __lbs_cmd(struct lbs_private *priv, uint16_t command,
>  	      struct cmd_header *in_cmd, int in_cmd_size,
>  	      int (*callback)(struct lbs_private *, unsigned long, struct cmd_header *),