On 6/10/2024 7:26 PM, Kalle Valo wrote:
Rameshkumar Sundaram <quic_ramess@xxxxxxxxxxx> writes:
When multiple radios are advertised as a single wiphy which
supports various bands, vdev creation for the vif is deferred
until channel is assigned to it.
If a remain on channel(RoC) request is received from mac80211,
select the corresponding radio(ar) based on channel and create
a vdev on that radio to initiate an RoC scan.
Note that on RoC completion this vdev is not deleted. If a new
RoC/hw 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.
Also if the RoC scan is requested when the vdev is in started state,
no switching to new radio is allowed and RoC request can be accepted
only on channels within same radio.
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: Rameshkumar Sundaram <quic_ramess@xxxxxxxxxxx>
I did some white space changes to the commit message.
@@ -8416,12 +8416,63 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
struct ath12k_wmi_scan_req_arg arg;
- struct ath12k *ar;
+ struct ath12k *ar, *prev_ar;
u32 scan_time_msec;
+ bool create = true;
int ret;
- ar = ath12k_ah_to_ar(ah, 0);
+ if (ah->num_radio == 1) {
+ WARN_ON(!arvif->is_created);
+ ar = ath12k_ah_to_ar(ah, 0);
+ goto scan;
+ }
+
+ ar = ath12k_mac_select_scan_device(hw, vif, chan->center_freq);
+ if (!ar)
+ return -EINVAL;
+
+ /* If the vif is already assigned to a specific vdev of an ar,
+ * check whether its already started, vdev which is started
+ * are not allowed to switch to a new radio.
+ * If the vdev is not started, but was earlier created on a
+ * different ar, delete that vdev and create a new one. We don't
+ * delete at the scan stop as an optimization to avoid redundant
+ * delete-create vdev's for the same ar, in case the request is
+ * always on the same band for the vif
+ */
+ if (arvif->is_created) {
+ if (WARN_ON(!arvif->ar))
+ return -EINVAL;
+
+ if (ar != arvif->ar && arvif->is_started)
+ return -EINVAL;
I wonder if -EBUSY would be more descriptive here? I changed to that in
the pending branch.
Yeah, landing here would mean that user has already started the
interface on a radio(ar) and now requesting RoC on a channel that
doesn't belong to it.
+ if (ar != arvif->ar) {
+ /* backup the previously used ar ptr, since the vdev delete
+ * would assign the arvif->ar to NULL after the call
+ */
+ prev_ar = arvif->ar;
+ mutex_lock(&prev_ar->conf_mutex);
+ ret = ath12k_mac_vdev_delete(prev_ar, vif);
+ mutex_unlock(&prev_ar->conf_mutex);
+ if (ret)
+ ath12k_warn(prev_ar->ab,
+ "unable to delete scan vdev %d\n", ret);
Do we really want to continue if vdev_delete() fails? In the pending
branch I added 'return ret' here and modified the warning message a bit.
+ } else {
+ create = false;
+ }
+ }
+
+ if (create) {
+ mutex_lock(&ar->conf_mutex);
+ ret = ath12k_mac_vdev_create(ar, vif);
+ mutex_unlock(&ar->conf_mutex);
+ if (ret) {
+ ath12k_warn(ar->ab, "unable to create scan vdev %d\n", ret);
+ return -EINVAL;
+ }
Also here I modified the warning message a bit.
The pending commit here:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=9b4ec32e921b34bd7a03d39cc0a75cba7e85dc02
Changes looks fine for me, thanks a lot for Fixing them.
