diff mbox

[4/5] soc: qcom: wcnss_ctrl: Make wcnss_ctrl parent the other components

Message ID 1459226126-16725-4-git-send-email-bjorn.andersson@linaro.org
State New
Headers show

Commit Message

Bjorn Andersson March 29, 2016, 4:35 a.m. UTC 
We need the signal from wcnss_ctrl indicating that the firmware is up
and running before we can communicate with the other components of the
chip. So make these other components children of the wcnss_ctrl device,
so they can be probed in order.

The process seems to take between 1/2-5 seconds, so this is done in a
worker, instead of holding up the probe.

Also adding the wait for a cbc completion if the firmware indicates this is
needed - like on 8016.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---
 drivers/soc/qcom/wcnss_ctrl.c       | 117 ++++++++++++++++++++++++++++++------
 include/linux/soc/qcom/wcnss_ctrl.h |   8 +++
 2 files changed, 108 insertions(+), 17 deletions(-)
 create mode 100644 include/linux/soc/qcom/wcnss_ctrl.h

-- 
2.5.0

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

Comments

Bjorn Andersson March 31, 2016, 7:39 p.m. UTC | #1 
On Thu 31 Mar 11:47 PDT 2016, Andy Gross wrote:

> On Mon, Mar 28, 2016 at 09:35:25PM -0700, Bjorn Andersson wrote:

> > We need the signal from wcnss_ctrl indicating that the firmware is up

> > and running before we can communicate with the other components of the

> > chip. So make these other components children of the wcnss_ctrl device,

> > so they can be probed in order.

> > 

> > The process seems to take between 1/2-5 seconds, so this is done in a

> > worker, instead of holding up the probe.

> > 

> > Also adding the wait for a cbc completion if the firmware indicates this is

> > needed - like on 8016.

> 

> Can you define what a CBC is?

> 


Unfortunately I have no clue and have not yet seen this abbreviation in
any documentation...

> 

> <snip>

> 

> >  /**

> >   * wcnss_download_nv() - send nv binary to WCNSS

> > - * @work:	work struct to acquire wcnss context

> > + * @wcnss:	wcnss_ctrl state handle

> > + *

> > + * Returns 1 on success or 2 to indicate upcoming cbc completion. Negative

> 

> maybe a nit, but define the return values

> 


I'll remap them a step down, so the function returns 0 on success and 1
on "need to wait for cbc complete". That way we don't have an undefined
hole in the return set as well.

> > + * errno on failure.

> >   */

> > -static void wcnss_download_nv(struct work_struct *work)

> > +static int wcnss_download_nv(struct wcnss_ctrl *wcnss)

