| Message ID | 20240805110225.19690-1-toke@toke.dk |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath9k: Remove error checks when creating debugfs entries | expand |
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>
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.
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
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);