[6/8] wifi: ath12k: add multi-link flag in peer create command

From: Sriram R <quic_srirrama@quicinc.com>

Driver should indicate to firmware whether a peer is multi-link or not in peer
create command using multi-link flag. Add changes to support
WMI_TAG_MLO_PEER_CREATE_PARAMS in WMI_PEER_CREATE_CMDID.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-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: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c |  5 +++--
 drivers/net/wireless/ath/ath12k/wmi.c | 27 +++++++++++++++++++++++----
 drivers/net/wireless/ath/ath12k/wmi.h |  6 ++++++
 3 files changed, 32 insertions(+), 6 deletions(-)

Kalle Valo <kvalo@kernel.org> wrote:
> @@ -7016,7 +7017,7 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif)
>         struct ath12k_vif *ahvif = arvif->ahvif;
>         struct ieee80211_vif *vif = ath12k_ahvif_to_vif(ahvif);
>         struct ath12k_wmi_vdev_create_arg vdev_arg = {0};
> -       struct ath12k_wmi_peer_create_arg peer_param;
> +       struct ath12k_wmi_peer_create_arg peer_param = {0};

Using '= {}' as initializer would be better, no matter first field of struct
will be changed in the future.

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

>> @@ -1244,9 +1249,23 @@ int ath12k_wmi_send_peer_create_cmd(struct ath12k *ar,
>>  	cmd->peer_type = cpu_to_le32(arg->peer_type);
>>  	cmd->vdev_id = cpu_to_le32(arg->vdev_id);
>>  
>> +	ptr = skb->data + sizeof(*cmd);
>> +	tlv = ptr;
>> +	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_STRUCT,
>> +					 sizeof(*ml_param));
>
> using the same TLV size both here and for the TLV that follows doesn't seem
> logical. is this missing + TLV_HDR_SIZE to account for its own TLV header?

I have forgotten the details of WMI voodoo so I can't really comment
right now :)

>> +	ptr += TLV_HDR_SIZE;
>> +	ml_param = ptr;
>> +	ml_param->tlv_header =
>> +			ath12k_wmi_tlv_cmd_hdr(WMI_TAG_MLO_PEER_CREATE_PARAMS,
>> +					       sizeof(*ml_param));

But did you notice that here is used ath12k_wmi_tlv_cmd_hdr() and it
reduces the header size:

static __le32 ath12k_wmi_tlv_cmd_hdr(u32 cmd, u32 len)
{
	return ath12k_wmi_tlv_hdr(cmd, len - TLV_HDR_SIZE);
}

On 10/29/2024 8:54 AM, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> 
>>> @@ -1244,9 +1249,23 @@ int ath12k_wmi_send_peer_create_cmd(struct ath12k *ar,
>>>  	cmd->peer_type = cpu_to_le32(arg->peer_type);
>>>  	cmd->vdev_id = cpu_to_le32(arg->vdev_id);
>>>  
>>> +	ptr = skb->data + sizeof(*cmd);
>>> +	tlv = ptr;
>>> +	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_STRUCT,
>>> +					 sizeof(*ml_param));
>>
>> using the same TLV size both here and for the TLV that follows doesn't seem
>> logical. is this missing + TLV_HDR_SIZE to account for its own TLV header?
> 
> I have forgotten the details of WMI voodoo so I can't really comment
> right now :)
> 
>>> +	ptr += TLV_HDR_SIZE;
>>> +	ml_param = ptr;
>>> +	ml_param->tlv_header =
>>> +			ath12k_wmi_tlv_cmd_hdr(WMI_TAG_MLO_PEER_CREATE_PARAMS,
>>> +					       sizeof(*ml_param));
> 
> But did you notice that here is used ath12k_wmi_tlv_cmd_hdr() and it
> reduces the header size:
> 
> static __le32 ath12k_wmi_tlv_cmd_hdr(u32 cmd, u32 len)
> {
> 	return ath12k_wmi_tlv_hdr(cmd, len - TLV_HDR_SIZE);
> }
> 

Yes, I missed that since that is evil to use the _cmd_ TLV function on
something that isn't the command TLV.

Please fix to use the standard function and subtract the thv header size from
the length param