diff mbox series

[v8,6/8] wifi: ath12k: Introduce device group abstraction

Message ID 20240531180411.1149605-7-quic_hprem@quicinc.com
State Superseded
Headers show
Series wifi: ath12k: Introduce device group abstraction | expand

Commit Message

Harshitha Prem May 31, 2024, 6:04 p.m. UTC
From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>

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
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 <quic_periyasa@quicinc.com>
Co-developed-by: Harshitha Prem <quic_hprem@quicinc.com>
Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 218 ++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath12k/core.h |  19 +++
 drivers/net/wireless/ath/ath12k/pci.c  |   1 +
 3 files changed, 229 insertions(+), 9 deletions(-)

Comments

Kalle Valo June 6, 2024, 1:20 p.m. UTC | #1
Harshitha Prem <quic_hprem@quicinc.com> writes:

> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>
> 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                 |
> +------------------------------------------------------------------------+

This is a good diagram, thanks for that. But how does struct ath12k_hw
fit into the diagram?

> 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
> 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 <quic_periyasa@quicinc.com>
> Co-developed-by: Harshitha Prem <quic_hprem@quicinc.com>
> Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Like I have said before, for adding any new locks there needs to be a
proper analysis for the locking and good justifications why new locks
are needed. I don't see any of that above.

BTW I will from now on require proper analysis also for additions to
enum ath12k_dev_flags.

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

