Re: iwlwifi 0000:3a:00.0: Microcode SW error detected. Restarting 0x2000000.

[Johannes Berg <johannes@sipsolutions.net>]

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