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 <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
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
From: Xinming Hu <[email protected]>
This patch adds cfg80211 testmode support so that userspace tools can
download necessary commands to firmware during manufacturing mode tests.
Signed-off-by: Xinming <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
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
On Tue, Jul 26, 2016 at 03:09:20PM +0530, Amitkumar Karwar wrote:
> From: Xinming Hu <[email protected]>
>
> This patch adds cfg80211 testmode support so that userspace tools can
> download necessary commands to firmware during manufacturing mode tests.
>
> Signed-off-by: Xinming <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> ---
> 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,
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 <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> ---
> 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);
Hi Brian,
> From: Brian Norris [mailto:[email protected]]
> Sent: Thursday, September 01, 2016 1:10 AM
> To: Amitkumar Karwar
> Cc: [email protected]; 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 <[email protected]>
> >
> > This patch adds cfg80211 testmode support so that userspace tools can
> > download necessary commands to firmware during manufacturing mode
> tests.
> >
> > Signed-off-by: Xinming <[email protected]>
> > Signed-off-by: Amitkumar Karwar <[email protected]>
> > ---
> > 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
Hi Brian,
> From: Brian Norris [mailto:[email protected]]
> Sent: Thursday, September 01, 2016 1:05 AM
> To: Amitkumar Karwar
> Cc: [email protected]; 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 <[email protected]>
> > Signed-off-by: Amitkumar Karwar <[email protected]>
> > ---
> > 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);