| Message ID | 20210520034021.14690-1-wgong@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wireless: add check of field VHT Extended NSS BW Capable for 160/80+80 MHz setting | expand |
On 5/20/2021 5:40 AM, Wen Gong wrote: > Table 9-251—Supported VHT-MCS and NSS Set subfields, it has subfield VHT Extended > NSS BW Capable, its definition is: > Indicates whether the STA is capable of interpreting the Extended NSS BW > Support subfield of the VHT Capabilities Information field. > > This patch is to add check for the subfield. Almost looks good to me, but... > Signed-off-by: Wen Gong <wgong@codeaurora.org> > --- > net/wireless/chan.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/wireless/chan.c b/net/wireless/chan.c > index 285b8076054b..f7f86bd75cc0 100644 > --- a/net/wireless/chan.c > +++ b/net/wireless/chan.c [...] > @@ -1037,7 +1039,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, > cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK; > if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ && > cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ && > - !(vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) > + !(ext_nss_cap && vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) you should not mix logical and binary operators so add braces around the binary operation here. Regards, Arend > return false; > break; > default: >
On 2021-05-21 03:21, Arend van Spriel wrote: > On 5/20/2021 5:40 AM, Wen Gong wrote: >> Table 9-251—Supported VHT-MCS and NSS Set subfields, it has subfield >> VHT Extended >> NSS BW Capable, its definition is: >> Indicates whether the STA is capable of interpreting the Extended NSS >> BW >> Support subfield of the VHT Capabilities Information field. >> >> This patch is to add check for the subfield. > > Almost looks good to me, but... > >> Signed-off-by: Wen Gong <wgong@codeaurora.org> >> --- >> net/wireless/chan.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/net/wireless/chan.c b/net/wireless/chan.c >> index 285b8076054b..f7f86bd75cc0 100644 >> --- a/net/wireless/chan.c >> +++ b/net/wireless/chan.c > > [...] > >> @@ -1037,7 +1039,7 @@ bool cfg80211_chandef_usable(struct wiphy >> *wiphy, >> cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK; >> if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ && >> cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ && >> - !(vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) >> + !(ext_nss_cap && vht_cap->cap & >> IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) > > you should not mix logical and binary operators so add braces around > the binary operation here. priority of & is higher than &&, so it not need to add braces here. however, i have sent patch v2 which added the braces. https://lore.kernel.org/linux-wireless/20210521021809.23986-1-wgong@codeaurora.org/T/#u > > Regards, > Arend > >> return false; >> break; >> default: >>
diff --git a/net/wireless/chan.c b/net/wireless/chan.c index 285b8076054b..f7f86bd75cc0 100644 --- a/net/wireless/chan.c +++ b/net/wireless/chan.c @@ -942,7 +942,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, struct ieee80211_sta_vht_cap *vht_cap; struct ieee80211_edmg *edmg_cap; u32 width, control_freq, cap; - bool support_80_80 = false; + bool ext_nss_cap, support_80_80 = false; if (WARN_ON(!cfg80211_chandef_valid(chandef))) return false; @@ -950,6 +950,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, ht_cap = &wiphy->bands[chandef->chan->band]->ht_cap; vht_cap = &wiphy->bands[chandef->chan->band]->vht_cap; edmg_cap = &wiphy->bands[chandef->chan->band]->edmg_cap; + ext_nss_cap = vht_cap->vht_mcs.tx_highest & IEEE80211_VHT_EXT_NSS_BW_CAPABLE; if (edmg_cap->channels && !cfg80211_edmg_usable(wiphy, @@ -1015,7 +1016,8 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, (cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ) || (cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ && cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK) || - u32_get_bits(cap, IEEE80211_VHT_CAP_EXT_NSS_BW_MASK) > 1; + (ext_nss_cap && + u32_get_bits(cap, IEEE80211_VHT_CAP_EXT_NSS_BW_MASK) > 1); if (chandef->chan->band != NL80211_BAND_6GHZ && !support_80_80) return false; fallthrough; @@ -1037,7 +1039,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK; if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ && cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ && - !(vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) + !(ext_nss_cap && vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)) return false; break; default:
Table 9-251—Supported VHT-MCS and NSS Set subfields, it has subfield VHT Extended NSS BW Capable, its definition is: Indicates whether the STA is capable of interpreting the Extended NSS BW Support subfield of the VHT Capabilities Information field. This patch is to add check for the subfield. Signed-off-by: Wen Gong <wgong@codeaurora.org> --- net/wireless/chan.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)