| Message ID | 20240528082739.1226758-1-quic_ramess@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath12k: modify remain on channel for single wiphy | expand |
Rameshkumar Sundaram <quic_ramess@quicinc.com> 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@quicinc.com> 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. > + 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
On 6/10/2024 6:56 AM, Kalle Valo wrote: > The pending commit here: > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=9b4ec32e921b34bd7a03d39cc0a75cba7e85dc02 > LGTM
On 6/10/2024 7:26 PM, Kalle Valo wrote: > Rameshkumar Sundaram <quic_ramess@quicinc.com> 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@quicinc.com> > > 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.
Rameshkumar Sundaram <quic_ramess@quicinc.com> wrote: > 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@quicinc.com> > Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. 4938ba733ee2 wifi: ath12k: modify remain on channel for single wiphy
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 784964ae03ec..3668186d0926 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -3372,7 +3372,7 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw, static struct ath12k* ath12k_mac_select_scan_device(struct ieee80211_hw *hw, struct ieee80211_vif *vif, - struct ieee80211_scan_request *req) + u32 center_freq) { struct ath12k_hw *ah = hw->priv; enum nl80211_band band; @@ -3389,9 +3389,9 @@ ath12k_mac_select_scan_device(struct ieee80211_hw *hw, * split the hw request and perform multiple scans */ - if (req->req.channels[0]->center_freq < ATH12K_MIN_5G_FREQ) + if (center_freq < ATH12K_MIN_5G_FREQ) band = NL80211_BAND_2GHZ; - else if (req->req.channels[0]->center_freq < ATH12K_MIN_6G_FREQ) + else if (center_freq < ATH12K_MIN_6G_FREQ) band = NL80211_BAND_5GHZ; else band = NL80211_BAND_6GHZ; @@ -3591,7 +3591,7 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw, /* Since the targeted scan device could depend on the frequency * requested in the hw_req, select the corresponding radio */ - ar = ath12k_mac_select_scan_device(hw, vif, hw_req); + ar = ath12k_mac_select_scan_device(hw, vif, hw_req->req.channels[0]->center_freq); if (!ar) return -EINVAL; @@ -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; + 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); + } 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; + } + } +scan: mutex_lock(&ar->conf_mutex); spin_lock_bh(&ar->data_lock);
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@quicinc.com> --- drivers/net/wireless/ath/ath12k/mac.c | 63 ++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 6 deletions(-) base-commit: f8320064a28242448eeb9fece08abd865ea8a226