2024-04-25 05:58:30

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 0/8] wifi: ath12k: Introduce device group abstraction

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.


Depends-on:
[PATCH v2 0/3] wifi: ath12k: Refactor the hardware recovery
procedures.
Link - https://lore.kernel.org/ath12k/[email protected]/T/

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 soc to group

drivers/net/wireless/ath/ath12k/core.c | 428 +++++++++++++++++++++----
drivers/net/wireless/ath/ath12k/core.h | 88 ++++-
drivers/net/wireless/ath/ath12k/mac.c | 108 +++++--
drivers/net/wireless/ath/ath12k/mac.h | 9 +-
drivers/net/wireless/ath/ath12k/pci.c | 1 +
drivers/net/wireless/ath/ath12k/qmi.c | 10 +-
6 files changed, 527 insertions(+), 117 deletions(-)


base-commit: 326f8f68f28b0b831233acfabffb486a5b0f4717
prerequisite-patch-id: fa330433b950da57175cc84c5e97c2def1d24959
prerequisite-patch-id: 75254d9efeb5eb6e3b2027155be94334c85a76b9
prerequisite-patch-id: edd3d755bafc868bae646b54d279c2a8ba66acd1
--
2.34.1



2024-04-25 05:58:32

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 2/8] wifi: ath12k: Add helpers to get or set ath12k_hw

From: Karthikeyan Periyasamy <[email protected]>

Currently, one or more ath12k_hw is part of a device (ath12k_base) but
in future, it would be part of device group abstraction (ath12k_hw_group),
i.e., when multiple radios (ar) across different devices can be combined
together in a device group (ath12k_hw_group).

In order to facilitate the above transition, introduce helpers such as
ath12k_ab_to_ah() and ath12k_ab_set_ah() to get and set values of ath12k_hw
respectively.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 6 +++---
drivers/net/wireless/ath/ath12k/core.h | 11 +++++++++++
drivers/net/wireless/ath/ath12k/mac.c | 23 +++++++++++++----------
3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 63f0ba7bb57d..3733a6098ffc 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -950,7 +950,7 @@ static void ath12k_rfkill_work(struct work_struct *work)
spin_unlock_bh(&ab->base_lock);

for (i = 0; i < ab->num_hw; i++) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;

@@ -1002,7 +1002,7 @@ static void ath12k_core_pre_reconfigure_recovery(struct ath12k_base *ab)
set_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags);

for (i = 0; i < ab->num_hw; i++) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);
if (!ah || ah->state == ATH12K_HW_STATE_OFF)
continue;

@@ -1041,7 +1041,7 @@ static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
int i, j;

for (i = 0; i < ab->num_hw; i++) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);
if (!ah || ah->state == ATH12K_HW_STATE_OFF)
continue;

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index d833361948b7..313348e87eef 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1057,4 +1057,15 @@ static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
#define for_each_ar(ah, ar, index) \
for ((index) = 0; ((index) < (ah)->num_radio && \
((ar) = &(ah)->radio[(index)])); (index)++)
+
+static inline struct ath12k_hw *ath12k_ab_to_ah(struct ath12k_base *ab, int idx)
+{
+ return ab->ah[idx];
+}
+
+static inline void ath12k_ab_set_ah(struct ath12k_base *ab, int idx,
+ struct ath12k_hw *ah)
+{
+ ab->ah[idx] = ah;
+}
#endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 1f7e23b74632..3769a28ccff3 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -8885,7 +8885,7 @@ int ath12k_mac_register(struct ath12k_base *ab)
ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;

for (i = 0; i < ab->num_hw; i++) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);

ret = ath12k_mac_hw_register(ah);
if (ret)
@@ -8896,7 +8896,7 @@ int ath12k_mac_register(struct ath12k_base *ab)

err:
for (i = i - 1; i >= 0; i--) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;

@@ -8912,7 +8912,7 @@ void ath12k_mac_unregister(struct ath12k_base *ab)
int i;

for (i = ab->num_hw - 1; i >= 0; i--) {
- ah = ab->ah[i];
+ ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;

@@ -8970,6 +8970,7 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
void ath12k_mac_destroy(struct ath12k_base *ab)
{
struct ath12k_pdev *pdev;
+ struct ath12k_hw *ah;
int i;

for (i = 0; i < ab->num_radios; i++) {
@@ -8981,11 +8982,12 @@ void ath12k_mac_destroy(struct ath12k_base *ab)
}

for (i = 0; i < ab->num_hw; i++) {
- if (!ab->ah[i])
+ ah = ath12k_ab_to_ah(ab, i);
+ if (!ah)
continue;

- ath12k_mac_hw_destroy(ab->ah[i]);
- ab->ah[i] = NULL;
+ ath12k_mac_hw_destroy(ah);
+ ath12k_ab_set_ah(ab, i, NULL);
}
}

@@ -9016,7 +9018,7 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
goto err;
}

- ab->ah[i] = ah;
+ ath12k_ab_set_ah(ab, i, ah);
}

ath12k_dp_pdev_pre_alloc(ab);
@@ -9025,11 +9027,12 @@ int ath12k_mac_allocate(struct ath12k_base *ab)

err:
for (i = i - 1; i >= 0; i--) {
- if (!ab->ah[i])
+ ah = ath12k_ab_to_ah(ab, i);
+ if (!ah)
continue;

- ath12k_mac_hw_destroy(ab->ah[i]);
- ab->ah[i] = NULL;
+ ath12k_mac_hw_destroy(ah);
+ ath12k_ab_set_ah(ab, i, NULL);
}

return ret;
--
2.34.1


2024-04-25 05:58:42

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 5/8] wifi: ath12k: move ATH12K_FLAG_REGISTERED flag set to mac_register api

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

Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Signed-off-by: Harshitha Prem <[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 0ff526bfd104..ae0a56bb8d6d 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -8892,6 +8892,7 @@ int ath12k_mac_register(struct ath12k_base *ab)
goto err;
}

+ set_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
return 0;

err:
@@ -8911,6 +8912,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 7a1e4eb0370e..b37ab6244931 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -3327,11 +3327,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


2024-04-25 05:58:47

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 6/8] wifi: ath12k: Introduce device group abstraction

From: Karthikeyan Periyasamy <[email protected]>

Currently, single device is probed and once firmware is ready, the device
is registered to mac80211. For multi-link operation, different bands of
different devices or same device would be part of a single wiphy and for
this, hardware device group abstraction would be helpful.

Hardware device group abstraction - when there are multiple devices (with
single radio or dual radio) that are connected by any means of interface
for communicating between them, then these devices can be combined
together as a single group using a group id to form a group abstraction
and register to mac80211.

The grouping information of multiple devices would be based on device tree
during device probe. If no such information is available then a single
device will be part of group abstraction and registered to mac80211 else
multiple devices advertised in device tree are combined and then registered
to mac80211.

For device group abstraction, a base structure named ath12k_hw_group (ag)
and the following helpers are introduced:
ath12k_core_hw_group_alloc() : allocate ath12k_hw_group (ag)
based on group id and number
of devices that are going to
be part of this group.
ath12k_core_hw_group_free() : free ag during deinit.
ath12k_core_assign_hw_group() : assign/map the details of group
to ath12k_base (ab).
ath12k_core_unassign_hw_group() : unassign/unmap the details of ag
in ath12k_base (ab).
ath12k_core_hw_group_create() : create the devices which are part
of group (ag).
ath12k_core_hw_group_destroy() : cleanup the devices in ag

These helpers are used during device probe and mapping the group to the
devices involved.

Please find the illustration of how multiple devices might be combined
together in future based on group id.

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 |
+------------------------------------------------------------------------+

In the above representation, two devices are combined into single group
based on group id.

Add base code changes where single device would be part of a group with an
invalid group id forming an group abstraction. Multi device grouping will
be introduced in future.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

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 | 209 +++++++++++++++++++++++--
drivers/net/wireless/ath/ath12k/core.h | 20 +++
drivers/net/wireless/ath/ath12k/pci.c | 1 +
3 files changed, 220 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index aeb4af6842b6..bae0d36ddb24 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -21,6 +21,9 @@ unsigned int ath12k_debug_mask;
module_param_named(debug_mask, ath12k_debug_mask, uint, 0644);
MODULE_PARM_DESC(debug_mask, "Debugging mask");

+static DEFINE_MUTEX(ath12k_hw_lock);
+static struct list_head ath12k_hw_groups = LIST_HEAD_INIT(ath12k_hw_groups);
+
static int ath12k_core_rfkill_config(struct ath12k_base *ab)
{
struct ath12k *ar;
@@ -1188,20 +1191,111 @@ int ath12k_core_pre_init(struct ath12k_base *ab)
return 0;
}

-int ath12k_core_init(struct ath12k_base *ab)
+static inline
+bool ath12k_core_hw_group_create_ready(struct ath12k_hw_group *ag)
{
- int ret;
+ lockdep_assert_held(&ag->mutex_lock);

- ret = ath12k_core_soc_create(ab);
- if (ret) {
- ath12k_err(ab, "failed to create soc core: %d\n", ret);
- return ret;
+ return (ag->num_probed == ag->num_devices);
+}
+
+static struct ath12k_hw_group *
+ath12k_core_hw_group_alloc(u8 id, u8 max_devices)
+{
+ struct ath12k_hw_group *ag;
+
+ lockdep_assert_held(&ath12k_hw_lock);
+
+ ag = kzalloc(sizeof(*ag), GFP_KERNEL);
+ if (!ag)
+ return NULL;
+
+ ag->id = id;
+ ag->num_devices = max_devices;
+ list_add(&ag->list, &ath12k_hw_groups);
+ mutex_init(&ag->mutex_lock);
+
+ return ag;
+}
+
+static void ath12k_core_hw_group_free(struct ath12k_hw_group *ag)
+{
+ mutex_lock(&ath12k_hw_lock);
+
+ list_del(&ag->list);
+ kfree(ag);
+
+ mutex_unlock(&ath12k_hw_lock);
+}
+
+static struct ath12k_hw_group *ath12k_core_assign_hw_group(struct ath12k_base *ab)
+{
+ struct ath12k_hw_group *ag;
+ u32 group_id = ATH12K_INVALID_GROUP_ID;
+
+ lockdep_assert_held(&ath12k_hw_lock);
+
+ /* The grouping of multiple devices will be done based on device tree file.
+ * TODO: device tree file parsing to know about the devices involved in group.
+ *
+ * The platforms that do not have any valid group information would have each
+ * device to be part of its own invalid group.
+ *
+ * Currently, we are not parsing any device tree information and hence, grouping
+ * of multiple devices is not involved. Thus, single device is added to device
+ * group.
+ */
+ ag = ath12k_core_hw_group_alloc(group_id, 1);
+ if (!ag) {
+ ath12k_warn(ab, "unable to create new hw group\n");
+ return NULL;
}
+ ath12k_dbg(ab, ATH12K_DBG_BOOT, "Single device is added to hardware group\n");

- return 0;
+ ab->device_id = ag->num_probed++;
+ ag->ab[ab->device_id] = ab;
+ ab->ag = ag;
+
+ return ag;
}

-void ath12k_core_deinit(struct ath12k_base *ab)
+void ath12k_core_unassign_hw_group(struct ath12k_base *ab)
+{
+ struct ath12k_hw_group *ag = ab->ag;
+ u8 device_id = ab->device_id;
+ int num_probed;
+
+ if (!ag)
+ return;
+
+ mutex_lock(&ag->mutex_lock);
+
+ if (WARN_ON(device_id >= ag->num_devices)) {
+ mutex_unlock(&ag->mutex_lock);
+ return;
+ }
+
+ if (WARN_ON(ag->ab[device_id] != ab)) {
+ mutex_unlock(&ag->mutex_lock);
+ return;
+ }
+
+ ag->ab[device_id] = NULL;
+ ab->ag = NULL;
+ ab->device_id = ATH12K_INVALID_DEVICE_ID;
+
+ if (ag->num_probed)
+ ag->num_probed--;
+
+ num_probed = ag->num_probed;
+
+ mutex_unlock(&ag->mutex_lock);
+
+ if (!num_probed)
+ ath12k_core_hw_group_free(ag);
+}
+
+static void ath12k_core_device_cleanup(struct ath12k_base *ab)
{
mutex_lock(&ab->core_lock);

@@ -1212,9 +1306,104 @@ void ath12k_core_deinit(struct ath12k_base *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;
+ int i;
+
+ if (!ag)
+ return;
+
+ mutex_lock(&ag->mutex_lock);
+
+ 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);
+
+ if (test_and_clear_bit(ATH12K_FLAG_HW_GROUP_ATTACHED, &ab->dev_flags))
+ ath12k_core_soc_destroy(ab);
+
+ ath12k_fw_unmap(ab);
+ }
+
+ mutex_unlock(&ag->mutex_lock);
+}

- ath12k_core_soc_destroy(ab);
- ath12k_fw_unmap(ab);
+static int ath12k_core_hw_group_create(struct ath12k_hw_group *ag)
+{
+ int i, ret;
+ struct ath12k_base *ab;
+
+ 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);
+ ret = ath12k_core_soc_create(ab);
+ if (ret) {
+ mutex_unlock(&ab->core_lock);
+ ath12k_err(ab, "failed to create soc core: %d\n", ret);
+ return ret;
+ }
+ set_bit(ATH12K_FLAG_HW_GROUP_ATTACHED, &ab->dev_flags);
+ mutex_unlock(&ab->core_lock);
+ }
+
+ return 0;
+}
+
+int ath12k_core_init(struct ath12k_base *ab)
+{
+ struct ath12k_hw_group *ag;
+ int ret;
+
+ mutex_lock(&ath12k_hw_lock);
+ ag = ath12k_core_assign_hw_group(ab);
+ if (!ag) {
+ mutex_unlock(&ath12k_hw_lock);
+ ath12k_warn(ab, "unable to get hw group\n");
+ return -ENODEV;
+ }
+ mutex_unlock(&ath12k_hw_lock);
+
+ mutex_lock(&ag->mutex_lock);
+
+ ath12k_dbg(ab, ATH12K_DBG_BOOT, "num devices in group %d, num probed %d\n",
+ ag->num_devices, ag->num_probed);
+
+ if (ath12k_core_hw_group_create_ready(ag)) {
+ ret = ath12k_core_hw_group_create(ag);
+ if (ret) {
+ mutex_unlock(&ag->mutex_lock);
+ ath12k_warn(ab, "unable to create hw group\n");
+ goto err_hw_group;
+ }
+ }
+
+ mutex_unlock(&ag->mutex_lock);
+
+ return 0;
+
+err_hw_group:
+ ath12k_core_hw_group_destroy(ab->ag);
+ ath12k_core_unassign_hw_group(ab);
+
+ return ret;
+}
+
+void ath12k_core_deinit(struct ath12k_base *ab)
+{
+ ath12k_core_hw_group_destroy(ab->ag);
+ ath12k_core_unassign_hw_group(ab);
}

void ath12k_core_free(struct ath12k_base *ab)
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 58230aecb7b9..0bb05bf32eb4 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -59,6 +59,10 @@
#define ATH12K_RECONFIGURE_TIMEOUT_HZ (10 * HZ)
#define ATH12K_RECOVER_START_TIMEOUT_HZ (20 * HZ)

+#define ATH12K_MAX_SOCS 3
+#define ATH12K_INVALID_GROUP_ID 0xFF
+#define ATH12K_INVALID_DEVICE_ID 0xFF
+
enum ath12k_bdf_search {
ATH12K_BDF_SEARCH_DEFAULT,
ATH12K_BDF_SEARCH_BUS_AND_BOARD,
@@ -211,6 +215,7 @@ enum ath12k_dev_flags {
ATH12K_FLAG_CE_IRQ_ENABLED,
ATH12K_FLAG_EXT_IRQ_ENABLED,
ATH12K_FLAG_QMI_FW_READY_COMPLETE,
+ ATH12K_FLAG_HW_GROUP_ATTACHED,
};

enum ath12k_monitor_flags {
@@ -732,6 +737,17 @@ struct ath12k_soc_dp_stats {
struct ath12k_soc_dp_tx_err_stats tx_err;
};

+/* Holds info on the group of devices that are registered as a single wiphy */
+struct ath12k_hw_group {
+ struct list_head list;
+ u8 id;
+ u8 num_devices;
+ u8 num_probed;
+ struct ath12k_base *ab[ATH12K_MAX_SOCS];
+ /* To synchronize group create, assign, start, stop */
+ struct mutex mutex_lock;
+};
+
/**
* enum ath12k_link_capable_flags - link capable flags
*
@@ -931,6 +947,9 @@ struct ath12k_base {

#endif /* CONFIG_ACPI */

+ struct ath12k_hw_group *ag;
+ u8 device_id;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
@@ -961,6 +980,7 @@ int ath12k_core_resume_early(struct ath12k_base *ab);
int ath12k_core_resume(struct ath12k_base *ab);
int ath12k_core_suspend(struct ath12k_base *ab);
int ath12k_core_suspend_late(struct ath12k_base *ab);
+void ath12k_core_unassign_hw_group(struct ath12k_base *ab);

const struct firmware *ath12k_core_firmware_request(struct ath12k_base *ab,
const char *filename);
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 16af046c33d9..dd9e593e7b5d 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -1505,6 +1505,7 @@ static void ath12k_pci_remove(struct pci_dev *pdev)
if (test_bit(ATH12K_FLAG_QMI_FAIL, &ab->dev_flags)) {
ath12k_pci_power_down(ab, false);
ath12k_qmi_deinit_service(ab);
+ ath12k_core_unassign_hw_group(ab);
goto qmi_fail;
}

--
2.34.1


2024-04-25 05:58:48

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 7/8] wifi: ath12k: refactor core start based on hardware group

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

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 | 217 +++++++++++++++++++------
drivers/net/wireless/ath/ath12k/core.h | 32 ++++
2 files changed, 198 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index bae0d36ddb24..903d51a303d4 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -566,6 +566,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);
+
ath12k_acpi_stop(ab);

ath12k_dp_rx_pdev_reo_cleanup(ab);
@@ -692,11 +695,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);
}

@@ -705,6 +712,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);
@@ -798,6 +807,11 @@ 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);

+ /* 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:
@@ -809,6 +823,109 @@ 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;
+ }
+
+ ret = ath12k_core_rfkill_config(ab);
+ if (ret && ret != -EOPNOTSUPP) {
+ mutex_unlock(&ab->core_lock);
+ goto err;
+ }
+
+ ath12k_hif_irq_enable(ab);
+
+ set_bit(ATH12K_FLAG_CORE_HIF_IRQ_ENABLED, &ab->dev_flags);
+
+ 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)
{
@@ -826,9 +943,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;
+ int ret, i;

ret = ath12k_core_start_firmware(ab, ATH12K_FIRMWARE_MODE_NORMAL);
if (ret) {
@@ -848,59 +974,50 @@ int ath12k_core_qmi_firmware_ready(struct ath12k_base *ab)
goto err_firmware_stop;
}

+ ag = ath12k_ab_to_ag(ab);
+
+ 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;
}

@@ -1152,6 +1269,10 @@ static void ath12k_core_reset(struct work_struct *work)

ath12k_dbg(ab, ATH12K_DBG_BOOT, "reset starting\n");

+ mutex_lock(&ab->ag->mutex_lock);
+ ath12k_dec_num_core_started(ab);
+ mutex_unlock(&ab->ag->mutex_lock);
+
ab->is_reset = true;
atomic_set(&ab->recovery_start_count, 0);
reinit_completion(&ab->recovery_start);
@@ -1261,7 +1382,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;

@@ -1295,19 +1416,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;
@@ -1318,13 +1426,20 @@ static void ath12k_core_hw_group_destroy(struct ath12k_hw_group *ag)

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

if (test_and_clear_bit(ATH12K_FLAG_HW_GROUP_ATTACHED, &ab->dev_flags))
ath12k_core_soc_destroy(ab);
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 0bb05bf32eb4..a5bf4346ba88 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -202,6 +202,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,
@@ -216,6 +220,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,
};

enum ath12k_monitor_flags {
@@ -743,6 +750,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;
@@ -1094,4 +1103,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


2024-04-25 05:58:57

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 8/8] wifi: ath12k: move ath12k_hw from per soc to group

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

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/mac.c | 81 ++++++++++++++++++--------
drivers/net/wireless/ath/ath12k/mac.h | 9 +--
4 files changed, 98 insertions(+), 69 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 903d51a303d4..b304895ee295 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -833,11 +833,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);
}

@@ -854,6 +849,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)
@@ -864,6 +861,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)
@@ -871,31 +885,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);
@@ -922,7 +911,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 a5bf4346ba88..1f950226931a 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

@@ -755,6 +756,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[MAX_RADIOS];
+ u8 num_hw;
};

/**
@@ -825,15 +835,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;
@@ -1090,18 +1091,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/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index ae0a56bb8d6d..1df1c2b2a320 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -8871,19 +8871,13 @@ static void ath12k_mac_setup(struct ath12k *ar)
clear_bit(ATH12K_FLAG_MONITOR_ENABLED, &ar->monitor_flags);
}

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

@@ -8907,8 +8901,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;

@@ -8969,18 +8964,24 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
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++) {
@@ -8993,20 +8994,46 @@ void ath12k_mac_destroy(struct ath12k_base *ab)
}
}

-int ath12k_mac_allocate(struct ath12k_base *ab)
+static void ath12k_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;
+ ath12k_dp_pdev_pre_alloc(ab);
+}
+
+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;
+
+ 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++) {
+ ab = ag->ab[device_id];

- for (i = 0; i < ath12k_get_num_hw(ab); i++) {
for (j = 0; j < radio_per_hw; j++) {
pdev_map[j].ab = ab;
pdev_map[j].pdev_idx = (i * radio_per_hw) + j;
@@ -9020,11 +9047,19 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
goto err;
}

- ath12k_ab_set_ah(ab, i, ah);
+ mac_id++;
+ /* If mac_id falls beyond the current device MACs then
+ * move to next device
+ */
+ if (mac_id >= ab->num_radios) {
+ device_id++;
+ mac_id = 0;
+ ath12k_set_device_defaults(ab);
+ }
+ 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


2024-04-25 05:59:27

by Harshitha Prem

[permalink] [raw]
Subject: [PATCH v2 3/8] wifi: ath12k: Add ath12k_get_num_hw api

From: Karthikeyan Periyasamy <[email protected]>

Currently, one or more ath12k_hw is part of device (ath12k_base) but
in future, ath12k_hw would be part of device group (ath12k_hw_group).
Hence, num_hw under device would be moved to device group.

To facilitate above transition, add helper ath12k_get_num_hw() api to
get the number of radios per device. In future, this helper would be
able to get the number of radios in a device group.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Signed-off-by: Harshitha Prem <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 6 +++---
drivers/net/wireless/ath/ath12k/core.h | 5 +++++
drivers/net/wireless/ath/ath12k/mac.c | 8 ++++----
3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 3733a6098ffc..aeb4af6842b6 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -949,7 +949,7 @@ static void ath12k_rfkill_work(struct work_struct *work)
rfkill_radio_on = ab->rfkill_radio_on;
spin_unlock_bh(&ab->base_lock);

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;
@@ -1001,7 +1001,7 @@ static void ath12k_core_pre_reconfigure_recovery(struct ath12k_base *ab)
if (ab->is_reset)
set_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags);

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah || ah->state == ATH12K_HW_STATE_OFF)
continue;
@@ -1040,7 +1040,7 @@ static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
struct ath12k *ar;
int i, j;

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah || ah->state == ATH12K_HW_STATE_OFF)
continue;
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 313348e87eef..db6124c8f660 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1068,4 +1068,9 @@ static inline void ath12k_ab_set_ah(struct ath12k_base *ab, int idx,
{
ab->ah[idx] = ah;
}
+
+static inline int ath12k_get_num_hw(struct ath12k_base *ab)
+{
+ return ab->num_hw;
+}
#endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 3769a28ccff3..0ff526bfd104 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -8884,7 +8884,7 @@ int ath12k_mac_register(struct ath12k_base *ab)
ab->cc_freq_hz = 320000;
ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);

ret = ath12k_mac_hw_register(ah);
@@ -8911,7 +8911,7 @@ void ath12k_mac_unregister(struct ath12k_base *ab)
struct ath12k_hw *ah;
int i;

- for (i = ab->num_hw - 1; i >= 0; i--) {
+ for (i = ath12k_get_num_hw(ab) - 1; i >= 0; i--) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;
@@ -8981,7 +8981,7 @@ void ath12k_mac_destroy(struct ath12k_base *ab)
pdev->ar = NULL;
}

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;
@@ -9004,7 +9004,7 @@ int ath12k_mac_allocate(struct ath12k_base *ab)
ab->num_hw = ab->num_radios;
radio_per_hw = 1;

- for (i = 0; i < ab->num_hw; i++) {
+ for (i = 0; i < ath12k_get_num_hw(ab); i++) {
for (j = 0; j < radio_per_hw; j++) {
pdev_map[j].ab = ab;
pdev_map[j].pdev_idx = (i * radio_per_hw) + j;
--
2.34.1


2024-04-25 21:21:41

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] wifi: ath12k: Introduce device group abstraction

On 4/24/2024 10:57 PM, 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.
>
>
> Depends-on:
> [PATCH v2 0/3] wifi: ath12k: Refactor the hardware recovery
> procedures.
> Link - https://lore.kernel.org/ath12k/[email protected]/T/

current dependency link (v3):
https://lore.kernel.org/ath12k/[email protected]

I verified this series cleanly applies on top of that

>
> 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 soc to group
>
> drivers/net/wireless/ath/ath12k/core.c | 428 +++++++++++++++++++++----
> drivers/net/wireless/ath/ath12k/core.h | 88 ++++-
> drivers/net/wireless/ath/ath12k/mac.c | 108 +++++--
> drivers/net/wireless/ath/ath12k/mac.h | 9 +-
> drivers/net/wireless/ath/ath12k/pci.c | 1 +
> drivers/net/wireless/ath/ath12k/qmi.c | 10 +-
> 6 files changed, 527 insertions(+), 117 deletions(-)
>
>
> base-commit: 326f8f68f28b0b831233acfabffb486a5b0f4717
> prerequisite-patch-id: fa330433b950da57175cc84c5e97c2def1d24959
> prerequisite-patch-id: 75254d9efeb5eb6e3b2027155be94334c85a76b9
> prerequisite-patch-id: edd3d755bafc868bae646b54d279c2a8ba66acd1


2024-04-25 21:29:23

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 2/8] wifi: ath12k: Add helpers to get or set ath12k_hw

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> From: Karthikeyan Periyasamy <[email protected]>
>
> Currently, one or more ath12k_hw is part of a device (ath12k_base) but
> in future, it would be part of device group abstraction (ath12k_hw_group),
> i.e., when multiple radios (ar) across different devices can be combined
> together in a device group (ath12k_hw_group).
>
> In order to facilitate the above transition, introduce helpers such as
> ath12k_ab_to_ah() and ath12k_ab_set_ah() to get and set values of ath12k_hw
> respectively.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>


2024-04-25 21:29:40

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 3/8] wifi: ath12k: Add ath12k_get_num_hw api

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> From: Karthikeyan Periyasamy <[email protected]>
>
> Currently, one or more ath12k_hw is part of device (ath12k_base) but
> in future, ath12k_hw would be part of device group (ath12k_hw_group).
> Hence, num_hw under device would be moved to device group.
>
> To facilitate above transition, add helper ath12k_get_num_hw() api to
> get the number of radios per device. In future, this helper would be
> able to get the number of radios in a device group.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>


2024-04-25 21:30:31

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] wifi: ath12k: move ATH12K_FLAG_REGISTERED flag set to mac_register api

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> 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
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>



