Message ID | 20230920082349.29111-6-quic_wgong@quicinc.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Series | wifi: ath11k: add support for 6 GHz station for various modes : LPI, SP and VLP | expand |
On 9/20/2023 1:23 AM, Wen Gong wrote: > When station connect to AP on 6 GHz band, it needs switch the regulatory > rules according to the regulatory info sub field in HE operation element. > Switch to the power type which AP used for station interface. > > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23 > > Signed-off-by: Wen Gong <quic_wgong@quicinc.com> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
On 9/20/23 13:53, Wen Gong wrote: > + if (ath11k_mac_supports_6ghz_cc_ext(ar) && > + ctx->def.chan->band == NL80211_BAND_6GHZ && > + arvif->vdev_type == WMI_VDEV_TYPE_STA) { > + reg_info = &ab->reg_info_store[ar->pdev_idx]; > + power_type = vif->bss_conf.power_type; > + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", > + power_type); > + if (power_type == IEEE80211_REG_UNSET_AP) > + power_type = IEEE80211_REG_LPI_AP; Why having _UNSET_ power type in vif->bss_conf.power_type is fine? Typically, during association, we would be setting this from the beacon/association response frame's HE 6 GHz Operation Info Field. So is it fine if the field is carrying some info which we don't know (or may be don't support it yet)? Why are we masking it with _LPI_ mode? Also, I see that currently mac80211 only sets LPI or SP. So let's say STA is trying to connect to VLP AP, mac80211 will set it as _UNSET_. And then here, we will be treating it as LPI AP. Is that fine?
On 9/22/2023 5:39 PM, Aditya Kumar Singh wrote: > On 9/20/23 13:53, Wen Gong wrote: > >> + if (ath11k_mac_supports_6ghz_cc_ext(ar) && >> + ctx->def.chan->band == NL80211_BAND_6GHZ && >> + arvif->vdev_type == WMI_VDEV_TYPE_STA) { >> + reg_info = &ab->reg_info_store[ar->pdev_idx]; >> + power_type = vif->bss_conf.power_type; >> + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", >> + power_type); >> + if (power_type == IEEE80211_REG_UNSET_AP) >> + power_type = IEEE80211_REG_LPI_AP; > Why having _UNSET_ power type in vif->bss_conf.power_type is fine? > Typically, during association, we would be setting this from the > beacon/association response frame's HE 6 GHz Operation Info Field. So > is it fine if the field is carrying some info which we don't know (or > may be don't support it yet)? Why are we masking it with _LPI_ mode? > > Also, I see that currently mac80211 only sets LPI or SP. So let's say > STA is trying to connect to VLP AP, mac80211 will set it as _UNSET_. > And then here, we will be treating it as LPI AP. Is that fine? Currently only SP/LPI are defined in "Table E-12—Regulatory Info subfield encoding in the United States" of IEEE Std 802.11ax™‐2021. So maybe I need to set power_type to VLP when it is UNSET here. > > >
On 9/22/23 15:32, Wen Gong wrote: > On 9/22/2023 5:39 PM, Aditya Kumar Singh wrote: >> On 9/20/23 13:53, Wen Gong wrote: >> >>> + if (ath11k_mac_supports_6ghz_cc_ext(ar) && >>> + ctx->def.chan->band == NL80211_BAND_6GHZ && >>> + arvif->vdev_type == WMI_VDEV_TYPE_STA) { >>> + reg_info = &ab->reg_info_store[ar->pdev_idx]; >>> + power_type = vif->bss_conf.power_type; >>> + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", >>> + power_type); >>> + if (power_type == IEEE80211_REG_UNSET_AP) >>> + power_type = IEEE80211_REG_LPI_AP; >> Why having _UNSET_ power type in vif->bss_conf.power_type is fine? >> Typically, during association, we would be setting this from the >> beacon/association response frame's HE 6 GHz Operation Info Field. So >> is it fine if the field is carrying some info which we don't know (or >> may be don't support it yet)? Why are we masking it with _LPI_ mode? >> >> Also, I see that currently mac80211 only sets LPI or SP. So let's say >> STA is trying to connect to VLP AP, mac80211 will set it as _UNSET_. >> And then here, we will be treating it as LPI AP. Is that fine? > > Currently only SP/LPI are defined in "Table E-12—Regulatory Info > subfield encoding in the United States" of IEEE Std 802.11ax™‐2021. > > So maybe I need to set power_type to VLP when it is UNSET here. Yeah, or may be deny association if we don't support that mode? I would let others comment on this.
On 9/22/2023 9:18 PM, Aditya Kumar Singh wrote: > On 9/22/23 15:32, Wen Gong wrote: >> On 9/22/2023 5:39 PM, Aditya Kumar Singh wrote: >>> On 9/20/23 13:53, Wen Gong wrote: >>> >>>> + if (ath11k_mac_supports_6ghz_cc_ext(ar) && >>>> + ctx->def.chan->band == NL80211_BAND_6GHZ && >>>> + arvif->vdev_type == WMI_VDEV_TYPE_STA) { >>>> + reg_info = &ab->reg_info_store[ar->pdev_idx]; >>>> + power_type = vif->bss_conf.power_type; >>>> + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", >>>> + power_type); >>>> + if (power_type == IEEE80211_REG_UNSET_AP) >>>> + power_type = IEEE80211_REG_LPI_AP; >>> Why having _UNSET_ power type in vif->bss_conf.power_type is fine? >>> Typically, during association, we would be setting this from the >>> beacon/association response frame's HE 6 GHz Operation Info Field. >>> So is it fine if the field is carrying some info which we don't know >>> (or may be don't support it yet)? Why are we masking it with _LPI_ >>> mode? >>> >>> Also, I see that currently mac80211 only sets LPI or SP. So let's >>> say STA is trying to connect to VLP AP, mac80211 will set it as >>> _UNSET_. And then here, we will be treating it as LPI AP. Is that fine? >> >> Currently only SP/LPI are defined in "Table E-12—Regulatory Info >> subfield encoding in the United States" of IEEE Std 802.11ax™‐2021. >> >> So maybe I need to set power_type to VLP when it is UNSET here. > Yeah, or may be deny association if we don't support that mode? I > would let others comment on this. Do you know how to check which mode support by ath11k?
On 9/25/23 16:13, Wen Gong wrote: > On 9/22/2023 9:18 PM, Aditya Kumar Singh wrote: >> On 9/22/23 15:32, Wen Gong wrote: >>> On 9/22/2023 5:39 PM, Aditya Kumar Singh wrote: >>>> On 9/20/23 13:53, Wen Gong wrote: >>>> >>>>> + if (ath11k_mac_supports_6ghz_cc_ext(ar) && >>>>> + ctx->def.chan->band == NL80211_BAND_6GHZ && >>>>> + arvif->vdev_type == WMI_VDEV_TYPE_STA) { >>>>> + reg_info = &ab->reg_info_store[ar->pdev_idx]; >>>>> + power_type = vif->bss_conf.power_type; >>>>> + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", >>>>> + power_type); >>>>> + if (power_type == IEEE80211_REG_UNSET_AP) >>>>> + power_type = IEEE80211_REG_LPI_AP; >>>> Why having _UNSET_ power type in vif->bss_conf.power_type is fine? >>>> Typically, during association, we would be setting this from the >>>> beacon/association response frame's HE 6 GHz Operation Info Field. >>>> So is it fine if the field is carrying some info which we don't know >>>> (or may be don't support it yet)? Why are we masking it with _LPI_ >>>> mode? >>>> >>>> Also, I see that currently mac80211 only sets LPI or SP. So let's >>>> say STA is trying to connect to VLP AP, mac80211 will set it as >>>> _UNSET_. And then here, we will be treating it as LPI AP. Is that fine? >>> >>> Currently only SP/LPI are defined in "Table E-12—Regulatory Info >>> subfield encoding in the United States" of IEEE Std 802.11ax™‐2021. >>> >>> So maybe I need to set power_type to VLP when it is UNSET here. >> Yeah, or may be deny association if we don't support that mode? I >> would let others comment on this. > Do you know how to check which mode support by ath11k? IMHO we should not maintain two different supported modes, one in driver and another in mac80211. I mean w.r.t mac80211 we should try to match it as far as possible. In this case, we can return error from mac80211 itself when a non-supported mode (as in VLP) is trying to associate? Since as you have quoted, VLP is not yet defined in spec IEEE Std 802.11ax‐2021 so why to mask the mode and continue in mac80211 itself? If we do like this, then mac80211 will not simply set _UNSET_ for 6 GHz client interface and in driver if we see its still _UNSET_ then we can be sure that something has gone wrong and we can take steps accordingly.
On 9/25/2023 6:52 PM, Aditya Kumar Singh wrote: > On 9/25/23 16:13, Wen Gong wrote: >> On 9/22/2023 9:18 PM, Aditya Kumar Singh wrote: >>> On 9/22/23 15:32, Wen Gong wrote: >>>> On 9/22/2023 5:39 PM, Aditya Kumar Singh wrote: >>>>> On 9/20/23 13:53, Wen Gong wrote: >>>>> >>>>>> + if (ath11k_mac_supports_6ghz_cc_ext(ar) && >>>>>> + ctx->def.chan->band == NL80211_BAND_6GHZ && >>>>>> + arvif->vdev_type == WMI_VDEV_TYPE_STA) { >>>>>> + reg_info = &ab->reg_info_store[ar->pdev_idx]; >>>>>> + power_type = vif->bss_conf.power_type; >>>>>> + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type >>>>>> %d\n", >>>>>> + power_type); >>>>>> + if (power_type == IEEE80211_REG_UNSET_AP) >>>>>> + power_type = IEEE80211_REG_LPI_AP; >>>>> Why having _UNSET_ power type in vif->bss_conf.power_type is fine? >>>>> Typically, during association, we would be setting this from the >>>>> beacon/association response frame's HE 6 GHz Operation Info Field. >>>>> So is it fine if the field is carrying some info which we don't >>>>> know (or may be don't support it yet)? Why are we masking it with >>>>> _LPI_ mode? >>>>> >>>>> Also, I see that currently mac80211 only sets LPI or SP. So let's >>>>> say STA is trying to connect to VLP AP, mac80211 will set it as >>>>> _UNSET_. And then here, we will be treating it as LPI AP. Is that >>>>> fine? >>>> >>>> Currently only SP/LPI are defined in "Table E-12—Regulatory Info >>>> subfield encoding in the United States" of IEEE Std 802.11ax™‐2021. >>>> >>>> So maybe I need to set power_type to VLP when it is UNSET here. >>> Yeah, or may be deny association if we don't support that mode? I >>> would let others comment on this. >> Do you know how to check which mode support by ath11k? > IMHO we should not maintain two different supported modes, one in > driver and another in mac80211. I mean w.r.t mac80211 we should try to > match it as far as possible. In this case, we can return error from > mac80211 itself when a non-supported mode (as in VLP) is trying to > associate? Since as you have quoted, VLP is not yet defined in spec > IEEE Std 802.11ax‐2021 so why to mask the mode and continue in > mac80211 itself? > > If we do like this, then mac80211 will not simply set _UNSET_ for 6 > GHz client interface and in driver if we see its still _UNSET_ then we > can be sure that something has gone wrong and we can take steps > accordingly. Yes, I will change to return fail for _UNSET_.
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 0512c8b0661c..bc4ae3002322 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -7613,6 +7613,8 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif); int ret; struct peer_create_params param; + struct cur_regulatory_info *reg_info; + enum ieee80211_ap_reg_power power_type; mutex_lock(&ar->conf_mutex); @@ -7620,6 +7622,18 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, "chanctx assign ptr %p vdev_id %i\n", ctx, arvif->vdev_id); + if (ath11k_mac_supports_6ghz_cc_ext(ar) && + ctx->def.chan->band == NL80211_BAND_6GHZ && + arvif->vdev_type == WMI_VDEV_TYPE_STA) { + reg_info = &ab->reg_info_store[ar->pdev_idx]; + power_type = vif->bss_conf.power_type; + ath11k_dbg(ab, ATH11K_DBG_MAC, "mac chanctx power type %d\n", + power_type); + if (power_type == IEEE80211_REG_UNSET_AP) + power_type = IEEE80211_REG_LPI_AP; + ath11k_reg_handle_chan_list(ab, reg_info, power_type); + } + /* for QCA6390 bss peer must be created before vdev_start */ if (ab->hw_params.vdev_start_delay && arvif->vdev_type != WMI_VDEV_TYPE_AP &&
When station connect to AP on 6 GHz band, it needs switch the regulatory rules according to the regulatory info sub field in HE operation element. Switch to the power type which AP used for station interface. Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23 Signed-off-by: Wen Gong <quic_wgong@quicinc.com> --- drivers/net/wireless/ath/ath11k/mac.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)