linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding
@ 2016-05-12 23:17 Bjorn Andersson
  2016-05-12 23:17 ` [PATCH v2 2/2] soc: qcom: wcnss_ctrl: Make wcnss_ctrl parent the other components Bjorn Andersson
       [not found] ` <1463095068-32225-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 2 replies; 4+ messages in thread
From: Bjorn Andersson @ 2016-05-12 23:17 UTC (permalink / raw)
  To: Andy Gross, Rob Herring; +Cc: linux-arm-msm, linux-soc, linux-kernel

This binding describes the control interface for the Qualcomm WCNSS.

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

Changes since v1:
- Introduce reference to wcnss block node for register block definition
- Use wcnss block node compatible for hw version detection (riva vs pronto)
- Clean up compatible name for bt node

 .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 115 +++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
new file mode 100644
index 000000000000..5413098a1e1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
@@ -0,0 +1,115 @@
+Qualcomm WCNSS Binding
+
+This binding describes the Qualcomm WCNSS hardware. It consists of control
+block and a BT, WiFi and FM radio block, all useing SMD as command channels.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be: "qcom,wcnss",
+
+- qcom,smd-channel:
+	Usage: required
+	Value type: <string>
+	Definition: standard SMD property specifying the SMD channel used for
+		    communication with the WiFi firmware
+
+- qcom,mmio:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: reference to a node specifying the wcnss "ccu" and "dxe"
+		    register blocks. The node must be compatible with one of
+		    the following:
+		    "qcom,riva",
+		    "qcom,pronto"
+
+= SUBNODES
+The subnodes of the wcnss node are optional and describe the individual blocks in
+the WCNSS.
+
+== Bluetooth
+The following properties are defined to the bluetooth node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be:
+		    "qcom,wcnss-bt"
+
+== WiFi
+The following properties are defined to the WiFi node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,wcnss-wlan",
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify the "rx" and "tx" interrupts
+
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must contain "rx" and "tx"
+
+- qcom,state:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should reference the tx-enable and tx-rings-empty SMEM states
+
+- qcom,state-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must contain "tx-enable" and "tx-rings-empty"
+
+= EXAMPLE
+The following example represents a SMD node, with one edge representing the
+"pronto" subsystem, with the wcnss device and its wcn3680 BT and WiFi blocks
+described; as found on the 8974 platform.
+
+smd {
+	compatible = "qcom,smd";
+
+	pronto {
+		interrupts = <0 142 1>;
+
+		qcom,ipc = <&apcs 8 17>;
+		qcom,smd-edge = <6>;
+
+		wcnss {
+			compatible = "qcom,wcnss";
+			qcom,smd-channels = "WCNSS_CTRL";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			qcom,wcnss = <&pronto>;
+
+			bt {
+				compatible = "qcom,wcnss-bt";
+			};
+
+			wlan {
+				compatible = "qcom,wcnss-wlan";
+
+				interrupts = <0 145 0>, <0 146 0>;
+				interrupt-names = "tx", "rx";
+
+				qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;
+				qcom,state-names = "tx-enable", "tx-rings-empty";
+			};
+		};
+	};
+};
+
+soc {
+	pronto: pronto {
+		compatible = "qcom,pronto";
+
+		reg = <0xfb204000 0x2000>, <0xfb202000 0x1000>, <0xfb21b000 0x3000>;
+		reg-names = "ccu", "dxe", "pmu";
+	};
+};
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] soc: qcom: wcnss_ctrl: Make wcnss_ctrl parent the other components
  2016-05-12 23:17 [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding Bjorn Andersson
@ 2016-05-12 23:17 ` Bjorn Andersson
       [not found] ` <1463095068-32225-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2016-05-12 23:17 UTC (permalink / raw)
  To: Andy Gross, Rob Herring; +Cc: linux-arm-msm, linux-soc, linux-kernel

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.

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

Changes since v1:
- Clean up return information from wcnss_download_nv (success vs cbc expected)

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

diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c
index 7a986f881d5c..31d1abe10454 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,16 @@
 #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 WCNSS_ACK_DONE_BOOTING	1
+#define WCNSS_ACK_COLD_BOOTING	2
 
 #define NV_FRAGMENT_SIZE	3072
 #define NVBIN_FILE		"wlan/prima/WCNSS_qcom_wlan_nv.bin"
@@ -25,17 +34,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 +59,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,
 };
 
 /**
@@ -100,17 +116,17 @@ struct wcnss_download_nv_resp {
 
 /**
  * wcnss_ctrl_smd_callback() - handler from SMD responses
- * @qsdev:	smd device handle
+ * @channel:	smd channel handle
  * @data:	pointer to the incoming data packet
  * @count:	size of the incoming data packet
  *
  * Handles any incoming packets from the remote WCNSS_CTRL service.
  */
-static int wcnss_ctrl_smd_callback(struct qcom_smd_device *qsdev,
+static int wcnss_ctrl_smd_callback(struct qcom_smd_channel *channel,
 				   const void *data,
 				   size_t count)
 {
-	struct wcnss_ctrl *wcnss = dev_get_drvdata(&qsdev->dev);
+	struct wcnss_ctrl *wcnss = qcom_smd_get_drvdata(channel);
 	const struct wcnss_download_nv_resp *nvresp;
 	const struct wcnss_version_resp *version;
 	const struct wcnss_msg_hdr *hdr = data;
@@ -128,7 +144,7 @@ static int wcnss_ctrl_smd_callback(struct qcom_smd_device *qsdev,
 			 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 +157,10 @@ static int wcnss_ctrl_smd_callback(struct qcom_smd_device *qsdev,
 		wcnss->ack_status = nvresp->status;
 		complete(&wcnss->ack);
 		break;
+	case WCNSS_CBC_COMPLETE_IND:
+		dev_dbg(wcnss->dev, "cold boot complete\n");
+		complete(&wcnss->cbc);
+		break;
 	default:
 		dev_info(wcnss->dev, "unknown message type %d\n", hdr->type);
 		break;
@@ -156,20 +176,32 @@ static int wcnss_ctrl_smd_callback(struct qcom_smd_device *qsdev,
 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
+ * @expect_cbc:	indicator to caller that an cbc event is expected
+ *
+ * Returns 0 on success. Negative errno on failure.
  */
-static void wcnss_download_nv(struct work_struct *work)
+static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc)
 {
-	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,10 +210,10 @@ 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) {
+	if (ret < 0) {
 		dev_err(wcnss->dev, "Failed to load nv file %s: %d\n",
 			NVBIN_FILE, ret);
 		goto free_req;
@@ -207,7 +239,7 @@ static void wcnss_download_nv(struct work_struct *work)
 		memcpy(req->fragment, data, req->frag_size);
 
 		ret = qcom_smd_send(wcnss->channel, req, req->hdr.len);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(wcnss->dev, "failed to send smd packet\n");
 			goto release_fw;
 		}
@@ -220,16 +252,58 @@ 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 {
+		*expect_cbc = wcnss->ack_status == WCNSS_ACK_COLD_BOOTING;
+		ret = 0;
+	}
 
 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);
+	bool expect_cbc;
+	int ret;
+
+	ret = wcnss_request_version(wcnss);
+	if (ret < 0)
+		return;
+
+	ret = wcnss_download_nv(wcnss, &expect_cbc);
+	if (ret < 0)
+		return;
+
+	/* Wait for pending cold boot completion if indicated by the nv downloader */
+	if (expect_cbc) {
+		ret = wait_for_completion_timeout(&wcnss->cbc, WCNSS_REQUEST_TIMEOUT);
+		if (!ret)
+			dev_err(wcnss->dev, "expected cold boot 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 +318,40 @@ 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);
 	dev_set_drvdata(&sdev->dev, 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
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding
       [not found] ` <1463095068-32225-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2016-05-27  3:58   ` Rob Herring
  2016-05-27  4:28     ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Rob Herring @ 2016-05-27  3:58 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, linux-arm-msm, linux-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

+dt list

On Thu, May 12, 2016 at 6:17 PM, Bjorn Andersson
<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This binding describes the control interface for the Qualcomm WCNSS.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
> Changes since v1:
> - Introduce reference to wcnss block node for register block definition
> - Use wcnss block node compatible for hw version detection (riva vs pronto)
> - Clean up compatible name for bt node
>
>  .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 115 +++++++++++++++++++++
>  1 file changed, 115 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> new file mode 100644
> index 000000000000..5413098a1e1a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> @@ -0,0 +1,115 @@
> +Qualcomm WCNSS Binding
> +
> +This binding describes the Qualcomm WCNSS hardware. It consists of control
> +block and a BT, WiFi and FM radio block, all useing SMD as command channels.

s/useing/using/

> +
> +- compatible:
> +       Usage: required
> +       Value type: <string>
> +       Definition: must be: "qcom,wcnss",
> +
> +- qcom,smd-channel:
> +       Usage: required
> +       Value type: <string>
> +       Definition: standard SMD property specifying the SMD channel used for
> +                   communication with the WiFi firmware

Is the string value defined somewhere?

> +
> +- qcom,mmio:
> +       Usage: required
> +       Value type: <prop-encoded-array>
> +       Definition: reference to a node specifying the wcnss "ccu" and "dxe"
> +                   register blocks. The node must be compatible with one of
> +                   the following:
> +                   "qcom,riva",
> +                   "qcom,pronto"
> +
> += SUBNODES
> +The subnodes of the wcnss node are optional and describe the individual blocks in
> +the WCNSS.
> +
> +== Bluetooth
> +The following properties are defined to the bluetooth node:
> +
> +- compatible:
> +       Usage: required
> +       Value type: <string>
> +       Definition: must be:
> +                   "qcom,wcnss-bt"
> +
> +== WiFi
> +The following properties are defined to the WiFi node:
> +
> +- compatible:
> +       Usage: required
> +       Value type: <string>
> +       Definition: must be one of:
> +                   "qcom,wcnss-wlan",
> +
> +- interrupts:
> +       Usage: required
> +       Value type: <prop-encoded-array>
> +       Definition: should specify the "rx" and "tx" interrupts
> +
> +- interrupt-names:
> +       Usage: required
> +       Value type: <stringlist>
> +       Definition: must contain "rx" and "tx"
> +
> +- qcom,state:

This is kind of generic.

> +       Usage: required
> +       Value type: <prop-encoded-array>
> +       Definition: should reference the tx-enable and tx-rings-empty SMEM states
> +
> +- qcom,state-names:
> +       Usage: required
> +       Value type: <stringlist>
> +       Definition: must contain "tx-enable" and "tx-rings-empty"
> +
> += EXAMPLE
> +The following example represents a SMD node, with one edge representing the
> +"pronto" subsystem, with the wcnss device and its wcn3680 BT and WiFi blocks
> +described; as found on the 8974 platform.

So somewhere in here I'd expect to see 8974 in a compatible string.

> +
> +smd {
> +       compatible = "qcom,smd";
> +
> +       pronto {
> +               interrupts = <0 142 1>;
> +
> +               qcom,ipc = <&apcs 8 17>;
> +               qcom,smd-edge = <6>;
> +
> +               wcnss {
> +                       compatible = "qcom,wcnss";
> +                       qcom,smd-channels = "WCNSS_CTRL";
> +
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +
> +                       qcom,wcnss = <&pronto>;

Not documented and poorly named. The property name should reflect what
this is for or points too.

> +
> +                       bt {
> +                               compatible = "qcom,wcnss-bt";
> +                       };
> +
> +                       wlan {
> +                               compatible = "qcom,wcnss-wlan";
> +
> +                               interrupts = <0 145 0>, <0 146 0>;
> +                               interrupt-names = "tx", "rx";
> +
> +                               qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;
> +                               qcom,state-names = "tx-enable", "tx-rings-empty";
> +                       };
> +               };
> +       };
> +};
> +
> +soc {
> +       pronto: pronto {

2 pronto nodes? That's confusing...

> +               compatible = "qcom,pronto";
> +
> +               reg = <0xfb204000 0x2000>, <0xfb202000 0x1000>, <0xfb21b000 0x3000>;
> +               reg-names = "ccu", "dxe", "pmu";
> +       };
> +};
> --
> 2.5.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding
  2016-05-27  3:58   ` [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding Rob Herring
@ 2016-05-27  4:28     ` Bjorn Andersson
  0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2016-05-27  4:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Andy Gross, linux-arm-msm, linux-soc,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org

On Thu 26 May 20:58 PDT 2016, Rob Herring wrote:

> +dt list
> 
> On Thu, May 12, 2016 at 6:17 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
[..]
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> > new file mode 100644
> > index 000000000000..5413098a1e1a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> > @@ -0,0 +1,115 @@
> > +Qualcomm WCNSS Binding
> > +
> > +This binding describes the Qualcomm WCNSS hardware. It consists of control
> > +block and a BT, WiFi and FM radio block, all useing SMD as command channels.
> 
> s/useing/using/
> 

Right.

> > +
> > +- compatible:
> > +       Usage: required
> > +       Value type: <string>
> > +       Definition: must be: "qcom,wcnss",
> > +
> > +- qcom,smd-channel:
> > +       Usage: required
> > +       Value type: <string>
> > +       Definition: standard SMD property specifying the SMD channel used for
> > +                   communication with the WiFi firmware
> 
> Is the string value defined somewhere?
> 

It's never been anything else than WCNSS_CTRL so I guess I could
document that.

> > +
> > +- qcom,mmio:
> > +       Usage: required
> > +       Value type: <prop-encoded-array>
> > +       Definition: reference to a node specifying the wcnss "ccu" and "dxe"
> > +                   register blocks. The node must be compatible with one of
> > +                   the following:
> > +                   "qcom,riva",
> > +                   "qcom,pronto"
> > +
[..]
> > +
> > +- qcom,state:
> 
> This is kind of generic.
> 

We can rename it qcom,smem-state to make it less likely to collide in
the future. I believe this is the first binding referencing one of
those.

> > +       Usage: required
> > +       Value type: <prop-encoded-array>
> > +       Definition: should reference the tx-enable and tx-rings-empty SMEM states
> > +
> > +- qcom,state-names:

And then qcom,smem-state-names here

> > +       Usage: required
> > +       Value type: <stringlist>
> > +       Definition: must contain "tx-enable" and "tx-rings-empty"
> > +
> > += EXAMPLE
> > +The following example represents a SMD node, with one edge representing the
> > +"pronto" subsystem, with the wcnss device and its wcn3680 BT and WiFi blocks
> > +described; as found on the 8974 platform.
> 
> So somewhere in here I'd expect to see 8974 in a compatible string.
> 

The "Wireless CoNnectivity Sub-System" version is "pronto" and this is
the only thing that has a hardware dependency. So the only part that
makes this 8974 and not 8016 is the placing of those blocks in the
memory space (and the interrupts).

And the compatible qcom,iris would make this a 8064 node.

> > +
> > +smd {
> > +       compatible = "qcom,smd";
> > +
> > +       pronto {
> > +               interrupts = <0 142 1>;
> > +
> > +               qcom,ipc = <&apcs 8 17>;
> > +               qcom,smd-edge = <6>;
> > +
> > +               wcnss {
> > +                       compatible = "qcom,wcnss";
> > +                       qcom,smd-channels = "WCNSS_CTRL";
> > +
> > +                       #address-cells = <1>;
> > +                       #size-cells = <1>;
> > +
> > +                       qcom,wcnss = <&pronto>;
> 
> Not documented and poorly named. The property name should reflect what
> this is for or points too.
> 

It's documented as qcom,mmio, so the example is wrong.

> > +
> > +                       bt {
> > +                               compatible = "qcom,wcnss-bt";
> > +                       };
> > +
> > +                       wlan {
> > +                               compatible = "qcom,wcnss-wlan";
> > +
> > +                               interrupts = <0 145 0>, <0 146 0>;
> > +                               interrupt-names = "tx", "rx";
> > +
> > +                               qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;
> > +                               qcom,state-names = "tx-enable", "tx-rings-empty";
> > +                       };
> > +               };
> > +       };
> > +};
> > +
> > +soc {
> > +       pronto: pronto {
> 
> 2 pronto nodes? That's confusing...
> 

This one denotes the pronto register space, the one above denotes the
communication channel to the firmware running on the cpu in the wcnss.

But in SMD terms the one above is the "pronto edge", so I can change
that to pronto-edge.

> > +               compatible = "qcom,pronto";
> > +
> > +               reg = <0xfb204000 0x2000>, <0xfb202000 0x1000>, <0xfb21b000 0x3000>;
> > +               reg-names = "ccu", "dxe", "pmu";
> > +       };
> > +};

Thanks for your review.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-27  4:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-12 23:17 [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding Bjorn Andersson
2016-05-12 23:17 ` [PATCH v2 2/2] soc: qcom: wcnss_ctrl: Make wcnss_ctrl parent the other components Bjorn Andersson
     [not found] ` <1463095068-32225-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-05-27  3:58   ` [PATCH v2 1/2] dt: binding: Add Qualcomm WCNSS control binding Rob Herring
2016-05-27  4:28     ` Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).