| Message ID | 20240320190943.3850106-1-quic_ramess@quicinc.com |
|---|---|
| Headers | show |
| Series | wifi: ath12k: Add single wiphy support | expand |
On 3/20/2024 12:09 PM, Rameshkumar Sundaram wrote: > From: Sriram R <quic_srirrama@quicinc.com> > > With single wiphy, multiple radios are combined into a single wiphy. > Since any channel can be assigned to a vif being brought up, > the vdev cannot be created during add_interface(). Hence defer the > vdev creation till channel assignment. > > If only one radio is part of the wiphy, then the existing logic > is maintained, i.e vdevs are created during add interface and > started during channel assignment. This ensures no functional changes > to single pdev devices which has only one radio in the SoC. > > 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> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
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; > }
On 3/20/2024 12:09 PM, Rameshkumar Sundaram wrote: > From: Sriram R <quic_srirrama@quicinc.com> > > The radio for which the survey info needs to be collected > depends on the channel idx which could be based on the band. > Use the idx to identify the appropriate sband since multiple > bands could be combined for single wiphy case. > > Also use the channel idx and sband to identify the corresponding > radio on which the survey results needs to be populated. > > 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> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
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; >> } >
On 3/25/2024 8:24 AM, Rameshkumar Sundaram wrote: > 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> ... >>> 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. then if this function is only ever used to delete a scan vdev, then shouldn't the name be changed to reflect that? the current generic name doesn't reflect that specificity. /jeff
On 3/25/2024 9:03 PM, Jeff Johnson wrote: > On 3/25/2024 8:24 AM, Rameshkumar Sundaram wrote: >> 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> > ... >>>> 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. > > then if this function is only ever used to delete a scan vdev, then shouldn't > the name be changed to reflect that? the current generic name doesn't reflect > that specificity. > Ah Sorry, i misunderstood your point earlier, as i mentioned vdev is created for an arvif either by ath12k_mac_op_add_intf/assign_chanctx/hw_scan). So this vdev_delete can never say if its a scan vdev. For some reason it was put as scan vdev delete in print message :( . We should remove this change. Earlier I thought you were asking why vdev delete is being called from both places and was trying explaining the logic behind. Totally missed the print message, Really sorry about that.
On 3/21/2024 2:30 AM, Jeff Johnson wrote: > On 3/20/2024 12:09 PM, Rameshkumar Sundaram wrote: >> From: Sriram R <quic_srirrama@quicinc.com> >> >> With the introduction of Multi Link Operation (MLO) support in >> IEEE802.11be, each EHT AP/non AP interface is capable of >> operating with multiple radio links. >> >> cfg80211/mac80211 expects drivers to abstract the communication >> between such Multi Link HW and mac80211/cfg80211 since it depends >> on different driver/HW implementation. Hence the single wiphy >> abstraction with changes in datastructures were introduced in >> "wifi: ath12k: Introduce hw abstraction" >> >> This patchset extends the implementation to allow combination >> of multiple underlying radios into a single composite hw/wiphy >> for registration. Since now multiple radios are represented by >> a single wiphy, changes are required in various mac ops that the >> driver supports since the driver now needs to learn on how to tunnel >> various mac ops properly to a specific radio. >> >> This patchset covers the basic mac80211 ops for an interface bring up >> and operation. >> >> Note: >> Monitor and hw reconfig support for Single Wiphy will be done in future >> patchsets. >> >> --- >> v5: >> - Addressed Jeff's comments >> - Made arvif config cache to be dynamic in PATCH 07/12 > > In the future please be specific about what patches in the series were > modified so that reviewers don't have to spend time reviewing the parts that > didn't change. > Sure will follow this for future patches.
From: Sriram R <quic_srirrama@quicinc.com> With the introduction of Multi Link Operation (MLO) support in IEEE802.11be, each EHT AP/non AP interface is capable of operating with multiple radio links. cfg80211/mac80211 expects drivers to abstract the communication between such Multi Link HW and mac80211/cfg80211 since it depends on different driver/HW implementation. Hence the single wiphy abstraction with changes in datastructures were introduced in "wifi: ath12k: Introduce hw abstraction" This patchset extends the implementation to allow combination of multiple underlying radios into a single composite hw/wiphy for registration. Since now multiple radios are represented by a single wiphy, changes are required in various mac ops that the driver supports since the driver now needs to learn on how to tunnel various mac ops properly to a specific radio. This patchset covers the basic mac80211 ops for an interface bring up and operation. Note: Monitor and hw reconfig support for Single Wiphy will be done in future patchsets. --- v5: - Addressed Jeff's comments - Made arvif config cache to be dynamic in PATCH 07/12 v4: - Updated missing Signed-off details for patches. v3: - Rebased on ToT (added additional ar check in PATCH 08/12 for p2p) - Introduced iterator to loop through ars in an ah(for_each_ar()) - Addressed comments on reverse xmas tree declaration style. v2: - Rebased on ToT and dependent patchset Karthikeyan Periyasamy (1): wifi: ath12k: add multiple radio support in a single MAC HW un/register Sriram R (11): wifi: ath12k: Modify add and remove chanctx ops for single wiphy support wifi: ath12k: modify ath12k mac start/stop ops for single wiphy wifi: ath12k: vdev statemachine changes for single wiphy wifi: ath12k: scan statemachine changes for single wiphy wifi: ath12k: fetch correct radio based on vdev status wifi: ath12k: Cache vdev configs before vdev create wifi: ath12k: Add additional checks for vif and sta iterators wifi: ath12k: modify regulatory support for single wiphy architecture wifi: ath12k: Modify set and get antenna mac ops for single wiphy wifi: ath12k: Modify rts threshold mac op for single wiphy wifi: ath12k: support get_survey mac op for single wiphy drivers/net/wireless/ath/ath12k/core.h | 38 +- drivers/net/wireless/ath/ath12k/hw.h | 1 + drivers/net/wireless/ath/ath12k/mac.c | 946 +++++++++++++++++++------ drivers/net/wireless/ath/ath12k/p2p.c | 3 +- drivers/net/wireless/ath/ath12k/p2p.h | 1 + drivers/net/wireless/ath/ath12k/reg.c | 55 +- 6 files changed, 815 insertions(+), 229 deletions(-) base-commit: 4b2f0ce6f2fe0fd906d408a01e494b85c272c7d7