diff mbox series

[v5] cfg80211: save power spectral density(psd) of regulatory rule

Message ID 20210928085211.26186-1-wgong@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series [v5] cfg80211: save power spectral density(psd) of regulatory rule | expand

Commit Message

Wen Gong Sept. 28, 2021, 8:52 a.m. UTC
6 GHz regulatory domains introduces power spectral density(psd).
The power spectral density(psd) of regulatory rule should be take
effect to the channels. Save the values to the channel which has
psd value and add nl80211 attributes for it.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
v5: change by comments of johannes.
    1. add handler for nl80211 attributes
    2. add indentation for NL80211_RRF_PSD
    3. squashed with "cfg80211: add definition for 6 GHz power spectral density(psd)"
    4. remove all other patches

v4:
    1. rebased to top commit id 9e263e193af7 kernel/git/jberg/mac80211-next.git
    2. add NULL check for "mac80211: use ieee802_11_parse_elems() to find ies instead of ieee80211_bss_get_ie()"
    3. remove the 3 patches which already upstream:
       "mac80211: parse transmit power envelope element"
       "ieee80211: add definition for transmit power envelope element"
       "ieee80211: add definition of regulatory info in 6 GHz operation information"

v3: change per comments of Johannes.
    1. add patch "mac80211: use ieee802_11_parse_elems() to find ies instead of ieee80211_bss_get_ie()"
    2. move nl80211_ap_reg_power to ieee80211
    3. change some comments, length check, stack big size variable...

v2: change per comments of johannes.
    including code style, code logic, patch merge, commit log...

 include/net/cfg80211.h       |  5 +++++
 include/net/regulatory.h     |  1 +
 include/uapi/linux/nl80211.h |  9 +++++++++
 net/wireless/nl80211.c       | 18 ++++++++++++++++++
 net/wireless/reg.c           | 17 +++++++++++++++++
 5 files changed, 50 insertions(+)

Comments

