[PATCH 0/2] slimbus: Add QCOM SLIMBus NGD driver

[PATCH 0/2] slimbus: Add QCOM SLIMBus NGD driver

[Srinivas Kandagatla]

This patchset adds support to basic version of Qualcomm NGD SLIMBus
controller driver found SoCs from B family.

This controller is light-weight SLIMBus controller driver responsible for
communicating with slave HW directly over the bus using messaging
interface, and communicating with master component residing on ADSP
for bandwidth and data-channel management.

Tested this patchset on DB820c with WCD9335 codec.
I have pushed my working branch to [1] incase someone want to try.
 

Thanks,
srini

[1] https://git.linaro.org/people/srinivas.kandagatla/linux.git/log/?h=slimbus-ngd

Srinivas Kandagatla (2):
  slimbus: ngd: dt-bindings: Add slim ngd dt bindings
  slimbus: ngd: Add qcom SLIMBus NGD driver

 .../bindings/slimbus/slim-ngd-qcom-ctrl.txt        |   70 ++
 drivers/slimbus/Kconfig                            |   10 +
 drivers/slimbus/Makefile                           |    3 +
 drivers/slimbus/qcom-ngd-ctrl.c                    | 1298 ++++++++++++++++++++
 drivers/slimbus/slimbus.h                          |    8 +
 5 files changed, 1389 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
 create mode 100644 drivers/slimbus/qcom-ngd-ctrl.c

-- 
2.16.2

[PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Srinivas Kandagatla]

This patch adds bindings for Qualcomm SLIMBus NGD controller found in
all new SoCs starting from B family.
SLIMBus NGD controller is a light-weight driver responsible for
communicating with SLIMBus slaves directly over the bus using messaging
interface and communicating with master component residing on ADSP for
bandwidth and data-channel management

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/slimbus/slim-ngd-qcom-ctrl.txt        | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt

diff --git a/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
new file mode 100644
index 000000000000..c948fb098819
--- /dev/null
+++ b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
@@ -0,0 +1,70 @@
+Qualcomm SLIMBus Non Generic Device (NGD) Controller binding
+
+SLIMBus NGD controller is a light-weight driver responsible for communicating
+with SLIMBus slaves directly over the bus using messaging interface and
+communicating with master component residing on ADSP for bandwidth and
+data-channel management
+
+Please refer to slimbus/bus.txt for details of the common SLIMBus bindings.
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must be "qcom,slim-ngd"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: must specify the base address and size of the controller
+		    register blocks.
+
+- reg-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must be "ctrl"
+
+- qcom,ngd-id
+	Usage: required
+	Value type: <u32>
+	Definition: ngd instance id in the controller
+- dmas
+	Usage: required
+	Value type: <array of phandles>
+	Definition: List of rx and tx dma channels
+
+- dma-names
+	Usage: required
+	Value type: <stringlist>
+	Definition: must be "rx" and "tx".
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: must list controller IRQ.
+
+#address-cells
+	Usage: required
+	Refer to slimbus/bus.txt for details of the common SLIMBus bindings.
+
+#size-cells
+	Usage: required
+	Refer to slimbus/bus.txt for details of the common SLIMBus bindings.
+
+= EXAMPLE
+
+slim@91c0000 {
+	compatible = "qcom,slim-ngd";
+	reg = <0x91c0000 0x2C000>;
+	reg-names = "ctrl";
+	interrupts = <0 163 0>;
+	qcom,ngd-id = <1>;
+	dmas =	<&slimbam 3>, <&slimbam 4>;
+	dma-names = "rx", "tx";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+	codec@1 {
+		compatible = "slim217,1a0";
+		reg  = <1 0>;
+	};
+};
-- 
2.16.2

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Trilok Soni]

Hi Srinivas

On 5/16/2018 9:51 AM, Srinivas Kandagatla wrote:
> This patch adds bindings for Qualcomm SLIMBus NGD controller found in
> all new SoCs starting from B family.

"X/Y/Z family" has no meaning here in upstream and just put the 
processor name from which you are adding the support or tested to start 
with.


-- 
---Trilok Soni
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Srinivas Kandagatla]

On 18/05/18 21:47, Trilok Soni wrote:
> Hi Srinivas
> 
> On 5/16/2018 9:51 AM, Srinivas Kandagatla wrote:
>> This patch adds bindings for Qualcomm SLIMBus NGD controller found in
>> all new SoCs starting from B family.
> 
> "X/Y/Z family" has no meaning here in upstream and just put the 
> processor name from which you are adding the support or tested to start 
> with.

Thanks, Will update this when I send new version of this patch.

thanks,
srini
> 
> 

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Rob Herring]

On Wed, May 16, 2018 at 05:51:17PM +0100, Srinivas Kandagatla wrote:
> This patch adds bindings for Qualcomm SLIMBus NGD controller found in
> all new SoCs starting from B family.
> SLIMBus NGD controller is a light-weight driver responsible for
> communicating with SLIMBus slaves directly over the bus using messaging
> interface and communicating with master component residing on ADSP for
> bandwidth and data-channel management
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../bindings/slimbus/slim-ngd-qcom-ctrl.txt        | 70 ++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
> new file mode 100644
> index 000000000000..c948fb098819
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
> @@ -0,0 +1,70 @@
> +Qualcomm SLIMBus Non Generic Device (NGD) Controller binding
> +
> +SLIMBus NGD controller is a light-weight driver responsible for communicating
> +with SLIMBus slaves directly over the bus using messaging interface and
> +communicating with master component residing on ADSP for bandwidth and
> +data-channel management
> +
> +Please refer to slimbus/bus.txt for details of the common SLIMBus bindings.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be "qcom,slim-ngd"

SoC specific compatible needed.

> +
> +- reg:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: must specify the base address and size of the controller
> +		    register blocks.

blocks? Is there more than one? If so, how many?

> +
> +- reg-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be "ctrl"

reg-names is pointless when there is only 1.

> +
> +- qcom,ngd-id
> +	Usage: required
> +	Value type: <u32>
> +	Definition: ngd instance id in the controller

Why do you need this?

> +- dmas
> +	Usage: required
> +	Value type: <array of phandles>
> +	Definition: List of rx and tx dma channels
> +
> +- dma-names
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be "rx" and "tx".
> +
> +- interrupts:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: must list controller IRQ.
> +
> +#address-cells
> +	Usage: required
> +	Refer to slimbus/bus.txt for details of the common SLIMBus bindings.
> +
> +#size-cells
> +	Usage: required
> +	Refer to slimbus/bus.txt for details of the common SLIMBus bindings.
> +
> += EXAMPLE
> +
> +slim@91c0000 {
> +	compatible = "qcom,slim-ngd";
> +	reg = <0x91c0000 0x2C000>;
> +	reg-names = "ctrl";
> +	interrupts = <0 163 0>;
> +	qcom,ngd-id = <1>;
> +	dmas =	<&slimbam 3>, <&slimbam 4>;
> +	dma-names = "rx", "tx";
> +
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	codec@1 {
> +		compatible = "slim217,1a0";
> +		reg  = <1 0>;
> +	};
> +};
> -- 
> 2.16.2
> 

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Srinivas Kandagatla]

Thanks Rob for review,

On 23/05/18 17:40, Rob Herring wrote:
> On Wed, May 16, 2018 at 05:51:17PM +0100, Srinivas Kandagatla wrote:
>> This patch adds bindings for Qualcomm SLIMBus NGD controller found in
>> all new SoCs starting from B family.
>> SLIMBus NGD controller is a light-weight driver responsible for
>> communicating with SLIMBus slaves directly over the bus using messaging
>> interface and communicating with master component residing on ADSP for
>> bandwidth and data-channel management
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   .../bindings/slimbus/slim-ngd-qcom-ctrl.txt        | 70 ++++++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
>>
>> diff --git a/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
>> new file mode 100644
>> index 000000000000..c948fb098819
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt
>> @@ -0,0 +1,70 @@
>> +Qualcomm SLIMBus Non Generic Device (NGD) Controller binding
>> +
>> +SLIMBus NGD controller is a light-weight driver responsible for communicating
>> +with SLIMBus slaves directly over the bus using messaging interface and
>> +communicating with master component residing on ADSP for bandwidth and
>> +data-channel management
>> +
>> +Please refer to slimbus/bus.txt for details of the common SLIMBus bindings.
>> +
>> +- compatible:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: must be "qcom,slim-ngd"
> 
> SoC specific compatible needed.

Yes, I will add that in v2.
> 
>> +
>> +- reg:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: must specify the base address and size of the controller
>> +		    register blocks.
> 
> blocks? Is there more than one? If so, how many?
Its just one. I will fix the text to reflect this.

> 
>> +
>> +- reg-names:
>> +	Usage: required
>> +	Value type: <stringlist>
>> +	Definition: must be "ctrl"
> 
> reg-names is pointless when there is only 1.
> 
>> +
>> +- qcom,ngd-id
>> +	Usage: required
>> +	Value type: <u32>
>> +	Definition: ngd instance id in the controller
> 
> Why do you need this?
I have removed this totally in my next version, which I will be posting 
soon.

Thanks,
srini

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Srinivas Kandagatla]

On 23/05/18 17:40, Rob Herring wrote:
>> +
>> +- qcom,ngd-id
>> +	Usage: required
>> +	Value type: <u32>
>> +	Definition: ngd instance id in the controller
> Why do you need this?
> 
Please ignore my comment from previous reply.

There are more than one instances of ngd in this slim controller.
We need this to make sure we are programming the correct one.

We also need this instance ID during powering it up using QMI.


thanks,
srini

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Rob Herring]

On Wed, May 23, 2018 at 1:11 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
>
> On 23/05/18 17:40, Rob Herring wrote:
>>>
>>> +
>>> +- qcom,ngd-id
>>> +       Usage: required
>>> +       Value type: <u32>
>>> +       Definition: ngd instance id in the controller
>>
>> Why do you need this?
>>
> Please ignore my comment from previous reply.
>
> There are more than one instances of ngd in this slim controller.
> We need this to make sure we are programming the correct one.

Doesn't the parent-child relationship of devices on the bus provide
that? If you mean to provide consistent numbering to userspace, then
that's not a DT problem (nor one that Linux plans to solve).

> We also need this instance ID during powering it up using QMI.

Wouldn't that be a QMI ID?

Rob

Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings

[Srinivas Kandagatla]

On 23/05/18 20:28, Rob Herring wrote:
> On Wed, May 23, 2018 at 1:11 PM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>>
>>
>> On 23/05/18 17:40, Rob Herring wrote:
>>>>
>>>> +
>>>> +- qcom,ngd-id
>>>> +       Usage: required
>>>> +       Value type: <u32>
>>>> +       Definition: ngd instance id in the controller
>>>
>>> Why do you need this?
>>>
>> Please ignore my comment from previous reply.
>>
>> There are more than one instances of ngd in this slim controller.
>> We need this to make sure we are programming the correct one.
> 
> Doesn't the parent-child relationship of devices on the bus provide
> that? 
Thanks for the hint, that sounds like the actual problem here,
If I represent the node with proper parent-child relationship like this, 
it will remove the need of this property and would work perfectly in 
case we want to support multiple ngds in future!

slim@91c0000 {
	compatible = "qcom,msm8996-slim";
	reg = <0x91c0000 0x2C000>;
	interrupts = <0 163 0>;
	dmas = <&slimbam 3>, <&slimbam 4>;
	dma-names = "rx", "tx";
	#address-cells = <1>;
	#size-cells = <1>;
	ngd@1 {
		reg  = <1>;
		#address-cells = <1>;
		#size-cells = <1>;
		codec@1 {
			compatible = "slim217,1a0";
			reg  = <1 0>;
		};	
	};
};



