On Wed, 2022-01-12 at 10:27 +0100, Johannes Berg wrote: > > > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c > > b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c > > index 863fec150e53..f13825185094 100644 > > --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c > > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c > > @@ -834,6 +834,9 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm > > *mvm) > > offsetof(struct > > iwl_geo_tx_power_profiles_cmd_v4, ops) != > > offsetof(struct > > iwl_geo_tx_power_profiles_cmd_v5, ops)); > > > > + if (!iwl_sar_geo_support(&mvm->fwrt)) > > + return -EOPNOTSUPP; > > + > > /* the ops field is at the same spot for all versions, so > > set in v1 */ > > cmd.v1.ops = cpu_to_le32(IWL_PER_CHAIN_OFFSET_SET_TABLES); > > I was going to say it should probably return 0, but the caller looks > a > bit fishy too? > > ret = iwl_mvm_sar_init(mvm); > if (ret == 0) > ret = iwl_mvm_sar_geo_init(mvm); > else if (ret < 0) > goto error; > > ret = iwl_mvm_sgom_init(mvm); > > should that "else" be removed? Yeah, I noticed the same thing when I checked the return value... I don't think we want to abort everything if SAR GEO init failed, so maybe we should just remove the return value from the function? -- Cheers, Luca.
