Skip to contentSkip to conversation

ath11k: debugfs: fix to work with multiple PCI devices

PATCH In review Kalle Valo 3 participants 5 messages Dec 20, 2022 – Jan 12, 2023 Qualcomm Atheros ATH11K wireless driver

lore
Share & tools
lore

Kalle Valo

author
#0 lore raw

PATCH 1/1 ath11k: debugfs: fix to work with multiple PCI devices

From: Kalle Valo <[email protected]>

ath11k fails to load if there are multiple ath11k PCI devices with same name:

ath11k_pci 0000:01:00.0: Hardware name qcn9074 hw1.0
debugfs: Directory 'ath11k' with parent '/' already present! ath11k_pci 0000:01:00.0: failed to create ath11k debugfs ath11k_pci 0000:01:00.0: failed to create soc core: -17 ath11k_pci 0000:01:00.0: failed to init core: -17
ath11k_pci: probe of 0000:01:00.0 failed with error -17

Fix this by creating a directory for each ath11k device using schema <bus>-<devname>, for example "pci-0000:06:00.0". This directory created under the top-level ath11k directory, for example /sys/kernel/debug/ath11k.

The reference to the toplevel ath11k directory is not stored anymore within ath11k, instead it's retrieved using debugfs_lookup(). If the directory does not exist it will be created. After the last directory from the ath11k directory is removed, for example when doing rmmod ath11k, the empty ath11k directory is left in place, it's a minor cosmetic issue anyway.

Here's an example hierarchy with one WCN6855:

ath11k
`-- pci-0000:06:00.0

    |-- mac0
    |   |-- dfs_block_radar_events
    |   |-- dfs_simulate_radar
    |   |-- ext_rx_stats
    |   |-- ext_tx_stats
    |   |-- fw_dbglog_config
    |   |-- fw_stats
    |   |   |-- beacon_stats
    |   |   |-- pdev_stats
    |   |   `-- vdev_stats
    |   |-- htt_stats
    |   |-- htt_stats_reset
    |   |-- htt_stats_type
    |   `-- pktlog_filter
    |-- simulate_fw_crash
    `-- soc_dp_stats

I didn't have a test setup where I could connect multiple ath11k devices to the same the host, so I have only tested this with one device.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

Signed-off-by Kalle Valo <[email protected]>
---
 drivers/net/wireless/ath/ath11k/core.h    |  1 -
 drivers/net/wireless/ath/ath11k/debugfs.c | 48 +++++++++++++++++++----
 2 files changed, 40 insertions(+), 9 deletions(-)
