| Message ID | 20200128093107.9740-1-dmoulding@me.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a9149d243f259ad8f02b1e23dfe8ba06128f15e1 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2,5.5] iwlwifi: mvm: Do not require PHY_SKU NVM section for 3168 devices | expand |
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
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?
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.
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
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.
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/
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;
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(-)