From: Brian Norris <briannorris@chromium.org>
To: Amitkumar Karwar <akarwar@marvell.com>
Cc: linux-wireless@vger.kernel.org, Cathy Luo <cluo@marvell.com>,
Nishant Sarmukadam <nishants@marvell.com>
Subject: Re: [v4,1/2] mwifiex: add manufacturing mode support
Date: Wed, 31 Aug 2016 12:34:44 -0700 [thread overview]
Message-ID: <20160831193441.GA30908@localhost> (raw)
In-Reply-To: <1469525960-6643-1-git-send-email-akarwar@marvell.com>
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);
next prev parent reply other threads:[~2016-08-31 19:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-26 9:39 [PATCH v4 1/2] mwifiex: add manufacturing mode support Amitkumar Karwar
2016-07-26 9:39 ` [PATCH v4 2/2] mwifiex: add cfg80211 testmode support Amitkumar Karwar
2016-08-31 19:40 ` [v4,2/2] " Brian Norris
2016-09-02 7:40 ` Amitkumar Karwar
2016-08-31 19:34 ` Brian Norris [this message]
2016-09-02 7:39 ` [v4,1/2] mwifiex: add manufacturing mode support Amitkumar Karwar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160831193441.GA30908@localhost \
--to=briannorris@chromium.org \
--cc=akarwar@marvell.com \
--cc=cluo@marvell.com \
--cc=linux-wireless@vger.kernel.org \
--cc=nishants@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).