| Message ID | 20250410-mwifiex-drop-asynchronous-init-v1-3-6a212fa9185e@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mwifiex: drop asynchronous init waiting code | expand |
On Thu, Apr 10, 2025 at 12:28:45PM +0200, Sascha Hauer wrote: > Historically all commands sent to the mwifiex driver have been > asynchronous. The different commands sent during driver initialization > have been queued at once and only the final command has been waited > for being ready before finally starting the driver. > > This has been changed in 7bff9c974e1a ("mwifiex: send firmware > initialization commands synchronously"). With this the initialization > is finished once the last mwifiex_send_cmd_sync() (now > mwifiex_send_cmd()) has returned. This makes all the code used to > wait for the last initialization command to be finished unnecessary, > so it's removed in this patch. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 16 ---------------- > drivers/net/wireless/marvell/mwifiex/init.c | 5 +++-- > drivers/net/wireless/marvell/mwifiex/main.c | 12 ++---------- > drivers/net/wireless/marvell/mwifiex/main.h | 6 ------ > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ---- > drivers/net/wireless/marvell/mwifiex/util.c | 18 ------------------ > 6 files changed, 5 insertions(+), 56 deletions(-) The following hunk is missing in this patch. Will add next time. -------------------------------8<------------------------------- commit 707b4d85612123bee63b79947cb036211b59152f Author: Sascha Hauer <s.hauer@pengutronix.de> Date: Fri Apr 11 08:47:59 2025 +0200 fixup! wifi: mwifiex: drop asynchronous init waiting code diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c index ff094b5c32239..73298b0769c94 100644 --- a/drivers/net/wireless/marvell/mwifiex/main.c +++ b/drivers/net/wireless/marvell/mwifiex/main.c @@ -584,14 +584,6 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context) if (ret == -1) goto err_init_fw; - /* Wait for mwifiex_init to complete */ - 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; - } - maybe_quirk_fw_disable_ds(adapter); if (!adapter->wiphy) {
On Thu, Apr 10, 2025 at 12:28:45PM +0200, Sascha Hauer wrote: > Historically all commands sent to the mwifiex driver have been > asynchronous. The different commands sent during driver initialization > have been queued at once and only the final command has been waited > for being ready before finally starting the driver. > > This has been changed in 7bff9c974e1a ("mwifiex: send firmware > initialization commands synchronously"). > With this the initialization is finished once the last > mwifiex_send_cmd_sync() (now mwifiex_send_cmd()) has returned. Just for me, the rename/refactor happened in commit fa0ecbb9905d ("mwifiex: remove global variable cmd_wait_q_required"), in v3.14. > This makes all the code used to wait for the last initialization > command to be finished unnecessary, so it's removed in this patch. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 16 ---------------- > drivers/net/wireless/marvell/mwifiex/init.c | 5 +++-- > drivers/net/wireless/marvell/mwifiex/main.c | 12 ++---------- > drivers/net/wireless/marvell/mwifiex/main.h | 6 ------ > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ---- > drivers/net/wireless/marvell/mwifiex/util.c | 18 ------------------ > 6 files changed, 5 insertions(+), 56 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c > index 5573e2ded72f2..c07857c49a713 100644 > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c > @@ -900,18 +900,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter) > ret = mwifiex_process_sta_cmdresp(priv, cmdresp_no, resp); > } > > - /* Check init command response */ > - if (adapter->hw_status == MWIFIEX_HW_STATUS_INITIALIZING) { What about the code path from mwifiex_add_card()? mwifiex_add_card() -> hw_status = MWIFIEX_HW_STATUS_INITIALIZING -> mwifiex_init_hw_fw(adapter, true)) -> request_firmware_nowait(..., mwifiex_fw_dpc) mwifiex_fw_dpc() ... -> mwifiex_init_fw() -> adapter->hw_status = MWIFIEX_HW_STATUS_READY mwifiex_fw_dpc() is called asynchronously, is everything as safe as before, here? Francesco
On Fri, Apr 11, 2025 at 10:49:54AM +0200, Francesco Dolcini wrote: > On Thu, Apr 10, 2025 at 12:28:45PM +0200, Sascha Hauer wrote: > > Historically all commands sent to the mwifiex driver have been > > asynchronous. The different commands sent during driver initialization > > have been queued at once and only the final command has been waited > > for being ready before finally starting the driver. > > > > This has been changed in 7bff9c974e1a ("mwifiex: send firmware > > initialization commands synchronously"). > > > With this the initialization is finished once the last > > mwifiex_send_cmd_sync() (now mwifiex_send_cmd()) has returned. > > Just for me, the rename/refactor happened in commit fa0ecbb9905d > ("mwifiex: remove global variable cmd_wait_q_required"), in v3.14. > > > > This makes all the code used to wait for the last initialization > > command to be finished unnecessary, so it's removed in this patch. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 16 ---------------- > > drivers/net/wireless/marvell/mwifiex/init.c | 5 +++-- > > drivers/net/wireless/marvell/mwifiex/main.c | 12 ++---------- > > drivers/net/wireless/marvell/mwifiex/main.h | 6 ------ > > drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ---- > > drivers/net/wireless/marvell/mwifiex/util.c | 18 ------------------ > > 6 files changed, 5 insertions(+), 56 deletions(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c > > index 5573e2ded72f2..c07857c49a713 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c > > +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c > > @@ -900,18 +900,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter) > > ret = mwifiex_process_sta_cmdresp(priv, cmdresp_no, resp); > > } > > > > - /* Check init command response */ > > - if (adapter->hw_status == MWIFIEX_HW_STATUS_INITIALIZING) { > > What about the code path from mwifiex_add_card()? > > mwifiex_add_card() > -> hw_status = MWIFIEX_HW_STATUS_INITIALIZING > -> mwifiex_init_hw_fw(adapter, true)) > -> request_firmware_nowait(..., mwifiex_fw_dpc) > > mwifiex_fw_dpc() > ... > -> mwifiex_init_fw() > -> adapter->hw_status = MWIFIEX_HW_STATUS_READY > > mwifiex_fw_dpc() is called asynchronously, is everything as safe as > before, here? Yes, mwifiex_fw_dpc() is called asynchronously, but the first change in my patch is in mwifiex_init_fw() which is already called asynchronously, so my patch doesn't really change anything here. Sascha
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c index 5573e2ded72f2..c07857c49a713 100644 --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c @@ -900,18 +900,6 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter) ret = mwifiex_process_sta_cmdresp(priv, cmdresp_no, resp); } - /* Check init command response */ - if (adapter->hw_status == MWIFIEX_HW_STATUS_INITIALIZING) { - if (ret) { - mwifiex_dbg(adapter, ERROR, - "%s: cmd %#x failed during\t" - "initialization\n", __func__, cmdresp_no); - mwifiex_init_fw_complete(adapter); - return -1; - } else if (adapter->last_init_cmd == cmdresp_no) - adapter->hw_status = MWIFIEX_HW_STATUS_INIT_DONE; - } - if (adapter->curr_cmd) { if (adapter->curr_cmd->wait_q_enabled) adapter->cmd_wait_q.status = ret; @@ -1030,10 +1018,6 @@ mwifiex_cmd_timeout_func(struct timer_list *t) mwifiex_cancel_pending_ioctl(adapter); } } - if (adapter->hw_status == MWIFIEX_HW_STATUS_INITIALIZING) { - mwifiex_init_fw_complete(adapter); - return; - } if (adapter->if_ops.device_dump) adapter->if_ops.device_dump(adapter); diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c index dd24e8b140655..dd2c17d946d7c 100644 --- a/drivers/net/wireless/marvell/mwifiex/init.c +++ b/drivers/net/wireless/marvell/mwifiex/init.c @@ -480,14 +480,12 @@ int mwifiex_init_lock_list(struct mwifiex_adapter *adapter) * - Initialize the private structure * - Add BSS priority tables to the adapter structure * - For each interface, send the init commands to firmware - * - Send the first command in command pending queue, if available */ int mwifiex_init_fw(struct mwifiex_adapter *adapter) { int ret; struct mwifiex_private *priv; u8 i, first_sta = true; - int is_cmd_pend_q_empty; adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING; @@ -526,6 +524,9 @@ int mwifiex_init_fw(struct mwifiex_adapter *adapter) adapter->hw_status = MWIFIEX_HW_STATUS_READY; + if (adapter->if_ops.init_fw_port) + adapter->if_ops.init_fw_port(adapter); + return 0; } diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c index 3f1b8be5ad26c..ff094b5c32239 100644 --- a/drivers/net/wireless/marvell/mwifiex/main.c +++ b/drivers/net/wireless/marvell/mwifiex/main.c @@ -354,13 +354,6 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter) if (adapter->cmd_resp_received) { adapter->cmd_resp_received = false; mwifiex_process_cmdresp(adapter); - - /* call mwifiex back when init_fw is done */ - if (adapter->hw_status == MWIFIEX_HW_STATUS_INIT_DONE) { - adapter->hw_status = MWIFIEX_HW_STATUS_READY; - mwifiex_init_fw_complete(adapter); - maybe_quirk_fw_disable_ds(adapter); - } } /* Check if we need to confirm Sleep Request @@ -587,7 +580,6 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context) goto err_dnld_fw; } - adapter->init_wait_q_woken = false; ret = mwifiex_init_fw(adapter); if (ret == -1) goto err_init_fw; @@ -600,6 +592,8 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context) goto err_init_fw; } + maybe_quirk_fw_disable_ds(adapter); + if (!adapter->wiphy) { if (mwifiex_register_cfg80211(adapter)) { mwifiex_dbg(adapter, ERROR, @@ -1555,7 +1549,6 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter) adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING; clear_bit(MWIFIEX_SURPRISE_REMOVED, &adapter->work_flags); - init_waitqueue_head(&adapter->init_wait_q); clear_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags); adapter->hs_activated = false; clear_bit(MWIFIEX_IS_CMD_TIMEDOUT, &adapter->work_flags); @@ -1723,7 +1716,6 @@ mwifiex_add_card(void *card, struct completion *fw_done, adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING; clear_bit(MWIFIEX_SURPRISE_REMOVED, &adapter->work_flags); - init_waitqueue_head(&adapter->init_wait_q); clear_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags); adapter->hs_activated = false; init_waitqueue_head(&adapter->hs_activate_wait_q); diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h index 63f1c900e0967..35d13eada0868 100644 --- a/drivers/net/wireless/marvell/mwifiex/main.h +++ b/drivers/net/wireless/marvell/mwifiex/main.h @@ -239,7 +239,6 @@ struct mwifiex_dbg { enum MWIFIEX_HARDWARE_STATUS { MWIFIEX_HW_STATUS_READY, MWIFIEX_HW_STATUS_INITIALIZING, - MWIFIEX_HW_STATUS_INIT_DONE, MWIFIEX_HW_STATUS_RESET, MWIFIEX_HW_STATUS_NOT_READY }; @@ -865,8 +864,6 @@ struct mwifiex_adapter { unsigned long work_flags; u32 fw_release_number; u8 intf_hdr_len; - u16 init_wait_q_woken; - wait_queue_head_t init_wait_q; void *card; struct mwifiex_if_ops if_ops; atomic_t bypass_tx_pending; @@ -919,7 +916,6 @@ struct mwifiex_adapter { struct cmd_ctrl_node *curr_cmd; /* spin lock for command */ spinlock_t mwifiex_cmd_lock; - u16 last_init_cmd; struct timer_list cmd_timer; struct list_head cmd_free_q; /* spin lock for cmd_free_q */ @@ -1060,8 +1056,6 @@ void mwifiex_free_priv(struct mwifiex_private *priv); int mwifiex_init_fw(struct mwifiex_adapter *adapter); -int mwifiex_init_fw_complete(struct mwifiex_adapter *adapter); - void mwifiex_shutdown_drv(struct mwifiex_adapter *adapter); int mwifiex_dnld_fw(struct mwifiex_adapter *, struct mwifiex_fw_image *); diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c index af38744eddaa9..7a8a74df86ab1 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c @@ -2433,9 +2433,5 @@ int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init) ret = mwifiex_send_cmd(priv, HostCmd_CMD_11N_CFG, HostCmd_ACT_GEN_SET, 0, &tx_cfg, true); - if (init) - /* set last_init_cmd before sending the command */ - priv->adapter->last_init_cmd = HostCmd_CMD_11N_CFG; - return ret; } diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c index 1f1f6280a0f25..daa363f082612 100644 --- a/drivers/net/wireless/marvell/mwifiex/util.c +++ b/drivers/net/wireless/marvell/mwifiex/util.c @@ -115,24 +115,6 @@ static struct mwifiex_debug_data items[] = { static int num_of_items = ARRAY_SIZE(items); -/* - * Firmware initialization complete callback handler. - * - * This function wakes up the function waiting on the init - * wait queue for the firmware initialization to complete. - */ -int mwifiex_init_fw_complete(struct mwifiex_adapter *adapter) -{ - - if (adapter->hw_status == MWIFIEX_HW_STATUS_READY) - if (adapter->if_ops.init_fw_port) - adapter->if_ops.init_fw_port(adapter); - - adapter->init_wait_q_woken = true; - wake_up_interruptible(&adapter->init_wait_q); - return 0; -} - /* * This function sends init/shutdown command * to firmware.
Historically all commands sent to the mwifiex driver have been asynchronous. The different commands sent during driver initialization have been queued at once and only the final command has been waited for being ready before finally starting the driver. This has been changed in 7bff9c974e1a ("mwifiex: send firmware initialization commands synchronously"). With this the initialization is finished once the last mwifiex_send_cmd_sync() (now mwifiex_send_cmd()) has returned. This makes all the code used to wait for the last initialization command to be finished unnecessary, so it's removed in this patch. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/net/wireless/marvell/mwifiex/cmdevt.c | 16 ---------------- drivers/net/wireless/marvell/mwifiex/init.c | 5 +++-- drivers/net/wireless/marvell/mwifiex/main.c | 12 ++---------- drivers/net/wireless/marvell/mwifiex/main.h | 6 ------ drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ---- drivers/net/wireless/marvell/mwifiex/util.c | 18 ------------------ 6 files changed, 5 insertions(+), 56 deletions(-)