All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jouni Malinen <j@w1.fi>
To: Pkshih <pkshih@realtek.com>
Cc: "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 1/2] nl80211: vendor-cmd: qca: add dynamic SAR power limits
Date: Thu, 19 Dec 2019 17:48:28 +0200	[thread overview]
Message-ID: <20191219154828.GA12287@w1.fi> (raw)
In-Reply-To: <1576748692.7758.17.camel@realtek.com>

On Thu, Dec 19, 2019 at 09:44:52AM +0000, Pkshih wrote:
> On Wed, 2019-12-18 at 17:48 +0200, Kalle Valo wrote:
> > diff --git a/include/uapi/nl80211-vnd-qca.h b/include/uapi/nl80211-vnd-qca.h
> > + * NOTE: The authoritative place for definition of QCA_NL80211_VENDOR_ID,
> > + * vendor subcmd definitions prefixed with QCA_NL80211_VENDOR_SUBCMD, and
> > + * qca_wlan_vendor_attr is open source file src/common/qca-vendor.h in
> > + * git://w1.fi/srv/git/hostap.git; the values here are just a copy of that

> > + * @QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_BAND: Optional (u32) value to
> > + *	indicate for which band this specification applies. Valid
> > + *	values are enumerated in enum %nl80211_band (although not all
> > + *	bands may be supported by a given device). If the attribute is
> 
> Can we define separated enum to address four 5G sub-bands, likes
> 
> enum nl80211_sar_band {
> 	NL80211_SAR_BAND_2G,
> 	NL80211_SAR_BAND_5G_BAND1,
> 	NL80211_SAR_BAND_5G_BAND2,
> 	NL80211_SAR_BAND_5G_BAND3,
> 	NL80211_SAR_BAND_5G_BAND4,
> };

Please note that the vendor subcmd and attributes used here are already
deployed and in use as a kernel interface. As such, the existing
attributes cannot really be modified; if anything else would be needed,
that would need to be defined as a new attribute and/or command.

> I think this vendor command can be a generic nl80211 command, because
> we need SAR
> power limit as well.

This was discussed during the 2019 wireless workshop. The conclusion
from that discussion was that while there is clear need for SAR power
limits for various devices and multiple vendors/drivers, it did not look
clear that a single common interface could be defined cleanly taken into
account the differences in the ways vendors have designed the mechanism
in driver and firmware implementations. As such, vendor specific
commands were identified as the approach.

> > + * @QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_POWER_LIMIT: Required (u32)
> > + *	value to specify the actual power limit value in units of 0.5
> > + *	dBm (i.e., a value of 11 represents 5.5 dBm).
> 
> Can we have higher precision, in unit of 0.125?

This existing attribute cannot be modified, i.e., a new one would need
to be added if a different precision is needed. As far as the specific
need for the vendor command defined here is concerned, 0.5 dB unit is
sufficient.

