Baochen Qiang <quic_bqiang@xxxxxxxxxxx> writes:
>>> +int ath12k_wow_enable(struct ath12k *ar)
>>> +{
>>> + struct ath12k_base *ab = ar->ab;
>>> + int i, ret;
>>> +
>>> + clear_bit(ATH12K_FLAG_HTC_SUSPEND_COMPLETE, &ab->dev_flags);
>>> +
>>> + for (i = 0; i < ATH12K_WOW_RETRY_NUM; i++) {
>>> + reinit_completion(&ab->htc_suspend);
>>> +
>>> + ret = ath12k_wmi_wow_enable(ar);
>>> + if (ret) {
>>> + ath12k_warn(ab, "failed to issue wow enable: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = wait_for_completion_timeout(&ab->htc_suspend, 3 * HZ);
>>> + if (ret == 0) {
>>> + ath12k_warn(ab,
>>> + "timed out while waiting for htc suspend completion\n");
>>> + return -ETIMEDOUT;
>>> + }
>>> +
>>> + if (test_bit(ATH12K_FLAG_HTC_SUSPEND_COMPLETE, &ab->dev_flags))
>>> + /* success, suspend complete received */
>>> + return 0;
>>> +
>>> + ath12k_warn(ab, "htc suspend not complete, retrying (try %d)\n",
>>> + i);
>>> + msleep(ATH12K_WOW_RETRY_WAIT_MS);
>>> + }
>>> +
>>> + ath12k_warn(ab, "htc suspend not complete, failing after %d tries\n", i);
>>> +
>>> + return -ETIMEDOUT;
>>> +}
>>
>> Why the loop here? Looks really odd to me and no explanation why it's
>> needed. ATH12K_WOW_RETRY_NUM seems to be 10 so this can loop a lot.
>
> Host asks firmware to enter WoW mode using a WMI command. While
> receiving it, firmware might be busy so that can not enter WoW
> immediately. In that case firmware notifies host of
> ATH12K_HTC_MSG_NACK_SUSPEND message, asking host to try again later.
> Per firmware team there could be up to 10 loops.
Not really a good firmware interface, oh well. I added a comment to
ath12k_wow_enable() explaining, thanks.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
