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

["Coelho, Luciano" <luciano.coelho@intel.com>]

Hi,

On Wed, 2022-01-12 at 09:34 +0100, Johannes Berg wrote:
> On Tue, 2022-01-11 at 20:47 -0500, Len Brown wrote:
> > 
> > I recently installed Fedora 35, and the card worked fine.
> > But when I built and booted a 5.15 or 5.16 upstream kernel from src,
> > the card fails at initialization and I've not found a way to revive it.
> > (dmesg below)
> 
> Ouch.
> 
> > [   11.411533] iwlwifi 0000:3a:00.0: Loaded firmware version: 17.3216344376.0 3160-17.ucode
> > [   11.412815] iwlwifi 0000:3a:00.0: 0x00000038 | BAD_COMMAND
> 
> ...
> 
> > [   11.688317] iwlwifi 0000:3a:00.0: FW error in SYNC CMD PER_CHAIN_LIMIT_OFFSET_CMD
> 
> Looks like somehow this slipped through and we're sending a new command
> with an old firmware.
> 
> 
> Something like this might help?
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> index 7ee4802a5ef1..56b9363a9111 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> @@ -1026,7 +1026,9 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm *mvm)
>  	/* 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);
>  
> -	if (cmd_ver == 5) {
> +	if (cmd_ver == IWL_FW_CMD_VER_UNKNOWN) {
> +		return 0;
> +	} else if (cmd_ver == 5) {
>  		len = sizeof(cmd.v5);
>  		n_bands = ARRAY_SIZE(cmd.v5.table[0]);
>  		n_profiles = ACPI_NUM_GEO_PROFILES_REV3;
> 
> 
> Luca, are you aware of anything in this area?

I was not aware, not.  I looked into this briefly this morning, and I
noticed that we should be doing this:

	if (!iwl_sar_geo_support(&mvm->fwrt))
		return -EOPNOTSUPP;

...also in the iwl_mvm_sar_geo_init() function.

There was some refactoring in this area, to move the initialization to
the init phase, and that must be causing this.  I suspect this patch:

commit 78a19d5285d93e281b40c3b8d5a959fbbd2fe006
Author:     Miri Korenblit <miriam.rachel.korenblit@intel.com>
AuthorDate: Thu Aug 5 14:21:56 2021 +0300
Commit:     Luca Coelho <luciano.coelho@intel.com>
CommitDate: Thu Aug 26 23:36:10 2021 +0300

    iwlwifi: mvm: Read the PPAG and SAR tables at INIT stage

...which was introduced in v5.15.


TL;DR, Johannes patch should help with the problem, but maybe this
would be better?

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);
 

We had some issues with the versioning and support flags for this
feature in the past, so the iwl_sar_geo_supported() function is not
that straight-forward...

--
Cheers,
Luca.