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