Received-SPF: pass (google.com: domain of linux-wireless+bounces-523-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <db8b0734-8e4e-43f9-ba48-b3df2d33e29e@quicinc.com>
Date: Thu, 7 Dec 2023 08:45:20 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 02/12] wifi: ath11k: store cur_regulatory_info for each
 radio
Content-Language: en-US
To: Baochen Qiang <quic_bqiang@quicinc.com>, <ath11k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20231204081323.5582-1-quic_bqiang@quicinc.com>
 <20231204081323.5582-3-quic_bqiang@quicinc.com>
From: Aditya Kumar Singh <quic_adisi@quicinc.com>
In-Reply-To: <20231204081323.5582-3-quic_bqiang@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

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?