Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <12a77901c0f6b218c802db7434466d5eb49bcb46.camel@sipsolutions.net>
Subject: Re: [PATCH 04/19] ieee80211: Add EHT (802.11be) definitions
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Veerendranath Jakkam <quic_vjakkam@quicinc.com>,
        linux-wireless@vger.kernel.org
Cc:     Ilan Peer <ilan.peer@intel.com>, Jia Ding <quic_jiad@quicinc.com>,
        Mordechay Goodstein <mordechay.goodstein@intel.com>,
        Sriram R <quic_srirrama@quicinc.com>
Date:   Mon, 07 Feb 2022 13:10:02 +0100
In-Reply-To: <d51984f5-a035-36a5-75f6-3bff67d60b12@quicinc.com>
References: <20220204220255.448224-1-johannes@sipsolutions.net>
         <20220204230119.1ee92202ac30.Id30a3ef2844b296efbd5486fe1da9ca36a95c5cf@changeid>
         <d51984f5-a035-36a5-75f6-3bff67d60b12@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.3 (3.42.3-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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 <ilan.peer@intel.com>
> > 
> > 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