On 2024/10/23 22:02, Jeff Johnson wrote:
> On 10/23/2024 12:40 AM, Zhen Lei wrote:
>> Fix the incorrect return value check for debugfs_create_file(), which
>> returns ERR_PTR(-ERROR) instead of NULL when it fails.
>
> Based upon the commit text this change is incorrect.
>
> * NOTE: it's expected that most callers should _ignore_ the errors returned
> * by this function. Other debugfs functions handle the fact that the "dentry"
> * passed to them could be an error and they don't crash in that case.
> * Drivers should generally work fine even if debugfs fails to init anyway.
>
> So ath11k should not be checking the return value at all, and definitely
> should not be returning -EINVAL since the driver should still operate even if
> creating a debugfs file fails.
OK, so that members such as scan_ctl in struct ath11k_spectral can be removed.
I will update accordingly in v2.
>
>>
>> Fixes: 9d11b7bff950 ("ath11k: add support for spectral scan")
>> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>> ---
>> drivers/net/wireless/ath/ath11k/spectral.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
>> index 79e091134515b43..4c545231292142a 100644
>> --- a/drivers/net/wireless/ath/ath11k/spectral.c
>> +++ b/drivers/net/wireless/ath/ath11k/spectral.c
>> @@ -942,7 +942,7 @@ static inline int ath11k_spectral_debug_register(struct ath11k *ar)
>> 0600,
>> ar->debug.debugfs_pdev, ar,
>> &fops_scan_ctl);
>> - if (!ar->spectral.scan_ctl) {
>> + if (IS_ERR(ar->spectral.scan_ctl)) {
>> ath11k_warn(ar->ab, "failed to open debugfs in pdev %d\n",
>> ar->pdev_idx);
>> ret = -EINVAL;
>> @@ -953,7 +953,7 @@ static inline int ath11k_spectral_debug_register(struct ath11k *ar)
>> 0600,
>> ar->debug.debugfs_pdev, ar,
>> &fops_scan_count);
>> - if (!ar->spectral.scan_count) {
>> + if (IS_ERR(ar->spectral.scan_count)) {
>> ath11k_warn(ar->ab, "failed to open debugfs in pdev %d\n",
>> ar->pdev_idx);
>> ret = -EINVAL;
>> @@ -964,7 +964,7 @@ static inline int ath11k_spectral_debug_register(struct ath11k *ar)
>> 0600,
>> ar->debug.debugfs_pdev, ar,
>> &fops_scan_bins);
>> - if (!ar->spectral.scan_bins) {
>> + if (IS_ERR(ar->spectral.scan_bins)) {
>> ath11k_warn(ar->ab, "failed to open debugfs in pdev %d\n",
>> ar->pdev_idx);
>> ret = -EINVAL;
>
>
> .
>
--
Regards,
Zhen Lei