> > + * These attributes are used with %QCA_NL80211_VENDOR_SUBCMD_SET_SAR_LIMITS
> > + * and %QCA_NL80211_VENDOR_SUBCMD_GET_SAR_LIMITS.
> > + */
> > +enum qca_vendor_attr_sar_limits {
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_INVALID = 0,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC = 3,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_BAND = 4,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_POWER_LIMIT = 7,
> 
> Why these enum aren't continual?
> The reason may be because QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SELECT and something
> me
> ntioned in above paragraph but missing in this enum?

This patch does not include all the assigned values (see hostap.git for
full details if desired), i.e., all the values are actually assigned,
but the proposed use case for ath10k does not need the values that were
left out from this header file that is a copy of the authoritative
definition of the vendor attributes.

-- 
Jouni Malinen                                            PGP id EFC895FA

WARNING: multiple messages have this Message-ID
From: Jouni Malinen <j@w1.fi>
To: Pkshih <pkshih@realtek.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"kvalo@codeaurora.org" <kvalo@codeaurora.org>
Subject: Re: [PATCH 1/2] nl80211: vendor-cmd: qca: add dynamic SAR power limits
Date: Thu, 19 Dec 2019 17:48:28 +0200	[thread overview]
Message-ID: <20191219154828.GA12287@w1.fi> (raw)
In-Reply-To: <1576748692.7758.17.camel@realtek.com>

On Thu, Dec 19, 2019 at 09:44:52AM +0000, Pkshih wrote:
> On Wed, 2019-12-18 at 17:48 +0200, Kalle Valo wrote:
> > diff --git a/include/uapi/nl80211-vnd-qca.h b/include/uapi/nl80211-vnd-qca.h
> > + * NOTE: The authoritative place for definition of QCA_NL80211_VENDOR_ID,
> > + * vendor subcmd definitions prefixed with QCA_NL80211_VENDOR_SUBCMD, and
> > + * qca_wlan_vendor_attr is open source file src/common/qca-vendor.h in
> > + * git://w1.fi/srv/git/hostap.git; the values here are just a copy of that

> > + * @QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_BAND: Optional (u32) value to
> > + *	indicate for which band this specification applies. Valid
> > + *	values are enumerated in enum %nl80211_band (although not all
> > + *	bands may be supported by a given device). If the attribute is
> 
> Can we define separated enum to address four 5G sub-bands, likes
> 
> enum nl80211_sar_band {
> 	NL80211_SAR_BAND_2G,
> 	NL80211_SAR_BAND_5G_BAND1,
> 	NL80211_SAR_BAND_5G_BAND2,
> 	NL80211_SAR_BAND_5G_BAND3,
> 	NL80211_SAR_BAND_5G_BAND4,
> };

Please note that the vendor subcmd and attributes used here are already
deployed and in use as a kernel interface. As such, the existing
attributes cannot really be modified; if anything else would be needed,
that would need to be defined as a new attribute and/or command.

> I think this vendor command can be a generic nl80211 command, because
> we need SAR
> power limit as well.

This was discussed during the 2019 wireless workshop. The conclusion
from that discussion was that while there is clear need for SAR power
limits for various devices and multiple vendors/drivers, it did not look
clear that a single common interface could be defined cleanly taken into
account the differences in the ways vendors have designed the mechanism
in driver and firmware implementations. As such, vendor specific
commands were identified as the approach.

> > + * @QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_POWER_LIMIT: Required (u32)
> > + *	value to specify the actual power limit value in units of 0.5
> > + *	dBm (i.e., a value of 11 represents 5.5 dBm).
> 
> Can we have higher precision, in unit of 0.125?

This existing attribute cannot be modified, i.e., a new one would need
to be added if a different precision is needed. As far as the specific
need for the vendor command defined here is concerned, 0.5 dB unit is
sufficient.

> > + * These attributes are used with %QCA_NL80211_VENDOR_SUBCMD_SET_SAR_LIMITS
> > + * and %QCA_NL80211_VENDOR_SUBCMD_GET_SAR_LIMITS.
> > + */
> > +enum qca_vendor_attr_sar_limits {
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_INVALID = 0,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC = 3,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_BAND = 4,
> > +	QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SPEC_POWER_LIMIT = 7,
> 
> Why these enum aren't continual?
> The reason may be because QCA_WLAN_VENDOR_ATTR_SAR_LIMITS_SELECT and something
> me
> ntioned in above paragraph but missing in this enum?

This patch does not include all the assigned values (see hostap.git for
full details if desired), i.e., all the values are actually assigned,
but the proposed use case for ath10k does not need the values that were
left out from this header file that is a copy of the authoritative
definition of the vendor attributes.

-- 
Jouni Malinen                                            PGP id EFC895FA

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2019-12-19 15:53 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18 15:48 [PATCH 0/2] ath10k: SAR power limit vendor command Kalle Valo
2019-12-18 15:48 ` Kalle Valo
2019-12-18 15:48 ` [PATCH 1/2] nl80211: vendor-cmd: qca: add dynamic SAR power limits Kalle Valo
2019-12-18 15:48   ` Kalle Valo
2019-12-19  9:44   ` Pkshih
2019-12-19  9:44     ` Pkshih
2019-12-19 15:48     ` Jouni Malinen [this message]
2019-12-19 15:48       ` Jouni Malinen
2019-12-19 18:32       ` Brian Norris
2019-12-19 18:32         ` Brian Norris
2019-12-19 18:55         ` Jouni Malinen
2019-12-19 18:55           ` Jouni Malinen
2019-12-19 23:40           ` Brian Norris
2019-12-19 23:40             ` Brian Norris
2020-03-17 16:54             ` Kalle Valo
2020-03-17 16:54               ` Kalle Valo
2020-03-20 12:55               ` Johannes Berg
2020-03-20 12:55                 ` Johannes Berg
2020-06-02  1:32                 ` Brian Norris
2020-06-02  1:32                   ` Brian Norris
2020-07-16  9:35                   ` Kalle Valo
2020-07-16  9:35                     ` Kalle Valo
2020-07-16 18:56                     ` Brian Norris
2020-07-16 18:56                       ` Brian Norris
2020-07-24  9:26                       ` Kalle Valo
2020-07-24  9:26                         ` Kalle Valo
2020-07-30 13:24                         ` Johannes Berg
2020-07-30 13:24                           ` Johannes Berg
2020-08-01  1:31                           ` Brian Norris
2020-08-01  1:31                             ` Brian Norris
2020-09-08  5:55                           ` Kalle Valo
2020-09-08  5:55                           ` Kalle Valo
2020-07-30 13:17                   ` Johannes Berg
2020-07-30 13:17                     ` Johannes Berg
2019-12-18 15:48 ` [PATCH 2/2] ath10k: allow dynamic SAR power limits to be configured Kalle Valo
2019-12-18 15:48   ` Kalle Valo
2019-12-19  9:45   ` Pkshih
2019-12-19  9:45     ` Pkshih
2020-04-16  7:38   ` Kalle Valo
2020-04-16  7:38   ` Kalle Valo

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=20191219154828.GA12287@w1.fi \
    --to=j@w1.fi \
    --cc=ath10k@lists.infradead.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pkshih@realtek.com \
    --subject='Re: [PATCH 1/2] nl80211: vendor-cmd: qca: add dynamic SAR power limits' \
    /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: link

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.