If you mean to provide consistent numbering to userspace, then
> that's not a DT problem (nor one that Linux plans to solve).
> 
No, this is not problem am trying to solve.

>> We also need this instance ID during powering it up using QMI.
> 
> Wouldn't that be a QMI ID?

It is passed as parameter to SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01 request.

thanks,
srini
> 
> Rob
> 

[PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver

[Srinivas Kandagatla]

This patch adds suppor to Qualcomm SLIMBus Non-Generic Device (NGD)
controller driver.
This is light-weight SLIMBus controller driver responsible for
communicating with slave HW directly over the bus using messaging
interface, and communicating with master component residing on ADSP
for bandwidth and data-channel management

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/Kconfig         |   10 +
 drivers/slimbus/Makefile        |    3 +
 drivers/slimbus/qcom-ngd-ctrl.c | 1298 +++++++++++++++++++++++++++++++++++++++
 drivers/slimbus/slimbus.h       |    8 +
 4 files changed, 1319 insertions(+)
 create mode 100644 drivers/slimbus/qcom-ngd-ctrl.c

diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
index 1a632fad597e..8c5d81c5d73b 100644
--- a/drivers/slimbus/Kconfig
+++ b/drivers/slimbus/Kconfig
@@ -21,4 +21,14 @@ config SLIM_QCOM_CTRL
 	  Select driver if Qualcomm's SLIMbus Manager Component is
 	  programmed using Linux kernel.
 
+config SLIM_QCOM_NGD_CTRL
+	tristate "Qualcomm SLIMbus Satellite Non-Generic Device Component"
+	depends on QCOM_QMI_HELPERS
+	help
+	  Select driver if Qualcomm's SLIMbus Satellite Non-Generic Device
+	  Component is programmed using Linux kernel.
+	  This is light-weight slimbus controller driver responsible for
+	  communicating with slave HW directly over the bus using messaging
+	  interface, and communicating with master component residing on ADSP
+	  for bandwidth and data-channel management.
 endif
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
index a35a3da4eb78..c78c6e16c2df 100644
--- a/drivers/slimbus/Makefile
+++ b/drivers/slimbus/Makefile
@@ -8,3 +8,6 @@ slimbus-y				:= core.o messaging.o sched.o
 #Controllers
 obj-$(CONFIG_SLIM_QCOM_CTRL)		+= slim-qcom-ctrl.o
 slim-qcom-ctrl-y			:= qcom-ctrl.o
+
+obj-$(CONFIG_SLIM_QCOM_NGD_CTRL)	+= slim-qcom-ngd-ctrl.o
+slim-qcom-ngd-ctrl-y			:= qcom-ngd-ctrl.o
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
new file mode 100644
index 000000000000..a74bbf3b08df
--- /dev/null
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -0,0 +1,1298 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2011-2017, The Linux Foundation. All rights reserved.
+// Copyright (c) 2018, Linaro Limited
+
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/slimbus.h>
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/soc/qcom/qmi.h>
+#include <net/sock.h>
+#include "slimbus.h"
+
+#define NGD_BASE_V1(r)	(((r) % 2) ? 0x800 : 0xA00)
+#define NGD_BASE_V2(r)	(((r) % 2) ? 0x1000 : 0x2000)
+#define NGD_BASE(r, v) ((v) ? NGD_BASE_V2(r) : NGD_BASE_V1(r))
+
+/* NGD (Non-ported Generic Device) registers */
+#define	NGD_CFG			0x0
+#define	NGD_CFG_ENABLE		BIT(0)
+#define	NGD_CFG_RX_MSGQ_EN	BIT(1)
+#define	NGD_CFG_TX_MSGQ_EN	BIT(2)
+#define	NGD_STATUS		0x4
+#define NGD_LADDR		BIT(1)
+#define	NGD_RX_MSGQ_CFG		0x8
+#define	NGD_INT_EN		0x10
+#define	NGD_INT_RECFG_DONE	BIT(24)
+#define	NGD_INT_TX_NACKED_2	BIT(25)
+#define	NGD_INT_MSG_BUF_CONTE	BIT(26)
+#define	NGD_INT_MSG_TX_INVAL	BIT(27)
+#define	NGD_INT_IE_VE_CHG	BIT(28)
+#define	NGD_INT_DEV_ERR		BIT(29)
+#define	NGD_INT_RX_MSG_RCVD	BIT(30)
+#define	NGD_INT_TX_MSG_SENT	BIT(31)
+#define	NGD_INT_STAT		0x14
+#define	NGD_INT_CLR		0x18
+#define DEF_NGD_INT_MASK (NGD_INT_TX_NACKED_2 | NGD_INT_MSG_BUF_CONTE | \
+				NGD_INT_MSG_TX_INVAL | NGD_INT_IE_VE_CHG | \
+				NGD_INT_DEV_ERR | NGD_INT_TX_MSG_SENT | \
+				NGD_INT_RX_MSG_RCVD)
+
+/* Slimbus QMI service */
+#define SLIMBUS_QMI_SVC_ID 0x0301
+#define SLIMBUS_QMI_SVC_V1 1
+#define SLIMBUS_QMI_INS_ID 0
+#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01 0x0020
+#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_V01 0x0020
+#define SLIMBUS_QMI_POWER_REQ_V01 0x0021
+#define SLIMBUS_QMI_POWER_RESP_V01 0x0021
+#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_REQ 0x0022
+#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_RESP 0x0022
+#define SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN 14
+#define SLIMBUS_QMI_POWER_RESP_MAX_MSG_LEN 7
+#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_MAX_MSG_LEN 14
+#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_MAX_MSG_LEN 7
+#define SLIMBUS_QMI_CHECK_FRAMER_STAT_RESP_MAX_MSG_LEN 7
+/* QMI response timeout of 500ms */
+#define SLIMBUS_QMI_RESP_TOUT 1000
+
+/* User defined commands */
+#define SLIM_USR_MC_GENERIC_ACK		0x25
+#define SLIM_USR_MC_MASTER_CAPABILITY	0x0
+#define SLIM_USR_MC_REPORT_SATELLITE	0x1
+#define SLIM_USR_MC_ADDR_QUERY		0xD
+#define SLIM_USR_MC_ADDR_REPLY		0xE
+#define SLIM_USR_MC_DEFINE_CHAN		0x20
+#define SLIM_USR_MC_DEF_ACT_CHAN	0x21
+#define SLIM_USR_MC_CHAN_CTRL		0x23
+#define SLIM_USR_MC_RECONFIG_NOW	0x24
+#define SLIM_USR_MC_REQ_BW		0x28
+#define SLIM_USR_MC_CONNECT_SRC		0x2C
+#define SLIM_USR_MC_CONNECT_SINK	0x2D
+#define SLIM_USR_MC_DISCONNECT_PORT	0x2E
+#define SLIM_USR_MC_REPEAT_CHANGE_VALUE	0x0
+
+#define QCOM_SLIM_NGD_AUTOSUSPEND	MSEC_PER_SEC
+#define SLIM_RX_MSGQ_TIMEOUT_VAL	0x10000
+
+#define SLIM_LA_MGR	0xFF
+#define SLIM_ROOT_FREQ	24576000
+#define LADDR_RETRY	5
+
+/* Per spec.max 40 bytes per received message */
+#define SLIM_MSGQ_BUF_LEN	40
+#define QCOM_SLIM_NGD_DESC_NUM	32
+
+#define SLIM_MSG_ASM_FIRST_WORD(l, mt, mc, dt, ad) \
+		((l) | ((mt) << 5) | ((mc) << 8) | ((dt) << 15) | ((ad) << 16))
+
+#define INIT_MX_RETRIES 10
+#define DEF_RETRY_MS	10
+#define SAT_MAGIC_LSB	0xD9
+#define SAT_MAGIC_MSB	0xC5
+#define SAT_MSG_VER	0x1
+#define SAT_MSG_PROT	0x1
+
+enum qcom_slim_ngd_state {
+	QCOM_SLIM_NGD_CTRL_AWAKE,
+	QCOM_SLIM_NGD_CTRL_IDLE,
+	QCOM_SLIM_NGD_CTRL_ASLEEP,
+	QCOM_SLIM_NGD_CTRL_DOWN,
+};
+
+struct qcom_slim_ngd_qmi {
+	struct qmi_handle qmi;
+	struct sockaddr_qrtr svc_info;
+	struct qmi_handle svc_event_hdl;
+	struct qmi_response_type_v01 resp;
+	struct qmi_handle *handle;
+	struct completion qmi_comp;
+};
+
+struct qcom_slim_ngd_ctrl;
+
+struct qcom_slim_ngd_dma_desc {
+	struct dma_async_tx_descriptor *desc;
+	struct qcom_slim_ngd_ctrl *ctrl;
+	struct completion *comp;
+	dma_cookie_t cookie;
+	dma_addr_t phys;
+	void *base;
+};
+
+struct qcom_slim_ngd_ctrl {
+	struct slim_controller ctrl;
+	struct slim_framer framer;
+	struct qcom_slim_ngd_qmi qmi;
+	struct device *dev;
+	void __iomem *base;
+#ifdef CONFIG_DMA_ENGINE
+	/* DMA stuff */
+	struct dma_chan *dma_rx_channel;
+	struct dma_chan	*dma_tx_channel;
+	struct qcom_slim_ngd_dma_desc rx_desc[QCOM_SLIM_NGD_DESC_NUM];
+	struct qcom_slim_ngd_dma_desc txdesc[QCOM_SLIM_NGD_DESC_NUM];
+#endif
+	struct completion reconf;
+	struct work_struct m_work;
+	struct workqueue_struct *mwq;
+	spinlock_t tx_buf_lock;
+	enum qcom_slim_ngd_state state;
+	dma_addr_t rx_phys_base;
+	dma_addr_t tx_phys_base;
+	void *rx_base;
+	void *tx_base;
+	int tx_tail;
+	int tx_head;
+	int id;
+	u32 ver;
+};
+
+enum slimbus_mode_enum_type_v01 {
+	/* To force a 32 bit signed enum. Do not change or use*/
+	SLIMBUS_MODE_ENUM_TYPE_MIN_ENUM_VAL_V01 = INT_MIN,
+	SLIMBUS_MODE_SATELLITE_V01 = 1,
+	SLIMBUS_MODE_MASTER_V01 = 2,
+	SLIMBUS_MODE_ENUM_TYPE_MAX_ENUM_VAL_V01 = INT_MAX,
+};
+
+enum slimbus_pm_enum_type_v01 {
+	/* To force a 32 bit signed enum. Do not change or use*/
+	SLIMBUS_PM_ENUM_TYPE_MIN_ENUM_VAL_V01 = INT_MIN,
+	SLIMBUS_PM_INACTIVE_V01 = 1,
+	SLIMBUS_PM_ACTIVE_V01 = 2,
+	SLIMBUS_PM_ENUM_TYPE_MAX_ENUM_VAL_V01 = INT_MAX,
+};
+
+enum slimbus_resp_enum_type_v01 {
+	SLIMBUS_RESP_ENUM_TYPE_MIN_VAL_V01 = INT_MIN,
+	SLIMBUS_RESP_SYNCHRONOUS_V01 = 1,
+	SLIMBUS_RESP_ENUM_TYPE_MAX_VAL_V01 = INT_MAX,
+};
+
+struct slimbus_select_inst_req_msg_v01 {
+	uint32_t instance;
+	uint8_t mode_valid;
+	enum slimbus_mode_enum_type_v01 mode;
+};
+
+struct slimbus_select_inst_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+};
+
+struct slimbus_power_req_msg_v01 {
+	enum slimbus_pm_enum_type_v01 pm_req;
+	uint8_t resp_type_valid;
+	enum slimbus_resp_enum_type_v01 resp_type;
+};
+
+struct slimbus_power_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+};
+
+static struct qmi_elem_info slimbus_select_inst_req_msg_v01_ei[] = {
+	{
+		.data_type  = QMI_UNSIGNED_4_BYTE,
+		.elem_len   = 1,
+		.elem_size  = sizeof(uint32_t),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x01,
+		.offset     = offsetof(struct slimbus_select_inst_req_msg_v01,
+				       instance),
+		.ei_array   = NULL,
+	},
+	{
+		.data_type  = QMI_OPT_FLAG,
+		.elem_len   = 1,
+		.elem_size  = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x10,
+		.offset     = offsetof(struct slimbus_select_inst_req_msg_v01,
+				       mode_valid),
+		.ei_array   = NULL,
+	},
+	{
+		.data_type  = QMI_UNSIGNED_4_BYTE,
+		.elem_len   = 1,
+		.elem_size  = sizeof(enum slimbus_mode_enum_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x10,
+		.offset     = offsetof(struct slimbus_select_inst_req_msg_v01,
+				       mode),
+		.ei_array   = NULL,
+	},
+	{
+		.data_type  = QMI_EOTI,
+		.elem_len   = 0,
+		.elem_size  = 0,
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x00,
+		.offset     = 0,
+		.ei_array   = NULL,
+	},
+};
+
+static struct qmi_elem_info slimbus_select_inst_resp_msg_v01_ei[] = {
+	{
+		.data_type  = QMI_STRUCT,
+		.elem_len   = 1,
+		.elem_size  = sizeof(struct qmi_response_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x02,
+		.offset     = offsetof(struct slimbus_select_inst_resp_msg_v01,
+				       resp),
+		.ei_array   = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type  = QMI_EOTI,
+		.elem_len   = 0,
+		.elem_size  = 0,
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x00,
+		.offset     = 0,
+		.ei_array   = NULL,
+	},
+};
+
+static struct qmi_elem_info slimbus_power_req_msg_v01_ei[] = {
+	{
+		.data_type  = QMI_UNSIGNED_4_BYTE,
+		.elem_len   = 1,
+		.elem_size  = sizeof(enum slimbus_pm_enum_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x01,
+		.offset     = offsetof(struct slimbus_power_req_msg_v01,
+				       pm_req),
+		.ei_array   = NULL,
+	},
+	{
+		.data_type  = QMI_OPT_FLAG,
+		.elem_len   = 1,
+		.elem_size  = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x10,
+		.offset     = offsetof(struct slimbus_power_req_msg_v01,
+				       resp_type_valid),
+	},
+	{
+		.data_type  = QMI_SIGNED_4_BYTE_ENUM,
+		.elem_len   = 1,
+		.elem_size  = sizeof(enum slimbus_resp_enum_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x10,
+		.offset     = offsetof(struct slimbus_power_req_msg_v01,
+				       resp_type),
+	},
+	{
+		.data_type  = QMI_EOTI,
+		.elem_len   = 0,
+		.elem_size  = 0,
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x00,
+		.offset     = 0,
+		.ei_array   = NULL,
+	},
+};
+
+static struct qmi_elem_info slimbus_power_resp_msg_v01_ei[] = {
+	{
+		.data_type  = QMI_STRUCT,
+		.elem_len   = 1,
+		.elem_size  = sizeof(struct qmi_response_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x02,
+		.offset     = offsetof(struct slimbus_power_resp_msg_v01, resp),
+		.ei_array   = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type  = QMI_EOTI,
+		.elem_len   = 0,
+		.elem_size  = 0,
+		.array_type = NO_ARRAY,
+		.tlv_type   = 0x00,
+		.offset     = 0,
+		.ei_array   = NULL,
+	},
+};
+
+static int qcom_slim_qmi_send_select_inst_req(struct qcom_slim_ngd_ctrl *ctrl,
+				struct slimbus_select_inst_req_msg_v01 *req)
+{
+	struct slimbus_select_inst_resp_msg_v01 resp = { { 0, 0 } };
+	struct qmi_txn txn;
+	int rc;
+
+	rc = qmi_txn_init(ctrl->qmi.handle, &txn,
+				slimbus_select_inst_resp_msg_v01_ei, &resp);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: QMI TXN init fail: %d\n", __func__, rc);
+		return rc;
+	}
+
+	rc = qmi_send_request(ctrl->qmi.handle, NULL, &txn,
+				SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01,
+				SLIMBUS_QMI_SELECT_INSTANCE_REQ_MAX_MSG_LEN,
+				slimbus_select_inst_req_msg_v01_ei, req);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: QMI send req fail %d\n", __func__, rc);
+		qmi_txn_cancel(&txn);
+		return rc;
+	}
+
+	rc = qmi_txn_wait(&txn, SLIMBUS_QMI_RESP_TOUT);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: QMI TXN wait fail: %d\n", __func__, rc);
+		return rc;
+	}
+	/* Check the response */
+	if (resp.resp.result != QMI_RESULT_SUCCESS_V01) {
+		dev_err(ctrl->dev, "%s: QMI request failed 0x%x\n",
+			__func__, resp.resp.result);
+		return -EREMOTEIO;
+	}
+
+	return 0;
+}
+
+static void qcom_slim_qmi_power_resp_cb(struct qmi_handle *handle,
+					struct sockaddr_qrtr *sq,
+					struct qmi_txn *txn, const void *data)
+{
+	struct slimbus_power_resp_msg_v01 *resp;
+
+	resp = (struct slimbus_power_resp_msg_v01 *)data;
+	if (resp->resp.result != QMI_RESULT_SUCCESS_V01)
+		pr_err("%s: QMI power request failed 0x%x\n", __func__,
+				resp->resp.result);
+
+	complete(&txn->completion);
+}
+
+static int qcom_slim_qmi_send_power_request(struct qcom_slim_ngd_ctrl *ctrl,
+				struct slimbus_power_req_msg_v01 *req)
+{
+	struct slimbus_power_resp_msg_v01 resp = { { 0, 0 } };
+	struct qmi_txn txn;
+	int rc;
+
+	rc = qmi_txn_init(ctrl->qmi.handle, &txn,
+				slimbus_power_resp_msg_v01_ei, &resp);
+
+	rc = qmi_send_request(ctrl->qmi.handle, NULL, &txn,
+				SLIMBUS_QMI_POWER_REQ_V01,
+				SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN,
+				slimbus_power_req_msg_v01_ei, req);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: QMI send req fail %d\n", __func__, rc);
+		qmi_txn_cancel(&txn);
+	}
+
+	if (rc < 0)
+		return rc;
+
+	rc = qmi_txn_wait(&txn, SLIMBUS_QMI_RESP_TOUT);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: QMI TXN wait fail: %d\n", __func__, rc);
+		return rc;
+	}
+
+	/* Check the response */
+	if (resp.resp.result != QMI_RESULT_SUCCESS_V01) {
+		dev_err(ctrl->dev, "%s: QMI request failed 0x%x\n",
+			__func__, resp.resp.result);
+		return -EREMOTEIO;
+	}
+
+	return 0;
+}
+
+static struct qmi_msg_handler qcom_slim_qmi_msg_handlers[] = {
+	{
+		.type = QMI_RESPONSE,
+		.msg_id = SLIMBUS_QMI_POWER_RESP_V01,
+		.ei = slimbus_power_resp_msg_v01_ei,
+		.decoded_size = sizeof(struct slimbus_power_resp_msg_v01),
+		.fn = qcom_slim_qmi_power_resp_cb,
+	},
+	{}
+};
+
+static int qcom_slim_qmi_init(struct qcom_slim_ngd_ctrl *ctrl,
+			      bool apps_is_master)
+{
+	int rc = 0;
+	struct qmi_handle *handle;
+	struct slimbus_select_inst_req_msg_v01 req;
+
+	handle = devm_kzalloc(ctrl->dev, sizeof(*handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	rc = qmi_handle_init(handle, SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN,
+				NULL, qcom_slim_qmi_msg_handlers);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "QMI client init failed: %d\n", rc);
+		goto qmi_handle_init_failed;
+	}
+
+	rc = kernel_connect(handle->sock,
+				(struct sockaddr *)&ctrl->qmi.svc_info,
+				sizeof(ctrl->qmi.svc_info), 0);
+	if (rc < 0) {
+		dev_err(ctrl->dev, "%s: Remote Service connect failed: %d\n",
+								__func__, rc);
+		goto qmi_connect_to_service_failed;
+	}
+
+	/* Instance is 0 based */
+	req.instance = (ctrl->id >> 1);
+	req.mode_valid = 1;
+
+	/* Mode indicates the role of the ADSP */
+	if (apps_is_master)
+		req.mode = SLIMBUS_MODE_SATELLITE_V01;
+	else
+		req.mode = SLIMBUS_MODE_MASTER_V01;
+
+	ctrl->qmi.handle = handle;
+
+	rc = qcom_slim_qmi_send_select_inst_req(ctrl, &req);
+	if (rc) {
+		dev_err(ctrl->dev, "failed to select h/w instance\n");
+		goto qmi_select_instance_failed;
+	}
+
+	return 0;
+
+qmi_select_instance_failed:
+	ctrl->qmi.handle = NULL;
+qmi_connect_to_service_failed:
+	qmi_handle_release(handle);
+qmi_handle_init_failed:
+	devm_kfree(ctrl->dev, handle);
+	return rc;
+}
+
+static void qcom_slim_qmi_exit(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	if (!ctrl->qmi.handle)
+		return;
+
+	qmi_handle_release(ctrl->qmi.handle);
+	devm_kfree(ctrl->dev, ctrl->qmi.handle);
+	ctrl->qmi.handle = NULL;
+}
+
+static int qcom_slim_qmi_power_request(struct qcom_slim_ngd_ctrl *ctrl,
+				       bool active)
+{
+	struct slimbus_power_req_msg_v01 req;
+
+	if (active)
+		req.pm_req = SLIMBUS_PM_ACTIVE_V01;
+	else
+		req.pm_req = SLIMBUS_PM_INACTIVE_V01;
+
+	req.resp_type_valid = 0;
+
+	return qcom_slim_qmi_send_power_request(ctrl, &req);
+}
+
+static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
+				     struct completion *comp)
+{
+	struct qcom_slim_ngd_dma_desc *desc;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
+
+	if ((ctrl->tx_tail + 1) % QCOM_SLIM_NGD_DESC_NUM == ctrl->tx_head) {
+		spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+		return NULL;
+	}
+	desc  = &ctrl->txdesc[ctrl->tx_tail];
+	desc->base = (u32 *)((u8 *)ctrl->tx_base +
+				(ctrl->tx_tail * SLIM_MSGQ_BUF_LEN));
+	desc->comp = comp;
+	ctrl->tx_tail = (ctrl->tx_tail + 1) % QCOM_SLIM_NGD_DESC_NUM;
+
+	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+
+	return desc->base;
+}
+
+static void qcom_slim_ngd_tx_msg_dma_cb(void *args)
+{
+	struct qcom_slim_ngd_dma_desc *desc = args;
+	struct qcom_slim_ngd_ctrl *ctrl = desc->ctrl;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
+
+	if (desc->comp) {
+		complete(desc->comp);
+		desc->comp = NULL;
+	}
+
+	ctrl->tx_head = (ctrl->tx_head + 1) % QCOM_SLIM_NGD_DESC_NUM;
+	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+}
+
+static int qcom_slim_ngd_tx_msg_post(struct qcom_slim_ngd_ctrl *ctrl,
+				     void *buf, int len)
+{
+	struct qcom_slim_ngd_dma_desc *desc;
+	struct dma_slave_config conf = {
+		.direction = DMA_MEM_TO_DEV,
+	};
+	unsigned long flags;
+	int index, offset;
+
+	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
+	offset = buf - ctrl->tx_base;
+	index = offset/SLIM_MSGQ_BUF_LEN;
+
+	desc = &ctrl->txdesc[index];
+	desc->phys = ctrl->tx_phys_base + offset;
+	desc->base = ctrl->tx_base + offset;
+	desc->ctrl = ctrl;
+	len = (len + 3) & 0xfc;
+
+	dmaengine_slave_config(ctrl->dma_tx_channel, &conf);
+	desc->desc = dmaengine_prep_slave_single(ctrl->dma_tx_channel,
+						desc->phys, len,
+						DMA_MEM_TO_DEV,
+						DMA_PREP_INTERRUPT);
+	if (!desc->desc) {
+		dev_err(ctrl->dev, "unable to prepare channel\n");
+		spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+		return -EINVAL;
+	}
+
+	desc->desc->callback = qcom_slim_ngd_tx_msg_dma_cb;
+	desc->desc->callback_param = desc;
+	desc->desc->cookie = dmaengine_submit(desc->desc);
+	dma_async_issue_pending(ctrl->dma_tx_channel);
+	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+
+	return 0;
+}
+
+static void qcom_slim_ngd_rx(struct qcom_slim_ngd_ctrl *ctrl, u8 *buf)
+{
+	u8 mc, mt, len;
+
+	mt = SLIM_HEADER_GET_MT(buf[0]);
+	len = SLIM_HEADER_GET_RL(buf[0]);
+	mc = SLIM_HEADER_GET_MC(buf[1]);
+
+	if (mc == SLIM_USR_MC_MASTER_CAPABILITY &&
+		mt == SLIM_MSG_MT_SRC_REFERRED_USER)
+		queue_work(ctrl->mwq, &ctrl->m_work);
+
+	if (mc == SLIM_MSG_MC_REPLY_INFORMATION ||
+	    mc == SLIM_MSG_MC_REPLY_VALUE || (mc == SLIM_USR_MC_ADDR_REPLY &&
+	    mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
+		slim_msg_response(&ctrl->ctrl, &buf[4], buf[3], len - 4);
+		pm_runtime_mark_last_busy(ctrl->dev);
+	}
+}
+
+static void qcom_slim_ngd_rx_msgq_cb(void *args)
+{
+	struct qcom_slim_ngd_dma_desc *desc = args;
+	struct qcom_slim_ngd_ctrl *ctrl = desc->ctrl;
+
+	qcom_slim_ngd_rx(ctrl, (u8 *)desc->base);
+	/* Add descriptor back to the queue */
+	desc->desc = dmaengine_prep_slave_single(ctrl->dma_rx_channel,
+					desc->phys, SLIM_MSGQ_BUF_LEN,
+					DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT);
+	if (!desc->desc) {
+		dev_err(ctrl->dev, "Unable to preppare rx channel\n");
+		return;
+	}
+
+	desc->desc->callback = qcom_slim_ngd_rx_msgq_cb;
+	desc->desc->callback_param = desc;
+	desc->desc->cookie = dmaengine_submit(desc->desc);
+	dma_async_issue_pending(ctrl->dma_rx_channel);
+}
+
+static int qcom_slim_ngd_post_rx_msgq(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	struct qcom_slim_ngd_dma_desc *desc;
+	struct dma_slave_config conf = {
+		.direction = DMA_DEV_TO_MEM,
+	};
+	int ret, i;
+
+	ret = dmaengine_slave_config(ctrl->dma_rx_channel, &conf);
+	if (ret)
+		dev_err(ctrl->dev, "Error Configuring rx dma\n");
+
+	for (i = 0; i < QCOM_SLIM_NGD_DESC_NUM; i++) {
+		desc = &ctrl->rx_desc[i];
+		desc->phys = ctrl->rx_phys_base + i * SLIM_MSGQ_BUF_LEN;
+		desc->ctrl = ctrl;
+		desc->base = ctrl->rx_base + i * SLIM_MSGQ_BUF_LEN;
+		desc->desc = dmaengine_prep_slave_single(ctrl->dma_rx_channel,
+						desc->phys, SLIM_MSGQ_BUF_LEN,
+						DMA_DEV_TO_MEM,
+						DMA_PREP_INTERRUPT);
+	if (!desc->desc) {
+		dev_err(ctrl->dev, "Unable to preppare rx channel\n");
+		return -EINVAL;
+	}
+
+		desc->desc->callback = qcom_slim_ngd_rx_msgq_cb;
+		desc->desc->callback_param = desc;
+		desc->desc->cookie = dmaengine_submit(desc->desc);
+	}
+	dma_async_issue_pending(ctrl->dma_rx_channel);
+
+	return 0;
+}
+
+static int qcom_slim_ngd_init_rx_msgq(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	struct device *dev = ctrl->dev;
+	int ret, size;
+
+	ctrl->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+	if (!ctrl->dma_rx_channel) {
+		dev_err(dev, "Failed to request dma channels");
+		return -EINVAL;
+	}
+
+	size = QCOM_SLIM_NGD_DESC_NUM * SLIM_MSGQ_BUF_LEN;
+	ctrl->rx_base = dma_alloc_coherent(dev, size, &ctrl->rx_phys_base,
+					   GFP_KERNEL);
+	if (!ctrl->rx_base) {
+		dev_err(dev, "dma_alloc_coherent failed\n");
+		ret = -ENOMEM;
+		goto rel_rx;
+	}
+
+	ret = qcom_slim_ngd_post_rx_msgq(ctrl);
+	if (ret) {
+		dev_err(dev, "post_rx_msgq() failed 0x%x\n", ret);
+		goto rx_post_err;
+	}
+
+	return 0;
+
+rx_post_err:
+	dma_free_coherent(dev, size, ctrl->rx_base, ctrl->rx_phys_base);
+rel_rx:
+	dma_release_channel(ctrl->dma_rx_channel);
+	return ret;
+}
+
+static int qcom_slim_ngd_init_tx_msgq(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	struct device *dev = ctrl->dev;
+	unsigned long flags;
+	int ret = 0;
+	int size;
+
+	ctrl->dma_tx_channel = dma_request_slave_channel(dev, "tx");
+	if (!ctrl->dma_tx_channel) {
+		dev_err(dev, "Failed to request dma channels");
+		return -EINVAL;
+	}
+
+	size = ((QCOM_SLIM_NGD_DESC_NUM + 1) * SLIM_MSGQ_BUF_LEN);
+	ctrl->tx_base = dma_alloc_coherent(dev, size, &ctrl->tx_phys_base,
+					   GFP_KERNEL);
+	if (!ctrl->tx_base) {
+		dev_err(dev, "dma_alloc_coherent failed\n");
+		ret = -EINVAL;
+		goto rel_tx;
+	}
+
+	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
+	ctrl->tx_tail = 0;
+	ctrl->tx_head = 0;
+	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+
+	return 0;
+rel_tx:
+	dma_release_channel(ctrl->dma_tx_channel);
+	return ret;
+}
+
+static int qcom_slim_ngd_init_dma(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	int ret = 0;
+
+	ret = qcom_slim_ngd_init_rx_msgq(ctrl);
+	if (ret) {
+		dev_err(ctrl->dev, "rx dma init failed\n");
+		return ret;
+	}
+
+	ret = qcom_slim_ngd_init_tx_msgq(ctrl);
+	if (ret)
+		dev_err(ctrl->dev, "tx dma init failed\n");
+
+	return ret;
+}
+
+static irqreturn_t qcom_slim_ngd_interrupt(int irq, void *d)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = d;
+	void __iomem *ngd = ctrl->base + NGD_BASE(ctrl->id, ctrl->ver);
+	u32 stat = readl(ngd + NGD_INT_STAT);
+
+	if ((stat & NGD_INT_MSG_BUF_CONTE) ||
+		(stat & NGD_INT_MSG_TX_INVAL) || (stat & NGD_INT_DEV_ERR) ||
+		(stat & NGD_INT_TX_NACKED_2)) {
+		dev_err(ctrl->dev, "Error Interrupt received 0x%x\n", stat);
+	}
+
+	writel(stat, ngd + NGD_INT_CLR);
+
+	return IRQ_HANDLED;
+}
+
+static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
+				  struct slim_msg_txn *txn)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(sctrl->dev);
+	DECLARE_COMPLETION_ONSTACK(tx_sent);
+	int ret, timeout;
+	u32 *pbuf;
+	u8 *puc;
+
+	if (txn->mc & SLIM_MSG_CLK_PAUSE_SEQ_FLG)
+		return -EPROTONOSUPPORT;
+
+	if (txn->mt == SLIM_MSG_MT_CORE &&
+		(txn->mc >= SLIM_MSG_MC_BEGIN_RECONFIGURATION &&
+		 txn->mc <= SLIM_MSG_MC_RECONFIGURE_NOW))
+		return 0;
+
+	if (txn->dt == SLIM_MSG_DEST_ENUMADDR)
+		return -EPROTONOSUPPORT;
+
+	if (txn->msg->num_bytes > SLIM_MSGQ_BUF_LEN ||
+			txn->rl > SLIM_MSGQ_BUF_LEN) {
+		dev_err(ctrl->dev, "msg exeeds HW limit\n");
+		return -EINVAL;
+	}
+
+	pbuf = qcom_slim_ngd_tx_msg_get(ctrl, txn->rl, &tx_sent);
+	if (!pbuf) {
+		dev_err(ctrl->dev, "Message buffer unavailable\n");
+		return -ENOMEM;
+	}
+
+	/* HW expects length field to be excluded */
+	txn->rl--;
+	puc = (u8 *)pbuf;
+	*pbuf = 0;
+	if (txn->dt == SLIM_MSG_DEST_LOGICALADDR) {
+		*pbuf = SLIM_MSG_ASM_FIRST_WORD(txn->rl, txn->mt, txn->mc, 0,
+				txn->la);
+		puc += 3;
+	} else {
+		*pbuf = SLIM_MSG_ASM_FIRST_WORD(txn->rl, txn->mt, txn->mc, 1,
+				txn->la);
+		puc += 2;
+	}
+
+	if (slim_tid_txn(txn->mt, txn->mc))
+		*(puc++) = txn->tid;
+
+	if (slim_ec_txn(txn->mt, txn->mc)) {
+		*(puc++) = (txn->ec & 0xFF);
+		*(puc++) = (txn->ec >> 8) & 0xFF;
+	}
+
+	if (txn->msg && txn->msg->wbuf)
+		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
+
+	ret = qcom_slim_ngd_tx_msg_post(ctrl, pbuf, txn->rl);
+	if (ret)
+		return ret;
+
+	timeout = wait_for_completion_timeout(&tx_sent, HZ);
+	if (!timeout) {
+		dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x", txn->mc,
+					txn->mt);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int qcom_slim_ngd_get_laddr(struct slim_controller *sctrl,
+				   struct slim_eaddr *ea, u8 *laddr)
+{
+	DECLARE_COMPLETION_ONSTACK(done);
+	struct slim_val_inf msg =  {0};
+	struct slim_msg_txn txn;
+	u8 wbuf[10] = {0};
+	u8 rbuf[10] = {0};
+	int ret;
+
+	txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
+	txn.dt = SLIM_MSG_DEST_LOGICALADDR;
+	txn.la = SLIM_LA_MGR;
+	txn.ec = 0;
+
+	txn.mc = SLIM_USR_MC_ADDR_QUERY;
+	txn.rl = 11;
+	txn.msg = &msg;
+	txn.msg->num_bytes = 7;
+	txn.msg->wbuf = wbuf;
+	txn.msg->rbuf = rbuf;
+
+	ret = slim_prepare_txn(sctrl, &txn, &done, true);
+	if (ret)
+		return ret;
+
+	wbuf[0] = (u8)txn.tid;
+	memcpy(&wbuf[1], ea, sizeof(*ea));
+	ret = slim_do_transfer(sctrl, &txn);
+
+	*laddr = rbuf[6];
+
+	return ret;
+}
+
+static int qcom_slim_ngd_exit_dma(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	if (ctrl->dma_rx_channel)
+		dma_release_channel(ctrl->dma_rx_channel);
+
+	if (ctrl->dma_tx_channel)
+		dma_release_channel(ctrl->dma_tx_channel);
+
+	ctrl->dma_tx_channel = ctrl->dma_rx_channel = NULL;
+
+	return 0;
+}
+
+static void qcom_slim_ngd_setup(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	u32 cfg = readl_relaxed(ctrl->base +
+				 NGD_BASE(ctrl->id, ctrl->ver));
+
+	if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
+		qcom_slim_ngd_init_dma(ctrl);
+
+	/* By default enable message queues */
+	cfg |= NGD_CFG_RX_MSGQ_EN;
+	cfg |= NGD_CFG_TX_MSGQ_EN;
+
+	/* Enable NGD if it's not already enabled*/
+	if (!(cfg & NGD_CFG_ENABLE))
+		cfg |= NGD_CFG_ENABLE;
+
+	writel_relaxed(cfg, ctrl->base + NGD_BASE(ctrl->id, ctrl->ver));
+}
+
+static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	enum qcom_slim_ngd_state cur_state = ctrl->state;
+	void __iomem *ngd;
+	u32 laddr, rx_msgq;
+	int timeout, ret = 0;
+
+	if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
+		timeout = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
+		if (!timeout)
+			return -EREMOTEIO;
+	}
+
+	if (ctrl->state == QCOM_SLIM_NGD_CTRL_ASLEEP ||
+		ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
+		ret = qcom_slim_qmi_power_request(ctrl, true);
+		if (ret) {
+			dev_err(ctrl->dev, "SLIM QMI power request failed:%d\n",
+					ret);
+			return ret;
+		}
+	}
+
+	ctrl->ver = readl_relaxed(ctrl->base);
+	/* Version info in 16 MSbits */
+	ctrl->ver >>= 16;
+	ngd = ctrl->base + NGD_BASE(ctrl->id, ctrl->ver);
+	laddr = readl_relaxed(ngd + NGD_STATUS);
+	if (laddr & NGD_LADDR) {
+		/*
+		 * external MDM restart case where ADSP itself was active framer
+		 * For example, modem restarted when playback was active
+		 */
+		if (cur_state == QCOM_SLIM_NGD_CTRL_AWAKE) {
+			dev_info(ctrl->dev, "Subsys restart: ADSP active framer\n");
+			return 0;
+		}
+		return 0;
+	}
+
+	writel_relaxed(DEF_NGD_INT_MASK, ctrl->base + NGD_INT_EN +
+				NGD_BASE(ctrl->id, ctrl->ver));
+	rx_msgq = readl_relaxed(ngd + NGD_RX_MSGQ_CFG);
+
+	writel_relaxed(rx_msgq|SLIM_RX_MSGQ_TIMEOUT_VAL, ngd + NGD_RX_MSGQ_CFG);
+	qcom_slim_ngd_setup(ctrl);
+
+	timeout = wait_for_completion_timeout(&ctrl->reconf, HZ);
+	if (!timeout) {
+		dev_err(ctrl->dev, "capability exchange timed-out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static void qcom_slim_ngd_notify_slaves(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	struct slim_device *sbdev;
+	struct device_node *node;
+
+	for_each_child_of_node(ctrl->dev->of_node, node) {
+		sbdev = of_slim_get_device(&ctrl->ctrl, node);
+		if (!sbdev)
+			continue;
+
+		if (slim_get_logical_addr(sbdev))
+			dev_err(ctrl->dev, "Failed to get logical address\n");
+	}
+}
+
+static void qcom_slim_ngd_master_worker(struct work_struct *work)
+{
+	struct qcom_slim_ngd_ctrl *ctrl;
+	struct slim_msg_txn txn;
+	struct slim_val_inf msg = {0};
+	int retries = 0;
+	u8 wbuf[8];
+	int ret = 0;
+
+	ctrl = container_of(work, struct qcom_slim_ngd_ctrl, m_work);
+	txn.dt = SLIM_MSG_DEST_LOGICALADDR;
+	txn.ec = 0;
+	txn.mc = SLIM_USR_MC_REPORT_SATELLITE;
+	txn.mt = SLIM_MSG_MT_SRC_REFERRED_USER;
+	txn.la = SLIM_LA_MGR;
+	wbuf[0] = SAT_MAGIC_LSB;
+	wbuf[1] = SAT_MAGIC_MSB;
+	wbuf[2] = SAT_MSG_VER;
+	wbuf[3] = SAT_MSG_PROT;
+	txn.msg = &msg;
+	txn.msg->wbuf = wbuf;
+	txn.msg->num_bytes = 4;
+	txn.rl = 8;
+
+	dev_info(ctrl->dev, "SLIM SAT: Rcvd master capability\n");
+
+capability_retry:
+	ret = qcom_slim_ngd_xfer_msg(&ctrl->ctrl, &txn);
+	if (!ret) {
+		if (ctrl->state >= QCOM_SLIM_NGD_CTRL_ASLEEP)
+			complete(&ctrl->reconf);
+		else
+			dev_err(ctrl->dev, "unexpected state:%d\n",
+						ctrl->state);
+
+		if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
+			qcom_slim_ngd_notify_slaves(ctrl);
+
+	} else if (ret == -EIO) {
+		dev_err(ctrl->dev, "capability message NACKed, retrying\n");
+		if (retries < INIT_MX_RETRIES) {
+			msleep(DEF_RETRY_MS);
+			retries++;
+			goto capability_retry;
+		}
+	} else {
+		dev_err(ctrl->dev, "SLIM: capability TX failed:%d\n", ret);
+	}
+}
+
+static int qcom_slim_ngd_runtime_resume(struct device *dev)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (ctrl->state >= QCOM_SLIM_NGD_CTRL_ASLEEP)
+		ret = qcom_slim_ngd_power_up(ctrl);
+	if (ret) {
+		/* Did SSR cause this power up failure */
+		if (ctrl->state != QCOM_SLIM_NGD_CTRL_DOWN)
+			ctrl->state = QCOM_SLIM_NGD_CTRL_ASLEEP;
+		else
+			dev_err(ctrl->dev, "HW wakeup attempt during SSR\n");
+	} else {
+		ctrl->state = QCOM_SLIM_NGD_CTRL_AWAKE;
+	}
+
+	return 0;
+}
+
+static int qcom_slim_ngd_enable(struct qcom_slim_ngd_ctrl *ctrl, bool enable)
+{
+	if (enable) {
+		int ret = qcom_slim_qmi_init(ctrl, false);
+
+		if (ret) {
+			dev_err(ctrl->dev, "qmi init fail, ret:%d, state:%d\n",
+				ret, ctrl->state);
+			return ret;
+		}
+		/* controller state should be in sync with framework state */
+		complete(&ctrl->qmi.qmi_comp);
+		if (!pm_runtime_enabled(ctrl->dev) ||
+				!pm_runtime_suspended(ctrl->dev))
+			qcom_slim_ngd_runtime_resume(ctrl->dev);
+		else
+			pm_runtime_resume(ctrl->dev);
+		pm_runtime_mark_last_busy(ctrl->dev);
+		pm_runtime_put(ctrl->dev);
+	} else {
+		qcom_slim_qmi_exit(ctrl);
+	}
+
+	return 0;
+}
+
+static int qcom_slim_ngd_qmi_new_server(struct qmi_handle *hdl,
+					struct qmi_service *service)
+{
+	struct qcom_slim_ngd_qmi *qmi =
+		container_of(hdl, struct qcom_slim_ngd_qmi, svc_event_hdl);
+	struct qcom_slim_ngd_ctrl *ctrl =
+		container_of(qmi, struct qcom_slim_ngd_ctrl, qmi);
+
+	qmi->svc_info.sq_family = AF_QIPCRTR;
+	qmi->svc_info.sq_node = service->node;
+	qmi->svc_info.sq_port = service->port;
+
+	qcom_slim_ngd_enable(ctrl, true);
+
+	return 0;
+}
+
+static void qcom_slim_ngd_qmi_del_server(struct qmi_handle *hdl,
+					 struct qmi_service *service)
+{
+	struct qcom_slim_ngd_qmi *qmi =
+		container_of(hdl, struct qcom_slim_ngd_qmi, svc_event_hdl);
+
+	qmi->svc_info.sq_node = 0;
+	qmi->svc_info.sq_port = 0;
+}
+
+static struct qmi_ops qcom_slim_ngd_qmi_svc_event_ops = {
+	.new_server = qcom_slim_ngd_qmi_new_server,
+	.del_server = qcom_slim_ngd_qmi_del_server,
+};
+
+static int qcom_slim_ngd_qmi_svc_event_init(struct qcom_slim_ngd_qmi *qmi)
+{
+	int ret = 0;
+
+	ret = qmi_handle_init(&qmi->svc_event_hdl, 0,
+				&qcom_slim_ngd_qmi_svc_event_ops, NULL);
+	if (ret < 0) {
+		pr_err("%s: qmi_handle_init failed: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = qmi_add_lookup(&qmi->svc_event_hdl, SLIMBUS_QMI_SVC_ID,
+			SLIMBUS_QMI_SVC_V1, SLIMBUS_QMI_INS_ID);
+	if (ret < 0) {
+		pr_err("%s: qmi_add_lookup failed: %d\n", __func__, ret);
+		qmi_handle_release(&qmi->svc_event_hdl);
+	}
+	return ret;
+}
+
+static void qcom_slim_ngd_qmi_svc_event_deinit(struct qcom_slim_ngd_qmi *qmi)
+{
+	qmi_handle_release(&qmi->svc_event_hdl);
+}
+
+static int qcom_slim_ngd_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct qcom_slim_ngd_ctrl *ctrl;
+	struct resource *res;
+	int ret;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, ctrl);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
+	ctrl->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ctrl->base))
+		return PTR_ERR(ctrl->base);
+
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no slimbus IRQ resource\n");
+		return -ENODEV;
+	}
+
+	ret = devm_request_irq(dev, res->start, qcom_slim_ngd_interrupt,
+			       IRQF_TRIGGER_HIGH, "ngd", ctrl);
+	if (ret) {
+		dev_err(&pdev->dev, "request IRQ failed\n");
+		return ret;
+	}
+
+	if (dev->of_node) {
+		ret = of_property_read_u32(dev->of_node, "qcom,ngd-id",
+					   &ctrl->id);
+		if (ret) {
+			dev_err(&pdev->dev, "Cell index not specified:%d", ret);
+			return ret;
+		}
+	}
+
+	ctrl->dev = dev;
+	ctrl->framer.rootfreq = SLIM_ROOT_FREQ >> 3;
+	ctrl->framer.superfreq =
+		ctrl->framer.rootfreq / SLIM_CL_PER_SUPERFRAME_DIV8;
+	ctrl->ctrl.dev = &pdev->dev;
+	ctrl->ctrl.a_framer = &ctrl->framer;
+	ctrl->ctrl.clkgear = SLIM_MAX_CLK_GEAR;
+	ctrl->ctrl.get_laddr = qcom_slim_ngd_get_laddr;
+	ctrl->ctrl.xfer_msg = qcom_slim_ngd_xfer_msg;
+	ctrl->ctrl.wakeup = NULL;
+	ctrl->state = QCOM_SLIM_NGD_CTRL_DOWN;
+
+	spin_lock_init(&ctrl->tx_buf_lock);
+	init_completion(&ctrl->reconf);
+	init_completion(&ctrl->qmi.qmi_comp);
+
+	ret = slim_register_controller(&ctrl->ctrl);
+	if (ret) {
+		dev_err(ctrl->dev, "error adding controller\n");
+		return ret;
+	}
+
+	pm_runtime_use_autosuspend(ctrl->dev);
+	pm_runtime_set_autosuspend_delay(ctrl->dev, QCOM_SLIM_NGD_AUTOSUSPEND);
+	pm_runtime_set_suspended(ctrl->dev);
+	pm_runtime_enable(ctrl->dev);
+	pm_runtime_get_noresume(ctrl->dev);
+	ret = qcom_slim_ngd_qmi_svc_event_init(&ctrl->qmi);
+	if (ret) {
+		dev_err(&pdev->dev, "QMI service registration failed:%d", ret);
+		goto err;
+	}
+
+	INIT_WORK(&ctrl->m_work, qcom_slim_ngd_master_worker);
+	ctrl->mwq = create_singlethread_workqueue("ngd_master");
+	if (!ctrl->mwq) {
+		dev_err(&pdev->dev, "Failed to start master worker\n");
+		ret = -ENOMEM;
+		goto wq_err;
+	}
+
+	dev_info(dev, "NGD SB controller is up!\n");
+
+	return 0;
+err:
+	slim_unregister_controller(&ctrl->ctrl);
+wq_err:
+	qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi);
+	if (ctrl->mwq)
+		destroy_workqueue(ctrl->mwq);
+
+	return ret;
+}
+
+static int qcom_slim_ngd_remove(struct platform_device *pdev)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+	slim_unregister_controller(&ctrl->ctrl);
+	qcom_slim_ngd_exit_dma(ctrl);
+	qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi);
+	if (ctrl->mwq)
+		destroy_workqueue(ctrl->mwq);
+
+	return 0;
+}
+
+static int qcom_slim_ngd_runtime_idle(struct device *dev)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
+
+	if (ctrl->state == QCOM_SLIM_NGD_CTRL_AWAKE)
+		ctrl->state = QCOM_SLIM_NGD_CTRL_IDLE;
+	pm_request_autosuspend(dev);
+	return -EAGAIN;
+}
+
+
+#ifdef CONFIG_PM
+static int qcom_slim_ngd_runtime_suspend(struct device *dev)
+{
+	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = qcom_slim_qmi_power_request(ctrl, false);
+	if (ret && ret != -EBUSY)
+		dev_info(ctrl->dev, "slim resource not idle:%d\n", ret);
+	if (!ret || ret == -ETIMEDOUT)
+		ctrl->state = QCOM_SLIM_NGD_CTRL_ASLEEP;
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops qcom_slim_ngd_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(
+		qcom_slim_ngd_runtime_suspend,
+		qcom_slim_ngd_runtime_resume,
+		qcom_slim_ngd_runtime_idle
+	)
+};
+
+static const struct of_device_id qcom_slim_ngd_dt_match[] = {
+	{
+		.compatible = "qcom,slim-ngd",
+	},
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, qcom_slim_ngd_dt_match);
+
+static struct platform_driver qcom_slim_ngd_driver = {
+	.probe = qcom_slim_ngd_probe,
+	.remove = qcom_slim_ngd_remove,
+	.driver	= {
+		.name = "qcom,slim-ngd-ctrl",
+		.owner = THIS_MODULE,
+		.pm = &qcom_slim_ngd_dev_pm_ops,
+		.of_match_table = qcom_slim_ngd_dt_match,
+	},
+};
+
+module_platform_driver(qcom_slim_ngd_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Qualcomm SLIMBus NGD controller");
diff --git a/drivers/slimbus/slimbus.h b/drivers/slimbus/slimbus.h
index 67e774b2b2dd..f0051b5497c3 100644
--- a/drivers/slimbus/slimbus.h
+++ b/drivers/slimbus/slimbus.h
@@ -17,6 +17,8 @@
 
 /* SLIMbus message types. Related to interpretation of message code. */
 #define SLIM_MSG_MT_CORE			0x0
+#define SLIM_MSG_MT_DEST_REFERRED_USER		0x2
+#define SLIM_MSG_MT_SRC_REFERRED_USER		0x6
 
 /*
  * SLIM Broadcast header format
@@ -48,6 +50,12 @@
 #define SLIM_MSG_MC_NEXT_PAUSE_CLOCK             0x4A
 #define SLIM_MSG_MC_RECONFIGURE_NOW              0x5F
 
+/*
+ * Clock pause flag to indicate that the reconfig message
+ * corresponds to clock pause sequence
+ */
+#define SLIM_MSG_CLK_PAUSE_SEQ_FLG		(1U << 8)
+
 /* Clock pause values per SLIMbus spec */
 #define SLIM_CLK_FAST				0
 #define SLIM_CLK_CONST_PHASE			1
-- 
2.16.2

Re: [PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6270 bytes --]

Hi Srinivas,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/slimbus-ngd-dt-bindings-Add-slim-ngd-dt-bindings/20180518-193916
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_get_laddr':
>> drivers/slimbus/qcom-ngd-ctrl.c:862:8: error: implicit declaration of function 'slim_prepare_txn'; did you mean 'slab_prepare_cpu'? [-Werror=implicit-function-declaration]
     ret = slim_prepare_txn(sctrl, &txn, &done, true);
           ^~~~~~~~~~~~~~~~
           slab_prepare_cpu
   drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_notify_slaves':
>> drivers/slimbus/qcom-ngd-ctrl.c:969:11: error: implicit declaration of function 'of_slim_get_device'; did you mean 'slim_get_device'? [-Werror=implicit-function-declaration]
      sbdev = of_slim_get_device(&ctrl->ctrl, node);
              ^~~~~~~~~~~~~~~~~~
              slim_get_device
>> drivers/slimbus/qcom-ngd-ctrl.c:969:9: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      sbdev = of_slim_get_device(&ctrl->ctrl, node);
            ^
   cc1: some warnings being treated as errors

vim +862 drivers/slimbus/qcom-ngd-ctrl.c

   839	
   840	static int qcom_slim_ngd_get_laddr(struct slim_controller *sctrl,
   841					   struct slim_eaddr *ea, u8 *laddr)
   842	{
   843		DECLARE_COMPLETION_ONSTACK(done);
   844		struct slim_val_inf msg =  {0};
   845		struct slim_msg_txn txn;
   846		u8 wbuf[10] = {0};
   847		u8 rbuf[10] = {0};
   848		int ret;
   849	
   850		txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
   851		txn.dt = SLIM_MSG_DEST_LOGICALADDR;
   852		txn.la = SLIM_LA_MGR;
   853		txn.ec = 0;
   854	
   855		txn.mc = SLIM_USR_MC_ADDR_QUERY;
   856		txn.rl = 11;
   857		txn.msg = &msg;
   858		txn.msg->num_bytes = 7;
   859		txn.msg->wbuf = wbuf;
   860		txn.msg->rbuf = rbuf;
   861	
 > 862		ret = slim_prepare_txn(sctrl, &txn, &done, true);
   863		if (ret)
   864			return ret;
   865	
   866		wbuf[0] = (u8)txn.tid;
   867		memcpy(&wbuf[1], ea, sizeof(*ea));
   868		ret = slim_do_transfer(sctrl, &txn);
   869	
   870		*laddr = rbuf[6];
   871	
   872		return ret;
   873	}
   874	
   875	static int qcom_slim_ngd_exit_dma(struct qcom_slim_ngd_ctrl *ctrl)
   876	{
   877		if (ctrl->dma_rx_channel)
   878			dma_release_channel(ctrl->dma_rx_channel);
   879	
   880		if (ctrl->dma_tx_channel)
   881			dma_release_channel(ctrl->dma_tx_channel);
   882	
   883		ctrl->dma_tx_channel = ctrl->dma_rx_channel = NULL;
   884	
   885		return 0;
   886	}
   887	
   888	static void qcom_slim_ngd_setup(struct qcom_slim_ngd_ctrl *ctrl)
   889	{
   890		u32 cfg = readl_relaxed(ctrl->base +
   891					 NGD_BASE(ctrl->id, ctrl->ver));
   892	
   893		if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
   894			qcom_slim_ngd_init_dma(ctrl);
   895	
   896		/* By default enable message queues */
   897		cfg |= NGD_CFG_RX_MSGQ_EN;
   898		cfg |= NGD_CFG_TX_MSGQ_EN;
   899	
   900		/* Enable NGD if it's not already enabled*/
   901		if (!(cfg & NGD_CFG_ENABLE))
   902			cfg |= NGD_CFG_ENABLE;
   903	
   904		writel_relaxed(cfg, ctrl->base + NGD_BASE(ctrl->id, ctrl->ver));
   905	}
   906	
   907	static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
   908	{
   909		enum qcom_slim_ngd_state cur_state = ctrl->state;
   910		void __iomem *ngd;
   911		u32 laddr, rx_msgq;
   912		int timeout, ret = 0;
   913	
   914		if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
   915			timeout = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
   916			if (!timeout)
   917				return -EREMOTEIO;
   918		}
   919	
   920		if (ctrl->state == QCOM_SLIM_NGD_CTRL_ASLEEP ||
   921			ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
   922			ret = qcom_slim_qmi_power_request(ctrl, true);
   923			if (ret) {
   924				dev_err(ctrl->dev, "SLIM QMI power request failed:%d\n",
   925						ret);
   926				return ret;
   927			}
   928		}
   929	
   930		ctrl->ver = readl_relaxed(ctrl->base);
   931		/* Version info in 16 MSbits */
   932		ctrl->ver >>= 16;
   933		ngd = ctrl->base + NGD_BASE(ctrl->id, ctrl->ver);
   934		laddr = readl_relaxed(ngd + NGD_STATUS);
   935		if (laddr & NGD_LADDR) {
   936			/*
   937			 * external MDM restart case where ADSP itself was active framer
   938			 * For example, modem restarted when playback was active
   939			 */
   940			if (cur_state == QCOM_SLIM_NGD_CTRL_AWAKE) {
   941				dev_info(ctrl->dev, "Subsys restart: ADSP active framer\n");
   942				return 0;
   943			}
   944			return 0;
   945		}
   946	
   947		writel_relaxed(DEF_NGD_INT_MASK, ctrl->base + NGD_INT_EN +
   948					NGD_BASE(ctrl->id, ctrl->ver));
   949		rx_msgq = readl_relaxed(ngd + NGD_RX_MSGQ_CFG);
   950	
   951		writel_relaxed(rx_msgq|SLIM_RX_MSGQ_TIMEOUT_VAL, ngd + NGD_RX_MSGQ_CFG);
   952		qcom_slim_ngd_setup(ctrl);
   953	
   954		timeout = wait_for_completion_timeout(&ctrl->reconf, HZ);
   955		if (!timeout) {
   956			dev_err(ctrl->dev, "capability exchange timed-out\n");
   957			return -ETIMEDOUT;
   958		}
   959	
   960		return 0;
   961	}
   962	
   963	static void qcom_slim_ngd_notify_slaves(struct qcom_slim_ngd_ctrl *ctrl)
   964	{
   965		struct slim_device *sbdev;
   966		struct device_node *node;
   967	
   968		for_each_child_of_node(ctrl->dev->of_node, node) {
 > 969			sbdev = of_slim_get_device(&ctrl->ctrl, node);
   970			if (!sbdev)
   971				continue;
   972	
   973			if (slim_get_logical_addr(sbdev))
   974				dev_err(ctrl->dev, "Failed to get logical address\n");
   975		}
   976	}
   977	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65207 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver

[Srinivas Kandagatla]

Thanks for the report!

This patch has dependency on https://lkml.org/lkml/2018/5/17/251

I should have mentioned this in cover letter!

thanks,
srini

On 18/05/18 22:39, kbuild test robot wrote:
> Hi Srinivas,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc5 next-20180517]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/slimbus-ngd-dt-bindings-Add-slim-ngd-dt-bindings/20180518-193916
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm
> 
> All error/warnings (new ones prefixed by >>):
> 
>     drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_get_laddr':
>>> drivers/slimbus/qcom-ngd-ctrl.c:862:8: error: implicit declaration of function 'slim_prepare_txn'; did you mean 'slab_prepare_cpu'? [-Werror=implicit-function-declaration]
>       ret = slim_prepare_txn(sctrl, &txn, &done, true);
>             ^~~~~~~~~~~~~~~~
>             slab_prepare_cpu
>     drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_notify_slaves':
>>> drivers/slimbus/qcom-ngd-ctrl.c:969:11: error: implicit declaration of function 'of_slim_get_device'; did you mean 'slim_get_device'? [-Werror=implicit-function-declaration]
>        sbdev = of_slim_get_device(&ctrl->ctrl, node);
>                ^~~~~~~~~~~~~~~~~~
>                slim_get_device
>>> drivers/slimbus/qcom-ngd-ctrl.c:969:9: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>        sbdev = of_slim_get_device(&ctrl->ctrl, node);
>              ^
>     cc1: some warnings being treated as errors
> 
> vim +862 drivers/slimbus/qcom-ngd-ctrl.c
> 
>     839	
>     840	static int qcom_slim_ngd_get_laddr(struct slim_controller *sctrl,
>     841					   struct slim_eaddr *ea, u8 *laddr)
>     842	{
>     843		DECLARE_COMPLETION_ONSTACK(done);
>     844		struct slim_val_inf msg =  {0};
>     845		struct slim_msg_txn txn;
>     846		u8 wbuf[10] = {0};
>     847		u8 rbuf[10] = {0};
>     848		int ret;
>     849	
>     850		txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
>     851		txn.dt = SLIM_MSG_DEST_LOGICALADDR;
>     852		txn.la = SLIM_LA_MGR;
>     853		txn.ec = 0;
>     854	
>     855		txn.mc = SLIM_USR_MC_ADDR_QUERY;
>     856		txn.rl = 11;
>     857		txn.msg = &msg;
>     858		txn.msg->num_bytes = 7;
>     859		txn.msg->wbuf = wbuf;
>     860		txn.msg->rbuf = rbuf;
>     861	
>   > 862		ret = slim_prepare_txn(sctrl, &txn, &done, true);
>     863		if (ret)
>     864			return ret;
>     865	
>     866		wbuf[0] = (u8)txn.tid;
>     867		memcpy(&wbuf[1], ea, sizeof(*ea));
>     868		ret = slim_do_transfer(sctrl, &txn);
>     869	
>     870		*laddr = rbuf[6];
>     871	
>     872		return ret;
>     873	}
>     874	
>     875	static int qcom_slim_ngd_exit_dma(struct qcom_slim_ngd_ctrl *ctrl)
>     876	{
>     877		if (ctrl->dma_rx_channel)
>     878			dma_release_channel(ctrl->dma_rx_channel);
>     879	
>     880		if (ctrl->dma_tx_channel)
>     881			dma_release_channel(ctrl->dma_tx_channel);
>     882	
>     883		ctrl->dma_tx_channel = ctrl->dma_rx_channel = NULL;
>     884	
>     885		return 0;
>     886	}
>     887	
>     888	static void qcom_slim_ngd_setup(struct qcom_slim_ngd_ctrl *ctrl)
>     889	{
>     890		u32 cfg = readl_relaxed(ctrl->base +
>     891					 NGD_BASE(ctrl->id, ctrl->ver));
>     892	
>     893		if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
>     894			qcom_slim_ngd_init_dma(ctrl);
>     895	
>     896		/* By default enable message queues */
>     897		cfg |= NGD_CFG_RX_MSGQ_EN;
>     898		cfg |= NGD_CFG_TX_MSGQ_EN;
>     899	
>     900		/* Enable NGD if it's not already enabled*/
>     901		if (!(cfg & NGD_CFG_ENABLE))
>     902			cfg |= NGD_CFG_ENABLE;
>     903	
>     904		writel_relaxed(cfg, ctrl->base + NGD_BASE(ctrl->id, ctrl->ver));
>     905	}
>     906	
>     907	static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
>     908	{
>     909		enum qcom_slim_ngd_state cur_state = ctrl->state;
>     910		void __iomem *ngd;
>     911		u32 laddr, rx_msgq;
>     912		int timeout, ret = 0;
>     913	
>     914		if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
>     915			timeout = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
>     916			if (!timeout)
>     917				return -EREMOTEIO;
>     918		}
>     919	
>     920		if (ctrl->state == QCOM_SLIM_NGD_CTRL_ASLEEP ||
>     921			ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
>     922			ret = qcom_slim_qmi_power_request(ctrl, true);
>     923			if (ret) {
>     924				dev_err(ctrl->dev, "SLIM QMI power request failed:%d\n",
>     925						ret);
>     926				return ret;
>     927			}
>     928		}
>     929	
>     930		ctrl->ver = readl_relaxed(ctrl->base);
>     931		/* Version info in 16 MSbits */
>     932		ctrl->ver >>= 16;
>     933		ngd = ctrl->base + NGD_BASE(ctrl->id, ctrl->ver);
>     934		laddr = readl_relaxed(ngd + NGD_STATUS);
>     935		if (laddr & NGD_LADDR) {
>     936			/*
>     937			 * external MDM restart case where ADSP itself was active framer
>     938			 * For example, modem restarted when playback was active
>     939			 */
>     940			if (cur_state == QCOM_SLIM_NGD_CTRL_AWAKE) {
>     941				dev_info(ctrl->dev, "Subsys restart: ADSP active framer\n");
>     942				return 0;
>     943			}
>     944			return 0;
>     945		}
>     946	
>     947		writel_relaxed(DEF_NGD_INT_MASK, ctrl->base + NGD_INT_EN +
>     948					NGD_BASE(ctrl->id, ctrl->ver));
>     949		rx_msgq = readl_relaxed(ngd + NGD_RX_MSGQ_CFG);
>     950	
>     951		writel_relaxed(rx_msgq|SLIM_RX_MSGQ_TIMEOUT_VAL, ngd + NGD_RX_MSGQ_CFG);
>     952		qcom_slim_ngd_setup(ctrl);
>     953	
>     954		timeout = wait_for_completion_timeout(&ctrl->reconf, HZ);
>     955		if (!timeout) {
>     956			dev_err(ctrl->dev, "capability exchange timed-out\n");
>     957			return -ETIMEDOUT;
>     958		}
>     959	
>     960		return 0;
>     961	}
>     962	
>     963	static void qcom_slim_ngd_notify_slaves(struct qcom_slim_ngd_ctrl *ctrl)
>     964	{
>     965		struct slim_device *sbdev;
>     966		struct device_node *node;
>     967	
>     968		for_each_child_of_node(ctrl->dev->of_node, node) {
>   > 969			sbdev = of_slim_get_device(&ctrl->ctrl, node);
>     970			if (!sbdev)
>     971				continue;
>     972	
>     973			if (slim_get_logical_addr(sbdev))
>     974				dev_err(ctrl->dev, "Failed to get logical address\n");
>     975		}
>     976	}
>     977	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

Re: [PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver

[Vinod]

On 16-05-18, 17:51, Srinivas Kandagatla wrote:
> This patch adds suppor to Qualcomm SLIMBus Non-Generic Device (NGD)

/s/suppor/support

> +/* NGD (Non-ported Generic Device) registers */
> +#define	NGD_CFG			0x0
> +#define	NGD_CFG_ENABLE		BIT(0)
> +#define	NGD_CFG_RX_MSGQ_EN	BIT(1)
> +#define	NGD_CFG_TX_MSGQ_EN	BIT(2)
> +#define	NGD_STATUS		0x4
> +#define NGD_LADDR		BIT(1)
> +#define	NGD_RX_MSGQ_CFG		0x8
> +#define	NGD_INT_EN		0x10
> +#define	NGD_INT_RECFG_DONE	BIT(24)
> +#define	NGD_INT_TX_NACKED_2	BIT(25)
> +#define	NGD_INT_MSG_BUF_CONTE	BIT(26)
> +#define	NGD_INT_MSG_TX_INVAL	BIT(27)
> +#define	NGD_INT_IE_VE_CHG	BIT(28)
> +#define	NGD_INT_DEV_ERR		BIT(29)
> +#define	NGD_INT_RX_MSG_RCVD	BIT(30)
> +#define	NGD_INT_TX_MSG_SENT	BIT(31)
> +#define	NGD_INT_STAT		0x14
> +#define	NGD_INT_CLR		0x18
> +#define DEF_NGD_INT_MASK (NGD_INT_TX_NACKED_2 | NGD_INT_MSG_BUF_CONTE | \
> +				NGD_INT_MSG_TX_INVAL | NGD_INT_IE_VE_CHG | \
> +				NGD_INT_DEV_ERR | NGD_INT_TX_MSG_SENT | \
> +				NGD_INT_RX_MSG_RCVD)
> +
> +/* Slimbus QMI service */
> +#define SLIMBUS_QMI_SVC_ID 0x0301
> +#define SLIMBUS_QMI_SVC_V1 1
> +#define SLIMBUS_QMI_INS_ID 0
> +#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01 0x0020
> +#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_V01 0x0020
> +#define SLIMBUS_QMI_POWER_REQ_V01 0x0021
> +#define SLIMBUS_QMI_POWER_RESP_V01 0x0021
> +#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_REQ 0x0022
> +#define SLIMBUS_QMI_CHECK_FRAMER_STATUS_RESP 0x0022
> +#define SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN 14
> +#define SLIMBUS_QMI_POWER_RESP_MAX_MSG_LEN 7
> +#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_MAX_MSG_LEN 14
> +#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_MAX_MSG_LEN 7
> +#define SLIMBUS_QMI_CHECK_FRAMER_STAT_RESP_MAX_MSG_LEN 7
> +/* QMI response timeout of 500ms */
> +#define SLIMBUS_QMI_RESP_TOUT 1000

