Bug 206479
Summary: | [iwlwifi, v5.5 regression, bisected] Wireless-AC 3168NGW no longer initializes | ||
---|---|---|---|
Product: | Networking | Reporter: | Duane Robertson (duane) |
Component: | Wireless | Assignee: | networking_wireless (networking_wireless) |
Status: | CLOSED PATCH_ALREADY_AVAILABLE | ||
Severity: | normal | CC: | belegdol, briannorris, bugzilla, grawcho, janek+kernel, joanbrugueram, kernel, me, stiletto |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 5.5 | Subsystem: | |
Regression: | Yes | Bisected commit-id: | |
Attachments: |
kernel log with error
Fix error validation in iwl_mvm_sar_geo_init / iwl_sar_geo_init after 39c1a9728f93 |
Description
Duane Robertson
2020-02-10 01:59:01 UTC
My work-around is to use the iwlwifi driver from v5.4. Completely a shot in the dark since I'm not familiar with the driver code and I can't reproduce the problem but... Something that looks a bit suspicious in that "iwlwifi: refactor the SAR tables from mvm to acpi" commit is that whereas before iwl_mvm_sar_geo_init used to return early without sending a command if iwl_mvm_sar_geo_support or iwl_mvm_sar_get_wgds_table failed, with that commit now that those calls are split into a separate function iwl_sar_geo_init, it will go ahead anyway and send the command anyway. Unless I am missing something this seems like a behaviour change in what should be a refactor. Any chance it does work if you apply the following patch (in addition to the one from #206329)? Again, I'm just guessing from the code without being able to reproduce, so sorry if this doesn't work. --- drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c index c09624d8d..8a7f12498 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c @@ -753,6 +753,7 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm *mvm) cmd.geo_cmd.ops = cpu_to_le32(IWL_PER_CHAIN_OFFSET_SET_TABLES); iwl_sar_geo_init(&mvm->fwrt, cmd.geo_cmd.table); + return 0; cmd.geo_cmd.table_revision = cpu_to_le32(mvm->fwrt.geo_rev); -- 2.25.0 Nice! That seems to work, so far, in both the vanilla kernel and gentoo 5.5.2-r1. I see no errors in dmesg or signs of any problems on the network. I'm not sure what would be required to fix this in the general case. Created attachment 287283 [details]
Fix error validation in iwl_mvm_sar_geo_init / iwl_sar_geo_init after 39c1a9728f93
Here's a patch that should (hopefully) fix the problem correctly by not sending the GEO_TX_POWER_LIMIT command in the same cases it was doing before 39c1a9728f93.
I haven't been able to test this on a 100% real case but I've found a way to hack up the driver to get a trace like Duane's. Applying this patch fixes that.
Attachment 287283 [details] works on the Motile.
Great, this confirms that the cause of the problem is the change in the error handling logic in 39c1a9728f93. If so, and since this change was likely accidental, there should hopefully be relatively little problem getting this patch accepted. (In reply to Duane Robertson from comment #5) > Attachment 287283 [details] works on the Motile. Works for me too could this be a duplicate of https://bugzilla.kernel.org/show_bug.cgi?id=206395? The upstream fix for that bug landed in 5.6: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0433ae556ec8fb588a0735ddb09d3eb9806df479 Yes, this and that bug are the same. 5.6 appears to have fixed the issue, at least in the gentoo sources. Just for the record here, bugzilla.kernel.org is really not the preferred way to reach mainline Linux developers. Some people may make best efforts to watch this bug tracker, but especially in cases where you have a bisection, it's preferred to reach out to linux-wireless@vger.kernel.org (perhaps including the bugzilla link, for more details). The icing on the cake here is that the patch provided at comment #4 is effectively identical to the patch developed and accepted independently at comment #8, just with a month delay. There's a little more info on bug reporting here: https://www.kernel.org/doc/html/latest/admin-guide/reporting-bugs.html#identify-who-to-notify Regardless, thanks for reporting. @Brian Norris: Thanks, this is good to know. I assumed that the devs would take a look at the Bugzilla, even if sparingly. Maybe they generally do, but for some reason iwlwifi seemed to be at a standstill at that time (c.f. bug 206329). Anyway, I guess this is an indication I should get more familiar with the kernel dev. procedures, so I'll check your link out (I often run mainline RCs or nigthlies so I may be able to catch some issues early now and then). |