On 14 February 2014 17:07, Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote:
> On 02/13/2014 10:47 PM, Michal Kazior wrote:
>>
>> On 13 February 2014 20:09, <greearb@xxxxxxxxxxxxxxx> wrote:
>>>
>>> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>>
>>> Properly clean up driver state in case firmware fails
>>> to start scan for some reason.
>>>
>>> Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/net/wireless/ath/ath10k/wmi.c | 20 +++++++++++++++++++-
>>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
>>> b/drivers/net/wireless/ath/ath10k/wmi.c
>>> index 20f7c79..a5be0d3 100644
>>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>>> @@ -770,7 +770,25 @@ static int ath10k_wmi_event_scan(struct ath10k *ar,
>>> struct sk_buff *skb)
>>> ath10k_dbg(ATH10K_DBG_WMI,
>>> "WMI_SCAN_EVENT_PREEMPTED\n");
>>> break;
>>> case WMI_SCAN_EVENT_START_FAILED:
>>> - ath10k_dbg(ATH10K_DBG_WMI,
>>> "WMI_SCAN_EVENT_START_FAILED\n");
>>> + ath10k_warn("WMI_SCAN_EVENT_START_FAILED, reason: %i\n",
>>> reason);
>>> +
>>> + ar->scan_channel = NULL;
>>> + if (!ar->scan.in_progress) {
>>> + ath10k_warn("scan-start-failed: no scan
>>> requested, ignoring\n");
>>> + break;
>>> + }
>>> +
>>> + if (ar->scan.is_roc) {
>>> + ath10k_offchan_tx_purge(ar);
>>> +
>>> + if (!ar->scan.aborting)
>>> +
>>> ieee80211_remain_on_channel_expired(ar->hw);
>>> + } else {
>>> + ieee80211_scan_completed(ar->hw,
>>> ar->scan.aborting);
>>> + }
>>> +
>>> + del_timer(&ar->scan.timeout);
>>> + ar->scan.in_progress = false;
>>> break;
>>> default:
>>> break;
>>
>>
>> We already wait for EVENT_STARTED in mac.c (see ath10k_start_scan) and
>> clean up stuff (ath10k_abort_scan). Why not add the missing bits in
>> there? Or is it possible to get EVENT_START_FAILED *after*
>> EVENT_STARTED? Or am I missing something else here?
>
>
> I think a lot of this would be firmware dependent, and might change between
> various versions of the firmware.
It doesn't make any sense. That would suggest a really ugly firmware
bug. Did you see this (i.e. START_FAILED after STARTED) on 636 or
10.1.467?
> It seems to me we should handle this case and do cleanup just to be safe,
> but maybe cleanup is needed in failure case of ath10k_start_scan as well?
If you really get START_FAILED then you shouldn't have received
STARTED before that. ath10k_start_scan() already waits for the STARTED
event with a timeout and if it fails it triggers a cleanup. If it
doesn't work for you then what perhaps needs to be fixed is the
current cleanup code?
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
