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

[Kalle Valo <kvalo@kernel.org>]

Jeff Johnson <quic_jjohnson@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?

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

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