All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jouni Malinen <j@w1.fi>
To: Brian Norris <briannorris@chromium.org>
Cc: Pkshih <pkshih@realtek.com>,
	"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 20:55:22 +0200	[thread overview]
Message-ID: <20191219185522.GA16010@w1.fi> (raw)
In-Reply-To: <CA+ASDXMTYLGbEkBPHSqtyitMEvY5o_MjU0s+NoWdnN_ORy1gDw@mail.gmail.com>

On Thu, Dec 19, 2019 at 10:32:06AM -0800, Brian Norris wrote:
> On Thu, Dec 19, 2019 at 7:48 AM Jouni Malinen <j@w1.fi> wrote:
> > 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
> 
> By the way, I wonder -- why have this statement? That's not how I
> recall any other piece of kernel development ever working; upstream
> Linux defines the upstream Linux API, not some arbitrary user space
> project. This statement could be useful for saying, "don't stomp on
> those command numbers please," but the response should probably be to
> go out and define a totally new vendor ID or something. (See below.)

As far as the OUI 00:13:74 is concerned, the defined mechanism for
getting any new identifier values assigned is by getting a patch applied
into hostap.git. If another OUI is used, that can clearly use different
mechanism for this. Anyway, I'd expect the process for OUI 00:13:74 to
be the most convenient one to use.

This does not mean that there could not be new subcmds and attributes
defined as part of the upstream kernel review and those changes being
modified as part of that review. However, the final ID values for the
subcmds/attributes would happen through whatever mechanism defined for
the particular OUI. For 00:13:74, that would mean defining the
subcmds/attributes first with TBD ID values and once the definitions are
otherwise fine in the kernel patch review, contributing a patch to
hostap.git to get the identifiers assigned, updating the kernel patch to
use the assigned values, and apply it to the kernel.

> > 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.
> 
> Clarification: you're talking about out-of-tree drivers, which really
> have no relevance in upstream discussion, except possibly as examples.
> I don't think it's ever been a valid approach to dictate upstream
> kernel design based simply on "what $vendor already implemented for
> Android."

There is clearly no requirement to use an existing attribute, but since
there is such an attribute already defined, I'd claim it is perfectly
fine to consider it as an option for this. If something else is
identified to be needed, a new subcmd/attribute can obviously be
defined.

> Maybe it's a better idea to just use different command numbers (or
> vendor ID?) here, to avoid stomping on each others' implementation
> choices. Otherwise, it sounds like our only choice here is to copy
> your Android driver verbatim, or get lost.

I'd use the same OUI 00:13:74 since there is a defined process for
getting identifiers assigned from there for upstream Linux WLAN needs.
Defining other/new non-conflicting subcmds/attributes is of course fine
when needed.

> > 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.
> 
> [citation needed]

I'm not aware of any publicly available meeting minutes that covered the
details for that discussion. My personal notes indicate that there were
at least two vendors indicating existence of vendor specific commands
for configuring SAR parameters, a discussion about the parameters used
for this being different, and a conclusion that this would be an example
kernel interface where a generic nl80211 interface may not be achievable
and a vendor specific interface would be more likely. This discussion
resulted in the discussion on how to use vendor specific nl80211
commands/attributes in upstream drivers and the eventual documentation
of that in the location you noted.

-- 
Jouni Malinen                                            PGP id EFC895FA

WARNING: multiple messages have this Message-ID
From: Jouni Malinen <j@w1.fi>
To: Brian Norris <briannorris@chromium.org>
Cc: Pkshih <pkshih@realtek.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>
Subject: Re: [PATCH 1/2] nl80211: vendor-cmd: qca: add dynamic SAR power limits
Date: Thu, 19 Dec 2019 20:55:22 +0200	[thread overview]
Message-ID: <20191219185522.GA16010@w1.fi> (raw)
In-Reply-To: <CA+ASDXMTYLGbEkBPHSqtyitMEvY5o_MjU0s+NoWdnN_ORy1gDw@mail.gmail.com>

On Thu, Dec 19, 2019 at 10:32:06AM -0800, Brian Norris wrote:
> On Thu, Dec 19, 2019 at 7:48 AM Jouni Malinen <j@w1.fi> wrote:
> > 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
> 
> By the way, I wonder -- why have this statement? That's not how I
> recall any other piece of kernel development ever working; upstream
> Linux defines the upstream Linux API, not some arbitrary user space
> project. This statement could be useful for saying, "don't stomp on
> those command numbers please," but the response should probably be to
> go out and define a totally new vendor ID or something. (See below.)

As far as the OUI 00:13:74 is concerned, the defined mechanism for
getting any new identifier values assigned is by getting a patch applied
into hostap.git. If another OUI is used, that can clearly use different
mechanism for this. Anyway, I'd expect the process for OUI 00:13:74 to
be the most convenient one to use.

This does not mean that there could not be new subcmds and attributes
defined as part of the upstream kernel review and those changes being
modified as part of that review. However, the final ID values for the
subcmds/attributes would happen through whatever mechanism defined for
the particular OUI. For 00:13:74, that would mean defining the
subcmds/attributes first with TBD ID values and once the definitions are
otherwise fine in the kernel patch review, contributing a patch to
hostap.git to get the identifiers assigned, updating the kernel patch to
use the assigned values, and apply it to the kernel.

> > 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.
> 
> Clarification: you're talking about out-of-tree drivers, which really
> have no relevance in upstream discussion, except possibly as examples.
> I don't think it's ever been a valid approach to dictate upstream
> kernel design based simply on "what $vendor already implemented for
> Android."

There is clearly no requirement to use an existing attribute, but since
there is such an attribute already defined, I'd claim it is perfectly
fine to consider it as an option for this. If something else is
identified to be needed, a new subcmd/attribute can obviously be
defined.

> Maybe it's a better idea to just use different command numbers (or
> vendor ID?) here, to avoid stomping on each others' implementation
> choices. Otherwise, it sounds like our only choice here is to copy
> your Android driver verbatim, or get lost.

I'd use the same OUI 00:13:74 since there is a defined process for
getting identifiers assigned from there for upstream Linux WLAN needs.
Defining other/new non-conflicting subcmds/attributes is of course fine
when needed.

> > 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.
> 
> [citation needed]

I'm not aware of any publicly available meeting minutes that covered the
details for that discussion. My personal notes indicate that there were
at least two vendors indicating existence of vendor specific commands
for configuring SAR parameters, a discussion about the parameters used
for this being different, and a conclusion that this would be an example
kernel interface where a generic nl80211 interface may not be achievable
and a vendor specific interface would be more likely. This discussion
resulted in the discussion on how to use vendor specific nl80211
commands/attributes in upstream drivers and the eventual documentation
of that in the location you noted.

-- 
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 18:58 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
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 [this message]
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=20191219185522.GA16010@w1.fi \
    --to=j@w1.fi \
    --cc=ath10k@lists.infradead.org \
    --cc=briannorris@chromium.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.