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 ? > +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. > + 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 ? -Abhishek