Re: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-21 Thread Johannes Berg
On Tue, 2022-07-19 at 00:28 +0200, Linus Lüssing wrote:
> 
> Therefore fixing this within mac80211: For an aggergated AMPDU only
> update the RX "last_rate" variable from the last sub-frame of an
> aggregate. In theory, without hardware bugs, rate/bandwidth info
> should be the same for all sub-frames of an aggregate anyway.
> 

What if other drivers do it only on the first? :)

I'd be more inclined to squeeze in a "RATE_INVALID" flag or so somewhere
there in the rx status, and make it depend on that.

johannes

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-20 Thread Toke Høiland-Jørgensen
Linus Lüssing  writes:

> On 19/07/2022 17:03, Adrian Chadd wrote:
>> Hi!
>> 
>> It's not a hardware bug. Dating back to the original AR5416 11n chip,
>> most flags aren't valid for subframes in an aggregate. Only the final
>> frame has valid flags. This was explicitly covered internally way back
>> when.
>
> Ah, thanks for the clarification! I see it in the datasheet for the 
> QCA9531, too, now. And thanks for the confirmation, that what we are 
> doing so far is not correct for ath9k.
>
> Words 0+2 are valid for all RX descriptors, 0+2+11 valid for the last RX 
> descriptor of each packet and 0-11 for the last RX descriptor of an 
> aggregate or last RX descriptor of a stand-alone packet. Or in other 
> words, word 4, which contains the 20 vs. 40 MHz indicator, is invalid 
> for any aggregate sub-frame other than the last one. I can rename that 
> in the commit message.
>
>
> Another approach that also came to my mind was introducing more explicit 
> flags in cfg80211.h's "struct rate_info", like a RATE_INFO_BW_UNKNOWN in 
> "enum rate_info_bw" and/or RATE_INFO_FLAGS_UNKNOWN in "enum 
> rate_info_flags". And setting those flags in ath9k_cmn_process_rate().
>
> The current approach is smaller though, as it simply uses the already 
> existing flags. If anyone has any preferences, please let me know.

I have no objections to doing it in mac80211 like you're proposing here :)

-Toke


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-19 Thread Linus Lüssing

On 19/07/2022 17:03, Adrian Chadd wrote:

Hi!

It's not a hardware bug. Dating back to the original AR5416 11n chip,
most flags aren't valid for subframes in an aggregate. Only the final
frame has valid flags. This was explicitly covered internally way back
when.


Ah, thanks for the clarification! I see it in the datasheet for the 
QCA9531, too, now. And thanks for the confirmation, that what we are 
doing so far is not correct for ath9k.


Words 0+2 are valid for all RX descriptors, 0+2+11 valid for the last RX 
descriptor of each packet and 0-11 for the last RX descriptor of an 
aggregate or last RX descriptor of a stand-alone packet. Or in other 
words, word 4, which contains the 20 vs. 40 MHz indicator, is invalid 
for any aggregate sub-frame other than the last one. I can rename that 
in the commit message.



Another approach that also came to my mind was introducing more explicit 
flags in cfg80211.h's "struct rate_info", like a RATE_INFO_BW_UNKNOWN in 
"enum rate_info_bw" and/or RATE_INFO_FLAGS_UNKNOWN in "enum 
rate_info_flags". And setting those flags in ath9k_cmn_process_rate().


The current approach is smaller though, as it simply uses the already 
existing flags. If anyone has any preferences, please let me know.


Regards, Linus

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


Re: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-19 Thread Adrian Chadd
On Mon, 18 Jul 2022 at 15:28, Linus Lüssing  wrote:
>
> From: Linus Lüssing 
>
> AR9003 based wifi chips have a hardware bug, they always report a
> channel bandwidth of HT40 for any sub-frame of an aggregate which is
> not the last one. Only the last sub-frame has correct channel bandwidth
> information.

Hi!

It's not a hardware bug. Dating back to the original AR5416 11n chip,
most flags aren't valid for subframes in an aggregate. Only the final
frame has valid flags. This was explicitly covered internally way back
when.




-adrian

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k