Skip to contentSkip to conversation

Add Qualcomm ADM dmaengine driver

PATCH Superseded Andy Gross 3 participants 7 messages Feb 12, 2015 – Mar 17, 2015

Subsystems (3)
Device tree bindings Arm Qualcomm mailing list dmaengine

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Andy Gross

author
#0 lore raw

v4 cover Add Qualcomm ADM dmaengine driver

This patch set introduces the dmaengine driver for the Qualcomm Application Data Mover (ADM) DMA controller present on MSM8x60, APQ8064, and IPQ8064 devices.

The initial version of this driver will only support slave DMA operations between system memory and peripherals. Flow control via the CRCI (client rate control interface) is supported and can be configured via device tree configuration. Flow control usage is required for some peripheral devices.

Changes from v3:
- Remove .owner field

Changes from v2:
- Removed extraneous achan variable from xlate function
- Reworked crci check in slave_sg function
- Added mux field to async_desc structure.
- Reworked dma start function to use crci and mux values directly from structure.
- Added disable of clocks in probe error paths.
- Changed to use #define for fixed number of channels.

Changes since v1:
- Fixed various review comments
- Fixed some descriptor programming issues.
- Added single descriptors to support sub burst length transactions. Selection of single or box descriptors depends on the sg length and burst size.
- Removed use of crci in the dmas property. CRCI is now designated via the slave_config structure and will be stored in slave_id.

Andy Gross (2):
dt/bindings: qcom_adm: Fix channel specifiers
dmaengine: Add ADM driver

 Documentation/devicetree/bindings/dma/qcom_adm.txt |   16 +-
 drivers/dma/Kconfig                                |   10 +
 drivers/dma/Makefile                               |    1 +
 drivers/dma/qcom_adm.c                             |  901 ++++++++++++++++++++
 4 files changed, 918 insertions(+), 10 deletions(-)
 create mode 100644 drivers/dma/qcom_adm.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
author
#1 lore raw

PATCH v4 1/2 dt/bindings: qcom_adm: Fix channel specifiers

This patch removes the crci information from the dma channel property. At least one client device requires using more than one CRCI value for a channel. This does not match the current binding and the crci information needs to be removed.

Instead, the client device will provide this information via other means.

Signed-off-by Andy Gross <[email protected]>
---
 Documentation/devicetree/bindings/dma/qcom_adm.txt |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
Documentation/devicetree/bindings/dma/qcom_adm.txt +6 -10
index 9bcab91..38d45f8 100644
@@ -4,8 +4,7 @@ Required properties:
 - compatible: must contain "qcom,adm" for IPQ/APQ8064 and MSM8960
 - reg: Address range for DMA registers
 - interrupts: Should contain one interrupt shared by all channels
-- #dma-cells: must be <2>.  First cell denotes the channel number.  Second cell
-  denotes CRCI (client rate control interface) flow control assignment.
+- #dma-cells: must be <1>.  First cell denotes the channel number.
 - clocks: Should contain the core clock and interface clock.
 - clock-names: Must contain "core" for the core clock and "iface" for the
   interface clock.

@@ -22,7 +21,7 @@ Example:
 			compatible = "qcom,adm";
 			reg = <0x18300000 0x100000>;
 			interrupts = <0 170 0>;
-			#dma-cells = <2>;
+			#dma-cells = <1>;
 
 			clocks = <&gcc ADM0_CLK>, <&gcc ADM0_PBUS_CLK>;
 			clock-names = "core", "iface";

@@ -35,15 +34,12 @@ Example:
 			qcom,ee = <0>;
 		};
 
-DMA clients must use the format descripted in the dma.txt file, using a three
+DMA clients must use the format descripted in the dma.txt file, using a two
 cell specifier for each channel.
 
-Each dmas request consists of 3 cells:
+Each dmas request consists of two cells:
  1. phandle pointing to the DMA controller
  2. channel number
- 3. CRCI assignment, if applicable.  If no CRCI flow control is required, use 0.
-    The CRCI is used for flow control.  It identifies the peripheral device that
-    is the source/destination for the transferred data.
 
 Example:
 

@@ -56,7 +52,7 @@ Example:
 
 		cs-gpios = <&qcom_pinmux 20 0>;
 
-		dmas = <&adm_dma 6 9>,
-			<&adm_dma 5 10>;
+		dmas = <&adm_dma 6>,
+			<&adm_dma 5>;
 		dma-names = "rx", "tx";
 	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
