On Wed, 2022-01-12 at 09:33 +0000, Coelho, Luciano wrote: > 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? > Well the only real failure path there is "we cannot send the command", in which case we might as well abort? IOW, we already return 0 in the cases where we don't have the data or something else happened (also for !CONFIG_ACPI). johannes
