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(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index a8acb8b7b8d5..beb552108ac3 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -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;
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
index ccdf3d5ba1ab..5bb6fd17fdf6 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs.c
@@ -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
On 12/20/2022 5:42 PM, Kalle Valo wrote:
> 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(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index a8acb8b7b8d5..beb552108ac3 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -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;
>
> diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
> index ccdf3d5ba1ab..5bb6fd17fdf6 100644
> --- a/drivers/net/wireless/ath/ath11k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath11k/debugfs.c
> @@ -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
looks good to me
Vasanth
On Tue, Dec 20, 2022 at 1:16 PM Kalle Valo <[email protected]> wrote:
>
> 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
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]>
>
> 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(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index a8acb8b7b8d5..beb552108ac3 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -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;
>
> diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
> index ccdf3d5ba1ab..5bb6fd17fdf6 100644
> --- a/drivers/net/wireless/ath/ath11k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath11k/debugfs.c
> @@ -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
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
Kalle Valo <[email protected]> wrote:
> 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
> 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
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Robert Marko <[email protected]> writes:
> On Tue, Dec 20, 2022 at 1:16 PM Kalle Valo <[email protected]> wrote:
>>
>> 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
>
>
> 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.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches