On 2020-10-31 10:46, Abhishek Kumar wrote:
From: kuabhs@xxxxxxxxxxxx
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.
+static int
+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;
I see some places nla_nest_start_noflag being used and here
nla_nest_start.
Whats the specific reason to do that ? In the newer Kernel versions, I
believe
nla_nest_start is preferred.
This will be addressed in next version.
+ power =
nla_get_u8(spec[NL80211_SAR_ATTR_SPECS_POWER]);
+ sar_spec->sub_specs[specs].power = power;
+
+ /* if NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX isn't
present,
+ * then the power applies to all bands. But it's only
valid
+ * for the first entry.
+ */
+ if (!spec[NL80211_SAR_ATTR_SPECS_FREQ_RANGE_INDEX]) {
+ if (specs) {
+ err = -EINVAL;
+ goto error;
+ } else {
+
sar_spec->sub_specs[specs].freq_range_index =
+ NL80211_SAR_ALL_FREQ_RNAGES;
+ specs++;
+ break;
+ }
+ }
Here I see you are assigning same power to all freq band if for the
first band
the freq index is not provided. Is there any specific reason to only
check the first
here ? Probably this logic should move into specific drivers. Thoughts
?
This logic will be removed per Johannes's comments.
Please read Johannes's comments. If you agree with him and has no other
advices,
then I will post the second version of it.
-Abhishek
_______________________________________________
ath10k mailing list
ath10k@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/ath10k
