| Message ID | 20151020112513.14879.47438.stgit@potku.adurom.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On 10/20/2015 04:55 PM, Kalle Valo wrote: > From: Alan Liu <alanliu@qca.qualcomm.com> > > 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@qca.qualcomm.com> > Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> > --- > 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? > + if (ie_len > sizeof(ar->testmode.utf_version) - 1) > + break; > + > + memcpy(ar->testmode.utf_version, data, ie_len); > + ar->testmode.utf_version[ie_len] = '\0'; > + > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, > + "testmode found fw utf version %s\n", > + ar->testmode.utf_version); > + break; > + case ATH10K_FW_IE_TIMESTAMP: ATH10K_UTF_IE_TIMESTAMP? > + /* ignore timestamp, but don't warn about it either */ > + break; > + case ATH10K_FW_IE_FW_IMAGE: ATH10K_UTF_IE_FW_IMAGE? > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, > + "testmode found fw image ie (%zd B)\n", > + ie_len); > + > + ar->testmode.utf_firmware_data = data; > + ar->testmode.utf_firmware_len = ie_len; > + break; > + case ATH10K_FW_IE_WMI_OP_VERSION: ATH10K_UTF_IE_WMI_OP_VERSION? > + if (ie_len != sizeof(u32)) > + break; > + version = (__le32 *)data; > + ar->testmode.op_version = le32_to_cpup(version); > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode found fw ie wmi op version %d\n", > + ar->testmode.op_version); > + break; > + default: > + ath10k_warn(ar, "Unknown testmode FW IE: %u\n", > + le32_to_cpu(hdr->id)); > + break; > + } > + /* jump over the padding */ > + ie_len = ALIGN(ie_len, 4); > + > + len -= ie_len; > + data += ie_len; > + } > + > + if (!ar->testmode.utf_firmware_data || !ar->testmode.utf_firmware_len) { > + ath10k_err(ar, "No ATH10K_FW_IE_FW_IMAGE found\n"); > + ret = -EINVAL; > + goto err; > + } > + > + return 0; > + > +err: > + release_firmware(ar->testmode.utf); > + > + return ret; > +} > + > +static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar) > { > char filename[100]; > int ret; > > + snprintf(filename, sizeof(filename), "%s/%s", > + ar->hw_params.fw.dir, ATH10K_FW_UTF_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; > + } > + > + /* We didn't find FW UTF API 1 ("utf.bin") does not advertise > + * firmware features. Do an ugly hack where we force the firmware > + * features to match with 10.1 branch so that wmi.c will use the > + * correct WMI interface. > + */ > + > + ar->testmode.op_version = ATH10K_FW_WMI_OP_VERSION_10_1; > + ar->testmode.utf_firmware_data = ar->testmode.utf->data; > + ar->testmode.utf_firmware_len = ar->testmode.utf->size; > + > + return 0; > +} > + > +static int ath10k_tm_fetch_firmware(struct ath10k *ar) > +{ > + int ret; > + > + ret = ath10k_tm_fetch_utf_firmware_api_2(ar); > + if (ret == 0) { > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode using fw utf api 2"); > + return 0; > + } > + > + ret = ath10k_tm_fetch_utf_firmware_api_1(ar); > + if (ret) { > + ath10k_err(ar, "failed to fetch utf firmware binary: %d", ret); > + return ret; > + } > + > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode using utf api 1"); > + > + return 0; > +} > + > +static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) > +{ > + const char *ver; > + int ret; > + > ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode cmd utf start\n"); > > mutex_lock(&ar->conf_mutex); > @@ -165,36 +335,27 @@ static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) > goto err; > } > > - snprintf(filename, sizeof(filename), "%s/%s", > - ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); > - > - /* load utf firmware image */ > - ret = request_firmware(&ar->testmode.utf, filename, ar->dev); > + ret = ath10k_tm_fetch_firmware(ar); > if (ret) { > - ath10k_warn(ar, "failed to retrieve utf firmware '%s': %d\n", > - filename, ret); > + ath10k_err(ar, "failed to fetch UTF firmware: %d", ret); > goto err; > } > > spin_lock_bh(&ar->data_lock); > - > ar->testmode.utf_monitor = true; > - > spin_unlock_bh(&ar->data_lock); > - > BUILD_BUG_ON(sizeof(ar->fw_features) != > sizeof(ar->testmode.orig_fw_features)); > > memcpy(ar->testmode.orig_fw_features, ar->fw_features, > sizeof(ar->fw_features)); > ar->testmode.orig_wmi_op_version = ar->wmi.op_version; > - > - /* utf.bin firmware image does not advertise firmware features. Do > - * an ugly hack where we force the firmware features so that wmi.c > - * will use the correct WMI interface. > - */ > memset(ar->fw_features, 0, sizeof(ar->fw_features)); > - ar->wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_1; > + > + ar->wmi.op_version = ar->testmode.op_version; > + > + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode wmi version %d\n", > + ar->wmi.op_version); > > ret = ath10k_hif_power_up(ar); > if (ret) { > @@ -212,7 +373,12 @@ static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) > > ar->state = ATH10K_STATE_UTF; > > - ath10k_info(ar, "UTF firmware started\n"); > + if (strlen(ar->testmode.utf_version) > 0) > + ver = ar->testmode.utf_version; > + else > + ver = "API 1"; > + > + ath10k_info(ar, "UTF firmware %s started\n", ver); > > mutex_unlock(&ar->conf_mutex); > > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c > index 8f835480a62c..6fbd17b69469 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c > @@ -23,6 +23,7 @@ > #include "wmi-ops.h" > #include "wmi-tlv.h" > #include "p2p.h" > +#include "testmode.h" > > /***************/ > /* TLV helpers */ > @@ -419,6 +420,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb) > { > struct wmi_cmd_hdr *cmd_hdr; > enum wmi_tlv_event_id id; > + bool consumed; > > cmd_hdr = (struct wmi_cmd_hdr *)skb->data; > id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID); > @@ -428,6 +430,18 @@ static void ath10k_wmi_tlv_op_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. > + */ > + if (consumed && id != WMI_TLV_READY_EVENTID) { > + ath10k_dbg(ar, ATH10K_DBG_WMI, > + "wmi tlv testmode consumed 0x%x\n", id); > + goto out; > + } > + > switch (id) { > case WMI_TLV_MGMT_RX_EVENTID: > ath10k_wmi_event_mgmt_rx(ar, skb); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20 October 2015 at 09:01, Manikanta Pubbisetty <manikanta.pubbisetty@gmail.com> wrote: > > > On 10/20/2015 04:55 PM, Kalle Valo wrote: >> >> From: Alan Liu <alanliu@qca.qualcomm.com> >> >> 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@qca.qualcomm.com> >> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> >> --- >> 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? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 21 October 2015 12:36 PM, Michal Kazior wrote: > On 20 October 2015 at 09:01, Manikanta Pubbisetty > <manikanta.pubbisetty@gmail.com> wrote: >> >> On 10/20/2015 04:55 PM, Kalle Valo wrote: >>> From: Alan Liu <alanliu@qca.qualcomm.com> >>> >>> 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@qca.qualcomm.com> >>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21 October 2015 at 09:22, Manikanta <manikanta.pubbisetty@gmail.com> wrote: > On Wednesday 21 October 2015 12:36 PM, Michal Kazior wrote: >> On 20 October 2015 at 09:01, Manikanta Pubbisetty >> <manikanta.pubbisetty@gmail.com> wrote: >>> On 10/20/2015 04:55 PM, Kalle Valo wrote: >>>> >>>> From: Alan Liu <alanliu@qca.qualcomm.com> >>>> >>>> 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@qca.qualcomm.com> >>>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> >>>> --- [...] >>>> + /* 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 > ?. Decoupling the parsing logic should be rather easy. I don't think there are any gotchas. > I mean can we do the changes for current parsing logic and then rework the > test mode patch ? what is your suggestion ? If you want to do the unified parsing logic approach you should first decouple the logic (i.e. make it not fill `struct ath10k` directly) and then rework the testmode patch on top of that. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior <michal.kazior@tieto.com> writes: >> 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 >> ?. > > Decoupling the parsing logic should be rather easy. I don't think > there are any gotchas. I agree. >> I mean can we do the changes for current parsing logic and then rework the >> test mode patch ? what is your suggestion ? > > If you want to do the unified parsing logic approach you should first > decouple the logic (i.e. make it not fill `struct ath10k` directly) > and then rework the testmode patch on top of that. Actually I prefer the other way around, I can first apply the test mode patch and later someone can decouple the logic in a new patch. The code size benefits that from all pretty small so I don't think it's sensible to delay this patch. But decoupling the logic would be nice cleanup to do.
Kalle Valo <kvalo@qca.qualcomm.com> writes: > From: Alan Liu <alanliu@qca.qualcomm.com> > > 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@qca.qualcomm.com> > Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> Applied, thanks.
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: + if (ie_len > sizeof(ar->testmode.utf_version) - 1) + break; + + memcpy(ar->testmode.utf_version, data, ie_len); + ar->testmode.utf_version[ie_len] = '\0'; + + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, + "testmode found fw utf version %s\n", + ar->testmode.utf_version); + break; + case ATH10K_FW_IE_TIMESTAMP: + /* ignore timestamp, but don't warn about it either */ + break; + case ATH10K_FW_IE_FW_IMAGE: + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, + "testmode found fw image ie (%zd B)\n", + ie_len); + + ar->testmode.utf_firmware_data = data; + ar->testmode.utf_firmware_len = ie_len; + break; + case ATH10K_FW_IE_WMI_OP_VERSION: + if (ie_len != sizeof(u32)) + break; + version = (__le32 *)data; + ar->testmode.op_version = le32_to_cpup(version); + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode found fw ie wmi op version %d\n", + ar->testmode.op_version); + break; + default: + ath10k_warn(ar, "Unknown testmode FW IE: %u\n", + le32_to_cpu(hdr->id)); + break; + } + /* jump over the padding */ + ie_len = ALIGN(ie_len, 4); + + len -= ie_len; + data += ie_len; + } + + if (!ar->testmode.utf_firmware_data || !ar->testmode.utf_firmware_len) { + ath10k_err(ar, "No ATH10K_FW_IE_FW_IMAGE found\n"); + ret = -EINVAL; + goto err; + } + + return 0; + +err: + release_firmware(ar->testmode.utf); + + return ret; +} + +static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar) { char filename[100]; int ret; + snprintf(filename, sizeof(filename), "%s/%s", + ar->hw_params.fw.dir, ATH10K_FW_UTF_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; + } + + /* We didn't find FW UTF API 1 ("utf.bin") does not advertise + * firmware features. Do an ugly hack where we force the firmware + * features to match with 10.1 branch so that wmi.c will use the + * correct WMI interface. + */ + + ar->testmode.op_version = ATH10K_FW_WMI_OP_VERSION_10_1; + ar->testmode.utf_firmware_data = ar->testmode.utf->data; + ar->testmode.utf_firmware_len = ar->testmode.utf->size; + + return 0; +} + +static int ath10k_tm_fetch_firmware(struct ath10k *ar) +{ + int ret; + + ret = ath10k_tm_fetch_utf_firmware_api_2(ar); + if (ret == 0) { + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode using fw utf api 2"); + return 0; + } + + ret = ath10k_tm_fetch_utf_firmware_api_1(ar); + if (ret) { + ath10k_err(ar, "failed to fetch utf firmware binary: %d", ret); + return ret; + } + + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode using utf api 1"); + + return 0; +} + +static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) +{ + const char *ver; + int ret; + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode cmd utf start\n"); mutex_lock(&ar->conf_mutex); @@ -165,36 +335,27 @@ static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) goto err; } - snprintf(filename, sizeof(filename), "%s/%s", - ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); - - /* load utf firmware image */ - ret = request_firmware(&ar->testmode.utf, filename, ar->dev); + ret = ath10k_tm_fetch_firmware(ar); if (ret) { - ath10k_warn(ar, "failed to retrieve utf firmware '%s': %d\n", - filename, ret); + ath10k_err(ar, "failed to fetch UTF firmware: %d", ret); goto err; } spin_lock_bh(&ar->data_lock); - ar->testmode.utf_monitor = true; - spin_unlock_bh(&ar->data_lock); - BUILD_BUG_ON(sizeof(ar->fw_features) != sizeof(ar->testmode.orig_fw_features)); memcpy(ar->testmode.orig_fw_features, ar->fw_features, sizeof(ar->fw_features)); ar->testmode.orig_wmi_op_version = ar->wmi.op_version; - - /* utf.bin firmware image does not advertise firmware features. Do - * an ugly hack where we force the firmware features so that wmi.c - * will use the correct WMI interface. - */ memset(ar->fw_features, 0, sizeof(ar->fw_features)); - ar->wmi.op_version = ATH10K_FW_WMI_OP_VERSION_10_1; + + ar->wmi.op_version = ar->testmode.op_version; + + ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode wmi version %d\n", + ar->wmi.op_version); ret = ath10k_hif_power_up(ar); if (ret) { @@ -212,7 +373,12 @@ static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) ar->state = ATH10K_STATE_UTF; - ath10k_info(ar, "UTF firmware started\n"); + if (strlen(ar->testmode.utf_version) > 0) + ver = ar->testmode.utf_version; + else + ver = "API 1"; + + ath10k_info(ar, "UTF firmware %s started\n", ver); mutex_unlock(&ar->conf_mutex); diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index 8f835480a62c..6fbd17b69469 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -23,6 +23,7 @@ #include "wmi-ops.h" #include "wmi-tlv.h" #include "p2p.h" +#include "testmode.h" /***************/ /* TLV helpers */ @@ -419,6 +420,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb) { struct wmi_cmd_hdr *cmd_hdr; enum wmi_tlv_event_id id; + bool consumed; cmd_hdr = (struct wmi_cmd_hdr *)skb->data; id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID); @@ -428,6 +430,18 @@ static void ath10k_wmi_tlv_op_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. + */ + if (consumed && id != WMI_TLV_READY_EVENTID) { + ath10k_dbg(ar, ATH10K_DBG_WMI, + "wmi tlv testmode consumed 0x%x\n", id); + goto out; + } + switch (id) { case WMI_TLV_MGMT_RX_EVENTID: ath10k_wmi_event_mgmt_rx(ar, skb);