On 2020-09-21 06:27, Kalle Valo wrote:
Kalle Valo <kvalo@xxxxxxxxxxxxxx> writes:
+ rajkumar
Bo YU <tsu.yubo@xxxxxxxxx> writes:
Return value form wait_for_completion_timeout should to be checked.
This is detected by Coverity,#CID:1464479 (CHECKED_RETURN)
FIXES: d5c65159f2895(ath11k: driver for Qualcomm IEEE 802.11ax
devices)
This should be
Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax
devices")
But I can fix that.
--- a/drivers/net/wireless/ath/ath11k/ahb.c
+++ b/drivers/net/wireless/ath/ath11k/ahb.c
@@ -981,12 +981,16 @@ static int ath11k_ahb_probe(struct
platform_device *pdev)
static int ath11k_ahb_remove(struct platform_device *pdev)
{
struct ath11k_base *ab = platform_get_drvdata(pdev);
-
+ int ret = 0;
reinit_completion(&ab->driver_recovery);
if (test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags))
- wait_for_completion_timeout(&ab->driver_recovery,
- ATH11K_AHB_RECOVERY_TIMEOUT);
+ if (!wait_for_completion_timeout(&ab->driver_recovery,
+ ATH11K_AHB_RECOVERY_TIMEOUT)) {
+ ath11k_warn(ab, "fail to receive recovery response
completion.\n");
+ ret = -ETIMEDOUT;
+ }
This is a good find. Rajkumar, can you take a look if this is ok?
Sorry for the delay. wait_for_completion status check LGTM. But return 0
is
intentional as it is required to complete platform deinit properly.
Better
to improve the logging message.
set_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags);
cancel_work_sync(&ab->restart_work);
@@ -999,7 +1003,7 @@ static int ath11k_ahb_remove(struct
platform_device *pdev)
ath11k_core_free(ab);
platform_set_drvdata(pdev, NULL);
- return 0;
+ return ret;
}
Especially I wonder what happens if ath11k_ahb_remove() returns an
error. Should we just print a warning and return 0 instead?
I changed this patch so that we return 0 even if timeout happens, just
to be on the safe side. The patch is now in the pending branch.
Thanks for taking care of this.
Rajkumar
