Re: [PATCH v4 2/2] ath10k: add testmode

[Kalle Valo <kvalo@qca.qualcomm.com>]

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

> On 10 September 2014 17:23, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> [...]
>
> Just a small nitpick:
>
>> +static int ath10k_tm_cmd_utf_stop(struct ath10k *ar, struct nlattr *tb[])
>> +{
>> +       int ret;
>> +
>> +       ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode cmd utf stop\n");
>> +
>> +       mutex_lock(&ar->conf_mutex);
>> +
>> +       if (ar->state != ATH10K_STATE_UTF) {
>> +               ret = -ENETDOWN;
>> +               goto out;
>> +       }
>> +
>> +       __ath10k_tm_cmd_utf_stop(ar);
>> +
>> +       ar->state = ATH10K_STATE_OFF; // <-- this
>
> I would move it to __ath10k_tm_cmd_utf_stop() itself so that the state
> is reset in ath10k_testmode_destroy() as well.

Ok. It doesn't make any difference on the functionality, but I'll change
it anyway. I folded this patch to the commit:

diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c
index 9d77d72042ff..483db9cb8c96 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -253,6 +253,8 @@ static void __ath10k_tm_cmd_utf_stop(struct ath10k *ar)
 
        release_firmware(ar->testmode.utf);
        ar->testmode.utf = NULL;
+
+       ar->state = ATH10K_STATE_OFF;
 }
 
 static int ath10k_tm_cmd_utf_stop(struct ath10k *ar, struct nlattr *tb[])
@@ -270,7 +272,6 @@ static int ath10k_tm_cmd_utf_stop(struct ath10k *ar, struct nlattr *tb[])
 
        __ath10k_tm_cmd_utf_stop(ar);
 
-       ar->state = ATH10K_STATE_OFF;
        ret = 0;
 
        ath10k_info(ar, "UTF firmware stopped\n");

-- 
Kalle Valo