To support multi-link operation, multiple devices with different bands say
2 GHz or 5 GHz or 6 GHz can be combined together as a group and provide
an abstraction to mac80211.
Device group abstraction - when there are multiple devices that are
connected by any means of communication interface between them, then these
devices can be combined together as a single group using a group id to form
a group abstraction. In ath12k driver, this abstraction would be named as
ath12k_hw_group (ag).
Please find below illustration of device group abstraction with two
devices.
Grouping of multiple devices (in future)
+------------------------------------------------------------------------+
| +-------------------------------------+ +-------------------+ |
| | +-----------+ | | +-----------+ | | +-----------+ | |
| | | ar (2GHz) | | | | ar (5GHz) | | | | ar (6GHz) | | |
| | +-----------+ | | +-----------+ | | +-----------+ | |
| | ath12k_base (ab) | | ath12k_base (ab) | |
| | (Dual band device) | | | |
| +-------------------------------------+ +-------------------+ |
| ath12k_hw_group (ag) based on group id |
+------------------------------------------------------------------------+
Say for example, device 1 has two radios (2 GHz and 5 GHz band) and
device 2 has one radio (6 GHz).
In existing code -
device 1 will have two hardware abstractions hw1 (2 GHz) and hw2
(5 GHz) will be registered separately to mac80211 as phy0 and phy1
respectively. Similarly, device 2 will register its hw (6GHz) as
phy2 to mac80211.
In future, with multi-link abstraction
combination 1 - Different group id for device1 and device 2
Device 1 will create a single hardware abstraction hw1
(2 GHz and 5 GHz) and will be registered to mac80211 as
phy0. similarly, device 2 will register its hardware
(6 GHz) to mac80211 as phy1.
combination 2 - Same group id for device1 and device 2
Both device details are combined together as a group, say
group1, with single hardware abstraction of radios 2 GHz,
5 GHz and 6 GHz band details and will be registered to
mac80211 as phy0.
Add base infrastructure changes to add device grouping abstraction with
a single device.
This patch series brings the base code changes with following order:
1. Refactor existing code which would facilitate in introducing
device group abstraction.
2. Create a device group abstraction during device probe.
3. Start the device group only after QMI firmware ready event is
received for all the devices that are combined in the group.
4. Move the hardware abstractions (ath12k_hw - ah) from device
(ath12k_base - ab) to device group abstraction (ag) as it would
ease in having different combinations of group abstraction that
can be registered to mac80211.
v8:
- Addressed firmware assert issue seen during hibernation scenario in
"[PATCH v7 7/8] wifi: ath12k: refactor core start based on hardware group"
v7:
- Added linux-wireless mailer to cc.
- Removed Acked-by tag from "[PATCH v6 8/8]" as it has minor change.
v6:
- Addressed smatch error seen on "[PATCH v5 8/8] wifi: ath12k: move
ath12k_hw from per soc to group"
- Rebased to ToT
v5:
- on "[PATCH 8/8] wifi: ath12k: move ath12k_hw from per soc to
group", refactor the ath12k_mac_hw_allocate() api based on ag rather
than ab and update hardware abstraction array size in ath12k_hw_group
as ATH12K_GROUP_MAX_RADIO.
- Rebased to ToT
v4:
- Modified the cover letter
v3:
- Removed depends-on tag of "wifi: ath12k: Refactor the hardware recovery
procedures" as it is merged to ToT
- Addressed the deadlock warning seen during rmmod.
v2:
- Rebased to ToT
Karthikeyan Periyasamy (8):
wifi: ath12k: Refactor core start api
wifi: ath12k: Add helpers to get or set ath12k_hw
wifi: ath12k: Add ath12k_get_num_hw api
wifi: ath12k: Introduce QMI firmware ready flag
wifi: ath12k: move ATH12K_FLAG_REGISTERED flag set to mac_register api
wifi: ath12k: Introduce device group abstraction
wifi: ath12k: refactor core start based on hardware group
wifi: ath12k: move ath12k_hw from per device to group
drivers/net/wireless/ath/ath12k/core.c | 427 +++++++++++++++++++++----
drivers/net/wireless/ath/ath12k/core.h | 87 ++++-
drivers/net/wireless/ath/ath12k/dp.c | 19 +-
drivers/net/wireless/ath/ath12k/dp.h | 2 +-
drivers/net/wireless/ath/ath12k/mac.c | 117 ++++---
drivers/net/wireless/ath/ath12k/mac.h | 9 +-
drivers/net/wireless/ath/ath12k/pci.c | 2 +
drivers/net/wireless/ath/ath12k/qmi.c | 10 +-
8 files changed, 540 insertions(+), 133 deletions(-)
base-commit: 6e7a5c6d5e38b93f9cc3289d66a597b9a4ca0403
--
2.34.1
From: Karthikeyan Periyasamy <[email protected]>
Currently, hardware abstractions (ah) of different radio bands
are tightly coupled to a single device (ab). But, with hardware
device group abstraction (ag), multiple radios across different
devices in a group can possibly form different combinations of
hardware abstractions (ah) within the group. Hence, the mapping
between ah to ab can be removed and instead it can be mapped with ag.
Please find below illustration of how mapping between ath12k_hw (ah),
ath12k_base (ab) and ath12k_hw_group (ag) is changed.
current mapping of hardware abstraction (ah) with device (ab)
+------------------------------------------------+
| +-------------------------------------+ |
| | +---------------+ +---------------+ | |
| | |ath12k_hw (ah) | |ath12k_hw (ah) | | |
| | +---------------+ +---------------+ | |
| | | |
| | +-----------+ | +-----------+ | |
| | | ar (2GHz) | | | ar (5GHz) | | |
| | +-----------+ | +-----------+ | |
| | Dual band device-1 (ab) | |
| +-------------------------------------+ |
| ath12k_hw_group (ag) based on group id |
+------------------------------------------------+
After, with hardware device group abstraction
(moving ah array out of ab to ag)
+----------------------------------------------+
| +---------------+ +---------------+ |
| |ath12k_hw (ah) | |ath12k_hw (ah) | |
| +---------------+ +---------------+ |
| +-------------------------------------+ |
| | +-----------+ +-----------+ | |
| | | ar (2GHz) | | ar (5GHz) | | |
| | +-----------+ +-----------+ | |
| | Dual band device-1 (ab) | |
| +-------------------------------------+ |
| ath12k_hw_group (ag) based on group id |
+----------------------------------------------+
This decoupling of ath12k_hw (ah) from ath12k_base (ab) and mapping it
to ath12k_hw_group (ag) will help in forming different combinations of
multi-link devices.
Say for example, device 1 has two radios (2 GHz and 5 GHz band) and
device 2 has one radio (6 GHz).
In existing code -
device 1 will have two hardware abstractions hw1 (2 GHz) and
hw2 (5 GHz) will be registered separately to mac80211 as phy0
and phy1 respectively. Similarly, device 2 will register its
hw (6 GHz) as phy2 to mac80211.
In future, with multi-link abstraction
combination 1 - Different group id for device1 and device 2
Device 1 will create a single hardware abstraction hw1
(2 GHz and 5 GHz) and will be registered to mac80211 as
phy0. similarly, device 2 will register its hardware
(6 GHz) to mac80211 as phy1.
combination 2 - Same group id for device1 and device 2
Both device details are combined together as a group, say
group1, with single hardware abstraction of radios 2 GHz,
5 GHz and 6 GHz band details and will be registered to
mac80211 as phy0.
Hence, Add changes to decouple ath12k_hw (ah) from ath12k_base (ab) and
map it to ath12k_hw_group (ag).
Refactor the following APIs to help simplify the registration based on
ath12k_hw_group (ag) rather than ath12k_base (ab)
* ath12k_mac_allocate()
* ath12k_mac_destroy()
* ath12k_mac_register()
* ath12k_mac_unregister()
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-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 52 +++++++--------
drivers/net/wireless/ath/ath12k/core.h | 25 +++----
drivers/net/wireless/ath/ath12k/dp.c | 19 ++----
drivers/net/wireless/ath/ath12k/dp.h | 2 +-
drivers/net/wireless/ath/ath12k/mac.c | 90 ++++++++++++++++++--------
drivers/net/wireless/ath/ath12k/mac.h | 9 +--
6 files changed, 110 insertions(+), 87 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 90c70dbfc50a..4c1d1a887758 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -831,11 +831,6 @@ static void ath12k_core_device_cleanup(struct ath12k_base *ab)
if (test_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags))
ath12k_core_pdev_destroy(ab);
- if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags)) {
- ath12k_mac_unregister(ab);
- ath12k_mac_destroy(ab);
- }
-
mutex_unlock(&ab->core_lock);
}
@@ -852,6 +847,8 @@ static void ath12k_core_hw_group_stop(struct ath12k_hw_group *ag)
continue;
ath12k_core_device_cleanup(ab);
}
+ ath12k_mac_unregister(ag);
+ ath12k_mac_destroy(ag);
}
static int ath12k_core_hw_group_start(struct ath12k_hw_group *ag)
@@ -862,6 +859,23 @@ static int ath12k_core_hw_group_start(struct ath12k_hw_group *ag)
lockdep_assert_held(&ag->mutex_lock);
+ /* Check if already registered or not, since same flow
+ * execute for HW restart case.
+ */
+ is_registered = test_bit(ATH12K_GROUP_FLAG_REGISTERED, &ag->flags);
+
+ if (is_registered)
+ goto core_pdev_create;
+
+ ret = ath12k_mac_allocate(ag);
+ if (WARN_ON(ret))
+ return ret;
+
+ ret = ath12k_mac_register(ag);
+ if (WARN_ON(ret))
+ goto err_mac_alloc;
+
+core_pdev_create:
for (i = 0; i < ag->num_devices; i++) {
ab = ag->ab[i];
if (!ab)
@@ -869,31 +883,6 @@ static int ath12k_core_hw_group_start(struct ath12k_hw_group *ag)
mutex_lock(&ab->core_lock);
- /* Check if already registered or not, since same flow
- * execute for HW restart case.
- */
- is_registered = test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
-
- if (is_registered)
- goto core_pdev_create;
-
- ret = ath12k_mac_allocate(ab);
- if (ret) {
- ath12k_err(ab, "failed to create new hw device with mac80211 :%d\n",
- ret);
- mutex_unlock(&ab->core_lock);
- return ret;
- }
-
- ret = ath12k_mac_register(ab);
- if (ret) {
- ath12k_err(ab, "failed to register radio with mac80211: %d\n",
- ret);
- mutex_unlock(&ab->core_lock);
- goto err;
- }
-
-core_pdev_create:
ret = ath12k_core_pdev_create(ab);
if (ret) {
ath12k_err(ab, "failed to create pdev core %d\n", ret);
@@ -919,7 +908,10 @@ static int ath12k_core_hw_group_start(struct ath12k_hw_group *ag)
err:
ath12k_core_hw_group_stop(ag);
+ return ret;
+err_mac_alloc:
+ ath12k_mac_destroy(ag);
return ret;
}
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index d955deb08fd4..fcdd945ba7ea 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -60,6 +60,7 @@
#define ATH12K_RECOVER_START_TIMEOUT_HZ (20 * HZ)
#define ATH12K_MAX_SOCS 3
+#define ATH12K_GROUP_MAX_RADIO (ATH12K_MAX_SOCS * MAX_RADIOS)
#define ATH12K_INVALID_GROUP_ID 0xFF
#define ATH12K_INVALID_DEVICE_ID 0xFF
@@ -748,6 +749,15 @@ struct ath12k_hw_group {
struct ath12k_base *ab[ATH12K_MAX_SOCS];
/* To synchronize group create, assign, start, stop */
struct mutex mutex_lock;
+
+ /* Holds information of wiphy (hw) registration.
+ *
+ * In Multi/Single Link Operation case, all pdevs are registered as
+ * a single wiphy. In other (legacy/Non-MLO) cases, each pdev is
+ * registered as separate wiphys.
+ */
+ struct ath12k_hw *ah[ATH12K_GROUP_MAX_RADIO];
+ u8 num_hw;
};
/**
@@ -819,15 +829,6 @@ struct ath12k_base {
struct ath12k_pdev __rcu *pdevs_active[MAX_RADIOS];
- /* Holds information of wiphy (hw) registration.
- *
- * In Multi/Single Link Operation case, all pdevs are registered as
- * a single wiphy. In other (legacy/Non-MLO) cases, each pdev is
- * registered as separate wiphys.
- */
- struct ath12k_hw *ah[MAX_RADIOS];
- u8 num_hw;
-
struct ath12k_wmi_hal_reg_capabilities_ext_arg hal_reg_cap[MAX_RADIOS];
unsigned long long free_vdev_map;
unsigned long long free_vdev_stats_id_map;
@@ -1083,18 +1084,18 @@ static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
static inline struct ath12k_hw *ath12k_ab_to_ah(struct ath12k_base *ab, int idx)
{
- return ab->ah[idx];
+ return ab->ag->ah[idx];
}
static inline void ath12k_ab_set_ah(struct ath12k_base *ab, int idx,
struct ath12k_hw *ah)
{
- ab->ah[idx] = ah;
+ ab->ag->ah[idx] = ah;
}
static inline int ath12k_get_num_hw(struct ath12k_base *ab)
{
- return ab->num_hw;
+ return ab->ag->num_hw;
}
static inline
diff --git a/drivers/net/wireless/ath/ath12k/dp.c b/drivers/net/wireless/ath/ath12k/dp.c
index 61aa78d8bd8c..5cc5eac26751 100644
--- a/drivers/net/wireless/ath/ath12k/dp.c
+++ b/drivers/net/wireless/ath/ath12k/dp.c
@@ -980,21 +980,14 @@ void ath12k_dp_pdev_free(struct ath12k_base *ab)
ath12k_dp_rx_pdev_free(ab, i);
}
-void ath12k_dp_pdev_pre_alloc(struct ath12k_base *ab)
+void ath12k_dp_pdev_pre_alloc(struct ath12k *ar)
{
- struct ath12k *ar;
- struct ath12k_pdev_dp *dp;
- int i;
+ struct ath12k_pdev_dp *dp = &ar->dp;
- for (i = 0; i < ab->num_radios; i++) {
- ar = ab->pdevs[i].ar;
- dp = &ar->dp;
- dp->mac_id = i;
- atomic_set(&dp->num_tx_pending, 0);
- init_waitqueue_head(&dp->tx_empty_waitq);
-
- /* TODO: Add any RXDMA setup required per pdev */
- }
+ dp->mac_id = ar->pdev_idx;
+ atomic_set(&dp->num_tx_pending, 0);
+ init_waitqueue_head(&dp->tx_empty_waitq);
+ /* TODO: Add any RXDMA setup required per pdev */
}
bool ath12k_dp_wmask_compaction_rx_tlv_supported(struct ath12k_base *ab)
diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h
index 742094545089..00c0a76841df 100644
--- a/drivers/net/wireless/ath/ath12k/dp.h
+++ b/drivers/net/wireless/ath/ath12k/dp.h
@@ -1815,7 +1815,7 @@ void ath12k_dp_free(struct ath12k_base *ab);
int ath12k_dp_alloc(struct ath12k_base *ab);
void ath12k_dp_cc_config(struct ath12k_base *ab);
int ath12k_dp_pdev_alloc(struct ath12k_base *ab);
-void ath12k_dp_pdev_pre_alloc(struct ath12k_base *ab);
+void ath12k_dp_pdev_pre_alloc(struct ath12k *ar);
void ath12k_dp_pdev_free(struct ath12k_base *ab);
int ath12k_dp_tx_htt_srng_setup(struct ath12k_base *ab, u32 ring_id,
int mac_id, enum hal_ring_type ring_type);
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 4ab9846f154c..870e7351ed08 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -9144,19 +9144,13 @@ static void ath12k_mac_setup(struct ath12k *ar)
skb_queue_head_init(&ar->wmi_mgmt_tx_queue);
}
-int ath12k_mac_register(struct ath12k_base *ab)
+int ath12k_mac_register(struct ath12k_hw_group *ag)
{
+ struct ath12k_base *ab = ag->ab[0];
struct ath12k_hw *ah;
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 < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
@@ -9180,8 +9174,9 @@ int ath12k_mac_register(struct ath12k_base *ab)
return ret;
}
-void ath12k_mac_unregister(struct ath12k_base *ab)
+void ath12k_mac_unregister(struct ath12k_hw_group *ag)
{
+ struct ath12k_base *ab = ag->ab[0];
struct ath12k_hw *ah;
int i;
@@ -9200,12 +9195,13 @@ static void ath12k_mac_hw_destroy(struct ath12k_hw *ah)
ieee80211_free_hw(ah->hw);
}
-static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
+static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_hw_group *ag,
struct ath12k_pdev_map *pdev_map,
u8 num_pdev_map)
{
struct ieee80211_hw *hw;
struct ath12k *ar;
+ struct ath12k_base *ab;
struct ath12k_pdev *pdev;
struct ath12k_hw *ah;
int i;
@@ -9218,7 +9214,7 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
ah = ath12k_hw_to_ah(hw);
ah->hw = hw;
- ah->ab = ab;
+ ah->ab = pdev_map[0].ab;
ah->num_radio = num_pdev_map;
mutex_init(&ah->hw_mutex);
@@ -9237,23 +9233,30 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
pdev->ar = ar;
ath12k_mac_setup(ar);
+ ath12k_dp_pdev_pre_alloc(ar);
}
return ah;
}
-void ath12k_mac_destroy(struct ath12k_base *ab)
+void ath12k_mac_destroy(struct ath12k_hw_group *ag)
{
struct ath12k_pdev *pdev;
- struct ath12k_hw *ah;
+ struct ath12k_base *ab = ag->ab[0];
int i;
+ struct ath12k_hw *ah;
- for (i = 0; i < ab->num_radios; i++) {
- pdev = &ab->pdevs[i];
- if (!pdev->ar)
+ for (i = 0; i < ag->num_devices; i++) {
+ ab = ag->ab[i];
+ if (!ab)
continue;
- pdev->ar = NULL;
+ for (i = 0; i < ab->num_radios; i++) {
+ pdev = &ab->pdevs[i];
+ if (!pdev->ar)
+ continue;
+ pdev->ar = NULL;
+ }
}
for (i = 0; i < ath12k_get_num_hw(ab); i++) {
@@ -9266,26 +9269,60 @@ void ath12k_mac_destroy(struct ath12k_base *ab)
}
}
-int ath12k_mac_allocate(struct ath12k_base *ab)
+static void ath12k_mac_set_device_defaults(struct ath12k_base *ab)
+{
+ /* Initialize channel counters frequency value in hertz */
+ ab->cc_freq_hz = 320000;
+ ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
+}
+
+int ath12k_mac_allocate(struct ath12k_hw_group *ag)
{
struct ath12k_hw *ah;
+ struct ath12k_base *ab;
struct ath12k_pdev_map pdev_map[MAX_RADIOS];
int ret, i, j;
u8 radio_per_hw;
+ int mac_id, device_id;
+ int total_radio, num_hw;
- if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
- return 0;
+ total_radio = 0;
+ for (i = 0; i < ag->num_devices; i++)
+ total_radio += ag->ab[i]->num_radios;
- ab->num_hw = ab->num_radios;
+ /* All pdev get combined and register as single wiphy based on
+ * hardware group which participate in multi-link operation else
+ * each pdev get register separately.
+ *
+ * Currently, registering as single pdevs.
+ */
radio_per_hw = 1;
+ num_hw = total_radio / radio_per_hw;
- for (i = 0; i < ath12k_get_num_hw(ab); i++) {
+ if (WARN_ON(num_hw >= ATH12K_GROUP_MAX_RADIO))
+ return -ENOSPC;
+
+ ag->num_hw = 0;
+ device_id = 0;
+ mac_id = 0;
+ for (i = 0; i < num_hw; i++) {
for (j = 0; j < radio_per_hw; j++) {
+ ab = ag->ab[device_id];
pdev_map[j].ab = ab;
- pdev_map[j].pdev_idx = (i * radio_per_hw) + j;
+ pdev_map[j].pdev_idx = mac_id;
+ mac_id++;
+
+ /* If mac_id falls beyond the current device MACs then
+ * move to next device
+ */
+ if (mac_id >= ab->num_radios) {
+ mac_id = 0;
+ device_id++;
+ ath12k_mac_set_device_defaults(ab);
+ }
}
- ah = ath12k_mac_hw_allocate(ab, pdev_map, radio_per_hw);
+ ah = ath12k_mac_hw_allocate(ag, pdev_map, radio_per_hw);
if (!ah) {
ath12k_warn(ab, "failed to allocate mac80211 hw device for hw_idx %d\n",
i);
@@ -9293,11 +9330,10 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
goto err;
}
- ath12k_ab_set_ah(ab, i, ah);
+ ag->ah[i] = ah;
+ ag->num_hw++;
}
- ath12k_dp_pdev_pre_alloc(ab);
-
return 0;
err:
diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
index 69fd282b9dd3..f0ea0b5f50e4 100644
--- a/drivers/net/wireless/ath/ath12k/mac.h
+++ b/drivers/net/wireless/ath/ath12k/mac.h
@@ -13,6 +13,7 @@
struct ath12k;
struct ath12k_base;
struct ath12k_hw;
+struct ath12k_hw_group;
struct ath12k_pdev_map;
struct ath12k_generic_iter {
@@ -50,10 +51,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_unregister(struct ath12k_base *ab);
-int ath12k_mac_register(struct ath12k_base *ab);
-int ath12k_mac_allocate(struct ath12k_base *ab);
+void ath12k_mac_destroy(struct ath12k_hw_group *ag);
+void ath12k_mac_unregister(struct ath12k_hw_group *ag);
+int ath12k_mac_register(struct ath12k_hw_group *ag);
+int ath12k_mac_allocate(struct ath12k_hw_group *ag);
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
From: Karthikeyan Periyasamy <[email protected]>
Currently, mac allocate/register and core_pdev_create are initiated
immediately when QMI firmware ready event is received for a particular
device.
With hardware device group abstraction, QMI firmware ready event can be
received simultaneously for different devices in the group and so, it
should not be registered immediately rather it has to be deferred until
all devices in the group has received QMI firmware ready.
To handle this, refactor the code of core start to move the following
apis inside a wrapper ath12k_core_hw_group_start()
* ath12k_mac_allocate()
* ath12k_core_pdev_create()
* ath12k_core_rfkill_config()
* ath12k_mac_register()
* ath12k_hif_irq_enable()
similarly, move the corresponding destroy/unregister/disable apis
inside wrapper ath12k_core_hw_group_stop()
Add the device flags to indicate pdev created and IRQ enabled which would
be helpful for device clean up during failure cases.
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-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Co-developed-by: Harshitha Prem <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 210 +++++++++++++++++++------
drivers/net/wireless/ath/ath12k/core.h | 32 ++++
2 files changed, 191 insertions(+), 51 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index ebe31cbb6435..90c70dbfc50a 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -563,6 +563,9 @@ u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab)
static void ath12k_core_stop(struct ath12k_base *ab)
{
+ clear_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
+ ath12k_dec_num_core_started(ab);
+
if (!test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
ath12k_qmi_firmware_stop(ab);
@@ -689,11 +692,15 @@ static int ath12k_core_pdev_create(struct ath12k_base *ab)
return ret;
}
+ set_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
+
return 0;
}
static void ath12k_core_pdev_destroy(struct ath12k_base *ab)
{
+ clear_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
+
ath12k_dp_pdev_free(ab);
}
@@ -702,6 +709,8 @@ static int ath12k_core_start(struct ath12k_base *ab,
{
int ret;
+ lockdep_assert_held(&ab->core_lock);
+
ret = ath12k_wmi_attach(ab);
if (ret) {
ath12k_err(ab, "failed to attach wmi: %d\n", ret);
@@ -795,6 +804,12 @@ static int ath12k_core_start(struct ath12k_base *ab,
/* ACPI is optional so continue in case of an error */
ath12k_dbg(ab, ATH12K_DBG_BOOT, "acpi failed: %d\n", ret);
+ if (!test_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags)) {
+ /* Indicate the core start in the appropriate group */
+ ath12k_inc_num_core_started(ab);
+ set_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
+ }
+
return 0;
err_reo_cleanup:
@@ -806,6 +821,108 @@ static int ath12k_core_start(struct ath12k_base *ab,
return ret;
}
+static void ath12k_core_device_cleanup(struct ath12k_base *ab)
+{
+ mutex_lock(&ab->core_lock);
+
+ if (test_and_clear_bit(ATH12K_FLAG_CORE_HIF_IRQ_ENABLED, &ab->dev_flags))
+ ath12k_hif_irq_disable(ab);
+
+ if (test_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags))
+ ath12k_core_pdev_destroy(ab);
+
+ if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags)) {
+ ath12k_mac_unregister(ab);
+ ath12k_mac_destroy(ab);
+ }
+
+ mutex_unlock(&ab->core_lock);
+}
+
+static void ath12k_core_hw_group_stop(struct ath12k_hw_group *ag)
+{
+ struct ath12k_base *ab;
+ int i;
+
+ lockdep_assert_held(&ag->mutex_lock);
+
+ for (i = ag->num_devices - 1; i >= 0; i--) {
+ ab = ag->ab[i];
+ if (!ab)
+ continue;
+ ath12k_core_device_cleanup(ab);
+ }
+}
+
+static int ath12k_core_hw_group_start(struct ath12k_hw_group *ag)
+{
+ struct ath12k_base *ab;
+ int ret, i;
+ bool is_registered;
+
+ lockdep_assert_held(&ag->mutex_lock);
+
+ for (i = 0; i < ag->num_devices; i++) {
+ ab = ag->ab[i];
+ if (!ab)
+ continue;
+
+ mutex_lock(&ab->core_lock);
+
+ /* Check if already registered or not, since same flow
+ * execute for HW restart case.
+ */
+ is_registered = test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
+
+ if (is_registered)
+ goto core_pdev_create;
+
+ ret = ath12k_mac_allocate(ab);
+ if (ret) {
+ ath12k_err(ab, "failed to create new hw device with mac80211 :%d\n",
+ ret);
+ mutex_unlock(&ab->core_lock);
+ return ret;
+ }
+
+ ret = ath12k_mac_register(ab);
+ if (ret) {
+ ath12k_err(ab, "failed to register radio with mac80211: %d\n",
+ ret);
+ mutex_unlock(&ab->core_lock);
+ goto err;
+ }
+
+core_pdev_create:
+ ret = ath12k_core_pdev_create(ab);
+ if (ret) {
+ ath12k_err(ab, "failed to create pdev core %d\n", ret);
+ mutex_unlock(&ab->core_lock);
+ goto err;
+ }
+
+ ath12k_hif_irq_enable(ab);
+ set_bit(ATH12K_FLAG_CORE_HIF_IRQ_ENABLED, &ab->dev_flags);
+
+ ret = ath12k_core_rfkill_config(ab);
+ if (ret && ret != -EOPNOTSUPP) {
+ mutex_unlock(&ab->core_lock);
+ goto err;
+ }
+
+ mutex_unlock(&ab->core_lock);
+ }
+
+ set_bit(ATH12K_GROUP_FLAG_REGISTERED, &ag->flags);
+
+ return 0;
+
+err:
+ ath12k_core_hw_group_stop(ag);
+
+ return ret;
+}
+
static int ath12k_core_start_firmware(struct ath12k_base *ab,
enum ath12k_firmware_mode mode)
{
@@ -823,9 +940,18 @@ static int ath12k_core_start_firmware(struct ath12k_base *ab,
return ret;
}
+static inline
+bool ath12k_core_hw_group_start_ready(struct ath12k_hw_group *ag)
+{
+ lockdep_assert_held(&ag->mutex_lock);
+
+ return (ag->num_started == ag->num_devices);
+}
+
int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab)
{
- int ret;
+ struct ath12k_hw_group *ag = ath12k_ab_to_ag(ab);
+ int ret, i;
ret = ath12k_core_start_firmware(ab, ATH12K_FIRMWARE_MODE_NORMAL);
if (ret) {
@@ -845,59 +971,48 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab)
goto err_firmware_stop;
}
+ mutex_lock(&ag->mutex_lock);
mutex_lock(&ab->core_lock);
ret = ath12k_core_start(ab, ATH12K_FIRMWARE_MODE_NORMAL);
if (ret) {
ath12k_err(ab, "failed to start core: %d\n", ret);
goto err_dp_free;
}
+ mutex_unlock(&ab->core_lock);
- ret = ath12k_mac_allocate(ab);
- if (ret) {
- ath12k_err(ab, "failed to create new hw device with mac80211 :%d\n",
- ret);
- goto err_core_stop;
- }
-
- ret = ath12k_mac_register(ab);
- if (ret) {
- ath12k_err(ab, "failed register the radio with mac80211: %d\n", ret);
- goto err_mac_destroy;
+ if (ath12k_core_hw_group_start_ready(ag)) {
+ ret = ath12k_core_hw_group_start(ag);
+ if (ret) {
+ ath12k_warn(ab, "unable to start hw group\n");
+ goto err_core_stop;
+ }
+ ath12k_dbg(ab, ATH12K_DBG_BOOT, "group %d started\n", ag->id);
}
+ mutex_unlock(&ag->mutex_lock);
- ret = ath12k_core_pdev_create(ab);
- if (ret) {
- ath12k_err(ab, "failed to create pdev core: %d\n", ret);
- goto err_mac_unregister;
- }
+ return 0;
- ath12k_hif_irq_enable(ab);
+err_core_stop:
+ for (i = ag->num_devices - 1; i >= 0; i--) {
+ ab = ag->ab[i];
+ if (!ab)
+ continue;
- ret = ath12k_core_rfkill_config(ab);
- if (ret && ret != -EOPNOTSUPP) {
- ath12k_err(ab, "failed to config rfkill: %d\n", ret);
- goto err_core_pdev_destroy;
+ mutex_lock(&ab->core_lock);
+ if (test_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags))
+ ath12k_core_stop(ab);
+ mutex_unlock(&ab->core_lock);
}
+ goto exit;
- mutex_unlock(&ab->core_lock);
-
- return 0;
-
-err_core_pdev_destroy:
- ath12k_hif_irq_disable(ab);
- ath12k_core_pdev_destroy(ab);
-err_mac_unregister:
- ath12k_mac_unregister(ab);
-err_mac_destroy:
- ath12k_mac_destroy(ab);
-err_core_stop:
- ath12k_core_stop(ab);
err_dp_free:
ath12k_dp_free(ab);
mutex_unlock(&ab->core_lock);
err_firmware_stop:
ath12k_qmi_firmware_stop(ab);
+exit:
+ mutex_unlock(&ag->mutex_lock);
return ret;
}
@@ -1258,7 +1373,7 @@ static struct ath12k_hw_group *ath12k_core_assign_hw_group(struct ath12k_base *a
void ath12k_core_unassign_hw_group(struct ath12k_base *ab)
{
- struct ath12k_hw_group *ag = ab->ag;
+ struct ath12k_hw_group *ag = ath12k_ab_to_ag(ab);
u8 device_id = ab->device_id;
int num_probed;
@@ -1292,19 +1407,6 @@ void ath12k_core_unassign_hw_group(struct ath12k_base *ab)
ath12k_core_hw_group_free(ag);
}
-static void ath12k_core_device_cleanup(struct ath12k_base *ab)
-{
- mutex_lock(&ab->core_lock);
-
- ath12k_hif_irq_disable(ab);
- ath12k_core_pdev_destroy(ab);
- ath12k_mac_unregister(ab);
- ath12k_mac_destroy(ab);
- ath12k_core_stop(ab);
-
- mutex_unlock(&ab->core_lock);
-}
-
static void ath12k_core_hw_group_destroy(struct ath12k_hw_group *ag)
{
struct ath12k_base *ab;
@@ -1332,14 +1434,20 @@ static void ath12k_core_hw_group_cleanup(struct ath12k_hw_group *ag)
return;
mutex_lock(&ag->mutex_lock);
+ if (test_and_clear_bit(ATH12K_GROUP_FLAG_REGISTERED, &ag->flags))
+ ath12k_core_hw_group_stop(ag);
+
for (i = 0; i < ag->num_devices; i++) {
ab = ag->ab[i];
if (!ab)
continue;
- if (test_bit(ATH12K_FLAG_QMI_FW_READY_COMPLETE, &ab->dev_flags))
- ath12k_core_device_cleanup(ab);
+ mutex_lock(&ab->core_lock);
+ if (test_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags))
+ ath12k_core_stop(ab);
+ mutex_unlock(&ab->core_lock);
}
+
mutex_unlock(&ag->mutex_lock);
}
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index a6b8c100ebc8..d955deb08fd4 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -200,6 +200,10 @@ enum ath12k_scan_state {
ATH12K_SCAN_ABORTING,
};
+enum ath12k_hw_group_flags {
+ ATH12K_GROUP_FLAG_REGISTERED,
+};
+
enum ath12k_dev_flags {
ATH12K_CAC_RUNNING,
ATH12K_FLAG_CRASH_FLUSH,
@@ -214,6 +218,9 @@ enum ath12k_dev_flags {
ATH12K_FLAG_EXT_IRQ_ENABLED,
ATH12K_FLAG_QMI_FW_READY_COMPLETE,
ATH12K_FLAG_HW_GROUP_ATTACHED,
+ ATH12K_FLAG_PDEV_CREATED,
+ ATH12K_FLAG_CORE_STARTED,
+ ATH12K_FLAG_CORE_HIF_IRQ_ENABLED,
};
struct ath12k_tx_conf {
@@ -736,6 +743,8 @@ struct ath12k_hw_group {
u8 id;
u8 num_devices;
u8 num_probed;
+ u8 num_started;
+ unsigned long flags;
struct ath12k_base *ab[ATH12K_MAX_SOCS];
/* To synchronize group create, assign, start, stop */
struct mutex mutex_lock;
@@ -1087,4 +1096,27 @@ static inline int ath12k_get_num_hw(struct ath12k_base *ab)
{
return ab->num_hw;
}
+
+static inline
+struct ath12k_hw_group *ath12k_ab_to_ag(struct ath12k_base *ab)
+{
+ return ab->ag;
+}
+
+static inline
+void ath12k_inc_num_core_started(struct ath12k_base *ab)
+{
+ lockdep_assert_held(&ab->ag->mutex_lock);
+
+ ab->ag->num_started++;
+}
+
+static inline
+void ath12k_dec_num_core_started(struct ath12k_base *ab)
+{
+ lockdep_assert_held(&ab->ag->mutex_lock);
+
+ ab->ag->num_started--;
+}
+
#endif /* _CORE_H_ */
--
2.34.1
From: Karthikeyan Periyasamy <[email protected]>
When hardware device group abstraction is introduced, in future, a group
abstraction is registered to mac80211 rather than a particular device.
Hence, setting ATH12K_FLAG_REGISTERED in QMI firmware ready event might not
be ideal.
Add changes to move set/unset of ATH12K_FLAG_REGISTERED flag inside
ath12k_mac_register() and ath12k_mac_unregister() respectively.
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-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
Acked-by: Jeff Johnson <[email protected]>
---
drivers/net/wireless/ath/ath12k/mac.c | 2 ++
drivers/net/wireless/ath/ath12k/qmi.c | 4 +---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 66587c5ea8fd..4ab9846f154c 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -9165,6 +9165,7 @@ int ath12k_mac_register(struct ath12k_base *ab)
goto err;
}
+ set_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
return 0;
err:
@@ -9184,6 +9185,7 @@ void ath12k_mac_unregister(struct ath12k_base *ab)
struct ath12k_hw *ah;
int i;
+ clear_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
for (i = ath12k_get_num_hw(ab) - 1; i >= 0; i--) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah)
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 23cd21a3fa97..c570b17fe150 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -3333,11 +3333,9 @@ static void ath12k_qmi_driver_event_work(struct work_struct *work)
&ab->dev_flags);
clear_bit(ATH12K_FLAG_RECOVERY, &ab->dev_flags);
ret = ath12k_core_qmi_firmware_ready(ab);
- if (!ret) {
+ if (!ret)
set_bit(ATH12K_FLAG_QMI_FW_READY_COMPLETE,
&ab->dev_flags);
- set_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
- }
break;
default:
--
2.34.1
On 5/31/2024 11:04 AM, Harshitha Prem wrote:
> To support multi-link operation, multiple devices with different bands say
> 2 GHz or 5 GHz or 6 GHz can be combined together as a group and provide
> an abstraction to mac80211.
>
> Device group abstraction - when there are multiple devices that are
> connected by any means of communication interface between them, then these
> devices can be combined together as a single group using a group id to form
> a group abstraction. In ath12k driver, this abstraction would be named as
> ath12k_hw_group (ag).
>
> Please find below illustration of device group abstraction with two
> devices.
>
> Grouping of multiple devices (in future)
> +------------------------------------------------------------------------+
> | +-------------------------------------+ +-------------------+ |
> | | +-----------+ | | +-----------+ | | +-----------+ | |
> | | | ar (2GHz) | | | | ar (5GHz) | | | | ar (6GHz) | | |
> | | +-----------+ | | +-----------+ | | +-----------+ | |
> | | ath12k_base (ab) | | ath12k_base (ab) | |
> | | (Dual band device) | | | |
> | +-------------------------------------+ +-------------------+ |
> | ath12k_hw_group (ag) based on group id |
> +------------------------------------------------------------------------+
>
> Say for example, device 1 has two radios (2 GHz and 5 GHz band) and
> device 2 has one radio (6 GHz).
>
> In existing code -
> device 1 will have two hardware abstractions hw1 (2 GHz) and hw2
> (5 GHz) will be registered separately to mac80211 as phy0 and phy1
> respectively. Similarly, device 2 will register its hw (6GHz) as
> phy2 to mac80211.
>
> In future, with multi-link abstraction
>
> combination 1 - Different group id for device1 and device 2
> Device 1 will create a single hardware abstraction hw1
> (2 GHz and 5 GHz) and will be registered to mac80211 as
> phy0. similarly, device 2 will register its hardware
> (6 GHz) to mac80211 as phy1.
>
> combination 2 - Same group id for device1 and device 2
> Both device details are combined together as a group, say
> group1, with single hardware abstraction of radios 2 GHz,
> 5 GHz and 6 GHz band details and will be registered to
> mac80211 as phy0.
>
> Add base infrastructure changes to add device grouping abstraction with
> a single device.
>
> This patch series brings the base code changes with following order:
> 1. Refactor existing code which would facilitate in introducing
> device group abstraction.
> 2. Create a device group abstraction during device probe.
> 3. Start the device group only after QMI firmware ready event is
> received for all the devices that are combined in the group.
> 4. Move the hardware abstractions (ath12k_hw - ah) from device
> (ath12k_base - ab) to device group abstraction (ag) as it would
> ease in having different combinations of group abstraction that
> can be registered to mac80211.
>
> v8:
> - Addressed firmware assert issue seen during hibernation scenario in
> "[PATCH v7 7/8] wifi: ath12k: refactor core start based on hardware group"
>
> v7:
> - Added linux-wireless mailer to cc.
> - Removed Acked-by tag from "[PATCH v6 8/8]" as it has minor change.
>
> v6:
> - Addressed smatch error seen on "[PATCH v5 8/8] wifi: ath12k: move
> ath12k_hw from per soc to group"
> - Rebased to ToT
> v5:
> - on "[PATCH 8/8] wifi: ath12k: move ath12k_hw from per soc to
> group", refactor the ath12k_mac_hw_allocate() api based on ag rather
> than ab and update hardware abstraction array size in ath12k_hw_group
> as ATH12K_GROUP_MAX_RADIO.
> - Rebased to ToT
> v4:
> - Modified the cover letter
> v3:
> - Removed depends-on tag of "wifi: ath12k: Refactor the hardware recovery
> procedures" as it is merged to ToT
> - Addressed the deadlock warning seen during rmmod.
>
> v2:
> - Rebased to ToT
>
>
> Karthikeyan Periyasamy (8):
> wifi: ath12k: Refactor core start api
> wifi: ath12k: Add helpers to get or set ath12k_hw
> wifi: ath12k: Add ath12k_get_num_hw api
> wifi: ath12k: Introduce QMI firmware ready flag
> wifi: ath12k: move ATH12K_FLAG_REGISTERED flag set to mac_register api
> wifi: ath12k: Introduce device group abstraction
> wifi: ath12k: refactor core start based on hardware group
> wifi: ath12k: move ath12k_hw from per device to group
>
> drivers/net/wireless/ath/ath12k/core.c | 427 +++++++++++++++++++++----
> drivers/net/wireless/ath/ath12k/core.h | 87 ++++-
> drivers/net/wireless/ath/ath12k/dp.c | 19 +-
> drivers/net/wireless/ath/ath12k/dp.h | 2 +-
> drivers/net/wireless/ath/ath12k/mac.c | 117 ++++---
> drivers/net/wireless/ath/ath12k/mac.h | 9 +-
> drivers/net/wireless/ath/ath12k/pci.c | 2 +
> drivers/net/wireless/ath/ath12k/qmi.c | 10 +-
> 8 files changed, 540 insertions(+), 133 deletions(-)
>
>
> base-commit: 6e7a5c6d5e38b93f9cc3289d66a597b9a4ca0403
this no longer applies cleanly to ath/master or ath/ath-next,
can you please rebase and repost?
Harshitha Prem <[email protected]> writes:
> From: Karthikeyan Periyasamy <[email protected]>
>
> Currently, mac allocate/register and core_pdev_create are initiated
> immediately when QMI firmware ready event is received for a particular
> device.
>
> With hardware device group abstraction, QMI firmware ready event can be
> received simultaneously for different devices in the group and so, it
> should not be registered immediately rather it has to be deferred until
> all devices in the group has received QMI firmware ready.
>
> To handle this, refactor the code of core start to move the following
> apis inside a wrapper ath12k_core_hw_group_start()
> * ath12k_mac_allocate()
> * ath12k_core_pdev_create()
> * ath12k_core_rfkill_config()
> * ath12k_mac_register()
> * ath12k_hif_irq_enable()
>
> similarly, move the corresponding destroy/unregister/disable apis
> inside wrapper ath12k_core_hw_group_stop()
>
> Add the device flags to indicate pdev created and IRQ enabled which would
> be helpful for device clean up during failure cases.
>
> 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-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Co-developed-by: Harshitha Prem <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>
> ---
> drivers/net/wireless/ath/ath12k/core.c | 210 +++++++++++++++++++------
> drivers/net/wireless/ath/ath12k/core.h | 32 ++++
> 2 files changed, 191 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index ebe31cbb6435..90c70dbfc50a 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -563,6 +563,9 @@ u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab)
>
> static void ath12k_core_stop(struct ath12k_base *ab)
> {
> + clear_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
> + ath12k_dec_num_core_started(ab);
> +
> if (!test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
> ath12k_qmi_firmware_stop(ab);
>
> @@ -689,11 +692,15 @@ static int ath12k_core_pdev_create(struct ath12k_base *ab)
> return ret;
> }
>
> + set_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
> +
> return 0;
> }
>
> static void ath12k_core_pdev_destroy(struct ath12k_base *ab)
> {
> + clear_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
> +
> ath12k_dp_pdev_free(ab);
> }
>
> @@ -702,6 +709,8 @@ static int ath12k_core_start(struct ath12k_base *ab,
> {
> int ret;
>
> + lockdep_assert_held(&ab->core_lock);
> +
> ret = ath12k_wmi_attach(ab);
> if (ret) {
> ath12k_err(ab, "failed to attach wmi: %d\n", ret);
> @@ -795,6 +804,12 @@ static int ath12k_core_start(struct ath12k_base *ab,
> /* ACPI is optional so continue in case of an error */
> ath12k_dbg(ab, ATH12K_DBG_BOOT, "acpi failed: %d\n", ret);
>
> + if (!test_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags)) {
> + /* Indicate the core start in the appropriate group */
> + ath12k_inc_num_core_started(ab);
> + set_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
> + }
> +
> return 0;
>
> err_reo_cleanup:
> @@ -806,6 +821,108 @@ static int ath12k_core_start(struct ath12k_base *ab,
> return ret;
> }
>
> +static void ath12k_core_device_cleanup(struct ath12k_base *ab)
> +{
> + mutex_lock(&ab->core_lock);
> +
> + if (test_and_clear_bit(ATH12K_FLAG_CORE_HIF_IRQ_ENABLED, &ab->dev_flags))
> + ath12k_hif_irq_disable(ab);
> +
> + if (test_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags))
> + ath12k_core_pdev_destroy(ab);
> +
> + if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags)) {
> + ath12k_mac_unregister(ab);
> + ath12k_mac_destroy(ab);
> + }
> +
> + mutex_unlock(&ab->core_lock);
> +}
This patch is just abusing flags and because of that we have spaghetti
code. I have been disliking use of enum ath12k_dev_flags before but this
is just looks too much. I am wondering do we need to cleanup the ath12k
architecture first, reduce the usage of flags and then revisit this
patchset?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 6/6/2024 6:34 PM, Kalle Valo wrote:
> Harshitha Prem <[email protected]> writes:
>
>> From: Karthikeyan Periyasamy <[email protected]>
>>
>> Currently, mac allocate/register and core_pdev_create are initiated
>> immediately when QMI firmware ready event is received for a particular
>> device.
>>
>> With hardware device group abstraction, QMI firmware ready event can be
>> received simultaneously for different devices in the group and so, it
>> should not be registered immediately rather it has to be deferred until
>> all devices in the group has received QMI firmware ready.
>>
>> To handle this, refactor the code of core start to move the following
>> apis inside a wrapper ath12k_core_hw_group_start()
>> * ath12k_mac_allocate()
>> * ath12k_core_pdev_create()
>> * ath12k_core_rfkill_config()
>> * ath12k_mac_register()
>> * ath12k_hif_irq_enable()
>>
>> similarly, move the corresponding destroy/unregister/disable apis
>> inside wrapper ath12k_core_hw_group_stop()
>>
>> Add the device flags to indicate pdev created and IRQ enabled which would
>> be helpful for device clean up during failure cases.
>>
>> 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-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>
>> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
>> Co-developed-by: Harshitha Prem <[email protected]>
>> Signed-off-by: Harshitha Prem <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath12k/core.c | 210 +++++++++++++++++++------
>> drivers/net/wireless/ath/ath12k/core.h | 32 ++++
>> 2 files changed, 191 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
>> index ebe31cbb6435..90c70dbfc50a 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>> @@ -563,6 +563,9 @@ u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab)
>>
>> static void ath12k_core_stop(struct ath12k_base *ab)
>> {
>> + clear_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
>> + ath12k_dec_num_core_started(ab);
>> +
>> if (!test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
>> ath12k_qmi_firmware_stop(ab);
>>
>> @@ -689,11 +692,15 @@ static int ath12k_core_pdev_create(struct ath12k_base *ab)
>> return ret;
>> }
>>
>> + set_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
>> +
>> return 0;
>> }
>>
>> static void ath12k_core_pdev_destroy(struct ath12k_base *ab)
>> {
>> + clear_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags);
>> +
>> ath12k_dp_pdev_free(ab);
>> }
>>
>> @@ -702,6 +709,8 @@ static int ath12k_core_start(struct ath12k_base *ab,
>> {
>> int ret;
>>
>> + lockdep_assert_held(&ab->core_lock);
>> +
>> ret = ath12k_wmi_attach(ab);
>> if (ret) {
>> ath12k_err(ab, "failed to attach wmi: %d\n", ret);
>> @@ -795,6 +804,12 @@ static int ath12k_core_start(struct ath12k_base *ab,
>> /* ACPI is optional so continue in case of an error */
>> ath12k_dbg(ab, ATH12K_DBG_BOOT, "acpi failed: %d\n", ret);
>>
>> + if (!test_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags)) {
>> + /* Indicate the core start in the appropriate group */
>> + ath12k_inc_num_core_started(ab);
>> + set_bit(ATH12K_FLAG_CORE_STARTED, &ab->dev_flags);
>> + }
>> +
>> return 0;
>>
>> err_reo_cleanup:
>> @@ -806,6 +821,108 @@ static int ath12k_core_start(struct ath12k_base *ab,
>> return ret;
>> }
>>
>> +static void ath12k_core_device_cleanup(struct ath12k_base *ab)
>> +{
>> + mutex_lock(&ab->core_lock);
>> +
>> + if (test_and_clear_bit(ATH12K_FLAG_CORE_HIF_IRQ_ENABLED, &ab->dev_flags))
>> + ath12k_hif_irq_disable(ab);
>> +
>> + if (test_bit(ATH12K_FLAG_PDEV_CREATED, &ab->dev_flags))
>> + ath12k_core_pdev_destroy(ab);
>> +
>> + if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags)) {
>> + ath12k_mac_unregister(ab);
>> + ath12k_mac_destroy(ab);
>> + }
>> +
>> + mutex_unlock(&ab->core_lock);
>> +}
>
> This patch is just abusing flags and because of that we have spaghetti
> code. I have been disliking use of enum ath12k_dev_flags before but this
> is just looks too much. I am wondering do we need to cleanup the ath12k
> architecture first, reduce the usage of flags and then revisit this
> patchset?
>
yeah., more dev flags :( but flags were needed for the race conditions
when multiple devices where involved in a group, some devices would have
completed till pdev create some might not. Some crashes were seen
because hif_irq_disable was called for a device in a group but that
device was not even at the stage of core register. Will check the
possibility to reduce the flag usage but it seemed necessary for
multiple device group clean up.
Thanks,
Harshitha
On 6/3/2024 9:41 PM, Jeff Johnson wrote:
> On 5/31/2024 11:04 AM, Harshitha Prem wrote:
>> To support multi-link operation, multiple devices with different bands say
>> 2 GHz or 5 GHz or 6 GHz can be combined together as a group and provide
>> an abstraction to mac80211.
>>
>> Device group abstraction - when there are multiple devices that are
>> connected by any means of communication interface between them, then these
>> devices can be combined together as a single group using a group id to form
>> a group abstraction. In ath12k driver, this abstraction would be named as
>> ath12k_hw_group (ag).
>>
>> Please find below illustration of device group abstraction with two
>> devices.
>>
>> Grouping of multiple devices (in future)
>> +------------------------------------------------------------------------+
>> | +-------------------------------------+ +-------------------+ |
>> | | +-----------+ | | +-----------+ | | +-----------+ | |
>> | | | ar (2GHz) | | | | ar (5GHz) | | | | ar (6GHz) | | |
>> | | +-----------+ | | +-----------+ | | +-----------+ | |
>> | | ath12k_base (ab) | | ath12k_base (ab) | |
>> | | (Dual band device) | | | |
>> | +-------------------------------------+ +-------------------+ |
>> | ath12k_hw_group (ag) based on group id |
>> +------------------------------------------------------------------------+
>>
>> Say for example, device 1 has two radios (2 GHz and 5 GHz band) and
>> device 2 has one radio (6 GHz).
>>
>> In existing code -
>> device 1 will have two hardware abstractions hw1 (2 GHz) and hw2
>> (5 GHz) will be registered separately to mac80211 as phy0 and phy1
>> respectively. Similarly, device 2 will register its hw (6GHz) as
>> phy2 to mac80211.
>>
>> In future, with multi-link abstraction
>>
>> combination 1 - Different group id for device1 and device 2
>> Device 1 will create a single hardware abstraction hw1
>> (2 GHz and 5 GHz) and will be registered to mac80211 as
>> phy0. similarly, device 2 will register its hardware
>> (6 GHz) to mac80211 as phy1.
>>
>> combination 2 - Same group id for device1 and device 2
>> Both device details are combined together as a group, say
>> group1, with single hardware abstraction of radios 2 GHz,
>> 5 GHz and 6 GHz band details and will be registered to
>> mac80211 as phy0.
>>
>> Add base infrastructure changes to add device grouping abstraction with
>> a single device.
>>
>> This patch series brings the base code changes with following order:
>> 1. Refactor existing code which would facilitate in introducing
>> device group abstraction.
>> 2. Create a device group abstraction during device probe.
>> 3. Start the device group only after QMI firmware ready event is
>> received for all the devices that are combined in the group.
>> 4. Move the hardware abstractions (ath12k_hw - ah) from device
>> (ath12k_base - ab) to device group abstraction (ag) as it would
>> ease in having different combinations of group abstraction that
>> can be registered to mac80211.
>>
>> v8:
>> - Addressed firmware assert issue seen during hibernation scenario in
>> "[PATCH v7 7/8] wifi: ath12k: refactor core start based on hardware group"
>>
>> v7:
>> - Added linux-wireless mailer to cc.
>> - Removed Acked-by tag from "[PATCH v6 8/8]" as it has minor change.
>>
>> v6:
>> - Addressed smatch error seen on "[PATCH v5 8/8] wifi: ath12k: move
>> ath12k_hw from per soc to group"
>> - Rebased to ToT
>> v5:
>> - on "[PATCH 8/8] wifi: ath12k: move ath12k_hw from per soc to
>> group", refactor the ath12k_mac_hw_allocate() api based on ag rather
>> than ab and update hardware abstraction array size in ath12k_hw_group
>> as ATH12K_GROUP_MAX_RADIO.
>> - Rebased to ToT
>> v4:
>> - Modified the cover letter
>> v3:
>> - Removed depends-on tag of "wifi: ath12k: Refactor the hardware recovery
>> procedures" as it is merged to ToT
>> - Addressed the deadlock warning seen during rmmod.
>>
>> v2:
>> - Rebased to ToT
>>
>>
>> Karthikeyan Periyasamy (8):
>> wifi: ath12k: Refactor core start api
>> wifi: ath12k: Add helpers to get or set ath12k_hw
>> wifi: ath12k: Add ath12k_get_num_hw api
>> wifi: ath12k: Introduce QMI firmware ready flag
>> wifi: ath12k: move ATH12K_FLAG_REGISTERED flag set to mac_register api
>> wifi: ath12k: Introduce device group abstraction
>> wifi: ath12k: refactor core start based on hardware group
>> wifi: ath12k: move ath12k_hw from per device to group
>>
>> drivers/net/wireless/ath/ath12k/core.c | 427 +++++++++++++++++++++----
>> drivers/net/wireless/ath/ath12k/core.h | 87 ++++-
>> drivers/net/wireless/ath/ath12k/dp.c | 19 +-
>> drivers/net/wireless/ath/ath12k/dp.h | 2 +-
>> drivers/net/wireless/ath/ath12k/mac.c | 117 ++++---
>> drivers/net/wireless/ath/ath12k/mac.h | 9 +-
>> drivers/net/wireless/ath/ath12k/pci.c | 2 +
>> drivers/net/wireless/ath/ath12k/qmi.c | 10 +-
>> 8 files changed, 540 insertions(+), 133 deletions(-)
>>
>>
>> base-commit: 6e7a5c6d5e38b93f9cc3289d66a597b9a4ca0403
>
> this no longer applies cleanly to ath/master or ath/ath-next,
> can you please rebase and repost?
>
Sure Jeff, will rebase and repost with comments addressed.
Thank you.