From mboxrd@z Thu Jan  1 00:00:00 1970
From: Kalle Valo <kvalo@qca.qualcomm.com>
Date: Fri, 5 Jul 2013 10:20:26 +0300
Subject: [ath9k-devel] [PATCH v3 5/9] ath10k: decouple suspend code
In-Reply-To: <CA+BoTQ=ef6+uKE7TFdP+N7rkN9ASu9K-pV0NyaTzZpyWSyFn-g@mail.gmail.com>
	(Michal Kazior's message of "Fri, 5 Jul 2013 09:12:40 +0200")
References: <1371041642-20273-1-git-send-email-michal.kazior@tieto.com>
	<1372147152-26799-1-git-send-email-michal.kazior@tieto.com>
	<1372147152-26799-6-git-send-email-michal.kazior@tieto.com>
	<87vc4ph53u.fsf@kamboji.qca.qualcomm.com>
	<CA+BoTQ=ef6+uKE7TFdP+N7rkN9ASu9K-pV0NyaTzZpyWSyFn-g@mail.gmail.com>
Message-ID: <87fvvth3r9.fsf@kamboji.qca.qualcomm.com>
List-Id: <ath9k-devel.lists.ath9k.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: ath9k-devel@lists.ath9k.org

Michal Kazior <michal.kazior@tieto.com> writes:

> On 5 July 2013 08:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>> +#ifdef CONFIG_PM
>>> +static int ath10k_suspend(struct ieee80211_hw *hw,
>>> +                       struct cfg80211_wowlan *wowlan)
>>> +{
>>> +     struct ath10k *ar = hw->priv;
>>> +     int ret;
>>> +
>>> +     ar->is_target_paused = false;
>>> +
>>> +     ret = ath10k_wmi_pdev_suspend_target(ar);
>>> +     if (ret) {
>>> +             ath10k_warn("could not suspend target (%d)\n", ret);
>>> +             return 1;
>>> +     }
>>> +
>>> +     ret = wait_event_interruptible_timeout(ar->event_queue,
>>> +                                            ar->is_target_paused == true,
>>> +                                            1 * HZ);
>>> +     if (ret < 0) {
>>> +             ath10k_warn("suspend interrupted (%d)\n", ret);
>>> +             goto resume;
>>> +     } else if (ret == 0) {
>>> +             ath10k_warn("suspend timed out - target pause event never came\n");
>>> +             goto resume;
>>> +     }
>>> +
>>> +     ret = ath10k_hif_suspend(ar);
>>> +     if (ret) {
>>> +             ath10k_warn("could not suspend hif (%d)\n", ret);
>>> +             goto resume;
>>> +     }
>>> +
>>> +     return 0;
>>> +resume:
>>> +     ret = ath10k_wmi_pdev_resume_target(ar);
>>> +     if (ret)
>>> +             ath10k_warn("could not resume target (%d)\n", ret);
>>> +     return 1;
>>> +}
>>
>> No need to change anything now, it's just moving code around anyway, but
>> from a quick look this function looks racy. Can we trust that there's no
>> other code path which could create problems during suspend? Or should we
>> properly serialise this, eg. by using conf_mutex and adding a new state?
>>
>> I hope I didn't ask this already before, I just came back from vacation :)
>
> Right. We could use a conf_mutex here. However this code is not used
> at all. Nothing should trigger it (it's just left for reference) since
> we don't advertise any WoWLAN capabilities to mac80211. I can update
> the patch and include conf_mutex in suspend/resume functions if you
> think that's necessary. Or perhaps we should just scrap the code. The
> code will still remain in git history.

No need to change anything now, the patch is fine as it is. I was just
thinking for the future.

-- 
Kalle Valo