Re: [PATCH v2 2/2] wifi: ath12k: Introduce the container for mac80211 hw

Karthikeyan Periyasamy
2024-01-16T05:14:59Z

On 1/15/2024 9:38 PM, Kalle Valo wrote:
> Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:
>
>> To support multi link operation, we need to combine all the link/pdev
>> under a single wiphy. This avoids the overhead of synchronization
>> across multiple hardware instances in both the cfg80211 and mac80211
>> layers. Currently, each link/pdev is registered as separate wiphy,
>> tightly coupled with link/pdev/radio (ar) structure. To enable single
>> wiphy registration within the chip, we decouple the wiphy data entity from
>> the link/pdev/radio (ar) structure and move it under the chip (ab)
>> structure with a new data container (ath12k_hw) structure. This approach
>> improves scalability for future multi link operation support.
> What about struct ath12k_pdev? Do we need it still or should it be removed?

ath12k_pdev still holds the pdev related information within the SoC/chip
(ab). I dont want it be remove.

>
>> static void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
>> struct ieee80211_vif *vif)
>> {
>> - struct ath12k *ar = hw->priv;
>> + struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>> + struct ath12k *ar;
>> +
>> + mutex_lock(&ah->conf_mutex);
>> +
>> + ar = ath12k_ah_to_ar(ah);
>>
>> mutex_lock(&ar->conf_mutex);
>> ath12k_scan_abort(ar);
>> mutex_unlock(&ar->conf_mutex);
>>
>> + mutex_unlock(&ah->conf_mutex);
>> +
>> cancel_delayed_work_sync(&ar->scan.timeout);
>> }
> Do we really need two mutexes? I don't see any analysis about that. And
> even if we do, I feel that it should be added in a separate patch.

Yes, ah->conf_mutex protect the concurrent mac80211 operation. But there
is other places like radio/link specific synchronous operation (ie MGMT
tx wait for the vdev deletion) is needed. To fulfill this need, we also
need radio/link specific (ar) mutex instead of all link (ah) mutex for
efficient lock/unlock.

Thanks,

Karthikeyan