From: Rajkumar Manoharan <rmanohar@codeaurora.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: kvalo@codeaurora.org, linux-wireless@vger.kernel.org,
ath11k@lists.infradead.org
Subject: Re: [PATCH v3 03/11] nl80211: add HE 6 GHz Band Capability support
Date: Wed, 27 May 2020 10:39:55 -0700 [thread overview]
Message-ID: <e9ab93034612fd476ac17fd25052cd06@codeaurora.org> (raw)
In-Reply-To: <4e3a5ab6eed1dc91f45a459bb616fa05a110483d.camel@sipsolutions.net>
On 2020-05-27 07:27, Johannes Berg wrote:
> On Wed, 2020-05-13 at 12:44 -0700, Rajkumar Manoharan wrote:
>>
>> /**
>> + * enum ieee80211_he_6ghz_chanwidth - HE 6 GHz channel width
>> + * @IEEE80211_HE_6GHZ_CHANWIDTH_20MHZ: 20 MHz bandwidth
>> + * @IEEE80211_HE_6GHZ_CHANWIDTH_40MHZ: 40 MHz bandwidth
>> + * @IEEE80211_HE_6GHZ_CHANWIDTH_80MHZ: 80 MHz bandwidth
>> + * @IEEE80211_HE_6GHZ_CHANWIDTH_80P80MHZ: 160 or 80+80 MHz bandwidth
>> + */
>> +enum ieee80211_he_6ghz_chanwidth {
>> + IEEE80211_HE_6GHZ_CHANWIDTH_20MHZ = 0,
>> + IEEE80211_HE_6GHZ_CHANWIDTH_40MHZ = 1,
>> + IEEE80211_HE_6GHZ_CHANWIDTH_80MHZ = 2,
>> + IEEE80211_HE_6GHZ_CHANWIDTH_160MHZ_80P80MHZ = 3,
>> +};
>> +
>> +/**
>> + * struct ieee80211_he_oper_6ghz_op_info - 6 GHz Operation
>> Information
>> + *
>> + * This structure is defined as described in IEEE P802.11ax/D6.0,
>> + * Figure 9-787k—6 GHz Operation Information field.
>> + *
>> + * @primary_chan: The channel number of the primary channel in the 6
>> GHz band.
>> + * @control: First two bits defines channel width field indicates the
>> BSS
>> + * channel width and is set to 0 for 20 MHz, 1 for 40 MHz, 2 for 80
>> MHz,
>> + * and 3 for 80+80 or 160 MHz.
>> + * @center_freq_seg0_idx: Channel center frequency index for the 20
>> MHz,
>> + * 40 MHz, or 80 MHz, or 80+80 MHz.
>> + * @center_freq_seg1_idx: Channel center frequency index of the 160
>> MHz.
>> + * @min_rate: Minimum rate, in units of 1 Mb/s, that the non-AP STA
>> is allowed
>> + * to use for sending PPDUs.
>> + */
>> +struct ieee80211_he_oper_6ghz_op_info {
>> + u8 primary_chan;
>> + u8 control;
>> + u8 center_freq_seg0_idx;
>> + u8 center_freq_seg1_idx;
>> + u8 min_rate;
>> +} __packed;
>>
>
> Looks like I had
>
> +/**
> + * ieee80211_he_6ghz_oper - HE 6 GHz operation Information field
> + * @primary: primary channel
> + * @control: control flags
> + * @ccfs0: channel center frequency segment 0
> + * @ccfs1: channel center frequency segment 1
> + * @minrate: minimum rate (in 1 Mbps units)
> + */
> +struct ieee80211_he_6ghz_oper {
> + u8 primary;
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH 0x3
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_20MHZ
> 0
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_40MHZ
> 1
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ
> 2
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ
> 3
> +#define IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON 0x4
> + u8 control;
> + u8 ccfs0;
> + u8 ccfs1;
> + u8 minrate;
> +} __packed;
>
>
> Any objection to that? The names are getting _really_ long the way you
> had them ...
>
Neat.. LGTM.. :)
> FWIW, I also had a fix in ieee80211_he_oper_size() where the size of
> the
> oper is now sizeof(struct ieee80211_he_6ghz_oper).
>
> And this, to find it:
>
> +/**
> + * ieee80211_he_6ghz_oper - obtain 6 GHz operation field
> + * @he_oper: HE operation element (must be pre-validated for size)
> + * but may be %NULL
> + *
> + * Return: a pointer to the 6 GHz operation field, or %NULL
> + */
> +static inline const struct ieee80211_he_6ghz_oper *
> +ieee80211_he_6ghz_oper(const struct ieee80211_he_operation *he_oper)
> +{
> + const u8 *ret = (void *)&he_oper->optional;
> + u32 he_oper_params;
> +
> + if (!he_oper)
> + return NULL;
> +
> + he_oper_params = le32_to_cpu(he_oper->he_oper_params);
> +
> + if (!(he_oper_params & IEEE80211_HE_OPERATION_6GHZ_OP_INFO))
> + return NULL;
> + if (he_oper_params & IEEE80211_HE_OPERATION_VHT_OPER_INFO)
> + ret += 3;
> + if (he_oper_params & IEEE80211_HE_OPERATION_CO_HOSTED_BSS)
> + ret++;
> +
> + return (void *)ret;
> +}
> +
>
Great.
>
>> #define IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895 0x00000000
>> #define IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991 0x00000001
>> @@ -1982,6 +2029,15 @@ int ieee80211_get_vht_max_nss(struct
>> ieee80211_vht_cap *cap,
>> #define IEEE80211_TX_RX_MCS_NSS_SUPP_TX_BITMAP_MASK 0x07c0
>> #define IEEE80211_TX_RX_MCS_NSS_SUPP_RX_BITMAP_MASK 0xf800
>>
>> +/* 802.11ax HE 6 GHz Band Capability */
>> +#define IEEE80211_HE_6GHZ_CAP_MIN_MPDU_START_SPACE_MASK GENMASK(2,
>> 0)
>> +#define
>> IEEE80211_HE_6GHZ_CAP_MAX_A_MPDU_LENGTH_EXPONENT_MASK GENMASK(5, 3)
>> +#define IEEE80211_HE_6GHZ_CAP_MAX_MPDU_LENGTH_MASK GENMASK(7, 6)
>> +#define IEEE80211_HE_6GHZ_CAP_SMPS_MASK GENMASK(10, 9)
>> +#define IEEE80211_HE_6GHZ_CAP_RD_RESP BIT(11)
>> +#define IEEE80211_HE_6GHZ_CAP_RX_ANTENNA_PATTERN BIT(12)
>> +#define IEEE80211_HE_6GHZ_CAP_TX_ANTENNA_PATTERN BIT(13)
>
> I don't like GENMASK() much ... but ok. FWIW, I had
>
Hope GENMASK defined in backports for older kernel. I started using this
since ath11k.
I feel GENMASK is more user readable and avoid masking errors.
> +struct ieee80211_he_6ghz_capa {
> + /* uses IEEE80211_HE_6GHZ_CAP_* below */
> + __le16 capa;
> +} __packed;
> +
> +/* HE 6 GHz band capabilities */
> +/* uses enum ieee80211_min_mpdu_spacing values */
> +#define IEEE80211_HE_6GHZ_CAP_MIN_MPDU_START 0x0007
> +/* uses enum ieee80211_vht_max_ampdu_length_exp values */
> +#define IEEE80211_HE_6GHZ_CAP_MAX_AMPDU_LEN_EXP 0x0038
> +/* uses IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_* values */
> +#define IEEE80211_HE_6GHZ_CAP_MAX_MPDU_LEN 0x00c0
> +/* WLAN_HT_CAP_SM_PS_* values */
> +#define IEEE80211_HE_6GHZ_CAP_SM_PS_SHIFT 9
> +#define IEEE80211_HE_6GHZ_CAP_SM_PS 0x0600
> +#define IEEE80211_HE_6GHZ_CAP_RD_RESPONDER 0x0800
> +#define IEEE80211_HE_6GHZ_CAP_RX_ANTPAT_CONS 0x1000
> +#define IEEE80211_HE_6GHZ_CAP_TX_ANTPAT_CONS 0x2000
>
>
> again, just shorter names ...
>
I am fine with this.. Leave it to you. Cheers.
-Rajkumar
next prev parent reply other threads:[~2020-05-27 17:40 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-13 19:44 [PATCH v3 01/11] cfg80211: use only HE capability to set prohibited flags in 6 GHz Rajkumar Manoharan
2020-05-13 19:44 ` [PATCH v3 02/11] cfg80211: handle 6 GHz capability of new station Rajkumar Manoharan
2020-05-27 14:00 ` Johannes Berg
2020-05-27 23:24 ` Rajkumar Manoharan
2020-05-28 7:40 ` Johannes Berg
2020-05-13 19:44 ` [PATCH v3 03/11] nl80211: add HE 6 GHz Band Capability support Rajkumar Manoharan
2020-05-27 14:27 ` Johannes Berg
2020-05-27 17:39 ` Rajkumar Manoharan [this message]
2020-05-13 19:44 ` [PATCH v3 04/11] mac80211: add HE 6 GHz Band Capabilities into parse extension Rajkumar Manoharan
2020-05-27 14:28 ` Johannes Berg
2020-05-13 19:44 ` [PATCH v3 05/11] mac80211: fix memory overlap due to variable length param Rajkumar Manoharan
2020-05-27 14:28 ` Johannes Berg
2020-05-13 19:45 ` [PATCH v3 06/11] mac80211: handle HE 6 GHz Capability in HE STA processing Rajkumar Manoharan
2020-05-27 14:43 ` Johannes Berg
2020-05-28 8:55 ` Johannes Berg
2020-05-28 9:43 ` Johannes Berg
2020-05-28 13:15 ` Johannes Berg
2020-05-13 19:45 ` [PATCH v3 07/11] mac80211: add HE 6 GHz Band Capability IE in Assoc. Request Rajkumar Manoharan
2020-05-27 14:37 ` Johannes Berg
2020-05-28 12:20 ` Johannes Berg
2020-05-13 19:45 ` [PATCH v3 08/11] mac80211: build HE operation with 6 GHz oper information Rajkumar Manoharan
2020-05-27 14:30 ` Johannes Berg
2020-05-13 19:45 ` [PATCH v3 09/11] mac80211: do not allow HT/VHT IEs in 6 GHz mesh mode Rajkumar Manoharan
2020-05-13 19:45 ` [PATCH v3 10/11] mac80211: determine chantype from HE operation in 6 GHz Rajkumar Manoharan
2020-05-27 14:41 ` Johannes Berg
2020-05-27 14:44 ` Johannes Berg
2020-05-27 18:34 ` Rajkumar Manoharan
2020-05-27 18:41 ` Johannes Berg
2020-05-28 9:41 ` Johannes Berg
2020-05-28 11:46 ` Johannes Berg
2020-05-13 19:45 ` [PATCH v3 11/11] ath11k: build HE 6 GHz capability Rajkumar Manoharan
2020-06-01 22:42 ` Rajkumar Manoharan
2020-05-27 13:43 ` [PATCH v3 01/11] cfg80211: use only HE capability to set prohibited flags in 6 GHz Johannes Berg
2020-05-27 23:32 ` Rajkumar Manoharan
2020-05-28 7:41 ` Johannes Berg
2020-05-28 7:42 ` Johannes Berg
-- strict thread matches above, loose matches on Subject: below --
2020-05-09 0:12 Rajkumar Manoharan
2020-05-09 0:12 ` [PATCH v3 03/11] nl80211: add HE 6 GHz Band Capability support Rajkumar Manoharan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e9ab93034612fd476ac17fd25052cd06@codeaurora.org \
--to=rmanohar@codeaurora.org \
--cc=ath11k@lists.infradead.org \
--cc=johannes@sipsolutions.net \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).