Re: [PATCH 11/12] ath10k: Add wlan mode on/off qmi message

[Bjorn Andersson]

On Sun 25 Mar 22:41 PDT 2018, Govind Singh wrote:

> Add qmi message required for enabling and disabling
> target to qmi server running in Q6.
> 
> Signed-off-by: Govind Singh 
> ---
>  drivers/net/wireless/ath/ath10k/qmi.c | 226 
> +-
>  drivers/net/wireless/ath/ath10k/qmi.h |  62 ++
>  2 files changed, 264 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/qmi.c 
> b/drivers/net/wireless/ath/ath10k/qmi.c
> index f23d0fe..331a528 100644
> --- a/drivers/net/wireless/ath/ath10k/qmi.c
> +++ b/drivers/net/wireless/ath/ath10k/qmi.c
> @@ -35,7 +35,30 @@
>  #define WLFW_CLIENT_ID   0x4b4e454c
>  #define WLFW_TIMEOUT 500
>  
> -static struct ath10k_qmi *qmi;
> +static struct ath10k_qmi {
> + struct platform_device *pdev;
> + struct qmi_handle qmi_hdl;
> + struct sockaddr_qrtr sq;
> + bool fw_ready;
> + bool msa_ready;
> + struct work_struct work_svc_arrive;
> + struct work_struct work_svc_exit;
> + struct work_struct work_msa_ready;
> + struct workqueue_struct *event_wq;
> + spinlock_t event_lock; /* spinlock for fw ready status*/
> + u32 nr_mem_region;
> + struct ath10k_msa_mem_region_info
> + mem_region[MAX_NUM_MEMORY_REGIONS];
> + phys_addr_t msa_pa;
> + u32 msa_mem_size;
> + void *msa_va;
> + struct ath10k_qmi_chip_info chip_info;
> + struct ath10k_qmi_board_info board_info;
> + struct ath10k_qmi_soc_info soc_info;
> + struct ath10k_qmi_fw_version_info fw_version_info;
> + char fw_build_id[MAX_BUILD_ID_LEN + 1];
> + struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01];
> +} *qmi;

Don't end your patch series with a move of this structure, fold it back
into each patch.

>  
>  static int
>  ath10k_qmi_map_msa_permissions(struct ath10k_msa_mem_region_info *mem_region)
> @@ -444,6 +467,207 @@ int ath10k_qmi_send_cal_report_req(struct ath10k_qmi 
> *qmi)
>   return ret;
>  }
>  
> +static int
> +ath10k_qmi_mode_send_sync_msg(enum wlfw_driver_mode_enum_v01 mode)
> +{
> + struct wlfw_wlan_mode_resp_msg_v01 *resp;

4 bytes

> + struct wlfw_wlan_mode_req_msg_v01 *req;

6 bytes, use the stack.

> + struct qmi_txn txn;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + resp = kzalloc(sizeof(*resp), GFP_KERNEL);
> + if (!resp) {
> + kfree(req);
> + return -ENOMEM;
> + }
> +
> + ret = qmi_txn_init(>qmi_hdl, ,
> +wlfw_wlan_mode_resp_msg_v01_ei,
> +resp);
> + if (ret < 0) {
> + pr_err("fail to init txn for mode req %d ret %d\n", mode, ret);
> + goto out;
> + }
> +
> + req->mode = mode;
> + req->hw_debug_valid = 1;
> + req->hw_debug = 0;
> +
> + ret = qmi_send_request(>qmi_hdl, NULL, ,
> +QMI_WLFW_WLAN_MODE_REQ_V01,
> +WLFW_WLAN_MODE_REQ_MSG_V01_MAX_MSG_LEN,
> +wlfw_wlan_mode_req_msg_v01_ei, req);
> + if (ret < 0) {
> + qmi_txn_cancel();
> + pr_err("send mode req failed, mode: %d ret: %d\n",
> +mode, ret);
> + goto out;
> + }
> +
> + ret = qmi_txn_wait(, WLFW_TIMEOUT * HZ);
> + if (ret < 0)
> + goto out;
> +
> + if (resp->resp.result != QMI_RESULT_SUCCESS_V01) {
> + pr_err("qmi mode request rejected:");
> + pr_err("mode:%d result:%d error:%d\n",
> +mode, resp->resp.result, resp->resp.error);
> + ret = resp->resp.result;
> + goto out;
> + }
> +
> + pr_debug("wlan Mode request completed, mode: %d\n", mode);
> + kfree(resp);
> + kfree(req);
> + return 0;
> +
> +out:
> + kfree(resp);
> + kfree(req);
> + return ret;
> +}
> +
> +static int
> +ath10k_qmi_cfg_send_sync_msg(struct wlfw_wlan_cfg_req_msg_v01 *data)
> +{
> + struct wlfw_wlan_cfg_resp_msg_v01 *resp;

4 bytes, use stack

> + struct wlfw_wlan_cfg_req_msg_v01 *req;

This is larger, use heap

> + struct qmi_txn txn;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + resp = kzalloc(sizeof(*resp), GFP_KERNEL);
> + if (!resp) {
> + kfree(req);
> + return -ENOMEM;
> + }
> +
> + ret = qmi_txn_init(>qmi_hdl, ,
> +wlfw_wlan_cfg_resp_msg_v01_ei,
> +resp);
> + if (ret < 0) {
> + pr_err("fail to init txn for config req %d\n", ret);
> + goto out;
> + }
> +
> + memcpy(req, data, sizeof(*req));

I see no reason to create a local copy of this struct, just pass data to
qmi_send_request.

> +
> + ret = qmi_send_request(>qmi_hdl, NULL, ,
> +

[PATCH 11/12] ath10k: Add wlan mode on/off qmi message

[Govind Singh]

Add qmi message required for enabling and disabling
target to qmi server running in Q6.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/qmi.c | 226 +-
 drivers/net/wireless/ath/ath10k/qmi.h |  62 ++
 2 files changed, 264 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/qmi.c 
b/drivers/net/wireless/ath/ath10k/qmi.c
index f23d0fe..331a528 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -35,7 +35,30 @@
 #define WLFW_CLIENT_ID 0x4b4e454c
 #define WLFW_TIMEOUT   500
 
-static struct ath10k_qmi *qmi;
+static struct ath10k_qmi {
+   struct platform_device *pdev;
+   struct qmi_handle qmi_hdl;
+   struct sockaddr_qrtr sq;
+   bool fw_ready;
+   bool msa_ready;
+   struct work_struct work_svc_arrive;
+   struct work_struct work_svc_exit;
+   struct work_struct work_msa_ready;
+   struct workqueue_struct *event_wq;
+   spinlock_t event_lock; /* spinlock for fw ready status*/
+   u32 nr_mem_region;
+   struct ath10k_msa_mem_region_info
+   mem_region[MAX_NUM_MEMORY_REGIONS];
+   phys_addr_t msa_pa;
+   u32 msa_mem_size;
+   void *msa_va;
+   struct ath10k_qmi_chip_info chip_info;
+   struct ath10k_qmi_board_info board_info;
+   struct ath10k_qmi_soc_info soc_info;
+   struct ath10k_qmi_fw_version_info fw_version_info;
+   char fw_build_id[MAX_BUILD_ID_LEN + 1];
+   struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01];
+} *qmi;
 
 static int
 ath10k_qmi_map_msa_permissions(struct ath10k_msa_mem_region_info *mem_region)
@@ -444,6 +467,207 @@ int ath10k_qmi_send_cal_report_req(struct ath10k_qmi *qmi)
return ret;
 }
 
