Currently, the driver does not support single/multi link operation in 11be
mode. Also, each link/radio gets registered as a separate wiphy through
mac80211. In order to support multi link operation (MLO), the current
separate wiphy registration approach brings a lot of complexity in cfg80211
and mac80211, such as synchronization across the multiple wiphy/hw.
Determining the compatibility of the hw across multiple wiphy/hw is another
challenge for userspace. To reduce these complexities in
userspace/cfg80211/mac80211, need to move from the multi wiphy model to a
single wiphy model. To support the single wiphy registration, we have to
decouple the wiphy/mac80211 hw data from the link/radio (ar) structure. So
refactor the MAC helper functions.
Current Multi wiphy Model
+---------------+ +---------------+ +-------------+
| Mac80211 hw | | Mac80211 hw | |Mac80211 hw |
| private data | | private data | |private data |
| | | | | |
| | | | | |
| | | | | |
| ar (2GHz) | | ar (5GHz) | | ar (6GHz) |
| | | | | |
| | | | | |
| | | | | |
+---------------+ +---------------+ +-------------+
Single wiphy Model
+--------------+
| Mac80211 hw |
| private data |
| |
|ath12k hw (ah)|
| +----------+ |
| |ar (2GHz) | |
| +----------+ |
| | | |
| |ar (5GHz) | |
| +----------+ |
| | | |
| |ar (6GHz) | |
| | | |
| +----------+ |
+--------------+
Karthikeyan Periyasamy (4):
wifi: ath12k: Refactor the DP pdev pre alloc call sequence
wifi: ath12k: Refactor the MAC allocation and destroy
wifi: ath12k: Refactor MAC setup channel helper function
wifi: ath12k: Refactor MAC un/register helper function
drivers/net/wireless/ath/ath12k/core.c | 96 +++++++++--
drivers/net/wireless/ath/ath12k/mac.c | 216 +++++++++++--------------
drivers/net/wireless/ath/ath12k/mac.h | 8 +-
3 files changed, 184 insertions(+), 136 deletions(-)
base-commit: 22d737065b8c4fbb29a3a818adcf88004ea7d5bb
--
2.34.1
Currently, the MAC setup helper function is accessing the mac80211 hw
data. In the future, to support single/multi link operation, need to
decouple the mac80211 hw data from this helper function so that it can be
easy to scale these functions to support single/multi link operations. So
remove the mac80211 hw access from the ath12k_mac_setup_channels_rates()
helper function.
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
---
drivers/net/wireless/ath/ath12k/mac.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index a110119ad701..be3ef388e9aa 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7158,9 +7158,9 @@ static u32 ath12k_get_phy_id(struct ath12k *ar, u32 band)
}
static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
- u32 supported_bands)
+ u32 supported_bands,
+ struct ieee80211_supported_band *bands[])
{
- struct ieee80211_hw *hw = ar->hw;
struct ieee80211_supported_band *band;
struct ath12k_wmi_hal_reg_capabilities_ext_arg *reg_cap;
void *channels;
@@ -7186,7 +7186,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
band->channels = channels;
band->n_bitrates = ath12k_g_rates_size;
band->bitrates = ath12k_g_rates;
- hw->wiphy->bands[NL80211_BAND_2GHZ] = band;
+ bands[NL80211_BAND_2GHZ] = band;
if (ar->ab->hw_params->single_pdev_only) {
phy_id = ath12k_get_phy_id(ar, WMI_HOST_WLAN_2G_CAP);
@@ -7213,7 +7213,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
band->channels = channels;
band->n_bitrates = ath12k_a_rates_size;
band->bitrates = ath12k_a_rates;
- hw->wiphy->bands[NL80211_BAND_6GHZ] = band;
+ bands[NL80211_BAND_6GHZ] = band;
ath12k_mac_update_ch_list(ar, band,
reg_cap->low_5ghz_chan,
reg_cap->high_5ghz_chan);
@@ -7235,7 +7235,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
band->channels = channels;
band->n_bitrates = ath12k_a_rates_size;
band->bitrates = ath12k_a_rates;
- hw->wiphy->bands[NL80211_BAND_5GHZ] = band;
+ bands[NL80211_BAND_5GHZ] = band;
if (ar->ab->hw_params->single_pdev_only) {
phy_id = ath12k_get_phy_id(ar, WMI_HOST_WLAN_5G_CAP);
@@ -7414,7 +7414,8 @@ static int __ath12k_mac_register(struct ath12k *ar)
SET_IEEE80211_DEV(hw, ab->dev);
ret = ath12k_mac_setup_channels_rates(ar,
- cap->supported_bands);
+ cap->supported_bands,
+ hw->wiphy->bands);
if (ret)
goto err;
--
2.34.1
Currently, the MAC allocation and destroy helper functions are tightly
coupled with the link/radio (ar) structure. In the future, to support
single/Multi link operations, need to refactor these helper functions
across the core and mac sub modules, so that it can be easy to scale
these functions to support single/Multi link operations.
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 33 ++++++++--
drivers/net/wireless/ath/ath12k/mac.c | 83 ++++++++++++++------------
drivers/net/wireless/ath/ath12k/mac.h | 4 +-
3 files changed, 73 insertions(+), 47 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 6f634b57dde8..e10c5f2cd8eb 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -529,6 +529,27 @@ static void ath12k_core_pdev_destroy(struct ath12k_base *ab)
ath12k_dp_pdev_free(ab);
}
+static void ath12k_core_mac_destroy(struct ath12k_base *ab)
+{
+ ath12k_mac_hw_destroy(ab);
+}
+
+static int ath12k_core_mac_allocate(struct ath12k_base *ab)
+{
+ int ret;
+
+ if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
+ return 0;
+
+ ret = ath12k_mac_hw_allocate(ab);
+ if (ret)
+ return ret;
+
+ ath12k_dp_pdev_pre_alloc(ab);
+
+ return 0;
+}
+
static int ath12k_core_start(struct ath12k_base *ab,
enum ath12k_firmware_mode mode)
{
@@ -583,7 +604,7 @@ static int ath12k_core_start(struct ath12k_base *ab,
goto err_hif_stop;
}
- ret = ath12k_mac_allocate(ab);
+ ret = ath12k_core_mac_allocate(ab);
if (ret) {
ath12k_err(ab, "failed to create new hw device with mac80211 :%d\n",
ret);
@@ -595,7 +616,7 @@ static int ath12k_core_start(struct ath12k_base *ab,
ret = ath12k_dp_rx_pdev_reo_setup(ab);
if (ret) {
ath12k_err(ab, "failed to initialize reo destination rings: %d\n", ret);
- goto err_mac_destroy;
+ goto err_core_mac_destroy;
}
ret = ath12k_wmi_cmd_init(ab);
@@ -631,8 +652,8 @@ static int ath12k_core_start(struct ath12k_base *ab,
err_reo_cleanup:
ath12k_dp_rx_pdev_reo_cleanup(ab);
-err_mac_destroy:
- ath12k_mac_destroy(ab);
+err_core_mac_destroy:
+ ath12k_core_mac_destroy(ab);
err_hif_stop:
ath12k_hif_stop(ab);
err_wmi_detach:
@@ -707,7 +728,7 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab)
ath12k_core_pdev_destroy(ab);
err_core_stop:
ath12k_core_stop(ab);
- ath12k_mac_destroy(ab);
+ ath12k_core_mac_destroy(ab);
err_dp_free:
ath12k_dp_free(ab);
mutex_unlock(&ab->core_lock);
@@ -1003,7 +1024,7 @@ void ath12k_core_deinit(struct ath12k_base *ab)
mutex_unlock(&ab->core_lock);
ath12k_hif_power_down(ab);
- ath12k_mac_destroy(ab);
+ ath12k_core_mac_destroy(ab);
ath12k_core_soc_destroy(ab);
}
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 49d56f5d8896..a110119ad701 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7607,7 +7607,47 @@ int ath12k_mac_register(struct ath12k_base *ab)
return ret;
}
-int ath12k_mac_allocate(struct ath12k_base *ab)
+static void ath12k_mac_setup(struct ath12k *ar)
+{
+ struct ath12k_base *ab = ar->ab;
+ struct ath12k_pdev *pdev = ar->pdev;
+ u8 pdev_idx = ar->pdev_idx;
+
+ ar->lmac_id = ath12k_hw_get_mac_from_pdev_id(ab->hw_params, pdev_idx);
+
+ ar->wmi = &ab->wmi_ab.wmi[pdev_idx];
+ /* FIXME: wmi[0] is already initialized during attach,
+ * Should we do this again?
+ */
+ ath12k_wmi_pdev_attach(ab, pdev_idx);
+
+ ar->cfg_tx_chainmask = pdev->cap.tx_chain_mask;
+ ar->cfg_rx_chainmask = pdev->cap.rx_chain_mask;
+ ar->num_tx_chains = hweight32(pdev->cap.tx_chain_mask);
+ ar->num_rx_chains = hweight32(pdev->cap.rx_chain_mask);
+
+ spin_lock_init(&ar->data_lock);
+ INIT_LIST_HEAD(&ar->arvifs);
+ INIT_LIST_HEAD(&ar->ppdu_stats_info);
+ mutex_init(&ar->conf_mutex);
+ init_completion(&ar->vdev_setup_done);
+ init_completion(&ar->vdev_delete_done);
+ init_completion(&ar->peer_assoc_done);
+ init_completion(&ar->peer_delete_done);
+ init_completion(&ar->install_key_done);
+ init_completion(&ar->bss_survey_done);
+ init_completion(&ar->scan.started);
+ init_completion(&ar->scan.completed);
+
+ INIT_DELAYED_WORK(&ar->scan.timeout, ath12k_scan_timeout_work);
+ INIT_WORK(&ar->regd_update_work, ath12k_regd_update_work);
+
+ INIT_WORK(&ar->wmi_mgmt_tx_work, ath12k_mgmt_over_wmi_tx_work);
+ skb_queue_head_init(&ar->wmi_mgmt_tx_queue);
+ clear_bit(ATH12K_FLAG_MONITOR_ENABLED, &ar->monitor_flags);
+}
+
+int ath12k_mac_hw_allocate(struct ath12k_base *ab)
{
struct ieee80211_hw *hw;
struct ath12k *ar;
@@ -7615,9 +7655,6 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
int ret;
int i;
- if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
- return 0;
-
for (i = 0; i < ab->num_radios; i++) {
pdev = &ab->pdevs[i];
hw = ieee80211_alloc_hw(sizeof(struct ath12k), &ath12k_ops);
@@ -7632,52 +7669,20 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
ar->ab = ab;
ar->pdev = pdev;
ar->pdev_idx = i;
- ar->lmac_id = ath12k_hw_get_mac_from_pdev_id(ab->hw_params, i);
-
- ar->wmi = &ab->wmi_ab.wmi[i];
- /* FIXME: wmi[0] is already initialized during attach,
- * Should we do this again?
- */
- ath12k_wmi_pdev_attach(ab, i);
-
- ar->cfg_tx_chainmask = pdev->cap.tx_chain_mask;
- ar->cfg_rx_chainmask = pdev->cap.rx_chain_mask;
- ar->num_tx_chains = hweight32(pdev->cap.tx_chain_mask);
- ar->num_rx_chains = hweight32(pdev->cap.rx_chain_mask);
-
pdev->ar = ar;
- spin_lock_init(&ar->data_lock);
- INIT_LIST_HEAD(&ar->arvifs);
- INIT_LIST_HEAD(&ar->ppdu_stats_info);
- mutex_init(&ar->conf_mutex);
- init_completion(&ar->vdev_setup_done);
- init_completion(&ar->vdev_delete_done);
- init_completion(&ar->peer_assoc_done);
- init_completion(&ar->peer_delete_done);
- init_completion(&ar->install_key_done);
- init_completion(&ar->bss_survey_done);
- init_completion(&ar->scan.started);
- init_completion(&ar->scan.completed);
- INIT_DELAYED_WORK(&ar->scan.timeout, ath12k_scan_timeout_work);
- INIT_WORK(&ar->regd_update_work, ath12k_regd_update_work);
-
- INIT_WORK(&ar->wmi_mgmt_tx_work, ath12k_mgmt_over_wmi_tx_work);
- skb_queue_head_init(&ar->wmi_mgmt_tx_queue);
- clear_bit(ATH12K_FLAG_MONITOR_ENABLED, &ar->monitor_flags);
+ ath12k_mac_setup(ar);
}
- ath12k_dp_pdev_pre_alloc(ab);
-
return 0;
err_free_mac:
- ath12k_mac_destroy(ab);
+ ath12k_mac_hw_destroy(ab);
return ret;
}
-void ath12k_mac_destroy(struct ath12k_base *ab)
+void ath12k_mac_hw_destroy(struct ath12k_base *ab)
{
struct ath12k *ar;
struct ath12k_pdev *pdev;
diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
index 7c63bb628adc..fb4c0662581b 100644
--- a/drivers/net/wireless/ath/ath12k/mac.h
+++ b/drivers/net/wireless/ath/ath12k/mac.h
@@ -48,10 +48,10 @@ enum ath12k_supported_bw {
extern const struct htt_rx_ring_tlv_filter ath12k_mac_mon_status_filter_default;
-void ath12k_mac_destroy(struct ath12k_base *ab);
+void ath12k_mac_hw_destroy(struct ath12k_base *ab);
void ath12k_mac_unregister(struct ath12k_base *ab);
int ath12k_mac_register(struct ath12k_base *ab);
-int ath12k_mac_allocate(struct ath12k_base *ab);
+int ath12k_mac_hw_allocate(struct ath12k_base *ab);
int ath12k_mac_hw_ratecode_to_legacy_rate(u8 hw_rc, u8 preamble, u8 *rateidx,
u16 *rate);
u8 ath12k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
--
2.34.1
Currently, the mac80211 hw registration procedure is tightly coupled with
the handling of link/radio (ar). Define a new helper function to separate
the link/radio handling from the mac80211 hw registration procedure for
improved code readability. Also, it can be easy to scale these
functionality to support single/multi link operation in the future.
Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 61 +++++++++++-
drivers/net/wireless/ath/ath12k/mac.c | 132 ++++++++++---------------
drivers/net/wireless/ath/ath12k/mac.h | 4 +-
3 files changed, 109 insertions(+), 88 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index e10c5f2cd8eb..d1ac00c59b8c 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -498,11 +498,62 @@ static void ath12k_core_soc_destroy(struct ath12k_base *ab)
ath12k_qmi_deinit_service(ab);
}
+static int ath12k_core_mac_register(struct ath12k_base *ab)
+{
+ struct ath12k *ar;
+ struct ath12k_pdev *pdev;
+ int i;
+ int ret;
+
+ if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
+ return 0;
+
+ /* Initialize channel counters frequency value in hertz */
+ ab->cc_freq_hz = 320000;
+ ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
+
+ for (i = 0; i < ab->num_radios; i++) {
+ pdev = &ab->pdevs[i];
+ ar = pdev->ar;
+
+ ret = ath12k_mac_hw_register(ar);
+ if (ret)
+ goto err_cleanup;
+ }
+
+ return 0;
+
+err_cleanup:
+ for (i = i - 1; i >= 0; i--) {
+ pdev = &ab->pdevs[i];
+ ar = pdev->ar;
+ ath12k_mac_hw_unregister(ar);
+ }
+
+ return ret;
+}
+
+static void ath12k_core_mac_unregister(struct ath12k_base *ab)
+{
+ struct ath12k *ar;
+ struct ath12k_pdev *pdev;
+ int i;
+
+ for (i = 0; i < ab->num_radios; i++) {
+ pdev = &ab->pdevs[i];
+ ar = pdev->ar;
+ if (!ar)
+ continue;
+
+ ath12k_mac_hw_unregister(ar);
+ }
+}
+
static int ath12k_core_pdev_create(struct ath12k_base *ab)
{
int ret;
- ret = ath12k_mac_register(ab);
+ ret = ath12k_core_mac_register(ab);
if (ret) {
ath12k_err(ab, "failed register the radio with mac80211: %d\n", ret);
return ret;
@@ -511,20 +562,20 @@ static int ath12k_core_pdev_create(struct ath12k_base *ab)
ret = ath12k_dp_pdev_alloc(ab);
if (ret) {
ath12k_err(ab, "failed to attach DP pdev: %d\n", ret);
- goto err_mac_unregister;
+ goto err_core_mac_unregister;
}
return 0;
-err_mac_unregister:
- ath12k_mac_unregister(ab);
+err_core_mac_unregister:
+ ath12k_core_mac_unregister(ab);
return ret;
}
static void ath12k_core_pdev_destroy(struct ath12k_base *ab)
{
- ath12k_mac_unregister(ab);
+ ath12k_core_mac_unregister(ab);
ath12k_hif_irq_disable(ab);
ath12k_dp_pdev_free(ab);
}
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index be3ef388e9aa..27f6067fd3fc 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7349,7 +7349,17 @@ static const struct wiphy_iftype_ext_capab ath12k_iftypes_ext_capa[] = {
},
};
-static void __ath12k_mac_unregister(struct ath12k *ar)
+static void ath12k_mac_cleanup_unregister(struct ath12k *ar)
+{
+ idr_for_each(&ar->txmgmt_idr, ath12k_mac_tx_mgmt_pending_free, ar);
+ idr_destroy(&ar->txmgmt_idr);
+
+ kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
+ kfree(ar->mac.sbands[NL80211_BAND_5GHZ].channels);
+ kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
+}
+
+void ath12k_mac_hw_unregister(struct ath12k *ar)
{
struct ieee80211_hw *hw = ar->hw;
struct wiphy *wiphy = hw->wiphy;
@@ -7358,12 +7368,7 @@ static void __ath12k_mac_unregister(struct ath12k *ar)
ieee80211_unregister_hw(hw);
- idr_for_each(&ar->txmgmt_idr, ath12k_mac_tx_mgmt_pending_free, ar);
- idr_destroy(&ar->txmgmt_idr);
-
- kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
- kfree(ar->mac.sbands[NL80211_BAND_5GHZ].channels);
- kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
+ ath12k_mac_cleanup_unregister(ar);
kfree(wiphy->iface_combinations[0].limits);
kfree(wiphy->iface_combinations);
@@ -7371,28 +7376,41 @@ static void __ath12k_mac_unregister(struct ath12k *ar)
SET_IEEE80211_DEV(hw, NULL);
}
-void ath12k_mac_unregister(struct ath12k_base *ab)
+static int ath12k_mac_setup_register(struct ath12k *ar,
+ u32 *ht_cap,
+ struct ieee80211_supported_band *bands[])
{
- struct ath12k *ar;
- struct ath12k_pdev *pdev;
- int i;
+ struct ath12k_pdev_cap *cap = &ar->pdev->cap;
+ int ret;
- for (i = 0; i < ab->num_radios; i++) {
- pdev = &ab->pdevs[i];
- ar = pdev->ar;
- if (!ar)
- continue;
+ init_waitqueue_head(&ar->txmgmt_empty_waitq);
+ idr_init(&ar->txmgmt_idr);
+ spin_lock_init(&ar->txmgmt_idr_lock);
- __ath12k_mac_unregister(ar);
- }
+ ath12k_pdev_caps_update(ar);
+
+ ret = ath12k_mac_setup_channels_rates(ar,
+ cap->supported_bands,
+ bands);
+ if (ret)
+ return ret;
+
+ ath12k_mac_setup_ht_vht_cap(ar, cap, ht_cap);
+ ath12k_mac_setup_sband_iftype_data(ar, cap);
+
+ ar->max_num_stations = TARGET_NUM_STATIONS;
+ ar->max_num_peers = TARGET_NUM_PEERS_PDEV;
+
+ return 0;
}
-static int __ath12k_mac_register(struct ath12k *ar)
+int ath12k_mac_hw_register(struct ath12k *ar)
{
struct ath12k_base *ab = ar->ab;
struct ieee80211_hw *hw = ar->hw;
struct wiphy *wiphy = hw->wiphy;
- struct ath12k_pdev_cap *cap = &ar->pdev->cap;
+ struct ath12k_pdev *pdev = ar->pdev;
+ struct ath12k_pdev_cap *cap = &pdev->cap;
static const u32 cipher_suites[] = {
WLAN_CIPHER_SUITE_TKIP,
WLAN_CIPHER_SUITE_CCMP,
@@ -7407,25 +7425,24 @@ static int __ath12k_mac_register(struct ath12k *ar)
int ret;
u32 ht_cap = 0;
- ath12k_pdev_caps_update(ar);
-
- SET_IEEE80211_PERM_ADDR(hw, ar->mac_addr);
-
- SET_IEEE80211_DEV(hw, ab->dev);
+ if (ab->pdevs_macaddr_valid) {
+ ether_addr_copy(ar->mac_addr, pdev->mac_addr);
+ } else {
+ ether_addr_copy(ar->mac_addr, ab->mac_addr);
+ ar->mac_addr[4] += ar->pdev_idx;
+ }
- ret = ath12k_mac_setup_channels_rates(ar,
- cap->supported_bands,
- hw->wiphy->bands);
+ ret = ath12k_mac_setup_register(ar, &ht_cap, hw->wiphy->bands);
if (ret)
- goto err;
+ goto out;
- ath12k_mac_setup_ht_vht_cap(ar, cap, &ht_cap);
- ath12k_mac_setup_sband_iftype_data(ar, cap);
+ SET_IEEE80211_PERM_ADDR(hw, ar->mac_addr);
+ SET_IEEE80211_DEV(hw, ab->dev);
ret = ath12k_mac_setup_iface_combinations(ar);
if (ret) {
ath12k_err(ar->ab, "failed to setup interface combinations: %d\n", ret);
- goto err_free_channels;
+ goto err_setup_unregister;
}
wiphy->available_antennas_rx = cap->rx_chain_mask;
@@ -7484,9 +7501,6 @@ static int __ath12k_mac_register(struct ath12k *ar)
wiphy->features |= NL80211_FEATURE_AP_MODE_CHAN_WIDTH_CHANGE |
NL80211_FEATURE_AP_SCAN;
- ar->max_num_stations = TARGET_NUM_STATIONS;
- ar->max_num_peers = TARGET_NUM_PEERS_PDEV;
-
wiphy->max_ap_assoc_sta = ar->max_num_stations;
hw->queues = ATH12K_HW_MAX_QUEUES;
@@ -7553,58 +7567,14 @@ static int __ath12k_mac_register(struct ath12k *ar)
kfree(wiphy->iface_combinations[0].limits);
kfree(wiphy->iface_combinations);
-err_free_channels:
+err_setup_unregister:
kfree(ar->mac.sbands[NL80211_BAND_2GHZ].channels);
kfree(ar->mac.sbands[NL80211_BAND_5GHZ].channels);
kfree(ar->mac.sbands[NL80211_BAND_6GHZ].channels);
-err:
SET_IEEE80211_DEV(hw, NULL);
- return ret;
-}
-
-int ath12k_mac_register(struct ath12k_base *ab)
-{
- struct ath12k *ar;
- struct ath12k_pdev *pdev;
- int i;
- int ret;
-
- if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
- return 0;
-
- for (i = 0; i < ab->num_radios; i++) {
- pdev = &ab->pdevs[i];
- ar = pdev->ar;
- if (ab->pdevs_macaddr_valid) {
- ether_addr_copy(ar->mac_addr, pdev->mac_addr);
- } else {
- ether_addr_copy(ar->mac_addr, ab->mac_addr);
- ar->mac_addr[4] += i;
- }
-
- ret = __ath12k_mac_register(ar);
- if (ret)
- goto err_cleanup;
-
- init_waitqueue_head(&ar->txmgmt_empty_waitq);
- idr_init(&ar->txmgmt_idr);
- spin_lock_init(&ar->txmgmt_idr_lock);
- }
-
- /* Initialize channel counters frequency value in hertz */
- ab->cc_freq_hz = 320000;
- ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
-
- return 0;
-
-err_cleanup:
- for (i = i - 1; i >= 0; i--) {
- pdev = &ab->pdevs[i];
- ar = pdev->ar;
- __ath12k_mac_unregister(ar);
- }
+out:
return ret;
}
diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
index fb4c0662581b..c30ebda77b0a 100644
--- a/drivers/net/wireless/ath/ath12k/mac.h
+++ b/drivers/net/wireless/ath/ath12k/mac.h
@@ -49,8 +49,8 @@ enum ath12k_supported_bw {
extern const struct htt_rx_ring_tlv_filter ath12k_mac_mon_status_filter_default;
void ath12k_mac_hw_destroy(struct ath12k_base *ab);
-void ath12k_mac_unregister(struct ath12k_base *ab);
-int ath12k_mac_register(struct ath12k_base *ab);
+void ath12k_mac_hw_unregister(struct ath12k *ar);
+int ath12k_mac_hw_register(struct ath12k *ar);
int ath12k_mac_hw_allocate(struct ath12k_base *ab);
int ath12k_mac_hw_ratecode_to_legacy_rate(u8 hw_rc, u8 preamble, u8 *rateidx,
u16 *rate);
--
2.34.1
On 12/5/2023 7:49 PM, Karthikeyan Periyasamy wrote:
> Currently, the MAC allocation and destroy helper functions are tightly
> coupled with the link/radio (ar) structure. In the future, to support
> single/Multi link operations, need to refactor these helper functions
> across the core and mac sub modules, so that it can be easy to scale
> these functions to support single/Multi link operations.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Acked-by: Jeff Johnson <[email protected]>
On 12/5/2023 7:49 PM, Karthikeyan Periyasamy wrote:
> Currently, the MAC setup helper function is accessing the mac80211 hw
> data. In the future, to support single/multi link operation, need to
> decouple the mac80211 hw data from this helper function so that it can be
> easy to scale these functions to support single/multi link operations. So
> remove the mac80211 hw access from the ath12k_mac_setup_channels_rates()
> helper function.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Acked-by: Jeff Johnson <[email protected]>
On 12/5/2023 7:49 PM, Karthikeyan Periyasamy wrote:
> Currently, the mac80211 hw registration procedure is tightly coupled with
> the handling of link/radio (ar). Define a new helper function to separate
> the link/radio handling from the mac80211 hw registration procedure for
> improved code readability. Also, it can be easy to scale these
> functionality to support single/multi link operation in the future.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Acked-by: Jeff Johnson <[email protected]>
Karthikeyan Periyasamy <[email protected]> writes:
> Currently, the mac80211 hw registration procedure is tightly coupled with
> the handling of link/radio (ar). Define a new helper function to separate
> the link/radio handling from the mac80211 hw registration procedure for
> improved code readability. Also, it can be easy to scale these
> functionality to support single/multi link operation in the future.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> ---
> drivers/net/wireless/ath/ath12k/core.c | 61 +++++++++++-
> drivers/net/wireless/ath/ath12k/mac.c | 132 ++++++++++---------------
> drivers/net/wireless/ath/ath12k/mac.h | 4 +-
> 3 files changed, 109 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index e10c5f2cd8eb..d1ac00c59b8c 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -498,11 +498,62 @@ static void ath12k_core_soc_destroy(struct ath12k_base *ab)
> ath12k_qmi_deinit_service(ab);
> }
>
> +static int ath12k_core_mac_register(struct ath12k_base *ab)
> +{
> + struct ath12k *ar;
> + struct ath12k_pdev *pdev;
> + int i;
> + int ret;
> +
> + if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
> + return 0;
> +
> + /* Initialize channel counters frequency value in hertz */
> + ab->cc_freq_hz = 320000;
> + ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
> +
> + for (i = 0; i < ab->num_radios; i++) {
> + pdev = &ab->pdevs[i];
> + ar = pdev->ar;
> +
> + ret = ath12k_mac_hw_register(ar);
> + if (ret)
> + goto err_cleanup;
> + }
> +
> + return 0;
> +
> +err_cleanup:
> + for (i = i - 1; i >= 0; i--) {
> + pdev = &ab->pdevs[i];
> + ar = pdev->ar;
> + ath12k_mac_hw_unregister(ar);
> + }
> +
> + return ret;
> +}
Is there a reason why you moved these two functions from mac.c to
core.c? This is mac level code anyway, right?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 1/9/2024 6:55 PM, Kalle Valo wrote:
> Karthikeyan Periyasamy <[email protected]> writes:
>
>> Currently, the mac80211 hw registration procedure is tightly coupled with
>> the handling of link/radio (ar). Define a new helper function to separate
>> the link/radio handling from the mac80211 hw registration procedure for
>> improved code readability. Also, it can be easy to scale these
>> functionality to support single/multi link operation in the future.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath12k/core.c | 61 +++++++++++-
>> drivers/net/wireless/ath/ath12k/mac.c | 132 ++++++++++---------------
>> drivers/net/wireless/ath/ath12k/mac.h | 4 +-
>> 3 files changed, 109 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
>> index e10c5f2cd8eb..d1ac00c59b8c 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>> @@ -498,11 +498,62 @@ static void ath12k_core_soc_destroy(struct ath12k_base *ab)
>> ath12k_qmi_deinit_service(ab);
>> }
>>
>> +static int ath12k_core_mac_register(struct ath12k_base *ab)
>> +{
>> + struct ath12k *ar;
>> + struct ath12k_pdev *pdev;
>> + int i;
>> + int ret;
>> +
>> + if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
>> + return 0;
>> +
>> + /* Initialize channel counters frequency value in hertz */
>> + ab->cc_freq_hz = 320000;
>> + ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
>> +
>> + for (i = 0; i < ab->num_radios; i++) {
>> + pdev = &ab->pdevs[i];
>> + ar = pdev->ar;
>> +
>> + ret = ath12k_mac_hw_register(ar);
>> + if (ret)
>> + goto err_cleanup;
>> + }
>> +
>> + return 0;
>> +
>> +err_cleanup:
>> + for (i = i - 1; i >= 0; i--) {
>> + pdev = &ab->pdevs[i];
>> + ar = pdev->ar;
>> + ath12k_mac_hw_unregister(ar);
>> + }
>> +
>> + return ret;
>> +}
> Is there a reason why you moved these two functions from mac.c to
> core.c? This is mac level code anyway, right?
This is not fully mac level code, some parts of SoC/chip level code
bindup in the mac level.
So i segregated the SoC level code like ab related param initialization
handling from the mac level procedure.
Now, mac/radio should take care only radio level handling procedure.
In future for MLO, SoC level can be extend easily.
Thanks,
Karthikeyan P.
Karthikeyan Periyasamy <[email protected]> writes:
>> Is there a reason why you moved these two functions from mac.c to
>> core.c? This is mac level code anyway, right?
>
> This is not fully mac level code, some parts of SoC/chip level code
> bindup in the mac level. So i segregated the SoC level code like ab
> related param initialization handling from the mac level procedure.
>
> Now, mac/radio should take care only radio level handling procedure.
>
> In future for MLO, SoC level can be extend easily.
But is there a concrete reason to have the functions in core.c? In your
following patches I couldn't see why to move these functions to core.c,
everything seems to be suitable for mac.c.
I experimented this in the pending branch and moved the functions to
mac.c (tag ath-pending-202401151424):
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bee89ac2b5755754849deaf7054828e982cc6658
I also fixed your other patchsets to match that and to me it makes more
sense to have everything in mac.c. I prefer to make core.c as simple as possible.
As you can see I also made changes to the patch titles to make them more
understandable:
wifi: ath12k: refactor ath12k_mac_register() and ath12k_mac_unregister()
Thoughts?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 1/15/2024 8:57 PM, Kalle Valo wrote:
> Karthikeyan Periyasamy <[email protected]> writes:
>
>>> Is there a reason why you moved these two functions from mac.c to
>>> core.c? This is mac level code anyway, right?
>> This is not fully mac level code, some parts of SoC/chip level code
>> bindup in the mac level. So i segregated the SoC level code like ab
>> related param initialization handling from the mac level procedure.
>>
>> Now, mac/radio should take care only radio level handling procedure.
>>
>> In future for MLO, SoC level can be extend easily.
> But is there a concrete reason to have the functions in core.c? In your
> following patches I couldn't see why to move these functions to core.c,
> everything seems to be suitable for mac.c.
>
> I experimented this in the pending branch and moved the functions to
> mac.c (tag ath-pending-202401151424):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bee89ac2b5755754849deaf7054828e982cc6658
>
> I also fixed your other patchsets to match that and to me it makes more
> sense to have everything in mac.c. I prefer to make core.c as simple as possible.
>
> As you can see I also made changes to the patch titles to make them more
> understandable:
>
> wifi: ath12k: refactor ath12k_mac_register() and ath12k_mac_unregister()
>
> Thoughts?
Looks fine to me.
Thanks,
Karthikeyan