> >  {

> > -	struct wcnss_ctrl *wcnss = container_of(work, struct wcnss_ctrl, download_nv_work);

> >  	struct wcnss_download_nv_req *req;

> >  	const struct firmware *fw;

> >  	const void *data;

> > @@ -178,7 +207,7 @@ static void wcnss_download_nv(struct work_struct *work)

> >  

> >  	req = kzalloc(sizeof(*req) + NV_FRAGMENT_SIZE, GFP_KERNEL);

> >  	if (!req)

> > -		return;

> > +		return -ENOMEM;

> >  

> >  	ret = request_firmware(&fw, NVBIN_FILE, wcnss->dev);

> >  	if (ret) {

> 

> <snip>

> 

> > +static void wcnss_async_probe(struct work_struct *work)

> > +{

> > +	struct wcnss_ctrl *wcnss = container_of(work, struct wcnss_ctrl, probe_work);

> > +	int ret;

> > +

> > +	ret = wcnss_request_version(wcnss);

> > +	if (ret < 0)

> > +		return;

> > +

> > +	ret = wcnss_download_nv(wcnss);

> > +	if (ret < 0)

> > +		return;

> > +

> > +	/* Wait for pending CBC completion if download nv returns 2 */

> > +	if (ret == 2) {

> 

> Perhaps have a #define for the value is warranted

> 


With the above remap I can rename ret here to need_cbc and the code will
be understandable.

> > +		ret = wait_for_completion_timeout(&wcnss->cbc, WCNSS_REQUEST_TIMEOUT);

> > +		if (!ret)

> > +			dev_err(wcnss->dev, "expected cbc completion\n");

> > +	}

> > +

> > +	of_platform_populate(wcnss->dev->of_node, NULL, NULL, wcnss->dev);

> >  }

> 

> <snip>


Thanks for the feedback!

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c
index c544f3d2c6ee..80bea66889a2 100644
--- a/drivers/soc/qcom/wcnss_ctrl.c
+++ b/drivers/soc/qcom/wcnss_ctrl.c
@@ -1,4 +1,5 @@ 
 /*
+ * Copyright (c) 2016, Linaro Ltd.
  * Copyright (c) 2015, Sony Mobile Communications Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -14,8 +15,13 @@ 
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/smd.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/wcnss_ctrl.h>
 
 #define WCNSS_REQUEST_TIMEOUT	(5 * HZ)
+#define WCNSS_CBC_TIMEOUT	(10 * HZ)
 
 #define NV_FRAGMENT_SIZE	3072
 #define NVBIN_FILE		"wlan/prima/WCNSS_qcom_wlan_nv.bin"
@@ -25,17 +31,19 @@ 
  * @dev:	device handle
  * @channel:	SMD channel handle
  * @ack:	completion for outstanding requests
+ * @cbc:	completion for cbc complete indication
  * @ack_status:	status of the outstanding request
- * @download_nv_work: worker for uploading nv binary
+ * @probe_work: worker for uploading nv binary
  */
 struct wcnss_ctrl {
 	struct device *dev;
 	struct qcom_smd_channel *channel;
 
 	struct completion ack;
+	struct completion cbc;
 	int ack_status;
 
-	struct work_struct download_nv_work;
+	struct work_struct probe_work;
 };
 
 /* message types */
@@ -48,6 +56,11 @@  enum {
 	WCNSS_UPLOAD_CAL_RESP,
 	WCNSS_DOWNLOAD_CAL_REQ,
 	WCNSS_DOWNLOAD_CAL_RESP,
+	WCNSS_VBAT_LEVEL_IND,
+	WCNSS_BUILD_VERSION_REQ,
+	WCNSS_BUILD_VERSION_RESP,
+	WCNSS_PM_CONFIG_REQ,
+	WCNSS_CBC_COMPLETE_IND,
 };
 
 /**
@@ -128,7 +141,7 @@  static int wcnss_ctrl_smd_callback(struct qcom_smd_channel *channel,
 			 version->major, version->minor,
 			 version->version, version->revision);
 
-		schedule_work(&wcnss->download_nv_work);
+		complete(&wcnss->ack);
 		break;
 	case WCNSS_DOWNLOAD_NV_RESP:
 		if (count != sizeof(*nvresp)) {
@@ -141,6 +154,10 @@  static int wcnss_ctrl_smd_callback(struct qcom_smd_channel *channel,
 		wcnss->ack_status = nvresp->status;
 		complete(&wcnss->ack);
 		break;
+	case WCNSS_CBC_COMPLETE_IND:
+		dev_dbg(wcnss->dev, "WCNSS booted\n");
+		complete(&wcnss->cbc);
+		break;
 	default:
 		dev_info(wcnss->dev, "unknown message type %d\n", hdr->type);
 		break;
@@ -156,20 +173,32 @@  static int wcnss_ctrl_smd_callback(struct qcom_smd_channel *channel,
 static int wcnss_request_version(struct wcnss_ctrl *wcnss)
 {
 	struct wcnss_msg_hdr msg;
+	int ret;
 
 	msg.type = WCNSS_VERSION_REQ;
 	msg.len = sizeof(msg);
+	ret = qcom_smd_send(wcnss->channel, &msg, sizeof(msg));
+	if (ret < 0)
+		return ret;
+
+	ret = wait_for_completion_timeout(&wcnss->ack, WCNSS_CBC_TIMEOUT);
+	if (!ret) {
+		dev_err(wcnss->dev, "timeout waiting for version response\n");
+		return -ETIMEDOUT;
+	}
 
-	return qcom_smd_send(wcnss->channel, &msg, sizeof(msg));
+	return 0;
 }
 
 /**
  * wcnss_download_nv() - send nv binary to WCNSS
- * @work:	work struct to acquire wcnss context
+ * @wcnss:	wcnss_ctrl state handle
+ *
+ * Returns 1 on success or 2 to indicate upcoming cbc completion. Negative
+ * errno on failure.
  */
-static void wcnss_download_nv(struct work_struct *work)
+static int wcnss_download_nv(struct wcnss_ctrl *wcnss)
 {
-	struct wcnss_ctrl *wcnss = container_of(work, struct wcnss_ctrl, download_nv_work);
 	struct wcnss_download_nv_req *req;
 	const struct firmware *fw;
 	const void *data;
@@ -178,7 +207,7 @@  static void wcnss_download_nv(struct work_struct *work)
 
 	req = kzalloc(sizeof(*req) + NV_FRAGMENT_SIZE, GFP_KERNEL);
 	if (!req)
-		return;
+		return -ENOMEM;
 
 	ret = request_firmware(&fw, NVBIN_FILE, wcnss->dev);
 	if (ret) {
@@ -220,16 +249,56 @@  static void wcnss_download_nv(struct work_struct *work)
 	} while (left > 0);
 
 	ret = wait_for_completion_timeout(&wcnss->ack, WCNSS_REQUEST_TIMEOUT);
-	if (!ret)
+	if (!ret) {
 		dev_err(wcnss->dev, "timeout waiting for nv upload ack\n");
-	else if (wcnss->ack_status != 1)
-		dev_err(wcnss->dev, "nv upload response failed err: %d\n",
-			wcnss->ack_status);
+		ret = -ETIMEDOUT;
+	} else {
+		ret = wcnss->ack_status;
+	}
 
 release_fw:
 	release_firmware(fw);
 free_req:
 	kfree(req);
+
+	return ret;
+}
+
+/**
+ * qcom_wcnss_open_channel() - open additional SMD channel to WCNSS
+ * @wcnss:	wcnss handle, retrieved from drvdata
+ * @name:	SMD channel name
+ * @cb:		callback to handle incoming data on the channel
+ */
+struct qcom_smd_channel *qcom_wcnss_open_channel(void *wcnss, const char *name, qcom_smd_cb_t cb)
+{
+	struct wcnss_ctrl *_wcnss = wcnss;
+
+	return qcom_smd_open_channel(_wcnss->channel, name, cb);
+}
+EXPORT_SYMBOL(qcom_wcnss_open_channel);
+
+static void wcnss_async_probe(struct work_struct *work)
+{
+	struct wcnss_ctrl *wcnss = container_of(work, struct wcnss_ctrl, probe_work);
+	int ret;
+
+	ret = wcnss_request_version(wcnss);
+	if (ret < 0)
+		return;
+
+	ret = wcnss_download_nv(wcnss);
+	if (ret < 0)
+		return;
+
+	/* Wait for pending CBC completion if download nv returns 2 */
+	if (ret == 2) {
+		ret = wait_for_completion_timeout(&wcnss->cbc, WCNSS_REQUEST_TIMEOUT);
+		if (!ret)
+			dev_err(wcnss->dev, "expected cbc completion\n");
+	}
+
+	of_platform_populate(wcnss->dev->of_node, NULL, NULL, wcnss->dev);
 }
 
 static int wcnss_ctrl_probe(struct qcom_smd_device *sdev)
@@ -244,25 +313,39 @@  static int wcnss_ctrl_probe(struct qcom_smd_device *sdev)
 	wcnss->channel = sdev->channel;
 
 	init_completion(&wcnss->ack);
-	INIT_WORK(&wcnss->download_nv_work, wcnss_download_nv);
+	init_completion(&wcnss->cbc);
+	INIT_WORK(&wcnss->probe_work, wcnss_async_probe);
 
 	qcom_smd_set_drvdata(sdev->channel, wcnss);
 
-	return wcnss_request_version(wcnss);
+	qcom_smd_set_drvdata(sdev->channel, wcnss);
+
+	schedule_work(&wcnss->probe_work);
+
+	return 0;
+}
+
+static void wcnss_ctrl_remove(struct qcom_smd_device *sdev)
+{
+	struct wcnss_ctrl *wcnss = qcom_smd_get_drvdata(sdev->channel);
+
+	cancel_work_sync(&wcnss->probe_work);
+	of_platform_depopulate(&sdev->dev);
 }
 
-static const struct qcom_smd_id wcnss_ctrl_smd_match[] = {
-	{ .name = "WCNSS_CTRL" },
+static const struct of_device_id wcnss_ctrl_of_match[] = {
+	{ .compatible = "qcom,wcnss", },
 	{}
 };
 
 static struct qcom_smd_driver wcnss_ctrl_driver = {
 	.probe = wcnss_ctrl_probe,
+	.remove = wcnss_ctrl_remove,
 	.callback = wcnss_ctrl_smd_callback,
-	.smd_match_table = wcnss_ctrl_smd_match,
 	.driver  = {
 		.name  = "qcom_wcnss_ctrl",
 		.owner = THIS_MODULE,
+		.of_match_table = wcnss_ctrl_of_match,
 	},
 };
 
diff --git a/include/linux/soc/qcom/wcnss_ctrl.h b/include/linux/soc/qcom/wcnss_ctrl.h
new file mode 100644
index 000000000000..a37bc5538f19
--- /dev/null
+++ b/include/linux/soc/qcom/wcnss_ctrl.h
@@ -0,0 +1,8 @@ 
+#ifndef __WCNSS_CTRL_H__
+#define __WCNSS_CTRL_H__
+
+#include <linux/soc/qcom/smd.h>
+
+struct qcom_smd_channel *qcom_wcnss_open_channel(void *wcnss, const char *name, qcom_smd_cb_t cb);
+
+#endif