iwlwifi: integrate with iwlmei

Hello Emmanuel Grumbach,

The patch 6d19a5eba5cd: "iwlwifi: integrate with iwlmei" from Nov 12, 2021, leads to the following Smatch static checker warning:

	drivers/net/wireless/intel/iwlwifi/mvm/ops.c:740 iwl_mvm_start_get_nvm()
	warn: inconsistent returns '&mvm->hw->wiphy->mtx'.

drivers/net/wireless/intel/iwlwifi/mvm/ops.c

    684 static int iwl_mvm_start_get_nvm(struct iwl_mvm *mvm)
    685 {
    686         struct iwl_trans *trans = mvm->trans;
    687         int ret;
    688 
    689         if (trans->csme_own) {
    690                 if (WARN(!mvm->mei_registered,
    691                          "csme is owner, but we aren't registered to iwlmei\n"))
    692                         goto get_nvm_from_fw;
    693 
    694                 mvm->mei_nvm_data = iwl_mei_get_nvm();
    695                 if (mvm->mei_nvm_data) {
    696                         /*
    697                          * mvm->mei_nvm_data is set and because of that,
    698                          * we'll load the NVM from the FW when we'll get
    699                          * ownership.
    700                          */
    701                         mvm->nvm_data =
    702                                 iwl_parse_mei_nvm_data(trans, trans->cfg,
    703                                                        mvm->mei_nvm_data, mvm->fw);
    704                         return 0;
    705                 }
    706 
    707                 IWL_ERR(mvm,
    708                         "Got a NULL NVM from CSME, trying to get it from the device\n");
    709         }
    710 
    711 get_nvm_from_fw:
    712         rtnl_lock();
    713         wiphy_lock(mvm->hw->wiphy);
    714         mutex_lock(&mvm->mutex);

This code takes three lines. I'm looking at linux-next next-20211129, so it's a little bit different. The original patch is buggy but it's made worse by a merge issue.

    715 
    716         ret = iwl_trans_start_hw(mvm->trans);
    717         if (ret) {
    718                 mutex_unlock(&mvm->mutex);
    719                 return ret;

This only drops one lock before returning. It should probably be a goto unlock; and we add an unlock at the end of the function. I would send a patch for that but it gets a bit confusing because of the merge. Emmanuel, could you take a look at this?

    720         }
    721 
    722         ret = iwl_run_init_mvm_ucode(mvm);
    723         if (ret && ret != -ERFKILL)
    724                 iwl_fw_dbg_error_collect(&mvm->fwrt, FW_DBG_TRIGGER_DRIVER);
    725         if (!ret && iwl_mvm_is_lar_supported(mvm)) {
    726                 mvm->hw->wiphy->regulatory_flags |= REGULATORY_WIPHY_SELF_MANAGED;
    727                 ret = iwl_mvm_init_mcc(mvm);
    728         }
    729 
    730         if (!iwlmvm_mod_params.init_dbg || !ret)
    731                 iwl_mvm_stop_device(mvm);
    732 
    733         mutex_unlock(&mvm->mutex);
    734         wiphy_unlock(mvm->hw->wiphy);
    735         rtnl_unlock();
    736 
    737         if (ret)
    738                 IWL_ERR(mvm, "Failed to run INIT ucode: %d\n", ret);
    739 
--> 740         return ret;
    741 }

regards,
dan carpenter

Hi Dan,

Luca has a patch in the pipe to fix this. Clearly, we had merge issues here. Luca, it looks like the commit:
"iwlwifi: mvm: unlock RTNL when starting HW fails" is the one we need.

What happened here is that, internally, we had two features that were implemented in a certain order (iwlmei before the load of the regdomain in INIT) and upstream they were merged the other way around. This caused merge issues that weren't handled properly apparently.

Luca, can you take a look?