drivers/net/wireless/ath/ath11k/core.h +0 -1
index a8acb8b7b8d5..beb552108ac3 100644
@@ -921,7 +921,6 @@ struct ath11k_base {
 	enum ath11k_dfs_region dfs_region;
 #ifdef CONFIG_ATH11K_DEBUGFS
 	struct dentry *debugfs_soc;
-	struct dentry *debugfs_ath11k;
 #endif
 	struct ath11k_soc_dp_stats soc_stats;
drivers/net/wireless/ath/ath11k/debugfs.c +40 -8

65 lines not on this page · raw message

base-commit: 922932ca02191a390f7f52fb6e21c44b50e14025

Signature · 1 line
2.30.2
#1 lore raw
Quoted 137 lines · Kalle Valo

From: Kalle Valo <[email protected]>

ath11k fails to load if there are multiple ath11k PCI devices with same name:

ath11k_pci 0000:01:00.0: Hardware name qcn9074 hw1.0
debugfs: Directory 'ath11k' with parent '/' already present! ath11k_pci 0000:01:00.0: failed to create ath11k debugfs ath11k_pci 0000:01:00.0: failed to create soc core: -17 ath11k_pci 0000:01:00.0: failed to init core: -17
ath11k_pci: probe of 0000:01:00.0 failed with error -17

Fix this by creating a directory for each ath11k device using schema <bus>-<devname>, for example "pci-0000:06:00.0". This directory created under the top-level ath11k directory, for example /sys/kernel/debug/ath11k.

The reference to the toplevel ath11k directory is not stored anymore within ath11k, instead it's retrieved using debugfs_lookup(). If the directory does not exist it will be created. After the last directory from the ath11k directory is removed, for example when doing rmmod ath11k, the empty ath11k directory is left in place, it's a minor cosmetic issue anyway.

Here's an example hierarchy with one WCN6855:

ath11k
`-- pci-0000:06:00.0

     |-- mac0
     |   |-- dfs_block_radar_events
     |   |-- dfs_simulate_radar
     |   |-- ext_rx_stats
     |   |-- ext_tx_stats
     |   |-- fw_dbglog_config
     |   |-- fw_stats
     |   |   |-- beacon_stats
     |   |   |-- pdev_stats
     |   |   `-- vdev_stats
     |   |-- htt_stats
     |   |-- htt_stats_reset
     |   |-- htt_stats_type
     |   `-- pktlog_filter
     |-- simulate_fw_crash
     `-- soc_dp_stats

I didn't have a test setup where I could connect multiple ath11k devices to the same the host, so I have only tested this with one device.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

Signed-off-by Kalle Valo <[email protected]>
---
  drivers/net/wireless/ath/ath11k/core.h    |  1 -
  drivers/net/wireless/ath/ath11k/debugfs.c | 48 +++++++++++++++++++----
  2 files changed, 40 insertions(+), 9 deletions(-)
drivers/net/wireless/ath/ath11k/core.h +0 -1
index a8acb8b7b8d5..beb552108ac3 100644
@@ -921,7 +921,6 @@ struct ath11k_base {
  	enum ath11k_dfs_region dfs_region;
  #ifdef CONFIG_ATH11K_DEBUGFS
  	struct dentry *debugfs_soc;
-	struct dentry *debugfs_ath11k;
  #endif
  	struct ath11k_soc_dp_stats soc_stats;
drivers/net/wireless/ath/ath11k/debugfs.c +40 -8
index ccdf3d5ba1ab..5bb6fd17fdf6 100644
@@ -976,10 +976,6 @@ int ath11k_debugfs_pdev_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);
-
  	debugfs_create_file("simulate_fw_crash", 0600, ab->debugfs_soc, ab,
  			    &fops_simulate_fw_crash);
 

@@ -1001,15 +997,51 @@ void ath11k_debugfs_pdev_destroy(struct ath11k_base *ab)
 
  int ath11k_debugfs_soc_create(struct ath11k_base *ab)
  {
-	ab->debugfs_ath11k = debugfs_create_dir("ath11k", NULL);
+	struct dentry *root;
+	bool dput_needed;
+	char name[64];
+	int ret;
+
+	root = debugfs_lookup("ath11k", NULL);
+	if (!root) {
+		root = debugfs_create_dir("ath11k", NULL);
+		if (IS_ERR_OR_NULL(root))
+			return PTR_ERR(root);
+
+		dput_needed = false;
+	} else {
+		/* a dentry from lookup() needs dput() after we don't use it */
+		dput_needed = true;
+	}
+
+	scnprintf(name, sizeof(name), "%s-%s", ath11k_bus_str(ab->hif.bus),
+		  dev_name(ab->dev));
+
+	ab->debugfs_soc = debugfs_create_dir(name, root);
+	if (IS_ERR_OR_NULL(ab->debugfs_soc)) {
+		ret = PTR_ERR(ab->debugfs_soc);
+		goto out;
+	}
+
+	ret = 0;
 
-	return PTR_ERR_OR_ZERO(ab->debugfs_ath11k);
+out:
+	if (dput_needed)
+		dput(root);
+
+	return ret;
  }
 
  void ath11k_debugfs_soc_destroy(struct ath11k_base *ab)
  {
-	debugfs_remove_recursive(ab->debugfs_ath11k);
-	ab->debugfs_ath11k = NULL;
+	debugfs_remove_recursive(ab->debugfs_soc);
+	ab->debugfs_soc = NULL;
+
+	/* We are not removing ath11k directory on purpose, even if it
+	 * would be empty. This simplifies the directory handling and it's
+	 * a minor cosmetic issue to leave an empty ath11k directory to
+	 * debugfs.
+	 */
  }
  EXPORT_SYMBOL(ath11k_debugfs_soc_destroy);
Quoted 1 line

base-commit: 922932ca02191a390f7f52fb6e21c44b50e14025

looks good to me

Vasanth

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
Tested-by on v1 1/1
#2 lore raw
In reply to Kalle Valo
Quoted 43 lines · Kalle Valo

From: Kalle Valo <[email protected]>

ath11k fails to load if there are multiple ath11k PCI devices with same name:

ath11k_pci 0000:01:00.0: Hardware name qcn9074 hw1.0
debugfs: Directory 'ath11k' with parent '/' already present! ath11k_pci 0000:01:00.0: failed to create ath11k debugfs ath11k_pci 0000:01:00.0: failed to create soc core: -17 ath11k_pci 0000:01:00.0: failed to init core: -17
ath11k_pci: probe of 0000:01:00.0 failed with error -17

Fix this by creating a directory for each ath11k device using schema <bus>-<devname>, for example "pci-0000:06:00.0". This directory created under the top-level ath11k directory, for example /sys/kernel/debug/ath11k.

The reference to the toplevel ath11k directory is not stored anymore within ath11k, instead it's retrieved using debugfs_lookup(). If the directory does not exist it will be created. After the last directory from the ath11k directory is removed, for example when doing rmmod ath11k, the empty ath11k directory is left in place, it's a minor cosmetic issue anyway.

Here's an example hierarchy with one WCN6855:

ath11k
`-- pci-0000:06:00.0

    |-- mac0
    |   |-- dfs_block_radar_events
    |   |-- dfs_simulate_radar
    |   |-- ext_rx_stats
    |   |-- ext_tx_stats
    |   |-- fw_dbglog_config
    |   |-- fw_stats
    |   |   |-- beacon_stats
    |   |   |-- pdev_stats
    |   |   `-- vdev_stats
    |   |-- htt_stats
    |   |-- htt_stats_reset
    |   |-- htt_stats_type
    |   `-- pktlog_filter
    |-- simulate_fw_crash
    `-- soc_dp_stats
Quoted 4 lines

I didn't have a test setup where I could connect multiple ath11k devices to the same the host, so I have only tested this with one device.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

I can confirm that this works on a combination of IPQ8074 with an external QCN9074 radio which
would previously clash and error out, so:

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1

Tested-by Robert Marko <[email protected]>
Quoted 100 lines
Signed-off-by Kalle Valo <[email protected]>
---
 drivers/net/wireless/ath/ath11k/core.h    |  1 -
 drivers/net/wireless/ath/ath11k/debugfs.c | 48 +++++++++++++++++++----
 2 files changed, 40 insertions(+), 9 deletions(-)
drivers/net/wireless/ath/ath11k/core.h +0 -1
index a8acb8b7b8d5..beb552108ac3 100644
@@ -921,7 +921,6 @@ struct ath11k_base {
        enum ath11k_dfs_region dfs_region;
 #ifdef CONFIG_ATH11K_DEBUGFS
        struct dentry *debugfs_soc;
-       struct dentry *debugfs_ath11k;
 #endif
        struct ath11k_soc_dp_stats soc_stats;
drivers/net/wireless/ath/ath11k/debugfs.c +40 -8
index ccdf3d5ba1ab..5bb6fd17fdf6 100644
@@ -976,10 +976,6 @@ int ath11k_debugfs_pdev_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);
-
        debugfs_create_file("simulate_fw_crash", 0600, ab->debugfs_soc, ab,
                            &fops_simulate_fw_crash);
 

@@ -1001,15 +997,51 @@ void ath11k_debugfs_pdev_destroy(struct ath11k_base *ab)
 
 int ath11k_debugfs_soc_create(struct ath11k_base *ab)
 {
-       ab->debugfs_ath11k = debugfs_create_dir("ath11k", NULL);
+       struct dentry *root;
+       bool dput_needed;
+       char name[64];
+       int ret;
+
+       root = debugfs_lookup("ath11k", NULL);
+       if (!root) {
+               root = debugfs_create_dir("ath11k", NULL);
+               if (IS_ERR_OR_NULL(root))
+                       return PTR_ERR(root);
+
+               dput_needed = false;
+       } else {
+               /* a dentry from lookup() needs dput() after we don't use it */
+               dput_needed = true;
+       }
+
+       scnprintf(name, sizeof(name), "%s-%s", ath11k_bus_str(ab->hif.bus),
+                 dev_name(ab->dev));
+
+       ab->debugfs_soc = debugfs_create_dir(name, root);
+       if (IS_ERR_OR_NULL(ab->debugfs_soc)) {
+               ret = PTR_ERR(ab->debugfs_soc);
+               goto out;
+       }
+
+       ret = 0;
 
-       return PTR_ERR_OR_ZERO(ab->debugfs_ath11k);
+out:
+       if (dput_needed)
+               dput(root);
+
+       return ret;
 }
 
 void ath11k_debugfs_soc_destroy(struct ath11k_base *ab)
 {
-       debugfs_remove_recursive(ab->debugfs_ath11k);
-       ab->debugfs_ath11k = NULL;
+       debugfs_remove_recursive(ab->debugfs_soc);
+       ab->debugfs_soc = NULL;
+
+       /* We are not removing ath11k directory on purpose, even if it
+        * would be empty. This simplifies the directory handling and it's
+        * a minor cosmetic issue to leave an empty ath11k directory to
+        * debugfs.
+        */
 }
 EXPORT_SYMBOL(ath11k_debugfs_soc_destroy);

base-commit: 922932ca02191a390f7f52fb6e21c44b50e14025 --
2.30.2

--
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k

Signature · 7 lines
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: www.sartura.hr
author
#3 lore raw
Quoted 50 lines · Robert Marko
Quoted 43 lines · Kalle Valo

From: Kalle Valo <[email protected]>

ath11k fails to load if there are multiple ath11k PCI devices with same name:

ath11k_pci 0000:01:00.0: Hardware name qcn9074 hw1.0
debugfs: Directory 'ath11k' with parent '/' already present! ath11k_pci 0000:01:00.0: failed to create ath11k debugfs ath11k_pci 0000:01:00.0: failed to create soc core: -17 ath11k_pci 0000:01:00.0: failed to init core: -17
ath11k_pci: probe of 0000:01:00.0 failed with error -17

Fix this by creating a directory for each ath11k device using schema <bus>-<devname>, for example "pci-0000:06:00.0". This directory created under the top-level ath11k directory, for example /sys/kernel/debug/ath11k.

The reference to the toplevel ath11k directory is not stored anymore within ath11k, instead it's retrieved using debugfs_lookup(). If the directory does not exist it will be created. After the last directory from the ath11k directory is removed, for example when doing rmmod ath11k, the empty ath11k directory is left in place, it's a minor cosmetic issue anyway.

Here's an example hierarchy with one WCN6855:

ath11k
`-- pci-0000:06:00.0

    |-- mac0
    |   |-- dfs_block_radar_events
    |   |-- dfs_simulate_radar
    |   |-- ext_rx_stats
    |   |-- ext_tx_stats
    |   |-- fw_dbglog_config
    |   |-- fw_stats
    |   |   |-- beacon_stats
    |   |   |-- pdev_stats
    |   |   `-- vdev_stats
    |   |-- htt_stats
    |   |-- htt_stats_reset
    |   |-- htt_stats_type
    |   `-- pktlog_filter
    |-- simulate_fw_crash
    `-- soc_dp_stats
Quoted 4 lines

I didn't have a test setup where I could connect multiple ath11k devices to the same the host, so I have only tested this with one device.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

Quoted 6 lines

I can confirm that this works on a combination of IPQ8074 with an external QCN9074 radio which would previously clash and error out, so:

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1

Tested-by Robert Marko <[email protected]>

Great, thank you for testing this. Very much appreciated. I added the tags tags to the commit.

Signature · 8 lines
https://patchwork.kernel.org/project/linux-wireless/list/

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

-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
author
#4 lore raw
In reply to Kalle Valo
Quoted 43 lines · Kalle Valo

ath11k fails to load if there are multiple ath11k PCI devices with same name:

ath11k_pci 0000:01:00.0: Hardware name qcn9074 hw1.0
debugfs: Directory 'ath11k' with parent '/' already present! ath11k_pci 0000:01:00.0: failed to create ath11k debugfs ath11k_pci 0000:01:00.0: failed to create soc core: -17 ath11k_pci 0000:01:00.0: failed to init core: -17
ath11k_pci: probe of 0000:01:00.0 failed with error -17

Fix this by creating a directory for each ath11k device using schema <bus>-<devname>, for example "pci-0000:06:00.0". This directory created under the top-level ath11k directory, for example /sys/kernel/debug/ath11k.

The reference to the toplevel ath11k directory is not stored anymore within ath11k, instead it's retrieved using debugfs_lookup(). If the directory does not exist it will be created. After the last directory from the ath11k directory is removed, for example when doing rmmod ath11k, the empty ath11k directory is left in place, it's a minor cosmetic issue anyway.

Here's an example hierarchy with one WCN6855:

ath11k
`-- pci-0000:06:00.0

    |-- mac0
    |   |-- dfs_block_radar_events
    |   |-- dfs_simulate_radar
    |   |-- ext_rx_stats
    |   |-- ext_tx_stats
    |   |-- fw_dbglog_config
    |   |-- fw_stats
    |   |   |-- beacon_stats
    |   |   |-- pdev_stats
    |   |   `-- vdev_stats
    |   |-- htt_stats
    |   |-- htt_stats_reset
    |   |-- htt_stats_type
    |   `-- pktlog_filter
    |-- simulate_fw_crash
    `-- soc_dp_stats

I didn't have a test setup where I could connect multiple ath11k devices to the same the host, so I have only tested this with one device.

Quoted 6 lines

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9
Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1

Tested-by Robert Marko <[email protected]>
Signed-off-by Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

323d91d4684d wifi: ath11k: debugfs: fix to work with multiple PCI devices

Signature · 9 lines
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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


-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k