Received-SPF: pass (google.com: domain of linux-wireless+bounces-3129-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Kalle Valo <kvalo@kernel.org>
To: Kang Yang <quic_kangyang@quicinc.com>
Cc: <ath12k@lists.infradead.org>,  <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v6 01/11] wifi: ath12k: fix broken structure
 wmi_vdev_create_cmd
References: <20240130040303.370590-1-quic_kangyang@quicinc.com>
	<20240130040303.370590-2-quic_kangyang@quicinc.com>
Date: Mon, 05 Feb 2024 17:46:40 +0200
In-Reply-To: <20240130040303.370590-2-quic_kangyang@quicinc.com> (Kang Yang's
	message of "Tue, 30 Jan 2024 12:02:53 +0800")
Message-ID: <87jznisylb.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Kang Yang <quic_kangyang@quicinc.com> writes:

> 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>
> ---
>
> v6: position adjustment.
> v5: no change.
> v4: no change.
> v3: no change.
> v2:
>     1. add Tested-on tag of QCN9274.
>     2. update copyright.

BTW it's quite a lot of work to add changelog to every patch, especially
if there are 11 patches. It's totally fine to have the list of changes
between versions in the cover letter, no need to add them individually
to every patch. Though this is not a problem, just trying to reduce your
workload.

> ---
>  drivers/net/wireless/ath/ath12k/wmi.c | 3 +++
>  drivers/net/wireless/ath/ath12k/wmi.h | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 2fa724e5851a..d8ebaf4b8359 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -826,6 +826,9 @@ int ath12k_wmi_vdev_create(struct ath12k *ar, u8 *macaddr,
>  	cmd->vdev_stats_id = cpu_to_le32(args->if_stats_id);
>  	ether_addr_copy(cmd->vdev_macaddr.addr, macaddr);
>  
> +	if (args->if_stats_id != ATH12K_INVAL_VDEV_STATS_ID)
> +		cmd->vdev_stats_id_valid = cpu_to_le32(true);

Using 'true' in a firmware interface looks odd to me and I think what we
mean is BIT(0), right? I'll change this in the pending branch.

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

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