[PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Dan Moulding]

The logic for checking required NVM sections was recently fixed in
commit b3f20e098293 ("iwlwifi: mvm: fix NVM check for 3168
devices"). However, with that fixed the else is now taken for 3168
devices and within the else clause there is a mandatory check for the
PHY_SKU section. This causes the parsing to fail for 3168 devices.

The PHY_SKU section is really only mandatory for the IWL_NVM_EXT
layout (the phy_sku parameter of iwl_parse_nvm_data is only used when
the NVM type is IWL_NVM_EXT). So this changes the PHY_SKU section
check so that it's only mandatory for IWL_NVM_EXT.

Fixes: b3f20e098293 ("iwlwifi: mvm: fix NVM check for 3168 devices")
Signed-off-by: Dan Moulding <dmoulding@me.com>
---
v2: Fixed incorrect commit title in commit references in the commit message

 drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
index 46128a2a9c6e..e98ce380c7b9 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
@@ -308,7 +308,8 @@ iwl_parse_nvm_sections(struct iwl_mvm *mvm)
 		}
 
 		/* PHY_SKU section is mandatory in B0 */
-		if (!mvm->nvm_sections[NVM_SECTION_TYPE_PHY_SKU].data) {
+		if (mvm->trans->cfg->nvm_type == IWL_NVM_EXT &&
+		    !mvm->nvm_sections[NVM_SECTION_TYPE_PHY_SKU].data) {
 			IWL_ERR(mvm,
 				"Can't parse phy_sku in B0, empty sections\n");
 			return NULL;
-- 
2.24.1

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Dan Moulding]

This is just a friendly reminder that this patch has been submitted,
for what looks like a fairly major regression in iwlwifi that impacts
(as far as I can tell) *all* 3168 devices. The regression is in the
v5.5.x series and was for a while back-ported to the stable trees, but
luckily was noticed before the releases were made.

There are at least a few bug reports for this regression:

https://bugzilla.kernel.org/show_bug.cgi?id=206329
https://bugs.gentoo.org/706810
https://lkml.org/lkml/2020/2/7/811
https://bbs.archlinux.org/viewtopic.php?id=252603

The Gentoo maintainers have already applied this patch to their Linux
sources and marked their bug report "fixed". But it would be really
nice if we could get this regression fixed in the next stable v5.5.x
release.

Thanks for your attention!

-- Dan

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Kalle Valo]

Dan Moulding <dmoulding@me.com> writes:

> This is just a friendly reminder that this patch has been submitted,
> for what looks like a fairly major regression in iwlwifi that impacts
> (as far as I can tell) *all* 3168 devices. The regression is in the
> v5.5.x series and was for a while back-ported to the stable trees, but
> luckily was noticed before the releases were made.
>
> There are at least a few bug reports for this regression:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=206329
> https://bugs.gentoo.org/706810
> https://lkml.org/lkml/2020/2/7/811
> https://bbs.archlinux.org/viewtopic.php?id=252603
>
> The Gentoo maintainers have already applied this patch to their Linux
> sources and marked their bug report "fixed". But it would be really
> nice if we could get this regression fixed in the next stable v5.5.x
> release.

I'll queue this directly to wireless-drivers. Intel folks, are you ok
with this?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Grumbach, Emmanuel]

Hi Kalle and Dan,

On Wed, 2020-02-12 at 16:46 +0200, Kalle Valo wrote:
> Dan Moulding <dmoulding@me.com> writes:
> 
> > This is just a friendly reminder that this patch has been
> > submitted,
> > for what looks like a fairly major regression in iwlwifi that
> > impacts
> > (as far as I can tell) *all* 3168 devices. The regression is in the
> > v5.5.x series and was for a while back-ported to the stable trees,
> > but
> > luckily was noticed before the releases were made.
> > 
> > There are at least a few bug reports for this regression:
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=206329
> > https://bugs.gentoo.org/706810
> > https://lkml.org/lkml/2020/2/7/811
> > https://bbs.archlinux.org/viewtopic.php?id=252603
> > 
> > The Gentoo maintainers have already applied this patch to their
> > Linux
> > sources and marked their bug report "fixed". But it would be really
> > nice if we could get this regression fixed in the next stable
> > v5.5.x
> > release.
> 
> I'll queue this directly to wireless-drivers. Intel folks, are you ok
> with this?
> 