I can somehow understand/guess why this mutex is needed (even though
there's no documentation) but the naming is not really clear as we
already have struct ath12k_hw::hw_mutex.

> +/* 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;
> +};

But why we really need this mutex? And does it really justify the extra
complexity it creates?
Kalle Valo June 6, 2024, 3:58 p.m. UTC | #2
Harshitha Prem <quic_hprem@quicinc.com> writes:

> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>
> 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.

BTW I'm not sure if Device Tree is the right approach to configure the
groups. For example, that would mean that systems not using DT would not
support MLO at all, right? And also it would not be very flexible to
change the group setup.
Harshitha Prem June 7, 2024, 1:29 p.m. UTC | #3
On 6/6/2024 6:50 PM, Kalle Valo wrote:
> Harshitha Prem <quic_hprem@quicinc.com> writes:
> 
>> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>>
>> 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                 |
>> +------------------------------------------------------------------------+
> 
> This is a good diagram, thanks for that. But how does struct ath12k_hw
> fit into the diagram?
> 
Currently with this base device group series, ath12k_hw sits above 
ath12k_base.

Please find the representation below:

+-----------------------------------------------+
|  +-------------------+  +-------------------+ |
|  |   +---------+     |  |   +---------+     | |
|  |   |         |     |  |   |         |     | |
|  |   | mac80211|     |  |   | mac80211|     | |
|  |   | hw->priv|     |  |   | hw->priv|     | |
|  |   |         |     |  |   |         |     | |
|  |   +---------+     |  |   +---------+     | |
|  |   +---------+     |  |   +---------+     | |
|  |   |         |     |  |   |         |     | |
|  |   |ath12k_hw|     |  |   |ath12k_hw|     | |
|  |   |   (ah)  |     |  |   |   (ah)  |     | |
|  |   |         |     |  |   |         |     | |
|  |   +---------+     |  |   +---------+     | |
|  |        ^          |  |        ^          | |
|  | +------|--------+ |  | +------|--------+ | |
|  | |      |        | |  | |      |        | | |
|  | | +----------+  | |  | | +----------+  | | |
|  | | |    ar    |  | |  | | |    ar    |  | | |
|  | | |   5 GHz  |  | |  | | |   6 GHz  |  | | |
|  | | |          |  | |  | | |          |  | | |
|  | | +----------+  | |  | | +----------+  | | |
|  | |  ath12k_base  | |  | |  ath12k_base  | | |
|  | |       (ab)    | |  | |       (ab)    | | |
|  | +---------------+ |  | +---------------+ | |
|  |   ath12k_hw_group |  |  ath12k_hw_group  | |
|  +-------------------+  +-------------------+ |
+-----------------------------------------------+

As multiple device grouping is not yet brought in with this patch, this 
is how it might look like.


>> 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
>> 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 <quic_periyasa@quicinc.com>
>> Co-developed-by: Harshitha Prem <quic_hprem@quicinc.com>
>> Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
>> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> 
> Like I have said before, for adding any new locks there needs to be a
> proper analysis for the locking and good justifications why new locks
> are needed. I don't see any of that above.
>
Sure, Kalle. Will add the details for new locks. As grouping has to be 
done in synchronous manner lock was introduced.

> BTW I will from now on require proper analysis also for additions to
> enum ath12k_dev_flags.
Sure, Will update the details.

There are few racy conditions with mutiple device grouping which needed 
these flags.
One such instance is, if a reboot is triggered between core_init and 
core_start, some of the devices in ath12k_hw_group would have already 
started and completed till FW ready but some would not even reached QMI 
firmware ready event. But, during reboot as part of pci shutdown group 
clean up would try to do core stop for all devices in group. This will 
lead to crashes as for some devices core start is yet to do but we tried 
to invoked core stop.

> 
>> --- 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);
> 
> I can somehow understand/guess why this mutex is needed (even though
> there's no documentation) but the naming is not really clear as we
> already have struct ath12k_hw::hw_mutex.
> 
This lock is for device group list handling. Will modify the name 
accordingly and update the documentation. Thanks.

>> +/* 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;
>> +};
> 
> But why we really need this mutex? And does it really justify the extra
> complexity it creates?
>
This lock is necessary because for some QMI handling and WMI MLO 
handling should happen only after all the devices in group are probed 
and devices are started. In those WMI & QMI MLO handshake, it requires 
the partner details.
QMI firmware ready can come from any device in a group and may not be 
synchronous but the group has to be registered only after all the 
devices have received QMI firmware ready. Similarly, core stop, firmware 
recovery scenarios. To handle this the ag lock is needed. Will update 
the details.


Thanks,
Harshitha
Kalle Valo July 3, 2024, 4:35 p.m. UTC | #4
Harshitha Prem <quic_hprem@quicinc.com> writes:

>> Like I have said before, for adding any new locks there needs to be
>> a
>> proper analysis for the locking and good justifications why new locks
>> are needed. I don't see any of that above.
>>
> Sure, Kalle. Will add the details for new locks. As grouping has to be
> done in synchronous manner lock was introduced.

The problem with Qualcomm code is that new locks seem to be always added
without any analysis. But if these new locks are being continuously
added the code will become so hard to maintain and convoluted.

I can understand adding ath12k_ag_list_lock (it's just badly named)
because there is not really any other way to do it. But this mutex_lock
looks so suspicious, has anyone analysed that?
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 7ad65ace7123..ebe31cbb6435 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;
@@ -1185,20 +1188,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);
 
@@ -1209,8 +1303,114 @@  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;
 
-	ath12k_core_soc_destroy(ab);
+	if (WARN_ON(!ag))
+		return;
+
+	for (i = 0; i < ag->num_devices; i++) {
+		ab = ag->ab[i];
+		if (!ab)
+			continue;
+
+		if (test_and_clear_bit(ATH12K_FLAG_HW_GROUP_ATTACHED, &ab->dev_flags))
+			ath12k_core_soc_destroy(ab);
+	}
+}
+
+static void ath12k_core_hw_group_cleanup(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);
+	}
+	mutex_unlock(&ag->mutex_lock);
+}
+
+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_cleanup(ab->ag);
+	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 bc2a9e1b1885..a6b8c100ebc8 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,
@@ -209,6 +213,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,
 };
 
 struct ath12k_tx_conf {
@@ -725,6 +730,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
  *
@@ -925,6 +941,8 @@  struct ath12k_base {
 
 #endif /* CONFIG_ACPI */
 
+	struct ath12k_hw_group *ag;
+
 	/* must be last */
 	u8 drv_priv[] __aligned(sizeof(void *));
 };
@@ -955,6 +973,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 e0a24ad0480e..446e57b47de6 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -1522,6 +1522,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;
 	}