Received-SPF: pass (google.com: domain of linux-wireless+bounces-2586-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <8cc71105-9901-4732-aafd-f8e116e485f3@quicinc.com>
Date: Fri, 26 Jan 2024 16:23:57 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 07/11] wifi: ath12k: fix broken structure
 wmi_vdev_create_cmd
Content-Language: en-US
To: Kang Yang <quic_kangyang@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240126115231.356658-1-quic_kangyang@quicinc.com>
 <20240126115231.356658-8-quic_kangyang@quicinc.com>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240126115231.356658-8-quic_kangyang@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 1/26/2024 3:52 AM, Kang Yang wrote:
> Current structure wmi_vdev_create_cmd is not matched to the firmware
> definition. So update it.
> 
> And update vdev_stats_id_valid for vdev_stats_id.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>
> ---
> 
> v4: no change.
> v3: no change.
> v2: add Tested-on tag of QCN9274.
> 
> ---
>  drivers/net/wireless/ath/ath12k/wmi.c | 5 +++++
>  drivers/net/wireless/ath/ath12k/wmi.h | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index c7e732c6c145..34e676799616 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -828,7 +828,12 @@ int ath12k_wmi_vdev_create(struct ath12k *ar, u8 *macaddr,
>  	cmd->vdev_subtype = cpu_to_le32(args->subtype);
>  	cmd->num_cfg_txrx_streams = cpu_to_le32(WMI_NUM_SUPPORTED_BAND_MAX);
>  	cmd->pdev_id = cpu_to_le32(args->pdev_id);
> +	if (args->if_stats_id != ATH12K_INVAL_VDEV_STATS_ID)
> +		cmd->vdev_stats_id_valid = cpu_to_le32(true);
> +	else
> +		cmd->vdev_stats_id_valid = cpu_to_le32(false);

else clause is unnecessary since the struct is zeroed when allocated

>  	cmd->vdev_stats_id = cpu_to_le32(args->if_stats_id);

suggest this only be done inside:
 if (args->if_stats_id != ATH12K_INVAL_VDEV_STATS_ID)

> +
>  	ether_addr_copy(cmd->vdev_macaddr.addr, macaddr);
>  
>  	ptr = skb->data + sizeof(*cmd);
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.h b/drivers/net/wireless/ath/ath12k/wmi.h
> index 0efed12dd7ab..74b0ee9f86bf 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.h
> +++ b/drivers/net/wireless/ath/ath12k/wmi.h
> @@ -2712,6 +2712,9 @@ struct wmi_vdev_create_cmd {
>  	struct ath12k_wmi_mac_addr_params vdev_macaddr;
>  	__le32 num_cfg_txrx_streams;
>  	__le32 pdev_id;
> +	__le32 mbssid_flags;
> +	__le32 mbssid_tx_vdev_id;
> +	__le32 vdev_stats_id_valid;
>  	__le32 vdev_stats_id;
>  } __packed;
>