Michal Kazior <michal.kaz...@tieto.com> writes:
> (On 4 September 2014 15:56, Kalle Valo <kv...@qca.qualcomm.com> wrote:
>> Add testmode interface for starting and using UTF firmware which is used to
>> run
>> factory tests. This is implemented by adding new state ATH10K_STATE_UTF and
>> user
>> space can enable this state with ATH10K_TM_CMD_UTF_START command. To go back
>> to
>> normal mode user space can send ATH10K_TM_CMD_UTF_STOP.
>>
>> Signed-off-by: Kalle Valo <kv...@qca.qualcomm.com>
>> ---
> [...]
>> +bool ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb)
>> +{
>> + struct sk_buff *nl_skb;
>> + bool consumed;
>> + int ret;
>> +
>> + ath10k_dbg(ar, ATH10K_DBG_TESTMODE,
>> + "testmode event wmi cmd_id %d skb %p skb->len %d\n",
>> + cmd_id, skb, skb->len);
>> +
>> + ath10k_dbg_dump(ar, ATH10K_DBG_TESTMODE, NULL, "", skb->data,
>> skb->len);
>> +
>> + spin_lock_bh(&ar->data_lock);
>> +
>> + if (!ar->testmode.utf_monitor) {
>> + consumed = false;
>> + goto out;
>> + }
>> +
>> + /* Only testmode.c should be handling events from utf firmware,
>> + * otherwise all sort of problems will arise as mac80211 operations
>> + * are not initialised. */
>
> Comment style.
Argh, will I ever learn :) Fixed now.
>> +static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[])
>> +{
>> + char filename[100];
>> + int ret;
>> +
>> + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode cmd utf start\n");
>> +
>> + mutex_lock(&ar->conf_mutex);
>> +
>> + if (ar->state == ATH10K_STATE_UTF) {
>> + ret = -EALREADY;
>> + goto err;
>> + }
>> +
>> + /* start utf only when the driver is not in use */
>> + if (ar->state != ATH10K_STATE_OFF) {
>> + ret = -EBUSY;
>> + goto err;
>> + }
>> +
>> + if (ar->testmode.utf != NULL)
>> + /* utf image is already downloaded */
>> + goto power_up;
>
> I believe this shouldn't happen with the current code. We should
> probably treat it with a if (WARN_ON(utf != NULL)) goto err;
Yeah, I'll add that.
>> +void ath10k_testmode_destroy(struct ath10k *ar)
>> +{
>> + mutex_lock(&ar->conf_mutex);
>> +
>> + release_firmware(ar->testmode.utf);
>> + ar->testmode.utf = NULL;
>> +
>> + if (ar->state != ATH10K_STATE_UTF) {
>> + /* utf firmware is not running, nothing to do */
>> + goto out;
>> + }
>> +
>> + ath10k_core_stop(ar);
>
> Hmm, you don't call power_down here. This isn't a problem from a
> practical point of view after my recent changes but hey.
>
> How about we split out guts from ath10k_tm_cmd_utf_stop() to
> __ath10k_tm_cmd_utf_stop() (which assumes conf_mutex is already held)
> and call it here like this:
>
> mutex_lock(conf_mutex);
> if (ar->state==UTF)
> __ath10k_tm_cmd_utf_stop(ar);
> mutex_unlock(conf_mutex);
Good idea, I'll do that.
>> @@ -2488,6 +2490,17 @@ static void ath10k_wmi_10x_process_rx(struct ath10k
>> *ar, struct sk_buff *skb)
>>
>> trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
>>
>> + consumed = ath10k_tm_event_wmi(ar, id, skb);
>> +
>> + /* Ready event must be handled normally also in UTF mode so that we
>> + * know the UTF firmware has booted, others we are just bypass WMI
>> + * events to testmode. */
>
> Comment style.
Fixed.
Thanks for the review! I'll send v4 soon.
--
Kalle Valo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