The only person who really understand what goes on here is Luca, he is
also the one who touched this area I think. Luca is OOO and should be
back next week I believe.
The patch looks sane and it fixes issues for people, so go ahead and
merge it please. If Luca wants to make changes on top of this, he can
ask to do those changes on top of that patch.

Thanks. 

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Luciano Coelho]

On Thu, 2020-02-13 at 07:39 +0000, Grumbach, Emmanuel wrote:
> Hi Kalle and Dan,
> 
> On Wed, 2020-02-12 at 16:46 +0200, Kalle Valo wrote:
> > Dan Moulding <dmoulding@me.com> writes:
> > 
> > > This is just a friendly reminder that this patch has been
> > > submitted,
> > > for what looks like a fairly major regression in iwlwifi that
> > > impacts
> > > (as far as I can tell) *all* 3168 devices. The regression is in the
> > > v5.5.x series and was for a while back-ported to the stable trees,
> > > but
> > > luckily was noticed before the releases were made.
> > > 
> > > There are at least a few bug reports for this regression:
> > > 
> > > https://bugzilla.kernel.org/show_bug.cgi?id=206329
> > > https://bugs.gentoo.org/706810
> > > https://lkml.org/lkml/2020/2/7/811
> > > https://bbs.archlinux.org/viewtopic.php?id=252603
> > > 
> > > The Gentoo maintainers have already applied this patch to their
> > > Linux
> > > sources and marked their bug report "fixed". But it would be really
> > > nice if we could get this regression fixed in the next stable
> > > v5.5.x
> > > release.
> > 
> > I'll queue this directly to wireless-drivers. Intel folks, are you ok
> > with this?
> > 
> 
> The only person who really understand what goes on here is Luca, he is
> also the one who touched this area I think. Luca is OOO and should be
> back next week I believe.
> The patch looks sane and it fixes issues for people, so go ahead and
> merge it please. If Luca wants to make changes on top of this, he can
> ask to do those changes on top of that patch.

Thanks for the fix, discussions and merging while I was away! This
indeed looks correct and I'll take this patch to our internal tree now
to align it with the upstream code.

I guess this should be sent to stable v5.5 as well.  Dan, would you
like to do that?

--
Cheers,
Luca.

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Dan Moulding]

On March 4, 2020 at 2:06 AM, Luciano Coelho <luciano.coelho@intel.com>
wrote:

> I guess this should be sent to stable v5.5 as well. Dan, would you
> like to do that?

Yes, sure I will do that. The regression was briefly discussed[1] on
the stable list as it was almost backported to the older trees (5.4.x,
etc). Greg K-H requested that I ping the stable list once the patch
lands in Linus's tree. So I've been keeping an eye on it, waiting for
the merges to happen. Let me know if there is something else I should
be doing to expedite the process if you think that's needed.

-- Dan

[1] https://lore.kernel.org/stable/20200205093102.GB1164405@kroah.com/

Re: [PATCH v2 5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

[Kalle Valo]

Dan Moulding <dmoulding@me.com> wrote:

> The logic for checking required NVM sections was recently fixed in
> commit b3f20e098293 ("iwlwifi: mvm: fix NVM check for 3168
> devices"). However, with that fixed the else is now taken for 3168
> devices and within the else clause there is a mandatory check for the
> PHY_SKU section. This causes the parsing to fail for 3168 devices.
> 
> The PHY_SKU section is really only mandatory for the IWL_NVM_EXT
> layout (the phy_sku parameter of iwl_parse_nvm_data is only used when
> the NVM type is IWL_NVM_EXT). So this changes the PHY_SKU section
> check so that it's only mandatory for IWL_NVM_EXT.
> 
> Fixes: b3f20e098293 ("iwlwifi: mvm: fix NVM check for 3168 devices")
> Signed-off-by: Dan Moulding <dmoulding@me.com>

Patch applied to wireless-drivers.git, thanks.

a9149d243f25 iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices

-- 
https://patchwork.kernel.org/patch/11353871/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches