[PATCH 1/5] soc: qcom: smd: Make callback pass channel reference

[PATCH 1/5] soc: qcom: smd: Make callback pass channel reference

[Bjorn Andersson]

By passing the smd channel reference to the callback, rather than the
smd device, we can open additional smd channels from sub-devices of smd
devices.

Also updates the two smd clients today found in mainline.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/smd-rpm.c    |  9 ++++++---
 drivers/soc/qcom/smd.c        | 22 ++++++++++++++++++----
 drivers/soc/qcom/wcnss_ctrl.c |  8 ++++----
 include/linux/soc/qcom/smd.h  |  7 +++++--
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/qcom/smd-rpm.c b/drivers/soc/qcom/smd-rpm.c
index 731fa066f712..6609d7e0edb0 100644
--- a/drivers/soc/qcom/smd-rpm.c
+++ b/drivers/soc/qcom/smd-rpm.c
@@ -33,6 +33,7 @@
  */
 struct qcom_smd_rpm {
 	struct qcom_smd_channel *rpm_channel;
+	struct device *dev;
 
 	struct completion ack;
 	struct mutex lock;
@@ -149,14 +150,14 @@ out:
 }
 EXPORT_SYMBOL(qcom_rpm_smd_write);
 
-static int qcom_smd_rpm_callback(struct qcom_smd_device *qsdev,
+static int qcom_smd_rpm_callback(struct qcom_smd_channel *channel,
 				 const void *data,
 				 size_t count)
 {
 	const struct qcom_rpm_header *hdr = data;
 	size_t hdr_length = le32_to_cpu(hdr->length);
 	const struct qcom_rpm_message *msg;
-	struct qcom_smd_rpm *rpm = dev_get_drvdata(&qsdev->dev);
+	struct qcom_smd_rpm *rpm = qcom_smd_get_drvdata(channel);
 	const u8 *buf = data + sizeof(struct qcom_rpm_header);
 	const u8 *end = buf + hdr_length;
 	char msgbuf[32];
@@ -165,7 +166,7 @@ static int qcom_smd_rpm_callback(struct qcom_smd_device *qsdev,
 
 	if (le32_to_cpu(hdr->service_type) != RPM_SERVICE_TYPE_REQUEST ||
 	    hdr_length < sizeof(struct qcom_rpm_message)) {
-		dev_err(&qsdev->dev, "invalid request\n");
+		dev_err(rpm->dev, "invalid request\n");
 		return 0;
 	}
 
@@ -206,7 +207,9 @@ static int qcom_smd_rpm_probe(struct qcom_smd_device *sdev)
 	mutex_init(&rpm->lock);
 	init_completion(&rpm->ack);
 
+	rpm->dev = &sdev->dev;
 	rpm->rpm_channel = sdev->channel;
+	qcom_smd_set_drvdata(sdev->channel, rpm);
 
 	dev_set_drvdata(&sdev->dev, rpm);
 
diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index b6434c4be86a..ac1957dfdf24 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -194,6 +194,8 @@ struct qcom_smd_channel {
 
 	int pkt_size;
 
+	void *drvdata;
+
 	struct list_head list;
 	struct list_head dev_list;
 };
@@ -513,7 +515,6 @@ static void qcom_smd_channel_advance(struct qcom_smd_channel *channel,
  */
 static int qcom_smd_channel_recv_single(struct qcom_smd_channel *channel)
 {
-	struct qcom_smd_device *qsdev = channel->qsdev;
 	unsigned tail;
 	size_t len;
 	void *ptr;
@@ -533,7 +534,7 @@ static int qcom_smd_channel_recv_single(struct qcom_smd_channel *channel)
 		len = channel->pkt_size;
 	}
 
-	ret = channel->cb(qsdev, ptr, len);
+	ret = channel->cb(channel, ptr, len);
 	if (ret < 0)
 		return ret;
 
@@ -1034,6 +1035,18 @@ int qcom_smd_driver_register(struct qcom_smd_driver *qsdrv)
 }
 EXPORT_SYMBOL(qcom_smd_driver_register);
 
+void *qcom_smd_get_drvdata(struct qcom_smd_channel *channel)
+{
+	return channel->drvdata;
+}
+EXPORT_SYMBOL(qcom_smd_get_drvdata);
+
+void qcom_smd_set_drvdata(struct qcom_smd_channel *channel, void *data)
+{
+	channel->drvdata = data;
+}
+EXPORT_SYMBOL(qcom_smd_set_drvdata);
+
 /**
  * qcom_smd_driver_unregister - unregister a smd driver
  * @qsdrv:	qcom_smd_driver struct
@@ -1079,12 +1092,13 @@ qcom_smd_find_channel(struct qcom_smd_edge *edge, const char *name)
  * Returns a channel handle on success, or -EPROBE_DEFER if the channel isn't
  * ready.
  */
-struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
+struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_channel *parent,
 					       const char *name,
 					       qcom_smd_cb_t cb)
 {
 	struct qcom_smd_channel *channel;
-	struct qcom_smd_edge *edge = sdev->channel->edge;
+	struct qcom_smd_device *sdev = parent->qsdev;
+	struct qcom_smd_edge *edge = parent->edge;
 	int ret;
 
 	/* Wait up to HZ for the channel to appear */
diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c
index 7a986f881d5c..c544f3d2c6ee 100644
--- a/drivers/soc/qcom/wcnss_ctrl.c
+++ b/drivers/soc/qcom/wcnss_ctrl.c
@@ -100,17 +100,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;
@@ -246,7 +246,7 @@ static int wcnss_ctrl_probe(struct qcom_smd_device *sdev)
 	init_completion(&wcnss->ack);
 	INIT_WORK(&wcnss->download_nv_work, wcnss_download_nv);
 
-	dev_set_drvdata(&sdev->dev, wcnss);
+	qcom_smd_set_drvdata(sdev->channel, wcnss);
 
 	return wcnss_request_version(wcnss);
 }
diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
index bd51c8a9d807..cb2f81559bc0 100644
--- a/include/linux/soc/qcom/smd.h
+++ b/include/linux/soc/qcom/smd.h
@@ -26,7 +26,7 @@ struct qcom_smd_device {
 	struct qcom_smd_channel *channel;
 };
 
-typedef int (*qcom_smd_cb_t)(struct qcom_smd_device *, const void *, size_t);
+typedef int (*qcom_smd_cb_t)(struct qcom_smd_channel *, const void *, size_t);
 
 /**
  * struct qcom_smd_driver - smd driver struct
@@ -50,13 +50,16 @@ struct qcom_smd_driver {
 int qcom_smd_driver_register(struct qcom_smd_driver *drv);
 void qcom_smd_driver_unregister(struct qcom_smd_driver *drv);
 
+void *qcom_smd_get_drvdata(struct qcom_smd_channel *channel);
+void qcom_smd_set_drvdata(struct qcom_smd_channel *channel, void *data);
+
 #define module_qcom_smd_driver(__smd_driver) \
 	module_driver(__smd_driver, qcom_smd_driver_register, \
 		      qcom_smd_driver_unregister)
 
 int qcom_smd_send(struct qcom_smd_channel *channel, const void *data, int len);
 
-struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_device *sdev,
+struct qcom_smd_channel *qcom_smd_open_channel(struct qcom_smd_channel *channel,
 					       const char *name,
 					       qcom_smd_cb_t cb);
 
-- 
2.5.0

[PATCH 2/5] soc: qcom: smem: Use write-combine remap for SMEM

[Bjorn Andersson]

Mapping the SMEM region as write combine makes the contiguous writes
in SMD perform better and also allows us to do unaligned read and writes
on ARM64.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/smem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 19019aa092e8..2e1aa9f130f4 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -684,8 +684,7 @@ static int qcom_smem_map_memory(struct qcom_smem *smem, struct device *dev,
 
 	smem->regions[i].aux_base = (u32)r.start;
 	smem->regions[i].size = resource_size(&r);
-	smem->regions[i].virt_base = devm_ioremap_nocache(dev, r.start,
-							  resource_size(&r));
+	smem->regions[i].virt_base = devm_ioremap_wc(dev, r.start, resource_size(&r));
 	if (!smem->regions[i].virt_base)
 		return -ENOMEM;
 
-- 
2.5.0

Re: [PATCH 2/5] soc: qcom: smem: Use write-combine remap for SMEM

[Andy Gross]

On Mon, Mar 28, 2016 at 09:35:23PM -0700, Bjorn Andersson wrote:
> Mapping the SMEM region as write combine makes the contiguous writes
> in SMD perform better and also allows us to do unaligned read and writes
> on ARM64.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Nice.  This should make things work much better.  The mappings should have been
writecombine from the get go.

Reviewed-by: Andy Gross <andy.gross@linaro.org>

[PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

This binding describes the control interface for the Qualcomm WCNSS.

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

Got a reviewed-by from Andy and acked-by from Rob on the WiFi part of this
binding. But during futher testing I spotted a timing issue, where the
wcnss_ctrl driver must finish the uploading of NV to the core os of the wcnss
before the wifi driver can initiate any communication with the wifi part
(although available from the get-go).

The sanest way I could figure to model this is to describe the core part as
parent of the wifi and bt pieces.

 .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 104 +++++++++++++++++++++
 1 file changed, 104 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..5488904b6185
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
@@ -0,0 +1,104 @@
+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
+
+= 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,btqcomsmd"
+
+== WiFi
+The following properties are defined to the WiFi node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,wcn3620-wlan",
+		    "qcom,wcn3660-wlan",
+		    "qcom,wcn3680-wlan"
+
+- qcom,wcnss-mmio:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify base address and size of the WiFi related
+		    registers of WCNSS
+
+- 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>;
+
+			bt {
+				compatible = "qcom,btqcomsmd";
+			};
+
+			wifi {
+				compatible = "qcom,wcn3680-wlan";
+
+				qcom,wcnss-mmio = <0xfb000000 0x21b000>;
+
+				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";
+			};
+		};
+	};
+};
-- 
2.5.0

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> This binding describes the control interface for the Qualcomm WCNSS.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> 
> Got a reviewed-by from Andy and acked-by from Rob on the WiFi part of this
> binding. But during futher testing I spotted a timing issue, where the
> wcnss_ctrl driver must finish the uploading of NV to the core os of the wcnss
> before the wifi driver can initiate any communication with the wifi part
> (although available from the get-go).
> 
> The sanest way I could figure to model this is to describe the core part as
> parent of the wifi and bt pieces.
> 
>  .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 104 +++++++++++++++++++++
>  1 file changed, 104 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..5488904b6185
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> @@ -0,0 +1,104 @@
> +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",

This should be more specific.


> +- qcom,smd-channel:
> +	Usage: required
> +	Value type: <string>
> +	Definition: standard SMD property specifying the SMD channel used for
> +		    communication with the WiFi firmware
> +
> += 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,btqcomsmd"

This should be more specific to the chip and there's no need to have 
qcom twice.

> +
> +== WiFi
> +The following properties are defined to the WiFi node:
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,wcn3620-wlan",
> +		    "qcom,wcn3660-wlan",
> +		    "qcom,wcn3680-wlan"
> +
> +- qcom,wcnss-mmio:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: should specify base address and size of the WiFi related
> +		    registers of WCNSS

This is an address visible to the cpu?

> +
> +- 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>;
> +
> +			bt {
> +				compatible = "qcom,btqcomsmd";
> +			};
> +
> +			wifi {
> +				compatible = "qcom,wcn3680-wlan";
> +
> +				qcom,wcnss-mmio = <0xfb000000 0x21b000>;
> +
> +				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";
> +			};
> +		};
> +	};
> +};
> -- 
> 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

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:

> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson 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..5488904b6185
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> > @@ -0,0 +1,104 @@
> > +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",
> 
> This should be more specific.
> 

Okay, will have to go back to Qualcomm and try to figure out what kind
of version there actually is on this component.

[..]
> > += 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,btqcomsmd"
> 
> This should be more specific to the chip and there's no need to have 
> qcom twice.
> 

There's only a single implementation of this downstream, so same answer
as above...

> > +
> > +== WiFi
> > +The following properties are defined to the WiFi node:
> > +
> > +- compatible:
> > +	Usage: required
> > +	Value type: <string>
> > +	Definition: must be one of:
> > +		    "qcom,wcn3620-wlan",
> > +		    "qcom,wcn3660-wlan",
> > +		    "qcom,wcn3680-wlan"

Digging through documentation and trying to answer the questions above
made me realize that these numbers are for the external rf component,
not the variants of the logic inside the SoC; and as such wrong.

> > +
> > +- qcom,wcnss-mmio:
> > +	Usage: required
> > +	Value type: <prop-encoded-array>
> > +	Definition: should specify base address and size of the WiFi related
> > +		    registers of WCNSS
> 
> This is an address visible to the cpu?
> 

Yes it is; the device is controlled both through SMD and mmio accessible
registers, where the SMD interface is the primary interface.

SMD being the primary "bus" I believe I can't use reg to denote this
register range. Should I describe this in some other form?

Regards,
Bjorn

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
>
>> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> [..]

[...]

>> > +
>> > +== WiFi
>> > +The following properties are defined to the WiFi node:
>> > +
>> > +- compatible:
>> > +   Usage: required
>> > +   Value type: <string>
>> > +   Definition: must be one of:
>> > +               "qcom,wcn3620-wlan",
>> > +               "qcom,wcn3660-wlan",
>> > +               "qcom,wcn3680-wlan"
>
> Digging through documentation and trying to answer the questions above
> made me realize that these numbers are for the external rf component,
> not the variants of the logic inside the SoC; and as such wrong.

Do you need to know both? Or only the firmware image needs to know?

>> > +
>> > +- qcom,wcnss-mmio:
>> > +   Usage: required
>> > +   Value type: <prop-encoded-array>
>> > +   Definition: should specify base address and size of the WiFi related
>> > +               registers of WCNSS
>>
>> This is an address visible to the cpu?
>>
>
> Yes it is; the device is controlled both through SMD and mmio accessible
> registers, where the SMD interface is the primary interface.
>
> SMD being the primary "bus" I believe I can't use reg to denote this
> register range. Should I describe this in some other form?

That's a tricky one. I would create a node for the memory-mapped
portion with proper compatible and reg properties, and then make this
a phandle to that node. Something similar to how we do phandles to
syscon's.

Rob

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:

> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
> <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
> >
> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> > [..]
> 
> [...]
> 
> >> > +
> >> > +== WiFi
> >> > +The following properties are defined to the WiFi node:
> >> > +
> >> > +- compatible:
> >> > +   Usage: required
> >> > +   Value type: <string>
> >> > +   Definition: must be one of:
> >> > +               "qcom,wcn3620-wlan",
> >> > +               "qcom,wcn3660-wlan",
> >> > +               "qcom,wcn3680-wlan"
> >
> > Digging through documentation and trying to answer the questions above
> > made me realize that these numbers are for the external rf component,
> > not the variants of the logic inside the SoC; and as such wrong.
> 
> Do you need to know both? Or only the firmware image needs to know?
> 

So far I've only found cases where we need to know the register map for
the DMA engine shuffling packets, so this is related to the SoC-internal
part only.

The differences in RF capabilities - at least for WiFi - seems to be
acquired in runtime from the firmware.

The other piece that depend on the RF part seems to be the availability
of e.g. ANT support, so if anything that needs to go into the wcnss
node, in some way (either compatible or the set of subnodes).

> >> > +
> >> > +- qcom,wcnss-mmio:
> >> > +   Usage: required
> >> > +   Value type: <prop-encoded-array>
> >> > +   Definition: should specify base address and size of the WiFi related
> >> > +               registers of WCNSS
> >>
> >> This is an address visible to the cpu?
> >>
> >
> > Yes it is; the device is controlled both through SMD and mmio accessible
> > registers, where the SMD interface is the primary interface.
> >
> > SMD being the primary "bus" I believe I can't use reg to denote this
> > register range. Should I describe this in some other form?
> 
> That's a tricky one. I would create a node for the memory-mapped
> portion with proper compatible and reg properties, and then make this
> a phandle to that node. Something similar to how we do phandles to
> syscon's.
> 

Okay, sounds reasonable. I don't see a need for a specific
implementation, so I'll just back it with the generic syscon
implementation (and a specific compatible).

Regards,
Bjorn
--
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

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Thu, Mar 31, 2016 at 1:24 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:
>
>> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
>> >
>> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
>> > [..]
>>
>> [...]
>>
>> >> > +
>> >> > +== WiFi
>> >> > +The following properties are defined to the WiFi node:
>> >> > +
>> >> > +- compatible:
>> >> > +   Usage: required
>> >> > +   Value type: <string>
>> >> > +   Definition: must be one of:
>> >> > +               "qcom,wcn3620-wlan",
>> >> > +               "qcom,wcn3660-wlan",
>> >> > +               "qcom,wcn3680-wlan"
>> >
>> > Digging through documentation and trying to answer the questions above
>> > made me realize that these numbers are for the external rf component,
>> > not the variants of the logic inside the SoC; and as such wrong.
>>
>> Do you need to know both? Or only the firmware image needs to know?
>>
>
> So far I've only found cases where we need to know the register map for
> the DMA engine shuffling packets, so this is related to the SoC-internal
> part only.
>
> The differences in RF capabilities - at least for WiFi - seems to be
> acquired in runtime from the firmware.
>
> The other piece that depend on the RF part seems to be the availability
> of e.g. ANT support, so if anything that needs to go into the wcnss
> node, in some way (either compatible or the set of subnodes).
>
>> >> > +
>> >> > +- qcom,wcnss-mmio:
>> >> > +   Usage: required
>> >> > +   Value type: <prop-encoded-array>
>> >> > +   Definition: should specify base address and size of the WiFi related
>> >> > +               registers of WCNSS
>> >>
>> >> This is an address visible to the cpu?
>> >>
>> >
>> > Yes it is; the device is controlled both through SMD and mmio accessible
>> > registers, where the SMD interface is the primary interface.
>> >
>> > SMD being the primary "bus" I believe I can't use reg to denote this
>> > register range. Should I describe this in some other form?
>>
>> That's a tricky one. I would create a node for the memory-mapped
>> portion with proper compatible and reg properties, and then make this
>> a phandle to that node. Something similar to how we do phandles to
>> syscon's.
>>
>
> Okay, sounds reasonable. I don't see a need for a specific
> implementation, so I'll just back it with the generic syscon
> implementation (and a specific compatible).

I don't think I'd do syscon here as it is mainly designed to have
multiple users. You just need to look-up the phandle, perhaps check
the compatible, and call of_address_to_resource to get the address.
Actually, you could skip the phandle entirely and just find the node
by compatible (assuming there is only one).

Rob

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 13:42 PDT 2016, Rob Herring wrote:

> On Thu, Mar 31, 2016 at 1:24 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:
> >
> >> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
> >> <bjorn.andersson@linaro.org> wrote:
> >> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
> >> >
> >> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> >> > [..]
> >>
> >> [...]
> >>
> >> >> > +
> >> >> > +== WiFi
> >> >> > +The following properties are defined to the WiFi node:
> >> >> > +
> >> >> > +- compatible:
> >> >> > +   Usage: required
> >> >> > +   Value type: <string>
> >> >> > +   Definition: must be one of:
> >> >> > +               "qcom,wcn3620-wlan",
> >> >> > +               "qcom,wcn3660-wlan",
> >> >> > +               "qcom,wcn3680-wlan"
> >> >
> >> > Digging through documentation and trying to answer the questions above
> >> > made me realize that these numbers are for the external rf component,
> >> > not the variants of the logic inside the SoC; and as such wrong.
> >>
> >> Do you need to know both? Or only the firmware image needs to know?
> >>
> >
> > So far I've only found cases where we need to know the register map for
> > the DMA engine shuffling packets, so this is related to the SoC-internal
> > part only.
> >
> > The differences in RF capabilities - at least for WiFi - seems to be
> > acquired in runtime from the firmware.
> >
> > The other piece that depend on the RF part seems to be the availability
> > of e.g. ANT support, so if anything that needs to go into the wcnss
> > node, in some way (either compatible or the set of subnodes).
> >
> >> >> > +
> >> >> > +- qcom,wcnss-mmio:
> >> >> > +   Usage: required
> >> >> > +   Value type: <prop-encoded-array>
> >> >> > +   Definition: should specify base address and size of the WiFi related
> >> >> > +               registers of WCNSS
> >> >>
> >> >> This is an address visible to the cpu?
> >> >>
> >> >
> >> > Yes it is; the device is controlled both through SMD and mmio accessible
> >> > registers, where the SMD interface is the primary interface.
> >> >
> >> > SMD being the primary "bus" I believe I can't use reg to denote this
> >> > register range. Should I describe this in some other form?
> >>
> >> That's a tricky one. I would create a node for the memory-mapped
> >> portion with proper compatible and reg properties, and then make this
> >> a phandle to that node. Something similar to how we do phandles to
> >> syscon's.
> >>
> >
> > Okay, sounds reasonable. I don't see a need for a specific
> > implementation, so I'll just back it with the generic syscon
> > implementation (and a specific compatible).
> 
> I don't think I'd do syscon here as it is mainly designed to have
> multiple users. You just need to look-up the phandle, perhaps check
> the compatible, and call of_address_to_resource to get the address.
> Actually, you could skip the phandle entirely and just find the node
> by compatible (assuming there is only one).
> 

Ahh, right. Thanks for the suggestion.

Regards,
Bjorn

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

[Bjorn Andersson]

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

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
-- 
2.5.0

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

[Andy Gross]

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?


<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

> + * 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

> +		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>

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

[Bjorn Andersson]

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

[PATCH 5/5] ARM: dts: qcom: msm8974: Introduce pronto node

[Bjorn Andersson]

From: Bjorn Andersson <bjorn.andersson@sonymobile.com>

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../boot/dts/qcom-msm8974-sony-xperia-honami.dts   |  4 +++
 arch/arm/boot/dts/qcom-msm8974.dtsi                | 29 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-sony-xperia-honami.dts b/arch/arm/boot/dts/qcom-msm8974-sony-xperia-honami.dts
index a0398b69f4f2..e4e1ff6b0d98 100644
--- a/arch/arm/boot/dts/qcom-msm8974-sony-xperia-honami.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-sony-xperia-honami.dts
@@ -59,6 +59,10 @@
 	};
 
 	smd {
+		pronto {
+			status = "ok";
+		};
+
 		rpm {
 			rpm_requests {
 				pm8841-regulators {
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index ef5330578431..b67963695e95 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -494,6 +494,35 @@
 	smd {
 		compatible = "qcom,smd";
 
+		pronto {
+			status = "disabled";
+			interrupts = <0 142 IRQ_TYPE_EDGE_RISING>;
+
+			qcom,ipc = <&apcs 8 17>;
+			qcom,smd-edge = <6>;
+
+			wcnss {
+				compatible = "qcom,wcnss";
+				qcom,smd-channels = "WCNSS_CTRL";
+
+				bt {
+					compatible = "qcom,btqcomsmd";
+				};
+
+				wifi {
+					compatible = "qcom,wcn3680-wlan";
+
+					interrupts = <0 145 0>, <0 146 0>;
+					interrupt-names = "tx", "rx";
+
+					qcom,wcnss-mmio = <0xfb000000 0x21b000>;
+
+					qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;
+					qcom,state-names = "tx-enable", "tx-rings-empty";
+				};
+			};
+		};
+
 		rpm {
 			interrupts = <0 168 1>;
 			qcom,ipc = <&apcs 8 0>;
-- 
2.5.0