Received-SPF: pass (google.com: domain of linux-wireless+bounces-5115-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <85e7f591-ed52-4554-bd92-09086eaebcdb@quicinc.com>
Date: Thu, 21 Mar 2024 12:54:26 -0700
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: Rameshkumar Sundaram <quic_ramess@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>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240320190943.3850106-6-quic_ramess@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

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?

>  			    arvif->vdev_id, ret);
>  		goto err_vdev_del;
>  	}