rtwn: don't set the RTS/CTS primary channel field for RTL8812AU/RTL8821AU

[adrian]

Summary

The vendor driver doesn't bother with this and it doesn't change
performance. My guess is that for modes like AP mode we MAY wantto
be able to control the RTS/CTS bandwidth choices rather than letting
the firmare do it, but we're not there yet. rtwn: don't set the RTS/CTS primary channel field for RTL8812AU/RTL8821AU

According to the rtl8812au reference driver, this seems to control
the bandwidth used by lower-bandwidth frames when transmitted in
a higher bandwidth channel.  For example, transmitting a 20MHz frame
on an 80MHz channel (eg in hostap mode) is doable, but you may want
to at least duplicate the RTS/CTS exchange across all four 20MHz
subchannels, AND perhaps duplicate the 20MHz frame.

I haven't fired this up with a spectrum analyser to see what the
result is.

The vendor driver doesn't bother with this and it doesn't change
performance.  My guess is that for modes like AP mode we MAY wantto
be able to control the RTS/CTS bandwidth choices rather than letting
the firmare do it, but we're not there yet.

The rtl8812au code in hal/rtl8812a_xmit.c:SCMapping_8812() has
the gory details, but then the one place it's used just has it
commented out and 0 (ie "do not care") is always programmed in.

[fuz]

Not a big fan of #if 0. Can we either remove this entirely (it can be restored from git history if required) or put it behind a sysctl?

[emaste]

Not a big fan of #if 0. Can we either remove this entirely (it can be restored from git history if required) or put it behind a sysctl?

I don't think a sysctl is worthwhile. It should at least have a comment or could be something like #ifdef WE_CAN_DO_BETTER_THAN_THE_FIRMWARE.

[adrian]

In D48113#1099180, @fuz wrote:

Not a big fan of #if 0. Can we either remove this entirely (it can be restored from git history if required) or put it behind a sysctl?

I'd rather leave it as #if 0 for now until I finish bringing up 11ac TX path and can properly dig into what's actually going on here.

It's possible that it's actually beneficial to force the RTS/CTS bandwidth versus letting the MAC do it; heck it could be the MAC is NOT sending RTS/CTS on other channels in wider channels (it needs to duplicate it for each 20MHz channel).

What I do know is the vendor driver and linux effectively ignore this field entirely and simply set it (by not setting it) to 0, but that doesn't mean it's "correct".

So, is a promise I'll go sort it out in more detail later enough? :-)

[bz]

In D48113#1100606, @adrian wrote:

In D48113#1099180, @fuz wrote:

Not a big fan of #if 0. Can we either remove this entirely (it can be restored from git history if required) or put it behind a sysctl?

I'd rather leave it as #if 0 for now until I finish bringing up 11ac TX path and can properly dig into what's actually going on here.

It's possible that it's actually beneficial to force the RTS/CTS bandwidth versus letting the MAC do it; heck it could be the MAC is NOT sending RTS/CTS on other channels in wider channels (it needs to duplicate it for each 20MHz channel).

What I do know is the vendor driver and linux effectively ignore this field entirely and simply set it (by not setting it) to 0, but that doesn't mean it's "correct".

So, is a promise I'll go sort it out in more detail later enough? :-)

Well your comment here has more information that the commit message or the code change; I am fine by the #if 0 but actually documenting the return (0) would be beneficial given it needs () anyway.

sys/dev/rtwn/rtl8812a/r12a_tx.c

82

return (0);

[adrian]

feedback from bz; explain it a bit more