[PATCH 8/8] wifi: ath12k: Add MLO peer assoc command support

[Jeff Johnson]

On 10/29/2024 9:05 AM, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson at quicinc.com> writes:
> 
>> On 10/23/2024 6:30 AM, Kalle Valo wrote:
>>
>>> +	rcu_read_lock();
>>> +
>>> +	i = 0;
>>
>> nit: setting i=0 doesn't need to be RCU protected
> 
> Yeah, but that doesn't cause any issues and this way it's closer to the
> for loop where it's used:
> 
>>> +	for_each_set_bit(link_id, &links, IEEE80211_MLD_MAX_NUM_LINKS) {
>>> +		if (i >= ATH12K_WMI_MLO_MAX_LINKS)
>>> +			break;
> 
> [...]
> 
>>> @@ -2243,12 +2251,38 @@ int ath12k_wmi_send_peer_assoc_cmd(struct ath12k *ar,
>>>  		ptr += sizeof(*he_mcs);
>>>  	}
>>>  
>>> -	/* MLO header tag with 0 length */
>>> -	len = 0;
>>>  	tlv = ptr;
>>> +	len = arg->ml.enabled ? sizeof(*ml_params) : 0;
>>>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_STRUCT, len);
>>>  	ptr += TLV_HDR_SIZE;
>>> +	if (!len)
>>> +		goto skip_ml_params;
>>>  
>>> +	ml_params = ptr;
>>> +	ml_params->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_MLO_PEER_ASSOC_PARAMS,
>>> +						       len);
>>
>> this is another instance where we are using the same length for two
>> consecutive TLVs -- that doesn't seem right
> 
> This is also a similar case of _hdr() vs _cmd_hdr(), does that look ok?
> 
here again this is evil. please change to _hdr() and explicitly subtract out
the tlv header size