From: Carl Huang <cjhuang@codeaurora.org> To: Abhishek Kumar <kuabhs@google.com> Cc: Johannes Berg <johannes@sipsolutions.net>, Abhishek Kumar <kuabhs@chromium.org>, briannorris@chromium.org, linux-wireless@vger.kernel.org, Doug Anderson <dianders@chromium.org>, ath10k@lists.infradead.org Subject: Re: [RFC 1/2] nl80211: add common API to configure SAR power limitations. Date: Wed, 04 Nov 2020 14:18:40 +0800 [thread overview] Message-ID: <0d820718ca30962f783fc13f2d9db2bc@codeaurora.org> (raw) In-Reply-To: <CAB5ih=PtneRW+sU9KbPNgEM8GvPA1rfa87M41fsL=y=VP71Y5Q@mail.gmail.com> On 2020-11-04 09:17, Abhishek Kumar wrote: > On Tue, Nov 3, 2020 at 5:15 AM Johannes Berg > <johannes@sipsolutions.net> wrote: >> >> On Tue, 2020-11-03 at 10:34 +0800, Carl Huang wrote: >> > On 2020-10-31 10:46, Abhishek Kumar wrote: >> > > From: kuabhs@chromium.org >> > > >> > > There are few more additional comments here. >> > > > + * @NL80211_CMD_SET_SAR_SPECS: SAR power limitation configuration is >> > > > + * passed using %NL80211_ATTR_SAR_SPEC. >> > > > + * >> > > >> > > This command requires NL80211_ATTR_IFINDEX, probably should be better >> > > to add >> > > this in the comment ? >> > > >> > Per Johannes's comments, this explicit index is not required. Are you >> > fine with it? >> > >> > Instead, user-space application records the array index when querying >> > the SAR >> > capability. When set, the nested array index will be used to set the >> > power. >> > This requires user-space has a strict mapping of index. and >> > NL80211_ATTR_IFINDEX >> > is to be removed. >> >> Wait, what? The IFINDEX is still required, that identifies the >> interface >> - though it probably shouldn't be required, this should be per wiphy, >> so >> you could also use ATTR_WIPHY_IDX? >> >> You're thinking of ... something >> else. NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX perhaps? > > Yes, probably the index mapping that you are talking about is using > NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX . > You're right. I just remembered Johannes's comments to remove the NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX and made mistake here. > I think Johannes has already covered most of the comments. I have one > last one mentioned below. > >> +nl80211_put_sar_specs(struct cfg80211_registered_device *rdev, >> + struct sk_buff *msg) >> +{ >> + struct nlattr *sar_capa, *specs, *sub_freq_range; >> + u8 num_freq_ranges; >> + int i; >> + >> + if (!rdev->wiphy.sar_capa) >> + return 0; >> + >> + num_freq_ranges = rdev->wiphy.sar_capa->num_freq_ranges; >> + >> + sar_capa = nla_nest_start(msg, NL80211_ATTR_SAR_SPEC); >> + if (!sar_capa) >> + return -ENOSPC; >> + >> + if (nla_put_u16(msg, NL80211_SAR_ATTR_TYPE, >> rdev->wiphy.sar_capa->type)) >> + goto fail; > > The nla_put_u16 NL80211_SAR_ATTR_TYPE here uses u16 whereas in the to > get this property below it uses nla_get_u32 . I think this should be > consistent ? > >> + if (!tb[NL80211_SAR_ATTR_TYPE]) >> + return -EINVAL; >> + >> + type = nla_get_u32(tb[NL80211_SAR_ATTR_TYPE]); >> + >> + if (!tb[NL80211_SAR_ATTR_SPECS]) >> + return -EINVAL; > > As mentioned above the get and put for NL80211_SAR_ATTR_TYPE is not > consistent. > Correct. Thanks for pointing out and it will be fixed in next version. > -Abhishek > > _______________________________________________ > ath10k mailing list > ath10k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath10k
WARNING: multiple messages have this Message-ID (diff)
From: Carl Huang <cjhuang@codeaurora.org> To: Abhishek Kumar <kuabhs@google.com> Cc: Abhishek Kumar <kuabhs@chromium.org>, briannorris@chromium.org, linux-wireless@vger.kernel.org, Doug Anderson <dianders@chromium.org>, ath10k@lists.infradead.org, Johannes Berg <johannes@sipsolutions.net> Subject: Re: [RFC 1/2] nl80211: add common API to configure SAR power limitations. Date: Wed, 04 Nov 2020 14:18:40 +0800 [thread overview] Message-ID: <0d820718ca30962f783fc13f2d9db2bc@codeaurora.org> (raw) In-Reply-To: <CAB5ih=PtneRW+sU9KbPNgEM8GvPA1rfa87M41fsL=y=VP71Y5Q@mail.gmail.com> On 2020-11-04 09:17, Abhishek Kumar wrote: > On Tue, Nov 3, 2020 at 5:15 AM Johannes Berg > <johannes@sipsolutions.net> wrote: >> >> On Tue, 2020-11-03 at 10:34 +0800, Carl Huang wrote: >> > On 2020-10-31 10:46, Abhishek Kumar wrote: >> > > From: kuabhs@chromium.org >> > > >> > > There are few more additional comments here. >> > > > + * @NL80211_CMD_SET_SAR_SPECS: SAR power limitation configuration is >> > > > + * passed using %NL80211_ATTR_SAR_SPEC. >> > > > + * >> > > >> > > This command requires NL80211_ATTR_IFINDEX, probably should be better >> > > to add >> > > this in the comment ? >> > > >> > Per Johannes's comments, this explicit index is not required. Are you >> > fine with it? >> > >> > Instead, user-space application records the array index when querying >> > the SAR >> > capability. When set, the nested array index will be used to set the >> > power. >> > This requires user-space has a strict mapping of index. and >> > NL80211_ATTR_IFINDEX >> > is to be removed. >> >> Wait, what? The IFINDEX is still required, that identifies the >> interface >> - though it probably shouldn't be required, this should be per wiphy, >> so >> you could also use ATTR_WIPHY_IDX? >> >> You're thinking of ... something >> else. NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX perhaps? > > Yes, probably the index mapping that you are talking about is using > NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX . > You're right. I just remembered Johannes's comments to remove the NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX and made mistake here. > I think Johannes has already covered most of the comments. I have one > last one mentioned below. > >> +nl80211_put_sar_specs(struct cfg80211_registered_device *rdev, >> + struct sk_buff *msg) >> +{ >> + struct nlattr *sar_capa, *specs, *sub_freq_range; >> + u8 num_freq_ranges; >> + int i; >> + >> + if (!rdev->wiphy.sar_capa) >> + return 0; >> + >> + num_freq_ranges = rdev->wiphy.sar_capa->num_freq_ranges; >> + >> + sar_capa = nla_nest_start(msg, NL80211_ATTR_SAR_SPEC); >> + if (!sar_capa) >> + return -ENOSPC; >> + >> + if (nla_put_u16(msg, NL80211_SAR_ATTR_TYPE, >> rdev->wiphy.sar_capa->type)) >> + goto fail; > > The nla_put_u16 NL80211_SAR_ATTR_TYPE here uses u16 whereas in the to > get this property below it uses nla_get_u32 . I think this should be > consistent ? > >> + if (!tb[NL80211_SAR_ATTR_TYPE]) >> + return -EINVAL; >> + >> + type = nla_get_u32(tb[NL80211_SAR_ATTR_TYPE]); >> + >> + if (!tb[NL80211_SAR_ATTR_SPECS]) >> + return -EINVAL; > > As mentioned above the get and put for NL80211_SAR_ATTR_TYPE is not > consistent. > Correct. Thanks for pointing out and it will be fixed in next version. > -Abhishek > > _______________________________________________ > ath10k mailing list > ath10k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath10k _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2020-11-04 6:19 UTC|newest] Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-22 5:49 [RFC 1/2] nl80211: add common API to configure SAR power limitations Carl Huang 2020-09-22 5:49 ` Carl Huang 2020-09-22 5:49 ` [RFC 2/2] ath10k: allow dynamic SAR power limits via common API Carl Huang 2020-09-22 5:49 ` Carl Huang 2020-09-22 7:33 ` kernel test robot 2020-09-22 13:32 ` kernel test robot 2020-11-04 23:11 ` Brian Norris 2020-11-04 23:11 ` Brian Norris 2020-11-05 11:27 ` Carl Huang 2020-11-05 11:27 ` Carl Huang 2020-11-05 17:49 ` Brian Norris 2020-11-05 17:49 ` Brian Norris 2020-09-28 12:36 ` [RFC 1/2] nl80211: add common API to configure SAR power limitations Johannes Berg 2020-09-28 12:36 ` Johannes Berg 2020-10-30 20:56 ` Abhishek Kumar 2020-10-30 20:56 ` Abhishek Kumar 2020-10-31 2:46 ` Abhishek Kumar 2020-11-03 2:34 ` Carl Huang 2020-11-03 2:34 ` Carl Huang 2020-11-03 13:15 ` Johannes Berg 2020-11-03 13:15 ` Johannes Berg 2020-11-04 1:17 ` Abhishek Kumar 2020-11-04 1:17 ` Abhishek Kumar 2020-11-04 6:18 ` Carl Huang [this message] 2020-11-04 6:18 ` Carl Huang 2020-11-04 8:44 ` Carl Huang 2020-11-04 8:44 ` Carl Huang 2020-11-04 17:48 ` Brian Norris 2020-11-04 17:48 ` Brian Norris 2020-11-05 11:37 ` Carl Huang 2020-11-05 11:37 ` Carl Huang 2020-11-04 23:18 ` Brian Norris 2020-11-04 23:18 ` Brian Norris 2020-11-04 23:27 ` Brian Norris 2020-11-04 23:27 ` Brian Norris 2020-11-05 11:30 ` Carl Huang 2020-11-05 11:30 ` Carl Huang -- strict thread matches above, loose matches on Subject: below -- 2020-09-22 5:36 Carl Huang 2020-09-22 5:36 ` Carl Huang 2020-09-22 5:47 ` cjhuang 2020-09-22 5:47 ` cjhuang 2020-09-22 8:09 ` Kalle Valo 2020-09-22 8:09 ` Kalle Valo 2020-11-04 2:00 ` Brian Norris 2020-11-04 2:00 ` Brian Norris 2020-11-04 7:32 ` Carl Huang 2020-11-04 7:32 ` Carl Huang 2020-11-04 17:44 ` Brian Norris 2020-11-04 17:44 ` Brian Norris 2020-11-04 17:44 ` Brian Norris 2020-11-05 8:35 ` Kalle Valo 2020-11-05 8:35 ` Kalle Valo 2020-11-05 8:35 ` Kalle Valo 2020-11-05 11:10 ` Carl Huang 2020-11-05 11:10 ` Carl Huang 2020-11-05 11:10 ` Carl Huang 2020-11-05 18:25 ` Brian Norris 2020-11-05 18:25 ` Brian Norris 2020-11-05 18:25 ` Brian Norris 2020-11-06 10:11 ` Carl Huang 2020-11-06 10:11 ` Carl Huang 2020-11-06 10:11 ` Carl Huang 2020-11-05 11:17 ` Carl Huang 2020-11-05 11:17 ` Carl Huang 2020-11-05 11:17 ` Carl Huang
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=0d820718ca30962f783fc13f2d9db2bc@codeaurora.org \ --to=cjhuang@codeaurora.org \ --cc=ath10k@lists.infradead.org \ --cc=briannorris@chromium.org \ --cc=dianders@chromium.org \ --cc=johannes@sipsolutions.net \ --cc=kuabhs@chromium.org \ --cc=kuabhs@google.com \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.