[PATCH v4 1/2] mwifiex: add manufacturing mode support

[PATCH v4 1/2] mwifiex: add manufacturing mode support

[Amitkumar Karwar]

By default normal mode is chosen when driver is loaded. This
patch adds a provision to choose manufacturing mode via module
parameters.

Below command loads driver in manufacturing mode
insmod mwifiex.ko mfg_mode=1.

Tested-by: chunfan chen <jeffc@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Removed mfg_firmware module parameter and hardcoded the firmware name
    in driver(Kalle Valo).
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |  8 ++++++++
 drivers/net/wireless/marvell/mwifiex/init.c   | 22 +++++++++++++++-------
 drivers/net/wireless/marvell/mwifiex/main.c   | 25 +++++++++++++++++++++----
 drivers/net/wireless/marvell/mwifiex/main.h   |  2 ++
 drivers/net/wireless/marvell/mwifiex/pcie.c   |  2 +-
 drivers/net/wireless/marvell/mwifiex/sdio.c   |  2 +-
 drivers/net/wireless/marvell/mwifiex/usb.c    |  2 +-
 7 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index d433aa0..636cfa0 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -595,6 +595,14 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
 			return -1;
 		}
 	}
+	/* We don't expect commands in manufacturing mode. They are cooked
+	 * in application and ready to download buffer is passed to the driver
+	*/
+	if (adapter->mfg_mode && cmd_no) {
+		dev_dbg(adapter->dev, "Ignoring commands in manufacturing mode\n");
+		return -1;
+	}
+
 
 	/* Get a new command node */
 	cmd_node = mwifiex_get_cmd_node(adapter);
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 1489c90..82839d9 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -298,6 +298,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
 	memset(&adapter->arp_filter, 0, sizeof(adapter->arp_filter));
 	adapter->arp_filter_size = 0;
 	adapter->max_mgmt_ie_index = MAX_MGMT_IE_INDEX;
+	adapter->mfg_mode = mfg_mode;
 	adapter->key_api_major_ver = 0;
 	adapter->key_api_minor_ver = 0;
 	eth_broadcast_addr(adapter->perm_addr);
@@ -553,15 +554,22 @@ int mwifiex_init_fw(struct mwifiex_adapter *adapter)
 				return -1;
 		}
 	}
+	if (adapter->mfg_mode) {
+		adapter->hw_status = MWIFIEX_HW_STATUS_READY;
+		ret = -EINPROGRESS;
+	} else {
+		for (i = 0; i < adapter->priv_num; i++) {
+			if (adapter->priv[i]) {
+				ret = mwifiex_sta_init_cmd(adapter->priv[i],
+							   first_sta, true);
+				if (ret == -1)
+					return -1;
+
+				first_sta = false;
+			}
+
 
-	for (i = 0; i < adapter->priv_num; i++) {
-		if (adapter->priv[i]) {
-			ret = mwifiex_sta_init_cmd(adapter->priv[i], first_sta,
-						   true);
-			if (ret == -1)
-				return -1;
 
-			first_sta = false;
 		}
 	}
 
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index db4925d..7fbf74b 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -23,6 +23,7 @@
 #include "11n.h"
 
 #define VERSION	"1.0"
+#define MFG_FIRMWARE	"mwifiex_mfg.bin"
 
 static unsigned int debug_mask = MWIFIEX_DEFAULT_DEBUG_MASK;
 module_param(debug_mask, uint, 0);
@@ -36,6 +37,9 @@ static unsigned short driver_mode;
 module_param(driver_mode, ushort, 0);
 MODULE_PARM_DESC(driver_mode,
 		 "station=0x1(default), ap-sta=0x3, station-p2p=0x5, ap-sta-p2p=0x7");
+bool mfg_mode;
+module_param(mfg_mode, bool, 0);
+MODULE_PARM_DESC(mfg_mode, "0:disable 1:enable (bool)");
 
 /*
  * This function registers the device and performs all the necessary
@@ -559,10 +563,12 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 		goto done;
 	}
 	/* Wait for mwifiex_init to complete */
-	wait_event_interruptible(adapter->init_wait_q,
-				 adapter->init_wait_q_woken);
-	if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
-		goto err_init_fw;
+	if (!adapter->mfg_mode) {
+		wait_event_interruptible(adapter->init_wait_q,
+					 adapter->init_wait_q_woken);
+		if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
+			goto err_init_fw;
+	}
 
 	priv = adapter->priv[MWIFIEX_BSS_ROLE_STA];
 	if (mwifiex_register_cfg80211(adapter)) {
@@ -666,6 +672,17 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
 {
 	int ret;
 
+	/* Override default firmware with manufacturing one if
+	 * manufacturing mode is enabled
+	 */
+	if (mfg_mode) {
+		if (strlcpy(adapter->fw_name, MFG_FIRMWARE,
+			    sizeof(adapter->fw_name)) >=
+			    sizeof(adapter->fw_name)) {
+			pr_err("%s: fw_name too long!\n", __func__);
+			return -1;
+		}
+	}
 	ret = request_firmware_nowait(THIS_MODULE, 1, adapter->fw_name,
 				      adapter->dev, GFP_KERNEL, adapter,
 				      mwifiex_fw_dpc);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 5902600..fcc2af35 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -58,6 +58,7 @@
 #include "sdio.h"
 
 extern const char driver_version[];
+extern bool mfg_mode;
 
 struct mwifiex_adapter;
 struct mwifiex_private;
@@ -990,6 +991,7 @@ struct mwifiex_adapter {
 	u32 drv_info_size;
 	bool scan_chan_gap_enabled;
 	struct sk_buff_head rx_data_q;
+	bool mfg_mode;
 	struct mwifiex_chan_stats *chan_stats;
 	u32 num_in_chan_stats;
 	int survey_idx;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 453ab6a..a6af85d 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -225,7 +225,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
 	if (!adapter || !adapter->priv_num)
 		return;
 
-	if (user_rmmod) {
+	if (user_rmmod && !adapter->mfg_mode) {
 #ifdef CONFIG_PM_SLEEP
 		if (adapter->is_suspended)
 			mwifiex_pcie_resume(&pdev->dev);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561..6dba409 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -289,7 +289,7 @@ mwifiex_sdio_remove(struct sdio_func *func)
 
 	mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
 
-	if (user_rmmod) {
+	if (user_rmmod && !adapter->mfg_mode) {
 		if (adapter->is_suspended)
 			mwifiex_sdio_resume(adapter->dev);
 
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 0857575..ba616ec 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -611,7 +611,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
 	if (!adapter->priv_num)
 		return;
 
-	if (user_rmmod) {
+	if (user_rmmod && !adapter->mfg_mode) {
 #ifdef CONFIG_PM
 		if (adapter->is_suspended)
 			mwifiex_usb_resume(intf);
-- 
1.9.1

[PATCH v4 2/2] mwifiex: add cfg80211 testmode support

[Amitkumar Karwar]

From: Xinming Hu <huxm@marvell.com>

This patch adds cfg80211 testmode support so that userspace tools can
download necessary commands to firmware during manufacturing mode tests.

Signed-off-by: Xinming <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Used cfg80211 testmode interface instead of wext in 2/2 patch.(Kalle Valo)
v3: Add "select WIRELESS_EXT" in Kconfig to resolve kbuild test robot errors.
    WEXT_PRIV seems to have a dependency with WIRELESS_EXT.
v2: 1) Sequence of these two patches are changed to resolve compilation
    error seen if only 1/2 is applied.
    2) Add "select WEXT_PRIV" in Kconfig to resolve warnings reported by
    kbuild test robot.
---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 83 +++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 235fb39..86b31b1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3919,6 +3919,88 @@ static int mwifiex_cfg80211_get_channel(struct wiphy *wiphy,
 	return ret;
 }
 
+#ifdef CONFIG_NL80211_TESTMODE
+
+enum mwifiex_tm_attr {
+	__MWIFIEX_TM_ATTR_INVALID	= 0,
+	MWIFIEX_TM_ATTR_CMD		= 1,
+	MWIFIEX_TM_ATTR_DATA		= 2,
+
+	/* keep last */
+	__MWIFIEX_TM_ATTR_AFTER_LAST,
+	MWIFIEX_TM_ATTR_MAX		= __MWIFIEX_TM_ATTR_AFTER_LAST - 1,
+};
+
+static const struct nla_policy mwifiex_tm_policy[MWIFIEX_TM_ATTR_MAX + 1] = {
+	[MWIFIEX_TM_ATTR_CMD]		= { .type = NLA_U32 },
+	[MWIFIEX_TM_ATTR_DATA]		= { .type = NLA_BINARY,
+					    .len = MWIFIEX_SIZE_OF_CMD_BUFFER },
+};
+
+enum mwifiex_tm_cmd {
+	MWIFIEX_TM_CMD_HOSTCMD	= 0,
+};
+
+int mwifiex_tm_cmd(struct wiphy *wiphy, struct wireless_dev *wdev,
+		   void *data, int len)
+{
+	struct mwifiex_private *priv = mwifiex_netdev_get_priv(wdev->netdev);
+	struct mwifiex_ds_misc_cmd *hostcmd;
+	struct nlattr *tb[MWIFIEX_TM_ATTR_MAX + 1];
+	struct mwifiex_adapter *adapter;
+	struct sk_buff *skb;
+	int err;
+
+	if (!priv)
+		return -EINVAL;
+	adapter = priv->adapter;
+
+	err = nla_parse(tb, MWIFIEX_TM_ATTR_MAX, data, len,
+			mwifiex_tm_policy);
+	if (err)
+		return err;
+
+	if (!tb[MWIFIEX_TM_ATTR_CMD])
+		return -EINVAL;
+
+	switch (nla_get_u32(tb[MWIFIEX_TM_ATTR_CMD])) {
+	case MWIFIEX_TM_CMD_HOSTCMD:
+		if (!tb[MWIFIEX_TM_ATTR_DATA])
+			return -EINVAL;
+
+		hostcmd = kzalloc(sizeof(*hostcmd), GFP_KERNEL);
+		if (!hostcmd)
+			return -ENOMEM;
+
+		hostcmd->len = nla_len(tb[MWIFIEX_TM_ATTR_DATA]);
+		memcpy(hostcmd->cmd, nla_data(tb[MWIFIEX_TM_ATTR_DATA]),
+		       hostcmd->len);
+
+		if (mwifiex_send_cmd(priv, 0, 0, 0, hostcmd, true)) {
+			dev_err(priv->adapter->dev, "Failed to process hostcmd\n");
+			return -EFAULT;
+		}
+
+		/* process hostcmd response*/
+		skb = cfg80211_testmode_alloc_reply_skb(wiphy, hostcmd->len);
+		if (!skb)
+			return -ENOMEM;
+		err = nla_put(skb, MWIFIEX_TM_ATTR_DATA,
+			      hostcmd->len, hostcmd->cmd);
+		if (err) {
+			kfree_skb(skb);
+			return -EMSGSIZE;
+		}
+
+		err = cfg80211_testmode_reply(skb);
+		kfree(hostcmd);
+		return err;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+#endif
+
 static int
 mwifiex_cfg80211_start_radar_detection(struct wiphy *wiphy,
 				       struct net_device *dev,
@@ -4031,6 +4113,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
 	.tdls_cancel_channel_switch = mwifiex_cfg80211_tdls_cancel_chan_switch,
 	.add_station = mwifiex_cfg80211_add_station,
 	.change_station = mwifiex_cfg80211_change_station,
+	CFG80211_TESTMODE_CMD(mwifiex_tm_cmd)
 	.get_channel = mwifiex_cfg80211_get_channel,
 	.start_radar_detection = mwifiex_cfg80211_start_radar_detection,
 	.channel_switch = mwifiex_cfg80211_channel_switch,
-- 
1.9.1

Re: [v4,1/2] mwifiex: add manufacturing mode support

[Brian Norris]

Hi,

On Tue, Jul 26, 2016 at 03:09:19PM +0530, Amitkumar Karwar wrote:
> By default normal mode is chosen when driver is loaded. This
> patch adds a provision to choose manufacturing mode via module
> parameters.
> 
> Below command loads driver in manufacturing mode
> insmod mwifiex.ko mfg_mode=1.
> 
> Tested-by: chunfan chen <jeffc@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v4: Removed mfg_firmware module parameter and hardcoded the firmware name
>     in driver(Kalle Valo).
> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c |  8 ++++++++
>  drivers/net/wireless/marvell/mwifiex/init.c   | 22 +++++++++++++++-------
>  drivers/net/wireless/marvell/mwifiex/main.c   | 25 +++++++++++++++++++++----
>  drivers/net/wireless/marvell/mwifiex/main.h   |  2 ++
>  drivers/net/wireless/marvell/mwifiex/pcie.c   |  2 +-
>  drivers/net/wireless/marvell/mwifiex/sdio.c   |  2 +-
>  drivers/net/wireless/marvell/mwifiex/usb.c    |  2 +-
>  7 files changed, 49 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index d433aa0..636cfa0 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -595,6 +595,14 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
>  			return -1;
>  		}
>  	}
> +	/* We don't expect commands in manufacturing mode. They are cooked
> +	 * in application and ready to download buffer is passed to the driver
> +	*/

You have the alignment wrong here. Needs an extra space.

> +	if (adapter->mfg_mode && cmd_no) {
> +		dev_dbg(adapter->dev, "Ignoring commands in manufacturing mode\n");
> +		return -1;
> +	}
> +
>  
>  	/* Get a new command node */
>  	cmd_node = mwifiex_get_cmd_node(adapter);
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
> index 1489c90..82839d9 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -298,6 +298,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
>  	memset(&adapter->arp_filter, 0, sizeof(adapter->arp_filter));
>  	adapter->arp_filter_size = 0;
>  	adapter->max_mgmt_ie_index = MAX_MGMT_IE_INDEX;
> +	adapter->mfg_mode = mfg_mode;
>  	adapter->key_api_major_ver = 0;
>  	adapter->key_api_minor_ver = 0;
>  	eth_broadcast_addr(adapter->perm_addr);
> @@ -553,15 +554,22 @@ int mwifiex_init_fw(struct mwifiex_adapter *adapter)
>  				return -1;
>  		}
>  	}
> +	if (adapter->mfg_mode) {
> +		adapter->hw_status = MWIFIEX_HW_STATUS_READY;
> +		ret = -EINPROGRESS;
> +	} else {
> +		for (i = 0; i < adapter->priv_num; i++) {
> +			if (adapter->priv[i]) {
> +				ret = mwifiex_sta_init_cmd(adapter->priv[i],
> +							   first_sta, true);
> +				if (ret == -1)
> +					return -1;
> +
> +				first_sta = false;
> +			}
> +
>  
> -	for (i = 0; i < adapter->priv_num; i++) {
> -		if (adapter->priv[i]) {
> -			ret = mwifiex_sta_init_cmd(adapter->priv[i], first_sta,
> -						   true);
> -			if (ret == -1)
> -				return -1;
>  
> -			first_sta = false;
>  		}
>  	}
>  
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index db4925d..7fbf74b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -23,6 +23,7 @@
>  #include "11n.h"
>  
>  #define VERSION	"1.0"
> +#define MFG_FIRMWARE	"mwifiex_mfg.bin"
>  
>  static unsigned int debug_mask = MWIFIEX_DEFAULT_DEBUG_MASK;
>  module_param(debug_mask, uint, 0);
> @@ -36,6 +37,9 @@ static unsigned short driver_mode;
>  module_param(driver_mode, ushort, 0);
>  MODULE_PARM_DESC(driver_mode,
>  		 "station=0x1(default), ap-sta=0x3, station-p2p=0x5, ap-sta-p2p=0x7");
> +bool mfg_mode;

Does this really need to be global? It's only actually used in init.c,
so could it help to move it to init.c and make it static?

> +module_param(mfg_mode, bool, 0);
> +MODULE_PARM_DESC(mfg_mode, "0:disable 1:enable (bool)");

That's not a very helpful description. Perhaps you could mention in a
word or two what this mode means?

Brian

>  
>  /*
>   * This function registers the device and performs all the necessary
> @@ -559,10 +563,12 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
>  		goto done;
>  	}
>  	/* Wait for mwifiex_init to complete */
> -	wait_event_interruptible(adapter->init_wait_q,
> -				 adapter->init_wait_q_woken);
> -	if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
> -		goto err_init_fw;
> +	if (!adapter->mfg_mode) {
> +		wait_event_interruptible(adapter->init_wait_q,
> +					 adapter->init_wait_q_woken);
> +		if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
> +			goto err_init_fw;
> +	}
>  
>  	priv = adapter->priv[MWIFIEX_BSS_ROLE_STA];
>  	if (mwifiex_register_cfg80211(adapter)) {
> @@ -666,6 +672,17 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
>  {
>  	int ret;
>  
> +	/* Override default firmware with manufacturing one if
> +	 * manufacturing mode is enabled
> +	 */
> +	if (mfg_mode) {
> +		if (strlcpy(adapter->fw_name, MFG_FIRMWARE,
> +			    sizeof(adapter->fw_name)) >=
> +			    sizeof(adapter->fw_name)) {
> +			pr_err("%s: fw_name too long!\n", __func__);
> +			return -1;
> +		}
> +	}
>  	ret = request_firmware_nowait(THIS_MODULE, 1, adapter->fw_name,
>  				      adapter->dev, GFP_KERNEL, adapter,
>  				      mwifiex_fw_dpc);
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index 5902600..fcc2af35 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -58,6 +58,7 @@
>  #include "sdio.h"
>  
>  extern const char driver_version[];
> +extern bool mfg_mode;
>  
>  struct mwifiex_adapter;
>  struct mwifiex_private;
> @@ -990,6 +991,7 @@ struct mwifiex_adapter {
>  	u32 drv_info_size;
>  	bool scan_chan_gap_enabled;
>  	struct sk_buff_head rx_data_q;
> +	bool mfg_mode;
>  	struct mwifiex_chan_stats *chan_stats;
>  	u32 num_in_chan_stats;
>  	int survey_idx;
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 453ab6a..a6af85d 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -225,7 +225,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
>  	if (!adapter || !adapter->priv_num)
>  		return;
>  
> -	if (user_rmmod) {
> +	if (user_rmmod && !adapter->mfg_mode) {
>  #ifdef CONFIG_PM_SLEEP
>  		if (adapter->is_suspended)
>  			mwifiex_pcie_resume(&pdev->dev);
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index d3e1561..6dba409 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -289,7 +289,7 @@ mwifiex_sdio_remove(struct sdio_func *func)
>  
>  	mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
>  
> -	if (user_rmmod) {
> +	if (user_rmmod && !adapter->mfg_mode) {
>  		if (adapter->is_suspended)
>  			mwifiex_sdio_resume(adapter->dev);
>  
> diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
> index 0857575..ba616ec 100644
> --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> @@ -611,7 +611,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
>  	if (!adapter->priv_num)
>  		return;
>  
> -	if (user_rmmod) {
> +	if (user_rmmod && !adapter->mfg_mode) {
>  #ifdef CONFIG_PM
>  		if (adapter->is_suspended)
>  			mwifiex_usb_resume(intf);

Re: [v4,2/2] mwifiex: add cfg80211 testmode support

[Brian Norris]

On Tue, Jul 26, 2016 at 03:09:20PM +0530, Amitkumar Karwar wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> This patch adds cfg80211 testmode support so that userspace tools can
> download necessary commands to firmware during manufacturing mode tests.
> 
> Signed-off-by: Xinming <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v4: Used cfg80211 testmode interface instead of wext in 2/2 patch.(Kalle Valo)
> v3: Add "select WIRELESS_EXT" in Kconfig to resolve kbuild test robot errors.
>     WEXT_PRIV seems to have a dependency with WIRELESS_EXT.
> v2: 1) Sequence of these two patches are changed to resolve compilation
>     error seen if only 1/2 is applied.
>     2) Add "select WEXT_PRIV" in Kconfig to resolve warnings reported by
>     kbuild test robot.
> ---
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 83 +++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index 235fb39..86b31b1 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -3919,6 +3919,88 @@ static int mwifiex_cfg80211_get_channel(struct wiphy *wiphy,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_NL80211_TESTMODE
> +
> +enum mwifiex_tm_attr {
> +	__MWIFIEX_TM_ATTR_INVALID	= 0,
> +	MWIFIEX_TM_ATTR_CMD		= 1,
> +	MWIFIEX_TM_ATTR_DATA		= 2,
> +
> +	/* keep last */
> +	__MWIFIEX_TM_ATTR_AFTER_LAST,
> +	MWIFIEX_TM_ATTR_MAX		= __MWIFIEX_TM_ATTR_AFTER_LAST - 1,
> +};
> +
> +static const struct nla_policy mwifiex_tm_policy[MWIFIEX_TM_ATTR_MAX + 1] = {
> +	[MWIFIEX_TM_ATTR_CMD]		= { .type = NLA_U32 },
> +	[MWIFIEX_TM_ATTR_DATA]		= { .type = NLA_BINARY,
> +					    .len = MWIFIEX_SIZE_OF_CMD_BUFFER },
> +};
> +
> +enum mwifiex_tm_cmd {
> +	MWIFIEX_TM_CMD_HOSTCMD	= 0,
> +};
> +
> +int mwifiex_tm_cmd(struct wiphy *wiphy, struct wireless_dev *wdev,

This function should be static, no?

Brian

> +		   void *data, int len)
> +{
> +	struct mwifiex_private *priv = mwifiex_netdev_get_priv(wdev->netdev);
> +	struct mwifiex_ds_misc_cmd *hostcmd;
> +	struct nlattr *tb[MWIFIEX_TM_ATTR_MAX + 1];
> +	struct mwifiex_adapter *adapter;
> +	struct sk_buff *skb;
> +	int err;
> +
> +	if (!priv)
> +		return -EINVAL;
> +	adapter = priv->adapter;
> +
> +	err = nla_parse(tb, MWIFIEX_TM_ATTR_MAX, data, len,
> +			mwifiex_tm_policy);
> +	if (err)
> +		return err;
> +
> +	if (!tb[MWIFIEX_TM_ATTR_CMD])
> +		return -EINVAL;
> +
> +	switch (nla_get_u32(tb[MWIFIEX_TM_ATTR_CMD])) {
> +	case MWIFIEX_TM_CMD_HOSTCMD:
> +		if (!tb[MWIFIEX_TM_ATTR_DATA])
> +			return -EINVAL;
> +
> +		hostcmd = kzalloc(sizeof(*hostcmd), GFP_KERNEL);
> +		if (!hostcmd)
> +			return -ENOMEM;
> +
> +		hostcmd->len = nla_len(tb[MWIFIEX_TM_ATTR_DATA]);
> +		memcpy(hostcmd->cmd, nla_data(tb[MWIFIEX_TM_ATTR_DATA]),
> +		       hostcmd->len);
> +
> +		if (mwifiex_send_cmd(priv, 0, 0, 0, hostcmd, true)) {
> +			dev_err(priv->adapter->dev, "Failed to process hostcmd\n");
> +			return -EFAULT;
> +		}
> +
> +		/* process hostcmd response*/
> +		skb = cfg80211_testmode_alloc_reply_skb(wiphy, hostcmd->len);
> +		if (!skb)
> +			return -ENOMEM;
> +		err = nla_put(skb, MWIFIEX_TM_ATTR_DATA,
> +			      hostcmd->len, hostcmd->cmd);
> +		if (err) {
> +			kfree_skb(skb);
> +			return -EMSGSIZE;
> +		}
> +
> +		err = cfg80211_testmode_reply(skb);
> +		kfree(hostcmd);
> +		return err;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +#endif
> +
>  static int
>  mwifiex_cfg80211_start_radar_detection(struct wiphy *wiphy,
>  				       struct net_device *dev,
> @@ -4031,6 +4113,7 @@ static struct cfg80211_ops mwifiex_cfg80211_ops = {
>  	.tdls_cancel_channel_switch = mwifiex_cfg80211_tdls_cancel_chan_switch,
>  	.add_station = mwifiex_cfg80211_add_station,
>  	.change_station = mwifiex_cfg80211_change_station,
> +	CFG80211_TESTMODE_CMD(mwifiex_tm_cmd)
>  	.get_channel = mwifiex_cfg80211_get_channel,
>  	.start_radar_detection = mwifiex_cfg80211_start_radar_detection,
>  	.channel_switch = mwifiex_cfg80211_channel_switch,

RE: [v4,1/2] mwifiex: add manufacturing mode support

[Amitkumar Karwar]

Hi Brian,

> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Thursday, September 01, 2016 1:05 AM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam
> Subject: Re: [v4,1/2] mwifiex: add manufacturing mode support
> 
> Hi,
> 
> On Tue, Jul 26, 2016 at 03:09:19PM +0530, Amitkumar Karwar wrote:
> > By default normal mode is chosen when driver is loaded. This patch
> > adds a provision to choose manufacturing mode via module parameters.
> >
> > Below command loads driver in manufacturing mode insmod mwifiex.ko
> > mfg_mode=1.
> >
> > Tested-by: chunfan chen <jeffc@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v4: Removed mfg_firmware module parameter and hardcoded the firmware
> name
> >     in driver(Kalle Valo).
> > ---
> >  drivers/net/wireless/marvell/mwifiex/cmdevt.c |  8 ++++++++
> >  drivers/net/wireless/marvell/mwifiex/init.c   | 22 +++++++++++++++---
> ----
> >  drivers/net/wireless/marvell/mwifiex/main.c   | 25
> +++++++++++++++++++++----
> >  drivers/net/wireless/marvell/mwifiex/main.h   |  2 ++
> >  drivers/net/wireless/marvell/mwifiex/pcie.c   |  2 +-
> >  drivers/net/wireless/marvell/mwifiex/sdio.c   |  2 +-
> >  drivers/net/wireless/marvell/mwifiex/usb.c    |  2 +-
> >  7 files changed, 49 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > index d433aa0..636cfa0 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> > @@ -595,6 +595,14 @@ int mwifiex_send_cmd(struct mwifiex_private
> *priv, u16 cmd_no,
> >  			return -1;
> >  		}
> >  	}
> > +	/* We don't expect commands in manufacturing mode. They are cooked
> > +	 * in application and ready to download buffer is passed to the
> driver
> > +	*/
> 
> You have the alignment wrong here. Needs an extra space.
> 
> > +	if (adapter->mfg_mode && cmd_no) {
> > +		dev_dbg(adapter->dev, "Ignoring commands in manufacturing
> mode\n");
> > +		return -1;
> > +	}
> > +
> >
> >  	/* Get a new command node */
> >  	cmd_node = mwifiex_get_cmd_node(adapter); diff --git
> > a/drivers/net/wireless/marvell/mwifiex/init.c
> > b/drivers/net/wireless/marvell/mwifiex/init.c
> > index 1489c90..82839d9 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/init.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> > @@ -298,6 +298,7 @@ static void mwifiex_init_adapter(struct
> mwifiex_adapter *adapter)
> >  	memset(&adapter->arp_filter, 0, sizeof(adapter->arp_filter));
> >  	adapter->arp_filter_size = 0;
> >  	adapter->max_mgmt_ie_index = MAX_MGMT_IE_INDEX;
> > +	adapter->mfg_mode = mfg_mode;
> >  	adapter->key_api_major_ver = 0;
> >  	adapter->key_api_minor_ver = 0;
> >  	eth_broadcast_addr(adapter->perm_addr);
> > @@ -553,15 +554,22 @@ int mwifiex_init_fw(struct mwifiex_adapter
> *adapter)
> >  				return -1;
> >  		}
> >  	}
> > +	if (adapter->mfg_mode) {
> > +		adapter->hw_status = MWIFIEX_HW_STATUS_READY;
> > +		ret = -EINPROGRESS;
> > +	} else {
> > +		for (i = 0; i < adapter->priv_num; i++) {
> > +			if (adapter->priv[i]) {
> > +				ret = mwifiex_sta_init_cmd(adapter->priv[i],
> > +							   first_sta, true);
> > +				if (ret == -1)
> > +					return -1;
> > +
> > +				first_sta = false;
> > +			}
> > +
> >
> > -	for (i = 0; i < adapter->priv_num; i++) {
> > -		if (adapter->priv[i]) {
> > -			ret = mwifiex_sta_init_cmd(adapter->priv[i],
> first_sta,
> > -						   true);
> > -			if (ret == -1)
> > -				return -1;
> >
> > -			first_sta = false;
> >  		}
> >  	}
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/main.c
> > b/drivers/net/wireless/marvell/mwifiex/main.c
> > index db4925d..7fbf74b 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/main.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> > @@ -23,6 +23,7 @@
> >  #include "11n.h"
> >
> >  #define VERSION	"1.0"
> > +#define MFG_FIRMWARE	"mwifiex_mfg.bin"
> >
> >  static unsigned int debug_mask = MWIFIEX_DEFAULT_DEBUG_MASK;
> > module_param(debug_mask, uint, 0); @@ -36,6 +37,9 @@ static unsigned
> > short driver_mode;  module_param(driver_mode, ushort, 0);
> > MODULE_PARM_DESC(driver_mode,
> >  		 "station=0x1(default), ap-sta=0x3, station-p2p=0x5,
> > ap-sta-p2p=0x7");
> > +bool mfg_mode;
> 
> Does this really need to be global? It's only actually used in init.c,
> so could it help to move it to init.c and make it static?

We also use mfg_mode in main.c(mwifiex_init_hw_fw), so it can't be moved to init.s

> 
> > +module_param(mfg_mode, bool, 0);
> > +MODULE_PARM_DESC(mfg_mode, "0:disable 1:enable (bool)");
> 
> That's not a very helpful description. Perhaps you could mention in a
> word or two what this mode means?
> 
> Brian

Thanks. I just submitted v5 patches addressing your comments.

Regards,
Amitkumar Karwar

> 
> >
> >  /*
> >   * This function registers the device and performs all the necessary
> > @@ -559,10 +563,12 @@ static void mwifiex_fw_dpc(const struct firmware
> *firmware, void *context)
> >  		goto done;
> >  	}
> >  	/* Wait for mwifiex_init to complete */
> > -	wait_event_interruptible(adapter->init_wait_q,
> > -				 adapter->init_wait_q_woken);
> > -	if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
> > -		goto err_init_fw;
> > +	if (!adapter->mfg_mode) {
> > +		wait_event_interruptible(adapter->init_wait_q,
> > +					 adapter->init_wait_q_woken);
> > +		if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
> > +			goto err_init_fw;
> > +	}
> >
> >  	priv = adapter->priv[MWIFIEX_BSS_ROLE_STA];
> >  	if (mwifiex_register_cfg80211(adapter)) { @@ -666,6 +672,17 @@
> > static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)  {
> >  	int ret;
> >
> > +	/* Override default firmware with manufacturing one if
> > +	 * manufacturing mode is enabled
> > +	 */
> > +	if (mfg_mode) {
> > +		if (strlcpy(adapter->fw_name, MFG_FIRMWARE,
> > +			    sizeof(adapter->fw_name)) >=
> > +			    sizeof(adapter->fw_name)) {
> > +			pr_err("%s: fw_name too long!\n", __func__);
> > +			return -1;
> > +		}
> > +	}
> >  	ret = request_firmware_nowait(THIS_MODULE, 1, adapter->fw_name,
> >  				      adapter->dev, GFP_KERNEL, adapter,
> >  				      mwifiex_fw_dpc);
> > diff --git a/drivers/net/wireless/marvell/mwifiex/main.h
> > b/drivers/net/wireless/marvell/mwifiex/main.h
> > index 5902600..fcc2af35 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/main.h
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> > @@ -58,6 +58,7 @@
> >  #include "sdio.h"
> >
> >  extern const char driver_version[];
> > +extern bool mfg_mode;
> >
> >  struct mwifiex_adapter;
> >  struct mwifiex_private;
> > @@ -990,6 +991,7 @@ struct mwifiex_adapter {
> >  	u32 drv_info_size;
> >  	bool scan_chan_gap_enabled;
> >  	struct sk_buff_head rx_data_q;
> > +	bool mfg_mode;
> >  	struct mwifiex_chan_stats *chan_stats;
> >  	u32 num_in_chan_stats;
> >  	int survey_idx;
> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > index 453ab6a..a6af85d 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > @@ -225,7 +225,7 @@ static void mwifiex_pcie_remove(struct pci_dev
> *pdev)
> >  	if (!adapter || !adapter->priv_num)
> >  		return;
> >
> > -	if (user_rmmod) {
> > +	if (user_rmmod && !adapter->mfg_mode) {
> >  #ifdef CONFIG_PM_SLEEP
> >  		if (adapter->is_suspended)
> >  			mwifiex_pcie_resume(&pdev->dev);
> > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > index d3e1561..6dba409 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > @@ -289,7 +289,7 @@ mwifiex_sdio_remove(struct sdio_func *func)
> >
> >  	mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
> >
> > -	if (user_rmmod) {
> > +	if (user_rmmod && !adapter->mfg_mode) {
> >  		if (adapter->is_suspended)
> >  			mwifiex_sdio_resume(adapter->dev);
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c
> > b/drivers/net/wireless/marvell/mwifiex/usb.c
> > index 0857575..ba616ec 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> > @@ -611,7 +611,7 @@ static void mwifiex_usb_disconnect(struct
> usb_interface *intf)
> >  	if (!adapter->priv_num)
> >  		return;
> >
> > -	if (user_rmmod) {
> > +	if (user_rmmod && !adapter->mfg_mode) {
> >  #ifdef CONFIG_PM
> >  		if (adapter->is_suspended)
> >  			mwifiex_usb_resume(intf);

RE: [v4,2/2] mwifiex: add cfg80211 testmode support

[Amitkumar Karwar]

Hi Brian,

> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Thursday, September 01, 2016 1:10 AM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> Xinming Hu
> Subject: Re: [v4,2/2] mwifiex: add cfg80211 testmode support
> 
> On Tue, Jul 26, 2016 at 03:09:20PM +0530, Amitkumar Karwar wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > This patch adds cfg80211 testmode support so that userspace tools can
> > download necessary commands to firmware during manufacturing mode
> tests.
> >
> > Signed-off-by: Xinming <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v4: Used cfg80211 testmode interface instead of wext in 2/2
> > patch.(Kalle Valo)
> > v3: Add "select WIRELESS_EXT" in Kconfig to resolve kbuild test robot
> errors.
> >     WEXT_PRIV seems to have a dependency with WIRELESS_EXT.
> > v2: 1) Sequence of these two patches are changed to resolve
> compilation
> >     error seen if only 1/2 is applied.
> >     2) Add "select WEXT_PRIV" in Kconfig to resolve warnings reported
> by
> >     kbuild test robot.
> > ---
> >  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 83
> > +++++++++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > index 235fb39..86b31b1 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > @@ -3919,6 +3919,88 @@ static int mwifiex_cfg80211_get_channel(struct
> wiphy *wiphy,
> >  	return ret;
> >  }
> >
> > +#ifdef CONFIG_NL80211_TESTMODE
> > +
> > +enum mwifiex_tm_attr {
> > +	__MWIFIEX_TM_ATTR_INVALID	= 0,
> > +	MWIFIEX_TM_ATTR_CMD		= 1,
> > +	MWIFIEX_TM_ATTR_DATA		= 2,
> > +
> > +	/* keep last */
> > +	__MWIFIEX_TM_ATTR_AFTER_LAST,
> > +	MWIFIEX_TM_ATTR_MAX		= __MWIFIEX_TM_ATTR_AFTER_LAST - 1,
> > +};
> > +
> > +static const struct nla_policy mwifiex_tm_policy[MWIFIEX_TM_ATTR_MAX
> + 1] = {
> > +	[MWIFIEX_TM_ATTR_CMD]		= { .type = NLA_U32 },
> > +	[MWIFIEX_TM_ATTR_DATA]		= { .type = NLA_BINARY,
> > +					    .len = MWIFIEX_SIZE_OF_CMD_BUFFER },
> };
> > +
> > +enum mwifiex_tm_cmd {
> > +	MWIFIEX_TM_CMD_HOSTCMD	= 0,
> > +};
> > +
> > +int mwifiex_tm_cmd(struct wiphy *wiphy, struct wireless_dev *wdev,
> 
> This function should be static, no?
> 
> Brian
> 

Yes. This have been taken care of in V5 patch.

Regards,
Amitkumar Karwar