diff mbox series

ath11k: Allow debugfs to work with 2+ radios installed.

Message ID 20200903172359.29199-1-greearb@candelatech.com (mailing list archive)
State RFC
Delegated to: Kalle Valo
Headers show
Series ath11k: Allow debugfs to work with 2+ radios installed. | expand

Commit Message

Ben Greear Sept. 3, 2020, 5:23 p.m. UTC
From: Ben Greear <greearb@candelatech.com>

The code is trying to create an ath11k directory on debugfs
root, but that fails when there is a second radio (and thus
second instance of the driver).

Work around this by finding a free name.  This may fall into
the HACK category, I'm not really sure what the original design
is supposed to accomplish.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

NOTE:  A second radio still does not get far enough to fully boot
the NIC and create a phy* device, but at least system doesn't crash
hard now.

 drivers/net/wireless/ath/ath11k/core.h  |  5 +++--
 drivers/net/wireless/ath/ath11k/debug.c | 23 ++++++++++++++++-------
 2 files changed, 19 insertions(+), 9 deletions(-)

Comments

Anilkumar Kolli Sept. 4, 2020, 3 p.m. UTC | #1
On 2020-09-03 22:53, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> The code is trying to create an ath11k directory on debugfs
> root, but that fails when there is a second radio (and thus
> second instance of the driver).
> 

Hi Ben,

IPQ8074 is soc and it does not need second debugfs entry, its on AHB.
QCA6390 is a pci card and multiple debug entries are needed if multiple 
cards are available,
I am planning to post a patch with bus specific abstraction function 
which creates single debugfs for soc based radios and multiple entries 
for PCI based radios.

AHB:
debugfs/ath11k/IPQ8074/

PCI
debugfs/ath11k/QCA6390_0001:01:00.0/
debugfs/ath11k/QCA6390_0000:01:00.0/

Thanks
Anil
Ben Greear Sept. 4, 2020, 3:34 p.m. UTC | #2
On 9/4/20 8:00 AM, akolli@codeaurora.org wrote:
> On 2020-09-03 22:53, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> The code is trying to create an ath11k directory on debugfs
>> root, but that fails when there is a second radio (and thus
>> second instance of the driver).
>>
> 
> Hi Ben,
> 
> IPQ8074 is soc and it does not need second debugfs entry, its on AHB.
> QCA6390 is a pci card and multiple debug entries are needed if multiple cards are available,
> I am planning to post a patch with bus specific abstraction function which creates single debugfs for soc based radios and multiple entries for PCI based radios.
> 
> AHB:
> debugfs/ath11k/IPQ8074/
> 
> PCI
> debugfs/ath11k/QCA6390_0001:01:00.0/
> debugfs/ath11k/QCA6390_0000:01:00.0/

I appreciate your feedback on this!

Why not just stick with the way ath10k does, having the ath11k debugfs entry be
under the phy debugfs directory?

Maybe there is a high-level design document for the ath11k driver somewhere that
explains how and why it is architected with the ath11k base object holding multiple
phy objects?

And, very curious to know if you are able to get multiple QCA6390 NICs running on the
same system, I'm seeing all sort of problems including NICs not always showing up in lspci
and such.

Thanks,
Ben

> 
> Thanks
> Anil
>
Anilkumar Kolli Sept. 4, 2020, 4:49 p.m. UTC | #3
On 2020-09-04 21:04, Ben Greear wrote:
> On 9/4/20 8:00 AM, akolli@codeaurora.org wrote:
>> On 2020-09-03 22:53, greearb@candelatech.com wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>> 
>>> The code is trying to create an ath11k directory on debugfs
>>> root, but that fails when there is a second radio (and thus
>>> second instance of the driver).
>>> 
>> 
>> Hi Ben,
>> 
>> IPQ8074 is soc and it does not need second debugfs entry, its on AHB.
>> QCA6390 is a pci card and multiple debug entries are needed if 
>> multiple cards are available,
>> I am planning to post a patch with bus specific abstraction function 
>> which creates single debugfs for soc based radios and multiple entries 
>> for PCI based radios.
>> 
>> AHB:
>> debugfs/ath11k/IPQ8074/
>> 
>> PCI
>> debugfs/ath11k/QCA6390_0001:01:00.0/
>> debugfs/ath11k/QCA6390_0000:01:00.0/
> 
> I appreciate your feedback on this!
> 
> Why not just stick with the way ath10k does, having the ath11k debugfs 
> entry be
> under the phy debugfs directory?
> 
> Maybe there is a high-level design document for the ath11k driver 
> somewhere that
> explains how and why it is architected with the ath11k base object
> holding multiple
> phy objects?
> 

The initial ath11k driver was supporting IPQ8074, its an SOC and has 
3-radios under wifi0,
- few entries in debugfs are common for SOC,all these are under
       debugfs/ath11k/IPQ8074/
           "simulate_fw_crash"
           "soc_dp_stats"