author
#2 lore raw

PATCH v4 2/2 dmaengine: Add ADM driver

In reply to Andy Gross

Add the DMA engine driver for the QCOM Application Data Mover (ADM) DMA controller found in the MSM8x60 and IPQ/APQ8064 platforms.

The ADM supports both memory to memory transactions and memory to/from peripheral device transactions. The controller also provides flow control capabilities for transactions to/from peripheral devices.

The initial release of this driver supports slave transfers to/from peripherals and also incorporates CRCI (client rate control interface) flow control.

Signed-off-by Andy Gross <[email protected]>
---
 drivers/dma/Kconfig    |   10 +
 drivers/dma/Makefile   |    1 +
 drivers/dma/qcom_adm.c |  901 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 912 insertions(+)
 create mode 100644 drivers/dma/qcom_adm.c
drivers/dma/Kconfig +10 -0
index f2b2c4e..69bc15e 100644
@@ -464,4 +464,14 @@ 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_ADM
+	tristate "Qualcomm ADM support"
+	depends on ARCH_QCOM || (COMPILE_TEST && OF && ARM)
+	select DMA_ENGINE
+	select DMA_VIRTUAL_CHANNELS
+	---help---
+	  Enable support for the Qualcomm ADM DMA controller.  This controller
+	  provides DMA capabilities for both general purpose and on-chip
+	  peripheral devices.
+
 endif
drivers/dma/Makefile +1 -0
index 2022b54..3b7ead6 100644
@@ -50,3 +50,4 @@ obj-y += xilinx/
 obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
 obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o
 obj-$(CONFIG_DMA_SUN6I) += sun6i-dma.o
+obj-$(CONFIG_QCOM_ADM) += qcom_adm.o
drivers/dma/qcom_adm.c new file +901 -0

901 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#3 lore raw

Re: [Patch v4 2/2] dmaengine: Add ADM driver

drivers/dma/qcom_adm.c line 2

Quoted 4 lines · Andy Gross

+++ b/drivers/dma/qcom_adm.c

drivers/dma/qcom_adm.c new file +2 -0
@@ -0,0 +1,901 @@
+/*
+ * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.

shouldn't this be 15 :)

+/* ADM registers - calculated from channel number and security domain */

Quoted 7 lines
drivers/dma/qcom_adm.c new file +7 -0
+#define HI_CH_CMD_PTR(chan, ee)		(4*chan + 0x20800*ee)
+#define HI_CH_RSLT(chan, ee)		(0x40 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE0(chan, ee)	(0x80 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE1(chan, ee)	(0xc0 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE2(chan, ee)	(0x100 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE3(chan, ee)	(0x140 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE4(chan, ee)	(0x180 + 4*chan + 0x20800*ee)

drivers/dma/qcom_adm.c line 53

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+#define HI_CH_FLUSH_STATE5(chan, ee)	(0x1c0 + 4*chan + 0x20800*ee)
+#define HI_CH_STATUS_SD(chan, ee)	(0x200 + 4*chan + 0x20800*ee)
+#define HI_CH_CONF(chan)		(0x240 + 4*chan)
+#define HI_CH_RSLT_CONF(chan, ee)	(0x300 + 4*chan + 0x20800*ee)
+#define HI_SEC_DOMAIN_IRQ_STATUS(ee)	(0x380 + 0x20800*ee)
+#define HI_CI_CONF(ci)			(0x390 + 4*ci)
+#define HI_CRCI_CONF0			0x3d0
+#define HI_CRCI_CONF1			0x3d4
+#define HI_GP_CTL			0x3d8
+#define HI_CRCI_CTL(crci, ee)		(0x400 + 0x4*crci + 0x20800*ee)

two things, one the names are quite generic and may cause conflicts so pls fix that. Second the values, what is the deal with 4*chan, should that be a define as well. Also rather than copy pasting a macros would be better for this expansion

Quoted 20 lines
drivers/dma/qcom_adm.c new file +20 -0
+
+/* channel status */
+#define CH_STATUS_VALID	BIT(1)
+
+/* channel result */
+#define CH_RSLT_VALID	BIT(31)
+#define CH_RSLT_ERR	BIT(3)
+#define CH_RSLT_FLUSH	BIT(2)
+#define CH_RSLT_TPD	BIT(1)
+
+/* channel conf */
+#define CH_CONF_MPU_DISABLE	BIT(11)
+#define CH_CONF_PERM_MPU_CONF	BIT(9)
+#define CH_CONF_FLUSH_RSLT_EN	BIT(8)
+#define CH_CONF_FORCE_RSLT_EN	BIT(7)
+#define CH_CONF_IRQ_EN		BIT(6)
+
+/* channel result conf */
+#define CH_RSLT_CONF_FLUSH_EN	BIT(1)
+#define CH_RSLT_CONF_IRQ_EN	BIT(0)

drivers/dma/qcom_adm.c line 83

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+
+/* CRCI CTL */
+#define CRCI_CTL_MUX_SEL	BIT(18)
+#define CRCI_CTL_RST		BIT(17)
+
+/* CI configuration */
+#define CI_RANGE_END(x)		(x << 24)
+#define CI_RANGE_START(x)	(x << 16)
+#define CI_BURST_4_WORDS	0x4
+#define CI_BURST_8_WORDS	0x8

shouldn't you be consistent is usage of BIT()

Quoted 6 lines
drivers/dma/qcom_adm.c new file +6 -0
+
+/* GP CTL */
+#define GP_CTL_LP_EN		BIT(12)
+#define GP_CTL_LP_CNT(x)	(x << 8)
+
+/* Command pointer list entry */

drivers/dma/qcom_adm.c

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+#define CPLE_LP			BIT(31)
+#define CPLE_CMD_PTR_LIST	BIT(29)
+
+/* Command list entry */
+#define CMD_LC			BIT(31)
+#define CMD_DST_CRCI(n)		(((n) & 0xf) << 7)
+#define CMD_SRC_CRCI(n)		(((n) & 0xf) << 3)
+
+#define CMD_TYPE_SINGLE		0x0
+#define CMD_TYPE_BOX		0x3a

naming issues...

drivers/dma/qcom_adm.c line 185

Quoted 4 lines
drivers/dma/qcom_adm.c new file +4 -0
+static int adm_alloc_chan(struct dma_chan *chan)
+{
+	return 0;
+}

This is no longer mandatory, so can be dropped

Quoted 13 lines
drivers/dma/qcom_adm.c new file +13 -0
+static int adm_get_blksize(unsigned int burst)
+{
+	int ret;
+
+	switch (burst) {
+	case 16:
+		ret = 0;
+		break;
+	case 32:
+		ret = 1;
+		break;
+	case 64:
+		ret = 2;

drivers/dma/qcom_adm.c line 225

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+		break;
+	case 128:
+		ret = 3;
+		break;
+	case 192:
+		ret = 4;
+		break;
+	case 256:
+		ret = 5;
+		break;

ffs(burst>>4) ?

Quoted 25 lines
drivers/dma/qcom_adm.c new file +25 -0
+static struct dma_async_tx_descriptor *adm_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 adm_chan *achan = to_adm_chan(chan);
+	struct adm_device *adev = achan->adev;
+	struct adm_async_desc *async_desc;
+	struct scatterlist *sg;
+	u32 i;
+	u32 single_count = 0, box_count = 0, desc_offset = 0, crci = 0;
+	struct adm_desc_hw_box *box_desc;
+	struct adm_desc_hw_single *single_desc;
+	void *desc;
+	u32 *cple, *last_cmd;
+	u32 burst;
+	int blk_size = 0;
+
+
+	if (!is_slave_direction(direction)) {
+		dev_err(adev->dev, "invalid dma direction\n");
+		return NULL;
+	}
+
+	/*

drivers/dma/qcom_adm.c line 278

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+	 * get burst value from slave configuration
+	 * If zero, default to maximum burst size
+	 * If larger than the max transfer size, set to ADM_MAX_XFER
+	 */
+	burst = (direction == DMA_MEM_TO_DEV) ?
+		achan->slave.dst_maxburst :
+		achan->slave.src_maxburst;
+
+	if (!burst || burst > ADM_MAX_XFER)
+		burst = ADM_MAX_XFER;

For slave, we should send error here. The DMA settings are matched with slave FIFO, so any change here can cause issues

Quoted 103 lines
drivers/dma/qcom_adm.c new file +103 -0
+
+	/* if using flow control, validate burst and crci values */
+	if (achan->slave.device_fc) {
+
+		blk_size = adm_get_blksize(burst);
+		if (blk_size < 0) {
+			dev_err(adev->dev, "invalid burst value w/ crci: %d\n",
+				burst);
+			return ERR_PTR(-EINVAL);
+		}
+
+		crci = achan->slave.slave_id & 0xf;
+		if (!crci || achan->slave.slave_id > 0x1f) {
+			dev_err(adev->dev, "invalid crci value\n");
+			return ERR_PTR(-EINVAL);
+		}
+	}
+
+	/* iterate through sgs and compute allocation size of structures */
+	for_each_sg(sgl, sg, sg_len, i)
+		if (sg_dma_len(sg) % burst)
+			single_count += DIV_ROUND_UP(sg_dma_len(sg), burst);
+		else
+			box_count += DIV_ROUND_UP(sg_dma_len(sg) / burst,
+							ADM_MAX_ROWS);
+
+	async_desc = kzalloc(sizeof(*async_desc), GFP_NOWAIT);
+	if (!async_desc)
+		return ERR_PTR(-ENOMEM);
+
+	if (crci)
+		async_desc->mux = achan->slave.slave_id >> 4 ?
+					CRCI_CTL_MUX_SEL : 0;
+	async_desc->crci = crci;
+	async_desc->blk_size = blk_size;
+	async_desc->dma_len = single_count * sizeof(*single_desc) +
+				box_count * sizeof(*box_desc) + sizeof(*cple) +
+				2*ADM_DESC_ALIGN;
+
+	async_desc->cpl = dma_alloc_writecombine(adev->dev, async_desc->dma_len,
+				&async_desc->dma_addr, GFP_NOWAIT);
+
+	if (!async_desc->cpl) {
+		kfree(async_desc);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	async_desc->adev = adev;
+
+	/* both command list entry and descriptors must be 8 byte aligned */
+	cple = PTR_ALIGN(async_desc->cpl, ADM_DESC_ALIGN);
+	desc = PTR_ALIGN(cple + 1, ADM_DESC_ALIGN);
+	last_cmd = desc;
+
+	/* init cmd list */
+	*cple = CPLE_LP;
+	*cple |= (desc - async_desc->cpl + async_desc->dma_addr) >> 3;
+
+	for_each_sg(sgl, sg, sg_len, i) {
+		unsigned int xfer_len, row_len, rows;
+		unsigned int remainder = sg_dma_len(sg);
+		unsigned int offset = 0;
+
+		do {
+			/* use single if length is not multiple of burst */
+			if (remainder % burst) {
+				single_desc = desc + desc_offset;
+				last_cmd = &single_desc->cmd;
+				single_desc->cmd = CMD_TYPE_SINGLE;
+				xfer_len = (remainder > ADM_MAX_XFER) ?
+					ADM_MAX_XFER : remainder;
+				if (direction == DMA_DEV_TO_MEM) {
+					single_desc->dst_addr =
+						sg_dma_address(sg) + offset;
+					single_desc->src_addr =
+						achan->slave.src_addr;
+					single_desc->len = xfer_len;
+					single_desc->cmd |= CMD_SRC_CRCI(crci);
+				} else {
+					single_desc->src_addr =
+						sg_dma_address(sg) + offset;
+					single_desc->dst_addr =
+						achan->slave.dst_addr;
+					single_desc->len = xfer_len;
+					single_desc->cmd |= CMD_DST_CRCI(crci);
+				}
+
+				remainder -= xfer_len;
+				async_desc->length += xfer_len;
+				offset += xfer_len;
+				desc_offset += sizeof(*single_desc);
+			} else {
+				box_desc = desc + desc_offset;
+				last_cmd = &box_desc->cmd;
+				box_desc->cmd = CMD_TYPE_BOX;
+				box_desc->row_offset = 0;
+
+				if (direction == DMA_DEV_TO_MEM) {
+					box_desc->dst_addr =
+						sg_dma_address(sg) + offset;
+					box_desc->src_addr =
+						achan->slave.src_addr;
+					box_desc->cmd |= CMD_SRC_CRCI(crci);

drivers/dma/qcom_adm.c line 391

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+					box_desc->row_offset = burst;
+
+				} else {
+					box_desc->src_addr =
+						sg_dma_address(sg) + offset;
+					box_desc->dst_addr =
+						achan->slave.dst_addr;
+					box_desc->cmd |= CMD_DST_CRCI(crci);
+					box_desc->row_offset = burst << 16;
+				}

till this both look quite similar, also you are way deeply nested. I would split this up to make it look better as well try reusing common parts for both here

drivers/dma/qcom_adm.c line 449

Quoted 3 lines
drivers/dma/qcom_adm.c new file +3 -0
+static int adm_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+	unsigned long arg)
+{

this is removed, so you need to rebase this

Thanks

Signature · 1 line
~Vinod
author
#4 lore raw

Re: [Patch v4 2/2] dmaengine: Add ADM driver

Quoted 6 lines · Vinod Koul

drivers/dma/qcom_adm.c line 2

Quoted 4 lines · Andy Gross

+++ b/drivers/dma/qcom_adm.c

drivers/dma/qcom_adm.c new file +2 -0
@@ -0,0 +1,901 @@
+/*
+ * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.

shouldn't this be 15 :)

yeah, need to update.

Quoted 15 lines

+/* ADM registers - calculated from channel number and security domain */

Quoted 13 lines
drivers/dma/qcom_adm.c new file +13 -0
+#define HI_CH_CMD_PTR(chan, ee)		(4*chan + 0x20800*ee)
+#define HI_CH_RSLT(chan, ee)		(0x40 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE0(chan, ee)	(0x80 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE1(chan, ee)	(0xc0 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE2(chan, ee)	(0x100 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE3(chan, ee)	(0x140 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE4(chan, ee)	(0x180 + 4*chan + 0x20800*ee)
+#define HI_CH_FLUSH_STATE5(chan, ee)	(0x1c0 + 4*chan + 0x20800*ee)
+#define HI_CH_STATUS_SD(chan, ee)	(0x200 + 4*chan + 0x20800*ee)
+#define HI_CH_CONF(chan)		(0x240 + 4*chan)
+#define HI_CH_RSLT_CONF(chan, ee)	(0x300 + 4*chan + 0x20800*ee)
+#define HI_SEC_DOMAIN_IRQ_STATUS(ee)	(0x380 + 0x20800*ee)
+#define HI_CI_CONF(ci)			(0x390 + 4*ci)
Quoted 8 lines

drivers/dma/qcom_adm.c line 53

Quoted 4 lines
drivers/dma/qcom_adm.c new file +4 -0
+#define HI_CRCI_CONF0			0x3d0
+#define HI_CRCI_CONF1			0x3d4
+#define HI_GP_CTL			0x3d8
+#define HI_CRCI_CTL(crci, ee)		(0x400 + 0x4*crci + 0x20800*ee)

two things, one the names are quite generic and may cause conflicts so pls fix that. Second the values, what is the deal with 4*chan, should that be a define as well. Also rather than copy pasting a macros would be better for this expansion

Yeah there are sets of registers that have both channel and execution environment offsets. It is messy. I'll try to make it more sane.

Quoted 24 lines
Quoted 24 lines
drivers/dma/qcom_adm.c new file +23 -0
+
+/* channel status */
+#define CH_STATUS_VALID	BIT(1)
+
+/* channel result */
+#define CH_RSLT_VALID	BIT(31)
+#define CH_RSLT_ERR	BIT(3)
+#define CH_RSLT_FLUSH	BIT(2)
+#define CH_RSLT_TPD	BIT(1)
+
+/* channel conf */
+#define CH_CONF_MPU_DISABLE	BIT(11)
+#define CH_CONF_PERM_MPU_CONF	BIT(9)
+#define CH_CONF_FLUSH_RSLT_EN	BIT(8)
+#define CH_CONF_FORCE_RSLT_EN	BIT(7)
+#define CH_CONF_IRQ_EN		BIT(6)
+
+/* channel result conf */
+#define CH_RSLT_CONF_FLUSH_EN	BIT(1)
+#define CH_RSLT_CONF_IRQ_EN	BIT(0)
+
+/* CRCI CTL */
+#define CRCI_CTL_MUX_SEL	BIT(18)
Quoted 8 lines

drivers/dma/qcom_adm.c line 83

Quoted 7 lines
drivers/dma/qcom_adm.c new file +7 -0
+#define CRCI_CTL_RST		BIT(17)
+
+/* CI configuration */
+#define CI_RANGE_END(x)		(x << 24)
+#define CI_RANGE_START(x)	(x << 16)
+#define CI_BURST_4_WORDS	0x4
+#define CI_BURST_8_WORDS	0x8

shouldn't you be consistent is usage of BIT()

good catch

Quoted 9 lines
Quoted 9 lines
drivers/dma/qcom_adm.c new file +9 -0
+
+/* GP CTL */
+#define GP_CTL_LP_EN		BIT(12)
+#define GP_CTL_LP_CNT(x)	(x << 8)
+
+/* Command pointer list entry */
+#define CPLE_LP			BIT(31)
+#define CPLE_CMD_PTR_LIST	BIT(29)
+
Quoted 8 lines

drivers/dma/qcom_adm.c

Quoted 7 lines
drivers/dma/qcom_adm.c new file +7 -0
+/* Command list entry */
+#define CMD_LC			BIT(31)
+#define CMD_DST_CRCI(n)		(((n) & 0xf) << 7)
+#define CMD_SRC_CRCI(n)		(((n) & 0xf) << 3)
+
+#define CMD_TYPE_SINGLE		0x0
+#define CMD_TYPE_BOX		0x3a

naming issues...

ok. will try to come up with something better

Quoted 5 lines

drivers/dma/qcom_adm.c line 185

Quoted 4 lines
drivers/dma/qcom_adm.c new file +4 -0
+static int adm_alloc_chan(struct dma_chan *chan)
+{
+	return 0;
+}

This is no longer mandatory, so can be dropped

will remove

Quoted 17 lines
Quoted 17 lines
drivers/dma/qcom_adm.c new file +16 -0
+static int adm_get_blksize(unsigned int burst)
+{
+	int ret;
+
+	switch (burst) {
+	case 16:
+		ret = 0;
+		break;
+	case 32:
+		ret = 1;
+		break;
+	case 64:
+		ret = 2;
+		break;
+	case 128:
+		ret = 3;
Quoted 8 lines

drivers/dma/qcom_adm.c line 225

Quoted 7 lines
drivers/dma/qcom_adm.c new file +7 -0
+		break;
+	case 192:
+		ret = 4;
+		break;
+	case 256:
+		ret = 5;
+		break;

ffs(burst>>4) ?

that should work nicely. thanks.

Quoted 29 lines
Quoted 29 lines
drivers/dma/qcom_adm.c new file +29 -0
+static struct dma_async_tx_descriptor *adm_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 adm_chan *achan = to_adm_chan(chan);
+	struct adm_device *adev = achan->adev;
+	struct adm_async_desc *async_desc;
+	struct scatterlist *sg;
+	u32 i;
+	u32 single_count = 0, box_count = 0, desc_offset = 0, crci = 0;
+	struct adm_desc_hw_box *box_desc;
+	struct adm_desc_hw_single *single_desc;
+	void *desc;
+	u32 *cple, *last_cmd;
+	u32 burst;
+	int blk_size = 0;
+
+
+	if (!is_slave_direction(direction)) {
+		dev_err(adev->dev, "invalid dma direction\n");
+		return NULL;
+	}
+
+	/*
+	 * get burst value from slave configuration
+	 * If zero, default to maximum burst size
+	 * If larger than the max transfer size, set to ADM_MAX_XFER
+	 */
Quoted 8 lines

drivers/dma/qcom_adm.c line 278

Quoted 6 lines
drivers/dma/qcom_adm.c new file +6 -0
+	burst = (direction == DMA_MEM_TO_DEV) ?
+		achan->slave.dst_maxburst :
+		achan->slave.src_maxburst;
+
+	if (!burst || burst > ADM_MAX_XFER)
+		burst = ADM_MAX_XFER;

For slave, we should send error here. The DMA settings are matched with slave FIFO, so any change here can cause issues

ok. I was trying to unify the flow control use case with the non flow control use case. the slave config burst is only used in the case of flow control.

Quoted 14 lines
Quoted 14 lines
drivers/dma/qcom_adm.c new file +13 -0
+
+	/* if using flow control, validate burst and crci values */
+	if (achan->slave.device_fc) {
+
+		blk_size = adm_get_blksize(burst);
+		if (blk_size < 0) {
+			dev_err(adev->dev, "invalid burst value w/ crci: %d\n",
+				burst);
+			return ERR_PTR(-EINVAL);
+		}
+
+		crci = achan->slave.slave_id & 0xf;
+		if (!crci || achan->slave.slave_id > 0x1f) {
Quoted 8 lines

drivers/dma/qcom_adm.c line 299

Quoted 8 lines
drivers/dma/qcom_adm.c new file +8 -0
+			dev_err(adev->dev, "invalid crci value\n");
+			return ERR_PTR(-EINVAL);
+		}
+	}
+
+	/* iterate through sgs and compute allocation size of structures */
+	for_each_sg(sgl, sg, sg_len, i)
+		if (sg_dma_len(sg) % burst)

i used it here to bust up the dma buffer into burstable chunks

Quoted 5 lines
Quoted 5 lines
drivers/dma/qcom_adm.c new file +5 -0
+			single_count += DIV_ROUND_UP(sg_dma_len(sg), burst);
+		else
+			box_count += DIV_ROUND_UP(sg_dma_len(sg) / burst,
+							ADM_MAX_ROWS);
+

<snip>

Quoted 17 lines
Quoted 17 lines
drivers/dma/qcom_adm.c new file +17 -0
+			} else {
+				box_desc = desc + desc_offset;
+				last_cmd = &box_desc->cmd;
+				box_desc->cmd = CMD_TYPE_BOX;
+				box_desc->row_offset = 0;
+
+				if (direction == DMA_DEV_TO_MEM) {
+					box_desc->dst_addr =
+						sg_dma_address(sg) + offset;
+					box_desc->src_addr =
+						achan->slave.src_addr;
+					box_desc->cmd |= CMD_SRC_CRCI(crci);
+					box_desc->row_offset = burst;
+
+				} else {
+					box_desc->src_addr =
+						sg_dma_address(sg) + offset;
Quoted 8 lines

drivers/dma/qcom_adm.c line 391

Quoted 5 lines
drivers/dma/qcom_adm.c new file +5 -0
+					box_desc->dst_addr =
+						achan->slave.dst_addr;
+					box_desc->cmd |= CMD_DST_CRCI(crci);
+					box_desc->row_offset = burst << 16;
+				}

till this both look quite similar, also you are way deeply nested. I would split this up to make it look better as well try reusing common parts for both here

Ok. I'll rework this.

Quoted 5 lines

drivers/dma/qcom_adm.c line 449

Quoted 4 lines
drivers/dma/qcom_adm.c new file +3 -0
+static int adm_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+	unsigned long arg)
+{

this is removed, so you need to rebase this

Will do.

Signature · 3 lines
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#5 lore raw

Re: [Patch v4 2/2] dmaengine: Add ADM driver

Hi,

<snip>

Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
drivers/dma/qcom_adm.c new file +16 -0
+static int adm_get_blksize(unsigned int burst)
+{
+	int ret;
+
+	switch (burst) {
+	case 16:
+		ret = 0;
+		break;
+	case 32:
+		ret = 1;
+		break;
+	case 64:
+		ret = 2;
+		break;
+	case 128:
+		ret = 3;
Quoted 8 lines

drivers/dma/qcom_adm.c line 225

Quoted 7 lines
drivers/dma/qcom_adm.c new file +7 -0
+		break;
+	case 192:
+		ret = 4;
+		break;
+	case 256:
+		ret = 5;
+		break;

ffs(burst>>4) ?

Quoted 2 lines

that should work nicely. thanks.

Will not work for 192, 256 ?

Regards,
Sricharan

author
#6 lore raw

Re: [Patch v4 2/2] dmaengine: Add ADM driver

Quoted 30 lines · sricharan at codeaurora.org

Hi,

<snip>

Quoted 27 lines
Quoted 26 lines
Quoted 15 lines
drivers/dma/qcom_adm.c new file +13 -0
+static int adm_get_blksize(unsigned int burst)
+{
+	int ret;
+
+	switch (burst) {
+	case 16:
+		ret = 0;
+		break;
+	case 32:
+		ret = 1;
+		break;
+	case 64:
+		ret = 2;

drivers/dma/qcom_adm.c line 225

Quoted 10 lines
drivers/dma/qcom_adm.c new file +10 -0
+		break;
+	case 128:
+		ret = 3;
+		break;
+	case 192:
+		ret = 4;
+		break;
+	case 256:
+		ret = 5;
+		break;

ffs(burst>>4) ?

Quoted 3 lines
Quoted 2 lines

that should work nicely. thanks.

Will not work for 192, 256 ?

you are right. I'll have to separate those out into 2 more cases. Good catch!

Signature · 3 lines
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project