All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Wen Gong <wgong@codeaurora.org>, ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v4 4/6] cfg80211: save power spectral density(psd) of regulatory rule
Date: Mon, 27 Sep 2021 13:27:51 +0200	[thread overview]
Message-ID: <a4e56dbbe88771456bcf3051a0891d66d5e34fd4.camel@sipsolutions.net> (raw)
In-Reply-To: <20210924100052.32029-5-wgong@codeaurora.org>

On Fri, 2021-09-24 at 06:00 -0400, Wen Gong wrote:
> The power spectral density(psd) of regulatory rule should be take
> effect to the channels. This patch is to save the values to the
> channel which has psd value.
> 
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> ---
>  net/wireless/reg.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index df87c7f3a049..8f765befb9bc 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1590,6 +1590,8 @@ static u32 map_regdom_flags(u32 rd_flags)
>  		channel_flags |= IEEE80211_CHAN_NO_160MHZ;
>  	if (rd_flags & NL80211_RRF_NO_HE)
>  		channel_flags |= IEEE80211_CHAN_NO_HE;
> +	if (rd_flags & NL80211_RRF_PSD)
> +		channel_flags |= IEEE80211_CHAN_PSD;

I went to go squash this with patch 3 and took a closer look, and then I
realized you're doing this weird.

Please when you resend also squash this - it's a bit weird to read in
two patches.

However, I think this is missing a lot of things - we already talked
about the regulatory database, and while that'd be nice, I guess I
conceded that you don't really have to do it now.

However, for visibility reasons, I *really* think you need to add
nl80211 attributes for all of this data - when the regdomain is dumped
in nl80211_put_regdom() you would have the flag now (it dumps the value
of reg_rule->flags in NL80211_ATTR_REG_RULE_FLAGS), but you didn't add
the value of reg_rule->psd which you've added.

Similarly, you're not adding the PSD flag nor the PSD value for the
*channel* in nl80211_msg_put_channel(), both of which I think you should
have for visibility into what's going on in the kernel/driver.

I've applied all the other patches, so please just resend 3 and 4,
squashed into a single patch, with the fixes.

johannes


WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net>
To: Wen Gong <wgong@codeaurora.org>, ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v4 4/6] cfg80211: save power spectral density(psd) of regulatory rule
Date: Mon, 27 Sep 2021 13:27:51 +0200	[thread overview]
Message-ID: <a4e56dbbe88771456bcf3051a0891d66d5e34fd4.camel@sipsolutions.net> (raw)
In-Reply-To: <20210924100052.32029-5-wgong@codeaurora.org>

On Fri, 2021-09-24 at 06:00 -0400, Wen Gong wrote:
> The power spectral density(psd) of regulatory rule should be take
> effect to the channels. This patch is to save the values to the
> channel which has psd value.
> 
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> ---
>  net/wireless/reg.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index df87c7f3a049..8f765befb9bc 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1590,6 +1590,8 @@ static u32 map_regdom_flags(u32 rd_flags)
>  		channel_flags |= IEEE80211_CHAN_NO_160MHZ;
>  	if (rd_flags & NL80211_RRF_NO_HE)
>  		channel_flags |= IEEE80211_CHAN_NO_HE;
> +	if (rd_flags & NL80211_RRF_PSD)
> +		channel_flags |= IEEE80211_CHAN_PSD;

I went to go squash this with patch 3 and took a closer look, and then I
realized you're doing this weird.

Please when you resend also squash this - it's a bit weird to read in
two patches.

However, I think this is missing a lot of things - we already talked
about the regulatory database, and while that'd be nice, I guess I
conceded that you don't really have to do it now.

However, for visibility reasons, I *really* think you need to add
nl80211 attributes for all of this data - when the regdomain is dumped
in nl80211_put_regdom() you would have the flag now (it dumps the value
of reg_rule->flags in NL80211_ATTR_REG_RULE_FLAGS), but you didn't add
the value of reg_rule->psd which you've added.

Similarly, you're not adding the PSD flag nor the PSD value for the
*channel* in nl80211_msg_put_channel(), both of which I think you should
have for visibility into what's going on in the kernel/driver.

I've applied all the other patches, so please just resend 3 and 4,
squashed into a single patch, with the fixes.

johannes


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

  reply	other threads:[~2021-09-27 11:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 10:00 [PATCH v4 0/6] cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP Wen Gong
2021-09-24 10:00 ` Wen Gong
2021-09-24 10:00 ` [PATCH v4 1/6] mac80211: add power type definition for 6 GHz Wen Gong
2021-09-24 10:00   ` Wen Gong
2021-09-24 10:00 ` [PATCH v4 2/6] mac80211: add parse regulatory info in 6 GHz operation information Wen Gong
2021-09-24 10:00   ` Wen Gong
2021-09-27 11:22   ` Johannes Berg
2021-09-27 11:22     ` Johannes Berg
2021-09-24 10:00 ` [PATCH v4 3/6] cfg80211: add definition for 6 GHz power spectral density(psd) Wen Gong
2021-09-24 10:00   ` Wen Gong
2021-09-27 11:28   ` Johannes Berg
2021-09-27 11:28     ` Johannes Berg
2021-09-24 10:00 ` [PATCH v4 4/6] cfg80211: save power spectral density(psd) of regulatory rule Wen Gong
2021-09-24 10:00   ` Wen Gong
2021-09-27 11:27   ` Johannes Berg [this message]
2021-09-27 11:27     ` Johannes Berg
2021-09-28  9:05     ` Wen Gong
2021-09-28  9:05       ` Wen Gong
2021-09-24 10:00 ` [PATCH v4 5/6] mac80211: use ieee802_11_parse_elems() to find ies instead of ieee80211_bss_get_ie() Wen Gong
2021-09-24 10:00   ` Wen Gong
2021-09-24 10:13   ` Johannes Berg
2021-09-24 10:13     ` Johannes Berg
2021-09-24 10:47     ` Johannes Berg
2021-09-24 10:47       ` Johannes Berg
2021-09-24 10:00 ` [PATCH v4 6/6] mac80211: save transmit power envelope element and power constraint Wen Gong
2021-09-24 10:00   ` Wen Gong

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=a4e56dbbe88771456bcf3051a0891d66d5e34fd4.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=ath11k@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=wgong@codeaurora.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: link
Be 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.