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;
MIME-Version: 1.0
References: <20201106100708.4609-1-cjhuang@codeaurora.org> <20201106100708.4609-2-cjhuang@codeaurora.org>
 <64e072a168c12f58847a5ee16bfdb7e47576284f.camel@sipsolutions.net> <00c810b30b91397e562ca54475940afc@codeaurora.org>
In-Reply-To: <00c810b30b91397e562ca54475940afc@codeaurora.org>
From:   Abhishek Kumar <kuabhs@chromium.org>
Date:   Thu, 19 Nov 2020 12:25:28 -0800
Message-ID: <CACTWRwtcd2rdE-JQTh0t=Xwu9Vv-2He2M4wVDXbbiV1FOyNq7g@mail.gmail.com>
Subject: Re: [PATCH 1/3] nl80211: add common API to configure SAR power limitations.
To:     Carl Huang <cjhuang@codeaurora.org>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        ath10k <ath10k@lists.infradead.org>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Brian Norris <briannorris@chromium.org>,
        Douglas Anderson <dianders@chromium.org>, kuabhs@google.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

Johannes has some good comments, apart for that I have some nits.
> > And wait, I thought we agreed to remove the index? Now I'm confused.
> >
> Using index in SET operation doesn't add burden to userspace and kernel,
> but it provides some flexibility so userspace can skip some certain
> ranges.

I agree with Carl's comment, we do need the frequency index. If the
frequency index is provided, then the order is not important which
makes the data more clear or the set_sar_spec function needs to parse
the frequency ranges (and ofcourse userspace has to populate that as
well). If the frequency index is not provided, then the driver has to
assume that the userspace is not making any error in mapping of the
power and desired frequency.
Other reason is, might be a bit unlikely, but if in future there are
new subbands, then it gives a flexibility to the userspace to
explicitly provide the band for which it needs to set the power for.

> + *     used with %NL80211_CMD_SET_SAR_SPECS. The message contains fileds
> + *     of %nl80211_sar_attrs which specifies the sar type and related

typo: fileds .. you mean fields

-Abhishek