Venkateswara Naralasetty Sept. 28, 2021, 1:12 p.m. UTC | #1
On 2021-09-28 14:22, Wen Gong wrote:
> 6 GHz regulatory domains introduces power spectral density(psd).
> The power spectral density(psd) of regulatory rule should be take
> effect to the channels. Save the values to the channel which has
> psd value and add nl80211 attributes for it.
> 
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> ---
> v5: change by comments of johannes.
>     1. add handler for nl80211 attributes
>     2. add indentation for NL80211_RRF_PSD
>     3. squashed with "cfg80211: add definition for 6 GHz power
> spectral density(psd)"
>     4. remove all other patches
> 
> v4:
>     1. rebased to top commit id 9e263e193af7 
> kernel/git/jberg/mac80211-next.git
>     2. add NULL check for "mac80211: use ieee802_11_parse_elems() to
> find ies instead of ieee80211_bss_get_ie()"
>     3. remove the 3 patches which already upstream:
>        "mac80211: parse transmit power envelope element"
>        "ieee80211: add definition for transmit power envelope element"
>        "ieee80211: add definition of regulatory info in 6 GHz
> operation information"
> 
> v3: change per comments of Johannes.
>     1. add patch "mac80211: use ieee802_11_parse_elems() to find ies
> instead of ieee80211_bss_get_ie()"
>     2. move nl80211_ap_reg_power to ieee80211
>     3. change some comments, length check, stack big size variable...
> 
> v2: change per comments of johannes.
>     including code style, code logic, patch merge, commit log...
> 
>  include/net/cfg80211.h       |  5 +++++
>  include/net/regulatory.h     |  1 +
>  include/uapi/linux/nl80211.h |  9 +++++++++
>  net/wireless/nl80211.c       | 18 ++++++++++++++++++
>  net/wireless/reg.c           | 17 +++++++++++++++++
>  5 files changed, 50 insertions(+)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 62dd8422e0dc..f8e0cc19e0ce 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -109,6 +109,8 @@ struct wiphy;
>   *	on this channel.
>   * @IEEE80211_CHAN_16MHZ: 16 MHz bandwidth is permitted
>   *	on this channel.
> + * @IEEE80211_CHAN_PSD: power spectral density (in dBm)
> + *	on this channel.
>   *
>   */
>  enum ieee80211_channel_flags {
> @@ -131,6 +133,7 @@ enum ieee80211_channel_flags {
>  	IEEE80211_CHAN_4MHZ		= 1<<16,
>  	IEEE80211_CHAN_8MHZ		= 1<<17,
>  	IEEE80211_CHAN_16MHZ		= 1<<18,
> +	IEEE80211_CHAN_PSD		= 1<<19,
>  };
> 
>  #define IEEE80211_CHAN_NO_HT40 \
> @@ -164,6 +167,7 @@ enum ieee80211_channel_flags {
>   *	on this channel.
>   * @dfs_state_entered: timestamp (jiffies) when the dfs state was 
> entered.
>   * @dfs_cac_ms: DFS CAC time in milliseconds, this is valid for DFS 
> channels.
> + * @psd: power spectral density (in dBm)
>   */
>  struct ieee80211_channel {
>  	enum nl80211_band band;
> @@ -180,6 +184,7 @@ struct ieee80211_channel {
>  	enum nl80211_dfs_state dfs_state;
>  	unsigned long dfs_state_entered;
>  	unsigned int dfs_cac_ms;
> +	s8 psd;
>  };
> 
>  /**
> diff --git a/include/net/regulatory.h b/include/net/regulatory.h
> index 47f06f6f5a67..ed20004fb6a9 100644
> --- a/include/net/regulatory.h
> +++ b/include/net/regulatory.h
> @@ -221,6 +221,7 @@ struct ieee80211_reg_rule {
>  	u32 flags;
>  	u32 dfs_cac_ms;
>  	bool has_wmm;
> +	s8 psd;
>  };
> 
>  struct ieee80211_regdomain {
> diff --git a/include/uapi/linux/nl80211.h 
> b/include/uapi/linux/nl80211.h
> index c2efea98e060..ae5f69aa727b 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -3851,6 +3851,8 @@ enum nl80211_wmm_rule {
>   *	on this channel in current regulatory domain.
>   * @NL80211_FREQUENCY_ATTR_16MHZ: 16 MHz operation is allowed
>   *	on this channel in current regulatory domain.
> + * @NL80211_FREQUENCY_ATTR_PSD: power spectral density (in dBm)
> + *	is allowed on this channel in current regulatory domain.
>   * @NL80211_FREQUENCY_ATTR_MAX: highest frequency attribute number
>   *	currently defined
>   * @__NL80211_FREQUENCY_ATTR_AFTER_LAST: internal use
> @@ -3887,6 +3889,7 @@ enum nl80211_frequency_attr {
>  	NL80211_FREQUENCY_ATTR_4MHZ,
>  	NL80211_FREQUENCY_ATTR_8MHZ,
>  	NL80211_FREQUENCY_ATTR_16MHZ,
> +	NL80211_FREQUENCY_ATTR_PSD,
> 
>  	/* keep last */
>  	__NL80211_FREQUENCY_ATTR_AFTER_LAST,
> @@ -3987,6 +3990,8 @@ enum nl80211_reg_type {
>   * 	a given frequency range. The value is in mBm (100 * dBm).
>   * @NL80211_ATTR_DFS_CAC_TIME: DFS CAC time in milliseconds.
>   *	If not present or 0 default CAC time will be used.
> + * @NL80211_ATTR_POWER_RULE_PSD: power spectral density (in dBm).
> + *	This could be negative.
>   * @NL80211_REG_RULE_ATTR_MAX: highest regulatory rule attribute 
> number
>   *	currently defined
>   * @__NL80211_REG_RULE_ATTR_AFTER_LAST: internal use
> @@ -4004,6 +4009,8 @@ enum nl80211_reg_rule_attr {
> 
>  	NL80211_ATTR_DFS_CAC_TIME,
> 
> +	NL80211_ATTR_POWER_RULE_PSD,
> +
>  	/* keep last */
>  	__NL80211_REG_RULE_ATTR_AFTER_LAST,
>  	NL80211_REG_RULE_ATTR_MAX = __NL80211_REG_RULE_ATTR_AFTER_LAST - 1
> @@ -4085,6 +4092,7 @@ enum nl80211_sched_scan_match_attr {
>   * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
>   * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
>   * @NL80211_RRF_NO_HE: HE operation not allowed
> + * @NL80211_RRF_PSD: channels has power spectral density value
>   */
>  enum nl80211_reg_rule_flags {
>  	NL80211_RRF_NO_OFDM		= 1<<0,
> @@ -4103,6 +4111,7 @@ enum nl80211_reg_rule_flags {
>  	NL80211_RRF_NO_80MHZ		= 1<<15,
>  	NL80211_RRF_NO_160MHZ		= 1<<16,
>  	NL80211_RRF_NO_HE		= 1<<17,
> +	NL80211_RRF_PSD			= 1<<18,
>  };
> 
>  #define NL80211_RRF_PASSIVE_SCAN	NL80211_RRF_NO_IR
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 0b4f29d689d2..9f1c5be11c95 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -1055,6 +1055,10 @@ static int nl80211_msg_put_channel(struct
> sk_buff *msg, struct wiphy *wiphy,
>  	if (nla_put_u32(msg, NL80211_FREQUENCY_ATTR_OFFSET, 
> chan->freq_offset))
>  		goto nla_put_failure;
> 
> +	if ((chan->flags & IEEE80211_CHAN_PSD) &&
> +	    nla_put_s8(msg, NL80211_FREQUENCY_ATTR_PSD, chan->psd))
> +		goto nla_put_failure;
> +
>  	if ((chan->flags & IEEE80211_CHAN_DISABLED) &&
>  	    nla_put_flag(msg, NL80211_FREQUENCY_ATTR_DISABLED))
>  		goto nla_put_failure;
> @@ -7755,6 +7759,11 @@ static int nl80211_put_regdom(const struct
> ieee80211_regdomain *regdom,
>  				reg_rule->dfs_cac_ms))
>  			goto nla_put_failure;
> 
> +		if ((reg_rule->flags & NL80211_RRF_PSD) &&
> +		    nla_put_s8(msg, NL80211_ATTR_POWER_RULE_PSD,
> +			       reg_rule->psd))
> +			goto nla_put_failure;
> +
>  		nla_nest_end(msg, nl_reg_rule);
>  	}
> 
> @@ -7926,6 +7935,7 @@ static const struct nla_policy
> reg_rule_policy[NL80211_REG_RULE_ATTR_MAX + 1] =
>  	[NL80211_ATTR_POWER_RULE_MAX_ANT_GAIN]	= { .type = NLA_U32 },
>  	[NL80211_ATTR_POWER_RULE_MAX_EIRP]	= { .type = NLA_U32 },
>  	[NL80211_ATTR_DFS_CAC_TIME]		= { .type = NLA_U32 },
> +	[NL80211_ATTR_POWER_RULE_PSD]		= { .type = NLA_S8 },
>  };
> 
>  static int parse_reg_rule(struct nlattr *tb[],
> @@ -7947,6 +7957,14 @@ static int parse_reg_rule(struct nlattr *tb[],
> 
>  	reg_rule->flags = nla_get_u32(tb[NL80211_ATTR_REG_RULE_FLAGS]);
> 
> +	if (reg_rule->flags & NL80211_RRF_PSD) {
> +		if (!tb[NL80211_ATTR_POWER_RULE_PSD])
> +			return -EINVAL;
> +
> +		reg_rule->psd =
> +			nla_get_s8(tb[NL80211_ATTR_POWER_RULE_PSD]);
> +	}
> +
>  	freq_range->start_freq_khz =
>  		nla_get_u32(tb[NL80211_ATTR_FREQ_RANGE_START]);
>  	freq_range->end_freq_khz =
> 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;
>  	return channel_flags;
>  }
> 
> @@ -1794,6 +1796,9 @@ static void handle_channel_single_rule(struct
> wiphy *wiphy,
>  				chan->dfs_cac_ms = reg_rule->dfs_cac_ms;
>  		}
> 
> +		if (chan->flags & IEEE80211_CHAN_PSD)
> +			chan->psd = reg_rule->psd;
> +
>  		return;
>  	}
> 
> @@ -1814,6 +1819,9 @@ static void handle_channel_single_rule(struct
> wiphy *wiphy,
>  			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
>  	}
> 
> +	if (chan->flags & IEEE80211_CHAN_PSD)
> +		chan->psd = reg_rule->psd;
> +
>  	if (chan->orig_mpwr) {
>  		/*
>  		 * Devices that use REGULATORY_COUNTRY_IE_FOLLOW_POWER
> @@ -1883,6 +1891,12 @@ static void
> handle_channel_adjacent_rules(struct wiphy *wiphy,
>  							 rrule2->dfs_cac_ms);
>  		}
> 
> +		if ((rrule1->flags & NL80211_RRF_PSD) &&
> +		    (rrule2->flags & NL80211_RRF_PSD))
> +			chan->psd = min_t(s8, rrule1->psd, rrule2->psd);
> +		else
> +			chan->flags &= ~NL80211_RRF_PSD;
> +
>  		return;
>  	}
> 
> @@ -2540,6 +2554,9 @@ static void handle_channel_custom(struct wiphy 
> *wiphy,
>  			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
>  	}
> 
> +	if (chan->flags & IEEE80211_CHAN_PSD)
> +		chan->psd = reg_rule->psd;
> +
>  	chan->max_power = chan->max_reg_power;

What about the case AP + STA concurrency? are we going to overwrite the 
PSD power and channel flags?

>  }
Wen Gong Sept. 29, 2021, 3:37 a.m. UTC | #2
On 2021-09-28 21:12, vnaralas@codeaurora.org wrote:
> On 2021-09-28 14:22, Wen Gong wrote:
>> 6 GHz regulatory domains introduces power spectral density(psd).
>> The power spectral density(psd) of regulatory rule should be take
>> effect to the channels. Save the values to the channel which has
>> psd value and add nl80211 attributes for it.
>> 
>> Signed-off-by: Wen Gong <wgong@codeaurora.org>
>> ---
...
>> 
>> @@ -2540,6 +2554,9 @@ static void handle_channel_custom(struct wiphy 
>> *wiphy,
>>  			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
>>  	}
>> 
>> +	if (chan->flags & IEEE80211_CHAN_PSD)
>> +		chan->psd = reg_rule->psd;
>> +
>>  	chan->max_power = chan->max_reg_power;
> 
> What about the case AP + STA concurrency? are we going to overwrite
> the PSD power and channel flags?
> 

Hi Venkateswara,

This patch is not relation with AP + STA concurrency.
For example, it also has other power intersection in 
handle_channel_adjacent_rules().

		chan->max_reg_power =
			min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
			      MBM_TO_DBM(power_rule2->max_eirp));

For AP + STA concurrency, it should to maintain 2 group of reg rules, 
one is for AP, another is for STA.
This patch is to handle PSD info in the same reg rules.
It is to process only one reg rule in the reg rules.
AP + STA concurrency is a higher level things than this patch.
>>  }
Venkateswara Naralasetty Sept. 29, 2021, 4:46 p.m. UTC | #3
On 2021-09-29 09:07, Wen Gong wrote:
> On 2021-09-28 21:12, vnaralas@codeaurora.org wrote:
>> On 2021-09-28 14:22, Wen Gong wrote:
>>> 6 GHz regulatory domains introduces power spectral density(psd).
>>> The power spectral density(psd) of regulatory rule should be take
>>> effect to the channels. Save the values to the channel which has
>>> psd value and add nl80211 attributes for it.
>>> 
>>> Signed-off-by: Wen Gong <wgong@codeaurora.org>
>>> ---
> ...
>>> 
>>> @@ -2540,6 +2554,9 @@ static void handle_channel_custom(struct wiphy 
>>> *wiphy,
>>>  			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
>>>  	}
>>> 
>>> +	if (chan->flags & IEEE80211_CHAN_PSD)
>>> +		chan->psd = reg_rule->psd;
>>> +
>>>  	chan->max_power = chan->max_reg_power;
>> 
>> What about the case AP + STA concurrency? are we going to overwrite
>> the PSD power and channel flags?
>> 
> 
> Hi Venkateswara,
> 
> This patch is not relation with AP + STA concurrency.
> For example, it also has other power intersection in
> handle_channel_adjacent_rules().
> 
> 		chan->max_reg_power =
> 			min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
> 			      MBM_TO_DBM(power_rule2->max_eirp));
> 
> For AP + STA concurrency, it should to maintain 2 group of reg rules,
> one is for AP, another is for STA.

Can we maintain two power rules in the same channel one for AP and one 
for STA. In this way, we can update the power rules in the same channel 
for both AP and STA from the reg rules.

Otherwise, we need to maintain multiple channel lists in sband for all 
supported power mode combinations to apply the respective power rules 
and build channel flags from the multiple reg rules.
right?

> This patch is to handle PSD info in the same reg rules.
> It is to process only one reg rule in the reg rules.
> AP + STA concurrency is a higher level things than this patch.
>>>  }
Wen Gong Sept. 30, 2021, 2:53 a.m. UTC | #4
On 2021-09-30 00:46, Venkateswara Naralasetty wrote:
> On 2021-09-29 09:07, Wen Gong wrote:
>> On 2021-09-28 21:12, vnaralas@codeaurora.org wrote:
>>> On 2021-09-28 14:22, Wen Gong wrote:
>>>> 6 GHz regulatory domains introduces power spectral density(psd).
>>>> The power spectral density(psd) of regulatory rule should be take
>>>> effect to the channels. Save the values to the channel which has
>>>> psd value and add nl80211 attributes for it.
>>>> 
>>>> Signed-off-by: Wen Gong <wgong@codeaurora.org>
>>>> ---
>> ...
>>>> 
>>>> @@ -2540,6 +2554,9 @@ static void handle_channel_custom(struct wiphy 
>>>> *wiphy,
>>>>  			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
>>>>  	}
>>>> 
>>>> +	if (chan->flags & IEEE80211_CHAN_PSD)
>>>> +		chan->psd = reg_rule->psd;
>>>> +
>>>>  	chan->max_power = chan->max_reg_power;
>>> 
>>> What about the case AP + STA concurrency? are we going to overwrite
>>> the PSD power and channel flags?
>>> 
>> 
>> Hi Venkateswara,
>> 
>> This patch is not relation with AP + STA concurrency.
>> For example, it also has other power intersection in
>> handle_channel_adjacent_rules().
>> 
>> 		chan->max_reg_power =
>> 			min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
>> 			      MBM_TO_DBM(power_rule2->max_eirp));
>> 
>> For AP + STA concurrency, it should to maintain 2 group of reg rules,
>> one is for AP, another is for STA.
> 
> Can we maintain two power rules in the same channel one for AP and one
> for STA. In this way, we can update the power rules in the same
> channel for both AP and STA from the reg rules.
> 
> Otherwise, we need to maintain multiple channel lists in sband for all
> supported power mode combinations to apply the respective power rules
> and build channel flags from the multiple reg rules.
> right?

If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is 
different
with STA, then it should maintain muti channel list for each band of the 
wiphy/ieee80211_hw
by my understand.

Currently there is only one "struct ieee80211_supported_band 
*bands[NUM_NL80211_BANDS]"
in "struct wiphy".

I advise to discuss the AP + STA concurrency in another mail thread 
since it is not
relative with this patch.

> 
>> This patch is to handle PSD info in the same reg rules.
>> It is to process only one reg rule in the reg rules.
>> AP + STA concurrency is a higher level things than this patch.
>>>>  }
Johannes Berg Sept. 30, 2021, 12:50 p.m. UTC | #5
On Thu, 2021-09-30 at 10:53 +0800, Wen Gong wrote:
> > > 
> > > 		chan->max_reg_power =
> > > 			min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
> > > 			      MBM_TO_DBM(power_rule2->max_eirp));
> > > 
> > > For AP + STA concurrency, it should to maintain 2 group of reg rules,
> > > one is for AP, another is for STA.
> > 
> > Can we maintain two power rules in the same channel one for AP and one
> > for STA. In this way, we can update the power rules in the same
> > channel for both AP and STA from the reg rules.
> > 
> > Otherwise, we need to maintain multiple channel lists in sband for all
> > supported power mode combinations to apply the respective power rules
> > and build channel flags from the multiple reg rules.
> > right?
> 
> If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is 
> different
> with STA, then it should maintain muti channel list for each band of the 
> wiphy/ieee80211_hw by my understand.

I don't think that's how it works. You can today have AP/STA concurrency
on a single wiphy with different netdevs, even with mesh or whatever.

> Currently there is only one "struct ieee80211_supported_band 
> *bands[NUM_NL80211_BANDS]"
> in "struct wiphy".
> 
> I advise to discuss the AP + STA concurrency in another mail thread 
> since it is not relative with this patch.

I actually explicitly pointed to this thread, but I'm not sure it's so
clear cut?

If we have completely separate rules here for AP and STA, we probably
should have different "max_reg_power" values for AP and STA? Maybe mesh
is treated like AP, maybe not?

But I don't know - does PSD really differ between AP and STA?

Maybe this discussion belongs rather to the power type patch? But that
didn't add any state!


So - does this PSD depend on mode? It kind of seems like it shouldn't
and then this *isn't* the right place to be discussing this, but if PSD
does in fact depend on the mode then we should be discussing it here?

Venkatesh seemed to be worried more about LPI/client power etc. as in
commit 405fca8a9461 ("ieee80211: add power type definition for 6 GHz"),
but that doesn't add state?

So what gives? From a regulatory POV it seems PSD should be independent,
but some other things might be dependent on mode?

johannes
Wen Gong Oct. 11, 2021, 4:06 a.m. UTC | #6
On 2021-09-30 20:50, Johannes Berg wrote:
> On Thu, 2021-09-30 at 10:53 +0800, Wen Gong wrote:
>> > >
>> > > 		chan->max_reg_power =
>> > > 			min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
>> > > 			      MBM_TO_DBM(power_rule2->max_eirp));
>> > >
>> > > For AP + STA concurrency, it should to maintain 2 group of reg rules,
>> > > one is for AP, another is for STA.
>> >
>> > Can we maintain two power rules in the same channel one for AP and one
>> > for STA. In this way, we can update the power rules in the same
>> > channel for both AP and STA from the reg rules.
>> >
>> > Otherwise, we need to maintain multiple channel lists in sband for all
>> > supported power mode combinations to apply the respective power rules
>> > and build channel flags from the multiple reg rules.
>> > right?
>> 
>> If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is
>> different
>> with STA, then it should maintain muti channel list for each band of 
>> the
>> wiphy/ieee80211_hw by my understand.
> 
> I don't think that's how it works. You can today have AP/STA 
> concurrency
> on a single wiphy with different netdevs, even with mesh or whatever.
> 
>> Currently there is only one "struct ieee80211_supported_band
>> *bands[NUM_NL80211_BANDS]"
>> in "struct wiphy".
>> 
>> I advise to discuss the AP + STA concurrency in another mail thread
>> since it is not relative with this patch.
> 
> I actually explicitly pointed to this thread, but I'm not sure it's so
> clear cut?
> 
> If we have completely separate rules here for AP and STA, we probably
> should have different "max_reg_power" values for AP and STA? Maybe mesh
> is treated like AP, maybe not?
> 
> But I don't know - does PSD really differ between AP and STA?
> 
> Maybe this discussion belongs rather to the power type patch? But that
> didn't add any state!
> 
> 
> So - does this PSD depend on mode? It kind of seems like it shouldn't
> and then this *isn't* the right place to be discussing this, but if PSD
> does in fact depend on the mode then we should be discussing it here?
> 
> Venkatesh seemed to be worried more about LPI/client power etc. as in
> commit 405fca8a9461 ("ieee80211: add power type definition for 6 GHz"),
> but that doesn't add state?
> 
> So what gives? From a regulatory POV it seems PSD should be 
> independent,
> but some other things might be dependent on mode?
> 

As I know, below values maybe all different for the AP and
STATION in the same wiphy/ieee80211_hw, not only PSD.

struct ieee80211_reg_rule {
	struct ieee80211_freq_range freq_range;
	struct ieee80211_power_rule power_rule;
	struct ieee80211_wmm_rule wmm_rule;
	u32 flags;
	u32 dfs_cac_ms;
	bool has_wmm;
	s8 psd;
};

@Venkateswara, please feel free to give more info to Johannes:)

> johannes
Venkateswara Naralasetty Oct. 11, 2021, 6:43 a.m. UTC | #7
> -----Original Message-----
> From: ath11k <ath11k-bounces@lists.infradead.org> On Behalf Of Wen Gong
> Sent: Monday, October 11, 2021 9:36 AM
> To: Johannes Berg <johannes@sipsolutions.net>
> Cc: Venkateswara Naralasetty <vnaralas@codeaurora.org>;
> ath11k@lists.infradead.org; linux-wireless@vger.kernel.org;
> wgong=codeaurora.org@codeaurora.org
> Subject: Re: [PATCH v5] cfg80211: save power spectral density(psd) of
> regulatory rule
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 2021-09-30 20:50, Johannes Berg wrote:
> > On Thu, 2021-09-30 at 10:53 +0800, Wen Gong wrote:
> >> > >
> >> > >          chan->max_reg_power =
> >> > >                  min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
> >> > >                        MBM_TO_DBM(power_rule2->max_eirp));
> >> > >
> >> > > For AP + STA concurrency, it should to maintain 2 group of reg
> >> > > rules, one is for AP, another is for STA.
> >> >
> >> > Can we maintain two power rules in the same channel one for AP and
> >> > one for STA. In this way, we can update the power rules in the same
> >> > channel for both AP and STA from the reg rules.
> >> >
> >> > Otherwise, we need to maintain multiple channel lists in sband for
> >> > all supported power mode combinations to apply the respective power
> >> > rules and build channel flags from the multiple reg rules.
> >> > right?
> >>
> >> If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is
> >> different with STA, then it should maintain muti channel list for
> >> each band of the wiphy/ieee80211_hw by my understand.
> >
> > I don't think that's how it works. You can today have AP/STA
> > concurrency on a single wiphy with different netdevs, even with mesh
> > or whatever.
> >
> >> Currently there is only one "struct ieee80211_supported_band
> >> *bands[NUM_NL80211_BANDS]"
> >> in "struct wiphy".
> >>
> >> I advise to discuss the AP + STA concurrency in another mail thread
> >> since it is not relative with this patch.
> >
> > I actually explicitly pointed to this thread, but I'm not sure it's so
> > clear cut?
> >
> > If we have completely separate rules here for AP and STA, we probably
> > should have different "max_reg_power" values for AP and STA? Maybe
> > mesh is treated like AP, maybe not?
> >
> > But I don't know - does PSD really differ between AP and STA?
> >
> > Maybe this discussion belongs rather to the power type patch? But that
> > didn't add any state!
> >
> >
> > So - does this PSD depend on mode? It kind of seems like it shouldn't
> > and then this *isn't* the right place to be discussing this, but if
> > PSD does in fact depend on the mode then we should be discussing it
> here?
> >
> > Venkatesh seemed to be worried more about LPI/client power etc. as in
> > commit 405fca8a9461 ("ieee80211: add power type definition for 6
> > GHz"), but that doesn't add state?
> >
> > So what gives? From a regulatory POV it seems PSD should be
> > independent, but some other things might be dependent on mode?
> >
> 
> As I know, below values maybe all different for the AP and
> STATION in the same wiphy/ieee80211_hw, not only PSD.
> 
> struct ieee80211_reg_rule {
>         struct ieee80211_freq_range freq_range;
>         struct ieee80211_power_rule power_rule;
>         struct ieee80211_wmm_rule wmm_rule;
>         u32 flags;
>         u32 dfs_cac_ms;
>         bool has_wmm;
>         s8 psd;
> };
IMO, Only power rules and PSD info might vary for AP and STATION. Rest of the rules will remains same right?

> 
> @Venkateswara, please feel free to give more info to Johannes:)
> 
> > johannes
> 
> --
> ath11k mailing list
> ath11k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath11k
Wen Gong Oct. 11, 2021, 7:48 a.m. UTC | #8
On 2021-10-11 14:43, Venkateswara Naralasetty wrote:
>> -----Original Message-----
>> From: ath11k <ath11k-bounces@lists.infradead.org> On Behalf Of Wen 
>> Gong
>> Sent: Monday, October 11, 2021 9:36 AM
>> To: Johannes Berg <johannes@sipsolutions.net>
>> Cc: Venkateswara Naralasetty <vnaralas@codeaurora.org>;
>> ath11k@lists.infradead.org; linux-wireless@vger.kernel.org;
>> wgong=codeaurora.org@codeaurora.org
>> Subject: Re: [PATCH v5] cfg80211: save power spectral density(psd) of
>> regulatory rule
>> 
>> WARNING: This email originated from outside of Qualcomm. Please be 
>> wary
>> of any links or attachments, and do not enable macros.
>> 
>> On 2021-09-30 20:50, Johannes Berg wrote:
>> > On Thu, 2021-09-30 at 10:53 +0800, Wen Gong wrote:
>> >> > >
>> >> > >          chan->max_reg_power =
>> >> > >                  min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
>> >> > >                        MBM_TO_DBM(power_rule2->max_eirp));
>> >> > >
>> >> > > For AP + STA concurrency, it should to maintain 2 group of reg
>> >> > > rules, one is for AP, another is for STA.
>> >> >
>> >> > Can we maintain two power rules in the same channel one for AP and
>> >> > one for STA. In this way, we can update the power rules in the same
>> >> > channel for both AP and STA from the reg rules.
>> >> >
>> >> > Otherwise, we need to maintain multiple channel lists in sband for
>> >> > all supported power mode combinations to apply the respective power
>> >> > rules and build channel flags from the multiple reg rules.
>> >> > right?
>> >>
>> >> If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is
>> >> different with STA, then it should maintain muti channel list for
>> >> each band of the wiphy/ieee80211_hw by my understand.
>> >
>> > I don't think that's how it works. You can today have AP/STA
>> > concurrency on a single wiphy with different netdevs, even with mesh
>> > or whatever.
>> >
>> >> Currently there is only one "struct ieee80211_supported_band
>> >> *bands[NUM_NL80211_BANDS]"
>> >> in "struct wiphy".
>> >>
>> >> I advise to discuss the AP + STA concurrency in another mail thread
>> >> since it is not relative with this patch.
>> >
>> > I actually explicitly pointed to this thread, but I'm not sure it's so
>> > clear cut?
>> >
>> > If we have completely separate rules here for AP and STA, we probably
>> > should have different "max_reg_power" values for AP and STA? Maybe
>> > mesh is treated like AP, maybe not?
>> >
>> > But I don't know - does PSD really differ between AP and STA?
>> >
>> > Maybe this discussion belongs rather to the power type patch? But that
>> > didn't add any state!
>> >
>> >
>> > So - does this PSD depend on mode? It kind of seems like it shouldn't
>> > and then this *isn't* the right place to be discussing this, but if
>> > PSD does in fact depend on the mode then we should be discussing it
>> here?
>> >
>> > Venkatesh seemed to be worried more about LPI/client power etc. as in
>> > commit 405fca8a9461 ("ieee80211: add power type definition for 6
>> > GHz"), but that doesn't add state?
>> >
>> > So what gives? From a regulatory POV it seems PSD should be
>> > independent, but some other things might be dependent on mode?
>> >
>> 
>> As I know, below values maybe all different for the AP and
>> STATION in the same wiphy/ieee80211_hw, not only PSD.
>> 
>> struct ieee80211_reg_rule {
>>         struct ieee80211_freq_range freq_range;
>>         struct ieee80211_power_rule power_rule;
>>         struct ieee80211_wmm_rule wmm_rule;
>>         u32 flags;
>>         u32 dfs_cac_ms;
>>         bool has_wmm;
>>         s8 psd;
>> };
> IMO, Only power rules and PSD info might vary for AP and STATION. Rest
> of the rules will remains same right?
> 
The freq_range may also be different for AP and STATION.
and reg_rules number also may also be different for AP and STATION.

for example:
SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
dB/MHz)
reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
dB/MHz)

INDOOR AP reg rules number 1
reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0 
dB/MHz)

>> 
>> @Venkateswara, please feel free to give more info to Johannes:)
>> 
>> > johannes
>> 
>> --
>> ath11k mailing list
>> ath11k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath11k
Wen Gong Oct. 13, 2021, 3:33 a.m. UTC | #9
Hi Johannes,

Do you think what change is needed for this patch?

It should need another patchset to handle AP+STA... in
same wiphy/ieee80211_hw feature by my understand.

On 2021-10-11 15:48, Wen Gong wrote:
> On 2021-10-11 14:43, Venkateswara Naralasetty wrote:
>>> -----Original Message-----
>>> From: ath11k <ath11k-bounces@lists.infradead.org> On Behalf Of Wen 
>>> Gong
>>> Sent: Monday, October 11, 2021 9:36 AM
>>> To: Johannes Berg <johannes@sipsolutions.net>
>>> Cc: Venkateswara Naralasetty <vnaralas@codeaurora.org>;
>>> ath11k@lists.infradead.org; linux-wireless@vger.kernel.org;
>>> wgong=codeaurora.org@codeaurora.org
>>> Subject: Re: [PATCH v5] cfg80211: save power spectral density(psd) of
>>> regulatory rule
>>> 
>>> WARNING: This email originated from outside of Qualcomm. Please be 
>>> wary
>>> of any links or attachments, and do not enable macros.
>>> 
>>> On 2021-09-30 20:50, Johannes Berg wrote:
>>> > On Thu, 2021-09-30 at 10:53 +0800, Wen Gong wrote:
>>> >> > >
>>> >> > >          chan->max_reg_power =
>>> >> > >                  min_t(int, MBM_TO_DBM(power_rule1->max_eirp),
>>> >> > >                        MBM_TO_DBM(power_rule2->max_eirp));
>>> >> > >
>>> >> > > For AP + STA concurrency, it should to maintain 2 group of reg
>>> >> > > rules, one is for AP, another is for STA.
>>> >> >
>>> >> > Can we maintain two power rules in the same channel one for AP and
>>> >> > one for STA. In this way, we can update the power rules in the same
>>> >> > channel for both AP and STA from the reg rules.
>>> >> >
>>> >> > Otherwise, we need to maintain multiple channel lists in sband for
>>> >> > all supported power mode combinations to apply the respective power
>>> >> > rules and build channel flags from the multiple reg rules.
>>> >> > right?
>>> >>
>>> >> If AP+STA is up in the same wiphy/ieee80211_hw, and AP's reg rules is
>>> >> different with STA, then it should maintain muti channel list for
>>> >> each band of the wiphy/ieee80211_hw by my understand.
>>> >
>>> > I don't think that's how it works. You can today have AP/STA
>>> > concurrency on a single wiphy with different netdevs, even with mesh
>>> > or whatever.
>>> >
>>> >> Currently there is only one "struct ieee80211_supported_band
>>> >> *bands[NUM_NL80211_BANDS]"
>>> >> in "struct wiphy".
>>> >>
>>> >> I advise to discuss the AP + STA concurrency in another mail thread
>>> >> since it is not relative with this patch.
>>> >
>>> > I actually explicitly pointed to this thread, but I'm not sure it's so
>>> > clear cut?
>>> >
>>> > If we have completely separate rules here for AP and STA, we probably
>>> > should have different "max_reg_power" values for AP and STA? Maybe
>>> > mesh is treated like AP, maybe not?
>>> >
>>> > But I don't know - does PSD really differ between AP and STA?
>>> >
>>> > Maybe this discussion belongs rather to the power type patch? But that
>>> > didn't add any state!
>>> >
>>> >
>>> > So - does this PSD depend on mode? It kind of seems like it shouldn't
>>> > and then this *isn't* the right place to be discussing this, but if
>>> > PSD does in fact depend on the mode then we should be discussing it
>>> here?
>>> >
>>> > Venkatesh seemed to be worried more about LPI/client power etc. as in
>>> > commit 405fca8a9461 ("ieee80211: add power type definition for 6
>>> > GHz"), but that doesn't add state?
>>> >
>>> > So what gives? From a regulatory POV it seems PSD should be
>>> > independent, but some other things might be dependent on mode?
>>> >
>>> 
>>> As I know, below values maybe all different for the AP and
>>> STATION in the same wiphy/ieee80211_hw, not only PSD.
>>> 
>>> struct ieee80211_reg_rule {
>>>         struct ieee80211_freq_range freq_range;
>>>         struct ieee80211_power_rule power_rule;
>>>         struct ieee80211_wmm_rule wmm_rule;
>>>         u32 flags;
>>>         u32 dfs_cac_ms;
>>>         bool has_wmm;
>>>         s8 psd;
>>> };
>> IMO, Only power rules and PSD info might vary for AP and STATION. Rest
>> of the rules will remains same right?
>> 
> The freq_range may also be different for AP and STATION.
> and reg_rules number also may also be different for AP and STATION.
> 
> for example:
> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
> dB/MHz)
> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
> dB/MHz)
> 
> INDOOR AP reg rules number 1
> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0 
> dB/MHz)
> 
>>> 
>>> @Venkateswara, please feel free to give more info to Johannes:)
>>> 
>>> > johannes
>>> 
>>> --
>>> ath11k mailing list
>>> ath11k@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/ath11k
Johannes Berg Oct. 25, 2021, 8:09 p.m. UTC | #10
On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
> 
> > IMO, Only power rules and PSD info might vary for AP and STATION. Rest
> > of the rules will remains same right?
> > 
> The freq_range may also be different for AP and STATION.
> and reg_rules number also may also be different for AP and STATION.
> 
> for example:
> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
> dB/MHz)
> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17 
> dB/MHz)
> 
> INDOOR AP reg rules number 1
> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0 
> dB/MHz)

That seems right, but isn't that an orthogonal question?

Here, on this patch, we're discussing what data we should have in the
channel information, and it would seem that if it's different for
AP/client, then we do need both information stored, so that we can cope
with concurrency between AP and client?

If we additionally need to have different data for the regulatory rules
for AP and client, that might mean we need to go back and actually
change the code there *as well*, and then fill in the right fields in
this patch?

Unless somehow we're convinced that for this feature we don't need to
worry about concurrently using AP and client modes?

johannes
Wen Gong Oct. 26, 2021, 11:26 a.m. UTC | #11
On 2021-10-26 04:09, Johannes Berg wrote:
> On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
>> 
>> > IMO, Only power rules and PSD info might vary for AP and STATION. Rest
>> > of the rules will remains same right?
>> >
>> The freq_range may also be different for AP and STATION.
>> and reg_rules number also may also be different for AP and STATION.
>> 
>> for example:
>> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
>> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17
>> dB/MHz)
>> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17
>> dB/MHz)
>> 
>> INDOOR AP reg rules number 1
>> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0
>> dB/MHz)
> 
> That seems right, but isn't that an orthogonal question?
> 
> Here, on this patch, we're discussing what data we should have in the
> channel information, and it would seem that if it's different for
> AP/client, then we do need both information stored, so that we can cope
> with concurrency between AP and client?
> 
> If we additionally need to have different data for the regulatory rules
> for AP and client, that might mean we need to go back and actually
> change the code there *as well*, and then fill in the right fields in
> this patch?
> 
> Unless somehow we're convinced that for this feature we don't need to
> worry about concurrently using AP and client modes?
> 
> johannes

Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP is
the base patches, to enable the feature of LPI/SP/VLP, it still need 
other
patches of lower drivers such as ath11k to enable it. It will not have
LPI/SP/VLP without patches of ath11k, it means all these patches will
not take effect.

When lower driver such as ath11k set max_interfaces of 
ieee80211_iface_combination
to 1, then it can not start more than 1 interface on the same 
ieee80211_hw/wiphy.
When STATION interface is up, then AP interface can not start up. AP 
interface
can start up after STATION interfacedown. Also when AP interface is up,
STATION interface can not start up. STATION interface can start up after
AP interface down.

I have sent out my ath11k 
patches(https://lore.kernel.org/linux-wireless/20211026111913.7346-1-quic_wgong@quicinc.com/),
it will allow only one interface
up simultaneously for the chip which enable LPI/SP/VLP feature in this
patch: "ath11k: allow only one interface up simultaneously for WCN6855"
https://lore.kernel.org/linux-wireless/20211026111913.7346-5-quic_wgong@quicinc.com/
It means it will not have both AP/STA together and these patches of 
mac80211/
cfg80211/ieee80211 not need changes and it will not have bugs.

If there are some chip want to both enable LPI/SP/VLP feature and
enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
then he/she need to refine reg rules and channels of mac80211/cfg80211/
ieee80211, but at that moment, this patch "cfg80211: save power
spectral density(psd) of regulatory rule" still not need change.
Because this patch is change in each reg rule/each channel in a
low layer, the refine reg rules and channels is a high layer, they
have no intersection.
Wen Gong Nov. 9, 2021, 9:57 a.m. UTC | #12
Hi Johannes,

do you have comments about my description for PSD?

On 2021-10-26 19:26, Wen Gong wrote:
> On 2021-10-26 04:09, Johannes Berg wrote:
>> On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
>>> 
>>> > IMO, Only power rules and PSD info might vary for AP and STATION. Rest
>>> > of the rules will remains same right?
>>> >
>>> The freq_range may also be different for AP and STATION.
>>> and reg_rules number also may also be different for AP and STATION.
>>> 
>>> for example:
>>> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
>>> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17
>>> dB/MHz)
>>> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 17
>>> dB/MHz)
>>> 
>>> INDOOR AP reg rules number 1
>>> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0
>>> dB/MHz)
>> 
>> That seems right, but isn't that an orthogonal question?
>> 
>> Here, on this patch, we're discussing what data we should have in the
>> channel information, and it would seem that if it's different for
>> AP/client, then we do need both information stored, so that we can 
>> cope
>> with concurrency between AP and client?
>> 
>> If we additionally need to have different data for the regulatory 
>> rules
>> for AP and client, that might mean we need to go back and actually
>> change the code there *as well*, and then fill in the right fields in
>> this patch?
>> 
>> Unless somehow we're convinced that for this feature we don't need to
>> worry about concurrently using AP and client modes?
>> 
>> johannes
> 
> Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP 
> is
> the base patches, to enable the feature of LPI/SP/VLP, it still need 
> other
> patches of lower drivers such as ath11k to enable it. It will not have
> LPI/SP/VLP without patches of ath11k, it means all these patches will
> not take effect.
> 
> When lower driver such as ath11k set max_interfaces of
> ieee80211_iface_combination
> to 1, then it can not start more than 1 interface on the same
> ieee80211_hw/wiphy.
> When STATION interface is up, then AP interface can not start up. AP 
> interface
> can start up after STATION interfacedown. Also when AP interface is up,
> STATION interface can not start up. STATION interface can start up 
> after
> AP interface down.
> 
> I have sent out my ath11k
> patches(https://lore.kernel.org/linux-wireless/20211026111913.7346-1-quic_wgong@quicinc.com/),
> it will allow only one interface
> up simultaneously for the chip which enable LPI/SP/VLP feature in this
> patch: "ath11k: allow only one interface up simultaneously for WCN6855"
> https://lore.kernel.org/linux-wireless/20211026111913.7346-5-quic_wgong@quicinc.com/
> It means it will not have both AP/STA together and these patches of 
> mac80211/
> cfg80211/ieee80211 not need changes and it will not have bugs.
> 
> If there are some chip want to both enable LPI/SP/VLP feature and
> enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
> then he/she need to refine reg rules and channels of mac80211/cfg80211/
> ieee80211, but at that moment, this patch "cfg80211: save power
> spectral density(psd) of regulatory rule" still not need change.
> Because this patch is change in each reg rule/each channel in a
> low layer, the refine reg rules and channels is a high layer, they
> have no intersection.
Wen Gong Dec. 6, 2021, 8:48 a.m. UTC | #13
Hi Johannes,

Are you waiting for the AP/STA concurrency patches, then apply this 
patch?

On 2021-11-09 17:57, Wen Gong wrote:
> Hi Johannes,
> 
> do you have comments about my description for PSD?
> 
> On 2021-10-26 19:26, Wen Gong wrote:
>> On 2021-10-26 04:09, Johannes Berg wrote:
>>> On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
>>>> 
>>>> > IMO, Only power rules and PSD info might vary for AP and STATION. Rest
>>>> > of the rules will remains same right?
>>>> >
>>>> The freq_range may also be different for AP and STATION.
>>>> and reg_rules number also may also be different for AP and STATION.
>>>> 
>>>> for example:
>>>> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
>>>> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 
>>>> 17
>>>> dB/MHz)
>>>> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 
>>>> 17
>>>> dB/MHz)
>>>> 
>>>> INDOOR AP reg rules number 1
>>>> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0
>>>> dB/MHz)
>>> 
>>> That seems right, but isn't that an orthogonal question?
>>> 
>>> Here, on this patch, we're discussing what data we should have in the
>>> channel information, and it would seem that if it's different for
>>> AP/client, then we do need both information stored, so that we can 
>>> cope
>>> with concurrency between AP and client?
>>> 
>>> If we additionally need to have different data for the regulatory 
>>> rules
>>> for AP and client, that might mean we need to go back and actually
>>> change the code there *as well*, and then fill in the right fields in
>>> this patch?
>>> 
>>> Unless somehow we're convinced that for this feature we don't need to
>>> worry about concurrently using AP and client modes?
>>> 
>>> johannes
>> 
>> Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP 
>> is
>> the base patches, to enable the feature of LPI/SP/VLP, it still need 
>> other
>> patches of lower drivers such as ath11k to enable it. It will not have
>> LPI/SP/VLP without patches of ath11k, it means all these patches will
>> not take effect.
>> 
>> When lower driver such as ath11k set max_interfaces of
>> ieee80211_iface_combination
>> to 1, then it can not start more than 1 interface on the same
>> ieee80211_hw/wiphy.
>> When STATION interface is up, then AP interface can not start up. AP 
>> interface
>> can start up after STATION interfacedown. Also when AP interface is 
>> up,
>> STATION interface can not start up. STATION interface can start up 
>> after
>> AP interface down.
>> 
>> I have sent out my ath11k
>> patches(https://lore.kernel.org/linux-wireless/20211026111913.7346-1-quic_wgong@quicinc.com/),
>> it will allow only one interface
>> up simultaneously for the chip which enable LPI/SP/VLP feature in this
>> patch: "ath11k: allow only one interface up simultaneously for 
>> WCN6855"
>> https://lore.kernel.org/linux-wireless/20211026111913.7346-5-quic_wgong@quicinc.com/
>> It means it will not have both AP/STA together and these patches of 
>> mac80211/
>> cfg80211/ieee80211 not need changes and it will not have bugs.
>> 
>> If there are some chip want to both enable LPI/SP/VLP feature and
>> enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
>> then he/she need to refine reg rules and channels of 
>> mac80211/cfg80211/
>> ieee80211, but at that moment, this patch "cfg80211: save power
>> spectral density(psd) of regulatory rule" still not need change.
>> Because this patch is change in each reg rule/each channel in a
>> low layer, the refine reg rules and channels is a high layer, they
>> have no intersection.
Wen Gong March 3, 2022, 2:13 a.m. UTC | #14
Hi Johannes,

Do you have comments for this patch?

Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP is
the base patches, to enable the feature of LPI/SP/VLP, it still need 
other
patches of lower drivers such as ath11k to enable it. It will not have
LPI/SP/VLP without patches of ath11k, it means all these patches of
mac80211/cfg80211 will not take effect.

When lower driver such as ath11k set max_interfaces of 
ieee80211_iface_combination
to 1, then it can not start more than 1 interface on the same 
ieee80211_hw/wiphy.
When STATION interface is up, then AP interface can not start up. AP 
interface
can start up after STATION interface down. Also when AP interface is up,
STATION interface can not start up. STATION interface can start up after
AP interface down.

I have sent out my ath11k v2 
patches(https://patchwork.kernel.org/project/linux-wireless/list/?series=601212),
it will allow only one interface
up simultaneously for the chip which enable LPI/SP/VLP feature in this
patch: "ath11k: allow only one interface up simultaneously for WCN6855"
https://patchwork.kernel.org/project/linux-wireless/patch/20211224085236.9064-5-quic_wgong@quicinc.com/
It means it will not have both AP/STA together and these patches of 
mac80211/
cfg80211/ieee80211 not need changes and it will not have bugs.

If there are some chip want to both enable LPI/SP/VLP feature and
enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
then he/she need to refine reg rules and channels of mac80211/cfg80211/
ieee80211, but at that moment, this patch "cfg80211: save power
spectral density(psd) of regulatory rule" still not need change.
Because this patch is change in each reg rule/each channel in a
low layer, the refine reg rules and channels is a high layer, they
have no intersection.

On 2021-12-06 16:48, Wen Gong wrote:
> Hi Johannes,
> 
> Are you waiting for the AP/STA concurrency patches, then apply this 
> patch?
> 
> On 2021-11-09 17:57, Wen Gong wrote:
>> Hi Johannes,
>> 
>> do you have comments about my description for PSD?
>> 
>> On 2021-10-26 19:26, Wen Gong wrote:
>>> On 2021-10-26 04:09, Johannes Berg wrote:
>>>> On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
>>>>> 
>>>>> > IMO, Only power rules and PSD info might vary for AP and STATION. Rest
>>>>> > of the rules will remains same right?
>>>>> >
>>>>> The freq_range may also be different for AP and STATION.
>>>>> and reg_rules number also may also be different for AP and STATION.
>>>>> 
>>>>> for example:
>>>>> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
>>>>> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 
>>>>> 17
>>>>> dB/MHz)
>>>>> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 EIRP 
>>>>> 17
>>>>> dB/MHz)
>>>>> 
>>>>> INDOOR AP reg rules number 1
>>>>> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 
>>>>> 0
>>>>> dB/MHz)
>>>> 
>>>> That seems right, but isn't that an orthogonal question?
>>>> 
>>>> Here, on this patch, we're discussing what data we should have in 
>>>> the
>>>> channel information, and it would seem that if it's different for
>>>> AP/client, then we do need both information stored, so that we can 
>>>> cope
>>>> with concurrency between AP and client?
>>>> 
>>>> If we additionally need to have different data for the regulatory 
>>>> rules
>>>> for AP and client, that might mean we need to go back and actually
>>>> change the code there *as well*, and then fill in the right fields 
>>>> in
>>>> this patch?
>>>> 
>>>> Unless somehow we're convinced that for this feature we don't need 
>>>> to
>>>> worry about concurrently using AP and client modes?
>>>> 
>>>> johannes
>>> 
>>> Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP 
>>> is
>>> the base patches, to enable the feature of LPI/SP/VLP, it still need 
>>> other
>>> patches of lower drivers such as ath11k to enable it. It will not 
>>> have
>>> LPI/SP/VLP without patches of ath11k, it means all these patches will
>>> not take effect.
>>> 
>>> When lower driver such as ath11k set max_interfaces of
>>> ieee80211_iface_combination
>>> to 1, then it can not start more than 1 interface on the same
>>> ieee80211_hw/wiphy.
>>> When STATION interface is up, then AP interface can not start up. AP 
>>> interface
>>> can start up after STATION interfacedown. Also when AP interface is 
>>> up,
>>> STATION interface can not start up. STATION interface can start up 
>>> after
>>> AP interface down.
>>> 
>>> I have sent out my ath11k
>>> patches(https://lore.kernel.org/linux-wireless/20211026111913.7346-1-quic_wgong@quicinc.com/),
>>> it will allow only one interface
>>> up simultaneously for the chip which enable LPI/SP/VLP feature in 
>>> this
>>> patch: "ath11k: allow only one interface up simultaneously for 
>>> WCN6855"
>>> https://lore.kernel.org/linux-wireless/20211026111913.7346-5-quic_wgong@quicinc.com/
>>> It means it will not have both AP/STA together and these patches of 
>>> mac80211/
>>> cfg80211/ieee80211 not need changes and it will not have bugs.
>>> 
>>> If there are some chip want to both enable LPI/SP/VLP feature and
>>> enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
>>> then he/she need to refine reg rules and channels of 
>>> mac80211/cfg80211/
>>> ieee80211, but at that moment, this patch "cfg80211: save power
>>> spectral density(psd) of regulatory rule" still not need change.
>>> Because this patch is change in each reg rule/each channel in a
>>> low layer, the refine reg rules and channels is a high layer, they
>>> have no intersection.
Wen Gong April 15, 2022, 2:27 a.m. UTC | #15
Hi Johannes,

PSD is another type power value as well as the power_rule in 
ieee80211_reg_rule
and max_reg_power/max_power in ieee80211_channel. For 6G, it need the
both the 2 values (psd and tx-power) for power control.

To support LPI/SP/VLP mode + concurrency, it is not only to process the
TX power and psd, it need to process all fields in ieee80211_reg_rule.
for example, for the same country US, it has some ieee80211_reg_rule for
6G:
LPI mode for AP mode is:
(5945 - 7125 @ 160) (0, 30) (FLAGS 512) (psd 5 dB/MHz)

LPI mode for client mode is:
(5945 - 7125 @ 160) (0, 24) (FLAGS 512) (psd -1 dB/MHz)

SP mode is:
(5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)
(6525 - 6875 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)

you can see the frequency range/flags/tx power/psd are not same between
the 3 mode. So the psd value is a same kind field as well as the frequency
range/flags/tx power in ieee80211_reg_rule and ieee80211_channel.
If it has only one interface in the same ieee80211_hw, the interface
must choose one the the above modes at any time, and the reg rules
will be updated when the interface changed its mode, then the info
of channels of ieee80211_supported_band for 6G also update, it has
no problem.

When it has 2 interface up simultaneously in the same ieee80211_hw,
then it will have problem. because the 2 interface share the same
ieee80211_reg_rule and the same channels and same ieee80211_supported_band
for 6G, if interface #1 is LPI-AP, and interface #2 is SP mode, then
even the channels count is not same for the 2 interface, channels count
of SP mode is smaller than LPI-AP, also the info of channel is not same.

So adding/not adding psd value into ieee80211_reg_rule/ieee80211_channel
will not effect the problem of muti-interface concurrency issue of 6G.
When muti-interface concurrency run, all the fields of ieee80211_reg_rule/
ieee80211_channel should be split.

I think the easy way is to save the channels of ieee80211_supported_band
of 6G in each interface instead of ieee80211_hw, then interface #1/#2
have/use their own channels, they use their own frequency range/flags/
tx power/psd... This is a high level change, and adding psd into
ieee80211_reg_rule/ieee80211_channel is a low level change. So I think
they have no dependency with each other.

On 3/3/2022 10:13 AM, Wen Gong wrote:
> Hi Johannes,
>
> Do you have comments for this patch?
>
> Currently these patches of mac80211/cfg80211/ieee80211 for LPI/SP/VLP is
> the base patches, to enable the feature of LPI/SP/VLP, it still need 
> other
> patches of lower drivers such as ath11k to enable it. It will not have
> LPI/SP/VLP without patches of ath11k, it means all these patches of
> mac80211/cfg80211 will not take effect.
>
> When lower driver such as ath11k set max_interfaces of 
> ieee80211_iface_combination
> to 1, then it can not start more than 1 interface on the same 
> ieee80211_hw/wiphy.
> When STATION interface is up, then AP interface can not start up. AP 
> interface
> can start up after STATION interface down. Also when AP interface is up,
> STATION interface can not start up. STATION interface can start up after
> AP interface down.
>
> I have sent out my ath11k v2 
> patches(https://patchwork.kernel.org/project/linux-wireless/list/?series=601212),
> it will allow only one interface
> up simultaneously for the chip which enable LPI/SP/VLP feature in this
> patch: "ath11k: allow only one interface up simultaneously for WCN6855"
> https://patchwork.kernel.org/project/linux-wireless/patch/20211224085236.9064-5-quic_wgong@quicinc.com/ 
>
> It means it will not have both AP/STA together and these patches of 
> mac80211/
> cfg80211/ieee80211 not need changes and it will not have bugs.
>
> If there are some chip want to both enable LPI/SP/VLP feature and
> enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
> then he/she need to refine reg rules and channels of mac80211/cfg80211/
> ieee80211, but at that moment, this patch "cfg80211: save power
> spectral density(psd) of regulatory rule" still not need change.
> Because this patch is change in each reg rule/each channel in a
> low layer, the refine reg rules and channels is a high layer, they
> have no intersection.
>
> On 2021-12-06 16:48, Wen Gong wrote:
>> Hi Johannes,
>>
>> Are you waiting for the AP/STA concurrency patches, then apply this 
>> patch?
>>
>> On 2021-11-09 17:57, Wen Gong wrote:
>>> Hi Johannes,
>>>
>>> do you have comments about my description for PSD?
>>>
>>> On 2021-10-26 19:26, Wen Gong wrote:
>>>> On 2021-10-26 04:09, Johannes Berg wrote:
>>>>> On Mon, 2021-10-11 at 15:48 +0800, Wen Gong wrote:
>>>>>>
>>>>>> > IMO, Only power rules and PSD info might vary for AP and 
>>>>>> STATION. Rest
>>>>>> > of the rules will remains same right?
>>>>>> >
>>>>>> The freq_range may also be different for AP and STATION.
>>>>>> and reg_rules number also may also be different for AP and STATION.
>>>>>>
>>>>>> for example:
>>>>>> SUBORDINATE CLIENT of STANDARD POWER reg rules number 2
>>>>>> reg rule 1: (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd flag 1 
>>>>>> EIRP 17
>>>>>> dB/MHz)
>>>>>> reg rule 2: (6525 - 6885 @ 160) (0, 30) (FLAGS 0) (psd flag 1 
>>>>>> EIRP 17
>>>>>> dB/MHz)
>>>>>>
>>>>>> INDOOR AP reg rules number 1
>>>>>> reg rule 1: (5945 - 7125 @ 160) (0, 24) (FLAGS 0) (psd flag 0 EIRP 0
>>>>>> dB/MHz)
>>>>>
>>>>> That seems right, but isn't that an orthogonal question?
>>>>>
>>>>> Here, on this patch, we're discussing what data we should have in the
>>>>> channel information, and it would seem that if it's different for
>>>>> AP/client, then we do need both information stored, so that we can 
>>>>> cope
>>>>> with concurrency between AP and client?
>>>>>
>>>>> If we additionally need to have different data for the regulatory 
>>>>> rules
>>>>> for AP and client, that might mean we need to go back and actually
>>>>> change the code there *as well*, and then fill in the right fields in
>>>>> this patch?
>>>>>
>>>>> Unless somehow we're convinced that for this feature we don't need to
>>>>> worry about concurrently using AP and client modes?
>>>>>
>>>>> johannes
>>>>
>>>> Currently these patches of mac80211/cfg80211/ieee80211 for 
>>>> LPI/SP/VLP is
>>>> the base patches, to enable the feature of LPI/SP/VLP, it still 
>>>> need other
>>>> patches of lower drivers such as ath11k to enable it. It will not have
>>>> LPI/SP/VLP without patches of ath11k, it means all these patches will
>>>> not take effect.
>>>>
>>>> When lower driver such as ath11k set max_interfaces of
>>>> ieee80211_iface_combination
>>>> to 1, then it can not start more than 1 interface on the same
>>>> ieee80211_hw/wiphy.
>>>> When STATION interface is up, then AP interface can not start up. 
>>>> AP interface
>>>> can start up after STATION interfacedown. Also when AP interface is 
>>>> up,
>>>> STATION interface can not start up. STATION interface can start up 
>>>> after
>>>> AP interface down.
>>>>
>>>> I have sent out my ath11k
>>>> patches(https://lore.kernel.org/linux-wireless/20211026111913.7346-1-quic_wgong@quicinc.com/), 
>>>>
>>>> it will allow only one interface
>>>> up simultaneously for the chip which enable LPI/SP/VLP feature in this
>>>> patch: "ath11k: allow only one interface up simultaneously for 
>>>> WCN6855"
>>>> https://lore.kernel.org/linux-wireless/20211026111913.7346-5-quic_wgong@quicinc.com/ 
>>>>
>>>> It means it will not have both AP/STA together and these patches of 
>>>> mac80211/
>>>> cfg80211/ieee80211 not need changes and it will not have bugs.
>>>>
>>>> If there are some chip want to both enable LPI/SP/VLP feature and
>>>> enable AP/STA simultaneously in same ieee80211_hw/wiphy in future,
>>>> then he/she need to refine reg rules and channels of 
>>>> mac80211/cfg80211/
>>>> ieee80211, but at that moment, this patch "cfg80211: save power
>>>> spectral density(psd) of regulatory rule" still not need change.
>>>> Because this patch is change in each reg rule/each channel in a
>>>> low layer, the refine reg rules and channels is a high layer, they
>>>> have no intersection.
>
Johannes Berg May 4, 2022, noon UTC | #16
Hi,

Sorry. I've been shying away from this, it's completely confusing me,
and the code isn't really helping that much, so far.

> PSD is another type power value as well as the power_rule in 
> ieee80211_reg_rule
> and max_reg_power/max_power in ieee80211_channel. For 6G, it need the
> both the 2 values (psd and tx-power) for power control.

Sure, that makes sense. You have PSD limits as well as TX power limits.

> To support LPI/SP/VLP mode + concurrency, it is not only to process the
> TX power and psd, it need to process all fields in ieee80211_reg_rule.
> for example, for the same country US, it has some ieee80211_reg_rule for
> 6G:
> LPI mode for AP mode is:
> (5945 - 7125 @ 160) (0, 30) (FLAGS 512) (psd 5 dB/MHz)
> 
> LPI mode for client mode is:
> (5945 - 7125 @ 160) (0, 24) (FLAGS 512) (psd -1 dB/MHz)
> 
> SP mode is:
> (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)
> (6525 - 6875 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)
> 
> you can see the frequency range/flags/tx power/psd are not same between
> the 3 mode.

Right. But here I already don't understand - does that mean your
hardware can only be in one mode today overall? 

It seems you'd need to have separate regulatory rules per mode, and
possibly per interface type, no?

Or do you try to adjust the regulatory rules when the mode is switched?
But we try to have regulatory rules capture the overall status, not a
temporary status, and then apply the rules that make sense at any given
time.

> So the psd value is a same kind field as well as the frequency
> range/flags/tx power in ieee80211_reg_rule and ieee80211_channel.
> If it has only one interface in the same ieee80211_hw, the interface
> must choose one the the above modes at any time, and the reg rules
> will be updated when the interface changed its mode, then the info
> of channels of ieee80211_supported_band for 6G also update, it has
> no problem.

Ah OK, so you answer that question - you're actually changing the
regulatory rules, which is totally unexpected to me, since I'd expect
those to only change when the regulatory changes, not when you mode
switch ...

> When it has 2 interface up simultaneously in the same ieee80211_hw,
> then it will have problem. because the 2 interface share the same
> ieee80211_reg_rule and the same channels and same ieee80211_supported_band
> for 6G, if interface #1 is LPI-AP, and interface #2 is SP mode, then
> even the channels count is not same for the 2 interface, channels count
> of SP mode is smaller than LPI-AP, also the info of channel is not same.
> 
> So adding/not adding psd value into ieee80211_reg_rule/ieee80211_channel
> will not effect the problem of muti-interface concurrency issue of 6G.

Oh but I disagree - adding it will mean that this is how we interpret
the reg_rule, that it only carries the current status according to "the"
current mode (which isn't even well-defined), rather than the entirety
of the regulatory information.

> When muti-interface concurrency run, all the fields of ieee80211_reg_rule/
> ieee80211_channel should be split.

Right.

> I think the easy way is to save the channels of ieee80211_supported_band
> of 6G in each interface instead of ieee80211_hw, then interface #1/#2
> have/use their own channels, they use their own frequency range/flags/
> tx power/psd... 
> 

I'm not sure that will work, because we often compare chan pointers with
==, and if you're doing something like remain-on-channel that's still
relevant. So having the same channel struct on the same hardware with
the same frequency is bound to be problematic.

> This is a high level change, and adding psd into
> ieee80211_reg_rule/ieee80211_channel is a low level change. So I think
> they have no dependency with each other.

I think I disagree, per the above disagreement.

johannes
Wen Gong May 6, 2022, 10:50 a.m. UTC | #17
On 5/4/2022 8:00 PM, Johannes Berg wrote:
> Hi,
>
> Sorry. I've been shying away from this, it's completely confusing me,
> and the code isn't really helping that much, so far.
>
>> PSD is another type power value as well as the power_rule in
>> ieee80211_reg_rule
>> and max_reg_power/max_power in ieee80211_channel. For 6G, it need the
>> both the 2 values (psd and tx-power) for power control.
> Sure, that makes sense. You have PSD limits as well as TX power limits.
>
>> To support LPI/SP/VLP mode + concurrency, it is not only to process the
>> TX power and psd, it need to process all fields in ieee80211_reg_rule.
>> for example, for the same country US, it has some ieee80211_reg_rule for
>> 6G:
>> LPI mode for AP mode is:
>> (5945 - 7125 @ 160) (0, 30) (FLAGS 512) (psd 5 dB/MHz)
>>
>> LPI mode for client mode is:
>> (5945 - 7125 @ 160) (0, 24) (FLAGS 512) (psd -1 dB/MHz)
>>
>> SP mode is:
>> (5945 - 6425 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)
>> (6525 - 6875 @ 160) (0, 30) (FLAGS 0) (psd 17 dB/MHz)
>>
>> you can see the frequency range/flags/tx power/psd are not same between
>> the 3 mode.
> Right. But here I already don't understand - does that mean your
> hardware can only be in one mode today overall?
>
> It seems you'd need to have separate regulatory rules per mode, and
> possibly per interface type, no?
>
> Or do you try to adjust the regulatory rules when the mode is switched?
> But we try to have regulatory rules capture the overall status, not a
> temporary status, and then apply the rules that make sense at any given
> time.
Yes, we adjust the regulatory rules when the mode is switched.
>
>> So the psd value is a same kind field as well as the frequency
>> range/flags/tx power in ieee80211_reg_rule and ieee80211_channel.
>> If it has only one interface in the same ieee80211_hw, the interface
>> must choose one the the above modes at any time, and the reg rules
>> will be updated when the interface changed its mode, then the info
>> of channels of ieee80211_supported_band for 6G also update, it has
>> no problem.
> Ah OK, so you answer that question - you're actually changing the
> regulatory rules, which is totally unexpected to me, since I'd expect
> those to only change when the regulatory changes, not when you mode
> switch ...
So I think you want to show all regulatory rules for all 
LPI/VLP/SP+AP/STA as below,
and it does not changed again untill regulatory/country code change, such
as "US" to "CN"/"KR", right?

for example self-managed of ath11k, it list all regulatory rules,
it has many regulatory rules for (5945 - 7125):

iw reg get
global
country US: DFS-FCC
         (2400 - 2472 @ 40), (N/A, 30), (N/A)
         (5150 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
         (5250 - 5350 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
         (5470 - 5730 @ 160), (N/A, 23), (0 ms), DFS
         (5730 - 5850 @ 80), (N/A, 30), (N/A)
         (57240 - 71000 @ 2160), (N/A, 40), (N/A)

phy#0 (self-managed)
country US: DFS-FCC
         (2402 - 2472 @ 40), (6, 30), (N/A)
         (5170 - 5250 @ 80), (N/A, 30), (N/A), AUTO-BW
         (5250 - 5330 @ 80), (N/A, 24), (0 ms), DFS, AUTO-BW
         (5490 - 5730 @ 160), (N/A, 24), (0 ms), DFS, AUTO-BW
         (5735 - 5895 @ 160), (N/A, 30), (N/A), AUTO-BW
         (5945 - 7125 @ 160), (N/A, 27), (N/A), NO-OUTDOOR, AUTO-BW
         (5945 - 7125 @ 160), (N/A, 21), (N/A), NO-OUTDOOR, AUTO-BW
         (5945 - 6425 @ 160), (N/A, 27), (N/A), AUTO-BW
         (6525 - 6865 @ 160), (N/A, 27), (N/A), AUTO-BW
         (5945 - 7125 @ 160), (N/A, 27), (N/A), NO-OUTDOOR, AUTO-BW
         (5945 - 6425 @ 160), (N/A, 27), (N/A), AUTO-BW
         (6525 - 6865 @ 160), (N/A, 27), (N/A), AUTO-BW

And currently my patch only show the regulatory rules of the power mode
it used now, and regulatory rules will changed when the power mode which
used change.

for example self-managed of ath11k:

global
country US: DFS-FCC
         (2400 - 2472 @ 40), (N/A, 30), (N/A)
         (5150 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
         (5250 - 5350 @ 80), (N/A, 23), (0 ms), DFS, AUTO-BW
         (5470 - 5730 @ 160), (N/A, 23), (0 ms), DFS
         (5730 - 5850 @ 80), (N/A, 30), (N/A)
         (57240 - 71000 @ 2160), (N/A, 40), (N/A)

phy#0 (self-managed)
country US: DFS-FCC
         (2402 - 2472 @ 40), (N/A, 20), (N/A)
         (2457 - 2472 @ 15), (N/A, 20), (N/A), PASSIVE-SCAN
         (5170 - 5250 @ 80), (N/A, 20), (N/A), AUTO-BW, PASSIVE-SCAN
         (5250 - 5330 @ 80), (N/A, 20), (0 ms), DFS, AUTO-BW, PASSIVE-SCAN
         (5490 - 5730 @ 160), (N/A, 20), (0 ms), DFS, AUTO-BW, PASSIVE-SCAN
         (5735 - 5835 @ 80), (N/A, 20), (N/A), AUTO-BW, PASSIVE-SCAN
         (2402 - 2472 @ 40), (N/A, 20), (N/A), AUTO-BW

>> When it has 2 interface up simultaneously in the same ieee80211_hw,
>> then it will have problem. because the 2 interface share the same
>> ieee80211_reg_rule and the same channels and same ieee80211_supported_band
>> for 6G, if interface #1 is LPI-AP, and interface #2 is SP mode, then
>> even the channels count is not same for the 2 interface, channels count
>> of SP mode is smaller than LPI-AP, also the info of channel is not same.
>>
>> So adding/not adding psd value into ieee80211_reg_rule/ieee80211_channel
>> will not effect the problem of muti-interface concurrency issue of 6G.
> Oh but I disagree - adding it will mean that this is how we interpret
> the reg_rule, that it only carries the current status according to "the"
> current mode (which isn't even well-defined), rather than the entirety
> of the regulatory information.
>
>> When muti-interface concurrency run, all the fields of ieee80211_reg_rule/
>> ieee80211_channel should be split.
> Right.
>
>> I think the easy way is to save the channels of ieee80211_supported_band
>> of 6G in each interface instead of ieee80211_hw, then interface #1/#2
>> have/use their own channels, they use their own frequency range/flags/
>> tx power/psd...
>>
> I'm not sure that will work, because we often compare chan pointers with
> ==, and if you're doing something like remain-on-channel that's still
> relevant. So having the same channel struct on the same hardware with
> the same frequency is bound to be problematic.
>
>> This is a high level change, and adding psd into
>> ieee80211_reg_rule/ieee80211_channel is a low level change. So I think
>> they have no dependency with each other.
> I think I disagree, per the above disagreement.
>
> johannes
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 62dd8422e0dc..f8e0cc19e0ce 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -109,6 +109,8 @@  struct wiphy;
  *	on this channel.
  * @IEEE80211_CHAN_16MHZ: 16 MHz bandwidth is permitted
  *	on this channel.
+ * @IEEE80211_CHAN_PSD: power spectral density (in dBm)
+ *	on this channel.
  *
  */
 enum ieee80211_channel_flags {
@@ -131,6 +133,7 @@  enum ieee80211_channel_flags {
 	IEEE80211_CHAN_4MHZ		= 1<<16,
 	IEEE80211_CHAN_8MHZ		= 1<<17,
 	IEEE80211_CHAN_16MHZ		= 1<<18,
+	IEEE80211_CHAN_PSD		= 1<<19,
 };
 
 #define IEEE80211_CHAN_NO_HT40 \
@@ -164,6 +167,7 @@  enum ieee80211_channel_flags {
  *	on this channel.
  * @dfs_state_entered: timestamp (jiffies) when the dfs state was entered.
  * @dfs_cac_ms: DFS CAC time in milliseconds, this is valid for DFS channels.
+ * @psd: power spectral density (in dBm)
  */
 struct ieee80211_channel {
 	enum nl80211_band band;
@@ -180,6 +184,7 @@  struct ieee80211_channel {
 	enum nl80211_dfs_state dfs_state;
 	unsigned long dfs_state_entered;
 	unsigned int dfs_cac_ms;
+	s8 psd;
 };
 
 /**
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 47f06f6f5a67..ed20004fb6a9 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -221,6 +221,7 @@  struct ieee80211_reg_rule {
 	u32 flags;
 	u32 dfs_cac_ms;
 	bool has_wmm;
+	s8 psd;
 };
 
 struct ieee80211_regdomain {
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c2efea98e060..ae5f69aa727b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3851,6 +3851,8 @@  enum nl80211_wmm_rule {
  *	on this channel in current regulatory domain.
  * @NL80211_FREQUENCY_ATTR_16MHZ: 16 MHz operation is allowed
  *	on this channel in current regulatory domain.
+ * @NL80211_FREQUENCY_ATTR_PSD: power spectral density (in dBm)
+ *	is allowed on this channel in current regulatory domain.
  * @NL80211_FREQUENCY_ATTR_MAX: highest frequency attribute number
  *	currently defined
  * @__NL80211_FREQUENCY_ATTR_AFTER_LAST: internal use
@@ -3887,6 +3889,7 @@  enum nl80211_frequency_attr {
 	NL80211_FREQUENCY_ATTR_4MHZ,
 	NL80211_FREQUENCY_ATTR_8MHZ,
 	NL80211_FREQUENCY_ATTR_16MHZ,
+	NL80211_FREQUENCY_ATTR_PSD,
 
 	/* keep last */
 	__NL80211_FREQUENCY_ATTR_AFTER_LAST,
@@ -3987,6 +3990,8 @@  enum nl80211_reg_type {
  * 	a given frequency range. The value is in mBm (100 * dBm).
  * @NL80211_ATTR_DFS_CAC_TIME: DFS CAC time in milliseconds.
  *	If not present or 0 default CAC time will be used.
+ * @NL80211_ATTR_POWER_RULE_PSD: power spectral density (in dBm).
+ *	This could be negative.
  * @NL80211_REG_RULE_ATTR_MAX: highest regulatory rule attribute number
  *	currently defined
  * @__NL80211_REG_RULE_ATTR_AFTER_LAST: internal use
@@ -4004,6 +4009,8 @@  enum nl80211_reg_rule_attr {
 
 	NL80211_ATTR_DFS_CAC_TIME,
 
+	NL80211_ATTR_POWER_RULE_PSD,
+
 	/* keep last */
 	__NL80211_REG_RULE_ATTR_AFTER_LAST,
 	NL80211_REG_RULE_ATTR_MAX = __NL80211_REG_RULE_ATTR_AFTER_LAST - 1
@@ -4085,6 +4092,7 @@  enum nl80211_sched_scan_match_attr {
  * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
  * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
  * @NL80211_RRF_NO_HE: HE operation not allowed
+ * @NL80211_RRF_PSD: channels has power spectral density value
  */
 enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_OFDM		= 1<<0,
@@ -4103,6 +4111,7 @@  enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_80MHZ		= 1<<15,
 	NL80211_RRF_NO_160MHZ		= 1<<16,
 	NL80211_RRF_NO_HE		= 1<<17,
+	NL80211_RRF_PSD			= 1<<18,
 };
 
 #define NL80211_RRF_PASSIVE_SCAN	NL80211_RRF_NO_IR
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0b4f29d689d2..9f1c5be11c95 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1055,6 +1055,10 @@  static int nl80211_msg_put_channel(struct sk_buff *msg, struct wiphy *wiphy,
 	if (nla_put_u32(msg, NL80211_FREQUENCY_ATTR_OFFSET, chan->freq_offset))
 		goto nla_put_failure;
 
+	if ((chan->flags & IEEE80211_CHAN_PSD) &&
+	    nla_put_s8(msg, NL80211_FREQUENCY_ATTR_PSD, chan->psd))
+		goto nla_put_failure;
+
 	if ((chan->flags & IEEE80211_CHAN_DISABLED) &&
 	    nla_put_flag(msg, NL80211_FREQUENCY_ATTR_DISABLED))
 		goto nla_put_failure;
@@ -7755,6 +7759,11 @@  static int nl80211_put_regdom(const struct ieee80211_regdomain *regdom,
 				reg_rule->dfs_cac_ms))
 			goto nla_put_failure;
 
+		if ((reg_rule->flags & NL80211_RRF_PSD) &&
+		    nla_put_s8(msg, NL80211_ATTR_POWER_RULE_PSD,
+			       reg_rule->psd))
+			goto nla_put_failure;
+
 		nla_nest_end(msg, nl_reg_rule);
 	}
 
@@ -7926,6 +7935,7 @@  static const struct nla_policy reg_rule_policy[NL80211_REG_RULE_ATTR_MAX + 1] =
 	[NL80211_ATTR_POWER_RULE_MAX_ANT_GAIN]	= { .type = NLA_U32 },
 	[NL80211_ATTR_POWER_RULE_MAX_EIRP]	= { .type = NLA_U32 },
 	[NL80211_ATTR_DFS_CAC_TIME]		= { .type = NLA_U32 },
+	[NL80211_ATTR_POWER_RULE_PSD]		= { .type = NLA_S8 },
 };
 
 static int parse_reg_rule(struct nlattr *tb[],
@@ -7947,6 +7957,14 @@  static int parse_reg_rule(struct nlattr *tb[],
 
 	reg_rule->flags = nla_get_u32(tb[NL80211_ATTR_REG_RULE_FLAGS]);
 
+	if (reg_rule->flags & NL80211_RRF_PSD) {
+		if (!tb[NL80211_ATTR_POWER_RULE_PSD])
+			return -EINVAL;
+
+		reg_rule->psd =
+			nla_get_s8(tb[NL80211_ATTR_POWER_RULE_PSD]);
+	}
+
 	freq_range->start_freq_khz =
 		nla_get_u32(tb[NL80211_ATTR_FREQ_RANGE_START]);
 	freq_range->end_freq_khz =
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;
 	return channel_flags;
 }
 
@@ -1794,6 +1796,9 @@  static void handle_channel_single_rule(struct wiphy *wiphy,
 				chan->dfs_cac_ms = reg_rule->dfs_cac_ms;
 		}
 
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = reg_rule->psd;
+
 		return;
 	}
 
@@ -1814,6 +1819,9 @@  static void handle_channel_single_rule(struct wiphy *wiphy,
 			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
 	}
 
+	if (chan->flags & IEEE80211_CHAN_PSD)
+		chan->psd = reg_rule->psd;
+
 	if (chan->orig_mpwr) {
 		/*
 		 * Devices that use REGULATORY_COUNTRY_IE_FOLLOW_POWER
@@ -1883,6 +1891,12 @@  static void handle_channel_adjacent_rules(struct wiphy *wiphy,
 							 rrule2->dfs_cac_ms);
 		}
 
+		if ((rrule1->flags & NL80211_RRF_PSD) &&
+		    (rrule2->flags & NL80211_RRF_PSD))
+			chan->psd = min_t(s8, rrule1->psd, rrule2->psd);
+		else
+			chan->flags &= ~NL80211_RRF_PSD;
+
 		return;
 	}
 
@@ -2540,6 +2554,9 @@  static void handle_channel_custom(struct wiphy *wiphy,
 			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
 	}
 
+	if (chan->flags & IEEE80211_CHAN_PSD)
+		chan->psd = reg_rule->psd;
+
 	chan->max_power = chan->max_reg_power;
 }