On Wed, 2020-05-13 at 12:45 -0700, Rajkumar Manoharan wrote:
> In 6 GHz band, determine chandef from 6 GHz operation information
> of HE operation element.
So here we get to real differences ...
> Reported-by: kernel test robot <rong.a.chen@xxxxxxxxx>
Huh?
> +bool ieee80211_chandef_he_oper(struct ieee80211_sub_if_data *sdata,
> + const struct ieee80211_he_operation *heop,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct ieee80211_he_oper_6ghz_op_info info;
> + const struct ieee80211_sta_he_cap *he_cap;
> + struct ieee80211_supported_band *sband;
> + struct cfg80211_chan_def new = *chandef;
> + int cf0, cf1;
> + int ccf0, ccf1;
> + bool support_80_80;
> + bool support_160;
> + u8 he_phy_cap;
> + u8 pos = 0;
> +
> + /* Below HE Operation check is required only for 6 GHz band */
> + if (chandef->chan->band != NL80211_BAND_6GHZ)
> + return true;
> +
> + if (!heop)
> + return false;
> +
> + sband = sdata->local->hw.wiphy->bands[chandef->chan->band];
> + if (!sband)
> + return false;
> +
> + he_cap = ieee80211_get_he_iftype_cap(sband, sdata->vif.type);
> + if (!he_cap)
> + return false;
> +
> + if (!(le32_to_cpu(heop->he_oper_params) &
> + IEEE80211_HE_OPERATION_6GHZ_OP_INFO))
> + return false;
> +
> + he_phy_cap = he_cap->he_cap_elem.phy_cap_info[0];
> + support_160 =
> + !!(he_phy_cap &
> + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G);
> + support_80_80 =
> + !!(he_phy_cap &
> + IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G);
> +
> + if (le32_to_cpu(heop->he_oper_params) &
> + IEEE80211_HE_OPERATION_VHT_OPER_INFO)
> + pos += 3;
> + if (le32_to_cpu(heop->he_oper_params) &
> + IEEE80211_HE_OPERATION_CO_HOSTED_BSS)
> + pos += 1;
This really gets better with the ieee80211_he_6ghz_oper() inline I wrote
and posted in the other reply.
> + case IEEE80211_HE_6GHZ_CHANWIDTH_160MHZ_80P80MHZ:
> + new.center_freq1 = cf0;
> + new.width = NL80211_CHAN_WIDTH_80;
> + if (ccf1) {
> + unsigned int diff;
> +
> + diff = abs(ccf1 - ccf0);
> + if (diff == 8 && support_160) {
> + new.width = NL80211_CHAN_WIDTH_160;
> + new.center_freq1 = cf1;
> + } else if ((diff > 8) && support_80_80) {
> + new.width = NL80211_CHAN_WIDTH_80P80;
> + new.center_freq2 = cf1;
> + }
> + }
Hmm. Yes, we just had
+ case IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ:
+ if (abs(he_6ghz_oper->ccfs1 - he_6ghz_oper->ccfs0) == 8)
+ he_chandef.width = NL80211_CHAN_WIDTH_160;
+ else
+ he_chandef.width = NL80211_CHAN_WIDTH_80P80;
+ break;
but that breaks if you don't support 80+80 or 160.
OTOH, we check this later again, I think, and downgrade if we don't
support it, so no harm done?
I think I'd prefer the parsing to be exact, and then downgrade as
necessary. That makes things a bit simpler.
That may mean the place where you call this should be different though.
johannes
