* [RFC PATCH 0/2] ath10k: testmode support
@ 2014-05-28 9:19 Kalle Valo
2014-05-28 9:19 ` [RFC PATCH 1/2] ath10k: make ath10k_wmi_cmd_send() public Kalle Valo
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Kalle Valo @ 2014-05-28 9:19 UTC (permalink / raw)
To: ath10k
Hi,
here's how I'm thinking we should implement testmode support to ath10k. Please
comment.
---
Kalle Valo (2):
ath10k: make ath10k_wmi_cmd_send() public
ath10k: add testmode
drivers/net/wireless/ath/ath10k/Makefile | 2
drivers/net/wireless/ath/ath10k/core.c | 66 ++++--
drivers/net/wireless/ath/ath10k/core.h | 17 +-
drivers/net/wireless/ath/ath10k/debug.h | 1
drivers/net/wireless/ath/ath10k/hw.h | 1
drivers/net/wireless/ath/ath10k/mac.c | 10 +
drivers/net/wireless/ath/ath10k/testmode.c | 312 ++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/testmode.h | 45 ++++
drivers/net/wireless/ath/ath10k/wmi.c | 11 +
drivers/net/wireless/ath/ath10k/wmi.h | 4
10 files changed, 446 insertions(+), 23 deletions(-)
create mode 100644 drivers/net/wireless/ath/ath10k/testmode.c
create mode 100644 drivers/net/wireless/ath/ath10k/testmode.h
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 13+ messages in thread* [RFC PATCH 1/2] ath10k: make ath10k_wmi_cmd_send() public 2014-05-28 9:19 [RFC PATCH 0/2] ath10k: testmode support Kalle Valo @ 2014-05-28 9:19 ` Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 2/2] ath10k: add testmode Kalle Valo 2014-06-01 19:51 ` [RFC PATCH 0/2] ath10k: testmode support Sven Schnelle 2 siblings, 0 replies; 13+ messages in thread From: Kalle Valo @ 2014-05-28 9:19 UTC (permalink / raw) To: ath10k We need this function to send wmi packets from testmode.c. Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com> --- drivers/net/wireless/ath/ath10k/wmi.c | 5 ++--- drivers/net/wireless/ath/ath10k/wmi.h | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index 4b7782a529ac..921d18d599fc 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -503,7 +503,7 @@ int ath10k_wmi_wait_for_unified_ready(struct ath10k *ar) return ret; } -static struct sk_buff *ath10k_wmi_alloc_skb(u32 len) +struct sk_buff *ath10k_wmi_alloc_skb(u32 len) { struct sk_buff *skb; u32 round_len = roundup(len, 4); @@ -604,8 +604,7 @@ static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar) wake_up(&ar->wmi.tx_credits_wq); } -static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, - u32 cmd_id) +int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id) { int ret = -EOPNOTSUPP; diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h index 89ef3b3749eb..02376b365098 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.h +++ b/drivers/net/wireless/ath/ath10k/wmi.h @@ -4260,6 +4260,10 @@ int ath10k_wmi_wait_for_service_ready(struct ath10k *ar); int ath10k_wmi_wait_for_unified_ready(struct ath10k *ar); int ath10k_wmi_connect(struct ath10k *ar); + +struct sk_buff *ath10k_wmi_alloc_skb(u32 len); +int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id); + int ath10k_wmi_pdev_set_channel(struct ath10k *ar, const struct wmi_channel_arg *); int ath10k_wmi_pdev_suspend_target(struct ath10k *ar, u32 suspend_opt); _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC PATCH 2/2] ath10k: add testmode 2014-05-28 9:19 [RFC PATCH 0/2] ath10k: testmode support Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 1/2] ath10k: make ath10k_wmi_cmd_send() public Kalle Valo @ 2014-05-28 9:19 ` Kalle Valo 2014-05-29 11:43 ` Michal Kazior 2014-06-01 19:51 ` [RFC PATCH 0/2] ath10k: testmode support Sven Schnelle 2 siblings, 1 reply; 13+ messages in thread From: Kalle Valo @ 2014-05-28 9:19 UTC (permalink / raw) To: ath10k 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 <kvalo@qca.qualcomm.com> --- drivers/net/wireless/ath/ath10k/Makefile | 2 drivers/net/wireless/ath/ath10k/core.c | 66 ++++-- drivers/net/wireless/ath/ath10k/core.h | 17 +- drivers/net/wireless/ath/ath10k/debug.h | 1 drivers/net/wireless/ath/ath10k/hw.h | 1 drivers/net/wireless/ath/ath10k/mac.c | 10 + drivers/net/wireless/ath/ath10k/testmode.c | 312 ++++++++++++++++++++++++++++ drivers/net/wireless/ath/ath10k/testmode.h | 45 ++++ drivers/net/wireless/ath/ath10k/wmi.c | 6 + 9 files changed, 440 insertions(+), 20 deletions(-) create mode 100644 drivers/net/wireless/ath/ath10k/testmode.c create mode 100644 drivers/net/wireless/ath/ath10k/testmode.h diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile index a4179f49ee1f..d7708ee65a09 100644 --- a/drivers/net/wireless/ath/ath10k/Makefile +++ b/drivers/net/wireless/ath/ath10k/Makefile @@ -10,6 +10,8 @@ ath10k_core-y += mac.o \ wmi.o \ bmi.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 82017f56e661..18b6d4d8ca1c 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; @@ -255,17 +256,36 @@ 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 void *data; int ret; address = ar->hw_params.patch_load_addr; - ret = ath10k_bmi_fast_download(ar, address, ar->firmware_data, - ar->firmware_len); + /* to shut up a compiler warning */ + data = NULL; + data_len = 0; + + switch (mode) { + case ATH10K_FIRMWARE_MODE_NORMAL: + data = ar->firmware_data; + data_len = ar->firmware_len; + break; + case ATH10K_FIRMWARE_MODE_UTF: + data = ar->testmode.utf->data; + data_len = ar->testmode.utf->size; + break; + } + + ath10k_dbg(ATH10K_DBG_BOOT, + "boot uploading firmware image %p len %d mode %d\n", + data, data_len, mode); + + ret = ath10k_bmi_fast_download(ar, address, data, data_len); if (ret) { - ath10k_err("could not write fw (%d)\n", ret); + ath10k_err("failed to download firmware: %d\n", ret); goto exit; } @@ -551,7 +571,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; @@ -567,7 +588,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("failed to download firmware: %d\n", ret); return ret; @@ -669,12 +690,15 @@ static void ath10k_core_restart(struct work_struct *work) case ATH10K_STATE_WEDGED: ath10k_warn("device is wedged, will not restart\n"); break; + case ATH10K_STATE_UTF: + ath10k_warn("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; @@ -687,7 +711,7 @@ int ath10k_core_start(struct ath10k *ar) goto err; } - status = ath10k_init_download_firmware(ar); + status = ath10k_init_download_firmware(ar, mode); if (status) goto err; @@ -744,10 +768,12 @@ int ath10k_core_start(struct ath10k *ar) goto err_hif_stop; } - status = ath10k_htt_connect(&ar->htt); - if (status) { - ath10k_err("failed to connect htt (%d)\n", status); - goto err_hif_stop; + if (mode != ATH10K_FIRMWARE_MODE_UTF) { + status = ath10k_htt_connect(&ar->htt); + if (status) { + ath10k_err("failed to connect htt (%d)\n", status); + goto err_hif_stop; + } } status = ath10k_wmi_connect(ar); @@ -785,10 +811,12 @@ int ath10k_core_start(struct ath10k *ar) goto err_htc_stop; } - status = ath10k_htt_setup(&ar->htt); - if (status) { - ath10k_err("failed to setup htt: %d\n", status); - goto err_htc_stop; + if (mode != ATH10K_FIRMWARE_MODE_UTF) { + status = ath10k_htt_setup(&ar->htt); + if (status) { + ath10k_err("failed to setup htt: %d\n", status); + goto err_htc_stop; + } } status = ath10k_debug_start(ar); @@ -908,7 +936,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar) mutex_lock(&ar->conf_mutex); - ret = ath10k_core_start(ar); + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_NORMAL); if (ret) { ath10k_err("could not init core (%d)\n", ret); ath10k_core_free_firmware_files(ar); @@ -1018,6 +1046,8 @@ void ath10k_core_unregister(struct ath10k *ar) * unhappy about callback failures. */ ath10k_mac_unregister(ar); + ath10k_testmode_unregister(ar); + ath10k_core_free_firmware_files(ar); ath10k_debug_destroy(ar); diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index 68ceef61933d..de657af3ccdc 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -312,6 +312,17 @@ enum ath10k_state { * prevents completion timeouts and makes the driver more responsive to * userspace commands. This is also prevents recursive recovery. */ ATH10K_STATE_WEDGED, + + /* factory tests */ + ATH10K_STATE_UTF, +}; + +enum ath10k_firmware_mode { + /* the default mode, standard 802.11 functionality */ + ATH10K_FIRMWARE_MODE_NORMAL, + + /* factory tests etc */ + ATH10K_FIRMWARE_MODE_UTF, }; enum ath10k_fw_features { @@ -491,13 +502,17 @@ struct ath10k { #ifdef CONFIG_ATH10K_DEBUGFS struct ath10k_debug debug; #endif + + struct { + const struct firmware *utf; + } testmode; }; struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev, const struct ath10k_hif_ops *hif_ops); void ath10k_core_destroy(struct ath10k *ar); -int ath10k_core_start(struct ath10k *ar); +int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode); int ath10k_wait_for_suspend(struct ath10k *ar, u32 suspend_opt); void ath10k_core_stop(struct ath10k *ar); int ath10k_core_register(struct ath10k *ar, u32 chip_id); diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h index a5824990bd2a..084176e82166 100644 --- a/drivers/net/wireless/ath/ath10k/debug.h +++ b/drivers/net/wireless/ath/ath10k/debug.h @@ -34,6 +34,7 @@ enum ath10k_debug_mask { ATH10K_DBG_DATA = 0x00000200, ATH10K_DBG_BMI = 0x00000400, ATH10K_DBG_REGULATORY = 0x00000800, + ATH10K_DBG_TESTMODE = 0x00001000, ATH10K_DBG_ANY = 0xffffffff, }; diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h index 007e855f4ba9..07a79761cd82 100644 --- a/drivers/net/wireless/ath/ath10k/hw.h +++ b/drivers/net/wireless/ath/ath10k/hw.h @@ -34,6 +34,7 @@ #define QCA988X_HW_2_0_PATCH_LOAD_ADDR 0x1234 #define ATH10K_FW_API2_FILE "firmware-2.bin" +#define ATH10K_FW_UTF_FILE "utf.bin" /* includes also the null byte */ #define ATH10K_FIRMWARE_MAGIC "QCA-ATH10K" diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index a21080028c54..c376152f213b 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -26,6 +26,7 @@ #include "wmi.h" #include "htt.h" #include "txrx.h" +#include "testmode.h" /**********/ /* Crypto */ @@ -2431,8 +2432,12 @@ static int ath10k_start(struct ieee80211_hw *hw) case ATH10K_STATE_RESTARTED: case ATH10K_STATE_WEDGED: WARN_ON(1); + /* fall through */ ret = -EINVAL; goto err; + case ATH10K_STATE_UTF: + ret = -EBUSY; + goto err; } ret = ath10k_hif_power_up(ar); @@ -2441,7 +2446,7 @@ static int ath10k_start(struct ieee80211_hw *hw) goto err_off; } - ret = ath10k_core_start(ar); + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_NORMAL); if (ret) { ath10k_err("Could not init core: %d\n", ret); goto err_power_down; @@ -4357,6 +4362,9 @@ static const struct ieee80211_ops ath10k_ops = { .set_bitrate_mask = ath10k_set_bitrate_mask, .sta_rc_update = ath10k_sta_rc_update, .get_tsf = ath10k_get_tsf, + + CFG80211_TESTMODE_CMD(ath10k_tm_cmd) + #ifdef CONFIG_PM .suspend = ath10k_suspend, .resume = ath10k_resume, diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c new file mode 100644 index 000000000000..1bef042d3d5a --- /dev/null +++ b/drivers/net/wireless/ath/ath10k/testmode.c @@ -0,0 +1,312 @@ +/* + * Copyright (c) 2014 Qualcomm Atheros, Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include "testmode.h" + +#include <net/netlink.h> +#include <linux/firmware.h> + +#include "debug.h" +#include "wmi.h" +#include "hif.h" +#include "hw.h" + +/* "API" level of the ath10k testmode interface. Bump it after every + * interface change. */ +#define ATH10K_TESTMODE_VERSION 1 + +#define ATH10K_TM_DATA_MAX_LEN 5000 + +enum ath10k_tm_attr { + __ATH10K_TM_ATTR_INVALID = 0, + ATH10K_TM_ATTR_CMD = 1, + ATH10K_TM_ATTR_DATA = 2, + ATH10K_TM_ATTR_WMI_CMDID = 3, + ATH10K_TM_ATTR_VERSION = 4, + + /* keep last */ + __ATH10K_TM_ATTR_AFTER_LAST, + ATH10K_TM_ATTR_MAX = __ATH10K_TM_ATTR_AFTER_LAST - 1, +}; + +enum ath10k_tm_cmd { + /* Returns the supported ath10k testmode interface version, always + * guaranteed to work. User space uses this to verify it's using + * the correct version of the testmode interface */ + ATH10K_TM_CMD_GET_VERSION = 0, + + /* boots the utf firmware, interface must be down */ + ATH10K_TM_CMD_UTF_START = 1, + + /* shuts down the utf firmware, interface must be down */ + ATH10K_TM_CMD_UTF_STOP = 2, + + /* transmits a wmi command to the firmware */ + ATH10K_TM_CMD_WMI = 3, + + /* emits a wmi event from firmware to the user space */ + ATH10K_TM_CMD_EVENT_WMI = 4, +}; + +static const struct nla_policy ath10k_tm_policy[ATH10K_TM_ATTR_MAX + 1] = { + [ATH10K_TM_ATTR_CMD] = { .type = NLA_U32 }, + [ATH10K_TM_ATTR_DATA] = { .type = NLA_BINARY, + .len = ATH10K_TM_DATA_MAX_LEN }, + [ATH10K_TM_ATTR_WMI_CMDID] = { .type = NLA_U32 }, + [ATH10K_TM_ATTR_VERSION] = { .type = NLA_U32 }, +}; + +/* skb owned by the caller, must not sleep */ +void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb) +{ + struct sk_buff *nl_skb; + int ret; + + ath10k_dbg(ATH10K_DBG_TESTMODE, + "testmode event wmi cmd_id %d skb %p skb->len %d\n", + cmd_id, skb, skb->len); + + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", skb->data, skb->len); + + /* FIXME: locking! */ + + if (ar->state != ATH10K_STATE_UTF) + return; + + nl_skb = cfg80211_testmode_alloc_event_skb(ar->hw->wiphy, + 2 * sizeof(u32) + skb->len, + GFP_ATOMIC); + if (!nl_skb) { + ath10k_warn("failed to allocate skb for testmode wmi event\n"); + return; + } + + ret = nla_put_u32(nl_skb, ATH10K_TM_ATTR_CMD, ATH10K_TM_CMD_EVENT_WMI); + if (ret) { + ath10k_warn("failed to to put testmode wmi event cmd attribute: %d\n", + ret); + kfree_skb(nl_skb); + return; + } + + ret = nla_put_u32(nl_skb, ATH10K_TM_ATTR_WMI_CMDID, cmd_id); + if (ret) { + ath10k_warn("failed to to put testmode wmi even cmd_id: %d\n", + ret); + kfree_skb(nl_skb); + return; + } + + ret = nla_put(nl_skb, ATH10K_TM_ATTR_DATA, skb->len, skb->data); + if (ret) { + ath10k_warn("failed to copy skb to testmode wmi event: %d\n", + ret); + kfree_skb(nl_skb); + return; + } + + cfg80211_testmode_event(nl_skb, GFP_ATOMIC); + + return; +} + +static int ath10k_tm_cmd_get_version(struct ath10k *ar, struct nlattr *tb[]) +{ + struct sk_buff *skb; + int ret; + + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd get version %d\n", + ATH10K_TESTMODE_VERSION); + + skb = cfg80211_testmode_alloc_reply_skb(ar->hw->wiphy, + nla_total_size(sizeof(u32))); + if (!skb) + return -ENOMEM; + + ret = nla_put_u32(skb, ATH10K_TM_ATTR_VERSION, + ATH10K_TESTMODE_VERSION); + if (ret) { + kfree_skb(skb); + return ret; + } + + return cfg80211_testmode_reply(skb); +} + +static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) +{ + char filename[100]; + int ret; + + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd utf start\n"); + + mutex_lock(&ar->conf_mutex); + + if (ar->state == ATH10K_STATE_UTF) { + ret = -EALREADY; + goto out; + } + + /* start utf only when the driver is not in use */ + if (ar->state != ATH10K_STATE_OFF) { + ret = -EBUSY; + goto out; + } + + if (ar->testmode.utf != NULL) + /* utf is already loaded */ + goto power_up; + + + snprintf(filename, sizeof(filename), "%s/%s", + ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); + + /* load utf image now and release only when the device is removed */ + ret = request_firmware(&ar->testmode.utf, filename, ar->dev); + if (ret) { + ath10k_warn("failed to retrieve utf firmware '%s': %d\n", + filename, ret); + goto out; + } + +power_up: + ret = ath10k_hif_power_up(ar); + if (ret) { + ath10k_err("failed to power up hif (testmode): %d\n", ret); + ar->state = ATH10K_STATE_OFF; + goto out; + } + + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_UTF); + if (ret) { + ath10k_err("failed to start core (testmode): %d\n", ret); + ath10k_hif_power_down(ar); + ar->state = ATH10K_STATE_OFF; + goto out; + } + + ar->state = ATH10K_STATE_UTF; + ret = 0; + +out: + mutex_unlock(&ar->conf_mutex); + return ret; +} + +static int ath10k_tm_cmd_utf_stop(struct ath10k *ar, struct nlattr *tb[]) +{ + int ret; + + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd utf stop\n"); + + mutex_lock(&ar->conf_mutex); + + if (ar->state != ATH10K_STATE_UTF) { + ret = -ENETDOWN; + goto out; + } + + ath10k_core_stop(ar); + ath10k_hif_power_down(ar); + + ar->state = ATH10K_STATE_OFF; + ret = 0; + +out: + mutex_unlock(&ar->conf_mutex); + return ret; +} + +static int ath10k_tm_cmd_wmi(struct ath10k *ar, struct nlattr *tb[]) +{ + struct sk_buff *skb; + int ret, buf_len; + u32 cmd_id; + void *buf; + + if (!tb[ATH10K_TM_ATTR_DATA]) + return -EINVAL; + + if (!tb[ATH10K_TM_ATTR_WMI_CMDID]) + return -EINVAL; + + buf = nla_data(tb[ATH10K_TM_ATTR_DATA]); + buf_len = nla_len(tb[ATH10K_TM_ATTR_DATA]); + cmd_id = nla_get_u32(tb[ATH10K_TM_ATTR_WMI_CMDID]); + + ath10k_dbg(ATH10K_DBG_TESTMODE, + "testmode cmd wmi cmd_id %d buf %p buf_len %d\n", + cmd_id, buf, buf_len); + + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", buf, buf_len); + + skb = ath10k_wmi_alloc_skb(buf_len); + + memcpy(skb->data, buf, buf_len); + + ret = ath10k_wmi_cmd_send(ar, skb, cmd_id); + if (ret) { + ath10k_warn("failed to transmit wmi command (testmode): %d\n", + ret); + return ret; + } + + return 0; +} + +int ath10k_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif, + void *data, int len) +{ + struct ath10k *ar = hw->priv; + struct nlattr *tb[ATH10K_TM_ATTR_MAX + 1]; + int err; + + err = nla_parse(tb, ATH10K_TM_ATTR_MAX, data, len, + ath10k_tm_policy); + if (err) + return err; + + if (!tb[ATH10K_TM_ATTR_CMD]) + return -EINVAL; + + switch (nla_get_u32(tb[ATH10K_TM_ATTR_CMD])) { + case ATH10K_TM_CMD_GET_VERSION: + return ath10k_tm_cmd_get_version(ar, tb); + case ATH10K_TM_CMD_UTF_START: + return ath10k_tm_cmd_utf_start(ar, tb); + case ATH10K_TM_CMD_UTF_STOP: + return ath10k_tm_cmd_utf_stop(ar, tb); + case ATH10K_TM_CMD_WMI: + return ath10k_tm_cmd_wmi(ar, tb); + default: + return -EOPNOTSUPP; + } +} + +void ath10k_testmode_unregister(struct ath10k *ar) +{ + /* FIXME: locking! */ + + release_firmware(ar->testmode.utf); + + if (ar->state != ATH10K_STATE_UTF) + /* utf firmware is not running */ + return; + + ath10k_htc_stop(&ar->htc); + ath10k_wmi_detach(ar); + ath10k_hif_power_down(ar); +} diff --git a/drivers/net/wireless/ath/ath10k/testmode.h b/drivers/net/wireless/ath/ath10k/testmode.h new file mode 100644 index 000000000000..fc35c7e6f066 --- /dev/null +++ b/drivers/net/wireless/ath/ath10k/testmode.h @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2014 Qualcomm Atheros, Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include "core.h" + +#ifdef CONFIG_NL80211_TESTMODE + +void ath10k_testmode_unregister(struct ath10k *ar); + +void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb); +int ath10k_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif, + void *data, int len); + +#else + +static inline void ath10k_testmode_unregister(struct ath10k *ar) +{ +} + +static inline void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, + struct sk_buff *skb) +{ +} + +static inline int ath10k_tm_cmd(struct ieee80211_hw *hw, + struct ieee80211_vif *vif, + void *data, int len) +{ + return 0; +} + +#endif diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index 921d18d599fc..204c0e293b8e 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -23,6 +23,7 @@ #include "debug.h" #include "wmi.h" #include "mac.h" +#include "testmode.h" /* MAIN WMI cmd track */ static struct wmi_cmd_map wmi_cmd_map = { @@ -2235,6 +2236,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) len = skb->len; trace_ath10k_wmi_event(id, skb->data, skb->len); + ath10k_tm_event_wmi(ar, id, skb); switch (id) { case WMI_10X_MGMT_RX_EVENTID: @@ -2322,6 +2324,9 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) case WMI_10X_READY_EVENTID: ath10k_wmi_ready_event_rx(ar, skb); break; + case WMI_10X_PDEV_UTF_EVENTID: + /* ignore utf events */ + break; default: ath10k_warn("Unknown eventid: %d\n", id); break; @@ -2333,6 +2338,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb) { + /* FIXME: somehow force ot use 10x with UTF */ if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) ath10k_wmi_10x_process_rx(ar, skb); else _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-28 9:19 ` [RFC PATCH 2/2] ath10k: add testmode Kalle Valo @ 2014-05-29 11:43 ` Michal Kazior 2014-05-29 12:51 ` Kalle Valo 0 siblings, 1 reply; 13+ messages in thread From: Michal Kazior @ 2014-05-29 11:43 UTC (permalink / raw) To: Kalle Valo; +Cc: ath10k@lists.infradead.org On 28 May 2014 11:19, Kalle Valo <kvalo@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 <kvalo@qca.qualcomm.com> > --- > drivers/net/wireless/ath/ath10k/Makefile | 2 > drivers/net/wireless/ath/ath10k/core.c | 66 ++++-- > drivers/net/wireless/ath/ath10k/core.h | 17 +- > drivers/net/wireless/ath/ath10k/debug.h | 1 > drivers/net/wireless/ath/ath10k/hw.h | 1 > drivers/net/wireless/ath/ath10k/mac.c | 10 + > drivers/net/wireless/ath/ath10k/testmode.c | 312 ++++++++++++++++++++++++++++ > drivers/net/wireless/ath/ath10k/testmode.h | 45 ++++ > drivers/net/wireless/ath/ath10k/wmi.c | 6 + > 9 files changed, 440 insertions(+), 20 deletions(-) > create mode 100644 drivers/net/wireless/ath/ath10k/testmode.c > create mode 100644 drivers/net/wireless/ath/ath10k/testmode.h > > diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile > index a4179f49ee1f..d7708ee65a09 100644 > --- a/drivers/net/wireless/ath/ath10k/Makefile > +++ b/drivers/net/wireless/ath/ath10k/Makefile > @@ -10,6 +10,8 @@ ath10k_core-y += mac.o \ > wmi.o \ > bmi.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 82017f56e661..18b6d4d8ca1c 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; > @@ -255,17 +256,36 @@ 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 void *data; > int ret; > > address = ar->hw_params.patch_load_addr; > > - ret = ath10k_bmi_fast_download(ar, address, ar->firmware_data, > - ar->firmware_len); > + /* to shut up a compiler warning */ > + data = NULL; > + data_len = 0; > + > + switch (mode) { > + case ATH10K_FIRMWARE_MODE_NORMAL: > + data = ar->firmware_data; > + data_len = ar->firmware_len; > + break; > + case ATH10K_FIRMWARE_MODE_UTF: > + data = ar->testmode.utf->data; > + data_len = ar->testmode.utf->size; > + break; > + } > + > + ath10k_dbg(ATH10K_DBG_BOOT, > + "boot uploading firmware image %p len %d mode %d\n", > + data, data_len, mode); > + > + ret = ath10k_bmi_fast_download(ar, address, data, data_len); > if (ret) { > - ath10k_err("could not write fw (%d)\n", ret); > + ath10k_err("failed to download firmware: %d\n", ret); > goto exit; > } > > @@ -551,7 +571,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; > > @@ -567,7 +588,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("failed to download firmware: %d\n", ret); > return ret; > @@ -669,12 +690,15 @@ static void ath10k_core_restart(struct work_struct *work) > case ATH10K_STATE_WEDGED: > ath10k_warn("device is wedged, will not restart\n"); > break; > + case ATH10K_STATE_UTF: > + ath10k_warn("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; > > @@ -687,7 +711,7 @@ int ath10k_core_start(struct ath10k *ar) > goto err; > } > > - status = ath10k_init_download_firmware(ar); > + status = ath10k_init_download_firmware(ar, mode); > if (status) > goto err; > > @@ -744,10 +768,12 @@ int ath10k_core_start(struct ath10k *ar) > goto err_hif_stop; > } > > - status = ath10k_htt_connect(&ar->htt); > - if (status) { > - ath10k_err("failed to connect htt (%d)\n", status); > - goto err_hif_stop; > + if (mode != ATH10K_FIRMWARE_MODE_UTF) { > + status = ath10k_htt_connect(&ar->htt); > + if (status) { > + ath10k_err("failed to connect htt (%d)\n", status); > + goto err_hif_stop; > + } > } > > status = ath10k_wmi_connect(ar); > @@ -785,10 +811,12 @@ int ath10k_core_start(struct ath10k *ar) > goto err_htc_stop; > } > > - status = ath10k_htt_setup(&ar->htt); > - if (status) { > - ath10k_err("failed to setup htt: %d\n", status); > - goto err_htc_stop; > + if (mode != ATH10K_FIRMWARE_MODE_UTF) { > + status = ath10k_htt_setup(&ar->htt); > + if (status) { > + ath10k_err("failed to setup htt: %d\n", status); > + goto err_htc_stop; > + } > } > > status = ath10k_debug_start(ar); > @@ -908,7 +936,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar) > > mutex_lock(&ar->conf_mutex); > > - ret = ath10k_core_start(ar); > + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_NORMAL); > if (ret) { > ath10k_err("could not init core (%d)\n", ret); > ath10k_core_free_firmware_files(ar); > @@ -1018,6 +1046,8 @@ void ath10k_core_unregister(struct ath10k *ar) > * unhappy about callback failures. */ > ath10k_mac_unregister(ar); > > + ath10k_testmode_unregister(ar); > + > ath10k_core_free_firmware_files(ar); > > ath10k_debug_destroy(ar); > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h > index 68ceef61933d..de657af3ccdc 100644 > --- a/drivers/net/wireless/ath/ath10k/core.h > +++ b/drivers/net/wireless/ath/ath10k/core.h > @@ -312,6 +312,17 @@ enum ath10k_state { > * prevents completion timeouts and makes the driver more responsive to > * userspace commands. This is also prevents recursive recovery. */ > ATH10K_STATE_WEDGED, > + > + /* factory tests */ > + ATH10K_STATE_UTF, > +}; > + > +enum ath10k_firmware_mode { > + /* the default mode, standard 802.11 functionality */ > + ATH10K_FIRMWARE_MODE_NORMAL, > + > + /* factory tests etc */ > + ATH10K_FIRMWARE_MODE_UTF, > }; > > enum ath10k_fw_features { > @@ -491,13 +502,17 @@ struct ath10k { > #ifdef CONFIG_ATH10K_DEBUGFS > struct ath10k_debug debug; > #endif > + > + struct { > + const struct firmware *utf; > + } testmode; > }; > > struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev, > const struct ath10k_hif_ops *hif_ops); > void ath10k_core_destroy(struct ath10k *ar); > > -int ath10k_core_start(struct ath10k *ar); > +int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode); > int ath10k_wait_for_suspend(struct ath10k *ar, u32 suspend_opt); > void ath10k_core_stop(struct ath10k *ar); > int ath10k_core_register(struct ath10k *ar, u32 chip_id); > diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h > index a5824990bd2a..084176e82166 100644 > --- a/drivers/net/wireless/ath/ath10k/debug.h > +++ b/drivers/net/wireless/ath/ath10k/debug.h > @@ -34,6 +34,7 @@ enum ath10k_debug_mask { > ATH10K_DBG_DATA = 0x00000200, > ATH10K_DBG_BMI = 0x00000400, > ATH10K_DBG_REGULATORY = 0x00000800, > + ATH10K_DBG_TESTMODE = 0x00001000, > ATH10K_DBG_ANY = 0xffffffff, > }; > > diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h > index 007e855f4ba9..07a79761cd82 100644 > --- a/drivers/net/wireless/ath/ath10k/hw.h > +++ b/drivers/net/wireless/ath/ath10k/hw.h > @@ -34,6 +34,7 @@ > #define QCA988X_HW_2_0_PATCH_LOAD_ADDR 0x1234 > > #define ATH10K_FW_API2_FILE "firmware-2.bin" > +#define ATH10K_FW_UTF_FILE "utf.bin" > > /* includes also the null byte */ > #define ATH10K_FIRMWARE_MAGIC "QCA-ATH10K" > diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c > index a21080028c54..c376152f213b 100644 > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -26,6 +26,7 @@ > #include "wmi.h" > #include "htt.h" > #include "txrx.h" > +#include "testmode.h" > > /**********/ > /* Crypto */ > @@ -2431,8 +2432,12 @@ static int ath10k_start(struct ieee80211_hw *hw) > case ATH10K_STATE_RESTARTED: > case ATH10K_STATE_WEDGED: > WARN_ON(1); > + /* fall through */ > ret = -EINVAL; > goto err; > + case ATH10K_STATE_UTF: > + ret = -EBUSY; > + goto err; > } > > ret = ath10k_hif_power_up(ar); > @@ -2441,7 +2446,7 @@ static int ath10k_start(struct ieee80211_hw *hw) > goto err_off; > } > > - ret = ath10k_core_start(ar); > + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_NORMAL); > if (ret) { > ath10k_err("Could not init core: %d\n", ret); > goto err_power_down; > @@ -4357,6 +4362,9 @@ static const struct ieee80211_ops ath10k_ops = { > .set_bitrate_mask = ath10k_set_bitrate_mask, > .sta_rc_update = ath10k_sta_rc_update, > .get_tsf = ath10k_get_tsf, > + > + CFG80211_TESTMODE_CMD(ath10k_tm_cmd) > + > #ifdef CONFIG_PM > .suspend = ath10k_suspend, > .resume = ath10k_resume, > diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c > new file mode 100644 > index 000000000000..1bef042d3d5a > --- /dev/null > +++ b/drivers/net/wireless/ath/ath10k/testmode.c > @@ -0,0 +1,312 @@ > +/* > + * Copyright (c) 2014 Qualcomm Atheros, Inc. > + * > + * Permission to use, copy, modify, and/or distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ > + > +#include "testmode.h" > + > +#include <net/netlink.h> > +#include <linux/firmware.h> > + > +#include "debug.h" > +#include "wmi.h" > +#include "hif.h" > +#include "hw.h" > + > +/* "API" level of the ath10k testmode interface. Bump it after every > + * interface change. */ > +#define ATH10K_TESTMODE_VERSION 1 > + > +#define ATH10K_TM_DATA_MAX_LEN 5000 > + > +enum ath10k_tm_attr { > + __ATH10K_TM_ATTR_INVALID = 0, > + ATH10K_TM_ATTR_CMD = 1, > + ATH10K_TM_ATTR_DATA = 2, > + ATH10K_TM_ATTR_WMI_CMDID = 3, > + ATH10K_TM_ATTR_VERSION = 4, > + > + /* keep last */ > + __ATH10K_TM_ATTR_AFTER_LAST, > + ATH10K_TM_ATTR_MAX = __ATH10K_TM_ATTR_AFTER_LAST - 1, > +}; > + > +enum ath10k_tm_cmd { > + /* Returns the supported ath10k testmode interface version, always > + * guaranteed to work. User space uses this to verify it's using > + * the correct version of the testmode interface */ > + ATH10K_TM_CMD_GET_VERSION = 0, > + > + /* boots the utf firmware, interface must be down */ > + ATH10K_TM_CMD_UTF_START = 1, > + > + /* shuts down the utf firmware, interface must be down */ > + ATH10K_TM_CMD_UTF_STOP = 2, > + > + /* transmits a wmi command to the firmware */ > + ATH10K_TM_CMD_WMI = 3, > + > + /* emits a wmi event from firmware to the user space */ > + ATH10K_TM_CMD_EVENT_WMI = 4, > +}; > + > +static const struct nla_policy ath10k_tm_policy[ATH10K_TM_ATTR_MAX + 1] = { > + [ATH10K_TM_ATTR_CMD] = { .type = NLA_U32 }, > + [ATH10K_TM_ATTR_DATA] = { .type = NLA_BINARY, > + .len = ATH10K_TM_DATA_MAX_LEN }, > + [ATH10K_TM_ATTR_WMI_CMDID] = { .type = NLA_U32 }, > + [ATH10K_TM_ATTR_VERSION] = { .type = NLA_U32 }, > +}; > + > +/* skb owned by the caller, must not sleep */ > +void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb) > +{ > + struct sk_buff *nl_skb; > + int ret; > + > + ath10k_dbg(ATH10K_DBG_TESTMODE, > + "testmode event wmi cmd_id %d skb %p skb->len %d\n", > + cmd_id, skb, skb->len); > + > + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", skb->data, skb->len); > + > + /* FIXME: locking! */ > + > + if (ar->state != ATH10K_STATE_UTF) > + return; You shouldn't really be depending on the value of ar->state without conf_mutex being held. Since this is tasklet context you can't just grab the mutex here. The simplest way is to probably to move this into a worker and a have sk_buff_head. Another way is to rework the state system to be async (it might be easier to do now after my recent hw restart changes) and have ar->state be protected only by ar->data_lock. This might be tricky though. I'll take a look at it.. > + > + nl_skb = cfg80211_testmode_alloc_event_skb(ar->hw->wiphy, > + 2 * sizeof(u32) + skb->len, > + GFP_ATOMIC); > + if (!nl_skb) { > + ath10k_warn("failed to allocate skb for testmode wmi event\n"); > + return; > + } > + > + ret = nla_put_u32(nl_skb, ATH10K_TM_ATTR_CMD, ATH10K_TM_CMD_EVENT_WMI); > + if (ret) { > + ath10k_warn("failed to to put testmode wmi event cmd attribute: %d\n", > + ret); > + kfree_skb(nl_skb); > + return; > + } > + > + ret = nla_put_u32(nl_skb, ATH10K_TM_ATTR_WMI_CMDID, cmd_id); > + if (ret) { > + ath10k_warn("failed to to put testmode wmi even cmd_id: %d\n", > + ret); > + kfree_skb(nl_skb); > + return; > + } > + > + ret = nla_put(nl_skb, ATH10K_TM_ATTR_DATA, skb->len, skb->data); > + if (ret) { > + ath10k_warn("failed to copy skb to testmode wmi event: %d\n", > + ret); > + kfree_skb(nl_skb); > + return; > + } > + > + cfg80211_testmode_event(nl_skb, GFP_ATOMIC); > + > + return; > +} > + > +static int ath10k_tm_cmd_get_version(struct ath10k *ar, struct nlattr *tb[]) > +{ > + struct sk_buff *skb; > + int ret; > + > + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd get version %d\n", > + ATH10K_TESTMODE_VERSION); > + > + skb = cfg80211_testmode_alloc_reply_skb(ar->hw->wiphy, > + nla_total_size(sizeof(u32))); > + if (!skb) > + return -ENOMEM; > + > + ret = nla_put_u32(skb, ATH10K_TM_ATTR_VERSION, > + ATH10K_TESTMODE_VERSION); > + if (ret) { > + kfree_skb(skb); > + return ret; > + } > + > + return cfg80211_testmode_reply(skb); > +} > + > +static int ath10k_tm_cmd_utf_start(struct ath10k *ar, struct nlattr *tb[]) > +{ > + char filename[100]; > + int ret; > + > + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd utf start\n"); > + > + mutex_lock(&ar->conf_mutex); > + > + if (ar->state == ATH10K_STATE_UTF) { > + ret = -EALREADY; > + goto out; > + } > + > + /* start utf only when the driver is not in use */ > + if (ar->state != ATH10K_STATE_OFF) { > + ret = -EBUSY; > + goto out; > + } > + > + if (ar->testmode.utf != NULL) > + /* utf is already loaded */ > + goto power_up; > + > + > + snprintf(filename, sizeof(filename), "%s/%s", > + ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); 1 extraneous empty line. > + > + /* load utf image now and release only when the device is removed */ > + ret = request_firmware(&ar->testmode.utf, filename, ar->dev); > + if (ret) { > + ath10k_warn("failed to retrieve utf firmware '%s': %d\n", > + filename, ret); > + goto out; > + } > + > +power_up: > + ret = ath10k_hif_power_up(ar); > + if (ret) { > + ath10k_err("failed to power up hif (testmode): %d\n", ret); > + ar->state = ATH10K_STATE_OFF; > + goto out; > + } > + > + ret = ath10k_core_start(ar, ATH10K_FIRMWARE_MODE_UTF); > + if (ret) { > + ath10k_err("failed to start core (testmode): %d\n", ret); > + ath10k_hif_power_down(ar); > + ar->state = ATH10K_STATE_OFF; > + goto out; > + } > + > + ar->state = ATH10K_STATE_UTF; > + ret = 0; > + > +out: > + mutex_unlock(&ar->conf_mutex); > + return ret; > +} > + > +static int ath10k_tm_cmd_utf_stop(struct ath10k *ar, struct nlattr *tb[]) > +{ > + int ret; > + > + ath10k_dbg(ATH10K_DBG_TESTMODE, "testmode cmd utf stop\n"); > + > + mutex_lock(&ar->conf_mutex); > + > + if (ar->state != ATH10K_STATE_UTF) { > + ret = -ENETDOWN; > + goto out; > + } > + > + ath10k_core_stop(ar); > + ath10k_hif_power_down(ar); > + > + ar->state = ATH10K_STATE_OFF; > + ret = 0; > + > +out: > + mutex_unlock(&ar->conf_mutex); > + return ret; > +} > + > +static int ath10k_tm_cmd_wmi(struct ath10k *ar, struct nlattr *tb[]) > +{ > + struct sk_buff *skb; > + int ret, buf_len; > + u32 cmd_id; > + void *buf; > + > + if (!tb[ATH10K_TM_ATTR_DATA]) > + return -EINVAL; > + > + if (!tb[ATH10K_TM_ATTR_WMI_CMDID]) > + return -EINVAL; > + > + buf = nla_data(tb[ATH10K_TM_ATTR_DATA]); > + buf_len = nla_len(tb[ATH10K_TM_ATTR_DATA]); > + cmd_id = nla_get_u32(tb[ATH10K_TM_ATTR_WMI_CMDID]); > + > + ath10k_dbg(ATH10K_DBG_TESTMODE, > + "testmode cmd wmi cmd_id %d buf %p buf_len %d\n", > + cmd_id, buf, buf_len); > + > + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", buf, buf_len); > + > + skb = ath10k_wmi_alloc_skb(buf_len); if (!skb) return -ENOMEM; ? > + > + memcpy(skb->data, buf, buf_len); > + > + ret = ath10k_wmi_cmd_send(ar, skb, cmd_id); > + if (ret) { > + ath10k_warn("failed to transmit wmi command (testmode): %d\n", > + ret); > + return ret; > + } > + > + return 0; > +} > + > +int ath10k_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif, > + void *data, int len) > +{ > + struct ath10k *ar = hw->priv; > + struct nlattr *tb[ATH10K_TM_ATTR_MAX + 1]; > + int err; > + > + err = nla_parse(tb, ATH10K_TM_ATTR_MAX, data, len, > + ath10k_tm_policy); > + if (err) > + return err; s/err/ret/ ? > + > + if (!tb[ATH10K_TM_ATTR_CMD]) > + return -EINVAL; > + > + switch (nla_get_u32(tb[ATH10K_TM_ATTR_CMD])) { > + case ATH10K_TM_CMD_GET_VERSION: > + return ath10k_tm_cmd_get_version(ar, tb); > + case ATH10K_TM_CMD_UTF_START: > + return ath10k_tm_cmd_utf_start(ar, tb); > + case ATH10K_TM_CMD_UTF_STOP: > + return ath10k_tm_cmd_utf_stop(ar, tb); > + case ATH10K_TM_CMD_WMI: > + return ath10k_tm_cmd_wmi(ar, tb); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +void ath10k_testmode_unregister(struct ath10k *ar) > +{ > + /* FIXME: locking! */ I think this should be protected by conf_mutex. > + > + release_firmware(ar->testmode.utf); It might be a good idea to ar->testmode.utf = NULL ? > + > + if (ar->state != ATH10K_STATE_UTF) > + /* utf firmware is not running */ > + return; > + > + ath10k_htc_stop(&ar->htc); > + ath10k_wmi_detach(ar); Why not ath10k_core_stop() ? > + ath10k_hif_power_down(ar); > +} > /* MAIN WMI cmd track */ > static struct wmi_cmd_map wmi_cmd_map = { > @@ -2235,6 +2236,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) > len = skb->len; > > trace_ath10k_wmi_event(id, skb->data, skb->len); > + ath10k_tm_event_wmi(ar, id, skb); > > switch (id) { > case WMI_10X_MGMT_RX_EVENTID: > @@ -2322,6 +2324,9 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) > case WMI_10X_READY_EVENTID: > ath10k_wmi_ready_event_rx(ar, skb); > break; > + case WMI_10X_PDEV_UTF_EVENTID: > + /* ignore utf events */ > + break; > default: > ath10k_warn("Unknown eventid: %d\n", id); > break; > @@ -2333,6 +2338,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) > > static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb) > { > + /* FIXME: somehow force ot use 10x with UTF */ `ot`: typo? I'm a little confused how this works. When UTF firmware is loaded does it generate regular WMI events? Or does it generate UTF_EVENTID only? Is there a particular reason for ath10k_tm_event_wmi() to be called before the switch() and have it no effect on processing of the event later whatsoever? Maybe we don't need to check for ar->state==UTF at all? Michał _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-29 11:43 ` Michal Kazior @ 2014-05-29 12:51 ` Kalle Valo 2014-05-30 6:17 ` Michal Kazior 0 siblings, 1 reply; 13+ messages in thread From: Kalle Valo @ 2014-05-29 12:51 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k@lists.infradead.org Hi Michal, BTW, please edit your quotes. It's pain to find your comments from a 700 line monster :) Michal Kazior <michal.kazior@tieto.com> writes: > On 28 May 2014 11:19, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> +/* skb owned by the caller, must not sleep */ >> +void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb) >> +{ >> + struct sk_buff *nl_skb; >> + int ret; >> + >> + ath10k_dbg(ATH10K_DBG_TESTMODE, >> + "testmode event wmi cmd_id %d skb %p skb->len %d\n", >> + cmd_id, skb, skb->len); >> + >> + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", skb->data, skb->len); >> + >> + /* FIXME: locking! */ >> + >> + if (ar->state != ATH10K_STATE_UTF) >> + return; > > You shouldn't really be depending on the value of ar->state without > conf_mutex being held. > > Since this is tasklet context you can't just grab the mutex here. > > The simplest way is to probably to move this into a worker and a have > sk_buff_head. Yeah, this is what I was planning to do. > Another way is to rework the state system to be async (it might be > easier to do now after my recent hw restart changes) and have > ar->state be protected only by ar->data_lock. This might be tricky > though. I'll take a look at it.. Please don't waste time on that, unless there are other advantages. This testmode interface is no way performance critical and workqueue is more than adequate. >> + if (ar->testmode.utf != NULL) >> + /* utf is already loaded */ >> + goto power_up; >> + >> + >> + snprintf(filename, sizeof(filename), "%s/%s", >> + ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); > > 1 extraneous empty line. Will fix. >> + ath10k_dbg(ATH10K_DBG_TESTMODE, >> + "testmode cmd wmi cmd_id %d buf %p buf_len %d\n", >> + cmd_id, buf, buf_len); >> + >> + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", buf, buf_len); >> + >> + skb = ath10k_wmi_alloc_skb(buf_len); > > if (!skb) return -ENOMEM; ? Yup. >> +int ath10k_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif, >> + void *data, int len) >> +{ >> + struct ath10k *ar = hw->priv; >> + struct nlattr *tb[ATH10K_TM_ATTR_MAX + 1]; >> + int err; >> + >> + err = nla_parse(tb, ATH10K_TM_ATTR_MAX, data, len, >> + ath10k_tm_policy); >> + if (err) >> + return err; > > s/err/ret/ ? Yes, I'll change that. >> +void ath10k_testmode_unregister(struct ath10k *ar) >> +{ >> + /* FIXME: locking! */ > > I think this should be protected by conf_mutex. Yeah, that's why I have the FIXME :) >> + >> + release_firmware(ar->testmode.utf); > > It might be a good idea to ar->testmode.utf = NULL ? I agree. >> + if (ar->state != ATH10K_STATE_UTF) >> + /* utf firmware is not running */ >> + return; >> + >> + ath10k_htc_stop(&ar->htc); >> + ath10k_wmi_detach(ar); > > Why not ath10k_core_stop() ? Doh, I can't remember anymore. Maybe because of the htt calls? But I agree, we should call core_stop() for symmetry. I'll investigate. >> @@ -2333,6 +2338,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) >> >> static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb) >> { >> + /* FIXME: somehow force ot use 10x with UTF */ > > `ot`: typo? Yeah. And the FIXME will be gone in the final version, this was just an RFC to get early comments on the design. > I'm a little confused how this works. When UTF firmware is loaded does > it generate regular WMI events? Or does it generate UTF_EVENTID only? Only UTF_EVENTID, at least for now. > Is there a particular reason for ath10k_tm_event_wmi() to be called > before the switch() and have it no effect on processing of the event > later whatsoever? I want to send the full WMI packet (instead of UTF_EVENTID) to user space to make it easier to extend this later, in case there will be a need for that in the future. For example, there can be new event ids and whatnot. The less we need to change the user space interface the easier it will be. > Maybe we don't need to check for ar->state==UTF at all? That's a good idea, I'll explore how to do that. One way is to just have a new boolean to control if wmi events should be delivered to user space or not. I would not be surprised if at some point we would want to use the testmode WMI interface also in "normal mode". -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-29 12:51 ` Kalle Valo @ 2014-05-30 6:17 ` Michal Kazior 2014-05-30 10:06 ` Kalle Valo 0 siblings, 1 reply; 13+ messages in thread From: Michal Kazior @ 2014-05-30 6:17 UTC (permalink / raw) To: Kalle Valo; +Cc: ath10k@lists.infradead.org On 29 May 2014 14:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Hi Michal, > > BTW, please edit your quotes. It's pain to find your comments from a 700 > line monster :) > > Michal Kazior <michal.kazior@tieto.com> writes: > >> On 28 May 2014 11:19, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >>> +/* skb owned by the caller, must not sleep */ >>> +void ath10k_tm_event_wmi(struct ath10k *ar, u32 cmd_id, struct sk_buff *skb) >>> +{ >>> + struct sk_buff *nl_skb; >>> + int ret; >>> + >>> + ath10k_dbg(ATH10K_DBG_TESTMODE, >>> + "testmode event wmi cmd_id %d skb %p skb->len %d\n", >>> + cmd_id, skb, skb->len); >>> + >>> + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", skb->data, skb->len); >>> + >>> + /* FIXME: locking! */ >>> + >>> + if (ar->state != ATH10K_STATE_UTF) >>> + return; >> >> You shouldn't really be depending on the value of ar->state without >> conf_mutex being held. >> >> Since this is tasklet context you can't just grab the mutex here. >> >> The simplest way is to probably to move this into a worker and a have >> sk_buff_head. > > Yeah, this is what I was planning to do. > >> Another way is to rework the state system to be async (it might be >> easier to do now after my recent hw restart changes) and have >> ar->state be protected only by ar->data_lock. This might be tricky >> though. I'll take a look at it.. > > Please don't waste time on that, unless there are other advantages. This > testmode interface is no way performance critical and workqueue is more > than adequate. > >>> + if (ar->testmode.utf != NULL) >>> + /* utf is already loaded */ >>> + goto power_up; >>> + >>> + >>> + snprintf(filename, sizeof(filename), "%s/%s", >>> + ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); >> >> 1 extraneous empty line. > > Will fix. > >>> + ath10k_dbg(ATH10K_DBG_TESTMODE, >>> + "testmode cmd wmi cmd_id %d buf %p buf_len %d\n", >>> + cmd_id, buf, buf_len); >>> + >>> + ath10k_dbg_dump(ATH10K_DBG_TESTMODE, NULL, "", buf, buf_len); >>> + >>> + skb = ath10k_wmi_alloc_skb(buf_len); >> >> if (!skb) return -ENOMEM; ? > > Yup. > >>> +int ath10k_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif, >>> + void *data, int len) >>> +{ >>> + struct ath10k *ar = hw->priv; >>> + struct nlattr *tb[ATH10K_TM_ATTR_MAX + 1]; >>> + int err; >>> + >>> + err = nla_parse(tb, ATH10K_TM_ATTR_MAX, data, len, >>> + ath10k_tm_policy); >>> + if (err) >>> + return err; >> >> s/err/ret/ ? > > Yes, I'll change that. > >>> +void ath10k_testmode_unregister(struct ath10k *ar) >>> +{ >>> + /* FIXME: locking! */ >> >> I think this should be protected by conf_mutex. > > Yeah, that's why I have the FIXME :) > >>> + >>> + release_firmware(ar->testmode.utf); >> >> It might be a good idea to ar->testmode.utf = NULL ? > > I agree. > >>> + if (ar->state != ATH10K_STATE_UTF) >>> + /* utf firmware is not running */ >>> + return; >>> + >>> + ath10k_htc_stop(&ar->htc); >>> + ath10k_wmi_detach(ar); >> >> Why not ath10k_core_stop() ? > > Doh, I can't remember anymore. Maybe because of the htt calls? But I > agree, we should call core_stop() for symmetry. I'll investigate. Well, you already call ath10k_core_stop() in ath10k_tm_cmd_utf_stop(). I think it should be safe (without getting deep into this) because you only skip htt_connect (basically a htc service connect command) and htt_setup (htt rx ring setup and htt rx version req commands), but maybe I'm missing something. >>> @@ -2333,6 +2338,7 @@ static void ath10k_wmi_10x_process_rx(struct ath10k *ar, struct sk_buff *skb) >>> >>> static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb) >>> { >>> + /* FIXME: somehow force ot use 10x with UTF */ >> >> `ot`: typo? > > Yeah. And the FIXME will be gone in the final version, this was just an > RFC to get early comments on the design. > >> I'm a little confused how this works. When UTF firmware is loaded does >> it generate regular WMI events? Or does it generate UTF_EVENTID only? > > Only UTF_EVENTID, at least for now. > >> Is there a particular reason for ath10k_tm_event_wmi() to be called >> before the switch() and have it no effect on processing of the event >> later whatsoever? > > I want to send the full WMI packet (instead of UTF_EVENTID) to user > space to make it easier to extend this later, in case there will be a > need for that in the future. For example, there can be new event ids and > whatnot. The less we need to change the user space interface the easier > it will be. I get it now. We could call skb_push() in UTF_EVENTID case to get back the wmi header. It's not very pretty but saves a lot of work (i.e. no need to depend on ar->state and no need to create a worker/skb queue) now. We could also depend on cmd_id to accept/reject skbuffs in ath10k_tm_event_wmi() since, at least for now, there aren't any special cases to handle. But then.. will we really ever need to have a passthrough (i.e. pass buffers to userspace but still process them in the driver afterwards)? Does that even make sense? Either way I think this (why we want to send out a wmi packet including its header to userspace) should be documented. Michał _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-30 6:17 ` Michal Kazior @ 2014-05-30 10:06 ` Kalle Valo 2014-05-30 10:22 ` Michal Kazior 0 siblings, 1 reply; 13+ messages in thread From: Kalle Valo @ 2014-05-30 10:06 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k@lists.infradead.org Michal Kazior <michal.kazior@tieto.com> writes: > On 29 May 2014 14:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > >> BTW, please edit your quotes. It's pain to find your comments from a 700 >> line monster :) This request still stands, even though 170 lines is much better than 700 lines ;) >>>> + if (ar->state != ATH10K_STATE_UTF) >>>> + /* utf firmware is not running */ >>>> + return; >>>> + >>>> + ath10k_htc_stop(&ar->htc); >>>> + ath10k_wmi_detach(ar); >>> >>> Why not ath10k_core_stop() ? >> >> Doh, I can't remember anymore. Maybe because of the htt calls? But I >> agree, we should call core_stop() for symmetry. I'll investigate. > > Well, you already call ath10k_core_stop() in ath10k_tm_cmd_utf_stop(). > I think it should be safe (without getting deep into this) because you > only skip htt_connect (basically a htc service connect command) and > htt_setup (htt rx ring setup and htt rx version req commands), but > maybe I'm missing something. Ok, I'll check it carefully. >>> I'm a little confused how this works. When UTF firmware is loaded does >>> it generate regular WMI events? Or does it generate UTF_EVENTID only? >> >> Only UTF_EVENTID, at least for now. >> >>> Is there a particular reason for ath10k_tm_event_wmi() to be called >>> before the switch() and have it no effect on processing of the event >>> later whatsoever? >> >> I want to send the full WMI packet (instead of UTF_EVENTID) to user >> space to make it easier to extend this later, in case there will be a >> need for that in the future. For example, there can be new event ids and >> whatnot. The less we need to change the user space interface the easier >> it will be. > > I get it now. We could call skb_push() in UTF_EVENTID case to get back > the wmi header. It's not very pretty but saves a lot of work (i.e. no > need to depend on ar->state and no need to create a worker/skb queue) > now. That still limits us to only UTF_EVENTID, I would like to avoid that. And I want to send WMI events to user space only when it's explicitly enabled via testmode interface. But I have some ideas, let me send v2. (But only once I have managed to make my huge email backlog smaller, argh!) > We could also depend on cmd_id to accept/reject skbuffs in > ath10k_tm_event_wmi() since, at least for now, there aren't any > special cases to handle. But then.. will we really ever need to have a > passthrough (i.e. pass buffers to userspace but still process them in > the driver afterwards)? Does that even make sense? I think that's getting a bit too complicated. Basically we just need ath10k act as a pipe for WMI packets. > Either way I think this (why we want to send out a wmi packet > including its header to userspace) should be documented. Yes, I'll do that. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-30 10:06 ` Kalle Valo @ 2014-05-30 10:22 ` Michal Kazior 2014-05-30 10:35 ` Kalle Valo 0 siblings, 1 reply; 13+ messages in thread From: Michal Kazior @ 2014-05-30 10:22 UTC (permalink / raw) To: Kalle Valo; +Cc: ath10k@lists.infradead.org On 30 May 2014 12:06, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> On 29 May 2014 14:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> >>> BTW, please edit your quotes. It's pain to find your comments from a 700 >>> line monster :) > > This request still stands, even though 170 lines is much better than 700 > lines ;) I intended to trim it this time but I've hit 'send' before actually doing that.. sorry! >> We could also depend on cmd_id to accept/reject skbuffs in >> ath10k_tm_event_wmi() since, at least for now, there aren't any >> special cases to handle. But then.. will we really ever need to have a >> passthrough (i.e. pass buffers to userspace but still process them in >> the driver afterwards)? Does that even make sense? > > I think that's getting a bit too complicated. Basically we just need > ath10k act as a pipe for WMI packets. Hmm.. With v1 code if you assume UTF firmware sends an SWBA event then ath10k would send the testmode event AND try to actually handle it as if it was regular operation.. but how is that supposed to work if you have no mac80211 interface structures around at all? My point is ath10k_tm_event_wmi() should _consume_ buffers and the caller should not processes consumed buffers (assuming you want to keep ath10k_tm_event_wmi call where it is now, i.e. before switch-case). Michał _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-30 10:22 ` Michal Kazior @ 2014-05-30 10:35 ` Kalle Valo 2014-05-30 10:40 ` Michal Kazior 0 siblings, 1 reply; 13+ messages in thread From: Kalle Valo @ 2014-05-30 10:35 UTC (permalink / raw) To: Michal Kazior; +Cc: ath10k@lists.infradead.org Michal Kazior <michal.kazior@tieto.com> writes: > On 30 May 2014 12:06, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Michal Kazior <michal.kazior@tieto.com> writes: >> >>> On 29 May 2014 14:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >>> >>>> BTW, please edit your quotes. It's pain to find your comments from a 700 >>>> line monster :) >> >> This request still stands, even though 170 lines is much better than 700 >> lines ;) > > I intended to trim it this time but I've hit 'send' before actually > doing that.. sorry! No problem :) >>> We could also depend on cmd_id to accept/reject skbuffs in >>> ath10k_tm_event_wmi() since, at least for now, there aren't any >>> special cases to handle. But then.. will we really ever need to have a >>> passthrough (i.e. pass buffers to userspace but still process them in >>> the driver afterwards)? Does that even make sense? >> >> I think that's getting a bit too complicated. Basically we just need >> ath10k act as a pipe for WMI packets. > > Hmm.. > > With v1 code if you assume UTF firmware sends an SWBA event then > ath10k would send the testmode event AND try to actually handle it as > if it was regular operation.. but how is that supposed to work if you > have no mac80211 interface structures around at all? My point is > ath10k_tm_event_wmi() should _consume_ buffers and the caller should > not processes consumed buffers (assuming you want to keep > ath10k_tm_event_wmi call where it is now, i.e. before switch-case). I see your point now, thanks for banging it to my thick head with a sledge hammer :) I'll rethink this part and try to solve it for v2. The reason I would like to avoid your consuming idea is that at some point we might want to enable this WMI pipe also in "normal mode". -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ath10k: add testmode 2014-05-30 10:35 ` Kalle Valo @ 2014-05-30 10:40 ` Michal Kazior 0 siblings, 0 replies; 13+ messages in thread From: Michal Kazior @ 2014-05-30 10:40 UTC (permalink / raw) To: Kalle Valo; +Cc: ath10k@lists.infradead.org On 30 May 2014 12:35, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> On 30 May 2014 12:06, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >>> Michal Kazior <michal.kazior@tieto.com> writes: >>> >>>> On 29 May 2014 14:51, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >>>> >>>>> BTW, please edit your quotes. It's pain to find your comments from a 700 >>>>> line monster :) >>> >>> This request still stands, even though 170 lines is much better than 700 >>> lines ;) >> >> I intended to trim it this time but I've hit 'send' before actually >> doing that.. sorry! > > No problem :) > >>>> We could also depend on cmd_id to accept/reject skbuffs in >>>> ath10k_tm_event_wmi() since, at least for now, there aren't any >>>> special cases to handle. But then.. will we really ever need to have a >>>> passthrough (i.e. pass buffers to userspace but still process them in >>>> the driver afterwards)? Does that even make sense? >>> >>> I think that's getting a bit too complicated. Basically we just need >>> ath10k act as a pipe for WMI packets. >> >> Hmm.. >> >> With v1 code if you assume UTF firmware sends an SWBA event then >> ath10k would send the testmode event AND try to actually handle it as >> if it was regular operation.. but how is that supposed to work if you >> have no mac80211 interface structures around at all? My point is >> ath10k_tm_event_wmi() should _consume_ buffers and the caller should >> not processes consumed buffers (assuming you want to keep >> ath10k_tm_event_wmi call where it is now, i.e. before switch-case). > > I see your point now, thanks for banging it to my thick head with a > sledge hammer :) > > I'll rethink this part and try to solve it for v2. The reason I would > like to avoid your consuming idea is that at some point we might want to > enable this WMI pipe also in "normal mode". I'm not really sure what you mean by that. Can you give an example where this pass-through behaviour would be desired, please? Michał _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] ath10k: testmode support 2014-05-28 9:19 [RFC PATCH 0/2] ath10k: testmode support Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 1/2] ath10k: make ath10k_wmi_cmd_send() public Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 2/2] ath10k: add testmode Kalle Valo @ 2014-06-01 19:51 ` Sven Schnelle 2014-06-02 15:48 ` Kalle Valo 2 siblings, 1 reply; 13+ messages in thread From: Sven Schnelle @ 2014-06-01 19:51 UTC (permalink / raw) To: Kalle Valo, ath10k Hi Kalle, Am 28.05.2014 11:19, schrieb Kalle Valo: > here's how I'm thinking we should implement testmode support to ath10k. Please > comment. I tried the patch, and was able to continuous TX with a heavily modified ART version. Only one small thing: The firmware expects the UTF commands in segmented format. Should that splitting be done in your testmode patch, or should userspace handle that? Regards Sven _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] ath10k: testmode support 2014-06-01 19:51 ` [RFC PATCH 0/2] ath10k: testmode support Sven Schnelle @ 2014-06-02 15:48 ` Kalle Valo 2014-06-03 7:12 ` Sven Schnelle 0 siblings, 1 reply; 13+ messages in thread From: Kalle Valo @ 2014-06-02 15:48 UTC (permalink / raw) To: Sven Schnelle; +Cc: ath10k Sven Schnelle <svens@stackframe.org> writes: > Only one small thing: The firmware expects the UTF commands in > segmented format. Should that splitting be done in your testmode > patch, or should userspace handle that? I would prefer to keep the driver as simple as possible. My idea here is that the testmode interface is only about tranmitting and receiving WMI packets, it does not look at all what's inside the packet. AFAIK the user space should be able to handle everything on it's own. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] ath10k: testmode support 2014-06-02 15:48 ` Kalle Valo @ 2014-06-03 7:12 ` Sven Schnelle 0 siblings, 0 replies; 13+ messages in thread From: Sven Schnelle @ 2014-06-03 7:12 UTC (permalink / raw) To: kvalo; +Cc: ath10k Hi Kalle, On 06/02/2014 05:48 PM, Kalle Valo wrote: > Sven Schnelle <svens@stackframe.org> writes: > >> Only one small thing: The firmware expects the UTF commands in >> segmented format. Should that splitting be done in your testmode >> patch, or should userspace handle that? > I would prefer to keep the driver as simple as possible. My idea here is > that the testmode interface is only about tranmitting and receiving WMI > packets, it does not look at all what's inside the packet. AFAIK the > user space should be able to handle everything on it's own. > Ok, that's fine with me. I'm doing the fragmation in userspace already, just wanted to mention that. Thanks Sven _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-06-03 7:09 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-28 9:19 [RFC PATCH 0/2] ath10k: testmode support Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 1/2] ath10k: make ath10k_wmi_cmd_send() public Kalle Valo 2014-05-28 9:19 ` [RFC PATCH 2/2] ath10k: add testmode Kalle Valo 2014-05-29 11:43 ` Michal Kazior 2014-05-29 12:51 ` Kalle Valo 2014-05-30 6:17 ` Michal Kazior 2014-05-30 10:06 ` Kalle Valo 2014-05-30 10:22 ` Michal Kazior 2014-05-30 10:35 ` Kalle Valo 2014-05-30 10:40 ` Michal Kazior 2014-06-01 19:51 ` [RFC PATCH 0/2] ath10k: testmode support Sven Schnelle 2014-06-02 15:48 ` Kalle Valo 2014-06-03 7:12 ` Sven Schnelle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox