> > 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 <mcgrof@xxxxxxxxxx> 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