On 4/12/24 5:23 PM, "Nicolas Escande" <nico.escande@xxxxxxxxx> wrote:
On Fri Apr 12, 2024 at 10:47 AM CEST, Kang Yang wrote:
>
>
> On 4/12/2024 3:33 PM, Nicolas Escande wrote:
>> On Fri Apr 12, 2024 at 4:51 AM CEST, kangyang wrote:
>> [...]
>>> @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
>>> lockdep_assert_held(&ar->conf_mutex);
>>>
>>> arvif->ar = ar;
>>> + arvif->vif = vif;
>>> +
>>> + INIT_LIST_HEAD(&arvif->list);
>>> + INIT_DELAYED_WORK(&arvif->connection_loss_work,
>>> + ath12k_mac_vif_sta_connection_loss_work);
>>> +
>> Is there a need to move the following part ?
>> Isn't just adding the delay work enough ?
>
>
> Just checked, you are right, but should add delay work in add_interface().
>
> Will change in v3.
>
>
>>> + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
>>> + arvif->bitrate_mask.control[i].legacy = 0xffffffff;
>>> + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
>>> + sizeof(arvif->bitrate_mask.control[i].ht_mcs));
>>> + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
>>> + sizeof(arvif->bitrate_mask.control[i].vht_mcs));
>>> + }
>>> +
>>> vdev_id = __ffs64(ab->free_vdev_map);
>>> arvif->vdev_id = vdev_id;
>>> arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
>>> @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>>
>>> arvif->vif = vif;
>>>
>>> - INIT_LIST_HEAD(&arvif->list);
>>> -
>>> - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
>>> - arvif->bitrate_mask.control[i].legacy = 0xffffffff;
>>> - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
>>> - sizeof(arvif->bitrate_mask.control[i].ht_mcs));
>>> - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
>>> - sizeof(arvif->bitrate_mask.control[i].vht_mcs));
>>> - }
>>> -
>>> /* Allocate Default Queue now and reassign during actual vdev create */
>>> vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE;
>>> for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++)
>> [...]
>>
>> Thanks
Yeah, I wasn't clear enough, I meant adding the INIT_DELAY_WORK without moving
the rest of the code around.
I know what you mean --- Just add INIT_DELAY_WORK in ath12k_mac_vdev_create().
But i just checked with the person who just refactor the related code in patchset 'wifi: ath12k: Add single wiphy support'.
He also think add in ath12k_mac_op_add_interface() is better.
Thanks