Clean up scan when start fails?

Commit Message

Ben Greear Jan. 5, 2015, 7:04 p.m. UTC

Should we have something like this to clean up the scan
state machine when firmware fails to start the scan?

[greearb@ben-dt2 linux-3.19.dev.y]$ git diff
[greearb@ben-dt2 linux-3.19.dev.y]$


Thanks,
Ben

Comments

Michal Kazior Jan. 7, 2015, 9:41 a.m. UTC | #1

On 5 January 2015 at 20:04, Ben Greear <[e-mail]> wrote:
> Should we have something like this to clean up the scan
> state machine when firmware fails to start the scan?
>
> [greearb@ben-dt2 linux-3.19.dev.y]$ git diff
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index 6e4ece0..9020204 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1057,6 +1057,7 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
>                 break;
>         case WMI_SCAN_EVENT_START_FAILED:
>                 ath10k_warn(ar, "received scan start failure event\n");
> +               ath10k_wmi_event_scan_completed(ar);
>                 break;
>         case WMI_SCAN_EVENT_DEQUEUED:
>         case WMI_SCAN_EVENT_PREEMPTED:

Hmm.. I assume you still get the _START_FAILED sometimes?

In what state do you get it?

If you get it before _STARTED you'll hit a warning in
scan_completed(). It doesn't make sense to call it since all scan
users will fail to wait for ar->scan.started completion and clean up
anyway.

If you get it after _STARTED then I guess it does sense to clean up.


Micha?

Ben Greear Jan. 7, 2015, 1:33 p.m. UTC | #2

On 01/07/2015 01:41 AM, Michal Kazior wrote:
> On 5 January 2015 at 20:04, Ben Greear <[e-mail]> wrote:
>> Should we have something like this to clean up the scan
>> state machine when firmware fails to start the scan?
>>
>> [greearb@ben-dt2 linux-3.19.dev.y]$ git diff
>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
>> index 6e4ece0..9020204 100644
>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>> @@ -1057,6 +1057,7 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
>>                  break;
>>          case WMI_SCAN_EVENT_START_FAILED:
>>                  ath10k_warn(ar, "received scan start failure event\n");
>> +               ath10k_wmi_event_scan_completed(ar);
>>                  break;
>>          case WMI_SCAN_EVENT_DEQUEUED:
>>          case WMI_SCAN_EVENT_PREEMPTED:
>
> Hmm.. I assume you still get the _START_FAILED sometimes?
>
> In what state do you get it?
>
> If you get it before _STARTED you'll hit a warning in
> scan_completed(). It doesn't make sense to call it since all scan
> users will fail to wait for ar->scan.started completion and clean up
> anyway.
>
> If you get it after _STARTED then I guess it does sense to clean up.

I have a better version of this patch that I am testing.

I can get the scan-start-failed even in CT firmware when a vdev
fails to properly start due to firmware resource exhaustion, and
then the driver asks it to scan.

At least in 10.1.467 firmware, the upstream firmware will just panic
when the vdev cannot start in this case, so I am not sure how to
reproduce the event in upstream firmware.  Since the event exists,
I do think it should be properly handled in the driver, however.

Thanks,
Ben

>
>
> Micha?
>

Patch

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 6e4ece0..9020204 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1057,6 +1057,7 @@  static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
                break;
        case WMI_SCAN_EVENT_START_FAILED:
                ath10k_warn(ar, "received scan start failure event\n");
+               ath10k_wmi_event_scan_completed(ar);
                break;
        case WMI_SCAN_EVENT_DEQUEUED:
        case WMI_SCAN_EVENT_PREEMPTED: