2022-02-08 22:27:21

by Veerendranath Jakkam

[permalink] [raw]
Subject: Re: [PATCH 04/19] ieee80211: Add EHT (802.11be) definitions

On 2/5/22 3:32 AM, Johannes Berg wrote:
> From: Ilan Peer <[email protected]>
>
> Based on Draft P802.11be_D1.3.


"Based on Draft P802.11be_D1.0".

based on documentation and MACRO definitions, These changes are aligned
with P802.11be_D1.0

> +
> +/**
> + * struct ieee80211_eht_mcs_nss_supp_bw - EHT max supported NSS per MCS (except
> + * 20MHz only stations).
> + *
> + * For each field below, bits 0 - 3 indicate the maximal number of spatial
> + * streams for Rx, and bits 4 - 7 indicate the maximal number of spatial streams
> + * for Tx.
> + *
> + * @rx_tx_mcs9_max_nss: indicates the maximum number of spatial streams
> + * supported for reception and the maximum number of spatial streams
> + * supported for transmission for MCS 8 - 9.


@rx_tx_mcs9_max_nss supported for transmission for MCS 0 - 9.

> +/**
> + * struct ieee80211_eht_mcs_nss_supp - EHT max supported NSS per MCS
> + *
> + * @only_20mhz: For a 20 MHz-only STA, indicates the maximum number of spatial
> + * streams supported for reception and the maximum number of spatial streams
> + * supported for transmission, for each MCS value. Optionally present in
> + * &struct ieee80211_eht_cap_elem.
> + * @bw_80: If the operating channel width of the STA is greater than or equal to
> + * 80 MHz, indicates the maximum number of spatial streams supported fo

@bw_80 applicable for 20 and 40 MHz operating band widths also for non 20 MHz-only STA

> +/* EHT beamformee SU number of spatial streams <= 80MHz is split between octet 0
> + * and octet 1
> + */
> +#define IEEE80211_EHT_PHY_CAP0_SU_BEAMFORMEE_SS_80MHZ 0x80
> +
> +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_80MHZ 0x03
> +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_160MHZ 0x1c
> +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_320MHZ 0xe0
> +


Append _MASK for the macros representing multi bit fields like above?

Or we can use GENMASK() to represent values


- veeru



2022-02-09 05:27:59

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 04/19] ieee80211: Add EHT (802.11be) definitions

Thanks for the review!

On Mon, 2022-02-07 at 12:37 +0530, Veerendranath Jakkam wrote:
> On 2/5/22 3:32 AM, Johannes Berg wrote:
> > From: Ilan Peer <[email protected]>
> >
> > Based on Draft P802.11be_D1.3.
>
>
> "Based on Draft P802.11be_D1.0".
>
> based on documentation and MACRO definitions, These changes are aligned
> with P802.11be_D1.0

Hmm. I thought I squashed in all the later update patches, but maybe we
forgot things!

>
> > +/* EHT beamformee SU number of spatial streams <= 80MHz is split between octet 0
> > + * and octet 1
> > + */
> > +#define IEEE80211_EHT_PHY_CAP0_SU_BEAMFORMEE_SS_80MHZ 0x80
> > +
> > +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_80MHZ 0x03
> > +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_160MHZ 0x1c
> > +#define IEEE80211_EHT_PHY_CAP1_SU_BEAMFORMEE_SS_320MHZ 0xe0
> > +
>
>
> Append _MASK for the macros representing multi bit fields like above?
>

I guess that'd make some sense.

> Or we can use GENMASK() to represent values
>

I don't like GENMASK() that much to be honest, it always feels confusing
which way around the arguments should be and whether the edges are
included or not, and then I don't feel we gain much from it?

johannes