- few entries are per radio, these are under
       debugfs/ath11k/IPQ8074/mac0/
             "ext_tx_stats"
             "ext_rx_stats"
             "pktlog_filter"
       debugfs/ath11k/IPQ8074/mac1
             "ext_tx_stats"
             "ext_rx_stats"
             "pktlog_filter"
       debugfs/ath11k/IPQ8074/mac2
             "ext_tx_stats"
             "ext_rx_stats"
             "pktlog_filter"

> And, very curious to know if you are able to get multiple QCA6390 NICs
> running on the
> same system, I'm seeing all sort of problems including NICs not always
> showing up in lspci
> and such.
> 
I am not aware of this, like to understand the output of, qrtr-lookup 
cmd

Thanks
Anil
Ben Greear Sept. 4, 2020, 4:57 p.m. UTC | #4
On 9/4/20 9:49 AM, akolli@codeaurora.org wrote:
> On 2020-09-04 21:04, Ben Greear wrote:
>> On 9/4/20 8:00 AM, akolli@codeaurora.org wrote:
>>> On 2020-09-03 22:53, greearb@candelatech.com wrote:
>>>> From: Ben Greear <greearb@candelatech.com>
>>>>
>>>> The code is trying to create an ath11k directory on debugfs
>>>> root, but that fails when there is a second radio (and thus
>>>> second instance of the driver).
>>>>
>>>
>>> Hi Ben,
>>>
>>> IPQ8074 is soc and it does not need second debugfs entry, its on AHB.
>>> QCA6390 is a pci card and multiple debug entries are needed if multiple cards are available,
>>> I am planning to post a patch with bus specific abstraction function which creates single debugfs for soc based radios and multiple entries for PCI based 
>>> radios.
>>>
>>> AHB:
>>> debugfs/ath11k/IPQ8074/
>>>
>>> PCI
>>> debugfs/ath11k/QCA6390_0001:01:00.0/
>>> debugfs/ath11k/QCA6390_0000:01:00.0/
>>
>> I appreciate your feedback on this!
>>
>> Why not just stick with the way ath10k does, having the ath11k debugfs entry be
>> under the phy debugfs directory?
>>
>> Maybe there is a high-level design document for the ath11k driver somewhere that
>> explains how and why it is architected with the ath11k base object
>> holding multiple
>> phy objects?
>>
> 
> The initial ath11k driver was supporting IPQ8074, its an SOC and has 3-radios under wifi0,
> - few entries in debugfs are common for SOC,all these are under
>        debugfs/ath11k/IPQ8074/
>            "simulate_fw_crash"
>            "soc_dp_stats"

Ok, so one 8074 firmware instance is handling multiple radios, and if firmware crashes, then
it would take down every radio at once?

> - few entries are per radio, these are under
>        debugfs/ath11k/IPQ8074/mac0/
>              "ext_tx_stats"
>              "ext_rx_stats"
>              "pktlog_filter"
>        debugfs/ath11k/IPQ8074/mac1
>              "ext_tx_stats"
>              "ext_rx_stats"
>              "pktlog_filter"
>        debugfs/ath11k/IPQ8074/mac2
>              "ext_tx_stats"
>              "ext_rx_stats"
>              "pktlog_filter"
> 
>> And, very curious to know if you are able to get multiple QCA6390 NICs
>> running on the
>> same system, I'm seeing all sort of problems including NICs not always
>> showing up in lspci
>> and such.
>>
> I am not aware of this, like to understand the output of, qrtr-lookup cmd

Based on my testing with debugs, whatever driver I pulled down from the ath tree had zero
chance of working due to immediate crash.  So, have you been able to test multiple
6390 radios yet?

How do I do this qrtr-lookup command?