Tabs or spaces, take your pick and use one, not both...

> +static void qcom_slim_qmi_power_resp_cb(struct qmi_handle *handle,
> +					struct sockaddr_qrtr *sq,
> +					struct qmi_txn *txn, const void *data)
> +{
> +	struct slimbus_power_resp_msg_v01 *resp;
> +
> +	resp = (struct slimbus_power_resp_msg_v01 *)data;

you dont need cast away from void

> +	if (resp->resp.result != QMI_RESULT_SUCCESS_V01)
> +		pr_err("%s: QMI power request failed 0x%x\n", __func__,
> +				resp->resp.result);

cant we use dev_err?

> +static int qcom_slim_qmi_send_power_request(struct qcom_slim_ngd_ctrl *ctrl,
> +				struct slimbus_power_req_msg_v01 *req)
> +{
> +	struct slimbus_power_resp_msg_v01 resp = { { 0, 0 } };
> +	struct qmi_txn txn;
> +	int rc;
> +
> +	rc = qmi_txn_init(ctrl->qmi.handle, &txn,
> +				slimbus_power_resp_msg_v01_ei, &resp);
> +
> +	rc = qmi_send_request(ctrl->qmi.handle, NULL, &txn,
> +				SLIMBUS_QMI_POWER_REQ_V01,
> +				SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN,
> +				slimbus_power_req_msg_v01_ei, req);
> +	if (rc < 0) {
> +		dev_err(ctrl->dev, "%s: QMI send req fail %d\n", __func__, rc);
> +		qmi_txn_cancel(&txn);
> +	}
> +
> +	if (rc < 0)
> +		return rc;

why not add this is prev error check?

> +static int qcom_slim_qmi_init(struct qcom_slim_ngd_ctrl *ctrl,
> +			      bool apps_is_master)
> +{
> +	int rc = 0;

superfluous init

> +static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
> +				     struct completion *comp)
> +{
> +	struct qcom_slim_ngd_dma_desc *desc;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
> +
> +	if ((ctrl->tx_tail + 1) % QCOM_SLIM_NGD_DESC_NUM == ctrl->tx_head) {
> +		spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
> +		return NULL;
> +	}
> +	desc  = &ctrl->txdesc[ctrl->tx_tail];
> +	desc->base = (u32 *)((u8 *)ctrl->tx_base +
> +				(ctrl->tx_tail * SLIM_MSGQ_BUF_LEN));

too many casts

> +static int qcom_slim_ngd_post_rx_msgq(struct qcom_slim_ngd_ctrl *ctrl)
> +{
> +	struct qcom_slim_ngd_dma_desc *desc;
> +	struct dma_slave_config conf = {
> +		.direction = DMA_DEV_TO_MEM,
> +	};
> +	int ret, i;
> +
> +	ret = dmaengine_slave_config(ctrl->dma_rx_channel, &conf);

so you are only setting direction for conf, not any other parameters? If not why
bother setting direction

> +	if (ret)
> +		dev_err(ctrl->dev, "Error Configuring rx dma\n");
> +
> +	for (i = 0; i < QCOM_SLIM_NGD_DESC_NUM; i++) {
> +		desc = &ctrl->rx_desc[i];
> +		desc->phys = ctrl->rx_phys_base + i * SLIM_MSGQ_BUF_LEN;
> +		desc->ctrl = ctrl;
> +		desc->base = ctrl->rx_base + i * SLIM_MSGQ_BUF_LEN;
> +		desc->desc = dmaengine_prep_slave_single(ctrl->dma_rx_channel,
> +						desc->phys, SLIM_MSGQ_BUF_LEN,
> +						DMA_DEV_TO_MEM,
> +						DMA_PREP_INTERRUPT);

why issue multiple slave_single to dmaengine, you can bundle them and issue
dmaengine_prep_slave_sg().. 

> +static int qcom_slim_ngd_qmi_svc_event_init(struct qcom_slim_ngd_qmi *qmi)
> +{
> +	int ret = 0;

superfluous init here too

> +static int qcom_slim_ngd_runtime_idle(struct device *dev)
> +{
> +	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
> +
> +	if (ctrl->state == QCOM_SLIM_NGD_CTRL_AWAKE)
> +		ctrl->state = QCOM_SLIM_NGD_CTRL_IDLE;
> +	pm_request_autosuspend(dev);
> +	return -EAGAIN;
> +}
> +
> +

double empty lines, here
-- 
~Vinod

Re: [PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver

[Srinivas Kandagatla]

Thanks Vinod for the review!

On 21/05/18 12:33, Vinod wrote:
> On 16-05-18, 17:51, Srinivas Kandagatla wrote:
>> This patch adds suppor to Qualcomm SLIMBus Non-Generic Device (NGD)
> 
> /s/suppor/support
> 
Yep, Will fix this in next version.

>> +/* NGD (Non-ported Generic Device) registers */
>> +#define	NGD_CFG			0x0
>> +#define	NGD_CFG_ENABLE		BIT(0)
>> +#define	NGD_CFG_RX_MSGQ_EN	BIT(1)
>> +#define	NGD_CFG_TX_MSGQ_EN	BIT(2)
>> +#define	NGD_STATUS		0x4
>> +#define NGD_LADDR		BIT(1)
>> +#define	NGD_RX_MSGQ_CFG		0x8
>> +#define	NGD_INT_EN		0x10
>> +#define SLIMBUS_QMI_POWER_RESP_MAX_MSG_LEN 7
>> +#define SLIMBUS_QMI_SELECT_INSTANCE_REQ_MAX_MSG_LEN 14
>> +#define SLIMBUS_QMI_SELECT_INSTANCE_RESP_MAX_MSG_LEN 7
>> +#define SLIMBUS_QMI_CHECK_FRAMER_STAT_RESP_MAX_MSG_LEN 7
>> +/* QMI response timeout of 500ms */
>> +#define SLIMBUS_QMI_RESP_TOUT 1000
> 
> Tabs or spaces, take your pick and use one, not both...
I will check for such instances once again before sending v2.

> 
>> +static void qcom_slim_qmi_power_resp_cb(struct qmi_handle *handle,
>> +					struct sockaddr_qrtr *sq,
>> +					struct qmi_txn *txn, const void *data)
>> +{
>> +	struct slimbus_power_resp_msg_v01 *resp;
>> +
>> +	resp = (struct slimbus_power_resp_msg_v01 *)data;
> 
> you dont need cast away from void
It's a const void * so the compiler keeps complaining about this without 
cast.

> 
>> +	if (resp->resp.result != QMI_RESULT_SUCCESS_V01)
>> +		pr_err("%s: QMI power request failed 0x%x\n", __func__,
>> +				resp->resp.result);
> 
> cant we use dev_err?
I will have a look!
> 
>> +static int qcom_slim_qmi_send_power_request(struct qcom_slim_ngd_ctrl *ctrl,
>> +				struct slimbus_power_req_msg_v01 *req)
>> +{
>> +	struct slimbus_power_resp_msg_v01 resp = { { 0, 0 } };
>> +	struct qmi_txn txn;
>> +	int rc;
>> +
>> +	rc = qmi_txn_init(ctrl->qmi.handle, &txn,
>> +				slimbus_power_resp_msg_v01_ei, &resp);
>> +
>> +	rc = qmi_send_request(ctrl->qmi.handle, NULL, &txn,
>> +				SLIMBUS_QMI_POWER_REQ_V01,
>> +				SLIMBUS_QMI_POWER_REQ_MAX_MSG_LEN,
>> +				slimbus_power_req_msg_v01_ei, req);
>> +	if (rc < 0) {
>> +		dev_err(ctrl->dev, "%s: QMI send req fail %d\n", __func__, rc);
>> +		qmi_txn_cancel(&txn);
>> +	}
>> +
>> +	if (rc < 0)
>> +		return rc;
> 
> why not add this is prev error check?
Yes, we should move this to previous check, Will fix this in v2.
> 
>> +static int qcom_slim_qmi_init(struct qcom_slim_ngd_ctrl *ctrl,
>> +			      bool apps_is_master)
>> +{
>> +	int rc = 0;
> 
> superfluous init
> 
>> +static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
>> +				     struct completion *comp)
>> +{
>> +	struct qcom_slim_ngd_dma_desc *desc;
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
>> +
>> +	if ((ctrl->tx_tail + 1) % QCOM_SLIM_NGD_DESC_NUM == ctrl->tx_head) {
>> +		spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
>> +		return NULL;
>> +	}
>> +	desc  = &ctrl->txdesc[ctrl->tx_tail];
>> +	desc->base = (u32 *)((u8 *)ctrl->tx_base +
>> +				(ctrl->tx_tail * SLIM_MSGQ_BUF_LEN));
> 
> too many casts
> 
>> +static int qcom_slim_ngd_post_rx_msgq(struct qcom_slim_ngd_ctrl *ctrl)
>> +{
>> +	struct qcom_slim_ngd_dma_desc *desc;
>> +	struct dma_slave_config conf = {
>> +		.direction = DMA_DEV_TO_MEM,
>> +	};
>> +	int ret, i;
>> +
>> +	ret = dmaengine_slave_config(ctrl->dma_rx_channel, &conf);
> 
> so you are only setting direction for conf, not any other parameters? If not why
> bother setting direction
Nice hint, I will remove this!
> 
>> +	if (ret)
>> +		dev_err(ctrl->dev, "Error Configuring rx dma\n");
>> +
>> +	for (i = 0; i < QCOM_SLIM_NGD_DESC_NUM; i++) {
>> +		desc = &ctrl->rx_desc[i];
>> +		desc->phys = ctrl->rx_phys_base + i * SLIM_MSGQ_BUF_LEN;
>> +		desc->ctrl = ctrl;
>> +		desc->base = ctrl->rx_base + i * SLIM_MSGQ_BUF_LEN;
>> +		desc->desc = dmaengine_prep_slave_single(ctrl->dma_rx_channel,
>> +						desc->phys, SLIM_MSGQ_BUF_LEN,
>> +						DMA_DEV_TO_MEM,
>> +						DMA_PREP_INTERRUPT);
> 
> why issue multiple slave_single to dmaengine, you can bundle them and issue
> dmaengine_prep_slave_sg()..
Am reusing the descriptors here, My plan is to issue multiple 
descriptors with callback for each, and in each callback queue it back.

> 
>> +static int qcom_slim_ngd_qmi_svc_event_init(struct qcom_slim_ngd_qmi *qmi)
>> +{
>> +	int ret = 0;
> 
> superfluous init here too
> 
Yep.
>> +static int qcom_slim_ngd_runtime_idle(struct device *dev)
>> +{
>> +	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
>> +
>> +	if (ctrl->state == QCOM_SLIM_NGD_CTRL_AWAKE)
>> +		ctrl->state = QCOM_SLIM_NGD_CTRL_IDLE;
>> +	pm_request_autosuspend(dev);
>> +	return -EAGAIN;
>> +}
>> +
>> +
> 
> double empty lines, here
Sure, will fix this in v2.
> 

thanks,
srini