>
> The return value of request_module() being 0 does not mean that the driver
> which was requested has loaded. To properly check that the driver was
> loaded each driver can use internal mechanisms to vet the driver is now
> present. The helper try_then_request_module() was added to help with
> this, allowing drivers to specify their own validation as the first argument.
>
> On iwlwifi the use case is a bit more complicated given that the value we
> need to check for is protected with a mutex later used on the
> module_init() of the module we are asking for. If we were to lock and
> request_module() we'd deadlock. iwlwifi needs its own wrapper then so it
> can handle its special locking requirements.
>
> Signed-off-by: Luis R. Rodriguez <mcg...@kernel.org>
I don't see the problem with the current code. We don't assume that everything
has been
loaded immediately after request_module returns. We just free the intermediate
firmware
structures that won't be using anymore. What happens here is that after
request_module
returns, we patiently wait until it is loaded, and when that happens,
iwl{d,m}vm's init function
will be called. That one is the one that continues the flow by calling:
ret = iwl_opmode_register("iwlmvm", &iwl_mvm_ops);
(for the iwlmvm case).
Where am I wrong here?
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 34 ++++++++++++++++++++---
> -----
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> index e198d6f5fcea..6beb92d19ea7 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> @@ -1231,6 +1231,29 @@ static void _iwl_op_mode_stop(struct iwl_drv
> *drv)
> }
> }
>
> +static void iwlwifi_try_load_op(struct iwlwifi_opmode_table *op,
> + struct iwl_drv *drv)
> +{
> + int ret = 0;
> +
> + ret = request_module("%s", op->name);
> + if (ret)
> + goto out;
> +
> + mutex_lock(&iwlwifi_opmode_table_mtx);
> + if (!op->ops)
> + ret = -ENOENT;
> + mutex_unlock(&iwlwifi_opmode_table_mtx);
> +
> +out:
> +#ifdef CONFIG_IWLWIFI_OPMODE_MODULAR
> + if (ret)
> + IWL_ERR(drv,
> + "failed to load module %s (error %d), is dynamic
> loading enabled?\n",
> + op->name, ret);
> +#endif
> +}
> +
> /**
> * iwl_req_fw_callback - callback when firmware was loaded
> *
> @@ -1471,15 +1494,8 @@ static void iwl_req_fw_callback(const struct
> firmware *ucode_raw, void *context)
> * else from proceeding if the module fails to load
> * or hangs loading.
> */
> - if (load_module) {
> - err = request_module("%s", op->name);
> -#ifdef CONFIG_IWLWIFI_OPMODE_MODULAR
> - if (err)
> - IWL_ERR(drv,
> - "failed to load module %s (error %d), is
> dynamic loading enabled?\n",
> - op->name, err);
> -#endif
> - }
> + if (load_module)
> + iwlwifi_try_load_op(op, drv);
> goto free;
>
> try_again:
> --
> 2.11.0
