Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Ramya Gnanasekar <quic_rgnanase@quicinc.com>
Cc:     ath12k@lists.infradead.org, linux-wireless@vger.kernel.org,
        P Praneesh <quic_ppranees@quicinc.com>
Subject: Re: [PATCH] wifi: ath12k: Add support to parse new WMI event for 6 GHz regulatory
References: <20230502142018.20301-1-quic_rgnanase@quicinc.com>
Date:   Wed, 17 May 2023 11:50:30 +0300
In-Reply-To: <20230502142018.20301-1-quic_rgnanase@quicinc.com> (Ramya
        Gnanasekar's message of "Tue, 2 May 2023 19:50:18 +0530")
Message-ID: <875y8rmje1.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Ramya Gnanasekar <quic_rgnanase@quicinc.com> writes:

> From: P Praneesh <quic_ppranees@quicinc.com>
>
> In order to support different power levels of 6 GHz AP and client,
> new WMI event for regulatory (WMI_REG_CHAN_LIST_CC_EXT_EVENTID) has been
> added in firmware to provide new parameters required for 6 GHz regulatory rules.
>
> Firmware advertises its capability of handling new event in WMI service ready
> event. Based on that, host needs to set host_service_flags in WMI init command
> to indicate that host supports processing of this WMI event.
> Based on advertised host capability, firmware sends event (WMI_REG_CHAN_LIST_CC_EXT_EVENTID).
> This new event contains 2G/5G/6G reg rules with additional power value
> fields for 6GHz and regd is built accordingly.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
> Signed-off-by: Ramya Gnanasekar <quic_rgnanase@quicinc.com>

I made some changes in the pending branch:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=a483b258992f80c657ed3175293a15cac4acd25d

> @@ -3403,6 +3403,10 @@ int ath12k_wmi_cmd_init(struct ath12k_base *ab)
>  	struct ath12k_wmi_base *wmi_sc = &ab->wmi_ab;
>  	struct ath12k_wmi_init_cmd_arg arg = {};
>  
> +	if (test_bit(WMI_TLV_SERVICE_REG_CC_EXT_EVENT_SUPPORT,
> +		     ab->wmi_ab.svc_map))
> +		arg.res_cfg.is_reg_cc_ext_event_supported = 1;

is_reg_cc_ext_event_supported is bool so I changed this to true.

> +static int ath12k_wmi_tlv_services_parser(struct ath12k_base *ab,
> +					  u16 tag, u16 len,
> +					  const void *ptr,
> +					  void *data)
>  {
> -	const void **tb;
>  	const struct wmi_service_available_event *ev;
> -	int ret;
> +	u32 *wmi_ext2_service_bitmap;
>  	int i, j;
> +	u16 expected_len;
>  
> -	tb = ath12k_wmi_tlv_parse_alloc(ab, skb->data, skb->len, GFP_ATOMIC);
> -	if (IS_ERR(tb)) {
> -		ret = PTR_ERR(tb);
> -		ath12k_warn(ab, "failed to parse tlv: %d\n", ret);
> -		return;
> +	expected_len = WMI_SERVICE_SEGMENT_BM_SIZE32 * sizeof(u32);
> +	if (len < expected_len) {
> +		ath12k_warn(ab, "invalid len %d for the tag 0x%x\n",
> +			    len, tag);
> +		return -EINVAL;
>  	}

I changed the warning to be more descriptive.

> -	ev = tb[WMI_TAG_SERVICE_AVAILABLE_EVENT];
> -	if (!ev) {
> -		ath12k_warn(ab, "failed to fetch svc available ev");
> -		kfree(tb);
> -		return;
> -	}
> +	switch (tag) {
> +	case WMI_TAG_SERVICE_AVAILABLE_EVENT:
> +		ev = (struct wmi_service_available_event *)ptr;
> +		for (i = 0, j = WMI_MAX_SERVICE;
> +		     i < WMI_SERVICE_SEGMENT_BM_SIZE32 && j < WMI_MAX_EXT_SERVICE;
> +		     i++) {
> +			do {
> +				if (le32_to_cpu(ev->wmi_service_segment_bitmap[i]) &
> +				    BIT(j % WMI_AVAIL_SERVICE_BITS_IN_SIZE32))
> +					set_bit(j, ab->wmi_ab.svc_map);
> +			} while (++j % WMI_AVAIL_SERVICE_BITS_IN_SIZE32);
> +		}
>  
> -	/* TODO: Use wmi_service_segment_offset information to get the service
> -	 * especially when more services are advertised in multiple service
> -	 * available events.
> -	 */
> -	for (i = 0, j = WMI_MAX_SERVICE;
> -	     i < WMI_SERVICE_SEGMENT_BM_SIZE32 && j < WMI_MAX_EXT_SERVICE;
> -	     i++) {
> -		do {
> -			if (le32_to_cpu(ev->wmi_service_segment_bitmap[i]) &
> -			    BIT(j % WMI_AVAIL_SERVICE_BITS_IN_SIZE32))
> -				set_bit(j, ab->wmi_ab.svc_map);
> -		} while (++j % WMI_AVAIL_SERVICE_BITS_IN_SIZE32);
> +		ath12k_dbg(ab, ATH12K_DBG_WMI,
> +			   "wmi_ext_service_bitmap 0:0x%x, 1:0x%x, 2:0x%x, 3:0x%x",
> +			   ev->wmi_service_segment_bitmap[0],
> +			   ev->wmi_service_segment_bitmap[1],
> +			   ev->wmi_service_segment_bitmap[2],
> +			   ev->wmi_service_segment_bitmap[3]);

I changed the debug messages to the recommended style.

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

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