| Message ID | 20190509201550.3977-2-john@phrozen.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath11k: add HE support | expand |
John Crispin <john@phrozen.org> writes: > With HE support getting added, the approach of using functions will quickly > get convoluted. Change the code to use an array lookup function instead. > > Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com> > Signed-off-by: John Crispin <john@phrozen.org> > --- > drivers/net/wireless/ath/ath11k/mac.c | 116 +++++++++++++++------------------- > include/uapi/linux/nl80211.h | 4 ++ > 2 files changed, 56 insertions(+), 64 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c > index 94a87d0fb08e..cd7fb9a13639 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -105,6 +105,51 @@ static struct ieee80211_rate ath11k_legacy_rates[] = { > { .bitrate = 540, .hw_value = ATH11K_HW_RATE_OFDM_54M }, > }; > > +static int ath11k_phymodes[NUM_NL80211_BANDS][2][__NL80211_CHAN_WIDTH_NUM] = { static const? > @@ -3889,7 +3872,12 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, > arg.channel.freq = chandef->chan->center_freq; > arg.channel.band_center_freq1 = chandef->center_freq1; > arg.channel.band_center_freq2 = chandef->center_freq2; > - arg.channel.mode = chan_to_phymode(chandef); > + arg.channel.mode = > + ath11k_phymodes[chandef->chan->band][he_support][chandef->width]; > + if (arg.channel.mode == MODE_11G && > + chandef->chan->flags & IEEE80211_CHAN_NO_OFDM) > + arg.channel.mode = MODE_11B; > + WARN_ON(arg.channel.mode == MODE_UNKNOWN); What happens when mode is MODE_UNKNOWN? Should we assign some default value after the warning message?
John Crispin <john@phrozen.org> wrote: > With HE support getting added, the approach of using functions will quickly > get convoluted. Change the code to use an array lookup function instead. > > Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com> > Signed-off-by: John Crispin <john@phrozen.org> Failed to apply: drivers/net/wireless/ath/ath10k/mac.c: In function 'chan_to_phymode': drivers/net/wireless/ath/ath10k/mac.c:567:3: warning: enumeration value '__NL80211_CHAN_WIDTH_NUM' not handled in switch [-Wswitch] switch (chandef->width) { ^~~~~~ drivers/net/wireless/ath/ath10k/mac.c:590:3: warning: enumeration value '__NL80211_CHAN_WIDTH_NUM' not handled in switch [-Wswitch] switch (chandef->width) { ^~~~~~ drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_peer_assoc_h_he': drivers/net/wireless/ath/ath11k/mac.c:1296:6: error: 'struct peer_assoc_params' has no member named 'twt_responder' arg->twt_responder = true; ^~ drivers/net/wireless/ath/ath11k/mac.c:1298:6: error: 'struct peer_assoc_params' has no member named 'twt_requester' arg->twt_requester = true; ^~ make[5]: *** [drivers/net/wireless/ath/ath11k/mac.o] Error 1 make[4]: *** [drivers/net/wireless/ath/ath11k] Error 2 make[4]: *** Waiting for unfinished jobs.... make[3]: *** [drivers/net/wireless/ath] Error 2 make[2]: *** [drivers/net/wireless] Error 2 make[1]: *** [drivers/net] Error 2 make: *** [drivers] Error 2 make: *** Waiting for unfinished jobs.... 5 patches set to Changes Requested. 10937617 [V4,1/5] ath11k: move phymode selection from function to array lookup 10937615 [V4,2/5] ath11k: add HE handling to the debug code 10937625 [V4,3/5] ath11k: extend reading of FW capabilities 10937619 [V4,4/5] ath11k: handle rx status for HE frames 10937623 [V4,5/5] ath11k: add HE support
On 13/05/2019 16:26, Kalle Valo wrote: >> @@ -3889,7 +3872,12 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, >> arg.channel.freq = chandef->chan->center_freq; >> arg.channel.band_center_freq1 = chandef->center_freq1; >> arg.channel.band_center_freq2 = chandef->center_freq2; >> - arg.channel.mode = chan_to_phymode(chandef); >> + arg.channel.mode = >> + ath11k_phymodes[chandef->chan->band][he_support][chandef->width]; >> + if (arg.channel.mode == MODE_11G && >> + chandef->chan->flags & IEEE80211_CHAN_NO_OFDM) >> + arg.channel.mode = MODE_11B; >> + WARN_ON(arg.channel.mode == MODE_UNKNOWN); > What happens when mode is MODE_UNKNOWN? Should we assign some default > value after the warning message? the driver uses that enum value at various places and it seems to be a valid wmi ID so i guess its better to let it blow up upon a bad configuration than silently falling back to a possibly unexpected behaviour John
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 94a87d0fb08e..cd7fb9a13639 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -105,6 +105,51 @@ static struct ieee80211_rate ath11k_legacy_rates[] = { { .bitrate = 540, .hw_value = ATH11K_HW_RATE_OFDM_54M }, }; +static int ath11k_phymodes[NUM_NL80211_BANDS][2][__NL80211_CHAN_WIDTH_NUM] = { + [NL80211_BAND_2GHZ] = { + { + [NL80211_CHAN_WIDTH_5] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_10] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_20_NOHT] = MODE_11G, + [NL80211_CHAN_WIDTH_20] = MODE_11NG_HT20, + [NL80211_CHAN_WIDTH_40] = MODE_11NG_HT40, + [NL80211_CHAN_WIDTH_80] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_80P80] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_160] = MODE_UNKNOWN, + }, { + [NL80211_CHAN_WIDTH_5] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_10] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_20_NOHT] = MODE_11G, + [NL80211_CHAN_WIDTH_20] = MODE_11AX_HE20_2G, + [NL80211_CHAN_WIDTH_40] = MODE_11AX_HE40_2G, + [NL80211_CHAN_WIDTH_80] = MODE_11AX_HE80_2G, + [NL80211_CHAN_WIDTH_80P80] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_160] = MODE_UNKNOWN, + }, + }, + [NL80211_BAND_5GHZ] = { + { + [NL80211_CHAN_WIDTH_5] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_10] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_20_NOHT] = MODE_11A, + [NL80211_CHAN_WIDTH_20] = MODE_11AC_VHT20, + [NL80211_CHAN_WIDTH_40] = MODE_11AC_VHT40, + [NL80211_CHAN_WIDTH_80] = MODE_11AC_VHT80, + [NL80211_CHAN_WIDTH_160] = MODE_11AC_VHT160, + [NL80211_CHAN_WIDTH_80P80] = MODE_11AC_VHT80_80, + }, { + [NL80211_CHAN_WIDTH_5] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_10] = MODE_UNKNOWN, + [NL80211_CHAN_WIDTH_20_NOHT] = MODE_11A, + [NL80211_CHAN_WIDTH_20] = MODE_11AX_HE20, + [NL80211_CHAN_WIDTH_40] = MODE_11AX_HE40, + [NL80211_CHAN_WIDTH_80] = MODE_11AX_HE80, + [NL80211_CHAN_WIDTH_160] = MODE_11AX_HE160, + [NL80211_CHAN_WIDTH_80P80] = MODE_11AX_HE80_80, + }, + }, +}; + #define ATH11K_MAC_FIRST_OFDM_RATE_IDX 4 #define ath11k_g_rates ath11k_legacy_rates #define ath11k_g_rates_size (ARRAY_SIZE(ath11k_legacy_rates)) @@ -151,69 +196,6 @@ static int get_num_chains(u32 mask) return num_chains; } -static inline enum wmi_phy_mode -chan_to_phymode(const struct cfg80211_chan_def *chandef) -{ - enum wmi_phy_mode phymode = MODE_UNKNOWN; - - switch (chandef->chan->band) { - case NL80211_BAND_2GHZ: - switch (chandef->width) { - case NL80211_CHAN_WIDTH_20_NOHT: - if (chandef->chan->flags & IEEE80211_CHAN_NO_OFDM) - phymode = MODE_11B; - else - phymode = MODE_11G; - break; - case NL80211_CHAN_WIDTH_20: - phymode = MODE_11NG_HT20; - break; - case NL80211_CHAN_WIDTH_40: - phymode = MODE_11NG_HT40; - break; - case NL80211_CHAN_WIDTH_5: - case NL80211_CHAN_WIDTH_10: - case NL80211_CHAN_WIDTH_80: - case NL80211_CHAN_WIDTH_80P80: - case NL80211_CHAN_WIDTH_160: - phymode = MODE_UNKNOWN; - break; - } - break; - case NL80211_BAND_5GHZ: - switch (chandef->width) { - case NL80211_CHAN_WIDTH_20_NOHT: - phymode = MODE_11A; - break; - case NL80211_CHAN_WIDTH_20: - phymode = MODE_11AC_VHT20; - break; - case NL80211_CHAN_WIDTH_40: - phymode = MODE_11AC_VHT40; - break; - 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: - phymode = MODE_UNKNOWN; - break; - } - break; - default: - break; - } - - WARN_ON(phymode == MODE_UNKNOWN); - return phymode; -} - u8 ath11k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband, u32 bitrate) { @@ -3877,6 +3859,7 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, struct ath11k_base *ab = ar->ab; struct wmi_vdev_start_req_arg arg = {}; int ret = 0; + int he_support = arvif->vif->bss_conf.he_support; lockdep_assert_held(&ar->conf_mutex); @@ -3889,7 +3872,12 @@ ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif, arg.channel.freq = chandef->chan->center_freq; arg.channel.band_center_freq1 = chandef->center_freq1; arg.channel.band_center_freq2 = chandef->center_freq2; - arg.channel.mode = chan_to_phymode(chandef); + arg.channel.mode = + ath11k_phymodes[chandef->chan->band][he_support][chandef->width]; + if (arg.channel.mode == MODE_11G && + chandef->chan->flags & IEEE80211_CHAN_NO_OFDM) + arg.channel.mode = MODE_11B; + WARN_ON(arg.channel.mode == MODE_UNKNOWN); arg.channel.min_power = 0; arg.channel.max_power = chandef->chan->max_power * 2; diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 31ae5c7f10e3..64b890b4df33 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -4142,6 +4142,10 @@ enum nl80211_chan_width { NL80211_CHAN_WIDTH_160, NL80211_CHAN_WIDTH_5, NL80211_CHAN_WIDTH_10, + + /* keep last */ + __NL80211_CHAN_WIDTH_NUM, + NL80211_CHAN_WIDTH_MAX = __NL80211_CHAN_WIDTH_NUM - 1, }; /**