[PATCH] wifi: ath9k: Remove error checks when creating debugfs entries

[PATCH] wifi: ath9k: Remove error checks when creating debugfs entries

[Toke Høiland-Jørgensen]

From: Toke Høiland-Jørgensen <toke@redhat.com>

We should not be checking the return values from debugfs creation at all: the
debugfs functions are designed to handle errors of previously called functions
and just transparently abort the creation of debugfs entries when debugfs is
disabled. If we check the return value and abort driver initialisation, we break
the driver if debugfs is disabled (such as when booting with debugfs=off).

Earlier versions of ath9k accidentally did the right thing by checking the
return value, but only for NULL, not for IS_ERR(). This was "fixed" by the two
commits referenced below, breaking ath9k with debugfs=off starting from the 6.6
kernel (as reported in the Bugzilla linked below).

Restore functionality by just getting rid of the return value check entirely.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=219122
Fixes: 1e4134610d93 ("wifi: ath9k: use IS_ERR() with debugfs_create_dir()")
Fixes: 6edb4ba6fb5b ("wifi: ath9k: fix parameter check in ath9k_init_debug()")
Reported-by: dan.g.tob@gmail.com
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 drivers/net/wireless/ath/ath9k/debug.c         | 2 --
 drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index d84e3ee7b5d9..bf3da631c69f 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1380,8 +1380,6 @@ int ath9k_init_debug(struct ath_hw *ah)
 
 	sc->debug.debugfs_phy = debugfs_create_dir("ath9k",
 						   sc->hw->wiphy->debugfsdir);
-	if (IS_ERR(sc->debug.debugfs_phy))
-		return -ENOMEM;
 
 #ifdef CONFIG_ATH_DEBUG
 	debugfs_create_file("debug", 0600, sc->debug.debugfs_phy,
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
index f7c6d9bc9311..9437d69877cc 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
@@ -486,8 +486,6 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
 
 	priv->debug.debugfs_phy = debugfs_create_dir(KBUILD_MODNAME,
 					     priv->hw->wiphy->debugfsdir);
-	if (IS_ERR(priv->debug.debugfs_phy))
-		return -ENOMEM;
 
 	ath9k_cmn_spectral_init_debug(&priv->spec_priv, priv->debug.debugfs_phy);
 
-- 
2.45.2

Re: [PATCH] wifi: ath9k: Remove error checks when creating debugfs entries

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> We should not be checking the return values from debugfs creation at all: the
> debugfs functions are designed to handle errors of previously called functions
> and just transparently abort the creation of debugfs entries when debugfs is
> disabled. If we check the return value and abort driver initialisation, we break
> the driver if debugfs is disabled (such as when booting with debugfs=off).
>
> Earlier versions of ath9k accidentally did the right thing by checking the
> return value, but only for NULL, not for IS_ERR(). This was "fixed" by the two
> commits referenced below, breaking ath9k with debugfs=off starting from the 6.6
> kernel (as reported in the Bugzilla linked below).
>
> Restore functionality by just getting rid of the return value check entirely.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219122
> Fixes: 1e4134610d93 ("wifi: ath9k: use IS_ERR() with debugfs_create_dir()")
> Fixes: 6edb4ba6fb5b ("wifi: ath9k: fix parameter check in ath9k_init_debug()")
> Reported-by: dan.g.tob@gmail.com
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

From the bugzilla entry above:

Tested-by: Daniel Tobias <dan.g.tob@gmail.com>


Kalle, if you could also update the reported-by to include Daniel's full
name when applying, that would be awesome :)

-Toke

Re: [PATCH] wifi: ath9k: Remove error checks when creating debugfs entries

[Kalle Valo]

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> Toke Høiland-Jørgensen <toke@toke.dk> writes:
>
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> We should not be checking the return values from debugfs creation at all: the
>> debugfs functions are designed to handle errors of previously called functions
>> and just transparently abort the creation of debugfs entries when debugfs is
>> disabled. If we check the return value and abort driver initialisation, we break
>> the driver if debugfs is disabled (such as when booting with debugfs=off).
>>
>> Earlier versions of ath9k accidentally did the right thing by checking the
>> return value, but only for NULL, not for IS_ERR(). This was "fixed" by the two
>> commits referenced below, breaking ath9k with debugfs=off starting from the 6.6
>> kernel (as reported in the Bugzilla linked below).
>>
>> Restore functionality by just getting rid of the return value check entirely.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219122
>> Fixes: 1e4134610d93 ("wifi: ath9k: use IS_ERR() with debugfs_create_dir()")
>> Fixes: 6edb4ba6fb5b ("wifi: ath9k: fix parameter check in ath9k_init_debug()")
>> Reported-by: dan.g.tob@gmail.com
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
>>From the bugzilla entry above:
>
> Tested-by: Daniel Tobias <dan.g.tob@gmail.com>
>
>
> Kalle, if you could also update the reported-by to include Daniel's full
> name when applying, that would be awesome :)

Will do.

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

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

Re: [PATCH] wifi: ath9k: Remove error checks when creating debugfs entries

[Kalle Valo]

Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> We should not be checking the return values from debugfs creation at all: the
> debugfs functions are designed to handle errors of previously called functions
> and just transparently abort the creation of debugfs entries when debugfs is
> disabled. If we check the return value and abort driver initialisation, we break
> the driver if debugfs is disabled (such as when booting with debugfs=off).
> 
> Earlier versions of ath9k accidentally did the right thing by checking the
> return value, but only for NULL, not for IS_ERR(). This was "fixed" by the two
> commits referenced below, breaking ath9k with debugfs=off starting from the 6.6
> kernel (as reported in the Bugzilla linked below).
> 
> Restore functionality by just getting rid of the return value check entirely.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219122
> Fixes: 1e4134610d93 ("wifi: ath9k: use IS_ERR() with debugfs_create_dir()")
> Fixes: 6edb4ba6fb5b ("wifi: ath9k: fix parameter check in ath9k_init_debug()")
> Reported-by: Daniel Tobias <dan.g.tob@gmail.com>
> Tested-by: Daniel Tobias <dan.g.tob@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

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

f6ffe7f01847 wifi: ath9k: Remove error checks when creating debugfs entries

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240805110225.19690-1-toke@toke.dk/

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