Re: [PATCH v2] ath10k: add FW API support to test mode

Manikanta <manikanta.pubbisetty@xxxxxxxxx> · Wed, 21 Oct 2015 12:52:39 +0530

On Wednesday 21 October 2015 12:36 PM, Michal Kazior wrote:
On 20 October 2015 at 09:01, Manikanta Pubbisetty
<manikanta.pubbisetty@xxxxxxxxx> wrote:

On 10/20/2015 04:55 PM, Kalle Valo wrote:
From: Alan Liu <alanliu@xxxxxxxxxxxxxxxx>

Add WMI-TLV and FW API support in ath10k testmode.
Ath10k can get right wmi command format from UTF image
to communicate UTF firmware.

Signed-off-by: Alan Liu <alanliu@xxxxxxxxxxxxxxxx>
Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx>
---
   drivers/net/wireless/ath/ath10k/core.c     |    4 -
   drivers/net/wireless/ath/ath10k/core.h     |    5 +
   drivers/net/wireless/ath/ath10k/hw.h       |    1
   drivers/net/wireless/ath/ath10k/testmode.c |  202
++++++++++++++++++++++++++--
   drivers/net/wireless/ath/ath10k/wmi-tlv.c  |   14 ++
   5 files changed, 205 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 13de3617d5ab..b7a82ae3b3fa 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -568,8 +568,8 @@ static int ath10k_download_fw(struct ath10k *ar, enum
ath10k_firmware_mode mode)
                 }
                 break;
         case ATH10K_FIRMWARE_MODE_UTF:
-               data = ar->testmode.utf->data;
-               data_len = ar->testmode.utf->size;
+               data = ar->testmode.utf_firmware_data;
+               data_len = ar->testmode.utf_firmware_len;
                 mode_name = "utf";
                 break;
         default:
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 7cc7cdd56c95..a6371108be9b 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -814,9 +814,12 @@ struct ath10k {
         struct {
                 /* protected by conf_mutex */
                 const struct firmware *utf;
+               char utf_version[32];
+               const void *utf_firmware_data;
+               size_t utf_firmware_len;
                 DECLARE_BITMAP(orig_fw_features, ATH10K_FW_FEATURE_COUNT);
                 enum ath10k_fw_wmi_op_version orig_wmi_op_version;
-
+               enum ath10k_fw_wmi_op_version op_version;
                 /* protected by data_lock */
                 bool utf_monitor;
         } testmode;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h
b/drivers/net/wireless/ath/ath10k/hw.h
index 2d87737e35ff..0c8ea0226684 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -94,6 +94,7 @@ enum qca6174_chip_id_rev {
   #define ATH10K_FW_API5_FILE           "firmware-5.bin"

   #define ATH10K_FW_UTF_FILE            "utf.bin"
+#define ATH10K_FW_UTF_API2_FILE                "utf-2.bin"

   /* includes also the null byte */
   #define ATH10K_FIRMWARE_MAGIC               "QCA-ATH10K"
diff --git a/drivers/net/wireless/ath/ath10k/testmode.c
b/drivers/net/wireless/ath/ath10k/testmode.c
index b084f88da102..1d5a2fdcbf56 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -139,11 +139,181 @@ static int ath10k_tm_cmd_get_version(struct ath10k
*ar, struct nlattr *tb[])
         return cfg80211_testmode_reply(skb);
   }

-static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr
*tb[])
+static int ath10k_tm_fetch_utf_firmware_api_2(struct ath10k *ar)
+{
+       size_t len, magic_len, ie_len;
+       struct ath10k_fw_ie *hdr;
+       char filename[100];
+       __le32 *version;
+       const u8 *data;
+       int ie_id, ret;
+
+       snprintf(filename, sizeof(filename), "%s/%s",
+                ar->hw_params.fw.dir, ATH10K_FW_UTF_API2_FILE);
+
+       /* load utf firmware image */
+       ret = request_firmware(&ar->testmode.utf, filename, ar->dev);
+       if (ret) {
+               ath10k_warn(ar, "failed to retrieve utf firmware '%s':
%d\n",
+                           filename, ret);
+               return ret;
+       }
+
+       data = ar->testmode.utf->data;
+       len = ar->testmode.utf->size;
+
+       /* FIXME: call release_firmware() in error cases */
+
+       /* magic also includes the null byte, check that as well */
+       magic_len = strlen(ATH10K_FIRMWARE_MAGIC) + 1;
+
+       if (len < magic_len) {
+               ath10k_err(ar, "utf firmware file is too small to contain
magic\n");
+               ret = -EINVAL;
+               goto err;
+       }
+
+       if (memcmp(data, ATH10K_FIRMWARE_MAGIC, magic_len) != 0) {
+               ath10k_err(ar, "invalid firmware magic\n");
+               ret = -EINVAL;
+               goto err;
+       }
+
+       /* jump over the padding */
+       magic_len = ALIGN(magic_len, 4);
+
+       len -= magic_len;
+       data += magic_len;
+
+       /* loop elements */
+       while (len > sizeof(struct ath10k_fw_ie)) {
+               hdr = (struct ath10k_fw_ie *)data;
+
+               ie_id = le32_to_cpu(hdr->id);
+               ie_len = le32_to_cpu(hdr->len);
+
+               len -= sizeof(*hdr);
+               data += sizeof(*hdr);
+
+               if (len < ie_len) {
+                       ath10k_err(ar, "invalid length for FW IE %d (%zu <
%zu)\n",
+                                  ie_id, len, ie_len);
+                       ret = -EINVAL;
+                       goto err;
+               }
+
+               switch (ie_id) {
+               case ATH10K_FW_IE_FW_VERSION:

Something like ATH10K_UTF_IE_FW_VERSION or ATH10K_TESTMODE_IE_FW_VERSION
would be less confusing and better, isn't it?
I don't really see a reason to. UTF is just another main program to
run on the device.

If anything I would suggest to unify the FW API parsing logic to work
with a dedicated structure instead of `struct ath10k` directly, i.e.

   struct ath10k_fw {
     void *data;
     size_t data_len;
     enum wmi_op_version wmi_op_version;
     // ...
   };

   int ath10k_core_fw_api_parse(struct ath10k *ar,
                                struct ath10k_fw *arfw,
                                struct firmware *fw)
    {
      // parse and fill in `arfw`
    }

   struct ath10k {
     // ...
     struct ath10k_fw fw_normal;
     struct ath10k_fw fw_utf;
     // ...
   };


Michał
Hmmm, this way we will have a unified firmware parsing logic. Is this a 
task which can be taken up easily or any other hidden complexities are 
invloved ?.
I mean can we do the changes for current parsing logic and then rework 
the test mode patch ? what is your suggestion ?

-Manikanta
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html






