rtwn: enable VHT if it's configured in the device

[adrian]

Summary

If the driver attach path adds the VHT flag then add the 20/40/80 MHz
VHT channels.

This is a no-op right now as nothing is enabling it.

[adrian]

remove debug printf

[emaste]

sys/dev/rtwn/if_rtwn.c

1628–1629

Why ternary op here, and not

if (sc->sc_ht40)
        cbw_flags |= NET80211_CBW_FLAG_VHT80;

[adrian]

sys/dev/rtwn/if_rtwn.c

1628–1629

You'll need to ask 2018 adrian :-) I'll go clean it up

[adrian]

• remove ternary, thanks emaste
• only set the 80mhz channel flag if VHT is enabled

[bz]

sys/dev/rtwn/if_rtwn.c

1625

This check should likely be: "if hardware supports VHT" and then set

ic->ic_flags_ext |= IEEE80211_FEXT_VHT;
and populate ic->ic_vht_cap.vht_cap_info

I do not see similar code doing this in the surroundings of the review stack currently. Are there adjustments to the rtwn_adj_devcaps() call somewhere doing so?
Hmm Found it way up the stack in D48103 where this happens. Okay.

If you were to use this check you would want to use IEEE80211_CONF_VHT() by your own comment from 2017 ;-)

1629

This sounds like a bogus comment (and check). 11ac requires VHT80. So either VHT20/40/80 are avail or not. Only 160/80P80 are optional.

[adrian]

sys/dev/rtwn/if_rtwn.c

1625

lol, good catch. Lemme go use my macro with its outdated (now) comment. :-)

1629

802.11ac /certification/ requires 80MHz support in 5GHz. You don't have to run the device at 80MHz. Heck my home network is VHT40 most of the time; I flip it up to 80MHz just to do 80MHz testing.

The way the driver is structured is if 40MHz is disabled by that sysctl then VHT40 won't work, and thus VHT80 will also not work right. So yes, we also need that sysctl check here whilst 40MHz is 'optional'.

[bz]

sys/dev/rtwn/if_rtwn.c

1629

802.11ac /certification/ requires 80MHz support in 5GHz. You don't have to run the device at 80MHz. Heck my home network is VHT40 most of the time; I flip it up to 80MHz just to do 80MHz testing.

How you run your network is your choice and yes, you can do as you like. But that's an operational choice.

See my reply to you from Fri, 22 Nov 2024 23:44:37 +0000 (UTC) where I quoted it to you before:

[IEEE-]802.11ac :: "Mandatory support for 40 MHz and 80 MHz channel widths."

So, not certification but standard.

The way the driver is structured is if 40MHz is disabled by that sysctl then VHT40 won't work, and thus VHT80 will also not work right. So yes, we also need that sysctl check here whilst 40MHz is 'optional'.

But again is a choice of yours while adding VHT to the driver so we'll likely hit this in other reviews too.
I can do VHT40/80 without supporting HT40 as HT40 was optional but VHT40/80 is not.
Whether we do it or not (operationally) we control by other flags which can also be changed from ifconfig (vht, vht40, vht80, vht160, vht80p80).

The sc_ht40 was only used to steer logic by adding IEEE80211_HTCAP_CHWIDTH40 and IEEE80211_HTCAP_SHORTGI40 as a HAL flag.
The sysctl is in fact a tunable (CTLFLAG_RDTUN) -- and undocumented in the man page -- so was likely left like this for debugging only by @avos and not being supposed to be changed by a user?

[adrian]

sys/dev/rtwn/if_rtwn.c

1629

I remember adding it with him a long while back because HT40 was just super unstable. Now that I'm neck deep in the driver again I'll eventually get it all working.

In any case, the 11ac chipset support defaults to HT40 support now (that was added in an earlier diff) so by default VHT40/VHT80 channels will be configured. This is just in here so we don't end up with something crazy and hard to debug if someone DOES set that tunable or has it hiding somewhere in their config from years ago.

[bz]

sys/dev/rtwn/if_rtwn.c

1625

I'll let you change to the macro then ... :)

[adrian]

feedback from bz