From: Sebastian Gottschall <s.gottschall@dd-wrt.com> To: Ben Greear <greearb@candelatech.com>, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org Cc: kvalo@codeaurora.org Subject: Re: [PATCH v2] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter Date: Sat, 28 Apr 2018 02:35:51 +0200 [thread overview] Message-ID: <e7036e06-9f4d-05bc-ea77-e0a6876f3394@dd-wrt.com> (raw) In-Reply-To: <ebaf2806-a63b-5611-41f4-817b1f471fd1@candelatech.com> > I did some testing with the patch below. i mean you tested your own patch here but i dont see results for mine. i tested my patch in the same way, just not on firmware side but i logged the values for rxnss_override and the looked correct all the time > > The CHAIMASK_ERR is a debug log from FW that I added to help make sure > the patch is > acting as desired. The first hex is an identifier, second is the > value passed in, > third is phymode, 4th is the tx-chain-mask for 160Mhz frames. > > On station side, when associating a 4x4 9984 station to 9984 > configured for nss4, 160Mhz, I see: > [86376.620303] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 1560 rxnss_override: 0x80000009 nss160: 2 > spatial-streams: 4 > ath10k-fw: ts: 15229 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000009 0x0000000f 0x00000003 > > On station side, when associating a 4x4 9984 station with chain-mask > of 0x3 (2x2) to 9984 configured for nss4, 160Mhz, I see: > > [86147.569319] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 780 rxnss_override: 0x80000000 nss160: 1 > spatial-streams: 2 > ath10k-fw: ts: 6807 args: 4 RATE_CTRL(19) vid: 255 CHAINMASK_ERR(03) > 0x000000af 0x80000000 0x0000000f 0x00000001 > > > On AP side, when associating a 4x4 9984 station to 9984 configured for > 160Mhz, I see: > > [11167.635324] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 1560 rxnss_override: 0x80000009 nss160: 2 > spatial-streams: 4 > ath10k-fw: ts: 72917 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000009 0x0000000f 0x00000003 > > On AP side, when associating a 4x4 9984 station with chain-mask of 0x3 > (2x2) to 9984 configured for nss4, 160Mhz, I see: > > [11422.887181] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 780 rxnss_override: 0x80000000 nss160: 1 > spatial-streams: 2 > ath10k-fw: ts: 334266 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000000 0x0000000f 0x00000001 > > On AP side, when associating a 4x4 9984 station configured for 80Mhz > instead of 160, > then logging from firmware indicates full 4x4 rates are supported for > 80Mhz and below, > and the rxnss_override does not have the (1<<31) bit set: > > ath10k-fw: ts: 519211 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x00000000 0x0000000a 0x00000001 > > > So, I think this might be a better fix for this problem (included > inline for discussion, > probably white-space damaged by email client: > > > diff --git a/drivers/net/wireless/ath/ath10k/mac.c > b/drivers/net/wireless/ath/ath10k/mac.c > index e1ad983..8bce916 100644 > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -2860,18 +2860,39 @@ static void ath10k_peer_assoc_h_vht(struct > ath10k *ar, > arg->peer_vht_rates.rx_max_rate, > arg->peer_vht_rates.rx_mcs_set, > arg->peer_vht_rates.tx_max_rate, > arg->peer_vht_rates.tx_mcs_set); > > - if (arg->peer_vht_rates.rx_max_rate && > - (sta->vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK)) { > - switch (arg->peer_vht_rates.rx_max_rate) { > + if (arg->peer_phymode == MODE_11AC_VHT80_80 || > + arg->peer_phymode == MODE_11AC_VHT160) { > + int nss160; > + int rx = arg->peer_vht_rates.rx_max_rate; > + /* Deal with cases where chainmask has been decreased. > + * All known chips that support 160Mhz can do only 1/2 of > + * the available chains at 160Mhz. > + */ > + rx = min((int)(arg->peer_num_spatial_streams * 390), rx); > + > + switch (rx) { > + /* When a NIC shows up that can do 4x4 at 160Mhz, its > + * max-rate should be higher, and we can set nss160 > + * to 4 here. > + */ > case 1560: > /* Must be 2x2 at 160Mhz is all it can do. */ > - arg->peer_bw_rxnss_override = 2; > + nss160 = 2; > break; > - case 780: > - /* Can only do 1x1 at 160Mhz (Long Guard Interval) */ > - arg->peer_bw_rxnss_override = 1; > + default: > + /* Assume we can only do 1x1 at 160Mhz */ > + nss160 = 1; > break; > } > + > + arg->peer_bw_rxnss_override = ((nss160 - 1) | /* 160Mhz nss */ > + ((nss160 - 1) << 3) | /* 80+80 nss */ > + BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET)); > + > + ath10k_warn(ar, "NIC rx-max-rate: %d calculated-max: %d > rxnss_override: 0x%x nss160: %d spatial-streams: %d\n", > + arg->peer_vht_rates.rx_max_rate, rx, > + arg->peer_bw_rxnss_override, nss160, > + arg->peer_num_spatial_streams); > } > } > > @@ -3115,9 +3136,9 @@ static int ath10k_peer_assoc_prepare(struct > ath10k *ar, > ath10k_peer_assoc_h_crypto(ar, vif, sta, arg); > ath10k_peer_assoc_h_rates(ar, vif, sta, arg); > ath10k_peer_assoc_h_ht(ar, vif, sta, arg); > + ath10k_peer_assoc_h_phymode(ar, vif, sta, arg); > ath10k_peer_assoc_h_vht(ar, vif, sta, arg); > ath10k_peer_assoc_h_qos(ar, vif, sta, arg); > - ath10k_peer_assoc_h_phymode(ar, vif, sta, arg); > > ath10k_peer_assoc_h_rate_overrides(ar, vif, sta, arg); > > > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c > b/drivers/net/wireless/ath/ath10k/wmi.c > index 8eeeab0..365d509 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.c > +++ b/drivers/net/wireless/ath/ath10k/wmi.c > @@ -7442,12 +7442,8 @@ ath10k_wmi_peer_assoc_fill_10_4(struct ath10k > *ar, void *buf, > struct wmi_10_4_peer_assoc_complete_cmd *cmd = buf; > > ath10k_wmi_peer_assoc_fill_10_2(ar, buf, arg); > - if (arg->peer_bw_rxnss_override) > - cmd->peer_bw_rxnss_override = > - __cpu_to_le32((arg->peer_bw_rxnss_override - 1) | > - BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET)); > - else > - cmd->peer_bw_rxnss_override = 0; > + > + cmd->peer_bw_rxnss_override = > __cpu_to_le32(arg->peer_bw_rxnss_override); > } > > static int > > > > Thanks, > Ben > -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottschall@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565
WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Gottschall <s.gottschall@dd-wrt.com> To: Ben Greear <greearb@candelatech.com>, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org Cc: kvalo@codeaurora.org Subject: Re: [PATCH v2] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter Date: Sat, 28 Apr 2018 02:35:51 +0200 [thread overview] Message-ID: <e7036e06-9f4d-05bc-ea77-e0a6876f3394@dd-wrt.com> (raw) In-Reply-To: <ebaf2806-a63b-5611-41f4-817b1f471fd1@candelatech.com> > I did some testing with the patch below. i mean you tested your own patch here but i dont see results for mine. i tested my patch in the same way, just not on firmware side but i logged the values for rxnss_override and the looked correct all the time > > The CHAIMASK_ERR is a debug log from FW that I added to help make sure > the patch is > acting as desired. The first hex is an identifier, second is the > value passed in, > third is phymode, 4th is the tx-chain-mask for 160Mhz frames. > > On station side, when associating a 4x4 9984 station to 9984 > configured for nss4, 160Mhz, I see: > [86376.620303] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 1560 rxnss_override: 0x80000009 nss160: 2 > spatial-streams: 4 > ath10k-fw: ts: 15229 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000009 0x0000000f 0x00000003 > > On station side, when associating a 4x4 9984 station with chain-mask > of 0x3 (2x2) to 9984 configured for nss4, 160Mhz, I see: > > [86147.569319] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 780 rxnss_override: 0x80000000 nss160: 1 > spatial-streams: 2 > ath10k-fw: ts: 6807 args: 4 RATE_CTRL(19) vid: 255 CHAINMASK_ERR(03) > 0x000000af 0x80000000 0x0000000f 0x00000001 > > > On AP side, when associating a 4x4 9984 station to 9984 configured for > 160Mhz, I see: > > [11167.635324] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 1560 rxnss_override: 0x80000009 nss160: 2 > spatial-streams: 4 > ath10k-fw: ts: 72917 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000009 0x0000000f 0x00000003 > > On AP side, when associating a 4x4 9984 station with chain-mask of 0x3 > (2x2) to 9984 configured for nss4, 160Mhz, I see: > > [11422.887181] ath10k_pci 0000:04:00.0: NIC rx-max-rate: 1560 > calculated-max: 780 rxnss_override: 0x80000000 nss160: 1 > spatial-streams: 2 > ath10k-fw: ts: 334266 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x80000000 0x0000000f 0x00000001 > > On AP side, when associating a 4x4 9984 station configured for 80Mhz > instead of 160, > then logging from firmware indicates full 4x4 rates are supported for > 80Mhz and below, > and the rxnss_override does not have the (1<<31) bit set: > > ath10k-fw: ts: 519211 args: 4 RATE_CTRL(19) vid: 255 > CHAINMASK_ERR(03) 0x000000af 0x00000000 0x0000000a 0x00000001 > > > So, I think this might be a better fix for this problem (included > inline for discussion, > probably white-space damaged by email client: > > > diff --git a/drivers/net/wireless/ath/ath10k/mac.c > b/drivers/net/wireless/ath/ath10k/mac.c > index e1ad983..8bce916 100644 > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -2860,18 +2860,39 @@ static void ath10k_peer_assoc_h_vht(struct > ath10k *ar, > arg->peer_vht_rates.rx_max_rate, > arg->peer_vht_rates.rx_mcs_set, > arg->peer_vht_rates.tx_max_rate, > arg->peer_vht_rates.tx_mcs_set); > > - if (arg->peer_vht_rates.rx_max_rate && > - (sta->vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK)) { > - switch (arg->peer_vht_rates.rx_max_rate) { > + if (arg->peer_phymode == MODE_11AC_VHT80_80 || > + arg->peer_phymode == MODE_11AC_VHT160) { > + int nss160; > + int rx = arg->peer_vht_rates.rx_max_rate; > + /* Deal with cases where chainmask has been decreased. > + * All known chips that support 160Mhz can do only 1/2 of > + * the available chains at 160Mhz. > + */ > + rx = min((int)(arg->peer_num_spatial_streams * 390), rx); > + > + switch (rx) { > + /* When a NIC shows up that can do 4x4 at 160Mhz, its > + * max-rate should be higher, and we can set nss160 > + * to 4 here. > + */ > case 1560: > /* Must be 2x2 at 160Mhz is all it can do. */ > - arg->peer_bw_rxnss_override = 2; > + nss160 = 2; > break; > - case 780: > - /* Can only do 1x1 at 160Mhz (Long Guard Interval) */ > - arg->peer_bw_rxnss_override = 1; > + default: > + /* Assume we can only do 1x1 at 160Mhz */ > + nss160 = 1; > break; > } > + > + arg->peer_bw_rxnss_override = ((nss160 - 1) | /* 160Mhz nss */ > + ((nss160 - 1) << 3) | /* 80+80 nss */ > + BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET)); > + > + ath10k_warn(ar, "NIC rx-max-rate: %d calculated-max: %d > rxnss_override: 0x%x nss160: %d spatial-streams: %d\n", > + arg->peer_vht_rates.rx_max_rate, rx, > + arg->peer_bw_rxnss_override, nss160, > + arg->peer_num_spatial_streams); > } > } > > @@ -3115,9 +3136,9 @@ static int ath10k_peer_assoc_prepare(struct > ath10k *ar, > ath10k_peer_assoc_h_crypto(ar, vif, sta, arg); > ath10k_peer_assoc_h_rates(ar, vif, sta, arg); > ath10k_peer_assoc_h_ht(ar, vif, sta, arg); > + ath10k_peer_assoc_h_phymode(ar, vif, sta, arg); > ath10k_peer_assoc_h_vht(ar, vif, sta, arg); > ath10k_peer_assoc_h_qos(ar, vif, sta, arg); > - ath10k_peer_assoc_h_phymode(ar, vif, sta, arg); > > ath10k_peer_assoc_h_rate_overrides(ar, vif, sta, arg); > > > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c > b/drivers/net/wireless/ath/ath10k/wmi.c > index 8eeeab0..365d509 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.c > +++ b/drivers/net/wireless/ath/ath10k/wmi.c > @@ -7442,12 +7442,8 @@ ath10k_wmi_peer_assoc_fill_10_4(struct ath10k > *ar, void *buf, > struct wmi_10_4_peer_assoc_complete_cmd *cmd = buf; > > ath10k_wmi_peer_assoc_fill_10_2(ar, buf, arg); > - if (arg->peer_bw_rxnss_override) > - cmd->peer_bw_rxnss_override = > - __cpu_to_le32((arg->peer_bw_rxnss_override - 1) | > - BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET)); > - else > - cmd->peer_bw_rxnss_override = 0; > + > + cmd->peer_bw_rxnss_override = > __cpu_to_le32(arg->peer_bw_rxnss_override); > } > > static int > > > > Thanks, > Ben > -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottschall@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2018-04-28 0:35 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-26 9:28 [PATCH v2] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter s.gottschall 2018-04-26 9:28 ` s.gottschall 2018-04-26 15:30 ` Ben Greear 2018-04-26 15:30 ` Ben Greear 2018-04-26 20:21 ` Sebastian Gottschall 2018-04-26 20:21 ` Sebastian Gottschall 2018-04-26 20:35 ` Ben Greear 2018-04-26 20:35 ` Ben Greear 2018-04-27 4:40 ` Sebastian Gottschall 2018-04-27 4:40 ` Sebastian Gottschall 2018-04-27 16:07 ` Ben Greear 2018-04-27 16:07 ` Ben Greear 2018-04-27 18:54 ` Sebastian Gottschall 2018-04-27 18:54 ` Sebastian Gottschall 2018-04-27 21:57 ` Ben Greear 2018-04-27 21:57 ` Ben Greear 2018-04-28 0:24 ` Sebastian Gottschall 2018-04-28 0:24 ` Sebastian Gottschall 2018-04-28 15:08 ` Ben Greear 2018-04-28 15:08 ` Ben Greear 2018-04-28 0:35 ` Sebastian Gottschall [this message] 2018-04-28 0:35 ` Sebastian Gottschall
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e7036e06-9f4d-05bc-ea77-e0a6876f3394@dd-wrt.com \ --to=s.gottschall@dd-wrt.com \ --cc=ath10k@lists.infradead.org \ --cc=greearb@candelatech.com \ --cc=kvalo@codeaurora.org \ --cc=linux-wireless@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.