Op 04-12-2024 om 17:32 schreef Kalle Valo:
From: Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>
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 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 struct
ath12k_hw_group (ag).
[...]
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
[...]
-void ath12k_mac_destroy(struct ath12k_base *ab)
+void ath12k_mac_destroy(struct ath12k_hw_group *ag)
{
struct ath12k_pdev *pdev;
+ struct ath12k_base *ab = ag->ab[0];
+ int i, j;
struct ath12k_hw *ah;
- int i;
- 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 (j = 0; j < ab->num_radios; j++) {
+ pdev = &ab->pdevs[j];
+ if (!pdev->ar)
+ continue;
+ pdev->ar = NULL;
+ }
}
for (i = 0; i < ath12k_get_num_hw(ab); i++) {
@@ -10942,26 +10945,59 @@ void ath12k_mac_destroy(struct ath12k_base *ab)
}
}
The new ath12k_mac_destroy() looks suspicious with respect to "ab".
Here is the new function with comments.
void ath12k_mac_destroy(struct ath12k_hw_group *ag)
{
struct ath12k_pdev *pdev;
struct ath12k_base *ab = ag->ab[0];
==> here ag->ab[0] is used before checking if ag->num_devices is greater
than zero
==> If ag->num_devices will never be zero then this first assignment
will be repeated in the next for-loop
==> if ag->num_devices can be zero then you need to confirm that
ag->ab[0] is non-NULL
int i, j;
struct ath12k_hw *ah;
for (i = 0; i < ag->num_devices; i++) {
ab = ag->ab[i];
if (!ab)
=>> This if check indicates that ab can be NULL
continue;
for (j = 0; j < ab->num_radios; j++) {
pdev = &ab->pdevs[j];
if (!pdev->ar)
continue;
pdev->ar = NULL;
}
}
==> Now ab is going to be used, but as shown above, ab can be NULL
for (i = 0; i < ath12k_get_num_hw(ab); i++) {
ah = ath12k_ab_to_ah(ab, i);
if (!ah)
continue;
ath12k_mac_hw_destroy(ah);
ath12k_ab_set_ah(ab, i, NULL);
}
}
--
Kees
