Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: mkenna=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Tue, 10 Aug 2021 14:01:37 +0530
From:   Maharaja Kennadyrajan <mkenna@codeaurora.org>
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v4 1/3] nl80211: Add support for beacon tx mode
In-Reply-To: <f14e6e6b81978ace4580774981a7e032b4001182.camel@sipsolutions.net>
References: <1623164855-15910-1-git-send-email-mkenna@codeaurora.org>
 <1623164855-15910-2-git-send-email-mkenna@codeaurora.org>
 (sfid-20210608_171052_201356_8AEA5406)
 <f14e6e6b81978ace4580774981a7e032b4001182.camel@sipsolutions.net>
Message-ID: <65b594ccab0a30bfb5bc2122cbf73f20@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2021-06-23 16:46, Johannes Berg wrote:
> First, I'd like to see some input from other vendors - is this useful?
> 
> I have no objections to it and it comes with an in-kernel driver, so 
> I'm
> inclined to accept it, but maybe we should have other modes, etc.?
> 
> 
>> + * @NL80211_ATTR_BEACON_TX_MODE: used to configure the beacon tx mode 
>> as
>> + *      staggered mode = 1 or burst mode = 2 in %NL80211_CMD_START_AP 
>> or
>> + *      %NL80211_CMD_JOIN_MESH from user-space.
> 
[Maha]: will replace the hard-coded values with enum here.
> 
> This should refer to the enum, not the hard-coded constants.
>> 
>> +/**
>> + * enum nl80211_beacon_tx_mode - Beacon Tx Mode enum.
>> + *      Used to configure beacon staggered mode or beacon burst mode.
>> + */
>> +enum nl80211_beacon_tx_mode {
> 
> 
> That's missing kernel-doc, and you also don't specify what default
> means. Why is it even here?
> 
[Maha]: sure, I will add the same in the kernel-doc.
>> +	[NL80211_ATTR_BEACON_TX_MODE] = NLA_POLICY_RANGE(NLA_U32, 1,
> 2),
> 
> That really also needs to use the enum, not hard-coded constants...
[Maha]: will replace the hard-coded values with enum also here.
> 
> johannes