linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning
@ 2025-01-12  7:16 Karthikeyan Periyasamy
  2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Karthikeyan Periyasamy @ 2025-01-12  7:16 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Karthikeyan Periyasamy

This patch series fix the below Smatch static checker warnings

Warnings:
mac.c:10941 ath12k_mac_destroy() error: we previously assumed 'ab' could be null (see line 11348)
mac.c:11007 ath12k_mac_allocate() error: uninitialized symbol 'ab'.
mac.c:11013 ath12k_mac_allocate() error: uninitialized symbol 'ab'.

v4:
 wifi: ath12k: Remove ath12k_get_num_hw() helper function
  - updated copyright
v3:
 wifi: ath12k: Remove ath12k_get_num_hw() helper function
  - Renamed the patch title
  - Removed ath12k_get_num_hw() helper function
v2:
 - Rebased on ToT

Karthikeyan Periyasamy (4):
  wifi: ath12k: Refactor ath12k_hw set helper function argument
  wifi: ath12k: Refactor the ath12k_hw get helper function argument
  wifi: ath12k: Remove ath12k_get_num_hw() helper function
  wifi: ath12k: Fix uninitialized variable access in
    ath12k_mac_allocate() function

 drivers/net/wireless/ath/ath12k/core.c | 22 +++++++-----
 drivers/net/wireless/ath/ath12k/core.h | 15 +++-----
 drivers/net/wireless/ath/ath12k/mac.c  | 49 +++++++++++++++++---------
 3 files changed, 50 insertions(+), 36 deletions(-)


base-commit: 0c5fcd9069dd5f984e39820629acbfbe0f1b4256
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument
  2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
@ 2025-01-12  7:16 ` Karthikeyan Periyasamy
  2025-01-14 18:28   ` Kalle Valo
  2025-01-14 18:29   ` Kalle Valo
  2025-01-12  7:16 ` [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get " Karthikeyan Periyasamy
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Karthikeyan Periyasamy @ 2025-01-12  7:16 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Karthikeyan Periyasamy, Jeff Johnson

Currently, ath12k_hw is placed inside the ath12k_hw_group. However, the
ath12k_hw set helper function takes the device handle and the index as
parameters. Here, the index parameter is specific to the group handle.
Therefore, change this helper function argument from the device handle to
the group handle.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/core.h | 4 ++--
 drivers/net/wireless/ath/ath12k/mac.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 3dd01ad100c5..9eb4eb410af2 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1199,10 +1199,10 @@ static inline struct ath12k_hw *ath12k_ab_to_ah(struct ath12k_base *ab, int idx)
 	return ab->ag->ah[idx];
 }
 
-static inline void ath12k_ab_set_ah(struct ath12k_base *ab, int idx,
+static inline void ath12k_ag_set_ah(struct ath12k_hw_group *ag, int idx,
 				    struct ath12k_hw *ah)
 {
-	ab->ag->ah[idx] = ah;
+	ag->ah[idx] = ah;
 }
 
 static inline int ath12k_get_num_hw(struct ath12k_base *ab)
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 48d110e2a7de..6a7a01c35f02 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -11362,7 +11362,7 @@ void ath12k_mac_destroy(struct ath12k_hw_group *ag)
 			continue;
 
 		ath12k_mac_hw_destroy(ah);
-		ath12k_ab_set_ah(ab, i, NULL);
+		ath12k_ag_set_ah(ag, i, NULL);
 	}
 }
 
@@ -11443,7 +11443,7 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
 			continue;
 
 		ath12k_mac_hw_destroy(ah);
-		ath12k_ab_set_ah(ab, i, NULL);
+		ath12k_ag_set_ah(ag, i, NULL);
 	}
 
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get helper function argument
  2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
  2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
@ 2025-01-12  7:16 ` Karthikeyan Periyasamy
  2025-01-14 18:30   ` Kalle Valo
  2025-01-12  7:16 ` [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function Karthikeyan Periyasamy
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Karthikeyan Periyasamy @ 2025-01-12  7:16 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Karthikeyan Periyasamy, Jeff Johnson

Currently, ath12k_hw is placed inside the ath12k_hw_group. However, the
ath12k_hw get helper function takes the device handle and the index as
parameters. Here, the index parameter is specific to the group handle.
Therefore, change this helper function argument from the device handle
to the group handle.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/core.c |  8 ++++----
 drivers/net/wireless/ath/ath12k/core.h |  4 ++--
 drivers/net/wireless/ath/ath12k/mac.c  | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 0c6b35aac96e..af49e7db4928 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1184,7 +1184,7 @@ static void ath12k_rfkill_work(struct work_struct *work)
 	spin_unlock_bh(&ab->base_lock);
 
 	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ab->ag, i);
 		if (!ah)
 			continue;
 
@@ -1236,7 +1236,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 < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ab->ag, i);
 		if (!ah || ah->state == ATH12K_HW_STATE_OFF)
 			continue;
 
@@ -1275,7 +1275,7 @@ static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
 	int i, j;
 
 	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ab->ag, i);
 		if (!ah || ah->state == ATH12K_HW_STATE_OFF)
 			continue;
 
@@ -1337,7 +1337,7 @@ static void ath12k_core_restart(struct work_struct *work)
 		}
 
 		for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-			ah = ath12k_ab_to_ah(ab, i);
+			ah = ath12k_ag_to_ah(ab->ag, i);
 			ieee80211_restart_hw(ah->hw);
 		}
 	}
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 9eb4eb410af2..4932bd1c2096 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1194,9 +1194,9 @@ static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
 	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)
+static inline struct ath12k_hw *ath12k_ag_to_ah(struct ath12k_hw_group *ag, int idx)
 {
-	return ab->ag->ah[idx];
+	return ag->ah[idx];
 }
 
 static inline void ath12k_ag_set_ah(struct ath12k_hw_group *ag, int idx,
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 6a7a01c35f02..285bb88acb90 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -11244,7 +11244,7 @@ int ath12k_mac_register(struct ath12k_hw_group *ag)
 	int ret;
 
 	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ag, i);
 
 		ret = ath12k_mac_hw_register(ah);
 		if (ret)
@@ -11257,7 +11257,7 @@ int ath12k_mac_register(struct ath12k_hw_group *ag)
 
 err:
 	for (i = i - 1; i >= 0; i--) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
 
@@ -11276,7 +11276,7 @@ void ath12k_mac_unregister(struct ath12k_hw_group *ag)
 	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);
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
 
@@ -11357,7 +11357,7 @@ void ath12k_mac_destroy(struct ath12k_hw_group *ag)
 	}
 
 	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
 
@@ -11438,7 +11438,7 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
 
 err:
 	for (i = i - 1; i >= 0; i--) {
-		ah = ath12k_ab_to_ah(ab, i);
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function
  2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
  2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
  2025-01-12  7:16 ` [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get " Karthikeyan Periyasamy
@ 2025-01-12  7:16 ` Karthikeyan Periyasamy
  2025-01-13 16:12   ` Jeff Johnson
  2025-01-14 18:31   ` Kalle Valo
  2025-01-12  7:16 ` [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function Karthikeyan Periyasamy
  2025-01-14 20:54 ` [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Jeff Johnson
  4 siblings, 2 replies; 12+ messages in thread
From: Karthikeyan Periyasamy @ 2025-01-12  7:16 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Karthikeyan Periyasamy, Dan Carpenter

Currently, the ath12k_get_num_hw() helper function takes the device handle
as an argument. Here, the number of hardware is retrieved from the group
handle. Demanding the device handle from the caller is unnecessary since
in some cases the group handle is already available. Additionally, there
is no longer a need for multiple indirections to get the number of
hardware. Therefore, remove this helper function and directly use
ag->num_hw. This change also fixes the below Smatch static checker
warning.

Smatch warning:
ath12k_mac_destroy() error: we previously assumed 'ab' could be null

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/ath12k/3e705de0-67d1-4437-97ff-4828d83ae2af@stanley.mountain/
Closes: https://scan7.scan.coverity.com/#/project-view/52682/11354?selectedIssue=1602340
Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 20 ++++++++++++--------
 drivers/net/wireless/ath/ath12k/core.h |  7 +------
 drivers/net/wireless/ath/ath12k/mac.c  |  8 ++++----
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index af49e7db4928..0606116d6b9c 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause-Clear
 /*
  * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <linux/module.h>
@@ -1173,6 +1173,7 @@ static int ath12k_core_reconfigure_on_crash(struct ath12k_base *ab)
 static void ath12k_rfkill_work(struct work_struct *work)
 {
 	struct ath12k_base *ab = container_of(work, struct ath12k_base, rfkill_work);
+	struct ath12k_hw_group *ag = ab->ag;
 	struct ath12k *ar;
 	struct ath12k_hw *ah;
 	struct ieee80211_hw *hw;
@@ -1183,8 +1184,8 @@ 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 < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ag_to_ah(ab->ag, i);
+	for (i = 0; i < ag->num_hw; i++) {
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
 
@@ -1224,6 +1225,7 @@ void ath12k_core_halt(struct ath12k *ar)
 
 static void ath12k_core_pre_reconfigure_recovery(struct ath12k_base *ab)
 {
+	struct ath12k_hw_group *ag = ab->ag;
 	struct ath12k *ar;
 	struct ath12k_hw *ah;
 	int i, j;
@@ -1235,8 +1237,8 @@ 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 < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ag_to_ah(ab->ag, i);
+	for (i = 0; i < ag->num_hw; i++) {
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah || ah->state == ATH12K_HW_STATE_OFF)
 			continue;
 
@@ -1270,12 +1272,13 @@ static void ath12k_core_pre_reconfigure_recovery(struct ath12k_base *ab)
 
 static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
 {
+	struct ath12k_hw_group *ag = ab->ag;
 	struct ath12k_hw *ah;
 	struct ath12k *ar;
 	int i, j;
 
-	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
-		ah = ath12k_ag_to_ah(ab->ag, i);
+	for (i = 0; i < ag->num_hw; i++) {
+		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah || ah->state == ATH12K_HW_STATE_OFF)
 			continue;
 
@@ -1318,6 +1321,7 @@ static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
 static void ath12k_core_restart(struct work_struct *work)
 {
 	struct ath12k_base *ab = container_of(work, struct ath12k_base, restart_work);
+	struct ath12k_hw_group *ag = ab->ag;
 	struct ath12k_hw *ah;
 	int ret, i;
 
@@ -1336,7 +1340,7 @@ static void ath12k_core_restart(struct work_struct *work)
 			ath12k_dbg(ab, ATH12K_DBG_BOOT, "reset success\n");
 		}
 
-		for (i = 0; i < ath12k_get_num_hw(ab); i++) {
+		for (i = 0; i < ag->num_hw; i++) {
 			ah = ath12k_ag_to_ah(ab->ag, i);
 			ieee80211_restart_hw(ah->hw);
 		}
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 4932bd1c2096..ee595794a7ae 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause-Clear */
 /*
  * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #ifndef ATH12K_CORE_H
@@ -1205,11 +1205,6 @@ static inline void ath12k_ag_set_ah(struct ath12k_hw_group *ag, int idx,
 	ag->ah[idx] = ah;
 }
 
-static inline int ath12k_get_num_hw(struct ath12k_base *ab)
-{
-	return ab->ag->num_hw;
-}
-
 static inline struct ath12k_hw_group *ath12k_ab_to_ag(struct ath12k_base *ab)
 {
 	return ab->ag;
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 285bb88acb90..08f340afb970 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause-Clear
 /*
  * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <net/mac80211.h>
@@ -11243,7 +11243,7 @@ int ath12k_mac_register(struct ath12k_hw_group *ag)
 	int i;
 	int ret;
 
-	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
+	for (i = 0; i < ag->num_hw; i++) {
 		ah = ath12k_ag_to_ah(ag, i);
 
 		ret = ath12k_mac_hw_register(ah);
@@ -11275,7 +11275,7 @@ void ath12k_mac_unregister(struct ath12k_hw_group *ag)
 
 	clear_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags);
 
-	for (i = ath12k_get_num_hw(ab) - 1; i >= 0; i--) {
+	for (i = ag->num_hw - 1; i >= 0; i--) {
 		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
@@ -11356,7 +11356,7 @@ void ath12k_mac_destroy(struct ath12k_hw_group *ag)
 		}
 	}
 
-	for (i = 0; i < ath12k_get_num_hw(ab); i++) {
+	for (i = 0; i < ag->num_hw; i++) {
 		ah = ath12k_ag_to_ah(ag, i);
 		if (!ah)
 			continue;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function
  2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
                   ` (2 preceding siblings ...)
  2025-01-12  7:16 ` [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function Karthikeyan Periyasamy
@ 2025-01-12  7:16 ` Karthikeyan Periyasamy
  2025-01-14 18:32   ` Kalle Valo
  2025-01-14 20:54 ` [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Jeff Johnson
  4 siblings, 1 reply; 12+ messages in thread
From: Karthikeyan Periyasamy @ 2025-01-12  7:16 UTC (permalink / raw)
  To: ath12k; +Cc: linux-wireless, Karthikeyan Periyasamy, Jeff Johnson

Currently, the uninitialized variable 'ab' is accessed in the
ath12k_mac_allocate() function. Initialize 'ab' with the first radio device
present in the hardware abstraction handle (ah). Additionally, move the
default setting procedure from the pdev mapping iteration to the total
radio calculating iteration for better code readability. Perform the
maximum radio validation check for total_radio to ensure that both num_hw
and radio_per_hw are validated indirectly, as these variables are derived
from total_radio. This also fixes the below Smatch static checker warning.

Smatch warning:
ath12k_mac_allocate() error: uninitialized symbol 'ab'

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1

Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 08f340afb970..af614b301c61 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -11383,8 +11383,20 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
 	u8 radio_per_hw;
 
 	total_radio = 0;
-	for (i = 0; i < ag->num_devices; i++)
-		total_radio += ag->ab[i]->num_radios;
+	for (i = 0; i < ag->num_devices; i++) {
+		ab = ag->ab[i];
+		if (!ab)
+			continue;
+
+		ath12k_mac_set_device_defaults(ab);
+		total_radio += ab->num_radios;
+	}
+
+	if (!total_radio)
+		return -EINVAL;
+
+	if (WARN_ON(total_radio > ATH12K_GROUP_MAX_RADIO))
+		return -ENOSPC;
 
 	/* All pdev get combined and register as single wiphy based on
 	 * hardware group which participate in multi-link operation else
@@ -11397,14 +11409,16 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
 
 	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++) {
 		for (j = 0; j < radio_per_hw; j++) {
+			if (device_id >= ag->num_devices || !ag->ab[device_id]) {
+				ret = -ENOSPC;
+				goto err;
+			}
+
 			ab = ag->ab[device_id];
 			pdev_map[j].ab = ab;
 			pdev_map[j].pdev_idx = mac_id;
@@ -11416,10 +11430,11 @@ int ath12k_mac_allocate(struct ath12k_hw_group *ag)
 			if (mac_id >= ab->num_radios) {
 				mac_id = 0;
 				device_id++;
-				ath12k_mac_set_device_defaults(ab);
 			}
 		}
 
+		ab = pdev_map->ab;
+
 		ah = ath12k_mac_hw_allocate(ag, pdev_map, radio_per_hw);
 		if (!ah) {
 			ath12k_warn(ab, "failed to allocate mac80211 hw device for hw_idx %d\n",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function
  2025-01-12  7:16 ` [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function Karthikeyan Periyasamy
@ 2025-01-13 16:12   ` Jeff Johnson
  2025-01-14 18:31   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Johnson @ 2025-01-13 16:12 UTC (permalink / raw)
  To: Karthikeyan Periyasamy, ath12k; +Cc: linux-wireless, Dan Carpenter

On 1/11/2025 11:16 PM, Karthikeyan Periyasamy wrote:
> Currently, the ath12k_get_num_hw() helper function takes the device handle
> as an argument. Here, the number of hardware is retrieved from the group
> handle. Demanding the device handle from the caller is unnecessary since
> in some cases the group handle is already available. Additionally, there
> is no longer a need for multiple indirections to get the number of
> hardware. Therefore, remove this helper function and directly use
> ag->num_hw. This change also fixes the below Smatch static checker
> warning.
> 
> Smatch warning:
> ath12k_mac_destroy() error: we previously assumed 'ab' could be null
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/ath12k/3e705de0-67d1-4437-97ff-4828d83ae2af@stanley.mountain/
> Closes: https://scan7.scan.coverity.com/#/project-view/52682/11354?selectedIssue=1602340
> Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>

Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument
  2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
@ 2025-01-14 18:28   ` Kalle Valo
  2025-01-14 18:29   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2025-01-14 18:28 UTC (permalink / raw)
  To: Karthikeyan Periyasamy; +Cc: ath12k, linux-wireless, Jeff Johnson

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

> Currently, ath12k_hw is placed inside the ath12k_hw_group. However, the
> ath12k_hw set helper function takes the device handle and the index as
> parameters. Here, the index parameter is specific to the group handle.
> Therefore, change this helper function argument from the device handle to
> the group handle.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument
  2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
  2025-01-14 18:28   ` Kalle Valo
@ 2025-01-14 18:29   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2025-01-14 18:29 UTC (permalink / raw)
  To: Karthikeyan Periyasamy; +Cc: ath12k, linux-wireless, Jeff Johnson

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

> Currently, ath12k_hw is placed inside the ath12k_hw_group. However, the
> ath12k_hw set helper function takes the device handle and the index as
> parameters. Here, the index parameter is specific to the group handle.
> Therefore, change this helper function argument from the device handle to
> the group handle.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get helper function argument
  2025-01-12  7:16 ` [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get " Karthikeyan Periyasamy
@ 2025-01-14 18:30   ` Kalle Valo
  0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2025-01-14 18:30 UTC (permalink / raw)
  To: Karthikeyan Periyasamy; +Cc: ath12k, linux-wireless, Jeff Johnson

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

> Currently, ath12k_hw is placed inside the ath12k_hw_group. However, the
> ath12k_hw get helper function takes the device handle and the index as
> parameters. Here, the index parameter is specific to the group handle.
> Therefore, change this helper function argument from the device handle
> to the group handle.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function
  2025-01-12  7:16 ` [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function Karthikeyan Periyasamy
  2025-01-13 16:12   ` Jeff Johnson
@ 2025-01-14 18:31   ` Kalle Valo
  1 sibling, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2025-01-14 18:31 UTC (permalink / raw)
  To: Karthikeyan Periyasamy; +Cc: ath12k, linux-wireless, Dan Carpenter

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

> Currently, the ath12k_get_num_hw() helper function takes the device handle
> as an argument. Here, the number of hardware is retrieved from the group
> handle. Demanding the device handle from the caller is unnecessary since
> in some cases the group handle is already available. Additionally, there
> is no longer a need for multiple indirections to get the number of
> hardware. Therefore, remove this helper function and directly use
> ag->num_hw. This change also fixes the below Smatch static checker
> warning.
>
> Smatch warning:
> ath12k_mac_destroy() error: we previously assumed 'ab' could be null
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/ath12k/3e705de0-67d1-4437-97ff-4828d83ae2af@stanley.mountain/
> Closes: https://scan7.scan.coverity.com/#/project-view/52682/11354?selectedIssue=1602340
> Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function
  2025-01-12  7:16 ` [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function Karthikeyan Periyasamy
@ 2025-01-14 18:32   ` Kalle Valo
  0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2025-01-14 18:32 UTC (permalink / raw)
  To: Karthikeyan Periyasamy; +Cc: ath12k, linux-wireless, Jeff Johnson

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

> Currently, the uninitialized variable 'ab' is accessed in the
> ath12k_mac_allocate() function. Initialize 'ab' with the first radio device
> present in the hardware abstraction handle (ah). Additionally, move the
> default setting procedure from the pdev mapping iteration to the total
> radio calculating iteration for better code readability. Perform the
> maximum radio validation check for total_radio to ensure that both num_hw
> and radio_per_hw are validated indirectly, as these variables are derived
> from total_radio. This also fixes the below Smatch static checker warning.
>
> Smatch warning:
> ath12k_mac_allocate() error: uninitialized symbol 'ab'
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>
> Fixes: a343d97f27f5 ("wifi: ath12k: move struct ath12k_hw from per device to group")
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Acked-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning
  2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
                   ` (3 preceding siblings ...)
  2025-01-12  7:16 ` [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function Karthikeyan Periyasamy
@ 2025-01-14 20:54 ` Jeff Johnson
  4 siblings, 0 replies; 12+ messages in thread
From: Jeff Johnson @ 2025-01-14 20:54 UTC (permalink / raw)
  To: ath12k, Karthikeyan Periyasamy; +Cc: linux-wireless


On Sun, 12 Jan 2025 12:46:26 +0530, Karthikeyan Periyasamy wrote:
> This patch series fix the below Smatch static checker warnings
> 
> Warnings:
> mac.c:10941 ath12k_mac_destroy() error: we previously assumed 'ab' could be null (see line 11348)
> mac.c:11007 ath12k_mac_allocate() error: uninitialized symbol 'ab'.
> mac.c:11013 ath12k_mac_allocate() error: uninitialized symbol 'ab'.
> 
> [...]

Applied, thanks!

[1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument
      commit: 21261e4d564e866bdae810b4fb5278a3a6a1a6ed
[2/4] wifi: ath12k: Refactor the ath12k_hw get helper function argument
      commit: 812a30271d48cff5edc39aeb436b3bebd558f5cf
[3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function
      commit: 54fcdcf07baa9cb34359094f1b829b29d0845cb6
[4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function
      commit: 4aae869847ce50da902005ce4d84505d54e739c9

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-01-14 20:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-12  7:16 [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Karthikeyan Periyasamy
2025-01-12  7:16 ` [PATCH v4 1/4] wifi: ath12k: Refactor ath12k_hw set helper function argument Karthikeyan Periyasamy
2025-01-14 18:28   ` Kalle Valo
2025-01-14 18:29   ` Kalle Valo
2025-01-12  7:16 ` [PATCH v4 2/4] wifi: ath12k: Refactor the ath12k_hw get " Karthikeyan Periyasamy
2025-01-14 18:30   ` Kalle Valo
2025-01-12  7:16 ` [PATCH v4 3/4] wifi: ath12k: Remove ath12k_get_num_hw() helper function Karthikeyan Periyasamy
2025-01-13 16:12   ` Jeff Johnson
2025-01-14 18:31   ` Kalle Valo
2025-01-12  7:16 ` [PATCH v4 4/4] wifi: ath12k: Fix uninitialized variable access in ath12k_mac_allocate() function Karthikeyan Periyasamy
2025-01-14 18:32   ` Kalle Valo
2025-01-14 20:54 ` [PATCH v4 0/4] wifi: ath12k: Fix the static checker warning Jeff Johnson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).