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

2014-09-10 Thread Kalle Valo 
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

[PATCH v3 2/2] ath10k: add testmode

2014-09-04 Thread Kalle Valo 
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
---
 drivers/net/wireless/ath/ath10k/Makefile |1 
 drivers/net/wireless/ath/ath10k/core.c   |   88 --
 drivers/net/wireless/ath/ath10k/core.h   |   22 +-
 drivers/net/wireless/ath/ath10k/debug.c  |5 
 drivers/net/wireless/ath/ath10k/debug.h  |1 
 drivers/net/wireless/ath/ath10k/hw.h |2 
 drivers/net/wireless/ath/ath10k/mac.c|9 +
 drivers/net/wireless/ath/ath10k/testmode.c   |  375 ++
 drivers/net/wireless/ath/ath10k/testmode.h   |   46 +++
 drivers/net/wireless/ath/ath10k/testmode_i.h |   70 +
 drivers/net/wireless/ath/ath10k/wmi.c|   17 +
 11 files changed, 606 insertions(+), 30 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/testmode.c
 create mode 100644 drivers/net/wireless/ath/ath10k/testmode.h
 create mode 100644 drivers/net/wireless/ath/ath10k/testmode_i.h

diff --git a/drivers/net/wireless/ath/ath10k/Makefile 
b/drivers/net/wireless/ath/ath10k/Makefile
index 2cfb63ca9327..8b1b1adb477a 100644
--- a/drivers/net/wireless/ath/ath10k/Makefile
+++ b/drivers/net/wireless/ath/ath10k/Makefile
@@ -11,6 +11,7 @@ ath10k_core-y += mac.o \
 bmi.o
 
 ath10k_core-$(CONFIG_ATH10K_DEBUGFS) += spectral.o
+ath10k_core-$(CONFIG_NL80211_TESTMODE) += testmode.o
 ath10k_core-$(CONFIG_ATH10K_TRACING) += trace.o
 
 obj-$(CONFIG_ATH10K_PCI) += ath10k_pci.o
diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 651a6da8adf5..c01a37526ad5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -26,6 +26,7 @@
 #include bmi.h
 #include debug.h
 #include htt.h
+#include testmode.h
 
 unsigned int ath10k_debug_mask;
 static bool uart_print;
@@ -257,21 +258,42 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
return 0;
 }
 
-static int ath10k_download_fw(struct ath10k *ar)
+static int ath10k_download_fw(struct ath10k *ar, enum ath10k_firmware_mode 
mode)
 {
-   u32 address;
+   u32 address, data_len;
+   const char *mode_name;
+   const void *data;
int ret;
 
address = ar-hw_params.patch_load_addr;
 
-   ret = ath10k_bmi_fast_download(ar, address, ar-firmware_data,
-  ar-firmware_len);
+   switch (mode) {
+   case ATH10K_FIRMWARE_MODE_NORMAL:
+   data = ar-firmware_data;
+   data_len = ar-firmware_len;
+   mode_name = normal;
+   break;
+   case ATH10K_FIRMWARE_MODE_UTF:
+   data = ar-testmode.utf-data;
+   data_len = ar-testmode.utf-size;
+   mode_name = utf;
+   break;
+   default:
+   ath10k_err(ar, unknown firmware mode: %d\n, mode);
+   return -EINVAL;
+   }
+
+   ath10k_dbg(ar, ATH10K_DBG_BOOT,
+  boot uploading firmware image %p len %d mode %s\n,
+  data, data_len, mode_name);
+
+   ret = ath10k_bmi_fast_download(ar, address, data, data_len);
if (ret) {
-   ath10k_err(ar, could not write fw (%d)\n, ret);
-   goto exit;
+   ath10k_err(ar, failed to download %s firmware: %d\n,
+  mode_name, ret);
+   return ret;
}
 
-exit:
return ret;
 }
 
@@ -567,7 +589,8 @@ success:
return 0;
 }
 
-static int ath10k_init_download_firmware(struct ath10k *ar)
+static int ath10k_init_download_firmware(struct ath10k *ar,
+enum ath10k_firmware_mode mode)
 {
int ret;
 
@@ -583,7 +606,7 @@ static int ath10k_init_download_firmware(struct ath10k *ar)
return ret;
}
 
-   ret = ath10k_download_fw(ar);
+   ret = ath10k_download_fw(ar, mode);
if (ret) {
ath10k_err(ar, failed to download firmware: %d\n, ret);
return ret;
@@ -685,12 +708,15 @@ static void ath10k_core_restart(struct work_struct *work)
case ATH10K_STATE_WEDGED:
ath10k_warn(ar, device is wedged, will not restart\n);
break;
+   case ATH10K_STATE_UTF:
+   ath10k_warn(ar, firmware restart in UTF mode not supported\n);
+   break;
}
 
mutex_unlock(ar-conf_mutex);
 }
 
-int ath10k_core_start(struct ath10k *ar)
+int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode)
 {
int status;
 
@@ -703,7 +729,7 @@ int ath10k_core_start(struct ath10k *ar)
goto err;
}
 
-   status = ath10k_init_download_firmware(ar);
+   status =