2023-06-07 10:03:07

by Wen Gong

[permalink] [raw]
Subject: [PATCH v4 2/5] wifi: ath11k: store cur_regulatory_info for each radio

The regulatory info of WMI_REG_CHAN_LIST_CC_EXT_EVENTID is not saved
in ath11k now, the info should be saved in ath11k. Save the info for
each radio and support switch regulatory rules dynamically.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

Signed-off-by: Wen Gong <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.h | 1 +
drivers/net/wireless/ath/ath11k/mac.c | 11 +++
drivers/net/wireless/ath/ath11k/mac.h | 2 +-
drivers/net/wireless/ath/ath11k/reg.c | 6 ++
drivers/net/wireless/ath/ath11k/wmi.c | 122 +++++++++++++++++--------
drivers/net/wireless/ath/ath11k/wmi.h | 5 +
6 files changed, 107 insertions(+), 40 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 0830276e5028..e2f86abdb597 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -917,6 +917,7 @@ struct ath11k_base {
* This may or may not be used during the runtime
*/
struct ieee80211_regdomain *new_regd[MAX_RADIOS];
+ struct cur_regulatory_info *reg_info_store;

/* Current DFS Regulatory */
enum ath11k_dfs_region dfs_region;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 05920ad413c5..75843af7bac7 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -629,6 +629,17 @@ struct ath11k *ath11k_mac_get_ar_by_vdev_id(struct ath11k_base *ab, u32 vdev_id)
return NULL;
}

+enum wmi_vdev_type ath11k_mac_get_ar_vdev_type(struct ath11k *ar)
+{
+ struct ath11k_vif *arvif;
+
+ list_for_each_entry(arvif, &ar->arvifs, list) {
+ return arvif->vdev_type;
+ }
+
+ return WMI_VDEV_TYPE_UNSPEC;
+}
+
struct ath11k *ath11k_mac_get_ar_by_pdev_id(struct ath11k_base *ab, u32 pdev_id)
{
int i;
diff --git a/drivers/net/wireless/ath/ath11k/mac.h b/drivers/net/wireless/ath/ath11k/mac.h
index 0231783ad754..16e4c9ef4f44 100644
--- a/drivers/net/wireless/ath/ath11k/mac.h
+++ b/drivers/net/wireless/ath/ath11k/mac.h
@@ -158,7 +158,7 @@ struct ath11k_vif *ath11k_mac_get_vif_up(struct ath11k_base *ab);

struct ath11k *ath11k_mac_get_ar_by_vdev_id(struct ath11k_base *ab, u32 vdev_id);
struct ath11k *ath11k_mac_get_ar_by_pdev_id(struct ath11k_base *ab, u32 pdev_id);
-
+enum wmi_vdev_type ath11k_mac_get_ar_vdev_type(struct ath11k *ar);
void ath11k_mac_drain_tx(struct ath11k *ar);
void ath11k_mac_peer_cleanup_all(struct ath11k *ar);
int ath11k_mac_tx_mgmt_pending_free(int buf_id, void *skb, void *ctx);
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index 761b83c924f6..5d14a04a5c04 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -809,6 +809,12 @@ void ath11k_reg_free(struct ath11k_base *ab)
{
int i;

+ for (i = 0; i < ab->num_radios; i++)
+ ath11k_reg_reset_info(&ab->reg_info_store[i]);
+
+ kfree(ab->reg_info_store);
+ ab->reg_info_store = NULL;
+
for (i = 0; i < ab->hw_params.max_radios; i++) {
kfree(ab->default_regd[i]);
kfree(ab->new_regd[i]);
diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
index 975fd06eb21c..77de11985f5a 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -4652,6 +4652,11 @@ static int ath11k_wmi_tlv_ext_soc_hal_reg_caps_parse(struct ath11k_base *soc,
soc->pdevs[0].pdev_id = 0;
}

+ if (!soc->reg_info_store)
+ soc->reg_info_store = kcalloc(soc->num_radios,
+ sizeof(*soc->reg_info_store),
+ GFP_ATOMIC);
+
return 0;
}

@@ -6991,31 +6996,34 @@ static bool ath11k_reg_is_world_alpha(char *alpha)
return false;
}

-static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
- struct sk_buff *skb,
- enum wmi_reg_chan_list_cmd_type id)
+void ath11k_reg_reset_info(struct cur_regulatory_info *reg_info)
{
- struct cur_regulatory_info *reg_info = NULL;
- struct ieee80211_regdomain *regd = NULL;
- bool intersect = false;
- int ret = 0, pdev_idx, i, j;
- struct ath11k *ar;
+ int i, j;

- reg_info = kzalloc(sizeof(*reg_info), GFP_ATOMIC);
- if (!reg_info) {
- ret = -ENOMEM;
- goto fallback;
- }
+ if (reg_info) {
+ kfree(reg_info->reg_rules_2ghz_ptr);

- if (id == WMI_REG_CHAN_LIST_CC_ID)
- ret = ath11k_pull_reg_chan_list_update_ev(ab, skb, reg_info);
- else
- ret = ath11k_pull_reg_chan_list_ext_update_ev(ab, skb, reg_info);
+ kfree(reg_info->reg_rules_5ghz_ptr);

- if (ret) {
- ath11k_warn(ab, "failed to extract regulatory info from received event\n");
- goto fallback;
+ for (i = 0; i < WMI_REG_CURRENT_MAX_AP_TYPE; i++) {
+ kfree(reg_info->reg_rules_6ghz_ap_ptr[i]);
+ for (j = 0; j < WMI_REG_MAX_CLIENT_TYPE; j++)
+ kfree(reg_info->reg_rules_6ghz_client_ptr[i][j]);
+ }
+
+ memset(reg_info, 0, sizeof(*reg_info));
}
+}
+
+int ath11k_reg_handle_chan_list(struct ath11k_base *ab,
+ struct cur_regulatory_info *reg_info,
+ enum ieee80211_ap_reg_power power_type)
+{
+ struct ieee80211_regdomain *regd;
+ bool intersect = false;
+ int pdev_idx;
+ struct ath11k *ar;
+ enum wmi_vdev_type vdev_type;

if (reg_info->status_code != REG_SET_CC_STATUS_PASS) {
/* In case of failure to set the requested ctry,
@@ -7023,7 +7031,7 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
* and return from here.
*/
ath11k_warn(ab, "Failed to set the requested Country regulatory setting\n");
- goto mem_free;
+ return -EINVAL;
}

pdev_idx = reg_info->phy_id;
@@ -7035,7 +7043,7 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
if (test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags) &&
ab->default_regd[pdev_idx]) {
spin_unlock(&ab->base_lock);
- goto mem_free;
+ goto retfail;
}
spin_unlock(&ab->base_lock);

@@ -7046,7 +7054,7 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
*/
if (ab->hw_params.single_pdev_only &&
pdev_idx < ab->hw_params.num_rxmda_per_pdev)
- goto mem_free;
+ goto retfail;
else
goto fallback;
}
@@ -7057,7 +7065,7 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
if (ab->default_regd[pdev_idx] && !ab->new_regd[pdev_idx] &&
!memcmp((char *)ab->default_regd[pdev_idx]->alpha2,
(char *)reg_info->alpha2, 2))
- goto mem_free;
+ goto retfail;