2024-04-25 21:32:41

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] wifi: ath12k: Introduce device group abstraction

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> From: Karthikeyan Periyasamy <[email protected]>
>
> Currently, single device is probed and once firmware is ready, the device
> is registered to mac80211. For multi-link operation, different bands of
> different devices or same device would be part of a single wiphy and for
> this, hardware device group abstraction would be helpful.
>
> Hardware device group abstraction - when there are multiple devices (with
> single radio or dual radio) that are connected by any means of interface
> for communicating between them, then these devices can be combined
> together as a single group using a group id to form a group abstraction
> and register to mac80211.
>
> The grouping information of multiple devices would be based on device tree
> during device probe. If no such information is available then a single
> device will be part of group abstraction and registered to mac80211 else
> multiple devices advertised in device tree are combined and then registered
> to mac80211.
>
> For device group abstraction, a base structure named ath12k_hw_group (ag)
> and the following helpers are introduced:
> ath12k_core_hw_group_alloc() : allocate ath12k_hw_group (ag)
> based on group id and number
> of devices that are going to
> be part of this group.
> ath12k_core_hw_group_free() : free ag during deinit.
> ath12k_core_assign_hw_group() : assign/map the details of group
> to ath12k_base (ab).
> ath12k_core_unassign_hw_group() : unassign/unmap the details of ag
> in ath12k_base (ab).
> ath12k_core_hw_group_create() : create the devices which are part
> of group (ag).
> ath12k_core_hw_group_destroy() : cleanup the devices in ag
>
> These helpers are used during device probe and mapping the group to the
> devices involved.
>
> Please find the illustration of how multiple devices might be combined
> together in future based on group id.
>
> 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 |
> +------------------------------------------------------------------------+
>
> In the above representation, two devices are combined into single group
> based on group id.
>
> Add base code changes where single device would be part of a group with an
> invalid group id forming an group abstraction. Multi device grouping will
> be introduced in future.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Co-developed-by: Harshitha Prem <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>


2024-04-25 21:45:15

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] wifi: ath12k: refactor core start based on hardware group

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> 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
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Co-developed-by: Harshitha Prem <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>


2024-04-25 21:45:33

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] wifi: ath12k: move ath12k_hw from per soc to group

On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> 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
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Signed-off-by: Harshitha Prem <[email protected]>

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Acked-by: Jeff Johnson <[email protected]>


2024-04-26 02:34:12

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH v2 8/8] wifi: ath12k: move ath12k_hw from per soc to group

Hi Jeff,

Jeff Johnson <[email protected]> wrote:
>
> On 4/24/2024 10:57 PM, Harshitha Prem wrote:
> > From: Karthikeyan Periyasamy <[email protected]>
> >
> > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> >
> > Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> > Signed-off-by: Harshitha Prem <[email protected]>
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Acked-by: Jeff Johnson <[email protected]>
>

Will patchwork take 'Tested-on' tag you added automatically? Or you have a
local script to add that tag. I use pwclient to grab this patch, but only
your acked-by is added automatically.

I have this question just because I want to refine my workflow of patchwork.

Ping-Ke

2024-04-26 14:39:16

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] wifi: ath12k: move ath12k_hw from per soc to group

On 4/25/2024 7:33 PM, Ping-Ke Shih wrote:
> Hi Jeff,
>
> Jeff Johnson <[email protected]> wrote:
>>
>> On 4/24/2024 10:57 PM, Harshitha Prem wrote:
>>> From: Karthikeyan Periyasamy <[email protected]>
>>>
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
>>> Signed-off-by: Harshitha Prem <[email protected]>
>>
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Acked-by: Jeff Johnson <[email protected]>
>>
>
> Will patchwork take 'Tested-on' tag you added automatically? Or you have a
> local script to add that tag. I use pwclient to grab this patch, but only
> your acked-by is added automatically.
>
> I have this question just because I want to refine my workflow of patchwork.
>
> Ping-Ke
>

Tested-on: is not an 'official' upstream tag -- it is our own invention. That
is why there is a blank line between those tags and the official tags. So
those tags are always managed manually.

/jeff


2024-04-30 11:55:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] wifi: ath12k: Introduce device group abstraction

Harshitha Prem <[email protected]> writes:

> 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.
>
>
> Depends-on:
> [PATCH v2 0/3] wifi: ath12k: Refactor the hardware recovery
> procedures.
> Link - https://lore.kernel.org/ath12k/[email protected]/T/
>
> 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 soc to group

I see a deadlock warning in master-pending branch (tag
ath-pending-202404291731) and based on manual bisect between patchsets
it seems to come from this patchset. Do note that I didn't look at the
patchset otherwise.

Here's the warning I see during rmmod with WCN7850 and I see it every
time:

[ 147.211487] ======================================================
[ 147.211547] WARNING: possible circular locking dependency detected
[ 147.211599] 6.9.0-rc5-wt-ath+ #1403 Not tainted
[ 147.211646] ------------------------------------------------------
[ 147.211695] rmmod/1975 is trying to acquire lock:
[ 147.211741] ffff888105c7f158 ((wq_completion)ath12k_qmi_driver_event){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x46/0xf0
[ 147.211815] #012[ 147.211815] but task is already holding lock:
[ 147.211864] ffff88810db290a8 (&ag->mutex_lock){+.+.}-{3:3}, at: ath12k_core_hw_group_destroy.part.0+0x26/0x290 [ath12k]
[ 147.212003] #012[ 147.212003] which lock already depends on the new lock.#012[ 147.212003]
[ 147.212069] #012[ 147.212069] the existing dependency chain (in reverse order) is:
[ 147.212135] #012[ 147.212135] -> #2 (&ag->mutex_lock){+.+.}-{3:3}:
[ 147.212189] __lock_acquire+0xd43/0x1dd0
[ 147.212238] lock_acquire+0x1b0/0x560
[ 147.212280] __mutex_lock+0x154/0x1430
[ 147.212327] mutex_lock_nested+0x16/0x20
[ 147.212369] ath12k_core_qmi_firmware_ready+0x9d/0x400 [ath12k]
[ 147.212436] ath12k_qmi_driver_event_work+0x4e9/0x6e0 [ath12k]
[ 147.212507] process_one_work+0x8a4/0x1980
[ 147.212551] worker_thread+0x715/0x1270
[ 147.212594] kthread+0x2fa/0x3f0
[ 147.212636] ret_from_fork+0x31/0x70
[ 147.212679] ret_from_fork_asm+0x11/0x20
[ 147.212754] #012[ 147.212754] -> #1 ((work_completion)(&ab->qmi.event_work)){+.+.}-{0:0}:
[ 147.212828] __lock_acquire+0xd43/0x1dd0
[ 147.212885] lock_acquire+0x1b0/0x560
[ 147.213000] process_one_work+0x82d/0x1980
[ 147.213055] worker_thread+0x715/0x1270
[ 147.213779] kthread+0x2fa/0x3f0
[ 147.214529] ret_from_fork+0x31/0x70
[ 147.215291] ret_from_fork_asm+0x11/0x20
[ 147.216044] #012[ 147.216044] -> #0 ((wq_completion)ath12k_qmi_driver_event){+.+.}-{0:0}:
[ 147.217433] check_prev_add+0x1bd/0x2330
[ 147.218174] validate_chain+0xf4e/0x1cf0
[ 147.218853] __lock_acquire+0xd43/0x1dd0
[ 147.219589] lock_acquire+0x1b0/0x560
[ 147.220324] touch_wq_lockdep_map+0x66/0xf0
[ 147.221055] __flush_workqueue+0xeb/0x1120
[ 147.221731] drain_workqueue+0xf5/0x320
[ 147.222460] destroy_workqueue+0xb2/0x920
[ 147.223187] ath12k_qmi_deinit_service+0x5a/0x1f0 [ath12k]
[ 147.223876] ath12k_core_hw_group_destroy.part.0+0x1f8/0x290 [ath12k]
[ 147.224606] ath12k_core_deinit+0x37/0x50 [ath12k]
[ 147.225314] ath12k_pci_remove+0xad/0x1b0 [ath12k]
[ 147.226640] pci_device_remove+0x9b/0x1b0
[ 147.227326] device_remove+0xbf/0x150
[ 147.227986] device_release_driver_internal+0x3c3/0x580
[ 147.228619] driver_detach+0xc4/0x190
[ 147.229290] bus_remove_driver+0x130/0x2a0
[ 147.229900] driver_unregister+0x68/0x90
[ 147.230576] pci_unregister_driver+0x24/0x240
[ 147.231240] ath12k_pci_exit+0x10/0x20 [ath12k]
[ 147.231866] __do_sys_delete_module+0x32c/0x580
[ 147.232523] __x64_sys_delete_module+0x4f/0x70
[ 147.233168] x64_sys_call+0x51b/0x9e0
[ 147.233756] do_syscall_64+0x65/0x130
[ 147.234396] entry_SYSCALL_64_after_hwframe+0x4b/0x53
[ 147.235023] #012[ 147.235023] other info that might help us debug this:#012[ 147.235023]
[ 147.236688] Chain exists of:#012[ 147.236688] (wq_completion)ath12k_qmi_driver_event --> (work_completion)(&ab->qmi.event_work) --> &ag->mutex_lock#012[ 147.236688]
[ 147.238414] Possible unsafe locking scenario:#012[ 147.238414]
[ 147.239520] CPU0 CPU1
[ 147.240114] ---- ----
[ 147.240633] lock(&ag->mutex_lock);
[ 147.241212] lock((work_completion)(&ab->qmi.event_work));
[ 147.241746] lock(&ag->mutex_lock);
[ 147.242341] lock((wq_completion)ath12k_qmi_driver_event);
[ 147.242876] #012[ 147.242876] *** DEADLOCK ***#012[ 147.242876]
[ 147.244460] 2 locks held by rmmod/1975:
[ 147.245040] #0: ffff88810d7191b8 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0x9d/0x580
[ 147.245597] #1: ffff88810db290a8 (&ag->mutex_lock){+.+.}-{3:3}, at: ath12k_core_hw_group_destroy.part.0+0x26/0x290 [ath12k]
[ 147.246230] #012[ 147.246230] stack backtrace:
[ 147.247338] CPU: 5 PID: 1975 Comm: rmmod Not tainted 6.9.0-rc5-wt-ath+ #1403
[ 147.247900] Hardware name: Intel(R) Client Systems NUC8i7HVK/NUC8i7HVB, BIOS HNKBLi70.86A.0067.2021.0528.1339 05/28/2021
[ 147.248545] Call Trace:
[ 147.249181] <TASK>
[ 147.249757] dump_stack_lvl+0x7d/0xe0
[ 147.250401] dump_stack+0x10/0x20
[ 147.251031] print_circular_bug+0x2e8/0x480
[ 147.251606] check_noncircular+0x2f2/0x3d0
[ 147.252235] ? print_circular_bug+0x480/0x480
[ 147.252808] ? validate_chain+0x15e/0x1cf0
[ 147.253435] ? __kasan_check_read+0x11/0x20
[ 147.254065] ? mark_lock+0xe6/0x1470
[ 147.254636] ? alloc_chain_hlocks+0x4cc/0x790
[ 147.255265] check_prev_add+0x1bd/0x2330
[ 147.255839] ? __kasan_check_read+0x11/0x20
[ 147.256474] validate_chain+0xf4e/0x1cf0
[ 147.257104] ? check_prev_add+0x2330/0x2330
[ 147.257673] __lock_acquire+0xd43/0x1dd0
[ 147.258303] lock_acquire+0x1b0/0x560
[ 147.258875] ? touch_wq_lockdep_map+0x46/0xf0
[ 147.259502] ? lock_sync+0x1a0/0x1a0
[ 147.260131] ? __lock_acquired+0x208/0x810
[ 147.260699] ? lockdep_init_map_type+0x1a3/0x850
[ 147.261326] ? lockdep_init_map_type+0x1a3/0x850
[ 147.261892] ? touch_wq_lockdep_map+0x46/0xf0
[ 147.262513] touch_wq_lockdep_map+0x66/0xf0
[ 147.263126] ? touch_wq_lockdep_map+0x46/0xf0
[ 147.263684] __flush_workqueue+0xeb/0x1120
[ 147.264301] ? drain_workqueue+0xae/0x320
[ 147.264862] ? drain_workqueue+0xae/0x320
[ 147.265471] ? __this_cpu_preempt_check+0x13/0x20
[ 147.266086] ? wq_update_node_max_active+0x540/0x540
[ 147.266640] ? destroy_workqueue+0xaa/0x920
[ 147.267249] ? __this_cpu_preempt_check+0x13/0x20
[ 147.267806] ? bit_wait_timeout+0x160/0x160
[ 147.268420] ? __kasan_check_write+0x14/0x20
[ 147.269031] drain_workqueue+0xf5/0x320
[ 147.269586] destroy_workqueue+0xb2/0x920
[ 147.270202] ath12k_qmi_deinit_service+0x5a/0x1f0 [ath12k]
[ 147.270785] ? debugfs_remove+0x52/0x60
[ 147.271400] ath12k_core_hw_group_destroy.part.0+0x1f8/0x290 [ath12k]
[ 147.272046] ath12k_core_deinit+0x37/0x50 [ath12k]
[ 147.272628] ath12k_pci_remove+0xad/0x1b0 [ath12k]
[ 147.273271] pci_device_remove+0x9b/0x1b0
[ 147.273829] device_remove+0xbf/0x150
[ 147.274446] device_release_driver_internal+0x3c3/0x580
[ 147.275052] ? __kasan_check_read+0x11/0x20
[ 147.275592] driver_detach+0xc4/0x190
[ 147.276189] bus_remove_driver+0x130/0x2a0
[ 147.276728] driver_unregister+0x68/0x90
[ 147.277325] pci_unregister_driver+0x24/0x240
[ 147.277876] ? find_module_all+0x13e/0x1e0
[ 147.278469] ath12k_pci_exit+0x10/0x20 [ath12k]
[ 147.279087] __do_sys_delete_module+0x32c/0x580
[ 147.279626] ? __kasan_slab_free+0x102/0x170
[ 147.280209] ? module_flags+0x2f0/0x2f0
[ 147.280741] ? kmem_cache_free+0xed/0x3e0
[ 147.281334] ? __fput+0x40c/0xa60
[ 147.281870] ? __fput+0x40c/0xa60
[ 147.282462] ? debug_smp_processor_id+0x17/0x20
[ 147.283045] __x64_sys_delete_module+0x4f/0x70
[ 147.283558] x64_sys_call+0x51b/0x9e0
[ 147.284125] do_syscall_64+0x65/0x130
[ 147.284613] entry_SYSCALL_64_after_hwframe+0x4b/0x53
[ 147.285159] RIP: 0033:0x7f2df2adcc8b
[ 147.286954] Code: 73 01 c3 48 8b 0d 05 c2 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d d5 c1 0c 00 f7 d8 64 89 01 48
[ 147.288033] RSP: 002b:00007ffdc52ca298 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[ 147.288563] RAX: ffffffffffffffda RBX: 000055d10d0237e0 RCX: 00007f2df2adcc8b
[ 147.289155] RDX: 000000000000000a RSI: 0000000000000800 RDI: 000055d10d023848
[ 147.289691] RBP: 00007ffdc52ca2f8 R08: 0000000000000000 R09: 0000000000000000
[ 147.290292] R10: 00007f2df2b58ac0 R11: 0000000000000206 R12: 00007ffdc52ca4d0
[ 147.290830] R13: 00007ffdc52caebf R14: 000055d10d0222a0 R15: 000055d10d0237e0
[ 147.291444] </TASK>

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

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

2024-05-01 15:37:07

by Harshitha Prem

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] wifi: ath12k: Introduce device group abstraction



On 4/30/2024 5:24 PM, Kalle Valo wrote:
> 47] WARNING: possible circular locking dependency detected
> [ 147.211599] 6.9.0-rc5-wt-ath+ #1403 Not tainted

Thank you, Kalle, for sharing the full logs. Unfortunately, I did not
see this warning in my testbed with QCN9274.

Seems like during rmmod case, ath12k_core_soc_destroy() is invoked with
ag->mutex lock which in-turn invokes ath12k_qmi_deinit_service() where
cancel_work_sync for qmi events are called. So, if
ATH12K_QMI_EVENT_FW_READY was in queue during rmmod, it would hit a
deadlock as in core start it requires ag->mutex lock. But, this scenario
is quite unlikely to occur where rmmod and FW_READY event are going to
be in parallel. Nevertheless, it has to be addressed. I will check
further on this warning and see how to address this.

Thanks,
Harshitha

2024-05-01 16:55:43

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] wifi: ath12k: Introduce device group abstraction

On 5/1/2024 8:34 AM, Harshitha Prem wrote:
>
>
> On 4/30/2024 5:24 PM, Kalle Valo wrote:
>> 47] WARNING: possible circular locking dependency detected
>> [ 147.211599] 6.9.0-rc5-wt-ath+ #1403 Not tainted
>
> Thank you, Kalle, for sharing the full logs. Unfortunately, I did not
> see this warning in my testbed with QCN9274.

I didn't see it in my testing either, which caused me to look at my .config
and notice that I didn't have any of the lock debugging enabled.

Have fixed that now.