From: "John W. Linville" <linville@tuxdriver.com>
To: Bing Zhao <bzhao@marvell.com>
Cc: linux-wireless@vger.kernel.org,
Amitkumar Karwar <akarwar@marvell.com>,
Avinash Patil <patila@marvell.com>,
Yogesh Ashok Powar <yogeshp@marvell.com>,
Nishant Sarmukadam <nishants@marvell.com>,
Frank Huang <frankh@marvell.com>
Subject: Re: [PATCH 18/21] mwifiex: handle driver initialization error paths
Date: Wed, 24 Jul 2013 11:16:52 -0400 [thread overview]
Message-ID: <20130724151651.GD2385@tuxdriver.com> (raw)
In-Reply-To: <1374545878-15683-19-git-send-email-bzhao@marvell.com>
This one didn't apply on the current wireless-next tree. Can you
fix it up?
On Mon, Jul 22, 2013 at 07:17:55PM -0700, Bing Zhao wrote:
> From: Amitkumar Karwar <akarwar@marvell.com>
>
> mwifiex_fw_dpc() asynchronously takes care of firmware download
> and initialization. Currently the error paths in mwifiex_fw_dpc()
> are not handled. So if wrong firmware is downloaded, required
> cleanup work is not performed. memory is leaked and workqueue
> remains unterminated in this case.
>
> mwifiex_terminate_workqueue() is moved to avoid forward
> declaration.
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Bing Zhao <bzhao@marvell.com>
> ---
> drivers/net/wireless/mwifiex/main.c | 83 ++++++++++++++++++++++++-------------
> drivers/net/wireless/mwifiex/main.h | 1 +
> 2 files changed, 56 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
> index e64c369..5644c7f 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -390,6 +390,17 @@ static void mwifiex_free_adapter(struct mwifiex_adapter *adapter)
> }
>
> /*
> + * This function cancels all works in the queue and destroys
> + * the main workqueue.
> + */
> +static void mwifiex_terminate_workqueue(struct mwifiex_adapter *adapter)
> +{
> + flush_workqueue(adapter->workqueue);
> + destroy_workqueue(adapter->workqueue);
> + adapter->workqueue = NULL;
> +}
> +
> +/*
> * This function gets firmware and initializes it.
> *
> * The main initialization steps followed are -
> @@ -398,7 +409,7 @@ static void mwifiex_free_adapter(struct mwifiex_adapter *adapter)
> */
> static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> {
> - int ret;
> + int ret, i;
> char fmt[64];
> struct mwifiex_private *priv;
> struct mwifiex_adapter *adapter = context;
> @@ -407,7 +418,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> if (!firmware) {
> dev_err(adapter->dev,
> "Failed to get firmware %s\n", adapter->fw_name);
> - goto done;
> + goto err_dnld_fw;
> }
>
> memset(&fw, 0, sizeof(struct mwifiex_fw_image));
> @@ -420,7 +431,7 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> else
> ret = mwifiex_dnld_fw(adapter, &fw);
> if (ret == -1)
> - goto done;
> + goto err_dnld_fw;
>
> dev_notice(adapter->dev, "WLAN FW is active\n");
>
> @@ -432,13 +443,15 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> }
>
> /* enable host interrupt after fw dnld is successful */
> - if (adapter->if_ops.enable_int)
> - adapter->if_ops.enable_int(adapter);
> + if (adapter->if_ops.enable_int) {
> + if (adapter->if_ops.enable_int(adapter))
> + goto err_dnld_fw;
> + }
>
> adapter->init_wait_q_woken = false;
> ret = mwifiex_init_fw(adapter);
> if (ret == -1) {
> - goto done;
> + goto err_init_fw;
> } else if (!ret) {
> adapter->hw_status = MWIFIEX_HW_STATUS_READY;
> goto done;
> @@ -447,12 +460,12 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> wait_event_interruptible(adapter->init_wait_q,
> adapter->init_wait_q_woken);
> if (adapter->hw_status != MWIFIEX_HW_STATUS_READY)
> - goto done;
> + goto err_init_fw;
>
> priv = adapter->priv[MWIFIEX_BSS_ROLE_STA];
> if (mwifiex_register_cfg80211(adapter)) {
> dev_err(adapter->dev, "cannot register with cfg80211\n");
> - goto err_init_fw;
> + goto err_register_cfg80211;
> }
>
> rtnl_lock();
> @@ -483,13 +496,39 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
> goto done;
>
> err_add_intf:
> - mwifiex_del_virtual_intf(adapter->wiphy, priv->wdev);
> + for (i = 0; i < adapter->priv_num; i++) {
> + priv = adapter->priv[i];
> +
> + if (!priv)
> + continue;
> +
> + if (priv->wdev && priv->netdev)
> + mwifiex_del_virtual_intf(adapter->wiphy, priv->wdev);
> + }
> rtnl_unlock();
> +err_register_cfg80211:
> + wiphy_unregister(adapter->wiphy);
> + wiphy_free(adapter->wiphy);
> err_init_fw:
> if (adapter->if_ops.disable_int)
> adapter->if_ops.disable_int(adapter);
> +err_dnld_fw:
> pr_debug("info: %s: unregister device\n", __func__);
> - adapter->if_ops.unregister_dev(adapter);
> + if (adapter->if_ops.unregister_dev)
> + adapter->if_ops.unregister_dev(adapter);
> +
> + if ((adapter->hw_status == MWIFIEX_HW_STATUS_FW_READY) ||
> + (adapter->hw_status == MWIFIEX_HW_STATUS_READY)) {
> + pr_debug("info: %s: shutdown mwifiex\n", __func__);
> + adapter->init_wait_q_woken = false;
> +
> + if (mwifiex_shutdown_drv(adapter) == -EINPROGRESS)
> + wait_event_interruptible(adapter->init_wait_q,
> + adapter->init_wait_q_woken);
> + }
> + adapter->surprise_removed = true;
> + mwifiex_terminate_workqueue(adapter);
> + mwifiex_free_adapter(adapter);
> done:
> if (adapter->cal_data) {
> release_firmware(adapter->cal_data);
> @@ -497,6 +536,7 @@ done:
> }
> release_firmware(adapter->firmware);
> complete(&adapter->fw_load);
> + up(adapter->card_sem);
> return;
> }
>
> @@ -807,18 +847,6 @@ static void mwifiex_main_work_queue(struct work_struct *work)
> }
>
> /*
> - * This function cancels all works in the queue and destroys
> - * the main workqueue.
> - */
> -static void
> -mwifiex_terminate_workqueue(struct mwifiex_adapter *adapter)
> -{
> - flush_workqueue(adapter->workqueue);
> - destroy_workqueue(adapter->workqueue);
> - adapter->workqueue = NULL;
> -}
> -
> -/*
> * This function adds the card.
> *
> * This function follows the following major steps to set up the device -
> @@ -846,6 +874,7 @@ mwifiex_add_card(void *card, struct semaphore *sem,
> }
>
> adapter->iface_type = iface_type;
> + adapter->card_sem = sem;
>
> adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING;
> adapter->surprise_removed = false;
> @@ -876,17 +905,12 @@ mwifiex_add_card(void *card, struct semaphore *sem,
> goto err_init_fw;
> }
>
> - up(sem);
> return 0;
>
> err_init_fw:
> pr_debug("info: %s: unregister device\n", __func__);
> if (adapter->if_ops.unregister_dev)
> adapter->if_ops.unregister_dev(adapter);
> -err_registerdev:
> - adapter->surprise_removed = true;
> - mwifiex_terminate_workqueue(adapter);
> -err_kmalloc:
> if ((adapter->hw_status == MWIFIEX_HW_STATUS_FW_READY) ||
> (adapter->hw_status == MWIFIEX_HW_STATUS_READY)) {
> pr_debug("info: %s: shutdown mwifiex\n", __func__);
> @@ -896,7 +920,10 @@ err_kmalloc:
> wait_event_interruptible(adapter->init_wait_q,
> adapter->init_wait_q_woken);
> }
> -
> +err_registerdev:
> + adapter->surprise_removed = true;
> + mwifiex_terminate_workqueue(adapter);
> +err_kmalloc:
> mwifiex_free_adapter(adapter);
>
> err_init_sw:
> diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
> index bb28d3d..9ee3b1b 100644
> --- a/drivers/net/wireless/mwifiex/main.h
> +++ b/drivers/net/wireless/mwifiex/main.h
> @@ -749,6 +749,7 @@ struct mwifiex_adapter {
>
> atomic_t is_tx_received;
> atomic_t pending_bridged_pkts;
> + struct semaphore *card_sem;
> };
>
> int mwifiex_init_lock_list(struct mwifiex_adapter *adapter);
> --
> 1.8.2.3
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
next prev parent reply other threads:[~2013-07-24 15:30 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-23 2:17 [PATCH 00/21] mwifiex updates for 3.12 Bing Zhao
2013-07-23 2:17 ` [PATCH 01/21] mwifiex: remove stop_net_dev_queue operation in AP forwarding Bing Zhao
2013-07-23 2:17 ` [PATCH 02/21] mwifiex: rename pkt_count to ba_pkt_count in mwifiex_ra_list_tbl struct Bing Zhao
2013-07-23 2:17 ` [PATCH 03/21] mwifiex: maintain outstanding packet count for RA list instead of packet size Bing Zhao
2013-07-23 2:17 ` [PATCH 04/21] mwifiex: delete AP TX queues when bridged packets reach threshold Bing Zhao
2013-07-23 2:17 ` [PATCH 05/21] mwifiex: add tx info to skb when forming mgmt frame Bing Zhao
2013-07-23 2:17 ` [PATCH 06/21] mwifiex: discard deauth and disassoc event during WPS session Bing Zhao
2013-07-23 2:17 ` [PATCH 07/21] mwifiex: skip registering mgmt frame that has already registered Bing Zhao
2013-07-23 2:17 ` [PATCH 08/21] mwifiex: support to send deauth for P2P client Bing Zhao
2013-07-23 2:17 ` [PATCH 09/21] mwifiex: correct max IE length check for WPS IE Bing Zhao
2013-07-23 2:17 ` [PATCH 10/21] mwifiex: add PCIe shutdown handler to avoid system hang on reboot Bing Zhao
2013-07-23 2:17 ` [PATCH 11/21] mwifiex: move del_timer_sync(scan_delay_timer) call to fix memleak Bing Zhao
2013-07-23 2:17 ` [PATCH 12/21] mwifiex: remove unnecessary del_timer(cmd_timer) Bing Zhao
2013-07-23 2:17 ` [PATCH 13/21] mwifiex: move if_ops.cleanup_if() call Bing Zhao
2013-07-23 2:17 ` [PATCH 14/21] mwifiex: add unregister_dev handler for usb interface Bing Zhao
2013-07-23 2:17 ` [PATCH 15/21] mwifiex: reduce firmware poll retries Bing Zhao
2013-07-23 2:17 ` [PATCH 16/21] mwifiex: replace mdelay with msleep Bing Zhao
2013-07-23 2:17 ` [PATCH 17/21] mwifiex: correction in mwifiex_check_fw_status() return status Bing Zhao
2013-07-23 2:17 ` [PATCH 18/21] mwifiex: handle driver initialization error paths Bing Zhao
2013-07-24 15:16 ` John W. Linville [this message]
2013-07-24 18:48 ` Bing Zhao
2013-07-24 19:02 ` John W. Linville
2013-07-26 4:25 ` Bing Zhao
2013-07-23 2:17 ` [PATCH 19/21] mwifiex: code rearrangement in sdio.c Bing Zhao
2013-07-24 15:17 ` John W. Linville
2013-07-23 2:17 ` [PATCH 20/21] mwifiex: remove duplicate structure host_cmd_tlv Bing Zhao
2013-07-23 2:17 ` [PATCH 21/21] mwifiex: modify mwifiex_ap_sta_limits to advertise support for P2P Bing Zhao
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=20130724151651.GD2385@tuxdriver.com \
--to=linville@tuxdriver.com \
--cc=akarwar@marvell.com \
--cc=bzhao@marvell.com \
--cc=frankh@marvell.com \
--cc=linux-wireless@vger.kernel.org \
--cc=nishants@marvell.com \
--cc=patila@marvell.com \
--cc=yogeshp@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.