Peter Oh <peter.oh@xxxxxxxxxxxxxxxxx> writes: > On 11/06/2017 01:02 AM, Sven Eckelmann wrote: >> On Montag, 6. November 2017 09:28:42 CET Sebastian Gottschall wrote: >>> Am 06.11.2017 um 09:23 schrieb Sven Eckelmann: >>>> On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote: >>>>> the assumption made in this patch is obviously wrong (at least for more >>>>> recent firmwares and 9984) >>>>> my log is flooded with messages like >>>>> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats >>>>> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats >>>>> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats >>>>> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats >>>>> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats >> [...] >>>> This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. And for >>>> WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approach. But >>>> the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basically >>>> untouched. >>> then a question follows up. is this check really neccessary? >> Until we find out what the heck VHT MCS 15 should mean in this context - maybe. >> But to the message - no, the message is most likely not necessary for each >> received "invalid" peer tx stat. > > This validation check expects peer tx stat packets from FW contain > reasonable values and gives warning if values are different from > expectation. The problem comes from the assumption that "it always > contains reasonable stat value" which is wrong at least based on my > results. For instance, the reason MCS == 15 is because all 4 bits of > mcs potion set to 1, not because FW really sets it to 15 > intentionally. when the mcs potion bits are set to all 1s, the other > bits such as nss are also set to all 1s. > Hence it looks FW passed totally invalid stat info to me. > > I don't have preference on whether it's better to split VHT and HT > check or not, but it is more appropriate to change that warning level > to debug level. I think we should add a special case to not print the warning if mcs == 15 until we figure out what it means. -- Kalle Valo