/* Intersect new rules with default regd if a new country setting was
* requested, i.e a default regd was already set during initialization
@@ -7069,13 +7077,24 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
!ath11k_reg_is_world_alpha((char *)reg_info->alpha2))
intersect = true;

- regd = ath11k_reg_build_regd(ab, reg_info, intersect,
- WMI_VDEV_TYPE_AP, IEEE80211_REG_LPI_AP);
+ ar = ab->pdevs[pdev_idx].ar;
+ vdev_type = ath11k_mac_get_ar_vdev_type(ar);
+
+ ath11k_dbg(ab, ATH11K_DBG_WMI,
+ "wmi handle chan list power type %d vdev type %d intersect %d\n",
+ power_type, vdev_type, intersect);
+
+ regd = ath11k_reg_build_regd(ab, reg_info, intersect, vdev_type, power_type);
if (!regd) {
ath11k_warn(ab, "failed to build regd from reg_info\n");
goto fallback;
}

+ if (power_type == IEEE80211_REG_UNSET_AP) {
+ ath11k_reg_reset_info(&ab->reg_info_store[pdev_idx]);
+ ab->reg_info_store[pdev_idx] = *reg_info;
+ }
+
spin_lock(&ab->base_lock);
if (ab->default_regd[pdev_idx]) {
/* The initial rules from FW after WMI Init is to build
@@ -7098,7 +7117,7 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
ab->dfs_region = reg_info->dfs_region;
spin_unlock(&ab->base_lock);

- goto mem_free;
+ return 0;

fallback:
/* Fallback to older reg (by sending previous country setting
@@ -7110,20 +7129,45 @@ static int ath11k_reg_chan_list_event(struct ath11k_base *ab,
*/
/* TODO: This is rare, but still should also be handled */
WARN_ON(1);
-mem_free:
- if (reg_info) {
- kfree(reg_info->reg_rules_2ghz_ptr);
- kfree(reg_info->reg_rules_5ghz_ptr);
- if (reg_info->is_ext_reg_event) {
- for (i = 0; i < WMI_REG_CURRENT_MAX_AP_TYPE; i++)
- kfree(reg_info->reg_rules_6ghz_ap_ptr[i]);

- for (j = 0; j < WMI_REG_CURRENT_MAX_AP_TYPE; j++)
- for (i = 0; i < WMI_REG_MAX_CLIENT_TYPE; i++)
- kfree(reg_info->reg_rules_6ghz_client_ptr[j][i]);
- }
- kfree(reg_info);
+retfail:
+
+ return -EINVAL;
+}
+
+static int ath11k_reg_chan_list_event(struct ath11k_base *ab, struct sk_buff *skb,
+ enum wmi_reg_chan_list_cmd_type id)
+{
+ struct cur_regulatory_info *reg_info;
+ int ret;
+
+ reg_info = kzalloc(sizeof(*reg_info), GFP_ATOMIC);
+ if (!reg_info)
+ return -ENOMEM;
+
+ if (id == WMI_REG_CHAN_LIST_CC_ID)
+ ret = ath11k_pull_reg_chan_list_update_ev(ab, skb, reg_info);
+ else
+ ret = ath11k_pull_reg_chan_list_ext_update_ev(ab, skb, reg_info);
+
+ if (ret) {
+ ath11k_warn(ab, "failed to extract regulatory info from received event\n");
+ goto mem_free;
+ }
+
+ ret = ath11k_reg_handle_chan_list(ab, reg_info, IEEE80211_REG_UNSET_AP);
+ if (ret) {
+ ath11k_dbg(ab, ATH11K_DBG_WMI,
+ "failed to process regulatory info from received event\n");
+ goto mem_free;
}
+
+ kfree(reg_info);
+ return 0;
+
+mem_free:
+ ath11k_reg_reset_info(reg_info);
+ kfree(reg_info);
return ret;
}

diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
index 92fddb77669c..b9b5cac34f4c 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.h
+++ b/drivers/net/wireless/ath/ath11k/wmi.h
@@ -4923,6 +4923,7 @@ struct ath11k_targ_cap {
};

enum wmi_vdev_type {
+ WMI_VDEV_TYPE_UNSPEC = 0,
WMI_VDEV_TYPE_AP = 1,
WMI_VDEV_TYPE_STA = 2,
WMI_VDEV_TYPE_IBSS = 3,
@@ -6451,4 +6452,8 @@ int ath11k_wmi_pdev_set_bios_geo_table_param(struct ath11k *ar);
int ath11k_wmi_sta_keepalive(struct ath11k *ar,
const struct wmi_sta_keepalive_arg *arg);

+void ath11k_reg_reset_info(struct cur_regulatory_info *reg_info);
+int ath11k_reg_handle_chan_list(struct ath11k_base *ab,
+ struct cur_regulatory_info *reg_info,
+ enum ieee80211_ap_reg_power power_type);
#endif
--
2.40.1



2023-08-02 11:55:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] wifi: ath11k: store cur_regulatory_info for each radio

Wen Gong <[email protected]> writes:

> The regulatory info of WMI_REG_CHAN_LIST_CC_EXT_EVENTID is not saved
> in ath11k now, the info should be saved in ath11k. Save the info for
> each radio and support switch regulatory rules dynamically.
>
> Tested-on: WCN6855 hw2.0 PCI
> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>
> Signed-off-by: Wen Gong <[email protected]>

[...]

> +enum wmi_vdev_type ath11k_mac_get_ar_vdev_type(struct ath11k *ar)
> +{
> + struct ath11k_vif *arvif;
> +
> + list_for_each_entry(arvif, &ar->arvifs, list) {
> + return arvif->vdev_type;
> + }
> +
> + return WMI_VDEV_TYPE_UNSPEC;
> +}

This function looks odd to me and there are no comments to clarify.
What's the idea of using list_for_each_entry() and then immediately
return with the first entry? I guess the assumption here is that every
arvif has the same type? Can we really trust that? And at least it
should be documented here.

Also wouldn't list_first_entry_or_null() be more intuitive?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2023-08-02 12:22:21

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] wifi: ath11k: store cur_regulatory_info for each radio

Wen Gong <[email protected]> writes:

> The regulatory info of WMI_REG_CHAN_LIST_CC_EXT_EVENTID is not saved
> in ath11k now, the info should be saved in ath11k. Save the info for
> each radio and support switch regulatory rules dynamically.
>
> Tested-on: WCN6855 hw2.0 PCI
> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>
> Signed-off-by: Wen Gong <[email protected]>

[...]

> + ret = ath11k_reg_handle_chan_list(ab, reg_info, IEEE80211_REG_UNSET_AP);
> + if (ret) {
> + ath11k_dbg(ab, ATH11K_DBG_WMI,
> + "failed to process regulatory info from received event\n");
> + goto mem_free;
> }

Why the warning message is using ath11k_dbg()? I think it should be
ath11k_warn() and also print ret value.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches