| Message ID | 572a14ee-d1b5-7b85-d292-51f583f34f17@dd-wrt.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 17 May 2016 at 13:43, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: > Attached you will find a testing patch for VHT160 support. i tested it today > on a QCA9984 device and it seems to work. > feel free to make any corrections Hi, Send patches inline, please. Preferably via git send-email. It's a lot easier to review them this way. From a quick glance vht capabilities setup looks weird: > if (ar->vht_cap_info & IEEE80211_VHT_CAP_SHORT_GI_160) { > vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ; > vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ; > } Why? Is this really necessary? I would expect IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ and IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ to be already set in ar->vht_cap_info if the chip supports 160 or 80+80. You also don't seem to handle 80+80 because you just as phymode to 160 regardless of center frequencies: > !ath10k_peer_assoc_h_vht_masked(vht_mcs_mask)) { > if (sta->bandwidth == IEEE80211_STA_RX_BW_80) > phymode = MODE_11AC_VHT80; > else if (sta->bandwidth == IEEE80211_STA_RX_BW_160) > phymode = MODE_11AC_VHT160; > else if (sta->bandwidth == IEEE80211_STA_RX_BW_40) > phymode = MODE_11AC_VHT40; > else if (sta->bandwidth == IEEE80211_STA_RX_BW_20) You should probably look at sta->vht_cap and associated chanctx to check if it isn't 80+80. Micha?
+ re-adding ath10k list On 17 May 2016 at 14:05, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: >Am 17.05.2016 um 14:00 schrieb Michal Kazior: >> On 17 May 2016 at 13:43, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: >>> Attached you will find a testing patch for VHT160 support. i tested it today >>> on a QCA9984 device and it seems to work. >>> feel free to make any corrections >> Hi, >> >> Send patches inline, please. Preferably via git send-email. It's a lot >> easier to review them this way. > unfortunaty i dont work with git right now and i wanted to send it to > the list first for getting comments. > since the patch is also very small, it should be that complicated >> >> From a quick glance vht capabilities setup looks weird: >> >>> if (ar->vht_cap_info & IEEE80211_VHT_CAP_SHORT_GI_160) { >>> vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ; >>> vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ; >>> } >> Why? Is this really necessary? > the firmware does not report correct flags. just SHORT_GI_160. > i reviewed the firmware sourcecodes and found out that this is maybe a > misstake of the QCA developers. > so we need to add the 160MHZ support flags locally using the > SHORT_GI_160 which is always set on QCA9984. > otherwise mac80211 will not be capable of running vht160. I'm not sure if SGI160 implies 80+80. >> I would expect IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ and >> IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ to be already set in >> ar->vht_cap_info if the chip supports 160 or 80+80. >me too. but it isnt :-) Sigh. A comment explaining that would be really nice. >> You also don't seem to handle 80+80 because you just as phymode to 160 >> regardless of center frequencies: >> >>> !ath10k_peer_assoc_h_vht_masked(vht_mcs_mask)) { >>> if (sta->bandwidth == IEEE80211_STA_RX_BW_80) >>> phymode = MODE_11AC_VHT80; >>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_160) >>> phymode = MODE_11AC_VHT160; >>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_40) >>> phymode = MODE_11AC_VHT40; >>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_20) > there is no 80+80 STA_RX_BW flag. i dont know how to handle this. and > yes, just 160 mhz is tested right now. right now it looks like mac80211 > is not 80+80 ready. Hmm.. I wouldn't advertise 80+80 support then. All the more since there's no way of telling whether hw/fw actually supports it in the first place. Michal
Am 17.05.2016 um 14:18 schrieb Michal Kazior: > + re-adding ath10k list > > > On 17 May 2016 at 14:05, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: >> Am 17.05.2016 um 14:00 schrieb Michal Kazior: >>> On 17 May 2016 at 13:43, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: >>>> Attached you will find a testing patch for VHT160 support. i tested it today >>>> on a QCA9984 device and it seems to work. >>>> feel free to make any corrections >>> Hi, >>> >>> Send patches inline, please. Preferably via git send-email. It's a lot >>> easier to review them this way. >> unfortunaty i dont work with git right now and i wanted to send it to >> the list first for getting comments. >> since the patch is also very small, it should be that complicated >>> From a quick glance vht capabilities setup looks weird: >>> >>>> if (ar->vht_cap_info & IEEE80211_VHT_CAP_SHORT_GI_160) { >>>> vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ; >>>> vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ; >>>> } >>> Why? Is this really necessary? >> the firmware does not report correct flags. just SHORT_GI_160. >> i reviewed the firmware sourcecodes and found out that this is maybe a >> misstake of the QCA developers. >> so we need to add the 160MHZ support flags locally using the >> SHORT_GI_160 which is always set on QCA9984. >> otherwise mac80211 will not be capable of running vht160. > I'm not sure if SGI160 implies 80+80. according to the firmware sourcecode of 9984 it seems so. thats the only information source i have > > >>> I would expect IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ and >>> IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ to be already set in >>> ar->vht_cap_info if the chip supports 160 or 80+80. >> me too. but it isnt :-) > Sigh. A comment explaining that would be really nice. there is no comment available. the only information source is the firmware sourcecode for the 9984 which i reviewed. let me give you a small snippet #if defined(CONFIG_160MHZ_SUPPORT) #define VHT_CAP_WITH_STBC_1SS_VHT160 ((VHT_CAP_WITH_STBC_1SS) | (WMI_VHT_CAP_SGI_160MHZ)) #define VHT_CAP_WITH_STBC_2SS_VHT160 ((VHT_CAP_WITH_STBC_2SS) | (WMI_VHT_CAP_SGI_160MHZ)) #define VHT_CAP_WITH_STBC_3SS_VHT160 ((VHT_CAP_WITH_STBC_3SS) | (WMI_VHT_CAP_SGI_160MHZ)) #endif #if defined(CONFIG_160MHZ_SUPPORT) dev_info->vht_cap_info = VHT_CAP_WITH_STBC_1SS_VHT160; #else dev_info->vht_cap_info = VHT_CAP_WITH_STBC_1SS_VHT80; #endif so the firmware simply does not set the correct flags. thats all. > > >>> You also don't seem to handle 80+80 because you just as phymode to 160 >>> regardless of center frequencies: >>> >>>> !ath10k_peer_assoc_h_vht_masked(vht_mcs_mask)) { >>>> if (sta->bandwidth == IEEE80211_STA_RX_BW_80) >>>> phymode = MODE_11AC_VHT80; >>>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_160) >>>> phymode = MODE_11AC_VHT160; >>>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_40) >>>> phymode = MODE_11AC_VHT40; >>>> else if (sta->bandwidth == IEEE80211_STA_RX_BW_20) >> there is no 80+80 STA_RX_BW flag. i dont know how to handle this. and >> yes, just 160 mhz is tested right now. right now it looks like mac80211 >> is not 80+80 ready. > Hmm.. I wouldn't advertise 80+80 support then. All the more since > there's no way of telling whether hw/fw actually supports it in the > first place. we can leave it out at the end, but the firmware seem to support it. so far i just tested 160 in ap mode yet, but found other things in ath10k which need to be fixed. i'm on it right now > > > Michal >
Michal Kazior <michal.kazior@tieto.com> writes: > On 17 May 2016 at 13:43, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: >> Attached you will find a testing patch for VHT160 support. i tested it today >> on a QCA9984 device and it seems to work. >> feel free to make any corrections > > Send patches inline, please. Preferably via git send-email. It's a lot > easier to review them this way. Also remember to read Developer's Certificate of Origin from Documentation/SubmittingPatches and add Signed-off-by. Otherwise we cannot use the patch.
Sebastian Gottschall <s.gottschall@dd-wrt.com> writes: > Attached you will find a testing patch for VHT160 support. i tested it > today on a QCA9984 device and it seems to work. > feel free to make any corrections Signed-off-by is missing so I can't take this. Can you resend, please? Also remember to CC linux-wireless.
Am 14.09.2016 um 12:37 schrieb Valo, Kalle: > Sebastian Gottschall <s.gottschall@dd-wrt.com> writes: > >> Attached you will find a testing patch for VHT160 support. i tested it >> today on a QCA9984 device and it seems to work. >> feel free to make any corrections > Signed-off-by is missing so I can't take this. Can you resend, please i can send you a updated version. it tested with vht160 so far. however. on codeaurore there is a newer firmware for 9984 which provides the correct vht flags straight from the firmware but its crashing in vht160 mode. can you ask the firmware team to provide a bugfixed version for the official ath10k? with 80p80 and vht160 support? 80p80 works somewhat bug has wrong vht flags per station, vht160 works with my quirks so far which are all related to the buggy firmware > > Also remember to CC linux-wireless. >
Sebastian Gottschall <s.gottschall@dd-wrt.com> writes: > Am 14.09.2016 um 12:37 schrieb Valo, Kalle: >> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes: >> >>> Attached you will find a testing patch for VHT160 support. i tested it >>> today on a QCA9984 device and it seems to work. >>> feel free to make any corrections >> Signed-off-by is missing so I can't take this. Can you resend, please > > i can send you a updated version. it tested with vht160 so far. Great, thanks. > however. on codeaurore there is a newer firmware for 9984 which > provides the correct vht flags straight from the firmware but its > crashing in vht160 mode. can you ask the firmware team to provide a > bugfixed version for the official ath10k? with 80p80 and vht160 > support? 80p80 works somewhat bug has wrong vht flags per station, > vht160 works with my quirks so far which are all related to the buggy > firmware I can ask but I need more info. What's the exact location of the newer firmware image? Is there any detailed description about the wrong vht flags bug?
Index: drivers/net/wireless/ath/ath9k/gpio.c =================================================================== Index: drivers/net/wireless/ath/ath10k/mac.c =================================================================== --- drivers/net/wireless/ath/ath10k/mac.c (revision 2910) +++ drivers/net/wireless/ath/ath10k/mac.c (working copy) @@ -539,10 +539,14 @@ case NL80211_CHAN_WIDTH_80: phymode = MODE_11AC_VHT80; break; + case NL80211_CHAN_WIDTH_160: + phymode = MODE_11AC_VHT160; + break; + case NL80211_CHAN_WIDTH_80P80: + phymode = MODE_11AC_VHT80_80; + break; case NL80211_CHAN_WIDTH_5: case NL80211_CHAN_WIDTH_10: - case NL80211_CHAN_WIDTH_80P80: - case NL80211_CHAN_WIDTH_160: phymode = MODE_UNKNOWN; break; } @@ -2499,6 +2503,8 @@ !ath10k_peer_assoc_h_vht_masked(vht_mcs_mask)) { if (sta->bandwidth == IEEE80211_STA_RX_BW_80) phymode = MODE_11AC_VHT80; + else if (sta->bandwidth == IEEE80211_STA_RX_BW_160) + phymode = MODE_11AC_VHT160; else if (sta->bandwidth == IEEE80211_STA_RX_BW_40) phymode = MODE_11AC_VHT40; else if (sta->bandwidth == IEEE80211_STA_RX_BW_20) @@ -4185,6 +4191,10 @@ vht_cap.cap |= val; } + if (ar->vht_cap_info & IEEE80211_VHT_CAP_SHORT_GI_160) { + vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ; + vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ; + } mcs_map = 0; for (i = 0; i < 8; i++) { @@ -6798,6 +6808,9 @@ bw = WMI_PEER_CHWIDTH_80MHZ; break; case IEEE80211_STA_RX_BW_160: + bw = WMI_PEER_CHWIDTH_160MHZ; + break; + default: ath10k_warn(ar, "Invalid bandwidth %d in rc update for %pM\n", sta->bandwidth, sta->addr); bw = WMI_PEER_CHWIDTH_20MHZ; Index: drivers/net/wireless/ath/ath10k/htt_rx.c =================================================================== --- drivers/net/wireless/ath/ath10k/htt_rx.c (revision 2910) +++ drivers/net/wireless/ath/ath10k/htt_rx.c (working copy) @@ -707,6 +707,10 @@ /* 80MHZ */ case 2: status->vht_flag |= RX_VHT_FLAG_80MHZ; + break; + case 3: + status->vht_flag |= RX_VHT_FLAG_160MHZ; + break; } status->flag |= RX_FLAG_VHT; @@ -2193,6 +2197,8 @@ case MODE_11AC_VHT20: case MODE_11AC_VHT40: case MODE_11AC_VHT80: + case MODE_11AC_VHT160: + case MODE_11AC_VHT80_80: band = NL80211_BAND_5GHZ; break; case MODE_11G: Index: drivers/net/wireless/ath/ath10k/wmi.h =================================================================== --- drivers/net/wireless/ath/ath10k/wmi.h (revision 2910) +++ drivers/net/wireless/ath/ath10k/wmi.h (working copy) @@ -1723,8 +1723,10 @@ MODE_11AC_VHT20_2G = 11, MODE_11AC_VHT40_2G = 12, MODE_11AC_VHT80_2G = 13, - MODE_UNKNOWN = 14, - MODE_MAX = 14 + MODE_11AC_VHT80_80 = 14, + MODE_11AC_VHT160 = 15, + MODE_UNKNOWN = 16, + MODE_MAX = 16 }; static inline const char *ath10k_wmi_phymode_str(enum wmi_phy_mode mode) @@ -1752,6 +1754,10 @@ return "11ac-vht40"; case MODE_11AC_VHT80: return "11ac-vht80"; + case MODE_11AC_VHT160: + return "11ac-vht160"; + case MODE_11AC_VHT80_80: + return "11ac-vht80+80"; case MODE_11AC_VHT20_2G: return "11ac-vht20-2g"; case MODE_11AC_VHT40_2G: @@ -5748,6 +5754,7 @@ WMI_PEER_CHWIDTH_20MHZ = 0, WMI_PEER_CHWIDTH_40MHZ = 1, WMI_PEER_CHWIDTH_80MHZ = 2, + WMI_PEER_CHWIDTH_160MHZ = 3, }; enum wmi_peer_param {