[for,v5.2] iwlwifi: mvm: disable TX-AMSDU on older NICs
diff mbox series

Message ID 20190703081049.28831-1-luca@coelho.fi
State Superseded
Delegated to: Kalle Valo
Headers show
Series
  • [for,v5.2] iwlwifi: mvm: disable TX-AMSDU on older NICs
Related show

Commit Message

Luca Coelho July 3, 2019, 8:10 a.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

On older NICs, we occasionally see issues with A-MSDU support,
where the commands in the FIFO get confused and then we see an
assert EDC because the next command in the FIFO isn't TX.

We've tried to isolate this issue and understand where it comes
from, but haven't found any errors in building the A-MSDU in
software.

At least for now, disable A-MSDU support on older hardware so
that users can use it again without fearing the assert.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=203315.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Luca Coelho July 3, 2019, 8:14 a.m. UTC | #1
On Wed, 2019-07-03 at 11:10 +0300, Luca Coelho wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> On older NICs, we occasionally see issues with A-MSDU support,
> where the commands in the FIFO get confused and then we see an
> assert EDC because the next command in the FIFO isn't TX.
> 
> We've tried to isolate this issue and understand where it comes
> from, but haven't found any errors in building the A-MSDU in
> software.
> 
> At least for now, disable A-MSDU support on older hardware so
> that users can use it again without fearing the assert.
> 
> This fixes https://bugzilla.kernel.org/show_bug.cgi?id=203315.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---

Hi Dave,

This is an important fix for a bug that has been reported by several
users in bugzilla (and elsewhere).  It fixes FW crashes that disrupt
throughput and connectivity in general in very popular devices (Intel's
WiFi 7000 and 8000 series).

I know it's a bit late for v5.2, but if possible, it would be great to
take this.  Kalle is on vacation, so we agreed that I would send it
directly to you.

Please let me know if this is okay or if we should send this to -next
and get it into v5.2 via stable releases instead.

Thanks!

--
Cheers,
Luca.
Kalle Valo July 3, 2019, 11:45 a.m. UTC | #2
Luca Coelho <luca@coelho.fi> writes:

> On Wed, 2019-07-03 at 11:10 +0300, Luca Coelho wrote:
>> From: Johannes Berg <johannes.berg@intel.com>
>> 
>> On older NICs, we occasionally see issues with A-MSDU support,
>> where the commands in the FIFO get confused and then we see an
>> assert EDC because the next command in the FIFO isn't TX.
>> 
>> We've tried to isolate this issue and understand where it comes
>> from, but haven't found any errors in building the A-MSDU in
>> software.
>> 
>> At least for now, disable A-MSDU support on older hardware so
>> that users can use it again without fearing the assert.
>> 
>> This fixes https://bugzilla.kernel.org/show_bug.cgi?id=203315.
>> 
>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> ---
>
> Hi Dave,
>
> This is an important fix for a bug that has been reported by several
> users in bugzilla (and elsewhere).  It fixes FW crashes that disrupt
> throughput and connectivity in general in very popular devices (Intel's
> WiFi 7000 and 8000 series).
>
> I know it's a bit late for v5.2, but if possible, it would be great to
> take this.  Kalle is on vacation, so we agreed that I would send it
> directly to you.

Acked-by: Kalle Valo <kvalo@codeaurora.org>
Brian Norris July 24, 2019, 11:55 p.m. UTC | #3
On Wed, Jul 3, 2019 at 4:46 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> Luca Coelho <luca@coelho.fi> writes:
> > Hi Dave,
> >
> > This is an important fix for a bug that has been reported by several
> > users in bugzilla (and elsewhere).  It fixes FW crashes that disrupt
> > throughput and connectivity in general in very popular devices (Intel's
> > WiFi 7000 and 8000 series).
> >
> > I know it's a bit late for v5.2, but if possible, it would be great to
> > take this.  Kalle is on vacation, so we agreed that I would send it
> > directly to you.
>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>

5.2 has come and gone, and I believe Kalle has awoken from vacation.
Can we get this for 5.3?

Brian
Kalle Valo July 25, 2019, 5:55 a.m. UTC | #4
Brian Norris <briannorris@chromium.org> writes:

> On Wed, Jul 3, 2019 at 4:46 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>> Luca Coelho <luca@coelho.fi> writes:
>> > Hi Dave,
>> >
>> > This is an important fix for a bug that has been reported by several
>> > users in bugzilla (and elsewhere).  It fixes FW crashes that disrupt
>> > throughput and connectivity in general in very popular devices (Intel's
>> > WiFi 7000 and 8000 series).
>> >
>> > I know it's a bit late for v5.2, but if possible, it would be great to
>> > take this.  Kalle is on vacation, so we agreed that I would send it
>> > directly to you.
>>
>> Acked-by: Kalle Valo <kvalo@codeaurora.org>
>
> 5.2 has come and gone, and I believe Kalle has awoken from vacation.

Yeah, I have sort of crawled out of my cave ;)

> Can we get this for 5.3?

Ah, this slipped through the cracks. Thanks for pointing it out. I now
took this patch from Luca and assigned to myself in patchwork and I'll
queue this for v5.3.

https://patchwork.kernel.org/patch/11029027/

Patch
diff mbox series

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index fdbabca0280e..3a076e5e319f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -468,7 +468,19 @@  int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
 	ieee80211_hw_set(hw, SUPPORTS_VHT_EXT_NSS_BW);
 	ieee80211_hw_set(hw, BUFF_MMPDU_TXQ);
 	ieee80211_hw_set(hw, STA_MMPDU_TXQ);
-	ieee80211_hw_set(hw, TX_AMSDU);
+	/*
+	 * On older devices, enabling TX A-MSDU occasionally leads to
+	 * something getting messed up, the command read from the FIFO
+	 * gets out of sync and isn't a TX command, so that we have an
+	 * assert EDC.
+	 *
+	 * It's not clear where the bug is, but since we didn't used to
+	 * support A-MSDU until moving the mac80211 iTXQs, just leave it
+	 * for older devices. We also don't see this issue on any newer
+	 * devices.
+	 */
+	if (mvm->cfg->device_family >= IWL_DEVICE_FAMILY_9000)
+		ieee80211_hw_set(hw, TX_AMSDU);
 	ieee80211_hw_set(hw, TX_FRAG_LIST);
 
 	if (iwl_mvm_has_tlc_offload(mvm)) {