Date:   Tue, 30 Nov 2021 10:32:26 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     emmanuel.grumbach@intel.com,
        Stephen Rothwell <sfr@canb.auug.org.au>
Cc:     linux-wireless@vger.kernel.org
Subject: [bug report] iwlwifi: integrate with iwlmei
Message-ID: <20211130073226.GA21360@kili>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.10.1 (2018-07-13)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?q61VWBL/rX9yLw1OCH7B8eZpOi/iMvklI5Rbj9uxCGzffKE7m7WJ1jb+wJpt?=
 =?us-ascii?Q?l4M3g/DWWZOoH3EnJExFsaIHpQ+hiN9L/UIY/TUo/J31eEFoU828USpgCKSE?=
 =?us-ascii?Q?ioWpQqkbdaRBsZzgIJworzby1PFGcLlk9q2G7A2JI0V3y9NQI+LyDuMaKtSj?=
 =?us-ascii?Q?9EYpg0IKKciT9HM1Ya22hoITDn/NCIrgCUS99tVsLdMRCMs/EiDZXmY1DIxq?=
 =?us-ascii?Q?rzt1kBM7RFtAJW9oBE333ulx2j4s518uDFauEVb939zAdiH/g90S5NV3+OGc?=
 =?us-ascii?Q?ac0dV+LjwQIjvBXXU8FIz3HHSrNwwt8Vle/qt+vN2pmQvUwUeAQWMK/0VjbC?=
 =?us-ascii?Q?EDKNxcTjBxAWKKwg8pjXarTrx2eLXXHd6mCpnudldKCs70br1nXP/B1Jnful?=
 =?us-ascii?Q?51V08orrVBnOLfhq2Q4ZlaCDidmrdJQpxN63M3IZCxKFZisiec/NTPi4afQE?=
 =?us-ascii?Q?4728YST7IGFCVywKAD6smPrkSlKMfnzpQfg0E+yAaPdIvSPDtQ5kkpP71ulf?=
 =?us-ascii?Q?n/EZW33WLurO9p9iTyrvXhzgDNVq8Sbxb1SReyRtxL0C4M04pMXuN32oZYn9?=
 =?us-ascii?Q?wz2wMZB/jPvlI8vPgnbeFgOVyg7WhDerlpjib+KDnFdPM0E/KS28qKAjap+a?=
 =?us-ascii?Q?84LZHbkCIbaImP5p8dMV+sBHcEhtqNy9pAF9xBdNStWM7PShIdK2fBAVU4r9?=
 =?us-ascii?Q?XfzKS2+9OIbiTsIh/lApHoTBFfld3ZDLa4Wvg+HAbl9l82pG9HVb/qwi7Ci6?=
 =?us-ascii?Q?ruYBdw1DmsNKpkWl63qhhBbUC+iXw5EAHoWTeoT8OE+/WT+6aDpokkJry+qg?=
 =?us-ascii?Q?2Zuvqi0faEa2mGanCFmsfG4NkVAermhjNKRGG25HgmSFrNfy5xahkufYdLxy?=
 =?us-ascii?Q?Bly04ECy72UWR8tHWt9s/4m2rYnLV6hYUB4Qx06LlRmzTqjaDJ3txunU6KIT?=
 =?us-ascii?Q?bYoUJmTP3c2+wvgca/1UdRE9KJZHR1EbuXMDAJxY7Sy93qFZ1IsVr1UJC0XG?=
 =?us-ascii?Q?xN4p0CDLwObKb0bs4cJx5dxc8RB2DHpnXd1xfL7HqFqqyXpCvC1HUSEUwuX9?=
 =?us-ascii?Q?VR+e9KBfrZh4tgxoOpDXC3lJ0CzCTTtvh4pf+R+sh+UbfOi3SMOrEn+vklGh?=
 =?us-ascii?Q?181YPMPQZJtm25I3Cyh+WoLEKaEFj9EeCL8LfVplSApzi7fzco8Do7kn61zK?=
 =?us-ascii?Q?EPRbLJHtnLKo7PJGFdUFybvNB88RWa0EHTIkgV1q1Nza2YlL8kfxUa9wwfQz?=
 =?us-ascii?Q?WAXqTf0Tgdrcgj5IdUwni6YVZR8YMYO9flKc++eaWh0UQuf14ND7bLLAG9lO?=
 =?us-ascii?Q?RlKNLmdL3yfPLYDwKobRJti5lgvffOkF1nm2Cc2VQYuRr66YTDf5GG8daMNQ?=
 =?us-ascii?Q?Bo6Iekx1YBkOUZgjouGbhuGDDQJ3p3KxpVzhAeH5QOc9LizOJh0G4L+Kfm92?=
 =?us-ascii?Q?5Mxz0FDtNqeHlXe1OfUbfM66+rzlP0oKRwKcYoFVLK3lu8001HvL1xyGhq3K?=
 =?us-ascii?Q?Qq5TGaB4DUGjhzt+BjFOPTlkcsSeF2Uz4VKEdiuMeIsddBaWq8YsujHvmttD?=
 =?us-ascii?Q?57Pmug8Mns5GHZvbnrRl1R83vWHGFiT4acgxll+7QX6x/b0JwAeLJvEpDWqq?=
 =?us-ascii?Q?v4vpxrID0tbSMCNmOBCaYHnXUMbdXJTwjvdfJLt/4U+vhsRsDIv8lNOvjiLx?=
 =?us-ascii?Q?/M6zPny4Dj+HkbTepBoGPHwtDQA=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f1857521-f653-4f4d-9fd6-08d9b3d39691
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2021 07:32:39.6322
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dHCovJPtywXyvSaZV4Gju5IIhJpnfZjJq0TK5e7NKSVJXp/SJeYMHbi34AWQ59GCpqYX5+lDWPLJQscTHQIjQIVhkRzS/h/60rz9oARZh2U=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1424
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10183 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 malwarescore=0 mlxscore=0
 suspectscore=0 mlxlogscore=999 spamscore=0 phishscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111300042
X-Proofpoint-ORIG-GUID: VSruH0BNCe8QawmAI-1sWA58HiQDAPLC
X-Proofpoint-GUID: VSruH0BNCe8QawmAI-1sWA58HiQDAPLC
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

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