2024-05-28 08:28:10

by Rameshkumar Sundaram

[permalink] [raw]
Subject: [PATCH] wifi: ath12k: modify remain on channel for single wiphy

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 <[email protected]>
---
drivers/net/wireless/ath/ath12k/mac.c | 63 ++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 6 deletions(-)

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);


base-commit: f8320064a28242448eeb9fece08abd865ea8a226
--
2.25.1



2024-05-29 14:41:37

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] wifi: ath12k: modify remain on channel for single wiphy

On 5/28/2024 1:27 AM, Rameshkumar Sundaram 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 <[email protected]>
Acked-by: Jeff Johnson <[email protected]>


2024-06-10 14:23:29

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wifi: ath12k: modify remain on channel for single wiphy

Rameshkumar Sundaram <[email protected]> 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 <[email protected]>

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

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2024-06-11 06:46:36

by Rameshkumar Sundaram

[permalink] [raw]
Subject: Re: [PATCH] wifi: ath12k: modify remain on channel for single wiphy



On 6/10/2024 7:26 PM, Kalle Valo wrote:
> Rameshkumar Sundaram <[email protected]> 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 <[email protected]>
>
> 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.

2024-06-11 18:35:19

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wifi: ath12k: modify remain on channel for single wiphy

Rameshkumar Sundaram <[email protected]> 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 <[email protected]>
> Acked-by: Jeff Johnson <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

4938ba733ee2 wifi: ath12k: modify remain on channel for single wiphy

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches