dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] dmaengine: Add support for Qcom GSI dma controller
@ 2020-08-24  8:47 Vinod Koul
  2020-08-24  8:47 ` [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding Vinod Koul
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Vinod Koul @ 2020-08-24  8:47 UTC (permalink / raw)
  To: dmaengine; +Cc: Vinod Koul

This series adds support for Qcom GSI dma controller found on Qualcomm SoCs.
This controller can program the peripheral configuration so we add
additional parameters in dma_slave_config for configuring the peripherals
like spi and i2c.

Vinod Koul (3):
  dt-bindings: dmaengine: Document qcom,gpi dma binding
  dmaengine: add peripheral configuration
  dmaengine: qcom: Add GPI dma driver

 .../devicetree/bindings/dma/qcom-gpi.yaml     |   87 +
 drivers/dma/qcom/Kconfig                      |   12 +
 drivers/dma/qcom/Makefile                     |    1 +
 drivers/dma/qcom/gpi.c                        | 2269 +++++++++++++++++
 include/linux/dmaengine.h                     |   75 +
 5 files changed, 2444 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
 create mode 100644 drivers/dma/qcom/gpi.c

-- 
2.26.2


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

* [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-24  8:47 [PATCH 0/3] dmaengine: Add support for Qcom GSI dma controller Vinod Koul
@ 2020-08-24  8:47 ` Vinod Koul
  2020-08-24 17:40   ` Rob Herring
  2020-08-24  8:47 ` [RFC PATCH 2/3] dmaengine: add peripheral configuration Vinod Koul
  2020-08-24  8:47 ` [PATCH 3/3] dmaengine: qcom: Add GPI dma driver Vinod Koul
  2 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-24  8:47 UTC (permalink / raw)
  To: dmaengine, Rob Herring
  Cc: Vinod Koul, Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel

Add devicetree binding documentation for GPI DMA controller
implemented on Qualcomm SoCs

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml

diff --git a/Documentation/devicetree/bindings/dma/qcom-gpi.yaml b/Documentation/devicetree/bindings/dma/qcom-gpi.yaml
new file mode 100644
index 000000000000..c56d601ad2d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/qcom-gpi.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/gpi-dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies Inc GPI DMA controller
+
+description: |
+  QCOM GPI DMA controller provides DMA capabilities for
+  peripheral buses such as I2C, UART, and SPI.
+
+maintainers:
+  - Vinod Koul <vkoul@kernel.org>
+
+allOf:
+  - $ref: "dma-controller.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - qcom,gpi-dma
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt lines for each GPII instance
+    maxItems: 14
+
+  qcom,max-num-gpii:
+    description:
+      Maximum number of GPII instances available
+    maxItems: 1
+
+  "#dma-cells":
+    const: 1
+
+  qcom,gpii-mask:
+    description:
+      Bitmap of supported GPII instances for OS
+    maxItems: 1
+
+  qcom,ev-factor:
+    description:
+      Event ring transfer size compare to channel transfer ring. Event
+        ring length = ev-factor * transfer ring size
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - qcom,max-num-gpii
+  - qcom,gpii-mask
+  - qcom,ev-factor
+  - "#dma-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    gpi_dma0: dma@800000 {
+        #dma-cells = <5>;
+        compatible = "qcom,gpi-dma";
+        reg = <0 0x00800000 0 0x60000>;
+        qcom,max-num-gpii = <13>;
+        qcom,gpii-mask = <0xfa>;
+        qcom,ev-factor = <2>;
+        interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 253 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 254 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>;
+    };
+
+...
-- 
2.26.2


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

* [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-24  8:47 [PATCH 0/3] dmaengine: Add support for Qcom GSI dma controller Vinod Koul
  2020-08-24  8:47 ` [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding Vinod Koul
@ 2020-08-24  8:47 ` Vinod Koul
  2020-08-25  6:52   ` Peter Ujfalusi
  2020-08-24  8:47 ` [PATCH 3/3] dmaengine: qcom: Add GPI dma driver Vinod Koul
  2 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-24  8:47 UTC (permalink / raw)
  To: dmaengine
  Cc: Vinod Koul, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

Some complex dmaengine controllers have capability to program the
peripheral device, so pass on the peripheral configuration as part of
dma_slave_config

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 include/linux/dmaengine.h | 75 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 6fbd5c99e30c..264643ca9209 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -380,6 +380,73 @@ enum dma_slave_buswidth {
 	DMA_SLAVE_BUSWIDTH_64_BYTES = 64,
 };
 
+/**
+ * enum spi_transfer_cmd - spi transfer commands
+ */
+enum spi_transfer_cmd {
+	SPI_TX = 1,
+	SPI_RX,
+	SPI_DUPLEX,
+};
+
+/**
+ * struct dmaengine_spi_config - spi config for peripheral
+ *
+ * @loopback_en: spi loopback enable when set
+ * @clock_pol: clock polarity
+ * @data_pol: data polarity
+ * @pack_en: process tx/rx buffers as packed
+ * @word_len: spi word length
+ * @clk_div: source clock divider
+ * @clk_src: serial clock
+ * @cmd: spi cmd
+ * @cs: chip select toggle
+ * @rx_len: receive length for spi buffer
+ */
+struct dmaengine_spi_config {
+	u8 loopback_en;
+	u8 clock_pol;
+	u8 data_pol;
+	u8 pack_en;
+	u8 word_len;
+	u32 clk_div;
+	u32 clk_src;
+	u8 fragmentation;
+	enum spi_transfer_cmd cmd;
+	u8 cs;
+	u32 rx_len;
+};
+
+/**
+ * struct dmaengine_i2c_config - i2c config for peripheral
+ *
+ * @pack_enable: process tx/rx buffers as packed
+ * @cycle_count: clock cycles to be sent
+ * @high_count: high period of clock
+ * @low_count: low period of clock
+ * @clk_div: source clock divider
+ * @addr: i2c bus address
+ * @strech: strech the clock at eot
+ * @op: i2c cmd
+ */
+struct dmaengine_i2c_config {
+	u8 pack_enable;
+	u8 cycle_count;
+	u8 high_count;
+	u8 low_count;
+	u16 clk_div;
+	u8 addr;
+	u8 strech;
+	u8 op;
+};
+
+enum dmaengine_peripheral {
+	DMAENGINE_PERIPHERAL_SPI,
+	DMAENGINE_PERIPHERAL_I2C,
+	DMAENGINE_PERIPHERAL_UART,
+	DMAENGINE_PERIPHERAL_LAST = DMAENGINE_PERIPHERAL_UART,
+};
+
 /**
  * struct dma_slave_config - dma slave channel runtime config
  * @direction: whether the data shall go in or out on this slave
@@ -418,6 +485,10 @@ enum dma_slave_buswidth {
  * @slave_id: Slave requester id. Only valid for slave channels. The dma
  * slave peripheral will have unique id as dma requester which need to be
  * pass as slave config.
+ * @peripheral: type of peripheral to DMA to/from
+ * @set_config: set peripheral config
+ * @spi: peripheral config for spi
+ * @:i2c peripheral config for i2c
  *
  * This struct is passed in as configuration data to a DMA engine
  * in order to set up a certain channel for DMA transport at runtime.
@@ -443,6 +514,10 @@ struct dma_slave_config {
 	u32 dst_port_window_size;
 	bool device_fc;
 	unsigned int slave_id;
+	enum dmaengine_peripheral peripheral;
+	u8 set_config;
+	struct dmaengine_spi_config spi;
+	struct dmaengine_i2c_config i2c;
 };
 
 /**
-- 
2.26.2


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

* [PATCH 3/3] dmaengine: qcom: Add GPI dma driver
  2020-08-24  8:47 [PATCH 0/3] dmaengine: Add support for Qcom GSI dma controller Vinod Koul
  2020-08-24  8:47 ` [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding Vinod Koul
  2020-08-24  8:47 ` [RFC PATCH 2/3] dmaengine: add peripheral configuration Vinod Koul
@ 2020-08-24  8:47 ` Vinod Koul
  2020-08-24 14:04   ` kernel test robot
  2020-08-24 14:13   ` kernel test robot
  2 siblings, 2 replies; 17+ messages in thread
From: Vinod Koul @ 2020-08-24  8:47 UTC (permalink / raw)
  To: dmaengine
  Cc: Vinod Koul, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

This controller provides DMAengine capabilities for a variety of peripheral
buses such as I2C, UART, and SPI. By using GPI dmaengine driver, bus
drivers can use a standardize interface that is protocol independent to
transfer data between memory and peripheral.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/dma/qcom/Kconfig  |   12 +
 drivers/dma/qcom/Makefile |    1 +
 drivers/dma/qcom/gpi.c    | 2269 +++++++++++++++++++++++++++++++++++++
 3 files changed, 2282 insertions(+)
 create mode 100644 drivers/dma/qcom/gpi.c

diff --git a/drivers/dma/qcom/Kconfig b/drivers/dma/qcom/Kconfig
index 3bcb689162c6..f925296b0c85 100644
--- a/drivers/dma/qcom/Kconfig
+++ b/drivers/dma/qcom/Kconfig
@@ -8,6 +8,18 @@ config QCOM_BAM_DMA
 	  Enable support for the QCOM BAM DMA controller.  This controller
 	  provides DMA capabilities for a variety of on-chip devices.
 
+config QCOM_GPI_DMA
+        tristate "Qualcomm Technologies GPI DMA support"
+        depends on ARCH_QCOM
+        select DMA_ENGINE
+        select DMA_VIRTUAL_CHANNELS
+        help
+          Enable support for the QCOM GPI DMA controller. This controller
+          provides DMA capabilities for a variety of peripheral buses such
+          as I2C, UART, and SPI. By using GPI dmaengine driver, bus drivers
+          can use a standardize interface that is protocol independent to
+          transfer data between DDR and peripheral.
+
 config QCOM_HIDMA_MGMT
 	tristate "Qualcomm Technologies HIDMA Management support"
 	select DMA_ENGINE
diff --git a/drivers/dma/qcom/Makefile b/drivers/dma/qcom/Makefile
index 1ae92da88b0c..f33f027dd0fc 100644
--- a/drivers/dma/qcom/Makefile
+++ b/drivers/dma/qcom/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_QCOM_BAM_DMA) += bam_dma.o
+obj-$(CONFIG_QCOM_GPI_DMA) += gpi.o
 obj-$(CONFIG_QCOM_HIDMA_MGMT) += hdma_mgmt.o
 hdma_mgmt-objs	 := hidma_mgmt.o hidma_mgmt_sys.o
 obj-$(CONFIG_QCOM_HIDMA) +=  hdma.o
diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
new file mode 100644
index 000000000000..592fd39a7740
--- /dev/null
+++ b/drivers/dma/qcom/gpi.c
@@ -0,0 +1,2269 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define DEBUG
+
+#include <linux/bitfield.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/scatterlist.h>
+#include "../dmaengine.h"
+#include "../virt-dma.h"
+
+#define TRE_TYPE_DMA		0x10
+#define TRE_TYPE_GO		0x20
+#define TRE_TYPE_CONFIG0	0x22
+
+/* TRE flags */
+#define TRE_FLAGS_CHAIN		BIT(0)
+#define TRE_FLAGS_IEOB		BIT(8)
+#define TRE_FLAGS_IEOT		BIT(9)
+#define TRE_FLAGS_BEI		BIT(10)
+#define TRE_FLAGS_LINK		BIT(11)
+#define TRE_FLAGS_TYPE		GENMASK(23, 16)
+
+/* SPI CONFIG0 WD0 */
+#define TRE_SPI_C0_WORD_SZ	GENMASK(4, 0)
+#define TRE_SPI_C0_LOOPBACK	BIT(8)
+#define TRE_SPI_C0_CS		BIT(11)
+#define TRE_SPI_C0_CPHA		BIT(12)
+#define TRE_SPI_C0_CPOL		BIT(13)
+#define TRE_SPI_C0_TX_PACK	BIT(24)
+#define TRE_SPI_C0_RX_PACK	BIT(25)
+
+/* SPI CONFIG0 WD2 */
+#define TRE_SPI_C0_CLK_DIV	GENMASK(11, 0)
+#define TRE_SPI_C0_CLK_SRC	GENMASK(19, 16)
+
+/* SPI GO WD0 */
+#define TRE_SPI_GO_CMD		GENMASK(4, 0)
+#define TRE_SPI_GO_CS		GENMASK(10, 8)
+#define TRE_SPI_GO_FRAG		BIT(26)
+#define TRE_SPI_RX_LEN		GENMASK(23, 0)
+
+/* DMA TRE */
+#define TRE_DMA_LEN		GENMASK(23, 0)
+
+/* Register offsets from gpi-top */
+#define GPII_n_CH_k_CNTXT_0_OFFS(n, k)	(0x20000 + (0x4000 * (n)) + (0x80 * (k)))
+#define GPII_n_CH_k_CNTXT_0_EL_SIZE	GENMASK(31, 24)
+#define GPII_n_CH_k_CNTXT_0_CHSTATE	GENMASK(23, 20)
+#define GPII_n_CH_k_CNTXT_0_ERIDX	GENMASK(18, 14)
+#define GPII_n_CH_k_CNTXT_0_DIR		BIT(3)
+#define GPII_n_CH_k_CNTXT_0_PROTO	GENMASK(2, 0)
+
+#define GPII_n_CH_k_CNTXT_0(el_size, erindex, dir, chtype_proto)  \
+	(FIELD_PREP(GPII_n_CH_k_CNTXT_0_EL_SIZE, el_size)	| \
+	 FIELD_PREP(GPII_n_CH_k_CNTXT_0_ERIDX, erindex)		| \
+	 FIELD_PREP(GPII_n_CH_k_CNTXT_0_DIR, dir)		| \
+	 FIELD_PREP(GPII_n_CH_k_CNTXT_0_PROTO, chtype_proto))
+
+#define GPI_CHTYPE_DIR_IN	(0)
+#define GPI_CHTYPE_DIR_OUT	(1)
+
+#define GPI_CHTYPE_PROTO_GPI	(0x2)
+
+#define GPII_n_CH_k_DOORBELL_0_OFFS(n, k)	(0x22000 + (0x4000 * (n)) + (0x8 * (k)))
+#define GPII_n_CH_CMD_OFFS(n)			(0x23008 + (0x4000 * (n)))
+#define GPII_n_CH_CMD_OPCODE			GENMASK(31, 24)
+#define GPII_n_CH_CMD_CHID			GENMASK(7, 0)
+#define GPII_n_CH_CMD(opcode, chid)				 \
+		     (FIELD_PREP(GPII_n_CH_CMD_OPCODE, opcode) | \
+		      FIELD_PREP(GPII_n_CH_CMD_CHID, chid))
+
+#define GPII_n_CH_CMD_ALLOCATE		(0)
+#define GPII_n_CH_CMD_START		(1)
+#define GPII_n_CH_CMD_STOP		(2)
+#define GPII_n_CH_CMD_RESET		(9)
+#define GPII_n_CH_CMD_DE_ALLOC		(10)
+#define GPII_n_CH_CMD_UART_SW_STALE	(32)
+#define GPII_n_CH_CMD_UART_RFR_READY	(33)
+#define GPII_n_CH_CMD_UART_RFR_NOT_READY (34)
+
+/* EV Context Array */
+#define GPII_n_EV_CH_k_CNTXT_0_OFFS(n, k) (0x21000 + (0x4000 * (n)) + (0x80 * (k)))
+#define GPII_n_EV_k_CNTXT_0_EL_SIZE	GENMASK(31, 24)
+#define GPII_n_EV_k_CNTXT_0_CHSTATE	GENMASK(23, 20)
+#define GPII_n_EV_k_CNTXT_0_INTYPE	BIT(16)
+#define GPII_n_EV_k_CNTXT_0_CHTYPE	GENMASK(3, 0)
+
+#define GPII_n_EV_k_CNTXT_0(el_size, inttype, chtype)		\
+	(FIELD_PREP(GPII_n_EV_k_CNTXT_0_EL_SIZE, el_size) |	\
+	 FIELD_PREP(GPII_n_EV_k_CNTXT_0_INTYPE, inttype)  |	\
+	 FIELD_PREP(GPII_n_EV_k_CNTXT_0_CHTYPE, chtype))
+
+#define GPI_INTTYPE_IRQ		(1)
+#define GPI_CHTYPE_GPI_EV	(0x2)
+
+enum CNTXT_OFFS {
+	CNTXT_0_CONFIG = 0x0,
+	CNTXT_1_R_LENGTH = 0x4,
+	CNTXT_2_RING_BASE_LSB = 0x8,
+	CNTXT_3_RING_BASE_MSB = 0xC,
+	CNTXT_4_RING_RP_LSB = 0x10,
+	CNTXT_5_RING_RP_MSB = 0x14,
+	CNTXT_6_RING_WP_LSB = 0x18,
+	CNTXT_7_RING_WP_MSB = 0x1C,
+	CNTXT_8_RING_INT_MOD = 0x20,
+	CNTXT_9_RING_INTVEC = 0x24,
+	CNTXT_10_RING_MSI_LSB = 0x28,
+	CNTXT_11_RING_MSI_MSB = 0x2C,
+	CNTXT_12_RING_RP_UPDATE_LSB = 0x30,
+	CNTXT_13_RING_RP_UPDATE_MSB = 0x34,
+};
+
+#define GPII_n_EV_CH_k_DOORBELL_0_OFFS(n, k)	(0x22100 + (0x4000 * (n)) + (0x8 * (k)))
+#define GPII_n_EV_CH_CMD_OFFS(n)		(0x23010 + (0x4000 * (n)))
+#define GPII_n_EV_CMD_OPCODE			GENMASK(31, 24)
+#define GPII_n_EV_CMD_CHID			GENMASK(7, 0)
+#define GPII_n_EV_CMD(opcode, chid)				 \
+		     (FIELD_PREP(GPII_n_EV_CMD_OPCODE, opcode) | \
+		      FIELD_PREP(GPII_n_EV_CMD_CHID, chid))
+
+#define GPII_n_EV_CH_CMD_ALLOCATE		(0x00)
+#define GPII_n_EV_CH_CMD_RESET			(0x09)
+#define GPII_n_EV_CH_CMD_DE_ALLOC		(0x0A)
+
+#define GPII_n_CNTXT_TYPE_IRQ_OFFS(n)		(0x23080 + (0x4000 * (n)))
+
+/* mask type register */
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_OFFS(n)	(0x23088 + (0x4000 * (n)))
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_BMSK		GENMASK(6, 0)
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_GENERAL	BIT(6)
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB		BIT(3)
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_GLOB		BIT(2)
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL	BIT(1)
+#define GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL	BIT(0)
+
+#define GPII_n_CNTXT_SRC_GPII_CH_IRQ_OFFS(n)	(0x23090 + (0x4000 * (n)))
+#define GPII_n_CNTXT_SRC_EV_CH_IRQ_OFFS(n)	(0x23094 + (0x4000 * (n)))
+
+/* Mask channel control interrupt register */
+#define GPII_n_CNTXT_SRC_CH_IRQ_MSK_OFFS(n)	(0x23098 + (0x4000 * (n)))
+#define GPII_n_CNTXT_SRC_CH_IRQ_MSK_BMSK	GENMASK(1, 0)
+
+/* Mask event control interrupt register */
+#define GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_OFFS(n)	(0x2309C + (0x4000 * (n)))
+#define GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_BMSK	BIT(0)
+
+#define GPII_n_CNTXT_SRC_CH_IRQ_CLR_OFFS(n)	(0x230A0 + (0x4000 * (n)))
+#define GPII_n_CNTXT_SRC_EV_CH_IRQ_CLR_OFFS(n)	(0x230A4 + (0x4000 * (n)))
+
+/* Mask event interrupt register */
+#define GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_OFFS(n)	(0x230B8 + (0x4000 * (n)))
+#define GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_BMSK	BIT(0)
+
+#define GPII_n_CNTXT_SRC_IEOB_IRQ_CLR_OFFS(n)	(0x230C0 + (0x4000 * (n)))
+#define GPII_n_CNTXT_GLOB_IRQ_STTS_OFFS(n)	(0x23100 + (0x4000 * (n)))
+#define GPI_GLOB_IRQ_ERROR_INT_MSK		BIT(0)
+
+/* GPII specific Global - Enable bit register */
+#define GPII_n_CNTXT_GLOB_IRQ_EN_OFFS(n)	(0x23108 + (0x4000 * (n)))
+#define GPII_n_CNTXT_GLOB_IRQ_CLR_OFFS(n)	(0x23110 + (0x4000 * (n)))
+#define GPII_n_CNTXT_GPII_IRQ_STTS_OFFS(n)	(0x23118 + (0x4000 * (n)))
+
+/* GPII general interrupt - Enable bit register */
+#define GPII_n_CNTXT_GPII_IRQ_EN_OFFS(n)	(0x23120 + (0x4000 * (n)))
+#define GPII_n_CNTXT_GPII_IRQ_EN_BMSK		GENMASK(3, 0)
+
+#define GPII_n_CNTXT_GPII_IRQ_CLR_OFFS(n)	(0x23128 + (0x4000 * (n)))
+
+/* GPII Interrupt Type register */
+#define GPII_n_CNTXT_INTSET_OFFS(n)		(0x23180 + (0x4000 * (n)))
+#define GPII_n_CNTXT_INTSET_BMSK		BIT(0)
+
+#define GPII_n_CNTXT_MSI_BASE_LSB_OFFS(n)	(0x23188 + (0x4000 * (n)))
+#define GPII_n_CNTXT_MSI_BASE_MSB_OFFS(n)	(0x2318C + (0x4000 * (n)))
+#define GPII_n_CNTXT_SCRATCH_0_OFFS(n)		(0x23400 + (0x4000 * (n)))
+#define GPII_n_CNTXT_SCRATCH_1_OFFS(n)		(0x23404 + (0x4000 * (n)))
+
+#define GPII_n_ERROR_LOG_OFFS(n)		(0x23200 + (0x4000 * (n)))
+
+/* QOS Registers */
+#define GPII_n_CH_k_QOS_OFFS(n, k)		(0x2005C + (0x4000 * (n)) + (0x80 * (k)))
+
+/* Scratch registeres */
+#define GPII_n_CH_k_SCRATCH_0_OFFS(n, k)	(0x20060 + (0x4000 * (n)) + (0x80 * (k)))
+#define GPII_n_CH_k_SCRATCH_0_SEID		GENMASK(2, 0)
+#define GPII_n_CH_k_SCRATCH_0_PROTO		GENMASK(7, 4)
+#define GPII_n_CH_k_SCRATCH_0_PAIR		GENMASK(20, 16)
+#define GPII_n_CH_k_SCRATCH_0(pair, proto, seid)		\
+			     (FIELD_PREP(GPII_n_CH_k_SCRATCH_0_PAIR, pair)	| \
+			      FIELD_PREP(GPII_n_CH_k_SCRATCH_0_PROTO, proto)	| \
+			      FIELD_PREP(GPII_n_CH_k_SCRATCH_0_SEID, seid))
+#define GPII_n_CH_k_SCRATCH_1_OFFS(n, k)	(0x20064 + (0x4000 * (n)) + (0x80 * (k)))
+#define GPII_n_CH_k_SCRATCH_2_OFFS(n, k)	(0x20068 + (0x4000 * (n)) + (0x80 * (k)))
+#define GPII_n_CH_k_SCRATCH_3_OFFS(n, k)	(0x2006C + (0x4000 * (n)) + (0x80 * (k)))
+
+struct __packed gpi_tre {
+	u32 dword[4];
+};
+
+enum EV_PRIORITY {
+	EV_PRIORITY_ISR,
+	EV_PRIORITY_TASKLET,
+};
+
+enum msm_gpi_tce_code {
+	MSM_GPI_TCE_SUCCESS = 1,
+	MSM_GPI_TCE_EOT = 2,
+	MSM_GPI_TCE_EOB = 4,
+	MSM_GPI_TCE_UNEXP_ERR = 16,
+};
+
+#define CMD_TIMEOUT_MS		(250)
+
+#define MAX_CHANNELS_PER_GPII	(2)
+#define GPI_TX_CHAN		(0)
+#define GPI_RX_CHAN		(1)
+#define STATE_IGNORE		(U32_MAX)
+#define REQ_OF_DMA_ARGS		(5) /* # of arguments required from client */
+
+struct __packed gpi_error_log_entry {
+	u32 routine:4;
+	u32 type:4;
+	u32 reserved0:4;
+	u32 code:4;
+	u32 reserved1:3;
+	u32 chid:5;
+	u32 reserved2:1;
+	u32 chtype:1;
+	u32 ee:1;
+};
+
+struct __packed xfer_compl_event {
+	u64 ptr;
+	u32 length:24;
+	u8 code;
+	u16 status;
+	u8 type;
+	u8 chid;
+};
+
+struct __packed immediate_data_event {
+	u8 data_bytes[8];
+	u8 length:4;
+	u8 resvd:4;
+	u16 tre_index;
+	u8 code;
+	u16 status;
+	u8 type;
+	u8 chid;
+};
+
+struct __packed qup_notif_event {
+	u32 status;
+	u32 time;
+	u32 count:24;
+	u8 resvd;
+	u16 resvd1;
+	u8 type;
+	u8 chid;
+};
+
+struct __packed gpi_ere {
+	u32 dword[4];
+};
+
+enum GPI_EV_TYPE {
+	XFER_COMPLETE_EV_TYPE = 0x22,
+	IMMEDIATE_DATA_EV_TYPE = 0x30,
+	QUP_NOTIF_EV_TYPE = 0x31,
+	STALE_EV_TYPE = 0xFF,
+};
+
+union __packed gpi_event {
+	struct __packed xfer_compl_event xfer_compl_event;
+	struct __packed immediate_data_event immediate_data_event;
+	struct __packed qup_notif_event qup_notif_event;
+	struct __packed gpi_ere gpi_ere;
+};
+
+enum gpii_irq_settings {
+	DEFAULT_IRQ_SETTINGS,
+	MASK_IEOB_SETTINGS,
+};
+
+enum gpi_ev_state {
+	DEFAULT_EV_CH_STATE = 0,
+	EV_STATE_NOT_ALLOCATED = DEFAULT_EV_CH_STATE,
+	EV_STATE_ALLOCATED,
+	MAX_EV_STATES
+};
+
+static const char *const gpi_ev_state_str[MAX_EV_STATES] = {
+	[EV_STATE_NOT_ALLOCATED] = "NOT ALLOCATED",
+	[EV_STATE_ALLOCATED] = "ALLOCATED",
+};
+
+#define TO_GPI_EV_STATE_STR(_state) (((_state) >= MAX_EV_STATES) ? \
+				    "INVALID" : gpi_ev_state_str[(_state)])
+
+enum gpi_ch_state {
+	DEFAULT_CH_STATE = 0x0,
+	CH_STATE_NOT_ALLOCATED = DEFAULT_CH_STATE,
+	CH_STATE_ALLOCATED = 0x1,
+	CH_STATE_STARTED = 0x2,
+	CH_STATE_STOPPED = 0x3,
+	CH_STATE_STOP_IN_PROC = 0x4,
+	CH_STATE_ERROR = 0xf,
+	MAX_CH_STATES
+};
+
+enum gpi_cmd {
+	GPI_CH_CMD_BEGIN,
+	GPI_CH_CMD_ALLOCATE = GPI_CH_CMD_BEGIN,
+	GPI_CH_CMD_START,
+	GPI_CH_CMD_STOP,
+	GPI_CH_CMD_RESET,
+	GPI_CH_CMD_DE_ALLOC,
+	GPI_CH_CMD_UART_SW_STALE,
+	GPI_CH_CMD_UART_RFR_READY,
+	GPI_CH_CMD_UART_RFR_NOT_READY,
+	GPI_CH_CMD_END = GPI_CH_CMD_UART_RFR_NOT_READY,
+	GPI_EV_CMD_BEGIN,
+	GPI_EV_CMD_ALLOCATE = GPI_EV_CMD_BEGIN,
+	GPI_EV_CMD_RESET,
+	GPI_EV_CMD_DEALLOC,
+	GPI_EV_CMD_END = GPI_EV_CMD_DEALLOC,
+	GPI_MAX_CMD,
+};
+
+#define IS_CHAN_CMD(_cmd) ((_cmd) <= GPI_CH_CMD_END)
+
+static const char *const gpi_cmd_str[GPI_MAX_CMD] = {
+	[GPI_CH_CMD_ALLOCATE] = "CH ALLOCATE",
+	[GPI_CH_CMD_START] = "CH START",
+	[GPI_CH_CMD_STOP] = "CH STOP",
+	[GPI_CH_CMD_RESET] = "CH_RESET",
+	[GPI_CH_CMD_DE_ALLOC] = "DE ALLOC",
+	[GPI_CH_CMD_UART_SW_STALE] = "UART SW STALE",
+	[GPI_CH_CMD_UART_RFR_READY] = "UART RFR READY",
+	[GPI_CH_CMD_UART_RFR_NOT_READY] = "UART RFR NOT READY",
+	[GPI_EV_CMD_ALLOCATE] = "EV ALLOCATE",
+	[GPI_EV_CMD_RESET] = "EV RESET",
+	[GPI_EV_CMD_DEALLOC] = "EV DEALLOC",
+};
+
+#define TO_GPI_CMD_STR(_cmd) (((_cmd) >= GPI_MAX_CMD) ? "INVALID" : \
+				  gpi_cmd_str[(_cmd)])
+
+enum se_protocol {
+	SE_PROTOCOL_SPI = 1,
+	SE_PROTOCOL_UART = 2,
+	SE_PROTOCOL_I2C = 3,
+	SE_MAX_PROTOCOL
+};
+
+/*
+ * @DISABLE_STATE: no register access allowed
+ * @CONFIG_STATE:  client has configured the channel
+ * @PREP_HARDWARE: register access is allowed
+ *		   however, no processing EVENTS
+ * @ACTIVE_STATE: channels are fully operational
+ * @PREPARE_TERIMNATE: graceful termination of channels
+ *		       register access is allowed
+ * @PAUSE_STATE: channels are active, but not processing any events
+ */
+enum gpi_pm_state {
+	DISABLE_STATE,
+	CONFIG_STATE,
+	PREPARE_HARDWARE,
+	ACTIVE_STATE,
+	PREPARE_TERMINATE,
+	PAUSE_STATE,
+	MAX_PM_STATE
+};
+
+#define REG_ACCESS_VALID(_pm_state) ((_pm_state) >= PREPARE_HARDWARE)
+
+static const char *const gpi_pm_state_str[MAX_PM_STATE] = {
+	[DISABLE_STATE] = "DISABLE",
+	[CONFIG_STATE] = "CONFIG",
+	[PREPARE_HARDWARE] = "PREPARE HARDWARE",
+	[ACTIVE_STATE] = "ACTIVE",
+	[PREPARE_TERMINATE] = "PREPARE TERMINATE",
+	[PAUSE_STATE] = "PAUSE",
+};
+
+#define TO_GPI_PM_STR(_state) (((_state) >= MAX_PM_STATE) ? \
+			      "INVALID" : gpi_pm_state_str[(_state)])
+
+static const struct {
+	enum gpi_cmd gpi_cmd;
+	u32 opcode;
+	u32 state;
+} gpi_cmd_info[GPI_MAX_CMD] = {
+	{
+		GPI_CH_CMD_ALLOCATE,
+		GPII_n_CH_CMD_ALLOCATE,
+		CH_STATE_ALLOCATED,
+	},
+	{
+		GPI_CH_CMD_START,
+		GPII_n_CH_CMD_START,
+		CH_STATE_STARTED,
+	},
+	{
+		GPI_CH_CMD_STOP,
+		GPII_n_CH_CMD_STOP,
+		CH_STATE_STOPPED,
+	},
+	{
+		GPI_CH_CMD_RESET,
+		GPII_n_CH_CMD_RESET,
+		CH_STATE_ALLOCATED,
+	},
+	{
+		GPI_CH_CMD_DE_ALLOC,
+		GPII_n_CH_CMD_DE_ALLOC,
+		CH_STATE_NOT_ALLOCATED,
+	},
+	{
+		GPI_CH_CMD_UART_SW_STALE,
+		GPII_n_CH_CMD_UART_SW_STALE,
+		STATE_IGNORE,
+	},
+	{
+		GPI_CH_CMD_UART_RFR_READY,
+		GPII_n_CH_CMD_UART_RFR_READY,
+		STATE_IGNORE,
+	},
+	{
+		GPI_CH_CMD_UART_RFR_NOT_READY,
+		GPII_n_CH_CMD_UART_RFR_NOT_READY,
+		STATE_IGNORE,
+	},
+	{
+		GPI_EV_CMD_ALLOCATE,
+		GPII_n_EV_CH_CMD_ALLOCATE,
+		EV_STATE_ALLOCATED,
+	},
+	{
+		GPI_EV_CMD_RESET,
+		GPII_n_EV_CH_CMD_RESET,
+		EV_STATE_ALLOCATED,
+	},
+	{
+		GPI_EV_CMD_DEALLOC,
+		GPII_n_EV_CH_CMD_DE_ALLOC,
+		EV_STATE_NOT_ALLOCATED,
+	},
+};
+
+struct gpi_ring {
+	void *pre_aligned;
+	size_t alloc_size;
+	phys_addr_t phys_addr;
+	dma_addr_t dma_handle;
+	void *base;
+	void *wp;
+	void *rp;
+	u32 len;
+	u32 el_size;
+	u32 elements;
+	bool configured;
+};
+
+struct gpi_dev {
+	struct dma_device dma_device;
+	struct device *dev;
+	struct resource *res;
+	void __iomem *regs;
+	void *ee_base; /*ee register base address*/
+	u32 max_gpii; /* maximum # of gpii instances available per gpi block */
+	u32 gpii_mask; /* gpii instances available for apps */
+	u32 ev_factor; /* ev ring length factor */
+	u32 smmu_cfg;
+	dma_addr_t iova_base;
+	size_t iova_size;
+	struct gpii *gpiis;
+};
+
+struct reg_info {
+	char *name;
+	u32 offset;
+	u32 val;
+};
+
+struct gpi_reg_table {
+	u64 timestamp;
+	struct reg_info *ev_cntxt_info;
+	struct reg_info *chan[MAX_CHANNELS_PER_GPII];
+	struct reg_info *gpi_debug_regs;
+	struct reg_info *gpii_cntxt;
+	struct reg_info *gpi_debug_qsb_regs;
+	u32 ev_scratch_0;
+	u32 ch_scratch_0[MAX_CHANNELS_PER_GPII];
+	void *ev_ring;
+	u32 ev_ring_len;
+	void *ch_ring[MAX_CHANNELS_PER_GPII];
+	u32 ch_ring_len[MAX_CHANNELS_PER_GPII];
+	u32 error_log;
+};
+
+struct gpii_chan {
+	struct virt_dma_chan vc;
+	u32 chid;
+	u32 seid;
+	enum se_protocol protocol;
+	enum EV_PRIORITY priority; /* comes from clients DT node */
+	struct gpii *gpii;
+	enum gpi_ch_state ch_state;
+	enum gpi_pm_state pm_state;
+	void __iomem *ch_cntxt_base_reg;
+	void __iomem *ch_cntxt_db_reg;
+	void __iomem *ch_cmd_reg;
+	u32 req_tres; /* # of tre's client requested */
+	u32 dir;
+	struct gpi_ring ch_ring;
+	struct dma_slave_config config;
+};
+
+struct gpii {
+	u32 gpii_id;
+	struct gpii_chan gpii_chan[MAX_CHANNELS_PER_GPII];
+	struct gpi_dev *gpi_dev;
+	enum EV_PRIORITY ev_priority;
+	enum se_protocol protocol;
+	int irq;
+	void __iomem *regs; /* points to gpi top */
+	void __iomem *ev_cntxt_base_reg;
+	void __iomem *ev_cntxt_db_reg;
+	void __iomem *ev_ring_rp_lsb_reg;
+	void __iomem *ev_cmd_reg;
+	void __iomem *ieob_clr_reg;
+	struct mutex ctrl_lock;
+	enum gpi_ev_state ev_state;
+	bool configured_irq;
+	enum gpi_pm_state pm_state;
+	rwlock_t pm_lock;
+	struct gpi_ring ev_ring;
+	struct tasklet_struct ev_task; /* event processing tasklet */
+	struct completion cmd_completion;
+	enum gpi_cmd gpi_cmd;
+	u32 cntxt_type_irq_msk;
+	bool ieob_set;
+};
+
+#define MAX_TRE 3
+
+struct gpi_desc {
+	struct virt_dma_desc vd;
+	size_t len;
+	void *wp; /* points to TRE last queued during issue_pending */
+	void *db; /* DB register to program */
+	struct gpii_chan *gpii_chan;
+	struct gpi_tre tre[MAX_TRE];
+};
+
+const u32 GPII_CHAN_DIR[MAX_CHANNELS_PER_GPII] = {
+	GPI_CHTYPE_DIR_OUT, GPI_CHTYPE_DIR_IN
+};
+
+static irqreturn_t gpi_handle_irq(int irq, void *data);
+static void gpi_ring_recycle_ev_element(struct gpi_ring *ring);
+static int gpi_ring_add_element(struct gpi_ring *ring, void **wp);
+static void gpi_process_events(struct gpii *gpii);
+
+static inline struct gpii_chan *to_gpii_chan(struct dma_chan *dma_chan)
+{
+	return container_of(dma_chan, struct gpii_chan, vc.chan);
+}
+
+static inline struct gpi_desc *to_gpi_desc(struct virt_dma_desc *vd)
+{
+	return container_of(vd, struct gpi_desc, vd);
+}
+
+static inline phys_addr_t to_physical(const struct gpi_ring *const ring,
+				      void *addr)
+{
+	return ring->phys_addr + (addr - ring->base);
+}
+
+static inline void *to_virtual(const struct gpi_ring *const ring, phys_addr_t addr)
+{
+	return ring->base + (addr - ring->phys_addr);
+}
+
+static inline u32 gpi_read_reg(struct gpii *gpii, void __iomem *addr)
+{
+	return readl_relaxed(addr);
+}
+
+static inline void gpi_write_reg(struct gpii *gpii, void __iomem *addr, u32 val)
+{
+	writel_relaxed(val, addr);
+}
+
+/* gpi_write_reg_field - write to specific bit field */
+static inline void gpi_write_reg_field(struct gpii *gpii, void __iomem *addr,
+				       u32 mask, u32 shift, u32 val)
+{
+	u32 tmp = gpi_read_reg(gpii, addr);
+
+	tmp &= ~mask;
+	val = tmp | ((val << shift) & mask);
+	gpi_write_reg(gpii, addr, val);
+}
+
+static inline void
+gpi_update_reg(struct gpii *gpii, u32 offset, u32 mask, u32 val)
+{
+	void __iomem *addr = gpii->regs + offset;
+	u32 tmp = gpi_read_reg(gpii, addr);
+
+	tmp &= ~mask;
+	tmp |= u32_encode_bits(val, mask);
+
+	gpi_write_reg(gpii, addr, tmp);
+}
+
+static void gpi_disable_interrupts(struct gpii *gpii)
+{
+	gpi_update_reg(gpii, GPII_n_CNTXT_TYPE_IRQ_MSK_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_TYPE_IRQ_MSK_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_SRC_CH_IRQ_MSK_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_SRC_CH_IRQ_MSK_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_GLOB_IRQ_EN_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_GPII_IRQ_EN_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_GPII_IRQ_EN_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_GPII_IRQ_EN_BMSK, 0);
+	gpi_update_reg(gpii, GPII_n_CNTXT_INTSET_OFFS(gpii->gpii_id),
+		       GPII_n_CNTXT_INTSET_BMSK, 0);
+
+	gpii->cntxt_type_irq_msk = 0;
+	devm_free_irq(gpii->gpi_dev->dev, gpii->irq, gpii);
+	gpii->configured_irq = false;
+}
+
+/* configure and enable interrupts */
+static int gpi_config_interrupts(struct gpii *gpii, enum gpii_irq_settings settings, bool mask)
+{
+	const u32 enable = (GPII_n_CNTXT_TYPE_IRQ_MSK_GENERAL |
+			      GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB |
+			      GPII_n_CNTXT_TYPE_IRQ_MSK_GLOB |
+			      GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL |
+			      GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL);
+	int ret;
+
+	if (!gpii->configured_irq) {
+		ret = devm_request_irq(gpii->gpi_dev->dev, gpii->irq,
+				       gpi_handle_irq, IRQF_TRIGGER_HIGH,
+				       "gpi-dma", gpii);
+		if (ret < 0) {
+			dev_err(gpii->gpi_dev->dev, "error request irq:%d ret:%d\n",
+				gpii->irq, ret);
+			return ret;
+		}
+	}
+
+	if (settings == MASK_IEOB_SETTINGS) {
+		/*
+		 * GPII only uses one EV ring per gpii so we can globally
+		 * enable/disable IEOB interrupt
+		 */
+		if (mask)
+			gpii->cntxt_type_irq_msk |= GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB;
+		else
+			gpii->cntxt_type_irq_msk &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB);
+		gpi_update_reg(gpii, GPII_n_CNTXT_TYPE_IRQ_MSK_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_TYPE_IRQ_MSK_BMSK, gpii->cntxt_type_irq_msk);
+	} else {
+		gpi_update_reg(gpii, GPII_n_CNTXT_TYPE_IRQ_MSK_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_TYPE_IRQ_MSK_BMSK, enable);
+		gpi_update_reg(gpii, GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_BMSK,
+			       GPII_n_CNTXT_SRC_IEOB_IRQ_MSK_BMSK);
+		gpi_update_reg(gpii, GPII_n_CNTXT_SRC_CH_IRQ_MSK_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_SRC_CH_IRQ_MSK_BMSK,
+			       GPII_n_CNTXT_SRC_CH_IRQ_MSK_BMSK);
+		gpi_update_reg(gpii, GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_BMSK,
+			       GPII_n_CNTXT_SRC_EV_CH_IRQ_MSK_BMSK);
+		gpi_update_reg(gpii, GPII_n_CNTXT_GLOB_IRQ_EN_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_GPII_IRQ_EN_BMSK,
+			       GPII_n_CNTXT_GPII_IRQ_EN_BMSK);
+		gpi_update_reg(gpii, GPII_n_CNTXT_GPII_IRQ_EN_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_GPII_IRQ_EN_BMSK, GPII_n_CNTXT_GPII_IRQ_EN_BMSK);
+		gpi_update_reg(gpii, GPII_n_CNTXT_MSI_BASE_LSB_OFFS(gpii->gpii_id), U32_MAX, 0);
+		gpi_update_reg(gpii, GPII_n_CNTXT_MSI_BASE_MSB_OFFS(gpii->gpii_id), U32_MAX, 0);
+		gpi_update_reg(gpii, GPII_n_CNTXT_SCRATCH_0_OFFS(gpii->gpii_id), U32_MAX, 0);
+		gpi_update_reg(gpii, GPII_n_CNTXT_SCRATCH_1_OFFS(gpii->gpii_id), U32_MAX, 0);
+		gpi_update_reg(gpii, GPII_n_CNTXT_INTSET_OFFS(gpii->gpii_id),
+			       GPII_n_CNTXT_INTSET_BMSK, 1);
+		gpi_update_reg(gpii, GPII_n_ERROR_LOG_OFFS(gpii->gpii_id), U32_MAX, 0);
+
+		gpii->cntxt_type_irq_msk = enable;
+	}
+
+	gpii->configured_irq = true;
+	return 0;
+}
+
+/* Sends gpii event or channel command */
+static int gpi_send_cmd(struct gpii *gpii, struct gpii_chan *gpii_chan,
+			enum gpi_cmd gpi_cmd)
+{
+	u32 chid = MAX_CHANNELS_PER_GPII;
+	unsigned long timeout;
+	void __iomem *cmd_reg;
+	u32 cmd;
+
+	if (gpi_cmd >= GPI_MAX_CMD)
+		return -EINVAL;
+	if (IS_CHAN_CMD(gpi_cmd))
+		chid = gpii_chan->chid;
+
+	dev_dbg(gpii->gpi_dev->dev,
+		"sending cmd: %s:%u\n", TO_GPI_CMD_STR(gpi_cmd), chid);
+
+	/* send opcode and wait for completion */
+	reinit_completion(&gpii->cmd_completion);
+	gpii->gpi_cmd = gpi_cmd;
+
+	cmd_reg = IS_CHAN_CMD(gpi_cmd) ? gpii_chan->ch_cmd_reg : gpii->ev_cmd_reg;
+	cmd = IS_CHAN_CMD(gpi_cmd) ? GPII_n_CH_CMD(gpi_cmd_info[gpi_cmd].opcode, chid) :
+				     GPII_n_EV_CMD(gpi_cmd_info[gpi_cmd].opcode, 0);
+	gpi_write_reg(gpii, cmd_reg, cmd);
+	timeout = wait_for_completion_timeout(&gpii->cmd_completion,
+					      msecs_to_jiffies(CMD_TIMEOUT_MS));
+	if (!timeout) {
+		dev_err(gpii->gpi_dev->dev, "cmd: %s completion timeout:%u\n",
+			TO_GPI_CMD_STR(gpi_cmd), chid);
+		return -EIO;
+	}
+
+	/* confirm new ch state is correct , if the cmd is a state change cmd */
+	if (gpi_cmd_info[gpi_cmd].state == STATE_IGNORE)
+		return 0;
+
+	if (IS_CHAN_CMD(gpi_cmd) && gpii_chan->ch_state == gpi_cmd_info[gpi_cmd].state)
+		return 0;
+
+	if (!IS_CHAN_CMD(gpi_cmd) && gpii->ev_state == gpi_cmd_info[gpi_cmd].state)
+		return 0;
+
+	return -EIO;
+}
+
+/* program transfer ring DB register */
+static inline void gpi_write_ch_db(struct gpii_chan *gpii_chan,
+				   struct gpi_ring *ring, void *wp)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	phys_addr_t p_wp;
+
+	p_wp = to_physical(ring, wp);
+	gpi_write_reg(gpii, gpii_chan->ch_cntxt_db_reg, p_wp);
+}
+
+/* program event ring DB register */
+static inline void gpi_write_ev_db(struct gpii *gpii,
+				   struct gpi_ring *ring, void *wp)
+{
+	phys_addr_t p_wp;
+
+	p_wp = ring->phys_addr + (wp - ring->base);
+	gpi_write_reg(gpii, gpii->ev_cntxt_db_reg, p_wp);
+}
+
+/* process transfer completion interrupt */
+static void gpi_process_ieob(struct gpii *gpii)
+{
+	gpi_write_reg(gpii, gpii->ieob_clr_reg, BIT(0));
+
+	/* process events based on priority */
+	if (likely(gpii->ev_priority >= EV_PRIORITY_TASKLET)) {
+		gpi_config_interrupts(gpii, MASK_IEOB_SETTINGS, 0);
+		tasklet_schedule(&gpii->ev_task);
+	} else {
+		gpi_process_events(gpii);
+	}
+}
+
+/* process channel control interrupt */
+static void gpi_process_ch_ctrl_irq(struct gpii *gpii)
+{
+	u32 gpii_id = gpii->gpii_id;
+	u32 offset = GPII_n_CNTXT_SRC_GPII_CH_IRQ_OFFS(gpii_id);
+	u32 ch_irq = gpi_read_reg(gpii, gpii->regs + offset);
+	struct gpii_chan *gpii_chan;
+	u32 chid, state;
+
+	/* clear the status */
+	offset = GPII_n_CNTXT_SRC_CH_IRQ_CLR_OFFS(gpii_id);
+	gpi_write_reg(gpii, gpii->regs + offset, (u32)ch_irq);
+
+	for (chid = 0; chid < MAX_CHANNELS_PER_GPII; chid++) {
+		if (!(BIT(chid) & ch_irq))
+			continue;
+
+		gpii_chan = &gpii->gpii_chan[chid];
+		state = gpi_read_reg(gpii, gpii_chan->ch_cntxt_base_reg +
+				     CNTXT_0_CONFIG);
+		state = FIELD_GET(GPII_n_CH_k_CNTXT_0_CHSTATE, state);
+
+		/*
+		 * CH_CMD_DEALLOC cmd always successful. However cmd does
+		 * not change hardware status. So overwriting software state
+		 * to default state.
+		 */
+		if (gpii->gpi_cmd == GPI_CH_CMD_DE_ALLOC)
+			state = DEFAULT_CH_STATE;
+		gpii_chan->ch_state = state;
+
+		/*
+		 * Triggering complete all if ch_state is not a stop in process.
+		 * Stop in process is a transition state and we will wait for
+		 * stop interrupt before notifying.
+		 */
+		if (gpii_chan->ch_state != CH_STATE_STOP_IN_PROC)
+			complete_all(&gpii->cmd_completion);
+	}
+}
+
+/* processing gpi general error interrupts */
+static void gpi_process_gen_err_irq(struct gpii *gpii)
+{
+	u32 gpii_id = gpii->gpii_id;
+	u32 offset = GPII_n_CNTXT_GPII_IRQ_STTS_OFFS(gpii_id);
+	u32 irq_stts = gpi_read_reg(gpii, gpii->regs + offset);
+
+	/* clear the status */
+	dev_dbg(gpii->gpi_dev->dev, "irq_stts:0x%x\n", irq_stts);
+
+	/* Clear the register */
+	offset = GPII_n_CNTXT_GPII_IRQ_CLR_OFFS(gpii_id);
+	gpi_write_reg(gpii, gpii->regs + offset, irq_stts);
+}
+
+/* processing gpi level error interrupts */
+static void gpi_process_glob_err_irq(struct gpii *gpii)
+{
+	u32 gpii_id = gpii->gpii_id;
+	u32 offset = GPII_n_CNTXT_GLOB_IRQ_STTS_OFFS(gpii_id);
+	u32 irq_stts = gpi_read_reg(gpii, gpii->regs + offset);
+	u32 error_log, chid;
+
+	offset = GPII_n_CNTXT_GLOB_IRQ_CLR_OFFS(gpii_id);
+	gpi_write_reg(gpii, gpii->regs + offset, irq_stts);
+
+	/* only error interrupt should be set */
+	if (irq_stts & ~GPI_GLOB_IRQ_ERROR_INT_MSK) {
+		dev_err(gpii->gpi_dev->dev, "invalid error status:0x%x\n", irq_stts);
+		return;
+	}
+
+	offset = GPII_n_ERROR_LOG_OFFS(gpii_id);
+	error_log = gpi_read_reg(gpii, gpii->regs + offset);
+	gpi_write_reg(gpii, gpii->regs + offset, 0);
+
+	/* get channel info */
+	chid = ((struct gpi_error_log_entry *)&error_log)->chid;
+	if (unlikely(chid >= MAX_CHANNELS_PER_GPII))
+		dev_err(gpii->gpi_dev->dev, "invalid chid reported:%u\n",  chid);
+}
+
+/* gpii interrupt handler */
+static irqreturn_t gpi_handle_irq(int irq, void *data)
+{
+	struct gpii *gpii = data;
+	u32 gpii_id = gpii->gpii_id;
+	u32 type, offset;
+	unsigned long flags;
+
+	read_lock_irqsave(&gpii->pm_lock, flags);
+
+	/*
+	 * States are out of sync to receive interrupt
+	 * while software state is in DISABLE state, bailing out.
+	 */
+	if (!REG_ACCESS_VALID(gpii->pm_state)) {
+		dev_err(gpii->gpi_dev->dev, "receive interrupt while in %s state\n",
+			TO_GPI_PM_STR(gpii->pm_state));
+		goto exit_irq;
+	}
+
+	offset = GPII_n_CNTXT_TYPE_IRQ_OFFS(gpii->gpii_id);
+	type = gpi_read_reg(gpii, gpii->regs + offset);
+
+	do {
+		/* global gpii error */
+		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_GLOB) {
+			gpi_process_glob_err_irq(gpii);
+			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_GLOB);
+		}
+
+		/* transfer complete interrupt */
+		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB) {
+			gpi_process_ieob(gpii);
+			type &= ~GPII_n_CNTXT_TYPE_IRQ_MSK_IEOB;
+		}
+
+		/* event control irq */
+		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL) {
+			u32 ev_state;
+			u32 ev_ch_irq;
+
+			dev_dbg(gpii->gpi_dev->dev,
+				"processing EV CTRL interrupt\n");
+			offset = GPII_n_CNTXT_SRC_EV_CH_IRQ_OFFS(gpii_id);
+			ev_ch_irq = gpi_read_reg(gpii, gpii->regs + offset);
+
+			offset = GPII_n_CNTXT_SRC_EV_CH_IRQ_CLR_OFFS
+				(gpii_id);
+			gpi_write_reg(gpii, gpii->regs + offset, ev_ch_irq);
+			ev_state = gpi_read_reg(gpii, gpii->ev_cntxt_base_reg +
+						CNTXT_0_CONFIG);
+			ev_state = FIELD_GET(GPII_n_EV_k_CNTXT_0_CHSTATE, ev_state);
+
+			/*
+			 * CMD EV_CMD_DEALLOC is always successful. However
+			 * cmd does not change hardware status. So overwriting
+			 * software state to default state.
+			 */
+			if (gpii->gpi_cmd == GPI_EV_CMD_DEALLOC)
+				ev_state = DEFAULT_EV_CH_STATE;
+
+			gpii->ev_state = ev_state;
+			dev_dbg(gpii->gpi_dev->dev, "setting EV state to %s\n",
+				TO_GPI_EV_STATE_STR(gpii->ev_state));
+			complete_all(&gpii->cmd_completion);
+			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL);
+		}
+
+		/* channel control irq */
+		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL) {
+			dev_dbg(gpii->gpi_dev->dev, "process CH CTRL interrupts\n");
+			gpi_process_ch_ctrl_irq(gpii);
+			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL);
+		}
+
+		if (type) {
+			dev_err(gpii->gpi_dev->dev, "Unhandled interrupt status:0x%x\n", type);
+			gpi_process_gen_err_irq(gpii);
+			goto exit_irq;
+		}
+
+		offset = GPII_n_CNTXT_TYPE_IRQ_OFFS(gpii->gpii_id);
+		type = gpi_read_reg(gpii, gpii->regs + offset);
+	} while (type);
+
+exit_irq:
+	read_unlock_irqrestore(&gpii->pm_lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+/* process DMA Immediate completion data events */
+static void gpi_process_imed_data_event(struct gpii_chan *gpii_chan,
+					struct immediate_data_event *imed_event)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	struct gpi_ring *ch_ring = &gpii_chan->ch_ring;
+	void *tre = ch_ring->base + (ch_ring->el_size * imed_event->tre_index);
+	struct dmaengine_result result;
+	struct gpi_desc *gpi_desc;
+	struct virt_dma_desc *vd;
+	unsigned long flags;
+	u32 chid;
+
+	/*
+	 * If channel not active don't process event
+	 */
+	if (gpii_chan->pm_state != ACTIVE_STATE) {
+		dev_err(gpii->gpi_dev->dev, "skipping processing event because ch @ %s state\n",
+			TO_GPI_PM_STR(gpii_chan->pm_state));
+		return;
+	}
+
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	vd = vchan_next_desc(&gpii_chan->vc);
+	if (!vd) {
+		struct gpi_ere *gpi_ere;
+		struct gpi_tre *gpi_tre;
+
+		spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+		dev_dbg(gpii->gpi_dev->dev, "event without a pending descriptor!\n");
+		gpi_ere = (struct gpi_ere *)imed_event;
+		dev_dbg(gpii->gpi_dev->dev,
+			"Event: %08x %08x %08x %08x\n",
+			gpi_ere->dword[0], gpi_ere->dword[1],
+			gpi_ere->dword[2], gpi_ere->dword[3]);
+		gpi_tre = tre;
+		dev_dbg(gpii->gpi_dev->dev,
+			"Pending TRE: %08x %08x %08x %08x\n",
+			gpi_tre->dword[0], gpi_tre->dword[1],
+			gpi_tre->dword[2], gpi_tre->dword[3]);
+		return;
+	}
+	gpi_desc = to_gpi_desc(vd);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+
+	/*
+	 * RP pointed by Event is to last TRE processed,
+	 * we need to update ring rp to tre + 1
+	 */
+	tre += ch_ring->el_size;
+	if (tre >= (ch_ring->base + ch_ring->len))
+		tre = ch_ring->base;
+	ch_ring->rp = tre;
+
+	/* make sure rp updates are immediately visible to all cores */
+	smp_wmb();
+
+	chid = imed_event->chid;
+	if (imed_event->code == MSM_GPI_TCE_EOT && gpii->ieob_set) {
+		if (chid == GPI_RX_CHAN)
+			goto gpi_free_desc;
+		else
+			return;
+	}
+
+	if (imed_event->code == MSM_GPI_TCE_UNEXP_ERR)
+		result.result = DMA_TRANS_ABORTED;
+	else
+		result.result = DMA_TRANS_NOERROR;
+	result.residue = gpi_desc->len - imed_event->length;
+
+	dma_cookie_complete(&vd->tx);
+	dmaengine_desc_get_callback_invoke(&vd->tx, &result);
+
+gpi_free_desc:
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	list_del(&vd->node);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+	kfree(gpi_desc);
+	gpi_desc = NULL;
+}
+
+/* processing transfer completion events */
+static void gpi_process_xfer_compl_event(struct gpii_chan *gpii_chan,
+					 struct xfer_compl_event *compl_event)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	struct gpi_ring *ch_ring = &gpii_chan->ch_ring;
+	void *ev_rp = to_virtual(ch_ring, compl_event->ptr);
+	struct virt_dma_desc *vd;
+	struct gpi_desc *gpi_desc;
+	struct dmaengine_result result;
+	unsigned long flags;
+	u32 chid;
+
+	/* only process events on active channel */
+	if (unlikely(gpii_chan->pm_state != ACTIVE_STATE)) {
+		dev_err(gpii->gpi_dev->dev, "skipping processing event because ch @ %s state\n",
+			TO_GPI_PM_STR(gpii_chan->pm_state));
+		return;
+	}
+
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	vd = vchan_next_desc(&gpii_chan->vc);
+	if (!vd) {
+		struct gpi_ere *gpi_ere;
+
+		spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+		dev_err(gpii->gpi_dev->dev, "Event without a pending descriptor!\n");
+		gpi_ere = (struct gpi_ere *)compl_event;
+		dev_err(gpii->gpi_dev->dev,
+			"Event: %08x %08x %08x %08x\n",
+			gpi_ere->dword[0], gpi_ere->dword[1],
+			gpi_ere->dword[2], gpi_ere->dword[3]);
+		return;
+	}
+
+	gpi_desc = to_gpi_desc(vd);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+
+	/*
+	 * RP pointed by Event is to last TRE processed,
+	 * we need to update ring rp to ev_rp + 1
+	 */
+	ev_rp += ch_ring->el_size;
+	if (ev_rp >= (ch_ring->base + ch_ring->len))
+		ev_rp = ch_ring->base;
+	ch_ring->rp = ev_rp;
+
+	/* update must be visible to other cores */
+	smp_wmb();
+
+	chid = compl_event->chid;
+	if (compl_event->code == MSM_GPI_TCE_EOT && gpii->ieob_set) {
+		if (chid == GPI_RX_CHAN)
+			goto gpi_free_desc;
+		else
+			return;
+	}
+
+	if (compl_event->code == MSM_GPI_TCE_UNEXP_ERR)
+		result.result = DMA_TRANS_ABORTED;
+	else
+		result.result = DMA_TRANS_NOERROR;
+	result.residue = gpi_desc->len - compl_event->length;
+
+	dma_cookie_complete(&vd->tx);
+	dmaengine_desc_get_callback_invoke(&vd->tx, &result);
+
+gpi_free_desc:
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	list_del(&vd->node);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+	kfree(gpi_desc);
+	gpi_desc = NULL;
+}
+
+/* process all events */
+static void gpi_process_events(struct gpii *gpii)
+{
+	struct gpi_ring *ev_ring = &gpii->ev_ring;
+	phys_addr_t cntxt_rp;
+	void *rp;
+	union gpi_event *gpi_event;
+	struct gpii_chan *gpii_chan;
+	u32 chid, type;
+
+	cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
+	rp = to_virtual(ev_ring, cntxt_rp);
+
+	do {
+		while (rp != ev_ring->rp) {
+			gpi_event = ev_ring->rp;
+			chid = gpi_event->xfer_compl_event.chid;
+			type = gpi_event->xfer_compl_event.type;
+
+			switch (type) {
+			case XFER_COMPLETE_EV_TYPE:
+				gpii_chan = &gpii->gpii_chan[chid];
+				gpi_process_xfer_compl_event(gpii_chan,
+							     &gpi_event->xfer_compl_event);
+				break;
+			case STALE_EV_TYPE:
+				dev_dbg(gpii->gpi_dev->dev, "stale event, not processing\n");
+				break;
+			case IMMEDIATE_DATA_EV_TYPE:
+				gpii_chan = &gpii->gpii_chan[chid];
+				gpi_process_imed_data_event(gpii_chan,
+							    &gpi_event->immediate_data_event);
+				break;
+			case QUP_NOTIF_EV_TYPE:
+				dev_dbg(gpii->gpi_dev->dev, "QUP_NOTIF_EV_TYPE\n");
+				break;
+			default:
+				dev_dbg(gpii->gpi_dev->dev,
+					"not supported event type:0x%x\n", type);
+			}
+			gpi_ring_recycle_ev_element(ev_ring);
+		}
+		gpi_write_ev_db(gpii, ev_ring, ev_ring->wp);
+
+		/* clear pending IEOB events */
+		gpi_write_reg(gpii, gpii->ieob_clr_reg, BIT(0));
+
+		cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
+		rp = to_virtual(ev_ring, cntxt_rp);
+
+	} while (rp != ev_ring->rp);
+}
+
+/* processing events using tasklet */
+static void gpi_ev_tasklet(unsigned long data)
+{
+	struct gpii *gpii = (struct gpii *)data;
+
+	read_lock_bh(&gpii->pm_lock);
+	if (!REG_ACCESS_VALID(gpii->pm_state)) {
+		read_unlock_bh(&gpii->pm_lock);
+		dev_err(gpii->gpi_dev->dev, "not processing any events, pm_state:%s\n",
+			TO_GPI_PM_STR(gpii->pm_state));
+		return;
+	}
+
+	/* process the events */
+	gpi_process_events(gpii);
+
+	/* enable IEOB, switching back to interrupts */
+	gpi_config_interrupts(gpii, MASK_IEOB_SETTINGS, 1);
+	read_unlock_bh(&gpii->pm_lock);
+}
+
+/* marks all pending events for the channel as stale */
+static void gpi_mark_stale_events(struct gpii_chan *gpii_chan)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	struct gpi_ring *ev_ring = &gpii->ev_ring;
+	u32 cntxt_rp, local_rp;
+	void *ev_rp;
+
+	cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
+
+	ev_rp = ev_ring->rp;
+	local_rp = (u32)to_physical(ev_ring, ev_rp);
+	while (local_rp != cntxt_rp) {
+		union gpi_event *gpi_event = ev_rp;
+		u32 chid = gpi_event->xfer_compl_event.chid;
+
+		if (chid == gpii_chan->chid)
+			gpi_event->xfer_compl_event.type = STALE_EV_TYPE;
+		ev_rp += ev_ring->el_size;
+		if (ev_rp >= (ev_ring->base + ev_ring->len))
+			ev_rp = ev_ring->base;
+		cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
+		local_rp = (u32)to_physical(ev_ring, ev_rp);
+	}
+}
+
+/* reset sw state and issue channel reset or de-alloc */
+static int gpi_reset_chan(struct gpii_chan *gpii_chan, enum gpi_cmd gpi_cmd)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	struct gpi_ring *ch_ring = &gpii_chan->ch_ring;
+	unsigned long flags;
+	LIST_HEAD(list);
+	int ret;
+
+	ret = gpi_send_cmd(gpii, gpii_chan, gpi_cmd);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "Error with cmd:%s ret:%d\n",
+			TO_GPI_CMD_STR(gpi_cmd), ret);
+		return ret;
+	}
+
+	/* initialize the local ring ptrs */
+	ch_ring->rp = ch_ring->base;
+	ch_ring->wp = ch_ring->base;
+
+	/* visible to other cores */
+	smp_wmb();
+
+	/* check event ring for any stale events */
+	write_lock_irq(&gpii->pm_lock);
+	gpi_mark_stale_events(gpii_chan);
+
+	/* remove all async descriptors */
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	vchan_get_all_descriptors(&gpii_chan->vc, &list);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+	write_unlock_irq(&gpii->pm_lock);
+	vchan_dma_desc_free_list(&gpii_chan->vc, &list);
+
+	return 0;
+}
+
+static int gpi_start_chan(struct gpii_chan *gpii_chan)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	int ret;
+
+	ret = gpi_send_cmd(gpii, gpii_chan, GPI_CH_CMD_START);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "Error with cmd:%s ret:%d\n",
+			TO_GPI_CMD_STR(GPI_CH_CMD_START), ret);
+		return ret;
+	}
+
+	/* gpii CH is active now */
+	write_lock_irq(&gpii->pm_lock);
+	gpii_chan->pm_state = ACTIVE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+
+	return 0;
+}
+
+static int gpi_stop_chan(struct gpii_chan *gpii_chan)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	int ret;
+
+	ret = gpi_send_cmd(gpii, gpii_chan, GPI_CH_CMD_STOP);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "Error with cmd:%s ret:%d\n",
+			TO_GPI_CMD_STR(GPI_CH_CMD_STOP), ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* allocate and configure the transfer channel */
+static int gpi_alloc_chan(struct gpii_chan *chan, bool send_alloc_cmd)
+{
+	struct gpii *gpii = chan->gpii;
+	struct gpi_ring *ring = &chan->ch_ring;
+	int ret;
+	u32 id = gpii->gpii_id;
+	u32 chid = chan->chid;
+
+	if (send_alloc_cmd) {
+		ret = gpi_send_cmd(gpii, chan, GPI_CH_CMD_ALLOCATE);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error with cmd:%s ret:%d\n",
+				TO_GPI_CMD_STR(GPI_CH_CMD_ALLOCATE), ret);
+			return ret;
+		}
+	}
+
+	gpi_write_reg(gpii, chan->ch_cntxt_base_reg + CNTXT_0_CONFIG,
+		      GPII_n_CH_k_CNTXT_0(ring->el_size, 0, chan->dir, GPI_CHTYPE_PROTO_GPI));
+	gpi_write_reg(gpii, chan->ch_cntxt_base_reg + CNTXT_1_R_LENGTH, ring->len);
+	gpi_write_reg(gpii, chan->ch_cntxt_base_reg + CNTXT_2_RING_BASE_LSB, ring->phys_addr);
+	gpi_write_reg(gpii, chan->ch_cntxt_base_reg + CNTXT_3_RING_BASE_MSB,
+		      upper_32_bits(ring->phys_addr));
+	gpi_write_reg(gpii, chan->ch_cntxt_db_reg + CNTXT_5_RING_RP_MSB - CNTXT_4_RING_RP_LSB,
+		      upper_32_bits(ring->phys_addr));
+	gpi_write_reg(gpii, gpii->regs + GPII_n_CH_k_SCRATCH_0_OFFS(id, chid),
+		      GPII_n_CH_k_SCRATCH_0(!chid, chan->protocol, chan->seid));
+	gpi_write_reg(gpii, gpii->regs + GPII_n_CH_k_SCRATCH_1_OFFS(id, chid), 0);
+	gpi_write_reg(gpii, gpii->regs + GPII_n_CH_k_SCRATCH_2_OFFS(id, chid), 0);
+	gpi_write_reg(gpii, gpii->regs + GPII_n_CH_k_SCRATCH_3_OFFS(id, chid), 0);
+	gpi_write_reg(gpii, gpii->regs + GPII_n_CH_k_QOS_OFFS(id, chid), 1);
+
+	/* flush all the writes */
+	wmb();
+	return 0;
+}
+
+/* allocate and configure event ring */
+static int gpi_alloc_ev_chan(struct gpii *gpii)
+{
+	struct gpi_ring *ring = &gpii->ev_ring;
+	void *base = gpii->ev_cntxt_base_reg;
+	int ret;
+
+	ret = gpi_send_cmd(gpii, NULL, GPI_EV_CMD_ALLOCATE);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "error with cmd:%s ret:%d\n",
+			TO_GPI_CMD_STR(GPI_EV_CMD_ALLOCATE), ret);
+		return ret;
+	}
+
+	/* program event context */
+	gpi_write_reg(gpii, base + CNTXT_0_CONFIG,
+		      GPII_n_EV_k_CNTXT_0(ring->el_size, GPI_INTTYPE_IRQ, GPI_CHTYPE_GPI_EV));
+	gpi_write_reg(gpii, base + CNTXT_1_R_LENGTH, ring->len);
+	gpi_write_reg(gpii, base + CNTXT_2_RING_BASE_LSB, lower_32_bits(ring->phys_addr));
+	gpi_write_reg(gpii, base + CNTXT_3_RING_BASE_MSB, upper_32_bits(ring->phys_addr));
+	gpi_write_reg(gpii, gpii->ev_cntxt_db_reg + CNTXT_5_RING_RP_MSB - CNTXT_4_RING_RP_LSB,
+		      upper_32_bits(ring->phys_addr));
+	gpi_write_reg(gpii, base + CNTXT_8_RING_INT_MOD, 0);
+	gpi_write_reg(gpii, base + CNTXT_10_RING_MSI_LSB, 0);
+	gpi_write_reg(gpii, base + CNTXT_11_RING_MSI_MSB, 0);
+	gpi_write_reg(gpii, base + CNTXT_8_RING_INT_MOD, 0);
+	gpi_write_reg(gpii, base + CNTXT_12_RING_RP_UPDATE_LSB, 0);
+	gpi_write_reg(gpii, base + CNTXT_13_RING_RP_UPDATE_MSB, 0);
+
+	/* add events to ring */
+	ring->wp = (ring->base + ring->len - ring->el_size);
+
+	/* flush all the writes */
+	wmb();
+
+	/* gpii is active now */
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = ACTIVE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+	gpi_write_ev_db(gpii, ring, ring->wp);
+
+	return 0;
+}
+
+/* calculate # of ERE/TRE available to queue */
+static int gpi_ring_num_elements_avail(const struct gpi_ring * const ring)
+{
+	int elements = 0;
+
+	if (ring->wp < ring->rp) {
+		elements = ((ring->rp - ring->wp) / ring->el_size) - 1;
+	} else {
+		elements = (ring->rp - ring->base) / ring->el_size;
+		elements += ((ring->base + ring->len - ring->wp) / ring->el_size) - 1;
+	}
+
+	return elements;
+}
+
+static int gpi_ring_add_element(struct gpi_ring *ring, void **wp)
+{
+	if (gpi_ring_num_elements_avail(ring) <= 0)
+		return -ENOMEM;
+
+	*wp = ring->wp;
+	ring->wp += ring->el_size;
+	if (ring->wp  >= (ring->base + ring->len))
+		ring->wp = ring->base;
+
+	/* visible to other cores */
+	smp_wmb();
+
+	return 0;
+}
+
+static void gpi_ring_recycle_ev_element(struct gpi_ring *ring)
+{
+	/* Update the WP */
+	ring->wp += ring->el_size;
+	if (ring->wp  >= (ring->base + ring->len))
+		ring->wp = ring->base;
+
+	/* Update the RP */
+	ring->rp += ring->el_size;
+	if (ring->rp  >= (ring->base + ring->len))
+		ring->rp = ring->base;
+
+	/* visible to other cores */
+	smp_wmb();
+}
+
+static void gpi_free_ring(struct gpi_ring *ring,
+			  struct gpii *gpii)
+{
+	dma_free_coherent(gpii->gpi_dev->dev, ring->alloc_size,
+			  ring->pre_aligned, ring->dma_handle);
+	memset(ring, 0, sizeof(*ring));
+}
+
+/* allocate memory for transfer and event rings */
+static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
+			  u32 el_size, struct gpii *gpii)
+{
+	u64 len = elements * el_size;
+	int bit;
+
+	/* ring len must be power of 2 */
+	bit = find_last_bit((unsigned long *)&len, 32);
+	if (((1 << bit) - 1) & len)
+		bit++;
+	len = 1 << bit;
+	ring->alloc_size = (len + (len - 1));
+	dev_dbg(gpii->gpi_dev->dev,
+		"#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%lu\n",
+		  elements, el_size, (elements * el_size), len,
+		  ring->alloc_size);
+
+	ring->pre_aligned = dma_alloc_coherent(gpii->gpi_dev->dev,
+					       ring->alloc_size,
+					       &ring->dma_handle, GFP_KERNEL);
+	if (!ring->pre_aligned) {
+		dev_err(gpii->gpi_dev->dev, "could not alloc size:%lu mem for ring\n",
+			ring->alloc_size);
+		return -ENOMEM;
+	}
+
+	/* align the physical mem */
+	ring->phys_addr = (ring->dma_handle + (len - 1)) & ~(len - 1);
+	ring->base = ring->pre_aligned + (ring->phys_addr - ring->dma_handle);
+	ring->rp = ring->base;
+	ring->wp = ring->base;
+	ring->len = len;
+	ring->el_size = el_size;
+	ring->elements = ring->len / ring->el_size;
+	memset(ring->base, 0, ring->len);
+	ring->configured = true;
+
+	/* update to other cores */
+	smp_wmb();
+
+	dev_dbg(gpii->gpi_dev->dev,
+		"phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
+		ring->dma_handle, ring->phys_addr, ring->len,
+		ring->el_size, ring->elements);
+
+	return 0;
+}
+
+/* copy tre into transfer ring */
+static void gpi_queue_xfer(struct gpii *gpii, struct gpii_chan *gpii_chan,
+			   struct gpi_tre *gpi_tre, void **wp)
+{
+	struct gpi_tre *ch_tre;
+	int ret;
+
+	/* get next tre location we can copy */
+	ret = gpi_ring_add_element(&gpii_chan->ch_ring, (void **)&ch_tre);
+	if (unlikely(ret)) {
+		dev_err(gpii->gpi_dev->dev, "Error adding ring element to xfer ring\n");
+		return;
+	}
+
+	/* copy the tre info */
+	memcpy(ch_tre, gpi_tre, sizeof(*ch_tre));
+	*wp = ch_tre;
+}
+
+/* reset and restart transfer channel */
+static int gpi_terminate_all(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	int schid, echid, i;
+	int ret = 0;
+
+	mutex_lock(&gpii->ctrl_lock);
+
+	/*
+	 * treat both channels as a group if its protocol is not UART
+	 * STOP, RESET, or START needs to be in lockstep
+	 */
+	schid = (gpii->protocol == SE_PROTOCOL_UART) ? gpii_chan->chid : 0;
+	echid = (gpii->protocol == SE_PROTOCOL_UART) ? schid + 1 : MAX_CHANNELS_PER_GPII;
+
+	/* stop the channel */
+	for (i = schid; i < echid; i++) {
+		gpii_chan = &gpii->gpii_chan[i];
+
+		/* disable ch state so no more TRE processing */
+		write_lock_irq(&gpii->pm_lock);
+		gpii_chan->pm_state = PREPARE_TERMINATE;
+		write_unlock_irq(&gpii->pm_lock);
+
+		/* send command to Stop the channel */
+		ret = gpi_stop_chan(gpii_chan);
+	}
+
+	/* reset the channels (clears any pending tre) */
+	for (i = schid; i < echid; i++) {
+		gpii_chan = &gpii->gpii_chan[i];
+
+		ret = gpi_reset_chan(gpii_chan, GPI_CH_CMD_RESET);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error resetting channel ret:%d\n", ret);
+			goto terminate_exit;
+		}
+
+		/* reprogram channel CNTXT */
+		ret = gpi_alloc_chan(gpii_chan, false);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error alloc_channel ret:%d\n", ret);
+			goto terminate_exit;
+		}
+	}
+
+	/* restart the channels */
+	for (i = schid; i < echid; i++) {
+		gpii_chan = &gpii->gpii_chan[i];
+
+		ret = gpi_start_chan(gpii_chan);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error Starting Channel ret:%d\n", ret);
+			goto terminate_exit;
+		}
+	}
+
+terminate_exit:
+	mutex_unlock(&gpii->ctrl_lock);
+	return ret;
+}
+
+/* pause dma transfer for all channels */
+static int gpi_pause(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	int i, ret;
+
+	mutex_lock(&gpii->ctrl_lock);
+
+	/*
+	 * pause/resume are per gpii not per channel, so
+	 * client needs to call pause only once
+	 */
+	if (gpii->pm_state == PAUSE_STATE) {
+		dev_dbg(gpii->gpi_dev->dev, "channel is already paused\n");
+		mutex_unlock(&gpii->ctrl_lock);
+		return 0;
+	}
+
+	/* send stop command to stop the channels */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++) {
+		ret = gpi_stop_chan(&gpii->gpii_chan[i]);
+		if (ret) {
+			mutex_unlock(&gpii->ctrl_lock);
+			return ret;
+		}
+	}
+
+	disable_irq(gpii->irq);
+
+	/* Wait for threads to complete out */
+	tasklet_kill(&gpii->ev_task);
+
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = PAUSE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+	mutex_unlock(&gpii->ctrl_lock);
+
+	return 0;
+}
+
+/* resume dma transfer */
+static int gpi_resume(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	int i, ret;
+
+	mutex_lock(&gpii->ctrl_lock);
+	if (gpii->pm_state == ACTIVE_STATE) {
+		dev_dbg(gpii->gpi_dev->dev, "channel is already active\n");
+		mutex_unlock(&gpii->ctrl_lock);
+		return 0;
+	}
+
+	enable_irq(gpii->irq);
+
+	/* send start command to start the channels */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++) {
+		ret = gpi_send_cmd(gpii, &gpii->gpii_chan[i], GPI_CH_CMD_START);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error starting chan, ret:%d\n", ret);
+			mutex_unlock(&gpii->ctrl_lock);
+			return ret;
+		}
+	}
+
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = ACTIVE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+	mutex_unlock(&gpii->ctrl_lock);
+
+	return 0;
+}
+
+static void gpi_desc_free(struct virt_dma_desc *vd)
+{
+	struct gpi_desc *gpi_desc = to_gpi_desc(vd);
+
+	kfree(gpi_desc);
+	gpi_desc = NULL;
+}
+
+static int
+gpi_peripheral_config(struct dma_chan *chan, struct dma_slave_config *config)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+
+	memcpy(&gpii_chan->config, config, sizeof(gpii_chan->config));
+
+	return 0;
+}
+
+static int gpi_create_tre(struct gpii_chan *chan, struct gpi_desc *desc,
+			  struct scatterlist *sgl, enum dma_transfer_direction direction)
+{
+	struct dmaengine_spi_config *spi = &chan->config.spi;
+	struct device *dev = chan->gpii->gpi_dev->dev;
+	unsigned int tre_idx = 0;
+	dma_addr_t address;
+	struct gpi_tre *tre;
+	unsigned int i;
+
+	/* first create config tre if applicable */
+	if (direction == DMA_MEM_TO_DEV && chan->config.set_config) {
+		tre = &desc->tre[tre_idx];
+		tre_idx++;
+
+		tre->dword[0] = u32_encode_bits(spi->word_len, TRE_SPI_C0_WORD_SZ);
+		tre->dword[0] |= u32_encode_bits(spi->loopback_en, TRE_SPI_C0_LOOPBACK);
+		tre->dword[0] |= u32_encode_bits(spi->clock_pol, TRE_SPI_C0_CPOL);
+		tre->dword[0] |= u32_encode_bits(spi->data_pol, TRE_SPI_C0_CPHA);
+		tre->dword[0] |= u32_encode_bits(spi->pack_en, TRE_SPI_C0_TX_PACK);
+		tre->dword[0] |= u32_encode_bits(spi->pack_en, TRE_SPI_C0_RX_PACK);
+
+		tre->dword[1] = 0;
+
+		tre->dword[2] = u32_encode_bits(spi->clk_div, TRE_SPI_C0_CLK_DIV);
+		tre->dword[2] |= u32_encode_bits(spi->clk_src, TRE_SPI_C0_CLK_SRC);
+
+		tre->dword[3] = u32_encode_bits(TRE_TYPE_CONFIG0, TRE_FLAGS_TYPE);
+		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
+	}
+
+	/* create the GO tre for Tx */
+	if (direction == DMA_MEM_TO_DEV) {
+		tre = &desc->tre[tre_idx];
+		tre_idx++;
+
+		tre->dword[0] = u32_encode_bits(spi->fragmentation, TRE_SPI_GO_FRAG);
+		tre->dword[0] |= u32_encode_bits(spi->cs, TRE_SPI_GO_CS);
+		tre->dword[0] |= u32_encode_bits(spi->cmd, TRE_SPI_GO_CMD);
+
+		tre->dword[1] = 0;
+
+		tre->dword[2] = u32_encode_bits(spi->rx_len, TRE_SPI_RX_LEN);
+
+		tre->dword[3] = u32_encode_bits(TRE_TYPE_GO, TRE_FLAGS_TYPE);
+		if (spi->cmd == SPI_RX)
+			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOB);
+		else
+			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
+	}
+
+	/* create the dma tre */
+	tre = &desc->tre[tre_idx];
+	tre_idx++;
+
+	address = sg_dma_address(sgl);
+	tre->dword[0] = lower_32_bits(address);
+	tre->dword[1] = upper_32_bits(address);
+
+	tre->dword[2] = u32_encode_bits(sg_dma_len(sgl), TRE_DMA_LEN);
+
+	tre->dword[3] = u32_encode_bits(TRE_TYPE_DMA, TRE_FLAGS_TYPE);
+	if (direction == DMA_MEM_TO_DEV)
+		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOT);
+
+	for (i = 0; i < tre_idx; i++)
+		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
+			desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
+
+	return tre_idx;
+}
+
+/* copy tre into transfer ring */
+static struct dma_async_tx_descriptor *
+gpi_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+		  unsigned int sg_len, enum dma_transfer_direction direction,
+		  unsigned long flags, void *context)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	struct device *dev = gpii->gpi_dev->dev;
+	struct gpi_ring *ch_ring = &gpii_chan->ch_ring;
+	void *tre, *wp = NULL;
+	struct gpi_desc *gpi_desc;
+	u32 nr, nr_tre = 0;
+	int i, j;
+
+	gpii->ieob_set = false;
+	if (!is_slave_direction(direction)) {
+		dev_err(gpii->gpi_dev->dev, "invalid dma direction: %d\n", direction);
+		return NULL;
+	}
+
+	if (sg_len > 1) {
+		dev_err(dev, "Multi sg sent, we support only one atm: %d\n", sg_len);
+		return NULL;
+	}
+
+	nr_tre = 3;
+	if (!gpii_chan->config.set_config)
+		nr_tre = 2;
+	if (direction == DMA_DEV_TO_MEM) /* rx */
+		nr_tre = 1;
+
+	/* calculate # of elements required & available */
+	nr = gpi_ring_num_elements_avail(ch_ring);
+	if (nr < nr_tre) {
+		dev_err(dev, "not enough space in ring, avail:%u required:%u\n", nr, nr_tre);
+		return NULL;
+	}
+
+	gpi_desc = kzalloc(sizeof(*gpi_desc), GFP_NOWAIT);
+	if (!gpi_desc)
+		return NULL;
+
+	/* create TREs for xfer */
+	i = gpi_create_tre(gpii_chan, gpi_desc, sgl, direction);
+	for (j = 0; j < i; j++) {
+		tre = &gpi_desc->tre[j];
+		gpi_queue_xfer(gpii, gpii_chan, tre, &wp);
+	}
+
+	/* set up the descriptor */
+	gpi_desc->db = ch_ring->wp;
+	gpi_desc->wp = wp;
+	gpi_desc->gpii_chan = gpii_chan;
+	gpi_desc->len = sg_dma_len(sgl);
+
+	return vchan_tx_prep(&gpii_chan->vc, &gpi_desc->vd, flags);
+}
+
+/* rings transfer ring db to being transfer */
+static void gpi_issue_pending(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	unsigned long flags, pm_lock_flags;
+	struct virt_dma_desc *vd = NULL;
+	struct gpi_desc *gpi_desc;
+
+	read_lock_irqsave(&gpii->pm_lock, pm_lock_flags);
+
+	/* move all submitted discriptors to issued list */
+	spin_lock_irqsave(&gpii_chan->vc.lock, flags);
+	if (vchan_issue_pending(&gpii_chan->vc))
+		vd = list_last_entry(&gpii_chan->vc.desc_issued,
+				     struct virt_dma_desc, node);
+	spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
+
+	/* nothing to do list is empty */
+	if (!vd) {
+		read_unlock_irqrestore(&gpii->pm_lock, pm_lock_flags);
+		return;
+	}
+
+	gpi_desc = to_gpi_desc(vd);
+	gpi_write_ch_db(gpii_chan, &gpii_chan->ch_ring, gpi_desc->db);
+	read_unlock_irqrestore(&gpii->pm_lock, pm_lock_flags);
+}
+
+static int gpi_ch_init(struct gpii_chan *gpii_chan)
+{
+	struct gpii *gpii = gpii_chan->gpii;
+	const int ev_factor = gpii->gpi_dev->ev_factor;
+	u32 elements;
+	int i = 0, ret = 0;
+
+	gpii_chan->pm_state = CONFIG_STATE;
+
+	/* check if both channels are configured before continue */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++)
+		if (gpii->gpii_chan[i].pm_state != CONFIG_STATE)
+			goto exit_gpi_init;
+
+	/* configure to highest priority from  two channels */
+	gpii->ev_priority = min(gpii->gpii_chan[0].priority,
+				gpii->gpii_chan[1].priority);
+
+	/* protocol must be same for both channels */
+	if (gpii->gpii_chan[0].protocol !=
+			gpii->gpii_chan[1].protocol) {
+		dev_err(gpii->gpi_dev->dev, "protocol did not match protocol %u != %u\n",
+			gpii->gpii_chan[0].protocol, gpii->gpii_chan[1].protocol);
+		ret = -EINVAL;
+		goto exit_gpi_init;
+	}
+	gpii->protocol = gpii_chan->protocol;
+
+	/* allocate memory for event ring */
+	elements = max(gpii->gpii_chan[0].req_tres,
+		       gpii->gpii_chan[1].req_tres);
+	ret = gpi_alloc_ring(&gpii->ev_ring, elements << ev_factor,
+			     sizeof(union gpi_event), gpii);
+	if (ret)
+		goto exit_gpi_init;
+
+	/* configure interrupts */
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = PREPARE_HARDWARE;
+	write_unlock_irq(&gpii->pm_lock);
+	ret = gpi_config_interrupts(gpii, DEFAULT_IRQ_SETTINGS, 0);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "error config. interrupts, ret:%d\n", ret);
+		goto error_config_int;
+	}
+
+	/* allocate event rings */
+	ret = gpi_alloc_ev_chan(gpii);
+	if (ret) {
+		dev_err(gpii->gpi_dev->dev, "error alloc_ev_chan:%d\n", ret);
+		goto error_alloc_ev_ring;
+	}
+
+	/* Allocate all channels */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++) {
+		ret = gpi_alloc_chan(&gpii->gpii_chan[i], true);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error allocating chan:%d\n", ret);
+			goto error_alloc_chan;
+		}
+	}
+
+	/* start channels  */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++) {
+		ret = gpi_start_chan(&gpii->gpii_chan[i]);
+		if (ret) {
+			dev_err(gpii->gpi_dev->dev, "Error start chan:%d\n", ret);
+			goto error_start_chan;
+		}
+	}
+	return ret;
+
+error_start_chan:
+	for (i = i - 1; i >= 0; i++) {
+		gpi_stop_chan(&gpii->gpii_chan[i]);
+		gpi_send_cmd(gpii, gpii_chan, GPI_CH_CMD_RESET);
+	}
+	i = 2;
+error_alloc_chan:
+	for (i = i - 1; i >= 0; i--)
+		gpi_reset_chan(gpii_chan, GPI_CH_CMD_DE_ALLOC);
+error_alloc_ev_ring:
+	gpi_disable_interrupts(gpii);
+error_config_int:
+	gpi_free_ring(&gpii->ev_ring, gpii);
+exit_gpi_init:
+	mutex_unlock(&gpii->ctrl_lock);
+	return ret;
+}
+
+/* release all channel resources */
+static void gpi_free_chan_resources(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	enum gpi_pm_state cur_state;
+	int ret, i;
+
+	mutex_lock(&gpii->ctrl_lock);
+
+	cur_state = gpii_chan->pm_state;
+
+	/* disable ch state so no more TRE processing for this channel */
+	write_lock_irq(&gpii->pm_lock);
+	gpii_chan->pm_state = PREPARE_TERMINATE;
+	write_unlock_irq(&gpii->pm_lock);
+
+	/* attempt to do graceful hardware shutdown */
+	if (cur_state == ACTIVE_STATE) {
+		gpi_stop_chan(gpii_chan);
+
+		ret = gpi_send_cmd(gpii, gpii_chan, GPI_CH_CMD_RESET);
+		if (ret)
+			dev_err(gpii->gpi_dev->dev, "error resetting channel:%d\n", ret);
+
+		gpi_reset_chan(gpii_chan, GPI_CH_CMD_DE_ALLOC);
+	}
+
+	/* free all allocated memory */
+	gpi_free_ring(&gpii_chan->ch_ring, gpii);
+	vchan_free_chan_resources(&gpii_chan->vc);
+
+	write_lock_irq(&gpii->pm_lock);
+	gpii_chan->pm_state = DISABLE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+
+	/* if other rings are still active exit */
+	for (i = 0; i < MAX_CHANNELS_PER_GPII; i++)
+		if (gpii->gpii_chan[i].ch_ring.configured)
+			goto exit_free;
+
+	/* deallocate EV Ring */
+	cur_state = gpii->pm_state;
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = PREPARE_TERMINATE;
+	write_unlock_irq(&gpii->pm_lock);
+
+	/* wait for threads to complete out */
+	tasklet_kill(&gpii->ev_task);
+
+	/* send command to de allocate event ring */
+	if (cur_state == ACTIVE_STATE)
+		gpi_send_cmd(gpii, NULL, GPI_EV_CMD_DEALLOC);
+
+	gpi_free_ring(&gpii->ev_ring, gpii);
+
+	/* disable interrupts */
+	if (cur_state == ACTIVE_STATE)
+		gpi_disable_interrupts(gpii);
+
+	/* set final state to disable */
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = DISABLE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
+
+exit_free:
+	mutex_unlock(&gpii->ctrl_lock);
+}
+
+/* allocate channel resources */
+static int gpi_alloc_chan_resources(struct dma_chan *chan)
+{
+	struct gpii_chan *gpii_chan = to_gpii_chan(chan);
+	struct gpii *gpii = gpii_chan->gpii;
+	int ret;
+
+	mutex_lock(&gpii->ctrl_lock);
+
+	/* allocate memory for transfer ring */
+	ret = gpi_alloc_ring(&gpii_chan->ch_ring, gpii_chan->req_tres,
+			     sizeof(struct gpi_tre), gpii);
+	if (ret)
+		goto xfer_alloc_err;
+
+	ret = gpi_ch_init(gpii_chan);
+
+	mutex_unlock(&gpii->ctrl_lock);
+
+	return ret;
+xfer_alloc_err:
+	mutex_unlock(&gpii->ctrl_lock);
+
+	return ret;
+}
+
+static int gpi_find_avail_gpii(struct gpi_dev *gpi_dev, u32 seid)
+{
+	struct gpii_chan *tx_chan, *rx_chan;
+	unsigned int gpii;
+
+	/* check if same seid is already configured for another chid */
+	for (gpii = 0; gpii < gpi_dev->max_gpii; gpii++) {
+		if (!((1 << gpii) & gpi_dev->gpii_mask))
+			continue;
+
+		tx_chan = &gpi_dev->gpiis[gpii].gpii_chan[GPI_TX_CHAN];
+		rx_chan = &gpi_dev->gpiis[gpii].gpii_chan[GPI_RX_CHAN];
+
+		if (rx_chan->vc.chan.client_count && rx_chan->seid == seid)
+			return gpii;
+		if (tx_chan->vc.chan.client_count && tx_chan->seid == seid)
+			return gpii;
+	}
+
+	/* no channels configured with same seid, return next avail gpii */
+	for (gpii = 0; gpii < gpi_dev->max_gpii; gpii++) {
+		if (!((1 << gpii) & gpi_dev->gpii_mask))
+			continue;
+
+		tx_chan = &gpi_dev->gpiis[gpii].gpii_chan[GPI_TX_CHAN];
+		rx_chan = &gpi_dev->gpiis[gpii].gpii_chan[GPI_RX_CHAN];
+
+		/* check if gpii is configured */
+		if (tx_chan->vc.chan.client_count ||
+		    rx_chan->vc.chan.client_count)
+			continue;
+
+		/* found a free gpii */
+		return gpii;
+	}
+
+	/* no gpii instance available to use */
+	return -EIO;
+}
+
+/* gpi_of_dma_xlate: open client requested channel */
+static struct dma_chan *gpi_of_dma_xlate(struct of_phandle_args *args,
+					 struct of_dma *of_dma)
+{
+	struct gpi_dev *gpi_dev = (struct gpi_dev *)of_dma->of_dma_data;
+	u32 seid, chid;
+	int gpii;
+	struct gpii_chan *gpii_chan;
+
+	if (args->args_count < REQ_OF_DMA_ARGS) {
+		dev_err(gpi_dev->dev, "gpii require minimum 6 args, client passed:%d args\n",
+			args->args_count);
+		return NULL;
+	}
+
+	chid = args->args[0];
+	if (chid >= MAX_CHANNELS_PER_GPII) {
+		dev_err(gpi_dev->dev, "gpii channel:%d not valid\n", chid);
+		return NULL;
+	}
+
+	seid = args->args[1];
+
+	/* find next available gpii to use */
+	gpii = gpi_find_avail_gpii(gpi_dev, seid);
+	if (gpii < 0) {
+		dev_err(gpi_dev->dev, "no available gpii instances\n");
+		return NULL;
+	}
+
+	gpii_chan = &gpi_dev->gpiis[gpii].gpii_chan[chid];
+	if (gpii_chan->vc.chan.client_count) {
+		dev_err(gpi_dev->dev, "gpii:%d chid:%d seid:%d already configured\n",
+			gpii, chid, gpii_chan->seid);
+		return NULL;
+	}
+
+	/* get ring size, protocol, se_id, and priority */
+	gpii_chan->seid = seid;
+	gpii_chan->protocol = args->args[2];
+	gpii_chan->req_tres = args->args[3];
+	gpii_chan->priority = args->args[4];
+
+	dev_dbg(gpi_dev->dev,
+		"client req. gpii:%u chid:%u #_tre:%u priority:%u protocol:%u\n",
+		gpii, chid, gpii_chan->req_tres, gpii_chan->priority,
+		gpii_chan->protocol);
+
+	return dma_get_slave_channel(&gpii_chan->vc.chan);
+}
+
+static int gpi_probe(struct platform_device *pdev)
+{
+	struct gpi_dev *gpi_dev;
+	const char *mode = NULL;
+	u32 gpi_ee_offset;
+	unsigned int i;
+	int ret;
+
+	gpi_dev = devm_kzalloc(&pdev->dev, sizeof(*gpi_dev), GFP_KERNEL);
+	if (!gpi_dev)
+		return -ENOMEM;
+
+	gpi_dev->dev = &pdev->dev;
+	gpi_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	gpi_dev->regs = devm_ioremap_resource(gpi_dev->dev, gpi_dev->res);
+	if (IS_ERR(gpi_dev->regs))
+		return PTR_ERR(gpi_dev->regs);
+	gpi_dev->ee_base = gpi_dev->regs;
+
+	ret = of_property_read_u32(gpi_dev->dev->of_node, "qcom,max-num-gpii",
+				   &gpi_dev->max_gpii);
+	if (ret) {
+		dev_err(gpi_dev->dev, "missing 'max-no-gpii' DT node\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(gpi_dev->dev->of_node, "qcom,gpii-mask",
+				   &gpi_dev->gpii_mask);
+	if (ret) {
+		dev_err(gpi_dev->dev, "missing 'gpii-mask' DT node\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(gpi_dev->dev->of_node,
+				   "qcom,gpi-ee-offset", &gpi_ee_offset);
+	if (ret)
+		dev_dbg(gpi_dev->dev, "No variable ee offset present\n");
+	else
+		gpi_dev->ee_base = gpi_dev->ee_base - gpi_ee_offset;
+
+	ret = of_property_read_u32(gpi_dev->dev->of_node,
+				   "qcom,ev-factor", &gpi_dev->ev_factor);
+	if (ret) {
+		dev_err(gpi_dev->dev, "missing 'qcom,ev-factor' DT node\n");
+		return ret;
+	}
+
+	of_property_read_string(gpi_dev->dev->of_node, "qcom,iommu-dma", &mode);
+	ret = dma_set_mask(gpi_dev->dev, DMA_BIT_MASK(64));
+	if (ret) {
+		dev_err(gpi_dev->dev, "Error setting dma_mask to 64, ret:%d\n", ret);
+		return ret;
+	}
+
+	gpi_dev->gpiis = devm_kzalloc(gpi_dev->dev, sizeof(*gpi_dev->gpiis) *
+				      gpi_dev->max_gpii, GFP_KERNEL);
+	if (!gpi_dev->gpiis)
+		return -ENOMEM;
+
+	/* setup all the supported gpii */
+	INIT_LIST_HEAD(&gpi_dev->dma_device.channels);
+	for (i = 0; i < gpi_dev->max_gpii; i++) {
+		struct gpii *gpii = &gpi_dev->gpiis[i];
+		int chan;
+
+		if (!((1 << i) & gpi_dev->gpii_mask))
+			continue;
+
+		/* set up ev cntxt register map */
+		gpii->ev_cntxt_base_reg = gpi_dev->ee_base + GPII_n_EV_CH_k_CNTXT_0_OFFS(i, 0);
+		gpii->ev_cntxt_db_reg = gpi_dev->ee_base + GPII_n_EV_CH_k_DOORBELL_0_OFFS(i, 0);
+		gpii->ev_ring_rp_lsb_reg = gpii->ev_cntxt_base_reg + CNTXT_4_RING_RP_LSB;
+		gpii->ev_cmd_reg = gpi_dev->ee_base + GPII_n_EV_CH_CMD_OFFS(i);
+		gpii->ieob_clr_reg = gpi_dev->ee_base + GPII_n_CNTXT_SRC_IEOB_IRQ_CLR_OFFS(i);
+
+		/* set up irq */
+		ret = platform_get_irq(pdev, i);
+		if (ret < 0) {
+			dev_err(gpi_dev->dev, "platform_get_irq failed for %d:%d\n", i, ret);
+			return ret;
+		}
+		gpii->irq = ret;
+
+		/* set up channel specific register info */
+		for (chan = 0; chan < MAX_CHANNELS_PER_GPII; chan++) {
+			struct gpii_chan *gpii_chan = &gpii->gpii_chan[chan];
+
+			/* set up ch cntxt register map */
+			gpii_chan->ch_cntxt_base_reg = gpi_dev->ee_base +
+				GPII_n_CH_k_CNTXT_0_OFFS(i, chan);
+			gpii_chan->ch_cntxt_db_reg = gpi_dev->ee_base +
+				GPII_n_CH_k_DOORBELL_0_OFFS(i, chan);
+			gpii_chan->ch_cmd_reg = gpi_dev->ee_base + GPII_n_CH_CMD_OFFS(i);
+
+			/* vchan setup */
+			vchan_init(&gpii_chan->vc, &gpi_dev->dma_device);
+			gpii_chan->vc.desc_free = gpi_desc_free;
+			gpii_chan->chid = chan;
+			gpii_chan->gpii = gpii;
+			gpii_chan->dir = GPII_CHAN_DIR[chan];
+		}
+		mutex_init(&gpii->ctrl_lock);
+		rwlock_init(&gpii->pm_lock);
+		tasklet_init(&gpii->ev_task, gpi_ev_tasklet,
+			     (unsigned long)gpii);
+		init_completion(&gpii->cmd_completion);
+		gpii->gpii_id = i;
+		gpii->regs = gpi_dev->ee_base;
+		gpii->gpi_dev = gpi_dev;
+	}
+
+	platform_set_drvdata(pdev, gpi_dev);
+
+	/* clear and Set capabilities */
+	dma_cap_zero(gpi_dev->dma_device.cap_mask);
+	dma_cap_set(DMA_SLAVE, gpi_dev->dma_device.cap_mask);
+
+	/* configure dmaengine apis */
+	gpi_dev->dma_device.directions =
+		BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
+	gpi_dev->dma_device.residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
+	gpi_dev->dma_device.src_addr_widths = DMA_SLAVE_BUSWIDTH_8_BYTES;
+	gpi_dev->dma_device.dst_addr_widths = DMA_SLAVE_BUSWIDTH_8_BYTES;
+	gpi_dev->dma_device.device_alloc_chan_resources = gpi_alloc_chan_resources;
+	gpi_dev->dma_device.device_free_chan_resources = gpi_free_chan_resources;
+	gpi_dev->dma_device.device_tx_status = dma_cookie_status;
+	gpi_dev->dma_device.device_issue_pending = gpi_issue_pending;
+	gpi_dev->dma_device.device_prep_slave_sg = gpi_prep_slave_sg;
+	gpi_dev->dma_device.device_config = gpi_peripheral_config;
+	gpi_dev->dma_device.device_terminate_all = gpi_terminate_all;
+	gpi_dev->dma_device.dev = gpi_dev->dev;
+	gpi_dev->dma_device.device_pause = gpi_pause;
+	gpi_dev->dma_device.device_resume = gpi_resume;
+
+	/* register with dmaengine framework */
+	ret = dma_async_device_register(&gpi_dev->dma_device);
+	if (ret) {
+		dev_err(gpi_dev->dev, "async_device_register failed ret:%d", ret);
+		return ret;
+	}
+
+	ret = of_dma_controller_register(gpi_dev->dev->of_node,
+					 gpi_of_dma_xlate, gpi_dev);
+	if (ret) {
+		dev_err(gpi_dev->dev, "of_dma_controller_reg failed ret:%d", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct of_device_id gpi_of_match[] = {
+	{ .compatible = "qcom,gpi-dma" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gpi_of_match);
+
+static struct platform_driver gpi_driver = {
+	.probe = gpi_probe,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = gpi_of_match,
+	},
+};
+
+static int __init gpi_init(void)
+{
+	return platform_driver_register(&gpi_driver);
+}
+subsys_initcall(gpi_init)
+
+MODULE_DESCRIPTION("QCOM GPI DMA engine driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


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

* Re: [PATCH 3/3] dmaengine: qcom: Add GPI dma driver
  2020-08-24  8:47 ` [PATCH 3/3] dmaengine: qcom: Add GPI dma driver Vinod Koul
@ 2020-08-24 14:04   ` kernel test robot
  2020-08-24 14:13   ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-08-24 14:04 UTC (permalink / raw)
  To: Vinod Koul, dmaengine
  Cc: kbuild-all, Vinod Koul, Rob Herring, Bjorn Andersson,
	linux-arm-msm, devicetree, linux-kernel

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

Hi Vinod,

I love your patch! Yet something to improve:

[auto build test ERROR on vkoul-dmaengine/next]
[also build test ERROR on v5.9-rc2 next-20200824]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vinod-Koul/dmaengine-Add-support-for-Qcom-GSI-dma-controller/20200824-174027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

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

>> drivers/dma/qcom/gpi.c:7: warning: "DEBUG" redefined
       7 | #define DEBUG
         | 
   <command-line>: note: this is the location of the previous definition
   drivers/dma/qcom/gpi.c: In function 'gpi_process_imed_data_event':
>> drivers/dma/qcom/gpi.c:1048:2: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
    1048 |  kfree(gpi_desc);
         |  ^~~~~
         |  vfree
   drivers/dma/qcom/gpi.c: In function 'gpi_prep_slave_sg':
>> drivers/dma/qcom/gpi.c:1772:13: error: implicit declaration of function 'kzalloc'; did you mean 'vzalloc'? [-Werror=implicit-function-declaration]
    1772 |  gpi_desc = kzalloc(sizeof(*gpi_desc), GFP_NOWAIT);
         |             ^~~~~~~
         |             vzalloc
>> drivers/dma/qcom/gpi.c:1772:11: warning: assignment to 'struct gpi_desc *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
    1772 |  gpi_desc = kzalloc(sizeof(*gpi_desc), GFP_NOWAIT);
         |           ^
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/25062cc46a95e9de91752963ea85b934fe7acfa1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vinod-Koul/dmaengine-Add-support-for-Qcom-GSI-dma-controller/20200824-174027
git checkout 25062cc46a95e9de91752963ea85b934fe7acfa1
vim +1048 drivers/dma/qcom/gpi.c

   969	
   970	/* process DMA Immediate completion data events */
   971	static void gpi_process_imed_data_event(struct gpii_chan *gpii_chan,
   972						struct immediate_data_event *imed_event)
   973	{
   974		struct gpii *gpii = gpii_chan->gpii;
   975		struct gpi_ring *ch_ring = &gpii_chan->ch_ring;
   976		void *tre = ch_ring->base + (ch_ring->el_size * imed_event->tre_index);
   977		struct dmaengine_result result;
   978		struct gpi_desc *gpi_desc;
   979		struct virt_dma_desc *vd;
   980		unsigned long flags;
   981		u32 chid;
   982	
   983		/*
   984		 * If channel not active don't process event
   985		 */
   986		if (gpii_chan->pm_state != ACTIVE_STATE) {
   987			dev_err(gpii->gpi_dev->dev, "skipping processing event because ch @ %s state\n",
   988				TO_GPI_PM_STR(gpii_chan->pm_state));
   989			return;
   990		}
   991	
   992		spin_lock_irqsave(&gpii_chan->vc.lock, flags);
   993		vd = vchan_next_desc(&gpii_chan->vc);
   994		if (!vd) {
   995			struct gpi_ere *gpi_ere;
   996			struct gpi_tre *gpi_tre;
   997	
   998			spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
   999			dev_dbg(gpii->gpi_dev->dev, "event without a pending descriptor!\n");
  1000			gpi_ere = (struct gpi_ere *)imed_event;
  1001			dev_dbg(gpii->gpi_dev->dev,
  1002				"Event: %08x %08x %08x %08x\n",
  1003				gpi_ere->dword[0], gpi_ere->dword[1],
  1004				gpi_ere->dword[2], gpi_ere->dword[3]);
  1005			gpi_tre = tre;
  1006			dev_dbg(gpii->gpi_dev->dev,
  1007				"Pending TRE: %08x %08x %08x %08x\n",
  1008				gpi_tre->dword[0], gpi_tre->dword[1],
  1009				gpi_tre->dword[2], gpi_tre->dword[3]);
  1010			return;
  1011		}
  1012		gpi_desc = to_gpi_desc(vd);
  1013		spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
  1014	
  1015		/*
  1016		 * RP pointed by Event is to last TRE processed,
  1017		 * we need to update ring rp to tre + 1
  1018		 */
  1019		tre += ch_ring->el_size;
  1020		if (tre >= (ch_ring->base + ch_ring->len))
  1021			tre = ch_ring->base;
  1022		ch_ring->rp = tre;
  1023	
  1024		/* make sure rp updates are immediately visible to all cores */
  1025		smp_wmb();
  1026	
  1027		chid = imed_event->chid;
  1028		if (imed_event->code == MSM_GPI_TCE_EOT && gpii->ieob_set) {
  1029			if (chid == GPI_RX_CHAN)
  1030				goto gpi_free_desc;
  1031			else
  1032				return;
  1033		}
  1034	
  1035		if (imed_event->code == MSM_GPI_TCE_UNEXP_ERR)
  1036			result.result = DMA_TRANS_ABORTED;
  1037		else
  1038			result.result = DMA_TRANS_NOERROR;
  1039		result.residue = gpi_desc->len - imed_event->length;
  1040	
  1041		dma_cookie_complete(&vd->tx);
  1042		dmaengine_desc_get_callback_invoke(&vd->tx, &result);
  1043	
  1044	gpi_free_desc:
  1045		spin_lock_irqsave(&gpii_chan->vc.lock, flags);
  1046		list_del(&vd->node);
  1047		spin_unlock_irqrestore(&gpii_chan->vc.lock, flags);
> 1048		kfree(gpi_desc);
  1049		gpi_desc = NULL;
  1050	}
  1051	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

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

* Re: [PATCH 3/3] dmaengine: qcom: Add GPI dma driver
  2020-08-24  8:47 ` [PATCH 3/3] dmaengine: qcom: Add GPI dma driver Vinod Koul
  2020-08-24 14:04   ` kernel test robot
@ 2020-08-24 14:13   ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2020-08-24 14:13 UTC (permalink / raw)
  To: Vinod Koul, dmaengine
  Cc: kbuild-all, Vinod Koul, Rob Herring, Bjorn Andersson,
	linux-arm-msm, devicetree, linux-kernel

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

Hi Vinod,

I love your patch! Perhaps something to improve:

[auto build test WARNING on vkoul-dmaengine/next]
[also build test WARNING on v5.9-rc2 next-20200824]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vinod-Koul/dmaengine-Add-support-for-Qcom-GSI-dma-controller/20200824-174027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/dma/qcom/gpi.c:7: warning: "DEBUG" redefined
       7 | #define DEBUG
         | 
   <command-line>: note: this is the location of the previous definition
   drivers/dma/qcom/gpi.c: In function 'gpi_process_imed_data_event':
   drivers/dma/qcom/gpi.c:1048:2: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
    1048 |  kfree(gpi_desc);
         |  ^~~~~
         |  vfree
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from include/asm-generic/bug.h:20,
                    from arch/arm/include/asm/bug.h:60,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from include/asm-generic/current.h:5,
                    from ./arch/arm/include/generated/asm/current.h:1,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/qcom/gpi.c:10:
   drivers/dma/qcom/gpi.c: In function 'gpi_alloc_ring':
>> drivers/dma/qcom/gpi.c:1450:3: warning: format '%lu' expects argument of type 'long unsigned int', but argument 8 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
    1450 |   "#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%lu\n",
         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:125:15: note: in definition of macro '__dynamic_func_call'
     125 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:157:2: note: in expansion of macro '_dynamic_func_call'
     157 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/dma/qcom/gpi.c:1449:2: note: in expansion of macro 'dev_dbg'
    1449 |  dev_dbg(gpii->gpi_dev->dev,
         |  ^~~~~~~
   drivers/dma/qcom/gpi.c:1450:58: note: format string is defined here
    1450 |   "#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%lu\n",
         |                                                        ~~^
         |                                                          |
         |                                                          long unsigned int
         |                                                        %u
   In file included from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/qcom/gpi.c:10:
   drivers/dma/qcom/gpi.c:1458:31: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
    1458 |   dev_err(gpii->gpi_dev->dev, "could not alloc size:%lu mem for ring\n",
         |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   drivers/dma/qcom/gpi.c:1458:3: note: in expansion of macro 'dev_err'
    1458 |   dev_err(gpii->gpi_dev->dev, "could not alloc size:%lu mem for ring\n",
         |   ^~~~~~~
   drivers/dma/qcom/gpi.c:1458:55: note: format string is defined here
    1458 |   dev_err(gpii->gpi_dev->dev, "could not alloc size:%lu mem for ring\n",
         |                                                     ~~^
         |                                                       |
         |                                                       long unsigned int
         |                                                     %u
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from include/asm-generic/bug.h:20,
                    from arch/arm/include/asm/bug.h:60,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from include/asm-generic/current.h:5,
                    from ./arch/arm/include/generated/asm/current.h:1,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/qcom/gpi.c:10:
>> drivers/dma/qcom/gpi.c:1478:3: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Wformat=]
    1478 |   "phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:125:15: note: in definition of macro '__dynamic_func_call'
     125 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:157:2: note: in expansion of macro '_dynamic_func_call'
     157 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/dma/qcom/gpi.c:1477:2: note: in expansion of macro 'dev_dbg'
    1477 |  dev_dbg(gpii->gpi_dev->dev,
         |  ^~~~~~~
   drivers/dma/qcom/gpi.c:1478:18: note: format string is defined here
    1478 |   "phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
         |              ~~~~^
         |                  |
         |                  long long unsigned int
         |              %0x
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from include/asm-generic/bug.h:20,
                    from arch/arm/include/asm/bug.h:60,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from include/asm-generic/current.h:5,
                    from ./arch/arm/include/generated/asm/current.h:1,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/qcom/gpi.c:10:
>> drivers/dma/qcom/gpi.c:1478:3: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'phys_addr_t' {aka 'unsigned int'} [-Wformat=]
    1478 |   "phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:125:15: note: in definition of macro '__dynamic_func_call'
     125 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:157:2: note: in expansion of macro '_dynamic_func_call'
     157 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/dma/qcom/gpi.c:1477:2: note: in expansion of macro 'dev_dbg'
    1477 |  dev_dbg(gpii->gpi_dev->dev,
         |  ^~~~~~~
   drivers/dma/qcom/gpi.c:1478:35: note: format string is defined here
    1478 |   "phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
         |                               ~~~~^
         |                                   |
         |                                   long long unsigned int
         |                               %0x
   drivers/dma/qcom/gpi.c: In function 'gpi_prep_slave_sg':
   drivers/dma/qcom/gpi.c:1772:13: error: implicit declaration of function 'kzalloc'; did you mean 'vzalloc'? [-Werror=implicit-function-declaration]
    1772 |  gpi_desc = kzalloc(sizeof(*gpi_desc), GFP_NOWAIT);
         |             ^~~~~~~
         |             vzalloc
   drivers/dma/qcom/gpi.c:1772:11: warning: assignment to 'struct gpi_desc *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
    1772 |  gpi_desc = kzalloc(sizeof(*gpi_desc), GFP_NOWAIT);
         |           ^
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/25062cc46a95e9de91752963ea85b934fe7acfa1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vinod-Koul/dmaengine-Add-support-for-Qcom-GSI-dma-controller/20200824-174027
git checkout 25062cc46a95e9de91752963ea85b934fe7acfa1
vim +1450 drivers/dma/qcom/gpi.c

  1435	
  1436	/* allocate memory for transfer and event rings */
  1437	static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
  1438				  u32 el_size, struct gpii *gpii)
  1439	{
  1440		u64 len = elements * el_size;
  1441		int bit;
  1442	
  1443		/* ring len must be power of 2 */
  1444		bit = find_last_bit((unsigned long *)&len, 32);
  1445		if (((1 << bit) - 1) & len)
  1446			bit++;
  1447		len = 1 << bit;
  1448		ring->alloc_size = (len + (len - 1));
  1449		dev_dbg(gpii->gpi_dev->dev,
> 1450			"#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%lu\n",
  1451			  elements, el_size, (elements * el_size), len,
  1452			  ring->alloc_size);
  1453	
  1454		ring->pre_aligned = dma_alloc_coherent(gpii->gpi_dev->dev,
  1455						       ring->alloc_size,
  1456						       &ring->dma_handle, GFP_KERNEL);
  1457		if (!ring->pre_aligned) {
  1458			dev_err(gpii->gpi_dev->dev, "could not alloc size:%lu mem for ring\n",
  1459				ring->alloc_size);
  1460			return -ENOMEM;
  1461		}
  1462	
  1463		/* align the physical mem */
  1464		ring->phys_addr = (ring->dma_handle + (len - 1)) & ~(len - 1);
  1465		ring->base = ring->pre_aligned + (ring->phys_addr - ring->dma_handle);
  1466		ring->rp = ring->base;
  1467		ring->wp = ring->base;
  1468		ring->len = len;
  1469		ring->el_size = el_size;
  1470		ring->elements = ring->len / ring->el_size;
  1471		memset(ring->base, 0, ring->len);
  1472		ring->configured = true;
  1473	
  1474		/* update to other cores */
  1475		smp_wmb();
  1476	
  1477		dev_dbg(gpii->gpi_dev->dev,
> 1478			"phy_pre:0x%0llx phy_alig:0x%0llx len:%u el_size:%u elements:%u\n",
  1479			ring->dma_handle, ring->phys_addr, ring->len,
  1480			ring->el_size, ring->elements);
  1481	
  1482		return 0;
  1483	}
  1484	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-24  8:47 ` [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding Vinod Koul
@ 2020-08-24 17:40   ` Rob Herring
  2020-08-25 14:51     ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-08-24 17:40 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Bjorn Andersson, Rob Herring, linux-arm-msm, devicetree,
	linux-kernel, dmaengine

On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> Add devicetree binding documentation for GPI DMA controller
> implemented on Qualcomm SoCs
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,gpii-mask: {'description': 'Bitmap of supported GPII instances for OS', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,gpii-mask: 'not' is a required property

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,max-num-gpii: {'description': 'Maximum number of GPII instances available', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,max-num-gpii: 'not' is a required property

./Documentation/devicetree/bindings/dma/qcom-gpi.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/dma/qcom-gpi.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: ignoring, error in schema: properties: qcom,max-num-gpii
warning: no schema found in file: ./Documentation/devicetree/bindings/dma/qcom-gpi.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.example.dt.yaml: example-0: dma@800000:reg:0: [0, 8388608, 0, 393216] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml


See https://patchwork.ozlabs.org/patch/1350170

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.


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

* Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-24  8:47 ` [RFC PATCH 2/3] dmaengine: add peripheral configuration Vinod Koul
@ 2020-08-25  6:52   ` Peter Ujfalusi
  2020-08-25  7:10     ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2020-08-25  6:52 UTC (permalink / raw)
  To: Vinod Koul, dmaengine
  Cc: Rob Herring, Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel

Hi Vinod,

On 24/08/2020 11.47, Vinod Koul wrote:
> Some complex dmaengine controllers have capability to program the
> peripheral device, so pass on the peripheral configuration as part of
> dma_slave_config
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  include/linux/dmaengine.h | 75 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 6fbd5c99e30c..264643ca9209 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -380,6 +380,73 @@ enum dma_slave_buswidth {
>  	DMA_SLAVE_BUSWIDTH_64_BYTES = 64,
>  };
>  
> +/**
> + * enum spi_transfer_cmd - spi transfer commands
> + */
> +enum spi_transfer_cmd {
> +	SPI_TX = 1,
> +	SPI_RX,
> +	SPI_DUPLEX,
> +};
> +
> +/**
> + * struct dmaengine_spi_config - spi config for peripheral
> + *
> + * @loopback_en: spi loopback enable when set
> + * @clock_pol: clock polarity
> + * @data_pol: data polarity
> + * @pack_en: process tx/rx buffers as packed
> + * @word_len: spi word length
> + * @clk_div: source clock divider
> + * @clk_src: serial clock
> + * @cmd: spi cmd
> + * @cs: chip select toggle
> + * @rx_len: receive length for spi buffer
> + */
> +struct dmaengine_spi_config {
> +	u8 loopback_en;
> +	u8 clock_pol;
> +	u8 data_pol;
> +	u8 pack_en;
> +	u8 word_len;
> +	u32 clk_div;
> +	u32 clk_src;
> +	u8 fragmentation;
> +	enum spi_transfer_cmd cmd;
> +	u8 cs;
> +	u32 rx_len;
> +};
> +
> +/**
> + * struct dmaengine_i2c_config - i2c config for peripheral
> + *
> + * @pack_enable: process tx/rx buffers as packed
> + * @cycle_count: clock cycles to be sent
> + * @high_count: high period of clock
> + * @low_count: low period of clock
> + * @clk_div: source clock divider
> + * @addr: i2c bus address
> + * @strech: strech the clock at eot
> + * @op: i2c cmd
> + */
> +struct dmaengine_i2c_config {
> +	u8 pack_enable;
> +	u8 cycle_count;
> +	u8 high_count;
> +	u8 low_count;
> +	u16 clk_div;
> +	u8 addr;
> +	u8 strech;
> +	u8 op;
> +};
> +
> +enum dmaengine_peripheral {
> +	DMAENGINE_PERIPHERAL_SPI,
> +	DMAENGINE_PERIPHERAL_I2C,
> +	DMAENGINE_PERIPHERAL_UART,
> +	DMAENGINE_PERIPHERAL_LAST = DMAENGINE_PERIPHERAL_UART,
> +};
> +
>  /**
>   * struct dma_slave_config - dma slave channel runtime config
>   * @direction: whether the data shall go in or out on this slave
> @@ -418,6 +485,10 @@ enum dma_slave_buswidth {
>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
>   * slave peripheral will have unique id as dma requester which need to be
>   * pass as slave config.
> + * @peripheral: type of peripheral to DMA to/from
> + * @set_config: set peripheral config
> + * @spi: peripheral config for spi
> + * @:i2c peripheral config for i2c
>   *
>   * This struct is passed in as configuration data to a DMA engine
>   * in order to set up a certain channel for DMA transport at runtime.
> @@ -443,6 +514,10 @@ struct dma_slave_config {
>  	u32 dst_port_window_size;
>  	bool device_fc;
>  	unsigned int slave_id;
> +	enum dmaengine_peripheral peripheral;
> +	u8 set_config;
> +	struct dmaengine_spi_config spi;
> +	struct dmaengine_i2c_config i2c;

Would it be possible to reuse one of the existing feature already
supported by DMAengine?
We have DMA_PREP_CMD to give a command instead of a real transfer:
dmaengine_prep_slave_single(tx_chan, config_data, config_len,
			    DMA_MEM_TO_DEV, DMA_PREP_CMD);
dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len, DMA_MEM_TO_DEV,
			    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
dma_async_issue_pending(tx_chan);

or the metadata support:
tx = dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len,
				 DMA_MEM_TO_DEV,
				 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
dmaengine_desc_attach_metadata(tx, config_data, config_len);
dma_async_issue_pending(tx_chan);

By reading the driver itself, it is not clear if you always need to send
the config for TX, or only when the config is changing and what happens
if the first transfer (for SPI, since that is the only implemented one)
is RX, when you don't send config at all...

I'm concerned about the size increase of dma_slave_config (it grows by
>30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
to a sizeable amount.

>  };
>  
>  /**
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-25  6:52   ` Peter Ujfalusi
@ 2020-08-25  7:10     ` Vinod Koul
  2020-08-25  8:00       ` Peter Ujfalusi
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-25  7:10 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: dmaengine, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

Hello Peter,

On 25-08-20, 09:52, Peter Ujfalusi wrote:
> Hi Vinod,
> 
> On 24/08/2020 11.47, Vinod Koul wrote:
> > Some complex dmaengine controllers have capability to program the
> > peripheral device, so pass on the peripheral configuration as part of
> > dma_slave_config
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  include/linux/dmaengine.h | 75 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 75 insertions(+)
> > 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 6fbd5c99e30c..264643ca9209 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -380,6 +380,73 @@ enum dma_slave_buswidth {
> >  	DMA_SLAVE_BUSWIDTH_64_BYTES = 64,
> >  };
> >  
> > +/**
> > + * enum spi_transfer_cmd - spi transfer commands
> > + */
> > +enum spi_transfer_cmd {
> > +	SPI_TX = 1,
> > +	SPI_RX,
> > +	SPI_DUPLEX,
> > +};
> > +
> > +/**
> > + * struct dmaengine_spi_config - spi config for peripheral
> > + *
> > + * @loopback_en: spi loopback enable when set
> > + * @clock_pol: clock polarity
> > + * @data_pol: data polarity
> > + * @pack_en: process tx/rx buffers as packed
> > + * @word_len: spi word length
> > + * @clk_div: source clock divider
> > + * @clk_src: serial clock
> > + * @cmd: spi cmd
> > + * @cs: chip select toggle
> > + * @rx_len: receive length for spi buffer
> > + */
> > +struct dmaengine_spi_config {
> > +	u8 loopback_en;
> > +	u8 clock_pol;
> > +	u8 data_pol;
> > +	u8 pack_en;
> > +	u8 word_len;
> > +	u32 clk_div;
> > +	u32 clk_src;
> > +	u8 fragmentation;
> > +	enum spi_transfer_cmd cmd;
> > +	u8 cs;
> > +	u32 rx_len;
> > +};
> > +
> > +/**
> > + * struct dmaengine_i2c_config - i2c config for peripheral
> > + *
> > + * @pack_enable: process tx/rx buffers as packed
> > + * @cycle_count: clock cycles to be sent
> > + * @high_count: high period of clock
> > + * @low_count: low period of clock
> > + * @clk_div: source clock divider
> > + * @addr: i2c bus address
> > + * @strech: strech the clock at eot
> > + * @op: i2c cmd
> > + */
> > +struct dmaengine_i2c_config {
> > +	u8 pack_enable;
> > +	u8 cycle_count;
> > +	u8 high_count;
> > +	u8 low_count;
> > +	u16 clk_div;
> > +	u8 addr;
> > +	u8 strech;
> > +	u8 op;
> > +};
> > +
> > +enum dmaengine_peripheral {
> > +	DMAENGINE_PERIPHERAL_SPI,
> > +	DMAENGINE_PERIPHERAL_I2C,
> > +	DMAENGINE_PERIPHERAL_UART,
> > +	DMAENGINE_PERIPHERAL_LAST = DMAENGINE_PERIPHERAL_UART,
> > +};
> > +
> >  /**
> >   * struct dma_slave_config - dma slave channel runtime config
> >   * @direction: whether the data shall go in or out on this slave
> > @@ -418,6 +485,10 @@ enum dma_slave_buswidth {
> >   * @slave_id: Slave requester id. Only valid for slave channels. The dma
> >   * slave peripheral will have unique id as dma requester which need to be
> >   * pass as slave config.
> > + * @peripheral: type of peripheral to DMA to/from
> > + * @set_config: set peripheral config
> > + * @spi: peripheral config for spi
> > + * @:i2c peripheral config for i2c
> >   *
> >   * This struct is passed in as configuration data to a DMA engine
> >   * in order to set up a certain channel for DMA transport at runtime.
> > @@ -443,6 +514,10 @@ struct dma_slave_config {
> >  	u32 dst_port_window_size;
> >  	bool device_fc;
> >  	unsigned int slave_id;
> > +	enum dmaengine_peripheral peripheral;
> > +	u8 set_config;
> > +	struct dmaengine_spi_config spi;
> > +	struct dmaengine_i2c_config i2c;
> 
> Would it be possible to reuse one of the existing feature already
> supported by DMAengine?
> We have DMA_PREP_CMD to give a command instead of a real transfer:
> dmaengine_prep_slave_single(tx_chan, config_data, config_len,
> 			    DMA_MEM_TO_DEV, DMA_PREP_CMD);
> dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len, DMA_MEM_TO_DEV,
> 			    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> dma_async_issue_pending(tx_chan);
> 
> or the metadata support:
> tx = dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len,
> 				 DMA_MEM_TO_DEV,
> 				 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> dmaengine_desc_attach_metadata(tx, config_data, config_len);
> dma_async_issue_pending(tx_chan);
> 
> By reading the driver itself, it is not clear if you always need to send
> the config for TX, or only when the config is changing and what happens
> if the first transfer (for SPI, since that is the only implemented one)
> is RX, when you don't send config at all...

So this config is sent to driver everytime before the prep call (can be
optimized to once if we have similar transfers in queue).

This config is used to create the configuration passed to dmaengine
which is used to actually program both dmaengine as well as peripheral
registers (i2c/spi/etc), so we need a way to pass the spi/i2c config.

I think prep cmd can be used to send this data, I do not see any issues
with that, it would work if we want to go that route.

I did have a prototype with metadata but didnt work very well, the
problem is it assumes metadata for tx/rx but here i send the data
everytime from client data.

> I'm concerned about the size increase of dma_slave_config (it grows by
> >30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
> to a sizeable amount.

I agree that is indeed a valid concern, that is the reason I tagged this
as a RFC patch ;-)

I see the prep_cmd is a better approach for this, anyone else has better
suggestions?

Thanks for looking in.

-- 
~Vinod

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

* Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-25  7:10     ` Vinod Koul
@ 2020-08-25  8:00       ` Peter Ujfalusi
  2020-08-25 11:02         ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2020-08-25  8:00 UTC (permalink / raw)
  To: Vinod Koul
  Cc: dmaengine, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

Hi Vinod,

On 25/08/2020 10.10, Vinod Koul wrote:
>>>  /**
>>>   * struct dma_slave_config - dma slave channel runtime config
>>>   * @direction: whether the data shall go in or out on this slave
>>> @@ -418,6 +485,10 @@ enum dma_slave_buswidth {
>>>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
>>>   * slave peripheral will have unique id as dma requester which need to be
>>>   * pass as slave config.
>>> + * @peripheral: type of peripheral to DMA to/from
>>> + * @set_config: set peripheral config
>>> + * @spi: peripheral config for spi
>>> + * @:i2c peripheral config for i2c
>>>   *
>>>   * This struct is passed in as configuration data to a DMA engine
>>>   * in order to set up a certain channel for DMA transport at runtime.
>>> @@ -443,6 +514,10 @@ struct dma_slave_config {
>>>  	u32 dst_port_window_size;
>>>  	bool device_fc;
>>>  	unsigned int slave_id;
>>> +	enum dmaengine_peripheral peripheral;
>>> +	u8 set_config;
>>> +	struct dmaengine_spi_config spi;
>>> +	struct dmaengine_i2c_config i2c;
>>
>> Would it be possible to reuse one of the existing feature already
>> supported by DMAengine?
>> We have DMA_PREP_CMD to give a command instead of a real transfer:
>> dmaengine_prep_slave_single(tx_chan, config_data, config_len,
>> 			    DMA_MEM_TO_DEV, DMA_PREP_CMD);
>> dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len, DMA_MEM_TO_DEV,
>> 			    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> dma_async_issue_pending(tx_chan);
>>
>> or the metadata support:
>> tx = dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len,
>> 				 DMA_MEM_TO_DEV,
>> 				 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> dmaengine_desc_attach_metadata(tx, config_data, config_len);
>> dma_async_issue_pending(tx_chan);
>>
>> By reading the driver itself, it is not clear if you always need to send
>> the config for TX, or only when the config is changing and what happens
>> if the first transfer (for SPI, since that is the only implemented one)
>> is RX, when you don't send config at all...
> 
> So this config is sent to driver everytime before the prep call (can be
> optimized to once if we have similar transfers in queue).

I see that you queue the TREs in the prep callback.

> This config is used to create the configuration passed to dmaengine
> which is used to actually program both dmaengine as well as peripheral
> registers (i2c/spi/etc), so we need a way to pass the spi/i2c config.

But do you need to send it with each DMA_MEM_TO_DEV or only once?
DMA_DEV_TO_MEM does not set the config, so I assume you must have one TX
to initialize the peripheral as the first transfer.

> I think prep cmd can be used to send this data, I do not see any issues
> with that, it would work if we want to go that route.

The only thing which might be an issue is that with the DMA_PREP_CMD the
config_data is dma_addr_t (via dmaengine_prep_slave_single).

> I did have a prototype with metadata but didnt work very well, the
> problem is it assumes metadata for tx/rx but here i send the data
> everytime from client data.

Yes, the intended use case for metadata (per descriptor!) is for
channels where each transfer might have different metadata needed for
the given transfer (tx/rx).

In your case you have semi static peripheral configuration data, which
is not really changing between transfers.

A compromise would be to add:
void *peripheral_config;
to the dma_slave_config, move the set_config inside of the device
specific struct you are passing from a client to the core?

>> I'm concerned about the size increase of dma_slave_config (it grows by
>>> 30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
>> to a sizeable amount.
> 
> I agree that is indeed a valid concern, that is the reason I tagged this
> as a RFC patch ;-)
> 
> I see the prep_cmd is a better approach for this, anyone else has better
> suggestions?
> 
> Thanks for looking in.
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-25  8:00       ` Peter Ujfalusi
@ 2020-08-25 11:02         ` Vinod Koul
  2020-08-26  7:07           ` Peter Ujfalusi
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-25 11:02 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: dmaengine, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

Hi Peter,

On 25-08-20, 11:00, Peter Ujfalusi wrote:
> Hi Vinod,
> 
> On 25/08/2020 10.10, Vinod Koul wrote:
> >>>  /**
> >>>   * struct dma_slave_config - dma slave channel runtime config
> >>>   * @direction: whether the data shall go in or out on this slave
> >>> @@ -418,6 +485,10 @@ enum dma_slave_buswidth {
> >>>   * @slave_id: Slave requester id. Only valid for slave channels. The dma
> >>>   * slave peripheral will have unique id as dma requester which need to be
> >>>   * pass as slave config.
> >>> + * @peripheral: type of peripheral to DMA to/from
> >>> + * @set_config: set peripheral config
> >>> + * @spi: peripheral config for spi
> >>> + * @:i2c peripheral config for i2c
> >>>   *
> >>>   * This struct is passed in as configuration data to a DMA engine
> >>>   * in order to set up a certain channel for DMA transport at runtime.
> >>> @@ -443,6 +514,10 @@ struct dma_slave_config {
> >>>  	u32 dst_port_window_size;
> >>>  	bool device_fc;
> >>>  	unsigned int slave_id;
> >>> +	enum dmaengine_peripheral peripheral;
> >>> +	u8 set_config;
> >>> +	struct dmaengine_spi_config spi;
> >>> +	struct dmaengine_i2c_config i2c;
> >>
> >> Would it be possible to reuse one of the existing feature already
> >> supported by DMAengine?
> >> We have DMA_PREP_CMD to give a command instead of a real transfer:
> >> dmaengine_prep_slave_single(tx_chan, config_data, config_len,
> >> 			    DMA_MEM_TO_DEV, DMA_PREP_CMD);
> >> dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len, DMA_MEM_TO_DEV,
> >> 			    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> >> dma_async_issue_pending(tx_chan);
> >>
> >> or the metadata support:
> >> tx = dmaengine_prep_slave_single(tx_chan, tx_buff, tx_len,
> >> 				 DMA_MEM_TO_DEV,
> >> 				 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> >> dmaengine_desc_attach_metadata(tx, config_data, config_len);
> >> dma_async_issue_pending(tx_chan);
> >>
> >> By reading the driver itself, it is not clear if you always need to send
> >> the config for TX, or only when the config is changing and what happens
> >> if the first transfer (for SPI, since that is the only implemented one)
> >> is RX, when you don't send config at all...
> > 
> > So this config is sent to driver everytime before the prep call (can be
> > optimized to once if we have similar transfers in queue).
> 
> I see that you queue the TREs in the prep callback.

Yes

> > This config is used to create the configuration passed to dmaengine
> > which is used to actually program both dmaengine as well as peripheral
> > registers (i2c/spi/etc), so we need a way to pass the spi/i2c config.
> 
> But do you need to send it with each DMA_MEM_TO_DEV or only once?
> DMA_DEV_TO_MEM does not set the config, so I assume you must have one TX
> to initialize the peripheral as the first transfer.

Correct and we do transfers on same without sending configuration again.

> > I think prep cmd can be used to send this data, I do not see any issues
> > with that, it would work if we want to go that route.
> 
> The only thing which might be an issue is that with the DMA_PREP_CMD the
> config_data is dma_addr_t (via dmaengine_prep_slave_single).

Yes I came to same conclusion

> > I did have a prototype with metadata but didnt work very well, the
> > problem is it assumes metadata for tx/rx but here i send the data
> > everytime from client data.
> 
> Yes, the intended use case for metadata (per descriptor!) is for
> channels where each transfer might have different metadata needed for
> the given transfer (tx/rx).
> 
> In your case you have semi static peripheral configuration data, which
> is not really changing between transfers.
> 
> A compromise would be to add:
> void *peripheral_config;
> to the dma_slave_config, move the set_config inside of the device
> specific struct you are passing from a client to the core?

That sounds more saner to me and uses existing interfaces cleanly. I
think I like this option ;-)

> >> I'm concerned about the size increase of dma_slave_config (it grows by
> >>> 30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
> >> to a sizeable amount.
> > 
> > I agree that is indeed a valid concern, that is the reason I tagged this
> > as a RFC patch ;-)
> > 
> > I see the prep_cmd is a better approach for this, anyone else has better
> > suggestions?
> > 
> > Thanks for looking in.
> > 
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
~Vinod

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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-24 17:40   ` Rob Herring
@ 2020-08-25 14:51     ` Vinod Koul
  2020-08-26  6:32       ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-25 14:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Andersson, Rob Herring, linux-arm-msm, devicetree,
	linux-kernel, dmaengine

Hey Rob,

On 24-08-20, 11:40, Rob Herring wrote:
> On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> > Add devicetree binding documentation for GPI DMA controller
> > implemented on Qualcomm SoCs
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
> >  1 file changed, 87 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> > 
> 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property

Okay updating dt-schema I do see this, now the question is what is this
and what does it mean ;-) I am not sure I comprehend the error message.
I see this for all the new properties I added as required for this
device node

> 
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,gpii-mask: {'description': 'Bitmap of supported GPII instances for OS', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,gpii-mask: 'not' is a required property
> 
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,max-num-gpii: {'description': 'Maximum number of GPII instances available', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,max-num-gpii: 'not' is a required property
> 
> ./Documentation/devicetree/bindings/dma/qcom-gpi.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/dma/qcom-gpi.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: ignoring, error in schema: properties: qcom,max-num-gpii
> warning: no schema found in file: ./Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.example.dt.yaml: example-0: dma@800000:reg:0: [0, 8388608, 0, 393216] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> 
> 
> See https://patchwork.ozlabs.org/patch/1350170
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
> 
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> 
> Please check and re-submit.

-- 
~Vinod

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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-25 14:51     ` Vinod Koul
@ 2020-08-26  6:32       ` Vinod Koul
  2020-08-26 14:35         ` Rob Herring
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-26  6:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Andersson, Rob Herring, linux-arm-msm, devicetree,
	linux-kernel, dmaengine

On 25-08-20, 20:21, Vinod Koul wrote:
> Hey Rob,
> 
> On 24-08-20, 11:40, Rob Herring wrote:
> > On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> > > Add devicetree binding documentation for GPI DMA controller
> > > implemented on Qualcomm SoCs
> > > 
> > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > ---
> > >  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
> > >  1 file changed, 87 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> > > 
> > 
> > 
> > My bot found errors running 'make dt_binding_check' on your patch:
> > 
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> > 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property
> 
> Okay updating dt-schema I do see this, now the question is what is this
> and what does it mean ;-) I am not sure I comprehend the error message.
> I see this for all the new properties I added as required for this
> device node

Okay I think I have figured it out, I need to provide ref to
/schemas/types.yaml#definitions/uint32 for this to work, which does
makes sense to me.

  qcom,max-num-gpii:
    $ref: /schemas/types.yaml#definitions/uint32
    maxItems: 1
    description:
      Number of GPII instances

Looks good to schema tool
 
-- 
~Vinod

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

* Re: [RFC PATCH 2/3] dmaengine: add peripheral configuration
  2020-08-25 11:02         ` Vinod Koul
@ 2020-08-26  7:07           ` Peter Ujfalusi
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2020-08-26  7:07 UTC (permalink / raw)
  To: Vinod Koul
  Cc: dmaengine, Rob Herring, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

Hi Vinod,

On 25/08/2020 14.02, Vinod Koul wrote:
>> The only thing which might be an issue is that with the DMA_PREP_CMD the
>> config_data is dma_addr_t (via dmaengine_prep_slave_single).
> 
> Yes I came to same conclusion
> 
>>> I did have a prototype with metadata but didnt work very well, the
>>> problem is it assumes metadata for tx/rx but here i send the data
>>> everytime from client data.
>>
>> Yes, the intended use case for metadata (per descriptor!) is for
>> channels where each transfer might have different metadata needed for
>> the given transfer (tx/rx).
>>
>> In your case you have semi static peripheral configuration data, which
>> is not really changing between transfers.
>>
>> A compromise would be to add:
>> void *peripheral_config;
>> to the dma_slave_config, move the set_config inside of the device
>> specific struct you are passing from a client to the core?
> 
> That sounds more saner to me and uses existing interfaces cleanly. I
> think I like this option ;-)

The other option would be to use the descriptor metadata support and
that might be even cleaner.

In gpi_create_tre() via gpi_prep_slave_sg() you would set up the
desc->tre[1] and desc->tre[2] for TX
desc->tre[2] for RX
in the desc, you add a new variable, let's say first_tre and
set it to 1 for TX, 2 for RX.

If you need to send a config, you attach it via either way metadata
support allows you (get the pointer to desc->tre[0] or give a config
struct to the DMA driver.

In the metadata handler, you check if the transfer is TX, if it is, then
you update desc->tre[0] (or give the pointer to the client) and update
the first_tre to 0.

In issue_pending, or a small helper which can be used to start the
transfer you would do the queuing instead of prepare time:
for (i = gpi_desc->first_tre; i < MAX_TRE; i++)
	gpi_queue_xfer(gpii, gpii_chan,  &gpi_desc->tre[i], &wp);

With this change it should work neatly without any change to
dma_slave_config.

> 
>>>> I'm concerned about the size increase of dma_slave_config (it grows by
>>>>> 30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up
>>>> to a sizeable amount.
>>>
>>> I agree that is indeed a valid concern, that is the reason I tagged this
>>> as a RFC patch ;-)
>>>
>>> I see the prep_cmd is a better approach for this, anyone else has better
>>> suggestions?
>>>
>>> Thanks for looking in.
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-26  6:32       ` Vinod Koul
@ 2020-08-26 14:35         ` Rob Herring
  2020-08-27  4:50           ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-08-26 14:35 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM

On Wed, Aug 26, 2020 at 12:32 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 25-08-20, 20:21, Vinod Koul wrote:
> > Hey Rob,
> >
> > On 24-08-20, 11:40, Rob Herring wrote:
> > > On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> > > > Add devicetree binding documentation for GPI DMA controller
> > > > implemented on Qualcomm SoCs
> > > >
> > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > ---
> > > >  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
> > > >  1 file changed, 87 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> > > >
> > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> > >     /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property
> >
> > Okay updating dt-schema I do see this, now the question is what is this
> > and what does it mean ;-) I am not sure I comprehend the error message.
> > I see this for all the new properties I added as required for this
> > device node
>
> Okay I think I have figured it out, I need to provide ref to
> /schemas/types.yaml#definitions/uint32 for this to work, which does
> makes sense to me.
>
>   qcom,max-num-gpii:
>     $ref: /schemas/types.yaml#definitions/uint32
>     maxItems: 1

uint32 is always 1 item, so drop. Is there a max value you can define?
Otherwise, up to 2^32 - 1 is valid.

>     description:
>       Number of GPII instances
>
> Looks good to schema tool
>
> --
> ~Vinod

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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-26 14:35         ` Rob Herring
@ 2020-08-27  4:50           ` Vinod Koul
  2020-08-27 14:04             ` Rob Herring
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2020-08-27  4:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM

On 26-08-20, 08:35, Rob Herring wrote:
> On Wed, Aug 26, 2020 at 12:32 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 25-08-20, 20:21, Vinod Koul wrote:
> > > Hey Rob,
> > >
> > > On 24-08-20, 11:40, Rob Herring wrote:
> > > > On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> > > > > Add devicetree binding documentation for GPI DMA controller
> > > > > implemented on Qualcomm SoCs
> > > > >
> > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > > ---
> > > > >  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
> > > > >  1 file changed, 87 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> > > > >
> > > >
> > > >
> > > > My bot found errors running 'make dt_binding_check' on your patch:
> > > >
> > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> > > >     /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property
> > >
> > > Okay updating dt-schema I do see this, now the question is what is this
> > > and what does it mean ;-) I am not sure I comprehend the error message.
> > > I see this for all the new properties I added as required for this
> > > device node
> >
> > Okay I think I have figured it out, I need to provide ref to
> > /schemas/types.yaml#definitions/uint32 for this to work, which does
> > makes sense to me.
> >
> >   qcom,max-num-gpii:
> >     $ref: /schemas/types.yaml#definitions/uint32
> >     maxItems: 1
> 
> uint32 is always 1 item, so drop. Is there a max value you can define?

Sorry not sure I follow, to clarify you mean drop uint32, if so which
type to use u8? I can use u8 as max wont be beyond 255.

Yes I will define min as well max values too.

> Otherwise, up to 2^32 - 1 is valid.

I see one more warning given by your bot which I am able to reproduce as
well:
Documentation/devicetree/bindings/dma/qcom,gpi.example.dt.yaml: example-0: dma-controller@800000:reg:0: [0, 8388608, 0, 393216] is too long

So to fix this I added the #address-cells and #size-cells

        #address-cells = <2>;
        #size-cells = <2>;
        reg = <0x0 0x00800000 0x0 0x60000>;

But I am getting the warning, what am I doing incorrect

-- 
~Vinod

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

* Re: [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding
  2020-08-27  4:50           ` Vinod Koul
@ 2020-08-27 14:04             ` Rob Herring
  0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-08-27 14:04 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM

On Wed, Aug 26, 2020 at 10:50 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 26-08-20, 08:35, Rob Herring wrote:
> > On Wed, Aug 26, 2020 at 12:32 AM Vinod Koul <vkoul@kernel.org> wrote:
> > >
> > > On 25-08-20, 20:21, Vinod Koul wrote:
> > > > Hey Rob,
> > > >
> > > > On 24-08-20, 11:40, Rob Herring wrote:
> > > > > On Mon, 24 Aug 2020 14:17:10 +0530, Vinod Koul wrote:
> > > > > > Add devicetree binding documentation for GPI DMA controller
> > > > > > implemented on Qualcomm SoCs
> > > > > >
> > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > > > ---
> > > > > >  .../devicetree/bindings/dma/qcom-gpi.yaml     | 87 +++++++++++++++++++
> > > > > >  1 file changed, 87 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/dma/qcom-gpi.yaml
> > > > > >
> > > > >
> > > > >
> > > > > My bot found errors running 'make dt_binding_check' on your patch:
> > > > >
> > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: {'description': 'Event ring transfer size compare to channel transfer ring. Event ring length = ev-factor * transfer ring size', 'maxItems': 1} is not valid under any of the given schemas (Possible causes of the failure):
> > > > >     /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dma/qcom-gpi.yaml: properties:qcom,ev-factor: 'not' is a required property
> > > >
> > > > Okay updating dt-schema I do see this, now the question is what is this
> > > > and what does it mean ;-) I am not sure I comprehend the error message.
> > > > I see this for all the new properties I added as required for this
> > > > device node
> > >
> > > Okay I think I have figured it out, I need to provide ref to
> > > /schemas/types.yaml#definitions/uint32 for this to work, which does
> > > makes sense to me.
> > >
> > >   qcom,max-num-gpii:
> > >     $ref: /schemas/types.yaml#definitions/uint32
> > >     maxItems: 1
> >
> > uint32 is always 1 item, so drop. Is there a max value you can define?
>
> Sorry not sure I follow, to clarify you mean drop uint32, if so which
> type to use u8? I can use u8 as max wont be beyond 255.

maxItems applies to arrays. A uint32 is not an array, so drop 'maxItems'.

>
> Yes I will define min as well max values too.
>
> > Otherwise, up to 2^32 - 1 is valid.
>
> I see one more warning given by your bot which I am able to reproduce as
> well:
> Documentation/devicetree/bindings/dma/qcom,gpi.example.dt.yaml: example-0: dma-controller@800000:reg:0: [0, 8388608, 0, 393216] is too long
>
> So to fix this I added the #address-cells and #size-cells
>
>         #address-cells = <2>;
>         #size-cells = <2>;
>         reg = <0x0 0x00800000 0x0 0x60000>;
>
> But I am getting the warning, what am I doing incorrect

The cell sizes for reg come from the parent node. The default for
examples is 1 cell each. The easiest thing to do is change reg to
'<0x00800000 0x60000>'. Otherwise, you'd need to define a parent bus
node.

Rob

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

end of thread, other threads:[~2020-08-27 14:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24  8:47 [PATCH 0/3] dmaengine: Add support for Qcom GSI dma controller Vinod Koul
2020-08-24  8:47 ` [PATCH 1/3] dt-bindings: dmaengine: Document qcom,gpi dma binding Vinod Koul
2020-08-24 17:40   ` Rob Herring
2020-08-25 14:51     ` Vinod Koul
2020-08-26  6:32       ` Vinod Koul
2020-08-26 14:35         ` Rob Herring
2020-08-27  4:50           ` Vinod Koul
2020-08-27 14:04             ` Rob Herring
2020-08-24  8:47 ` [RFC PATCH 2/3] dmaengine: add peripheral configuration Vinod Koul
2020-08-25  6:52   ` Peter Ujfalusi
2020-08-25  7:10     ` Vinod Koul
2020-08-25  8:00       ` Peter Ujfalusi
2020-08-25 11:02         ` Vinod Koul
2020-08-26  7:07           ` Peter Ujfalusi
2020-08-24  8:47 ` [PATCH 3/3] dmaengine: qcom: Add GPI dma driver Vinod Koul
2020-08-24 14:04   ` kernel test robot
2020-08-24 14:13   ` kernel test robot

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