diff mbox series

[2/3] ath11k: Move pdev debugfs creation ahead

Message ID 20210913180902.193874-3-jouni@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series ath11k: Add support for sram dump | expand

Commit Message

Jouni Malinen Sept. 13, 2021, 6:09 p.m. UTC 
From: Baochen Qiang <bqiang@codeaurora.org>

The sram dump debugfs interface has to be put under pdev directory
located under /sys/kernel/debug/ath11k/<pdev name>. Currently pdev directory
is created after firmware is ready, this is too late for sram dump.
Suppose that if errors happen and ath11k fails to reach firmware
ready, we have no way to dump sram content to debug cause the
interface has not been created yet. So move it ahead.

Also move pdev debugfs destroy to ath11k_core_soc_destroy as a mirror.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/core.c    | 23 +++++++++++++----------
 drivers/net/wireless/ath/ath11k/debugfs.c | 21 +++++++++++++++------
 drivers/net/wireless/ath/ath11k/debugfs.h |  9 +++++++++
 3 files changed, 37 insertions(+), 16 deletions(-)

Comments

Kalle Valo Dec. 7, 2021, 5:47 p.m. UTC | #1 
Jouni Malinen <jouni@codeaurora.org> writes:

> From: Baochen Qiang <bqiang@codeaurora.org>
>
> The sram dump debugfs interface has to be put under pdev directory
> located under /sys/kernel/debug/ath11k/<pdev name>. Currently pdev directory
> is created after firmware is ready, this is too late for sram dump.
> Suppose that if errors happen and ath11k fails to reach firmware
> ready, we have no way to dump sram content to debug cause the
> interface has not been created yet. So move it ahead.

I'm not sure about this. What will happen with other debugfs files now
that they are created before the firmware is ready, doesn't that create
race conditions?

Also we need to do some refactoring in debugfs, for example see Anil's
patch:

https://patchwork.kernel.org/project/linux-wireless/patch/1614160542-27882-1-git-send-email-akolli@codeaurora.org/

So I recommend dropping patch 2 for now and get the basic sram dump
functionality ready first. After that we can discuss how to handle
firmware crashes during driver initialisation, maybe coredump would be a
better approach?
Baochen Qiang Dec. 8, 2021, 1:34 a.m. UTC | #2 
On 2021-12-08 01:47, Kalle Valo wrote:
> Jouni Malinen <jouni@codeaurora.org> writes:
> 
>> From: Baochen Qiang <bqiang@codeaurora.org>
>> 
>> The sram dump debugfs interface has to be put under pdev directory
>> located under /sys/kernel/debug/ath11k/<pdev name>. Currently pdev 
>> directory
>> is created after firmware is ready, this is too late for sram dump.
>> Suppose that if errors happen and ath11k fails to reach firmware
>> ready, we have no way to dump sram content to debug cause the
>> interface has not been created yet. So move it ahead.
> 
> I'm not sure about this. What will happen with other debugfs files now
> that they are created before the firmware is ready, doesn't that create
> race conditions?
> 
> Also we need to do some refactoring in debugfs, for example see Anil's
> patch:
> 
> https://patchwork.kernel.org/project/linux-wireless/patch/1614160542-27882-1-git-send-email-akolli@codeaurora.org/
> 
> So I recommend dropping patch 2 for now and get the basic sram dump
> functionality ready first. After that we can discuss how to handle
> firmware crashes during driver initialisation, maybe coredump would be 
> a
> better approach?

Sure, Kalle, let's get basic functionality ready first.
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 9f2c9795767e..59fa0ff06dff 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -701,14 +701,22 @@  static int ath11k_core_soc_create(struct ath11k_base *ab)
 		goto err_qmi_deinit;
 	}
 
+	ret = ath11k_debugfs_pdev_create(ab);
+	if (ret) {
+		ath11k_err(ab, "failed to create core pdev debugfs: %d\n", ret);
+		goto err_debugfs_reg;
+	}
+
 	ret = ath11k_hif_power_up(ab);
 	if (ret) {
 		ath11k_err(ab, "failed to power up :%d\n", ret);
-		goto err_debugfs_reg;
+		goto err_pdev_debug;
 	}
 
 	return 0;
 
+err_pdev_debug:
+	ath11k_debugfs_pdev_destroy(ab);
 err_debugfs_reg:
 	ath11k_debugfs_soc_destroy(ab);
 err_qmi_deinit:
@@ -718,6 +726,7 @@  static int ath11k_core_soc_create(struct ath11k_base *ab)
 
 static void ath11k_core_soc_destroy(struct ath11k_base *ab)
 {
+	ath11k_debugfs_pdev_destroy(ab);
 	ath11k_debugfs_soc_destroy(ab);
 	ath11k_dp_free(ab);
 	ath11k_reg_free(ab);
@@ -728,16 +737,13 @@  static int ath11k_core_pdev_create(struct ath11k_base *ab)
 {
 	int ret;
 
-	ret = ath11k_debugfs_pdev_create(ab);
-	if (ret) {
-		ath11k_err(ab, "failed to create core pdev debugfs: %d\n", ret);
-		return ret;
-	}
+	ath11k_debugfs_crash_trigger_create(ab);
+	ath11k_debugfs_dp_stats_create(ab);
 
 	ret = ath11k_mac_register(ab);
 	if (ret) {
 		ath11k_err(ab, "failed register the radio with mac80211: %d\n", ret);
-		goto err_pdev_debug;
+		return ret;
 	}
 
 	ret = ath11k_dp_pdev_alloc(ab);
@@ -767,8 +773,6 @@  static int ath11k_core_pdev_create(struct ath11k_base *ab)
 	ath11k_dp_pdev_free(ab);
 err_mac_unregister:
 	ath11k_mac_unregister(ab);
-err_pdev_debug:
-	ath11k_debugfs_pdev_destroy(ab);
 
 	return ret;
 }
@@ -780,7 +784,6 @@  static void ath11k_core_pdev_destroy(struct ath11k_base *ab)
 	ath11k_mac_unregister(ab);
 	ath11k_hif_irq_disable(ab);
 	ath11k_dp_pdev_free(ab);
-	ath11k_debugfs_pdev_destroy(ab);
 }
 
 static int ath11k_core_start(struct ath11k_base *ab,
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
index 17f0bbbac7ae..cdc492421807 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs.c
@@ -831,20 +831,29 @@  static const struct file_operations fops_soc_dp_stats = {
 	.llseek = default_llseek,
 };
 
-int ath11k_debugfs_pdev_create(struct ath11k_base *ab)
+void ath11k_debugfs_crash_trigger_create(struct ath11k_base *ab)
 {
 	if (test_bit(ATH11K_FLAG_REGISTERED, &ab->dev_flags))
-		return 0;
-
-	ab->debugfs_soc = debugfs_create_dir(ab->hw_params.name, ab->debugfs_ath11k);
-	if (IS_ERR(ab->debugfs_soc))
-		return PTR_ERR(ab->debugfs_soc);
+		return;
 
 	debugfs_create_file("simulate_fw_crash", 0600, ab->debugfs_soc, ab,
 			    &fops_simulate_fw_crash);
+}
+
+void ath11k_debugfs_dp_stats_create(struct ath11k_base *ab)
+{
+	if (test_bit(ATH11K_FLAG_REGISTERED, &ab->dev_flags))
+		return;
 
 	debugfs_create_file("soc_dp_stats", 0600, ab->debugfs_soc, ab,
 			    &fops_soc_dp_stats);
+}
+
+int ath11k_debugfs_pdev_create(struct ath11k_base *ab)
+{
+	ab->debugfs_soc = debugfs_create_dir(ab->hw_params.name, ab->debugfs_ath11k);
+	if (IS_ERR(ab->debugfs_soc))
+		return PTR_ERR(ab->debugfs_soc);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.h b/drivers/net/wireless/ath/ath11k/debugfs.h
index e5346af71f24..47b96848cf0a 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.h
+++ b/drivers/net/wireless/ath/ath11k/debugfs.h
@@ -145,6 +145,8 @@  static inline int ath11k_debugfs_rx_filter(struct ath11k *ar)
 	return ar->debug.rx_filter;
 }
 
+void ath11k_debugfs_crash_trigger_create(struct ath11k_base *ab);
+void ath11k_debugfs_dp_stats_create(struct ath11k_base *ab);
 #else
 static inline int ath11k_debugfs_soc_create(struct ath11k_base *ab)
 {
@@ -212,6 +214,13 @@  static inline int ath11k_debugfs_rx_filter(struct ath11k *ar)
 	return 0;
 }
 
+static inline void ath11k_debugfs_crash_trigger_create(struct ath11k_base *ab)
+{
+}
+
+static inline void ath11k_debugfs_dp_stats_create(struct ath11k_base *ab)
+{
+}
 #endif /* CONFIG_MAC80211_DEBUGFS*/
 
 #endif /* _ATH11K_DEBUGFS_H_ */