Received-SPF: pass (google.com: domain of linux-wireless+bounces-5207-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <e5fe8dc4-c44b-4734-906f-a9dcbb724bb1@quicinc.com>
Date: Mon, 25 Mar 2024 20:54:36 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 05/12] wifi: ath12k: scan statemachine changes for
 single wiphy
Content-Language: en-US
To: Jeff Johnson <quic_jjohnson@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>, Sriram R <quic_srirrama@quicinc.com>
References: <20240320190943.3850106-1-quic_ramess@quicinc.com>
 <20240320190943.3850106-6-quic_ramess@quicinc.com>
 <85e7f591-ed52-4554-bd92-09086eaebcdb@quicinc.com>
From: Rameshkumar Sundaram <quic_ramess@quicinc.com>
In-Reply-To: <85e7f591-ed52-4554-bd92-09086eaebcdb@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 3/22/2024 1:24 AM, Jeff Johnson wrote:
> On 3/20/2024 12:09 PM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> When multiple radios are advertised as a single wiphy which
>> supports various bands, a default scan request to mac80211
>> from cfg80211 will split the driver request based on band,
>> so each request will have channels belonging to the same band.
>> With this supported by default, the ath12k driver on receiving
>> this request checks for one of the channels in the request and
>> selects the corresponding radio(ar) on which the scan is going
>> to be performed and creates a vdev on that radio.
>>
>> Note that on scan completion this vdev is not deleted. If a new
>> scan request is seen on that same vif for a different band the
>> vdev will be deleted and created on the new radio supporting the
>> request. The vdev delete logic is refactored to have this done
>> dynamically.
>>
>> The reason for not deleting the vdev on scan stop is to avoid
>> repeated delete-create sequence if the scan is on the same band.
>> Also, during channel assign, new vdev creation can be optimized
>> as well.
>>
>> Also if the scan is requested when the vdev is in started state,
>> no switching to new radio is allowed and scan on channels only
>> within same radio is allowed.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 211 +++++++++++++++++++++-----
>>   1 file changed, 176 insertions(+), 35 deletions(-)
> ...
>> -static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>> -					   struct ieee80211_vif *vif)
>> +static int ath12k_mac_vdev_delete(struct ath12k *ar, struct ieee80211_vif *vif)
>>   {
>> -	struct ath12k *ar;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>> -	struct ath12k_base *ab;
>> +	struct ath12k_base *ab = ar->ab;
>>   	unsigned long time_left;
>>   	int ret;
>>   
>> -	if (!arvif->is_created)
>> -		return;
>> -
>> -	ar = arvif->ar;
>> -	ab = ar->ab;
>> -
>> -	mutex_lock(&ar->conf_mutex);
>> -
>> -	ath12k_dbg(ab, ATH12K_DBG_MAC, "mac remove interface (vdev %d)\n",
>> -		   arvif->vdev_id);
>> -
>> -	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
>> -		ret = ath12k_peer_delete(ar, arvif->vdev_id, vif->addr);
>> -		if (ret)
>> -			ath12k_warn(ab, "failed to submit AP self-peer removal on vdev %d: %d\n",
>> -				    arvif->vdev_id, ret);
>> -	}
>> -
>> +	lockdep_assert_held(&ar->conf_mutex);
>>   	reinit_completion(&ar->vdev_delete_done);
>>   
>>   	ret = ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
>>   	if (ret) {
>> -		ath12k_warn(ab, "failed to delete WMI vdev %d: %d\n",
>> +		ath12k_warn(ab, "failed to delete WMI scan vdev %d: %d\n",
> 
> this change seems strange. isn't ath12k_mac_vdev_delete() called from both the
> scan logic and from the normal ath12k_mac_op_remove_interface(), so it might
> not be a scan vdev that is being deleted?
> 
No, in Single wiphy, the vdev logic creation for an arvif is such that 
at any given point of time only one vdev is created for an arvif (either 
by ath12k_mac_op_add_intf/assign_chanctx/hw_scan).
Vdev created by mac_op_scan can either be re-used or deleted & 
re-created (on a different ar) by mac_op_assign_chanctx() if required.
Also once mac_op_assign_chanctx has started the vdev of an arvif, 
mac_op_hw_scan can only use that vdev. So mac_op_remove just simply 
deletes the one that is currently created.
>>   			    arvif->vdev_id, ret);
>>   		goto err_vdev_del;
>>   	}
>