From: Wen Gong <[email protected]>
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]>
Acked-by: Jeff Johnson <[email protected]>
Signed-off-by: Baochen Qiang <[email protected]>
---
v8:
no change.
v7:
1. fix ath11k-check warning
2. fix memory leak.
drivers/net/wireless/ath/ath11k/core.h | 1 +
drivers/net/wireless/ath/ath11k/mac.h | 1 -
drivers/net/wireless/ath/ath11k/reg.c | 6 +
drivers/net/wireless/ath/ath11k/wmi.c | 150 ++++++++++++++++++-------
drivers/net/wireless/ath/ath11k/wmi.h | 5 +
5 files changed, 119 insertions(+), 44 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 7e3b6779f4e9..cc91f7d3ca8e 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -922,6 +922,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.h b/drivers/net/wireless/ath/ath11k/mac.h
index 0dfdeed5177b..774a9f01c030 100644
--- a/drivers/net/wireless/ath/ath11k/mac.h
+++ b/drivers/net/wireless/ath/ath11k/mac.h
@@ -159,7 +159,6 @@ 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);
-
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 8a6fe9b495bf..b860d2fd7e5c 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -821,6 +821,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 75c79c99faa9..4d484d29826e 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -4749,6 +4749,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;
}
@@ -7071,33 +7076,54 @@ 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));
}
+}
+
+static
+enum wmi_vdev_type ath11k_reg_get_ar_vdev_type(struct ath11k *ar)
+{
+ struct ath11k_vif *arvif;
+
+ /* Currently each struct ath11k maps to one struct ieee80211_hw/wiphy
+ * and one struct ieee80211_regdomain, so it could only store one group
+ * reg rules. It means muti-interface concurrency in the same ath11k is
+ * not support for the regdomain. So get the vdev type of the first entry
+ * now. After concurrency support for the regdomain, this should change.
+ */
+ arvif = list_first_entry_or_null(&ar->arvifs, struct ath11k_vif, list);
+ if (arvif)
+ return arvif->vdev_type;
+
+ return WMI_VDEV_TYPE_UNSPEC;
+}
+
+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;
- ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg chan list id %d", id);
+ ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg handle chan list");
if (reg_info->status_code != REG_SET_CC_STATUS_PASS) {
/* In case of failure to set the requested ctry,
@@ -7105,7 +7131,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;
@@ -7117,20 +7143,23 @@ 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);
if (pdev_idx >= ab->num_radios) {
/* Process the event for phy0 only if single_pdev_only
* is true. If pdev_idx is valid but not 0, discard the
- * event. Otherwise, it goes to fallback.
+ * event. Otherwise, it goes to fallback. In either case
+ * ath11k_reg_reset_info() needs to be called to avoid
+ * memory leak issue.
*/
+ ath11k_reg_reset_info(reg_info);
+
if (ab->hw_params.single_pdev_only &&
pdev_idx < ab->hw_params.num_rxmda_per_pdev)
- goto mem_free;
- else
- goto fallback;
+ return 0;
+ goto fallback;
}
/* Avoid multiple overwrites to default regd, during core
@@ -7139,7 +7168,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
@@ -7151,13 +7180,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_reg_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
@@ -7180,7 +7220,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
@@ -7192,20 +7232,44 @@ 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\n");
+ goto mem_free;
+ }
+
+ ret = ath11k_reg_handle_chan_list(ab, reg_info, IEEE80211_REG_UNSET_AP);
+ if (ret) {
+ ath11k_warn(ab, "failed to process regulatory info %d\n", ret);
+ 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 ff0a9a92beeb..d46c70704774 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.h
+++ b/drivers/net/wireless/ath/ath11k/wmi.h
@@ -4951,6 +4951,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,
@@ -6480,4 +6481,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.25.1
On 12/4/23 13:43, Baochen Qiang wrote:
> --- a/drivers/net/wireless/ath/ath11k/mac.h
> +++ b/drivers/net/wireless/ath/ath11k/mac.h
> @@ -159,7 +159,6 @@ 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);
> -
Irrelevant change w.r.t commit message?
> 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);
...
> @@ -4749,6 +4749,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);
What if this memory allocation request fails? Any negative case check
should be present?
> +
> return 0;
> }
>
> @@ -7071,33 +7076,54 @@ 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));
> }
> +}
> +
> +static
> +enum wmi_vdev_type ath11k_reg_get_ar_vdev_type(struct ath11k *ar)
> +{
> + struct ath11k_vif *arvif;
> +
> + /* Currently each struct ath11k maps to one struct ieee80211_hw/wiphy
> + * and one struct ieee80211_regdomain, so it could only store one group
> + * reg rules. It means muti-interface concurrency in the same ath11k is
> + * not support for the regdomain. So get the vdev type of the first entry
> + * now. After concurrency support for the regdomain, this should change.
> + */
> + arvif = list_first_entry_or_null(&ar->arvifs, struct ath11k_vif, list);
> + if (arvif)
> + return arvif->vdev_type;
> +
> + return WMI_VDEV_TYPE_UNSPEC;
> +}
> +
> +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;
>
> - ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg chan list id %d", id);
> + ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg handle chan list");
I believe this debug was helpful in the sense it showed which type of
event came. Can't we still print this somehow? Or may be somewhere else?
On 12/7/2023 11:15 AM, Aditya Kumar Singh wrote:
> On 12/4/23 13:43, Baochen Qiang wrote:
>> --- a/drivers/net/wireless/ath/ath11k/mac.h
>> +++ b/drivers/net/wireless/ath/ath11k/mac.h
>> @@ -159,7 +159,6 @@ 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);
>> -
>
> Irrelevant change w.r.t commit message?
>
>
>> 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);
> ...
>> @@ -4749,6 +4749,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);
> What if this memory allocation request fails? Any negative case check
> should be present?
>
>
>> +
>> return 0;
>> }
>> @@ -7071,33 +7076,54 @@ 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));
>> }
>> +}
>> +
>> +static
>> +enum wmi_vdev_type ath11k_reg_get_ar_vdev_type(struct ath11k *ar)
>> +{
>> + struct ath11k_vif *arvif;
>> +
>> + /* Currently each struct ath11k maps to one struct
>> ieee80211_hw/wiphy
>> + * and one struct ieee80211_regdomain, so it could only store one
>> group
>> + * reg rules. It means muti-interface concurrency in the same
>> ath11k is
>> + * not support for the regdomain. So get the vdev type of the
>> first entry
>> + * now. After concurrency support for the regdomain, this should
>> change.
>> + */
>> + arvif = list_first_entry_or_null(&ar->arvifs, struct ath11k_vif,
>> list);
>> + if (arvif)
>> + return arvif->vdev_type;
>> +
>> + return WMI_VDEV_TYPE_UNSPEC;
>> +}
>> +
>> +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;
>> - ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg chan list id %d", id);
>> + ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg handle chan list");
>
> I believe this debug was helpful in the sense it showed which type of
> event came. Can't we still print this somehow? Or may be somewhere else?You can check the event type from logs of
ath11k_pull_reg_chan_list_update_ev() and
ath11k_pull_reg_chan_list_ext_update_ev().
Baochen Qiang <[email protected]> writes:
> On 12/7/2023 11:15 AM, Aditya Kumar Singh wrote:
>> On 12/4/23 13:43, Baochen Qiang wrote:
>>> --- a/drivers/net/wireless/ath/ath11k/mac.h
>>> +++ b/drivers/net/wireless/ath/ath11k/mac.h
>>> @@ -159,7 +159,6 @@ 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);
>>> -
>> Irrelevant change w.r.t commit message?
>>
>>> 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);
>> ...
>>> @@ -4749,6 +4749,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);
>> What if this memory allocation request fails? Any negative case
>> check should be present?
>>
>>> +
>>> return 0;
>>> }
>>> @@ -7071,33 +7076,54 @@ 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));
>>> }
>>> +}
>>> +
>>> +static
>>> +enum wmi_vdev_type ath11k_reg_get_ar_vdev_type(struct ath11k *ar)
>>> +{
>>> + struct ath11k_vif *arvif;
>>> +
>>> + /* Currently each struct ath11k maps to one struct
>>> ieee80211_hw/wiphy
>>> + * and one struct ieee80211_regdomain, so it could only store
>>> one group
>>> + * reg rules. It means muti-interface concurrency in the same
>>> ath11k is
>>> + * not support for the regdomain. So get the vdev type of the
>>> first entry
>>> + * now. After concurrency support for the regdomain, this
>>> should change.
>>> + */
>>> + arvif = list_first_entry_or_null(&ar->arvifs, struct
>>> ath11k_vif, list);
>>> + if (arvif)
>>> + return arvif->vdev_type;
>>> +
>>> + return WMI_VDEV_TYPE_UNSPEC;
>>> +}
>>> +
>>> +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;
>>> - ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg chan list id %d", id);
>>> + ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg handle chan list");
>> I believe this debug was helpful in the sense it showed which type
>> of event came. Can't we still print this somehow? Or may be
>> somewhere else?You can check the event type from logs of
> ath11k_pull_reg_chan_list_update_ev() and
> ath11k_pull_reg_chan_list_ext_update_ev().
Baochen, I didn't see any comments from you. Did you send an empty mail
by accident?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 12/11/2023 11:18 PM, Kalle Valo wrote:
> Baochen Qiang <[email protected]> writes:
>
>> On 12/7/2023 11:15 AM, Aditya Kumar Singh wrote:
>>> On 12/4/23 13:43, Baochen Qiang wrote:
>>>> --- a/drivers/net/wireless/ath/ath11k/mac.h
>>>> +++ b/drivers/net/wireless/ath/ath11k/mac.h
>>>> @@ -159,7 +159,6 @@ 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);
>>>> -
>>> Irrelevant change w.r.t commit message?
>>>
>>>> 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);
>>> ...
>>>> @@ -4749,6 +4749,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);
>>> What if this memory allocation request fails? Any negative case
>>> check should be present?
>>>
>>>> +
>>>> return 0;
>>>> }
>>>> @@ -7071,33 +7076,54 @@ 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));
>>>> }
>>>> +}
>>>> +
>>>> +static
>>>> +enum wmi_vdev_type ath11k_reg_get_ar_vdev_type(struct ath11k *ar)
>>>> +{
>>>> + struct ath11k_vif *arvif;
>>>> +
>>>> + /* Currently each struct ath11k maps to one struct
>>>> ieee80211_hw/wiphy
>>>> + * and one struct ieee80211_regdomain, so it could only store
>>>> one group
>>>> + * reg rules. It means muti-interface concurrency in the same
>>>> ath11k is
>>>> + * not support for the regdomain. So get the vdev type of the
>>>> first entry
>>>> + * now. After concurrency support for the regdomain, this
>>>> should change.
>>>> + */
>>>> + arvif = list_first_entry_or_null(&ar->arvifs, struct
>>>> ath11k_vif, list);
>>>> + if (arvif)
>>>> + return arvif->vdev_type;
>>>> +
>>>> + return WMI_VDEV_TYPE_UNSPEC;
>>>> +}
>>>> +
>>>> +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;
>>>> - ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg chan list id %d", id);
>>>> + ath11k_dbg(ab, ATH11K_DBG_WMI, "event reg handle chan list");
>>> I believe this debug was helpful in the sense it showed which type
>>> of event came. Can't we still print this somehow? Or may be
>>> somewhere else?You can check the event type from logs of
>> ath11k_pull_reg_chan_list_update_ev() and
>> ath11k_pull_reg_chan_list_ext_update_ev().
>
> Baochen, I didn't see any comments from you. Did you send an empty mail
> by accident?
I did have comments, but some how, I don't know why, it gets merged in
Aditya's comments. Will repost it here:
You can check the event type from logs of
ath11k_pull_reg_chan_list_update_ev() and
ath11k_pull_reg_chan_list_ext_update_ev().
>