[PATCH v3 1/2] wifi: ath12k: Update HTT_TCL_METADATA version and bit mask definitions

[Balamurugan Mahalingam]

On 2/3/2025 7:30 AM, Aditya Kumar Singh wrote:
> On 2/3/25 13:25, Balamurugan Mahalingam wrote:
>> Update the HTT_TCL_METADATA version to the latest version (2)
>> as the bit definitions have changed a little to support more
>> features. This new version allows the host to submit a packet with
>> more information to the firmware. Firmware uses this additional
>> information to do special processing for certain frames.
>>
>> All the firmware binaries available in upstream/public are compatible 
>> with
>> this HTT version update.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI 
>> WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Balamurugan Mahalingam <quic_bmahalin at quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/dp.h    | 21 ++++++++++++++-------
>>   drivers/net/wireless/ath/ath12k/dp_tx.c | 12 ++++++++++--
>>   2 files changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/dp.h 
>> b/drivers/net/wireless/ath/ath12k/dp.h
>> index f68bb78d4a11..7cdc62aa35be 100644
>> --- a/drivers/net/wireless/ath/ath12k/dp.h
>> +++ b/drivers/net/wireless/ath/ath12k/dp.h
>> @@ -1,7 +1,7 @@
>>   /* SPDX-License-Identifier: BSD-3-Clause-Clear */
>>   /*
>>    * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>>    */
>>   #ifndef ATH12K_DP_H
>> @@ -372,17 +372,18 @@ struct ath12k_dp {
>>   };
>>   /* HTT definitions */
>> +#define HTT_TAG_TCL_METADATA_VERSION        5
>> -#define HTT_TCL_META_DATA_TYPE        BIT(0)
>> -#define HTT_TCL_META_DATA_VALID_HTT        BIT(1)
>> +#define HTT_TCL_META_DATA_TYPE        GENMASK(1, 0)
>> +#define HTT_TCL_META_DATA_VALID_HTT        BIT(2)
>>   /* vdev meta data */
>> -#define HTT_TCL_META_DATA_VDEV_ID        GENMASK(9, 2)
>> -#define HTT_TCL_META_DATA_PDEV_ID        GENMASK(11, 10)
>> -#define HTT_TCL_META_DATA_HOST_INSPECTED    BIT(12)
>> +#define HTT_TCL_META_DATA_VDEV_ID         GENMASK(10, 3)
>> +#define HTT_TCL_META_DATA_PDEV_ID         GENMASK(12, 11)
>> +#define HTT_TCL_META_DATA_HOST_INSPECTED_MISSION BIT(13)
> 
> nit: Can we align these three new additions in same column?
It actually looks aligned like inside vim. Tabs are used for indentation 
instead of space
> 
>>   /* peer meta data */
>> -#define HTT_TCL_META_DATA_PEER_ID        GENMASK(15, 2)
>> +#define HTT_TCL_META_DATA_PEER_ID        GENMASK(15, 3)
>>   /* HTT tx completion is overlaid in wbm_release_ring */
>>   #define HTT_TX_WBM_COMP_INFO0_STATUS        GENMASK(16, 13)
>> @@ -413,9 +414,15 @@ enum htt_h2t_msg_type {
>>   };
>>   #define HTT_VER_REQ_INFO_MSG_ID        GENMASK(7, 0)
>> +#define HTT_OPTION_TCL_METADATA_VER_V2    2
>> +#define HTT_OPTION_TAG        GENMASK(7, 0)
>> +#define HTT_OPTION_LEN        GENMASK(15, 8)
>> +#define HTT_OPTION_VALUE        GENMASK(31, 16)
>> +#define HTT_TCL_METADATA_VER_SZ        4
>>
> 
> Same here. All aligning in same column would be good.
It actually looks aligned like inside vim. Tabs are used for indentation
instead of space.
> 
>>   struct htt_ver_req_cmd {
>>       __le32 ver_reg_info;
>> +    __le32 tcl_metadata_version;
>>   } __packed;
>>   enum htt_srng_ring_type {
>> diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c 
>> b/drivers/net/wireless/ath/ath12k/dp_tx.c
>> index aa8058dd2da6..5ed41783d039 100644
>> --- a/drivers/net/wireless/ath/ath12k/dp_tx.c
>> +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c
>> @@ -1,7 +1,7 @@
>>   // SPDX-License-Identifier: BSD-3-Clause-Clear
>>   /*
>>    * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>>    */
>>   #include "core.h"
>> @@ -1103,7 +1103,15 @@ int ath12k_dp_tx_htt_h2t_ver_req_msg(struct 
>> ath12k_base *ab)
>>       skb_put(skb, len);
>>       cmd = (struct htt_ver_req_cmd *)skb->data;
>>       cmd->ver_reg_info = le32_encode_bits(HTT_H2T_MSG_TYPE_VERSION_REQ,
>> -                         HTT_VER_REQ_INFO_MSG_ID);
>> +                         HTT_OPTION_TAG);
>> +
>> +    cmd->tcl_metadata_version = 
>> le32_encode_bits(HTT_TAG_TCL_METADATA_VERSION,
>> +                             HTT_OPTION_TAG);
>> +    cmd->tcl_metadata_version |= 
>> le32_encode_bits(HTT_TCL_METADATA_VER_SZ,
>> +                              HTT_OPTION_LEN);
>> +    cmd->tcl_metadata_version |=
>> +            le32_encode_bits(HTT_OPTION_TCL_METADATA_VER_V2,
>> +                     HTT_OPTION_VALUE);
>>
> 
> Can't we write like this ?
> 
> x = a |
>      b |
>      c;
> 
> instead of
> 
> x = a;
> x |= b;
> x |= c;

Fixed in v4.

> 
>>       ret = ath12k_htc_send(&ab->htc, dp->eid, skb);
>>       if (ret) {
> 
>