On 2024/10/24 9:30, Leizhen (ThunderTown) wrote:
>
>
> 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.
Sorry, the ath11k driver code is more complex than I think. I just went through
the code, and those members still need to be kept.
> 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
