* [RFC] wmi: Handle failure to start scan.
@ 2014-02-13 19:09 greearb
2014-02-14 6:13 ` Kalle Valo
2014-02-14 6:47 ` Michal Kazior
0 siblings, 2 replies; 6+ messages in thread
From: greearb @ 2014-02-13 19:09 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Ben Greear
From: Ben Greear <greearb@candelatech.com>
Properly clean up driver state in case firmware fails
to start scan for some reason.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
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;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [RFC] wmi: Handle failure to start scan.
2014-02-13 19:09 [RFC] wmi: Handle failure to start scan greearb
@ 2014-02-14 6:13 ` Kalle Valo
2014-02-14 6:47 ` Michal Kazior
1 sibling, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2014-02-14 6:13 UTC (permalink / raw)
To: greearb; +Cc: ath10k, linux-wireless
greearb@candelatech.com writes:
> From: Ben Greear <greearb@candelatech.com>
>
> Properly clean up driver state in case firmware fails
> to start scan for some reason.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
Why just RFC?
And "ath10k:" prefix missing.
> --- 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);
"scan failed to start: %i\n"
> + ar->scan_channel = NULL;
> + if (!ar->scan.in_progress) {
> + ath10k_warn("scan-start-failed: no scan requested, ignoring\n");
"scan failed to start but no scan requested, ignoring\n"
--
Kalle Valo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] wmi: Handle failure to start scan.
2014-02-13 19:09 [RFC] wmi: Handle failure to start scan greearb
2014-02-14 6:13 ` Kalle Valo
@ 2014-02-14 6:47 ` Michal Kazior
2014-02-14 16:07 ` Ben Greear
1 sibling, 1 reply; 6+ messages in thread
From: Michal Kazior @ 2014-02-14 6:47 UTC (permalink / raw)
To: Ben Greear; +Cc: ath10k@lists.infradead.org, linux-wireless
On 13 February 2014 20:09, <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Properly clean up driver state in case firmware fails
> to start scan for some reason.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 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?
Michał
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] wmi: Handle failure to start scan.
2014-02-14 6:47 ` Michal Kazior
@ 2014-02-14 16:07 ` Ben Greear
2014-02-17 7:42 ` Michal Kazior
0 siblings, 1 reply; 6+ messages in thread
From: Ben Greear @ 2014-02-14 16:07 UTC (permalink / raw)
To: Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
On 02/13/2014 10:47 PM, Michal Kazior wrote:
> On 13 February 2014 20:09, <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Properly clean up driver state in case firmware fails
>> to start scan for some reason.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>> 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 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?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] wmi: Handle failure to start scan.
2014-02-14 16:07 ` Ben Greear
@ 2014-02-17 7:42 ` Michal Kazior
2014-02-17 16:10 ` Ben Greear
0 siblings, 1 reply; 6+ messages in thread
From: Michal Kazior @ 2014-02-17 7:42 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless, ath10k@lists.infradead.org
On 14 February 2014 17:07, Ben Greear <greearb@candelatech.com> wrote:
> On 02/13/2014 10:47 PM, Michal Kazior wrote:
>>
>> On 13 February 2014 20:09, <greearb@candelatech.com> wrote:
>>>
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> Properly clean up driver state in case firmware fails
>>> to start scan for some reason.
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>> 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ł
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] wmi: Handle failure to start scan.
2014-02-17 7:42 ` Michal Kazior
@ 2014-02-17 16:10 ` Ben Greear
0 siblings, 0 replies; 6+ messages in thread
From: Ben Greear @ 2014-02-17 16:10 UTC (permalink / raw)
To: Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
On 02/16/2014 11:42 PM, Michal Kazior wrote:
> On 14 February 2014 17:07, Ben Greear <greearb@candelatech.com> wrote:
>>> 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?
I am working on a 10.1.389 release currently...will move forward when
I can get access to newer source code.
>> 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?
I am not certain the patch is needed. I was looking at my firmware and it
appeared that I could hit the START_FAILED case, but perhaps it was not
really possible, and maybe newer firmware keeps it from happening entirely.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-17 16:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-13 19:09 [RFC] wmi: Handle failure to start scan greearb
2014-02-14 6:13 ` Kalle Valo
2014-02-14 6:47 ` Michal Kazior
2014-02-14 16:07 ` Ben Greear
2014-02-17 7:42 ` Michal Kazior
2014-02-17 16:10 ` Ben Greear
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).