Thanks,
Ben
Anilkumar Kolli Sept. 4, 2020, 5:45 p.m. UTC | #5
On 2020-09-04 22:27, Ben Greear wrote:
> On 9/4/20 9:49 AM, akolli@codeaurora.org wrote:
>> On 2020-09-04 21:04, Ben Greear wrote:
>>> On 9/4/20 8:00 AM, akolli@codeaurora.org wrote:
>>>> On 2020-09-03 22:53, greearb@candelatech.com wrote:
>>>>> From: Ben Greear <greearb@candelatech.com>
>>>>> 
>>>>> The code is trying to create an ath11k directory on debugfs
>>>>> root, but that fails when there is a second radio (and thus
>>>>> second instance of the driver).
>>>>> 
>>>> 
>>>> Hi Ben,
>>>> 
>>>> IPQ8074 is soc and it does not need second debugfs entry, its on 
>>>> AHB.
>>>> QCA6390 is a pci card and multiple debug entries are needed if 
>>>> multiple cards are available,
>>>> I am planning to post a patch with bus specific abstraction function 
>>>> which creates single debugfs for soc based radios and multiple 
>>>> entries for PCI based radios.
>>>> 
>>>> AHB:
>>>> debugfs/ath11k/IPQ8074/
>>>> 
>>>> PCI
>>>> debugfs/ath11k/QCA6390_0001:01:00.0/
>>>> debugfs/ath11k/QCA6390_0000:01:00.0/
>>> 
>>> I appreciate your feedback on this!
>>> 
>>> Why not just stick with the way ath10k does, having the ath11k 
>>> debugfs entry be
>>> under the phy debugfs directory?
>>> 
>>> Maybe there is a high-level design document for the ath11k driver 
>>> somewhere that
>>> explains how and why it is architected with the ath11k base object
>>> holding multiple
>>> phy objects?
>>> 
>> 
>> The initial ath11k driver was supporting IPQ8074, its an SOC and has 
>> 3-radios under wifi0,
>> - few entries in debugfs are common for SOC,all these are under
>>        debugfs/ath11k/IPQ8074/
>>            "simulate_fw_crash"
>>            "soc_dp_stats"
> 
> Ok, so one 8074 firmware instance is handling multiple radios, and if
> firmware crashes, then
> it would take down every radio at once?
> 
>> - few entries are per radio, these are under
>>        debugfs/ath11k/IPQ8074/mac0/
>>              "ext_tx_stats"
>>              "ext_rx_stats"
>>              "pktlog_filter"
>>        debugfs/ath11k/IPQ8074/mac1
>>              "ext_tx_stats"
>>              "ext_rx_stats"
>>              "pktlog_filter"
>>        debugfs/ath11k/IPQ8074/mac2
>>              "ext_tx_stats"
>>              "ext_rx_stats"
>>              "pktlog_filter"
>> 
>>> And, very curious to know if you are able to get multiple QCA6390 
>>> NICs
>>> running on the
>>> same system, I'm seeing all sort of problems including NICs not 
>>> always
>>> showing up in lspci
>>> and such.
>>> 
>> I am not aware of this, like to understand the output of, qrtr-lookup 
>> cmd
> 
> Based on my testing with debugs, whatever driver I pulled down from
> the ath tree had zero
> chance of working due to immediate crash.  So, have you been able to
> test multiple
> 6390 radios yet?
> 
I am not working on 6390.

> How do I do this qrtr-lookup command?
> 
https://github.com/andersson/qrtr, this has the tools.

Also please share the dmesg | grep ath11k log with both radios.
I understand its crashing, please share the ath11k boot logs.

Thanks
Anil
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index bb22b41fefaa..858dfc700d54 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -707,8 +707,9 @@  struct ath11k_base {
 	/* Current DFS Regulatory */
 	enum ath11k_dfs_region dfs_region;
 #ifdef CONFIG_ATH11K_DEBUGFS
-	struct dentry *debugfs_soc;
-	struct dentry *debugfs_ath11k;
+	struct dentry *debugfs_soc; /* child of debugfs_ath11k */
+	char debugfs_ath11k_fname[36];
+	struct dentry *debugfs_ath11k; /* per driver instance */
 #endif
 	struct ath11k_soc_dp_stats soc_stats;
 
diff --git a/drivers/net/wireless/ath/ath11k/debug.c b/drivers/net/wireless/ath/ath11k/debug.c
index 31978ef9144e..70b4c4837879 100644
--- a/drivers/net/wireless/ath/ath11k/debug.c
+++ b/drivers/net/wireless/ath/ath11k/debug.c
@@ -1011,15 +1011,24 @@  void ath11k_debug_pdev_destroy(struct ath11k_base *ab)
 
 int ath11k_debug_soc_create(struct ath11k_base *ab)
 {
-	ab->debugfs_ath11k = debugfs_create_dir("ath11k", NULL);
+	/* We need one of these per driver instance, try until we find an un-used name */
+	int i;
 
-	if (IS_ERR_OR_NULL(ab->debugfs_ath11k)) {
-		if (IS_ERR(ab->debugfs_ath11k))
-			return PTR_ERR(ab->debugfs_ath11k);
-		return -ENOMEM;
+	for (i = 0; i<99; i++) {
+		snprintf(ab->debugfs_ath11k_fname, sizeof(ab->debugfs_ath11k_fname), "ath11k-%d", i);
+		ab->debugfs_ath11k = debugfs_create_dir(ab->debugfs_ath11k_fname, NULL);
+
+		if (!IS_ERR_OR_NULL(ab->debugfs_ath11k)) {
+			return 0;
+		}
 	}
 
-	return 0;
+	/* Couldn't create one */
+	ab->debugfs_ath11k_fname[0] = 0;
+
+	if (IS_ERR(ab->debugfs_ath11k))
+		return PTR_ERR(ab->debugfs_ath11k);
+	return -ENOMEM;
 }
 
 void ath11k_debug_soc_destroy(struct ath11k_base *ab)
@@ -1228,7 +1237,7 @@  int ath11k_debug_register(struct ath11k *ar)
 	}
 
 	/* Create a symlink under ieee80211/phy* */
-	snprintf(buf, 100, "../../ath11k/%pd2", ar->debug.debugfs_pdev);
+	snprintf(buf, 100, "../../%s/%pd2", ab->debugfs_ath11k_fname, ar->debug.debugfs_pdev);
 	debugfs_create_symlink("ath11k", ar->hw->wiphy->debugfsdir, buf);
 
 	ath11k_debug_htt_stats_init(ar);