All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] wmi:  Handle failure to start scan.
@ 2014-02-13 19:09 ` greearb
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread
* [RFC] wmi:  Handle failure to start scan.
@ 2014-02-13 19:09 ` greearb
  0 siblings, 0 replies; 12+ messages in thread
From: greearb @ 2014-02-13 19:09 UTC (permalink / raw)
  To: ath10k; +Cc: Ben Greear, linux-wireless

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RFC] wmi:  Handle failure to start scan.
  2014-02-13 19:09 ` greearb
@ 2014-02-14  6:13   ` Kalle Valo
  -1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [RFC] wmi:  Handle failure to start scan.
@ 2014-02-14  6:13   ` Kalle Valo
  0 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2014-02-14  6:13 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RFC] wmi: Handle failure to start scan.
  2014-02-13 19:09 ` greearb
@ 2014-02-14  6:47   ` Michal Kazior
  -1 siblings, 0 replies; 12+ messages in thread
From: Michal Kazior @ 2014-02-14  6:47 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k, 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] 12+ messages in thread
* Re: [RFC] wmi: Handle failure to start scan.
@ 2014-02-14  6:47   ` Michal Kazior
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Kazior @ 2014-02-14  6:47 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless, ath10k

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ł

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 12+ 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
  -1 siblings, 0 replies; 12+ messages in thread
From: Ben Greear @ 2014-02-14 16:07 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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] 12+ messages in thread
* Re: [RFC] wmi: Handle failure to start scan.
@ 2014-02-14 16:07     ` Ben Greear
  0 siblings, 0 replies; 12+ messages in thread
From: Ben Greear @ 2014-02-14 16:07 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 12+ 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
  -1 siblings, 0 replies; 12+ messages in thread
From: Michal Kazior @ 2014-02-17  7:42 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless, ath10k

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] 12+ messages in thread
* Re: [RFC] wmi: Handle failure to start scan.
@ 2014-02-17  7:42       ` Michal Kazior
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Kazior @ 2014-02-17  7:42 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless, ath10k

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ł

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 12+ 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
  -1 siblings, 0 replies; 12+ messages in thread
From: Ben Greear @ 2014-02-17 16:10 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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] 12+ messages in thread
* Re: [RFC] wmi: Handle failure to start scan.
@ 2014-02-17 16:10         ` Ben Greear
  0 siblings, 0 replies; 12+ messages in thread
From: Ben Greear @ 2014-02-17 16:10 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-02-17 16:11 UTC | newest]

Thread overview: 12+ 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-13 19:09 ` greearb
2014-02-14  6:13 ` Kalle Valo
2014-02-14  6:13   ` Kalle Valo
2014-02-14  6:47 ` Michal Kazior
2014-02-14  6:47   ` Michal Kazior
2014-02-14 16:07   ` Ben Greear
2014-02-14 16:07     ` Ben Greear
2014-02-17  7:42     ` Michal Kazior
2014-02-17  7:42       ` Michal Kazior
2014-02-17 16:10       ` Ben Greear
2014-02-17 16:10         ` Ben Greear

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.