+static int
+ath10k_qmi_mode_send_sync_msg(enum wlfw_driver_mode_enum_v01 mode)
+{
+   struct wlfw_wlan_mode_resp_msg_v01 *resp;
+   struct wlfw_wlan_mode_req_msg_v01 *req;
+   struct qmi_txn txn;
+   int ret;
+
+   req = kzalloc(sizeof(*req), GFP_KERNEL);
+   if (!req)
+   return -ENOMEM;
+
+   resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+   if (!resp) {
+   kfree(req);
+   return -ENOMEM;
+   }
+
+   ret = qmi_txn_init(>qmi_hdl, ,
+  wlfw_wlan_mode_resp_msg_v01_ei,
+  resp);
+   if (ret < 0) {
+   pr_err("fail to init txn for mode req %d ret %d\n", mode, ret);
+   goto out;
+   }
+
+   req->mode = mode;
+   req->hw_debug_valid = 1;
+   req->hw_debug = 0;
+
+   ret = qmi_send_request(>qmi_hdl, NULL, ,
+  QMI_WLFW_WLAN_MODE_REQ_V01,
+  WLFW_WLAN_MODE_REQ_MSG_V01_MAX_MSG_LEN,
+  wlfw_wlan_mode_req_msg_v01_ei, req);
+   if (ret < 0) {
+   qmi_txn_cancel();
+   pr_err("send mode req failed, mode: %d ret: %d\n",
+  mode, ret);
+   goto out;
+   }
+
+   ret = qmi_txn_wait(, WLFW_TIMEOUT * HZ);
+   if (ret < 0)
+   goto out;
+
+   if (resp->resp.result != QMI_RESULT_SUCCESS_V01) {
+   pr_err("qmi mode request rejected:");
+   pr_err("mode:%d result:%d error:%d\n",
+  mode, resp->resp.result, resp->resp.error);
+   ret = resp->resp.result;
+   goto out;
+   }
+
+   pr_debug("wlan Mode request completed, mode: %d\n", mode);
+   kfree(resp);
+   kfree(req);
+   return 0;
+
+out:
+   kfree(resp);
+   kfree(req);
+   return ret;
+}
+
+static int
+ath10k_qmi_cfg_send_sync_msg(struct wlfw_wlan_cfg_req_msg_v01 *data)
+{
+   struct wlfw_wlan_cfg_resp_msg_v01 *resp;
+   struct wlfw_wlan_cfg_req_msg_v01 *req;
+   struct qmi_txn txn;
+   int ret;
+
+   req = kzalloc(sizeof(*req), GFP_KERNEL);
+   if (!req)
+   return -ENOMEM;
+
+   resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+   if (!resp) {
+   kfree(req);
+   return -ENOMEM;
+   }
+
+   ret = qmi_txn_init(>qmi_hdl, ,
+  wlfw_wlan_cfg_resp_msg_v01_ei,
+  resp);
+   if (ret < 0) {
+   pr_err("fail to init txn for config req %d\n", ret);
+   goto out;
+   }
+
+   memcpy(req, data, sizeof(*req));
+
+   ret = qmi_send_request(>qmi_hdl, NULL, ,
+  QMI_WLFW_WLAN_CFG_REQ_V01,
+  WLFW_WLAN_CFG_REQ_MSG_V01_MAX_MSG_LEN,
+  wlfw_wlan_cfg_req_msg_v01_ei, req);
+   if (ret < 0) {
+   qmi_txn_cancel();
+   pr_err("send config req failed %d\n", ret);
+   goto out;
+   }
+
+   ret = qmi_txn_wait(, WLFW_TIMEOUT * HZ);
+   if (ret < 0)
+   goto out;
+
+