All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND 0/2] Enable DMA driver for MIC X200 Coprocessor
@ 2016-02-04 15:10 Jacek Lawrynowicz
  2016-02-04 15:10 ` [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver Jacek Lawrynowicz
  2016-02-04 15:10 ` [PATCH RESEND 2/2] MAINTAINERS: update for mic Jacek Lawrynowicz
  0 siblings, 2 replies; 6+ messages in thread
From: Jacek Lawrynowicz @ 2016-02-04 15:10 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul
  Cc: dmaengine, linux-kernel, ashutosh.dixit, sudeep.dutt,
	andrzej.kacprowski, jacek.lawrynowicz

This is the first set of patches adding MIC X200 Coprocessor support.
MIC PCIe card has a DMA controller with 2 channels: one for the host
and the other one for the card. On host the channel is private
and used only by the host driver to transfer data by the MIC virtio 
and SCIF drivers. Please refer to Documentation/mic/mic_overview.txt 
for a description about other MIC drivers.

The patches have been compiled/validated against v4.5-rc2. Tested using
dmatest module with module parameter "threads_per_chan=60". These patches
have also been scanned by Fengguang Wu's 0-day infrastructure and no
issues have been reported.

In order for the device to function when IOMMU is enabled DMA alias quirk
has to be enabled and it is being upstreamed on linux-pci 
(http://www.spinics.net/lists/linux-pci/msg48460.html).

Resending this patch set with corrected recipient list.

Jacek Lawrynowicz (2):
  dmaengine: added MIC X200 Coprocessor DMA driver
  MAINTAINERS: update for mic

 MAINTAINERS                |    2 +
 drivers/dma/Kconfig        |   18 +
 drivers/dma/Makefile       |    1 +
 drivers/dma/mic_x200_dma.c | 1114 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1135 insertions(+)
 create mode 100644 drivers/dma/mic_x200_dma.c

-- 
2.1.4

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

* [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver
  2016-02-04 15:10 [PATCH RESEND 0/2] Enable DMA driver for MIC X200 Coprocessor Jacek Lawrynowicz
@ 2016-02-04 15:10 ` Jacek Lawrynowicz
  2016-02-04 17:50   ` Andy Shevchenko
  2016-02-08  4:21   ` Vinod Koul
  2016-02-04 15:10 ` [PATCH RESEND 2/2] MAINTAINERS: update for mic Jacek Lawrynowicz
  1 sibling, 2 replies; 6+ messages in thread
From: Jacek Lawrynowicz @ 2016-02-04 15:10 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul
  Cc: dmaengine, linux-kernel, ashutosh.dixit, sudeep.dutt,
	andrzej.kacprowski, jacek.lawrynowicz

This patch implements the DMA Engine driver for the DMA controller on
MIC X200 Coprocessors which are PCIe cards running Linux. Separate but
identical DMA channels are available for the host and card. DMA
transfers can be done from both the host and card in host<->card and
host<->host directions.

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Signed-off-by: Sudeep Dutt <sudeep.dutt@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@intel.com>
---
 drivers/dma/Kconfig        |   18 +
 drivers/dma/Makefile       |    1 +
 drivers/dma/mic_x200_dma.c | 1114 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1133 insertions(+)
 create mode 100644 drivers/dma/mic_x200_dma.c

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 79b1390..a3ea8b9 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -277,6 +277,24 @@ config INTEL_MIC_X100_DMA
 	  OS and tools for MIC to use with this driver are available from
 	  <http://software.intel.com/en-us/mic-developer>.
 
+config INTEL_MIC_X200_DMA
+	tristate "Intel MIC X200 DMA Driver"
+        depends on 64BIT && X86 && PCI
+	select DMAENGINE
+	help
+	  This enables DMA support for the Intel Many Integrated Core
+	  (MIC) family of PCIe form factor coprocessor X200 devices that
+	  run a 64 bit Linux OS. This driver will be used by both MIC
+	  host and card drivers.
+
+	  If you are building host kernel with a MIC device or a card
+	  kernel for a MIC device, then say M (recommended) or Y, else
+	  say N. If unsure say N.
+
+	  More information about the Intel MIC family as well as the Linux
+	  OS and tools for MIC to use with this driver are available from
+	  <http://software.intel.com/en-us/mic-developer>.
+
 config K3_DMA
 	tristate "Hisilicon K3 DMA support"
 	depends on ARCH_HI3xxx
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 2dd0a067..a3756f5 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
 obj-$(CONFIG_INTEL_IOATDMA) += ioat/
 obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
 obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
+obj-$(CONFIG_INTEL_MIC_X200_DMA) += mic_x200_dma.o
 obj-$(CONFIG_K3_DMA) += k3dma.o
 obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
 obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
diff --git a/drivers/dma/mic_x200_dma.c b/drivers/dma/mic_x200_dma.c
new file mode 100644
index 0000000..80c4959
--- /dev/null
+++ b/drivers/dma/mic_x200_dma.c
@@ -0,0 +1,1114 @@
+/*
+ * Intel MIC Platform Software Stack (MPSS)
+ *
+ * Copyright(c) 2016 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Intel MIC X200 DMA driver
+ */
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/interrupt.h>
+#include <linux/vmalloc.h>
+
+#include "dmaengine.h"
+
+#define MIC_DMA_DRV_NAME	"mic_x200_dma"
+
+#define MIC_DMA_DESC_RX_SIZE	(128 * 1024)
+#define MIC_DMA_ABORT_TO_MS	3000
+#define MIC_DMA_RING_TO_MS	20
+#define MIC_DMA_PENDING_LEVEL	16
+#define MIC_DMA_ABORT_LIMIT	5
+
+/* DMA Descriptor related flags */
+#define MIC_DMA_DESC_VALID		BIT(31)
+#define MIC_DMA_DESC_INTR_ENABLE	BIT(30)
+#define MIC_DMA_DESC_SRC_LINK_ERR	BIT(29)
+#define MIC_DMA_DESC_DST_LINK_ERR	BIT(28)
+#define MIC_DMA_DESC_LOW_MASK		(0xffffffffUL)
+#define MIC_DMA_DESC_SRC_LOW_SHIFT	32
+#define MIC_DMA_DESC_BITS_32_TO_47	(0xffffUL << 32)
+#define MIC_DMA_DESC_SIZE_MASK		((1UL << 27) - 1)
+
+#define MIC_DMA_DESC_RING_ADDR_LOW		0x214
+#define MIC_DMA_DESC_RING_ADDR_HIGH		0x218
+#define MIC_DMA_NEXT_DESC_ADDR_LOW		0x21C
+#define MIC_DMA_DESC_RING_SIZE			0x220
+#define MIC_DMA_LAST_DESC_ADDR_LOW		0x224
+#define MIC_DMA_LAST_DESC_XFER_SIZE		0x228
+
+#define MIC_DMA_CTRL_STATUS			0x238
+#define MIC_DMA_CTRL_PAUSE			BIT(0)
+#define MIC_DMA_CTRL_ABORT			BIT(1)
+#define MIC_DMA_CTRL_WB_ENABLE			BIT(2)
+#define MIC_DMA_CTRL_START			BIT(3)
+#define MIC_DMA_CTRL_RING_STOP			BIT(4)
+#define MIC_DMA_CTRL_ONCHIP_MODE		BIT(5)
+#define MIC_DMA_CTRL_OFFCHIP_MODE		(2UL << 5)
+#define MIC_DMA_CTRL_DESC_INVLD_STATUS		BIT(8)
+#define MIC_DMA_CTRL_PAUSE_DONE_STATUS		BIT(9)
+#define MIC_DMA_CTRL_ABORT_DONE_STATUS		BIT(10)
+#define MIC_DMA_CTRL_FACTORY_TEST		BIT(11)
+#define MIC_DMA_CTRL_IMM_PAUSE_DONE_STATUS	BIT(12)
+#define MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE	(3UL << 16)
+#define MIC_DMA_CTRL_RELAXED_DATA_WRITE		BIT(25)
+#define MIC_DMA_CTRL_NO_SNOOP_DESC_READ		BIT(26)
+#define MIC_DMA_CTRL_NO_SNOOP_DATA_READ		BIT(27)
+#define MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE	BIT(28)
+#define MIC_DMA_CTRL_IN_PROGRESS		BIT(30)
+#define MIC_DMA_CTRL_HEADER_LOG			BIT(31)
+#define MIC_DMA_CTRL_MODE_BITS			(3UL << 5)
+#define MIC_DMA_CTRL_STATUS_BITS		(0x1FUL << 8)
+#define MIC_DMA_CTRL_STATUS_INIT  (MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE |	\
+				   MIC_DMA_CTRL_RELAXED_DATA_WRITE |	\
+				   MIC_DMA_CTRL_NO_SNOOP_DESC_READ |	\
+				   MIC_DMA_CTRL_NO_SNOOP_DATA_READ |	\
+				   MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE)
+
+#define MIC_DMA_INTR_CTRL_STATUS		0x23C
+#define MIC_DMA_ERROR_INTR_EN			BIT(0)
+#define MIC_DMA_INVLD_DESC_INTR_EN		BIT(1)
+#define MIC_DMA_ABORT_DONE_INTR_EN		BIT(3)
+#define MIC_DMA_GRACE_PAUSE_INTR_EN		BIT(4)
+#define MIC_DMA_IMM_PAUSE_INTR_EN		BIT(5)
+#define MIC_DMA_IRQ_PIN_INTR_EN			BIT(15)
+#define MIC_DMA_ERROR_INTR_STATUS		BIT(16)
+#define MIC_DMA_INVLD_DESC_INTR_STATUS		BIT(17)
+#define MIC_DMA_DESC_DONE_INTR_STATUS		BIT(18)
+#define MIC_DMA_ABORT_DONE_INTR_STATUS		BIT(19)
+#define MIC_DMA_GRACE_PAUSE_INTR_STATUS		BIT(20)
+#define MIC_DMA_IMM_PAUSE_INTR_STATUS		BIT(21)
+#define MIC_DMA_ALL_INTR_EN	(MIC_DMA_ERROR_INTR_EN |		\
+				 MIC_DMA_INVLD_DESC_INTR_EN |		\
+				 MIC_DMA_ABORT_DONE_INTR_EN |		\
+				 MIC_DMA_GRACE_PAUSE_INTR_EN |		\
+				 MIC_DMA_IMM_PAUSE_INTR_EN)
+
+#define MIC_DMA_ERROR_STATUS	(MIC_DMA_ERROR_INTR_STATUS |		\
+				 MIC_DMA_ABORT_DONE_INTR_STATUS |	\
+				 MIC_DMA_GRACE_PAUSE_INTR_STATUS |	\
+				 MIC_DMA_IMM_PAUSE_INTR_STATUS)
+
+#define MIC_DMA_ALL_INTR_STATUS	(MIC_DMA_ERROR_INTR_STATUS |		\
+				 MIC_DMA_INVLD_DESC_INTR_STATUS |	\
+				 MIC_DMA_DESC_DONE_INTR_STATUS |	\
+				 MIC_DMA_ABORT_DONE_INTR_STATUS |	\
+				 MIC_DMA_GRACE_PAUSE_INTR_STATUS |	\
+				 MIC_DMA_IMM_PAUSE_INTR_STATUS)
+/*
+ * Performance tuning registers. For explanation of the values
+ * used please refer to mic x200 dma hardware specification.
+ */
+#define MIC_DMA_CHAN_BANDWIDTH			0x22C
+#define MIC_DMA_MAX_DST_MEM_WRITE		(2 << 24)
+
+#define MIC_DMA_STA_RAM_THRESH			0x258
+#define MIC_DMA_HDR_RAM_USAGE_THRESH		46UL
+#define MIC_DMA_PLD_RAM_USAGE_THRESH		(61UL << 16)
+#define MIC_DMA_STA_RAM_THRESH_INIT (MIC_DMA_HDR_RAM_USAGE_THRESH |	\
+				     MIC_DMA_PLD_RAM_USAGE_THRESH)
+
+#define MIC_DMA_STA0_HDR_RAM			0x25C
+#define MIC_DMA_STA1_HDR_RAM			0x2A4
+#define MIC_DMA_HDR_RAM_UPPER_THRESH		160UL
+#define MIC_DMA_HDR_RAM_LOWER_THRESH		(152UL << 16)
+#define MIC_DMA_STA_HDR_THRESH_INIT (MIC_DMA_HDR_RAM_UPPER_THRESH |	\
+				     MIC_DMA_HDR_RAM_LOWER_THRESH)
+
+#define MIC_DMA_STA0_PLD_RAM			0x260
+#define MIC_DMA_STA1_PLD_RAM			0x2A8
+#define MIC_DMA_PLD_RAM_UPPER_THRESH		256UL
+#define MIC_DMA_PLD_RAM_LOWER_THRESH		(248UL << 16)
+#define MIC_DMA_STA_PLD_THRESH_INIT (MIC_DMA_PLD_RAM_UPPER_THRESH |	\
+				     MIC_DMA_PLD_RAM_LOWER_THRESH)
+
+/* HW DMA descriptor */
+struct mic_dma_desc {
+	u64 qw0;
+	u64 qw1;
+};
+
+/*
+ * mic_dma_chan - MIC X200 DMA channel specific data structures
+ *
+ * @ch_num: channel number
+ * @last_tail: cached value of descriptor ring tail
+ * @head: index of next descriptor in desc_ring
+ * @chan: dma engine api channel
+ * @desc_ring: dma descriptor ring
+ * @desc_ring_da: DMA address of desc_ring
+ * @tx_array: array of async_tx
+ * @cleanup_lock: used to synchronize descriptor cleanup
+ * @prep_lock: lock held in prep_memcpy & released in tx_submit
+ * @issue_lock: lock used to synchronize writes to head
+ * @notify_hw: flag used to notify HW that new descriptors are available
+ * @abort_tail: descriptor ring position at which last abort occurred
+ * @abort_counter: number of aborts at the last position in desc ring
+ */
+struct mic_dma_chan {
+	int ch_num;
+	u32 last_tail;
+	u32 head;
+	struct dma_chan chan;
+	struct mic_dma_desc *desc_ring;
+	dma_addr_t desc_ring_da;
+	struct dma_async_tx_descriptor *tx_array;
+	spinlock_t cleanup_lock;
+	spinlock_t prep_lock;
+	spinlock_t issue_lock;
+	bool notify_hw;
+	u32 abort_tail;
+	u32 abort_counter;
+};
+
+/*
+ * mic_dma_device - Per MIC X200 DMA device driver specific data structures
+ *
+ * @pdev: PCIe device
+ * @dma_dev: underlying dma device
+ * @reg_base: virtual address of the mmio space
+ * @mic_chan: Array of MIC X200 DMA channels
+ * @max_xfer_size: maximum transfer size per dma descriptor
+ */
+struct mic_dma_device {
+	struct pci_dev *pdev;
+	struct dma_device dma_dev;
+	void __iomem *reg_base;
+	struct mic_dma_chan mic_chan;
+	size_t max_xfer_size;
+};
+
+static inline struct mic_dma_device *to_mic_dma_dev(struct mic_dma_chan *ch)
+{
+	return
+	container_of((const typeof(((struct mic_dma_device *)0)->mic_chan) *)
+			 (ch - ch->ch_num), struct mic_dma_device, mic_chan);
+}
+
+static inline struct mic_dma_chan *to_mic_dma_chan(struct dma_chan *ch)
+{
+	return container_of(ch, struct mic_dma_chan, chan);
+}
+
+static inline struct device *mic_dma_ch_to_device(struct mic_dma_chan *ch)
+{
+	return to_mic_dma_dev(ch)->dma_dev.dev;
+}
+
+static inline u32 mic_dma_reg_read(struct mic_dma_device *pdma, u32 offset)
+{
+	return ioread32(pdma->reg_base + offset);
+}
+
+static inline void mic_dma_reg_write(struct mic_dma_device *pdma,
+				     u32 offset, u32 value)
+{
+	iowrite32(value, pdma->reg_base + offset);
+}
+
+static inline u32 mic_dma_ch_reg_read(struct mic_dma_chan *ch, u32 offset)
+{
+	return mic_dma_reg_read(to_mic_dma_dev(ch), offset);
+}
+
+static inline void mic_dma_ch_reg_write(struct mic_dma_chan *ch,
+					u32 offset, u32 value)
+{
+	mic_dma_reg_write(to_mic_dma_dev(ch), offset, value);
+}
+
+static inline u32 mic_dma_ring_inc(u32 val)
+{
+	return (val + 1) & (MIC_DMA_DESC_RX_SIZE - 1);
+}
+
+static inline u32 mic_dma_ring_dec(u32 val)
+{
+	return val ? val - 1 : MIC_DMA_DESC_RX_SIZE - 1;
+}
+
+static inline void mic_dma_inc_head(struct mic_dma_chan *ch)
+{
+	ch->head = mic_dma_ring_inc(ch->head);
+}
+
+static int mic_dma_alloc_desc_ring(struct mic_dma_chan *ch)
+{
+	/* Desc size must be >= depth of ring + prefetch size + 1 */
+	u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
+	struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
+
+	ch->desc_ring = dma_alloc_coherent(dev, desc_ring_size,
+					   &ch->desc_ring_da,
+					   GFP_KERNEL | __GFP_ZERO);
+	if (!ch->desc_ring)
+		return -ENOMEM;
+
+	dev_dbg(dev, "DMA desc ring VA = %p DA 0x%llx size 0x%llx\n",
+		ch->desc_ring, ch->desc_ring_da, desc_ring_size);
+	ch->tx_array = vzalloc(MIC_DMA_DESC_RX_SIZE * sizeof(*ch->tx_array));
+	if (!ch->tx_array)
+		goto tx_error;
+	return 0;
+tx_error:
+	dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
+	return -ENOMEM;
+}
+
+static inline void mic_dma_disable_chan(struct mic_dma_chan *ch)
+{
+	struct device *dev = mic_dma_ch_to_device(ch);
+	u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
+	int i;
+
+	/* set abort bit and clear start bit */
+	ctrl_reg |= MIC_DMA_CTRL_ABORT;
+	ctrl_reg &= ~MIC_DMA_CTRL_START;
+	mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
+
+	for (i = 0; i < MIC_DMA_ABORT_TO_MS; i++) {
+		ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
+
+		if (ctrl_reg & MIC_DMA_CTRL_ABORT_DONE_STATUS)
+			return;
+
+		mdelay(1);
+	}
+	dev_err(dev, "%s abort timed out 0x%x head %d tail %d\n",
+		__func__, ctrl_reg, ch->head, ch->last_tail);
+}
+
+static inline void mic_dma_enable_chan(struct mic_dma_chan *ch)
+{
+	u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
+
+	ctrl_reg |= MIC_DMA_CTRL_WB_ENABLE;
+	/* Clear any active status bits ([31,12:8]) */
+	ctrl_reg |= MIC_DMA_CTRL_HEADER_LOG | MIC_DMA_CTRL_STATUS_BITS;
+	ctrl_reg &= ~MIC_DMA_CTRL_MODE_BITS;
+	/* Enable SGL off-chip mode */
+	ctrl_reg |= MIC_DMA_CTRL_OFFCHIP_MODE;
+	/* Set start and clear abort and abort done bits */
+	ctrl_reg |= MIC_DMA_CTRL_START;
+	ctrl_reg &= ~MIC_DMA_CTRL_ABORT;
+	mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
+}
+
+static inline void mic_dma_chan_mask_intr(struct mic_dma_chan *ch)
+{
+	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
+
+	intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
+	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
+}
+
+static inline void mic_dma_chan_unmask_intr(struct mic_dma_chan *ch)
+{
+	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
+
+	intr_reg |= (MIC_DMA_ALL_INTR_EN);
+	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
+}
+
+static void mic_dma_chan_set_desc_ring(struct mic_dma_chan *ch)
+{
+	/* Set up the descriptor ring base address and size */
+	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_LOW,
+			     ch->desc_ring_da & 0xffffffff);
+	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_HIGH,
+			     (ch->desc_ring_da >> 32) & 0xffffffff);
+	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_SIZE, MIC_DMA_DESC_RX_SIZE);
+	/* Set up the next descriptor to the base of the descriptor ring */
+	mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW,
+			     ch->desc_ring_da & 0xffffffff);
+}
+
+static void mic_dma_cleanup(struct mic_dma_chan *ch)
+{
+	struct dma_async_tx_descriptor *tx;
+	u32 last_tail;
+	u32 cached_head;
+
+	spin_lock(&ch->cleanup_lock);
+	/*
+	 * Use a cached head rather than using ch->head directly, since a
+	 * different thread can be updating ch->head potentially leading to an
+	 * infinite loop below.
+	 */
+	cached_head = ACCESS_ONCE(ch->head);
+	/*
+	 * This is the barrier pair for smp_wmb() in fn.
+	 * mic_dma_tx_submit_unlock. It's required so that we read the
+	 * updated cookie value from tx->cookie.
+	 */
+	smp_rmb();
+
+	for (last_tail = ch->last_tail; last_tail != cached_head;) {
+		/* Break out of the loop if this desc is not yet complete */
+		if (ch->desc_ring[last_tail].qw0 & MIC_DMA_DESC_VALID)
+			break;
+
+		tx = &ch->tx_array[last_tail];
+		if (tx->cookie) {
+			dma_cookie_complete(tx);
+			if (tx->callback) {
+				tx->callback(tx->callback_param);
+				tx->callback = NULL;
+			}
+		}
+		last_tail = mic_dma_ring_inc(last_tail);
+	}
+	/* finish all completion callbacks before incrementing tail */
+	smp_mb();
+	ch->last_tail = last_tail;
+	spin_unlock(&ch->cleanup_lock);
+}
+
+static int mic_dma_chan_setup(struct mic_dma_chan *ch)
+{
+	mic_dma_chan_set_desc_ring(ch);
+	ch->last_tail = 0;
+	ch->head = 0;
+	ch->notify_hw = 1;
+	mic_dma_chan_unmask_intr(ch);
+	mic_dma_enable_chan(ch);
+	return 0;
+}
+
+static void mic_dma_free_desc_ring(struct mic_dma_chan *ch)
+{
+	u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
+	struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
+
+	vfree(ch->tx_array);
+	dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
+	ch->desc_ring = NULL;
+	ch->desc_ring_da = 0;
+
+	/* Reset description ring registers */
+	mic_dma_chan_set_desc_ring(ch);
+}
+
+static int mic_dma_chan_init(struct mic_dma_chan *ch)
+{
+	struct device *dev = mic_dma_ch_to_device(ch);
+	int rc;
+
+	rc = mic_dma_alloc_desc_ring(ch);
+	if (rc)
+		goto ring_error;
+	rc = mic_dma_chan_setup(ch);
+	if (rc)
+		goto chan_error;
+	return rc;
+chan_error:
+	mic_dma_free_desc_ring(ch);
+ring_error:
+	dev_err(dev, "%s ret %d\n", __func__, rc);
+	return rc;
+}
+
+static int mic_dma_alloc_chan_resources(struct dma_chan *ch)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+	struct device *dev = mic_dma_ch_to_device(mic_ch);
+	int rc = mic_dma_chan_init(mic_ch);
+
+	if (rc) {
+		dev_err(dev, "%s ret %d\n", __func__, rc);
+		return rc;
+	}
+	return MIC_DMA_DESC_RX_SIZE;
+}
+
+static inline u32 desc_ring_da_to_index(struct mic_dma_chan *ch, u32 descr)
+{
+	dma_addr_t last_dma_off = descr - (ch->desc_ring_da & ULONG_MAX);
+
+	return (last_dma_off / sizeof(struct mic_dma_desc)) &
+		(MIC_DMA_DESC_RX_SIZE - 1);
+}
+
+static inline u32 desc_ring_index_to_da(struct mic_dma_chan *ch, u32 index)
+{
+	dma_addr_t addr = ch->desc_ring_da +
+			sizeof(struct mic_dma_desc) * index;
+
+	return (addr & ULONG_MAX);
+}
+
+static void mic_dma_free_chan_resources(struct dma_chan *ch)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+
+	mic_dma_disable_chan(mic_ch);
+	mic_dma_chan_mask_intr(mic_ch);
+	mic_dma_cleanup(mic_ch);
+	mic_dma_free_desc_ring(mic_ch);
+}
+
+static enum dma_status
+mic_dma_tx_status(struct dma_chan *ch, dma_cookie_t cookie,
+		  struct dma_tx_state *txstate)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+
+	if (dma_cookie_status(ch, cookie, txstate) != DMA_COMPLETE)
+		mic_dma_cleanup(mic_ch);
+
+	return dma_cookie_status(ch, cookie, txstate);
+}
+
+static int mic_dma_ring_count(u32 head, u32 tail)
+{
+	int count;
+
+	if (head >= tail)
+		count = (tail - 0) + (MIC_DMA_DESC_RX_SIZE - head);
+	else
+		count = tail - head;
+	return count - 1;
+}
+
+static int mic_dma_avail_desc_ring_space(struct mic_dma_chan *ch, int required)
+{
+	struct device *dev = mic_dma_ch_to_device(ch);
+	int count;
+	unsigned long timeout = jiffies +
+			msecs_to_jiffies(MIC_DMA_RING_TO_MS);
+
+	count = mic_dma_ring_count(ch->head, ch->last_tail);
+	while (count < required) {
+		mic_dma_cleanup(ch);
+		count = mic_dma_ring_count(ch->head, ch->last_tail);
+		if ((count >= required) || time_after_eq(jiffies, timeout))
+			break;
+		usleep_range(50, 100);
+	}
+
+	if (count < required) {
+		dev_err(dev, "%s count %d reqd %d head %d tail %d\n",
+			__func__, count, required, ch->head, ch->last_tail);
+		return -ENOMEM;
+	}
+	return count;
+}
+
+static void
+mic_dma_memcpy_desc(struct mic_dma_desc *desc, dma_addr_t src_phys,
+		    dma_addr_t dst_phys, u64 size, int flags)
+{
+	u64 qw0, qw1;
+
+	qw0 = (src_phys & MIC_DMA_DESC_BITS_32_TO_47) << 16;
+	qw0 |= (dst_phys & MIC_DMA_DESC_BITS_32_TO_47);
+	qw0 |= size & MIC_DMA_DESC_SIZE_MASK;
+	qw0 |=  MIC_DMA_DESC_VALID;
+	if (flags & DMA_PREP_INTERRUPT)
+		qw0 |=  MIC_DMA_DESC_INTR_ENABLE;
+
+	qw1 = (src_phys & MIC_DMA_DESC_LOW_MASK) << MIC_DMA_DESC_SRC_LOW_SHIFT;
+	qw1 |= dst_phys & MIC_DMA_DESC_LOW_MASK;
+
+	desc->qw1 = qw1;
+	/*
+	 * Update QW1 before QW0 since QW0 has the valid bit and the
+	 * descriptor might be picked up by the hardware DMA engine
+	 * immediately if it finds a valid descriptor.
+	 */
+	wmb();
+	desc->qw0 = qw0;
+	/*
+	 * This is not smp_wmb() on purpose since we are also publishing the
+	 * descriptor updates to a dma device
+	 */
+	wmb();
+}
+
+static void mic_dma_fence_intr_desc(struct mic_dma_chan *ch, int flags)
+{
+	/*
+	* Zero-length DMA memcpy needs valid source and destination addresses
+	*/
+	mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
+			    ch->desc_ring_da, ch->desc_ring_da, 0, flags);
+	mic_dma_inc_head(ch);
+}
+
+static int mic_dma_prog_memcpy_desc(struct mic_dma_chan *ch, dma_addr_t src,
+				    dma_addr_t dst, size_t len, int flags)
+{
+	struct device *dev = mic_dma_ch_to_device(ch);
+	size_t current_transfer_len;
+	size_t max_xfer_size = to_mic_dma_dev(ch)->max_xfer_size;
+	int num_desc = len / max_xfer_size;
+	int ret;
+
+	if (len % max_xfer_size)
+		num_desc++;
+	/*
+	 * A single additional descriptor is programmed for fence/interrupt
+	 * during submit(..)
+	 */
+	if (flags)
+		num_desc++;
+
+	if (!num_desc) {
+		dev_err(dev, "%s nothing to do: rc %d\n", __func__, -EINVAL);
+		return -EINVAL;
+	}
+
+	ret = mic_dma_avail_desc_ring_space(ch, num_desc);
+	if (ret <= 0) {
+		dev_err(dev, "%s desc ring full ret %d\n", __func__, ret);
+		return ret;
+	}
+
+	while (len > 0) {
+		current_transfer_len = min(len, max_xfer_size);
+		/*
+		 * Set flags to 0 here, we don't want memcpy descriptors to
+		 * generate interrupts, a separate descriptor will be programmed
+		 * for fence/interrupt during submit, after a callback is
+		 * guaranteed to have been assigned
+		 */
+		mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
+				    src, dst, current_transfer_len, 0);
+		mic_dma_inc_head(ch);
+		len -= current_transfer_len;
+		dst = dst + current_transfer_len;
+		src = src + current_transfer_len;
+	}
+	return 0;
+}
+
+static int mic_dma_do_dma(struct mic_dma_chan *ch, int flags, dma_addr_t src,
+			  dma_addr_t dst, size_t len)
+{
+	return mic_dma_prog_memcpy_desc(ch, src, dst, len, flags);
+}
+
+static inline void mic_dma_issue_pending(struct dma_chan *ch)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+	unsigned long flags;
+	u32 ctrl_reg;
+
+	spin_lock_irqsave(&mic_ch->issue_lock, flags);
+	/*
+	 * The notify_hw is introduced because occasionally we were seeing an
+	 * issue where the HW tail seemed stuck, i.e. the DMA engine would not
+	 * pick up new descriptors available in the ring. We found that
+	 * serializing operations between SW and HW made this issue
+	 * disappear. We believe this issue arises because of a race between SW
+	 * and HW. SW adds new descriptors to the ring and clears
+	 * DESC_INVLD_STATUS to ask HW to pick them up. However HW has
+	 * prefetched descriptors and writes DESC_INVLD_STATUS to let SW know it
+	 * has fetched an invalid descriptor. It appears that in this way HW can
+	 * overwrite the clearing of DESC_INVLD_STATUS by SW (though we never
+	 * saw DESC_INVLD_STATUS set to 1 when the DMA engine had stopped) and
+	 * therefore HW never picks up these descriptors.
+	 *
+	 * Therefore it is necessary for SW to clear DESC_INVLD_STATUS only
+	 * after HW has set it, and the notify_hw flag accomplishes this
+	 * between issue_pending(..) and the invld_desc interrupt handler.
+	 */
+	if (mic_ch->notify_hw) {
+		ctrl_reg = mic_dma_ch_reg_read(mic_ch, MIC_DMA_CTRL_STATUS);
+		if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
+			mic_ch->notify_hw = 0;
+			/* Clear DESC_INVLD_STATUS flag */
+			mic_dma_ch_reg_write(mic_ch, MIC_DMA_CTRL_STATUS,
+					     ctrl_reg);
+		} else {
+			dev_err(mic_dma_ch_to_device(mic_ch),
+				"%s: DESC_INVLD_STATUS not set\n", __func__);
+		}
+	}
+	spin_unlock_irqrestore(&mic_ch->issue_lock, flags);
+}
+
+static inline void mic_dma_update_pending(struct mic_dma_chan *ch)
+{
+	if (mic_dma_ring_count(ch->head, ch->last_tail)
+		> MIC_DMA_PENDING_LEVEL)
+		mic_dma_issue_pending(&ch->chan);
+}
+
+static dma_cookie_t mic_dma_tx_submit_unlock(struct dma_async_tx_descriptor *tx)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(tx->chan);
+	dma_cookie_t cookie;
+
+	dma_cookie_assign(tx);
+	cookie = tx->cookie;
+	/*
+	 * smp_wmb pairs with smb_rmb in mic_dma_cleanup to ensure
+	 * that tx->cookie is visible before updating ch->head
+	 */
+	smp_wmb();
+
+	/*
+	 * The final fence/interrupt desc is now programmed in submit after the
+	 * cookie has been assigned
+	 */
+	if (tx->flags) {
+		mic_dma_fence_intr_desc(mic_ch, tx->flags);
+		tx->flags = 0;
+	}
+
+	spin_unlock(&mic_ch->prep_lock);
+	mic_dma_update_pending(mic_ch);
+	return cookie;
+}
+
+static inline struct dma_async_tx_descriptor *
+allocate_tx(struct mic_dma_chan *ch, unsigned long flags)
+{
+	/* index of the final desc which is or will be programmed */
+	u32 idx = flags ? ch->head : mic_dma_ring_dec(ch->head);
+	struct dma_async_tx_descriptor *tx = &ch->tx_array[idx];
+
+	dma_async_tx_descriptor_init(tx, &ch->chan);
+	tx->tx_submit = mic_dma_tx_submit_unlock;
+	tx->flags = flags;
+	return tx;
+}
+
+static struct dma_async_tx_descriptor *
+mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
+			 dma_addr_t dma_src, size_t len, unsigned long flags)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+	struct device *dev = mic_dma_ch_to_device(mic_ch);
+	int result;
+
+	/*
+	 * return holding prep_lock which will be released in submit
+	 */
+	spin_lock(&mic_ch->prep_lock);
+	result = mic_dma_do_dma(mic_ch, flags, dma_src, dma_dest, len);
+	if (result >= 0)
+		return allocate_tx(mic_ch, flags);
+	dev_err(dev, "Error enqueueing dma, error=%d\n", result);
+	spin_unlock(&mic_ch->prep_lock);
+	return NULL;
+}
+
+static struct dma_async_tx_descriptor *
+mic_dma_prep_interrupt_lock(struct dma_chan *ch, unsigned long flags)
+{
+	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
+	/*
+	 * Program 0 length DMA. Interrupt desc to be programmed in submit.
+	 * All DMA transfers need a valid source and destination.
+	 */
+	return mic_dma_prep_memcpy_lock(ch, mic_ch->desc_ring_da,
+					mic_ch->desc_ring_da, 0, flags);
+}
+
+static irqreturn_t mic_dma_thread_fn(int irq, void *data)
+{
+	struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
+
+	mic_dma_cleanup(&mic_dma_dev->mic_chan);
+	return IRQ_HANDLED;
+}
+
+static void mic_dma_invld_desc_interrupt(struct mic_dma_chan *ch)
+{
+	u32 ctrl_reg;
+
+	spin_lock(&ch->issue_lock);
+	if (!ch->notify_hw) {
+		ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
+		if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
+			/*
+			 * Check the descriptor ring for valid descriptors. If
+			 * valid descriptors are available clear the
+			 * DESC_INVLD_STATUS bit to signal to the hardware to
+			 * pick them up. If no valid descriptors are available,
+			 * when issue_pending is called, it will clear the
+			 * INVLD_STATUS bit to signal to the hardware to pick up
+			 * the posted descriptors.
+			 */
+			if (ch->desc_ring[mic_dma_ring_dec(ch->head)].qw0 &
+				MIC_DMA_DESC_VALID) {
+				mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS,
+						     ctrl_reg);
+			} else {
+				ch->notify_hw = 1;
+			}
+		} else {
+			dev_err(mic_dma_ch_to_device(ch),
+				"%s: DESC_INVLD_STATUS not set\n", __func__);
+		}
+	}
+	spin_unlock(&ch->issue_lock);
+}
+
+static void mic_dma_abort_interrupt(struct mic_dma_chan *ch, u32 intr_reg)
+{
+	u32 reg;
+	u32 ring_pos;
+
+	spin_lock(&ch->issue_lock);
+	reg = mic_dma_ch_reg_read(ch, MIC_DMA_LAST_DESC_ADDR_LOW);
+	ring_pos  = desc_ring_da_to_index(ch, reg);
+
+	if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID) {
+		/*
+		* Move back the HW next pointer so it points to the tail of
+		* the ring descriptor.
+		*/
+		mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, reg);
+	} else {
+		/* Last descriptor is not valid, find a valid one or head */
+		u32 tail = ch->last_tail;
+		u32 new_next;
+
+		while (tail != ch->head) {
+			tail = mic_dma_ring_inc(tail);
+			if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID)
+				break;
+		}
+		new_next = desc_ring_index_to_da(ch, tail);
+		mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, new_next);
+
+		dev_err(mic_dma_ch_to_device(ch),
+			"Abort at %d with invalid descriptor, new descr %d, head = %d, tail %d\n",
+			ring_pos, tail, ch->head, ch->last_tail);
+	}
+
+	/*
+	 * If abort was caused by hw error then restart DMA engine.
+	 * If abort was requested by driver then leave it stopped.
+	 */
+	if (intr_reg & MIC_DMA_ERROR_INTR_STATUS) {
+		if (ch->abort_tail != desc_ring_da_to_index(ch, reg)) {
+			ch->abort_tail = desc_ring_da_to_index(ch, reg);
+			ch->abort_counter = 0;
+		} else {
+			ch->abort_counter++;
+		}
+
+		if (ch->abort_counter < MIC_DMA_ABORT_LIMIT) {
+			dev_err(mic_dma_ch_to_device(ch),
+				"Abort/Error at %d (retry %d), reenabling DMA engine.\n",
+				 ch->abort_tail, ch->abort_counter);
+			mic_dma_enable_chan(ch);
+		} else {
+			/*
+			 * We have tried same descriptor several times and it is
+			 * still generating abort.
+			 */
+			dev_err(mic_dma_ch_to_device(ch),
+				"Abort Error at %d (retry %d), give up.\n",
+				ch->abort_tail, ch->abort_counter);
+		}
+	}
+	spin_unlock(&ch->issue_lock);
+}
+
+static u32 mic_dma_ack_interrupt(struct mic_dma_chan *ch)
+{
+	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
+
+	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
+	return intr_reg;
+}
+
+static irqreturn_t mic_dma_intr_handler(int irq, void *data)
+{
+	struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
+	bool wake_thread = false, error = false;
+	struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
+	struct device *dev = mic_dma_ch_to_device(ch);
+	u32 reg;
+
+	reg = mic_dma_ack_interrupt(&mic_dma_dev->mic_chan);
+	wake_thread = !!(reg & MIC_DMA_DESC_DONE_INTR_STATUS);
+	error = !!(reg & MIC_DMA_ERROR_STATUS);
+	if (error)
+		dev_err(dev, "%s error status 0x%x\n", __func__, reg);
+
+	if (reg & MIC_DMA_INVLD_DESC_INTR_STATUS)
+		mic_dma_invld_desc_interrupt(&mic_dma_dev->mic_chan);
+
+	if (reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
+		mic_dma_abort_interrupt(&mic_dma_dev->mic_chan, reg);
+
+	if (wake_thread)
+		return IRQ_WAKE_THREAD;
+
+	if (error || reg & MIC_DMA_INVLD_DESC_INTR_STATUS ||
+	    reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
+		return IRQ_HANDLED;
+
+	return IRQ_NONE;
+}
+
+static int mic_dma_setup_irq(struct mic_dma_device *mic_dma_dev)
+{
+	struct pci_dev *pdev = mic_dma_dev->pdev;
+	struct device *dev = &pdev->dev;
+	int rc = pci_enable_msi(pdev);
+
+	if (rc)
+		pci_intx(pdev, 1);
+
+	return devm_request_threaded_irq(dev, pdev->irq, mic_dma_intr_handler,
+					 mic_dma_thread_fn, IRQF_SHARED,
+					 "mic-x200-dma-intr", mic_dma_dev);
+}
+
+static inline void mic_dma_free_irq(struct mic_dma_device *mic_dma_dev)
+{
+	struct pci_dev *pdev = mic_dma_dev->pdev;
+	struct device *dev = &pdev->dev;
+
+	devm_free_irq(dev, pdev->irq, mic_dma_dev);
+	if (pci_dev_msi_enabled(pdev))
+		pci_disable_msi(pdev);
+}
+
+static void mic_dma_hw_init(struct mic_dma_device *mic_dma_dev)
+{
+	struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
+	u32 intr_reg;
+
+	dev_dbg(mic_dma_dev->dma_dev.dev, "Ctrl reg 0x%0x, Intr reg 0x%0x\n",
+		mic_dma_reg_read(mic_dma_dev, MIC_DMA_CTRL_STATUS),
+		mic_dma_reg_read(mic_dma_dev, MIC_DMA_INTR_CTRL_STATUS));
+
+	intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
+	if (intr_reg & MIC_DMA_ALL_INTR_EN) {
+		/*
+		 * Interrupts are enabled - that means card has been reset
+		 * without resetting DMA HW.
+		 * Disable interrupts and issue abort to stop DMA HW.
+		 */
+		intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
+		mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
+		mic_dma_disable_chan(ch);
+	}
+
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_CTRL_STATUS,
+			  MIC_DMA_CTRL_STATUS_INIT);
+
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_CHAN_BANDWIDTH,
+			  MIC_DMA_MAX_DST_MEM_WRITE);
+
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA_RAM_THRESH,
+			  MIC_DMA_STA_RAM_THRESH_INIT);
+
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_HDR_RAM,
+			  MIC_DMA_STA_HDR_THRESH_INIT);
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_HDR_RAM,
+			  MIC_DMA_STA_HDR_THRESH_INIT);
+
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_PLD_RAM,
+			  MIC_DMA_STA_PLD_THRESH_INIT);
+	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_PLD_RAM,
+			  MIC_DMA_STA_PLD_THRESH_INIT);
+}
+
+static int mic_dma_init(struct mic_dma_device *mic_dma_dev)
+{
+	struct mic_dma_chan *ch;
+	int rc = 0;
+
+	mic_dma_hw_init(mic_dma_dev);
+
+	rc = mic_dma_setup_irq(mic_dma_dev);
+	if (rc) {
+		dev_err(mic_dma_dev->dma_dev.dev,
+			"%s %d func error line %d\n", __func__, __LINE__, rc);
+		goto error;
+	}
+	ch = &mic_dma_dev->mic_chan;
+	spin_lock_init(&ch->cleanup_lock);
+	spin_lock_init(&ch->prep_lock);
+	spin_lock_init(&ch->issue_lock);
+error:
+	return rc;
+}
+
+static void mic_dma_uninit(struct mic_dma_device *mic_dma_dev)
+{
+	mic_dma_free_irq(mic_dma_dev);
+}
+
+static int mic_register_dma_device(struct mic_dma_device *mic_dma_dev)
+{
+	struct dma_device *dma_dev = &mic_dma_dev->dma_dev;
+	struct mic_dma_chan *ch;
+
+	dma_cap_zero(dma_dev->cap_mask);
+
+	dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
+	dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
+
+	dma_dev->device_alloc_chan_resources = mic_dma_alloc_chan_resources;
+	dma_dev->device_free_chan_resources = mic_dma_free_chan_resources;
+	dma_dev->device_tx_status = mic_dma_tx_status;
+	dma_dev->device_prep_dma_memcpy = mic_dma_prep_memcpy_lock;
+	dma_dev->device_prep_dma_interrupt = mic_dma_prep_interrupt_lock;
+	dma_dev->device_issue_pending = mic_dma_issue_pending;
+	INIT_LIST_HEAD(&dma_dev->channels);
+
+	/* Associate dma_dev with dma_chan */
+	ch = &mic_dma_dev->mic_chan;
+	ch->chan.device = dma_dev;
+	dma_cookie_init(&ch->chan);
+	list_add_tail(&ch->chan.device_node, &dma_dev->channels);
+	return dma_async_device_register(dma_dev);
+}
+
+static int mic_dma_probe(struct mic_dma_device *mic_dma_dev)
+{
+	struct dma_device *dma_dev;
+	int rc = -EINVAL;
+
+	dma_dev = &mic_dma_dev->dma_dev;
+	dma_dev->dev = &mic_dma_dev->pdev->dev;
+
+	/* MIC X200 has one DMA channel per function */
+	dma_dev->chancnt = 1;
+
+	mic_dma_dev->max_xfer_size = MIC_DMA_DESC_SIZE_MASK;
+
+	rc = mic_dma_init(mic_dma_dev);
+	if (rc) {
+		dev_err(&mic_dma_dev->pdev->dev,
+			"%s %d rc %d\n", __func__, __LINE__, rc);
+		goto init_error;
+	}
+
+	rc = mic_register_dma_device(mic_dma_dev);
+	if (rc) {
+		dev_err(&mic_dma_dev->pdev->dev,
+			"%s %d rc %d\n", __func__, __LINE__, rc);
+		goto reg_error;
+	}
+	return rc;
+reg_error:
+	mic_dma_uninit(mic_dma_dev);
+init_error:
+	return rc;
+}
+
+static void mic_dma_remove(struct mic_dma_device *mic_dma_dev)
+{
+	dma_async_device_unregister(&mic_dma_dev->dma_dev);
+	mic_dma_uninit(mic_dma_dev);
+}
+
+static struct mic_dma_device *
+mic_x200_alloc_dma(struct pci_dev *pdev, void __iomem *iobase)
+{
+	struct mic_dma_device *d = kzalloc(sizeof(*d), GFP_KERNEL);
+
+	if (!d)
+		return NULL;
+	d->pdev = pdev;
+	d->reg_base = iobase;
+	return d;
+}
+
+static int
+mic_x200_dma_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
+{
+	int rc;
+	struct mic_dma_device *dma_dev;
+	void __iomem * const *iomap;
+
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		goto done;
+	rc = pcim_iomap_regions(pdev, 0x1, MIC_DMA_DRV_NAME);
+	if (rc)
+		goto done;
+	iomap = pcim_iomap_table(pdev);
+	if (!iomap) {
+		rc = -ENOMEM;
+		goto done;
+	}
+	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
+	if (rc)
+		goto done;
+	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
+	if (rc)
+		goto done;
+	dma_dev = mic_x200_alloc_dma(pdev, iomap[0]);
+	if (!dma_dev) {
+		rc = -ENOMEM;
+		goto done;
+	}
+	pci_set_master(pdev);
+	pci_set_drvdata(pdev, dma_dev);
+	rc = mic_dma_probe(dma_dev);
+	if (rc)
+		goto probe_err;
+	return 0;
+probe_err:
+	kfree(dma_dev);
+done:
+	dev_err(&pdev->dev, "probe error rc %d\n", rc);
+	return rc;
+}
+
+static void  mic_x200_dma_remove(struct pci_dev *pdev)
+{
+	struct mic_dma_device *dma_dev = pci_get_drvdata(pdev);
+
+	mic_dma_remove(dma_dev);
+	kfree(dma_dev);
+}
+
+static struct pci_device_id mic_x200_dma_pci_id_table[] = {
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2264)},
+	{}
+};
+MODULE_DEVICE_TABLE(pci, mic_x200_dma_pci_id_table);
+
+static struct pci_driver mic_x200_dma_driver = {
+	.name     = MIC_DMA_DRV_NAME,
+	.id_table = mic_x200_dma_pci_id_table,
+	.probe    = mic_x200_dma_probe,
+	.remove   = mic_x200_dma_remove
+};
+
+static int __init mic_x200_dma_init(void)
+{
+	int rc = pci_register_driver(&mic_x200_dma_driver);
+
+	if (rc)
+		pr_err("%s %d rc %x\n", __func__, __LINE__, rc);
+	return rc;
+}
+
+static void __exit mic_x200_dma_exit(void)
+{
+	pci_unregister_driver(&mic_x200_dma_driver);
+}
+
+module_init(mic_x200_dma_init);
+module_exit(mic_x200_dma_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_DESCRIPTION("Intel(R) MIC X200 DMA Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH RESEND 2/2] MAINTAINERS: update for mic
  2016-02-04 15:10 [PATCH RESEND 0/2] Enable DMA driver for MIC X200 Coprocessor Jacek Lawrynowicz
  2016-02-04 15:10 ` [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver Jacek Lawrynowicz
@ 2016-02-04 15:10 ` Jacek Lawrynowicz
  1 sibling, 0 replies; 6+ messages in thread
From: Jacek Lawrynowicz @ 2016-02-04 15:10 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul
  Cc: dmaengine, linux-kernel, ashutosh.dixit, sudeep.dutt,
	andrzej.kacprowski, jacek.lawrynowicz

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Reviewed-by: Sudeep Dutt <sudeep.dutt@intel.com>
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0838e27..48d4b08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5778,6 +5778,7 @@ F:	Documentation/misc-devices/mei/*
 INTEL MIC DRIVERS (mic)
 M:	Sudeep Dutt <sudeep.dutt@intel.com>
 M:	Ashutosh Dixit <ashutosh.dixit@intel.com>
+M:	Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
 S:	Supported
 W:	https://github.com/sudeepdutt/mic
 W:	http://software.intel.com/en-us/mic-developer
@@ -5789,6 +5790,7 @@ F:	include/uapi/linux/scif_ioctl.h
 F:	drivers/misc/mic/
 F:	drivers/dma/mic_x100_dma.c
 F:	drivers/dma/mic_x100_dma.h
+F:	drivers/dma/mic_x200_dma.c
 F:	Documentation/mic/
 
 INTEL PMC/P-Unit IPC DRIVER
-- 
2.1.4

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

* Re: [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver
  2016-02-04 15:10 ` [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver Jacek Lawrynowicz
@ 2016-02-04 17:50   ` Andy Shevchenko
  2016-02-09 12:25     ` Lawrynowicz, Jacek
  2016-02-08  4:21   ` Vinod Koul
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2016-02-04 17:50 UTC (permalink / raw)
  To: Jacek Lawrynowicz
  Cc: Dan Williams, Vinod Koul, dmaengine, linux-kernel,
	ashutosh.dixit, sudeep.dutt, andrzej.kacprowski

On Thu, Feb 4, 2016 at 5:10 PM, Jacek Lawrynowicz
<jacek.lawrynowicz@intel.com> wrote:
> This patch implements the DMA Engine driver for the DMA controller on
> MIC X200 Coprocessors which are PCIe cards running Linux. Separate but
> identical DMA channels are available for the host and card. DMA
> transfers can be done from both the host and card in host<->card and
> host<->host directions.
>

Can it share code with 100 series?

> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> Signed-off-by: Sudeep Dutt <sudeep.dutt@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@intel.com>
> ---
>  drivers/dma/Kconfig        |   18 +
>  drivers/dma/Makefile       |    1 +
>  drivers/dma/mic_x200_dma.c | 1114 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1133 insertions(+)
>  create mode 100644 drivers/dma/mic_x200_dma.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 79b1390..a3ea8b9 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -277,6 +277,24 @@ config INTEL_MIC_X100_DMA
>           OS and tools for MIC to use with this driver are available from
>           <http://software.intel.com/en-us/mic-developer>.
>
> +config INTEL_MIC_X200_DMA
> +       tristate "Intel MIC X200 DMA Driver"
> +        depends on 64BIT && X86 && PCI

Doesn't it the same like
        depends on X86_64 && PCI
?

> +       select DMAENGINE
> +       help
> +         This enables DMA support for the Intel Many Integrated Core
> +         (MIC) family of PCIe form factor coprocessor X200 devices that
> +         run a 64 bit Linux OS. This driver will be used by both MIC
> +         host and card drivers.
> +
> +         If you are building host kernel with a MIC device or a card
> +         kernel for a MIC device, then say M (recommended) or Y, else
> +         say N. If unsure say N.
> +
> +         More information about the Intel MIC family as well as the Linux
> +         OS and tools for MIC to use with this driver are available from
> +         <http://software.intel.com/en-us/mic-developer>.
> +
>  config K3_DMA
>         tristate "Hisilicon K3 DMA support"
>         depends on ARCH_HI3xxx
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 2dd0a067..a3756f5 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
>  obj-$(CONFIG_INTEL_IOATDMA) += ioat/
>  obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
>  obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
> +obj-$(CONFIG_INTEL_MIC_X200_DMA) += mic_x200_dma.o
>  obj-$(CONFIG_K3_DMA) += k3dma.o
>  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
>  obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
> diff --git a/drivers/dma/mic_x200_dma.c b/drivers/dma/mic_x200_dma.c
> new file mode 100644
> index 0000000..80c4959
> --- /dev/null
> +++ b/drivers/dma/mic_x200_dma.c
> @@ -0,0 +1,1114 @@
> +/*
> + * Intel MIC Platform Software Stack (MPSS)
> + *
> + * Copyright(c) 2016 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + *
> + * Intel MIC X200 DMA driver
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/vmalloc.h>
> +
> +#include "dmaengine.h"
> +
> +#define MIC_DMA_DRV_NAME       "mic_x200_dma"
> +
> +#define MIC_DMA_DESC_RX_SIZE   (128 * 1024)
> +#define MIC_DMA_ABORT_TO_MS    3000
> +#define MIC_DMA_RING_TO_MS     20
> +#define MIC_DMA_PENDING_LEVEL  16
> +#define MIC_DMA_ABORT_LIMIT    5
> +
> +/* DMA Descriptor related flags */
> +#define MIC_DMA_DESC_VALID             BIT(31)
> +#define MIC_DMA_DESC_INTR_ENABLE       BIT(30)
> +#define MIC_DMA_DESC_SRC_LINK_ERR      BIT(29)
> +#define MIC_DMA_DESC_DST_LINK_ERR      BIT(28)
> +#define MIC_DMA_DESC_LOW_MASK          (0xffffffffUL)
> +#define MIC_DMA_DESC_SRC_LOW_SHIFT     32
> +#define MIC_DMA_DESC_BITS_32_TO_47     (0xffffUL << 32)
> +#define MIC_DMA_DESC_SIZE_MASK         ((1UL << 27) - 1)
> +
> +#define MIC_DMA_DESC_RING_ADDR_LOW             0x214
> +#define MIC_DMA_DESC_RING_ADDR_HIGH            0x218
> +#define MIC_DMA_NEXT_DESC_ADDR_LOW             0x21C
> +#define MIC_DMA_DESC_RING_SIZE                 0x220
> +#define MIC_DMA_LAST_DESC_ADDR_LOW             0x224
> +#define MIC_DMA_LAST_DESC_XFER_SIZE            0x228
> +
> +#define MIC_DMA_CTRL_STATUS                    0x238
> +#define MIC_DMA_CTRL_PAUSE                     BIT(0)
> +#define MIC_DMA_CTRL_ABORT                     BIT(1)
> +#define MIC_DMA_CTRL_WB_ENABLE                 BIT(2)
> +#define MIC_DMA_CTRL_START                     BIT(3)
> +#define MIC_DMA_CTRL_RING_STOP                 BIT(4)
> +#define MIC_DMA_CTRL_ONCHIP_MODE               BIT(5)
> +#define MIC_DMA_CTRL_OFFCHIP_MODE              (2UL << 5)
> +#define MIC_DMA_CTRL_DESC_INVLD_STATUS         BIT(8)
> +#define MIC_DMA_CTRL_PAUSE_DONE_STATUS         BIT(9)
> +#define MIC_DMA_CTRL_ABORT_DONE_STATUS         BIT(10)
> +#define MIC_DMA_CTRL_FACTORY_TEST              BIT(11)
> +#define MIC_DMA_CTRL_IMM_PAUSE_DONE_STATUS     BIT(12)
> +#define MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE     (3UL << 16)
> +#define MIC_DMA_CTRL_RELAXED_DATA_WRITE                BIT(25)
> +#define MIC_DMA_CTRL_NO_SNOOP_DESC_READ                BIT(26)
> +#define MIC_DMA_CTRL_NO_SNOOP_DATA_READ                BIT(27)
> +#define MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE       BIT(28)
> +#define MIC_DMA_CTRL_IN_PROGRESS               BIT(30)
> +#define MIC_DMA_CTRL_HEADER_LOG                        BIT(31)
> +#define MIC_DMA_CTRL_MODE_BITS                 (3UL << 5)
> +#define MIC_DMA_CTRL_STATUS_BITS               (0x1FUL << 8)
> +#define MIC_DMA_CTRL_STATUS_INIT  (MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE |        \
> +                                  MIC_DMA_CTRL_RELAXED_DATA_WRITE |    \
> +                                  MIC_DMA_CTRL_NO_SNOOP_DESC_READ |    \
> +                                  MIC_DMA_CTRL_NO_SNOOP_DATA_READ |    \
> +                                  MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE)
> +
> +#define MIC_DMA_INTR_CTRL_STATUS               0x23C
> +#define MIC_DMA_ERROR_INTR_EN                  BIT(0)
> +#define MIC_DMA_INVLD_DESC_INTR_EN             BIT(1)
> +#define MIC_DMA_ABORT_DONE_INTR_EN             BIT(3)
> +#define MIC_DMA_GRACE_PAUSE_INTR_EN            BIT(4)
> +#define MIC_DMA_IMM_PAUSE_INTR_EN              BIT(5)
> +#define MIC_DMA_IRQ_PIN_INTR_EN                        BIT(15)
> +#define MIC_DMA_ERROR_INTR_STATUS              BIT(16)
> +#define MIC_DMA_INVLD_DESC_INTR_STATUS         BIT(17)
> +#define MIC_DMA_DESC_DONE_INTR_STATUS          BIT(18)
> +#define MIC_DMA_ABORT_DONE_INTR_STATUS         BIT(19)
> +#define MIC_DMA_GRACE_PAUSE_INTR_STATUS                BIT(20)
> +#define MIC_DMA_IMM_PAUSE_INTR_STATUS          BIT(21)
> +#define MIC_DMA_ALL_INTR_EN    (MIC_DMA_ERROR_INTR_EN |                \
> +                                MIC_DMA_INVLD_DESC_INTR_EN |           \
> +                                MIC_DMA_ABORT_DONE_INTR_EN |           \
> +                                MIC_DMA_GRACE_PAUSE_INTR_EN |          \
> +                                MIC_DMA_IMM_PAUSE_INTR_EN)
> +
> +#define MIC_DMA_ERROR_STATUS   (MIC_DMA_ERROR_INTR_STATUS |            \
> +                                MIC_DMA_ABORT_DONE_INTR_STATUS |       \
> +                                MIC_DMA_GRACE_PAUSE_INTR_STATUS |      \
> +                                MIC_DMA_IMM_PAUSE_INTR_STATUS)
> +
> +#define MIC_DMA_ALL_INTR_STATUS        (MIC_DMA_ERROR_INTR_STATUS |            \
> +                                MIC_DMA_INVLD_DESC_INTR_STATUS |       \
> +                                MIC_DMA_DESC_DONE_INTR_STATUS |        \
> +                                MIC_DMA_ABORT_DONE_INTR_STATUS |       \
> +                                MIC_DMA_GRACE_PAUSE_INTR_STATUS |      \
> +                                MIC_DMA_IMM_PAUSE_INTR_STATUS)
> +/*
> + * Performance tuning registers. For explanation of the values
> + * used please refer to mic x200 dma hardware specification.
> + */
> +#define MIC_DMA_CHAN_BANDWIDTH                 0x22C
> +#define MIC_DMA_MAX_DST_MEM_WRITE              (2 << 24)
> +
> +#define MIC_DMA_STA_RAM_THRESH                 0x258
> +#define MIC_DMA_HDR_RAM_USAGE_THRESH           46UL
> +#define MIC_DMA_PLD_RAM_USAGE_THRESH           (61UL << 16)
> +#define MIC_DMA_STA_RAM_THRESH_INIT (MIC_DMA_HDR_RAM_USAGE_THRESH |    \
> +                                    MIC_DMA_PLD_RAM_USAGE_THRESH)
> +
> +#define MIC_DMA_STA0_HDR_RAM                   0x25C
> +#define MIC_DMA_STA1_HDR_RAM                   0x2A4
> +#define MIC_DMA_HDR_RAM_UPPER_THRESH           160UL
> +#define MIC_DMA_HDR_RAM_LOWER_THRESH           (152UL << 16)
> +#define MIC_DMA_STA_HDR_THRESH_INIT (MIC_DMA_HDR_RAM_UPPER_THRESH |    \
> +                                    MIC_DMA_HDR_RAM_LOWER_THRESH)
> +
> +#define MIC_DMA_STA0_PLD_RAM                   0x260
> +#define MIC_DMA_STA1_PLD_RAM                   0x2A8
> +#define MIC_DMA_PLD_RAM_UPPER_THRESH           256UL
> +#define MIC_DMA_PLD_RAM_LOWER_THRESH           (248UL << 16)
> +#define MIC_DMA_STA_PLD_THRESH_INIT (MIC_DMA_PLD_RAM_UPPER_THRESH |    \
> +                                    MIC_DMA_PLD_RAM_LOWER_THRESH)
> +
> +/* HW DMA descriptor */
> +struct mic_dma_desc {
> +       u64 qw0;
> +       u64 qw1;
> +};
> +
> +/*
> + * mic_dma_chan - MIC X200 DMA channel specific data structures
> + *
> + * @ch_num: channel number
> + * @last_tail: cached value of descriptor ring tail
> + * @head: index of next descriptor in desc_ring
> + * @chan: dma engine api channel
> + * @desc_ring: dma descriptor ring
> + * @desc_ring_da: DMA address of desc_ring
> + * @tx_array: array of async_tx
> + * @cleanup_lock: used to synchronize descriptor cleanup
> + * @prep_lock: lock held in prep_memcpy & released in tx_submit
> + * @issue_lock: lock used to synchronize writes to head
> + * @notify_hw: flag used to notify HW that new descriptors are available
> + * @abort_tail: descriptor ring position at which last abort occurred
> + * @abort_counter: number of aborts at the last position in desc ring
> + */
> +struct mic_dma_chan {
> +       int ch_num;
> +       u32 last_tail;
> +       u32 head;

> +       struct dma_chan chan;
> +       struct mic_dma_desc *desc_ring;
> +       dma_addr_t desc_ring_da;
> +       struct dma_async_tx_descriptor *tx_array;

Btw, can you use virt-chan API?

> +       spinlock_t cleanup_lock;
> +       spinlock_t prep_lock;
> +       spinlock_t issue_lock;
> +       bool notify_hw;
> +       u32 abort_tail;
> +       u32 abort_counter;
> +};
> +
> +/*
> + * mic_dma_device - Per MIC X200 DMA device driver specific data structures
> + *
> + * @pdev: PCIe device
> + * @dma_dev: underlying dma device
> + * @reg_base: virtual address of the mmio space
> + * @mic_chan: Array of MIC X200 DMA channels
> + * @max_xfer_size: maximum transfer size per dma descriptor
> + */
> +struct mic_dma_device {
> +       struct pci_dev *pdev;
> +       struct dma_device dma_dev;
> +       void __iomem *reg_base;
> +       struct mic_dma_chan mic_chan;
> +       size_t max_xfer_size;
> +};
> +
> +static inline struct mic_dma_device *to_mic_dma_dev(struct mic_dma_chan *ch)
> +{
> +       return
> +       container_of((const typeof(((struct mic_dma_device *)0)->mic_chan) *)
> +                        (ch - ch->ch_num), struct mic_dma_device, mic_chan);
> +}
> +
> +static inline struct mic_dma_chan *to_mic_dma_chan(struct dma_chan *ch)
> +{
> +       return container_of(ch, struct mic_dma_chan, chan);
> +}
> +
> +static inline struct device *mic_dma_ch_to_device(struct mic_dma_chan *ch)
> +{
> +       return to_mic_dma_dev(ch)->dma_dev.dev;
> +}
> +
> +static inline u32 mic_dma_reg_read(struct mic_dma_device *pdma, u32 offset)
> +{
> +       return ioread32(pdma->reg_base + offset);
> +}
> +
> +static inline void mic_dma_reg_write(struct mic_dma_device *pdma,
> +                                    u32 offset, u32 value)
> +{
> +       iowrite32(value, pdma->reg_base + offset);
> +}
> +
> +static inline u32 mic_dma_ch_reg_read(struct mic_dma_chan *ch, u32 offset)
> +{
> +       return mic_dma_reg_read(to_mic_dma_dev(ch), offset);
> +}
> +
> +static inline void mic_dma_ch_reg_write(struct mic_dma_chan *ch,
> +                                       u32 offset, u32 value)
> +{
> +       mic_dma_reg_write(to_mic_dma_dev(ch), offset, value);
> +}
> +
> +static inline u32 mic_dma_ring_inc(u32 val)
> +{
> +       return (val + 1) & (MIC_DMA_DESC_RX_SIZE - 1);

    return (val + 1) % MIC_DMA_DESC_RX_SIZE;

More flexible, if the size is power of 2 it will be optimized by
compiler to the same you had previously.

> +}
> +
> +static inline u32 mic_dma_ring_dec(u32 val)
> +{
> +       return val ? val - 1 : MIC_DMA_DESC_RX_SIZE - 1;

   return (val - 1) % MIC_DMA_DESC_RX_SIZE;

Would it be issues with that?

> +}
> +
> +static inline void mic_dma_inc_head(struct mic_dma_chan *ch)
> +{
> +       ch->head = mic_dma_ring_inc(ch->head);
> +}
> +
> +static int mic_dma_alloc_desc_ring(struct mic_dma_chan *ch)
> +{
> +       /* Desc size must be >= depth of ring + prefetch size + 1 */
> +       u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> +       struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> +
> +       ch->desc_ring = dma_alloc_coherent(dev, desc_ring_size,
> +                                          &ch->desc_ring_da,
> +                                          GFP_KERNEL | __GFP_ZERO);
> +       if (!ch->desc_ring)
> +               return -ENOMEM;
> +
> +       dev_dbg(dev, "DMA desc ring VA = %p DA 0x%llx size 0x%llx\n",
> +               ch->desc_ring, ch->desc_ring_da, desc_ring_size);

%pad for DMA addresses.

> +       ch->tx_array = vzalloc(MIC_DMA_DESC_RX_SIZE * sizeof(*ch->tx_array));

> +       if (!ch->tx_array)
> +               goto tx_error;
> +       return 0;

> +tx_error:
> +       dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
> +       return -ENOMEM;

Only one user, may be you can move it there?

> +}
> +
> +static inline void mic_dma_disable_chan(struct mic_dma_chan *ch)
> +{
> +       struct device *dev = mic_dma_ch_to_device(ch);
> +       u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);

> +       int i;

unsigned int i;

> +
> +       /* set abort bit and clear start bit */
> +       ctrl_reg |= MIC_DMA_CTRL_ABORT;
> +       ctrl_reg &= ~MIC_DMA_CTRL_START;
> +       mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> +
> +       for (i = 0; i < MIC_DMA_ABORT_TO_MS; i++) {
> +               ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +
> +               if (ctrl_reg & MIC_DMA_CTRL_ABORT_DONE_STATUS)
> +                       return;
> +
> +               mdelay(1);
> +       }
> +       dev_err(dev, "%s abort timed out 0x%x head %d tail %d\n",
> +               __func__, ctrl_reg, ch->head, ch->last_tail);
> +}
> +
> +static inline void mic_dma_enable_chan(struct mic_dma_chan *ch)
> +{
> +       u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +
> +       ctrl_reg |= MIC_DMA_CTRL_WB_ENABLE;
> +       /* Clear any active status bits ([31,12:8]) */
> +       ctrl_reg |= MIC_DMA_CTRL_HEADER_LOG | MIC_DMA_CTRL_STATUS_BITS;
> +       ctrl_reg &= ~MIC_DMA_CTRL_MODE_BITS;
> +       /* Enable SGL off-chip mode */
> +       ctrl_reg |= MIC_DMA_CTRL_OFFCHIP_MODE;
> +       /* Set start and clear abort and abort done bits */
> +       ctrl_reg |= MIC_DMA_CTRL_START;
> +       ctrl_reg &= ~MIC_DMA_CTRL_ABORT;
> +       mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> +}
> +
> +static inline void mic_dma_chan_mask_intr(struct mic_dma_chan *ch)
> +{
> +       u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +       intr_reg &= ~(MIC_DMA_ALL_INTR_EN);

Redundant parens.

> +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +}
> +
> +static inline void mic_dma_chan_unmask_intr(struct mic_dma_chan *ch)
> +{
> +       u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +       intr_reg |= (MIC_DMA_ALL_INTR_EN);

Ditto

> +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +}
> +
> +static void mic_dma_chan_set_desc_ring(struct mic_dma_chan *ch)
> +{
> +       /* Set up the descriptor ring base address and size */

> +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_LOW,
> +                            ch->desc_ring_da & 0xffffffff);
> +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_HIGH,
> +                            (ch->desc_ring_da >> 32) & 0xffffffff);

Better to provide mic_dma_ch_reg_writeq() based on lo_hi_writel().

> +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_SIZE, MIC_DMA_DESC_RX_SIZE);

> +       /* Set up the next descriptor to the base of the descriptor ring */
> +       mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW,
> +                            ch->desc_ring_da & 0xffffffff);

Only low?

> +}
> +
> +static void mic_dma_cleanup(struct mic_dma_chan *ch)
> +{
> +       struct dma_async_tx_descriptor *tx;
> +       u32 last_tail;
> +       u32 cached_head;
> +
> +       spin_lock(&ch->cleanup_lock);
> +       /*
> +        * Use a cached head rather than using ch->head directly, since a
> +        * different thread can be updating ch->head potentially leading to an
> +        * infinite loop below.
> +        */
> +       cached_head = ACCESS_ONCE(ch->head);
> +       /*
> +        * This is the barrier pair for smp_wmb() in fn.
> +        * mic_dma_tx_submit_unlock. It's required so that we read the
> +        * updated cookie value from tx->cookie.
> +        */
> +       smp_rmb();
> +
> +       for (last_tail = ch->last_tail; last_tail != cached_head;) {
> +               /* Break out of the loop if this desc is not yet complete */
> +               if (ch->desc_ring[last_tail].qw0 & MIC_DMA_DESC_VALID)
> +                       break;
> +
> +               tx = &ch->tx_array[last_tail];
> +               if (tx->cookie) {
> +                       dma_cookie_complete(tx);
> +                       if (tx->callback) {
> +                               tx->callback(tx->callback_param);
> +                               tx->callback = NULL;
> +                       }
> +               }
> +               last_tail = mic_dma_ring_inc(last_tail);
> +       }
> +       /* finish all completion callbacks before incrementing tail */
> +       smp_mb();
> +       ch->last_tail = last_tail;
> +       spin_unlock(&ch->cleanup_lock);
> +}
> +
> +static int mic_dma_chan_setup(struct mic_dma_chan *ch)
> +{
> +       mic_dma_chan_set_desc_ring(ch);
> +       ch->last_tail = 0;
> +       ch->head = 0;
> +       ch->notify_hw = 1;

> +       mic_dma_chan_unmask_intr(ch);
> +       mic_dma_enable_chan(ch);

Seems alogical. Perhaps first is to enable channel and then enable interrupts?

> +       return 0;
> +}
> +
> +static void mic_dma_free_desc_ring(struct mic_dma_chan *ch)
> +{
> +       u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> +       struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> +
> +       vfree(ch->tx_array);
> +       dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
> +       ch->desc_ring = NULL;
> +       ch->desc_ring_da = 0;
> +
> +       /* Reset description ring registers */
> +       mic_dma_chan_set_desc_ring(ch);
> +}
> +
> +static int mic_dma_chan_init(struct mic_dma_chan *ch)
> +{
> +       struct device *dev = mic_dma_ch_to_device(ch);
> +       int rc;
> +
> +       rc = mic_dma_alloc_desc_ring(ch);
> +       if (rc)
> +               goto ring_error;
> +       rc = mic_dma_chan_setup(ch);
> +       if (rc)
> +               goto chan_error;
> +       return rc;
> +chan_error:
> +       mic_dma_free_desc_ring(ch);

> +ring_error:
> +       dev_err(dev, "%s ret %d\n", __func__, rc);

Is it useful?

> +       return rc;
> +}
> +
> +static int mic_dma_alloc_chan_resources(struct dma_chan *ch)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +       struct device *dev = mic_dma_ch_to_device(mic_ch);
> +       int rc = mic_dma_chan_init(mic_ch);
> +
> +       if (rc) {
> +               dev_err(dev, "%s ret %d\n", __func__, rc);

Ditto.

> +               return rc;
> +       }
> +       return MIC_DMA_DESC_RX_SIZE;
> +}
> +
> +static inline u32 desc_ring_da_to_index(struct mic_dma_chan *ch, u32 descr)
> +{
> +       dma_addr_t last_dma_off = descr - (ch->desc_ring_da & ULONG_MAX);

Hmm… Why do you need that strange masking?

> +
> +       return (last_dma_off / sizeof(struct mic_dma_desc)) &
> +               (MIC_DMA_DESC_RX_SIZE - 1);

% _RX_SIZE

> +}
> +
> +static inline u32 desc_ring_index_to_da(struct mic_dma_chan *ch, u32 index)
> +{
> +       dma_addr_t addr = ch->desc_ring_da +
> +                       sizeof(struct mic_dma_desc) * index;
> +
> +       return (addr & ULONG_MAX);
> +}
> +
> +static void mic_dma_free_chan_resources(struct dma_chan *ch)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +
> +       mic_dma_disable_chan(mic_ch);
> +       mic_dma_chan_mask_intr(mic_ch);
> +       mic_dma_cleanup(mic_ch);
> +       mic_dma_free_desc_ring(mic_ch);
> +}
> +
> +static enum dma_status
> +mic_dma_tx_status(struct dma_chan *ch, dma_cookie_t cookie,
> +                 struct dma_tx_state *txstate)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +
> +       if (dma_cookie_status(ch, cookie, txstate) != DMA_COMPLETE)
> +               mic_dma_cleanup(mic_ch);
> +
> +       return dma_cookie_status(ch, cookie, txstate);
> +}
> +
> +static int mic_dma_ring_count(u32 head, u32 tail)
> +{
> +       int count;
> +
> +       if (head >= tail)
> +               count = (tail - 0) + (MIC_DMA_DESC_RX_SIZE - head);
> +       else
> +               count = tail - head;
> +       return count - 1;
> +}
> +
> +static int mic_dma_avail_desc_ring_space(struct mic_dma_chan *ch, int required)
> +{
> +       struct device *dev = mic_dma_ch_to_device(ch);
> +       int count;
> +       unsigned long timeout = jiffies +
> +                       msecs_to_jiffies(MIC_DMA_RING_TO_MS);
> +
> +       count = mic_dma_ring_count(ch->head, ch->last_tail);
> +       while (count < required) {
> +               mic_dma_cleanup(ch);
> +               count = mic_dma_ring_count(ch->head, ch->last_tail);
> +               if ((count >= required) || time_after_eq(jiffies, timeout))
> +                       break;
> +               usleep_range(50, 100);
> +       }
> +
> +       if (count < required) {
> +               dev_err(dev, "%s count %d reqd %d head %d tail %d\n",
> +                       __func__, count, required, ch->head, ch->last_tail);
> +               return -ENOMEM;
> +       }
> +       return count;
> +}
> +
> +static void
> +mic_dma_memcpy_desc(struct mic_dma_desc *desc, dma_addr_t src_phys,
> +                   dma_addr_t dst_phys, u64 size, int flags)
> +{
> +       u64 qw0, qw1;
> +
> +       qw0 = (src_phys & MIC_DMA_DESC_BITS_32_TO_47) << 16;
> +       qw0 |= (dst_phys & MIC_DMA_DESC_BITS_32_TO_47);
> +       qw0 |= size & MIC_DMA_DESC_SIZE_MASK;
> +       qw0 |=  MIC_DMA_DESC_VALID;
> +       if (flags & DMA_PREP_INTERRUPT)
> +               qw0 |=  MIC_DMA_DESC_INTR_ENABLE;
> +
> +       qw1 = (src_phys & MIC_DMA_DESC_LOW_MASK) << MIC_DMA_DESC_SRC_LOW_SHIFT;
> +       qw1 |= dst_phys & MIC_DMA_DESC_LOW_MASK;
> +
> +       desc->qw1 = qw1;
> +       /*
> +        * Update QW1 before QW0 since QW0 has the valid bit and the
> +        * descriptor might be picked up by the hardware DMA engine
> +        * immediately if it finds a valid descriptor.
> +        */
> +       wmb();
> +       desc->qw0 = qw0;
> +       /*
> +        * This is not smp_wmb() on purpose since we are also publishing the
> +        * descriptor updates to a dma device
> +        */
> +       wmb();
> +}
> +
> +static void mic_dma_fence_intr_desc(struct mic_dma_chan *ch, int flags)
> +{
> +       /*
> +       * Zero-length DMA memcpy needs valid source and destination addresses
> +       */
> +       mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> +                           ch->desc_ring_da, ch->desc_ring_da, 0, flags);
> +       mic_dma_inc_head(ch);
> +}
> +
> +static int mic_dma_prog_memcpy_desc(struct mic_dma_chan *ch, dma_addr_t src,
> +                                   dma_addr_t dst, size_t len, int flags)
> +{
> +       struct device *dev = mic_dma_ch_to_device(ch);
> +       size_t current_transfer_len;
> +       size_t max_xfer_size = to_mic_dma_dev(ch)->max_xfer_size;
> +       int num_desc = len / max_xfer_size;
> +       int ret;
> +
> +       if (len % max_xfer_size)
> +               num_desc++;
> +       /*
> +        * A single additional descriptor is programmed for fence/interrupt
> +        * during submit(..)
> +        */
> +       if (flags)
> +               num_desc++;
> +
> +       if (!num_desc) {
> +               dev_err(dev, "%s nothing to do: rc %d\n", __func__, -EINVAL);
> +               return -EINVAL;
> +       }
> +
> +       ret = mic_dma_avail_desc_ring_space(ch, num_desc);
> +       if (ret <= 0) {
> +               dev_err(dev, "%s desc ring full ret %d\n", __func__, ret);
> +               return ret;
> +       }
> +
> +       while (len > 0) {
> +               current_transfer_len = min(len, max_xfer_size);
> +               /*
> +                * Set flags to 0 here, we don't want memcpy descriptors to
> +                * generate interrupts, a separate descriptor will be programmed
> +                * for fence/interrupt during submit, after a callback is
> +                * guaranteed to have been assigned
> +                */
> +               mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> +                                   src, dst, current_transfer_len, 0);
> +               mic_dma_inc_head(ch);
> +               len -= current_transfer_len;
> +               dst = dst + current_transfer_len;
> +               src = src + current_transfer_len;
> +       }
> +       return 0;
> +}
> +
> +static int mic_dma_do_dma(struct mic_dma_chan *ch, int flags, dma_addr_t src,
> +                         dma_addr_t dst, size_t len)
> +{
> +       return mic_dma_prog_memcpy_desc(ch, src, dst, len, flags);
> +}
> +
> +static inline void mic_dma_issue_pending(struct dma_chan *ch)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +       unsigned long flags;
> +       u32 ctrl_reg;
> +
> +       spin_lock_irqsave(&mic_ch->issue_lock, flags);
> +       /*
> +        * The notify_hw is introduced because occasionally we were seeing an
> +        * issue where the HW tail seemed stuck, i.e. the DMA engine would not
> +        * pick up new descriptors available in the ring. We found that
> +        * serializing operations between SW and HW made this issue
> +        * disappear. We believe this issue arises because of a race between SW
> +        * and HW. SW adds new descriptors to the ring and clears
> +        * DESC_INVLD_STATUS to ask HW to pick them up. However HW has
> +        * prefetched descriptors and writes DESC_INVLD_STATUS to let SW know it
> +        * has fetched an invalid descriptor. It appears that in this way HW can
> +        * overwrite the clearing of DESC_INVLD_STATUS by SW (though we never
> +        * saw DESC_INVLD_STATUS set to 1 when the DMA engine had stopped) and
> +        * therefore HW never picks up these descriptors.
> +        *
> +        * Therefore it is necessary for SW to clear DESC_INVLD_STATUS only
> +        * after HW has set it, and the notify_hw flag accomplishes this
> +        * between issue_pending(..) and the invld_desc interrupt handler.
> +        */
> +       if (mic_ch->notify_hw) {
> +               ctrl_reg = mic_dma_ch_reg_read(mic_ch, MIC_DMA_CTRL_STATUS);
> +               if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
> +                       mic_ch->notify_hw = 0;
> +                       /* Clear DESC_INVLD_STATUS flag */
> +                       mic_dma_ch_reg_write(mic_ch, MIC_DMA_CTRL_STATUS,
> +                                            ctrl_reg);
> +               } else {
> +                       dev_err(mic_dma_ch_to_device(mic_ch),
> +                               "%s: DESC_INVLD_STATUS not set\n", __func__);
> +               }
> +       }
> +       spin_unlock_irqrestore(&mic_ch->issue_lock, flags);
> +}
> +
> +static inline void mic_dma_update_pending(struct mic_dma_chan *ch)
> +{
> +       if (mic_dma_ring_count(ch->head, ch->last_tail)
> +               > MIC_DMA_PENDING_LEVEL)

One line?

> +               mic_dma_issue_pending(&ch->chan);
> +}
> +
> +static dma_cookie_t mic_dma_tx_submit_unlock(struct dma_async_tx_descriptor *tx)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(tx->chan);
> +       dma_cookie_t cookie;
> +
> +       dma_cookie_assign(tx);
> +       cookie = tx->cookie;
> +       /*
> +        * smp_wmb pairs with smb_rmb in mic_dma_cleanup to ensure
> +        * that tx->cookie is visible before updating ch->head
> +        */
> +       smp_wmb();
> +
> +       /*
> +        * The final fence/interrupt desc is now programmed in submit after the
> +        * cookie has been assigned
> +        */
> +       if (tx->flags) {
> +               mic_dma_fence_intr_desc(mic_ch, tx->flags);
> +               tx->flags = 0;
> +       }
> +
> +       spin_unlock(&mic_ch->prep_lock);
> +       mic_dma_update_pending(mic_ch);
> +       return cookie;
> +}
> +
> +static inline struct dma_async_tx_descriptor *
> +allocate_tx(struct mic_dma_chan *ch, unsigned long flags)
> +{
> +       /* index of the final desc which is or will be programmed */
> +       u32 idx = flags ? ch->head : mic_dma_ring_dec(ch->head);
> +       struct dma_async_tx_descriptor *tx = &ch->tx_array[idx];
> +
> +       dma_async_tx_descriptor_init(tx, &ch->chan);
> +       tx->tx_submit = mic_dma_tx_submit_unlock;
> +       tx->flags = flags;
> +       return tx;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
> +                        dma_addr_t dma_src, size_t len, unsigned long flags)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +       struct device *dev = mic_dma_ch_to_device(mic_ch);
> +       int result;
> +
> +       /*
> +        * return holding prep_lock which will be released in submit
> +        */
> +       spin_lock(&mic_ch->prep_lock);
> +       result = mic_dma_do_dma(mic_ch, flags, dma_src, dma_dest, len);

> +       if (result >= 0)
> +               return allocate_tx(mic_ch, flags);

Hmm… Usual pattern is
if (err)
 do error path();

> +       dev_err(dev, "Error enqueueing dma, error=%d\n", result);
> +       spin_unlock(&mic_ch->prep_lock);
> +       return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +mic_dma_prep_interrupt_lock(struct dma_chan *ch, unsigned long flags)
> +{
> +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +       /*
> +        * Program 0 length DMA. Interrupt desc to be programmed in submit.
> +        * All DMA transfers need a valid source and destination.
> +        */
> +       return mic_dma_prep_memcpy_lock(ch, mic_ch->desc_ring_da,
> +                                       mic_ch->desc_ring_da, 0, flags);
> +}
> +
> +static irqreturn_t mic_dma_thread_fn(int irq, void *data)
> +{
> +       struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
> +
> +       mic_dma_cleanup(&mic_dma_dev->mic_chan);
> +       return IRQ_HANDLED;
> +}
> +
> +static void mic_dma_invld_desc_interrupt(struct mic_dma_chan *ch)
> +{
> +       u32 ctrl_reg;
> +
> +       spin_lock(&ch->issue_lock);
> +       if (!ch->notify_hw) {
> +               ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +               if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {

Isn't too long?
MIC_DMA_CTRL_DESC_INVLD_STATUS -> MIC_DMA_CTRL_DESC_INVAL ?

> +                       /*
> +                        * Check the descriptor ring for valid descriptors. If
> +                        * valid descriptors are available clear the
> +                        * DESC_INVLD_STATUS bit to signal to the hardware to
> +                        * pick them up. If no valid descriptors are available,
> +                        * when issue_pending is called, it will clear the
> +                        * INVLD_STATUS bit to signal to the hardware to pick up
> +                        * the posted descriptors.
> +                        */
> +                       if (ch->desc_ring[mic_dma_ring_dec(ch->head)].qw0 &
> +                               MIC_DMA_DESC_VALID) {
> +                               mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS,
> +                                                    ctrl_reg);
> +                       } else {
> +                               ch->notify_hw = 1;
> +                       }
> +               } else {
> +                       dev_err(mic_dma_ch_to_device(ch),
> +                               "%s: DESC_INVLD_STATUS not set\n", __func__);
> +               }
> +       }
> +       spin_unlock(&ch->issue_lock);
> +}
> +
> +static void mic_dma_abort_interrupt(struct mic_dma_chan *ch, u32 intr_reg)
> +{
> +       u32 reg;
> +       u32 ring_pos;
> +
> +       spin_lock(&ch->issue_lock);
> +       reg = mic_dma_ch_reg_read(ch, MIC_DMA_LAST_DESC_ADDR_LOW);
> +       ring_pos  = desc_ring_da_to_index(ch, reg);
> +
> +       if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID) {
> +               /*
> +               * Move back the HW next pointer so it points to the tail of
> +               * the ring descriptor.
> +               */
> +               mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, reg);
> +       } else {
> +               /* Last descriptor is not valid, find a valid one or head */
> +               u32 tail = ch->last_tail;
> +               u32 new_next;
> +
> +               while (tail != ch->head) {
> +                       tail = mic_dma_ring_inc(tail);
> +                       if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID)
> +                               break;
> +               }
> +               new_next = desc_ring_index_to_da(ch, tail);
> +               mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, new_next);
> +
> +               dev_err(mic_dma_ch_to_device(ch),
> +                       "Abort at %d with invalid descriptor, new descr %d, head = %d, tail %d\n",
> +                       ring_pos, tail, ch->head, ch->last_tail);
> +       }
> +
> +       /*
> +        * If abort was caused by hw error then restart DMA engine.
> +        * If abort was requested by driver then leave it stopped.
> +        */
> +       if (intr_reg & MIC_DMA_ERROR_INTR_STATUS) {
> +               if (ch->abort_tail != desc_ring_da_to_index(ch, reg)) {
> +                       ch->abort_tail = desc_ring_da_to_index(ch, reg);
> +                       ch->abort_counter = 0;
> +               } else {
> +                       ch->abort_counter++;
> +               }
> +
> +               if (ch->abort_counter < MIC_DMA_ABORT_LIMIT) {
> +                       dev_err(mic_dma_ch_to_device(ch),
> +                               "Abort/Error at %d (retry %d), reenabling DMA engine.\n",
> +                                ch->abort_tail, ch->abort_counter);
> +                       mic_dma_enable_chan(ch);
> +               } else {
> +                       /*
> +                        * We have tried same descriptor several times and it is
> +                        * still generating abort.
> +                        */
> +                       dev_err(mic_dma_ch_to_device(ch),
> +                               "Abort Error at %d (retry %d), give up.\n",
> +                               ch->abort_tail, ch->abort_counter);
> +               }
> +       }
> +       spin_unlock(&ch->issue_lock);
> +}
> +
> +static u32 mic_dma_ack_interrupt(struct mic_dma_chan *ch)
> +{
> +       u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +       return intr_reg;
> +}
> +
> +static irqreturn_t mic_dma_intr_handler(int irq, void *data)
> +{
> +       struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
> +       bool wake_thread = false, error = false;
> +       struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> +       struct device *dev = mic_dma_ch_to_device(ch);
> +       u32 reg;
> +
> +       reg = mic_dma_ack_interrupt(&mic_dma_dev->mic_chan);
> +       wake_thread = !!(reg & MIC_DMA_DESC_DONE_INTR_STATUS);
> +       error = !!(reg & MIC_DMA_ERROR_STATUS);
> +       if (error)
> +               dev_err(dev, "%s error status 0x%x\n", __func__, reg);
> +
> +       if (reg & MIC_DMA_INVLD_DESC_INTR_STATUS)
> +               mic_dma_invld_desc_interrupt(&mic_dma_dev->mic_chan);
> +
> +       if (reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> +               mic_dma_abort_interrupt(&mic_dma_dev->mic_chan, reg);
> +
> +       if (wake_thread)
> +               return IRQ_WAKE_THREAD;
> +
> +       if (error || reg & MIC_DMA_INVLD_DESC_INTR_STATUS ||
> +           reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> +               return IRQ_HANDLED;
> +
> +       return IRQ_NONE;
> +}
> +
> +static int mic_dma_setup_irq(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct pci_dev *pdev = mic_dma_dev->pdev;
> +       struct device *dev = &pdev->dev;
> +       int rc = pci_enable_msi(pdev);
> +
> +       if (rc)
> +               pci_intx(pdev, 1);
> +
> +       return devm_request_threaded_irq(dev, pdev->irq, mic_dma_intr_handler,
> +                                        mic_dma_thread_fn, IRQF_SHARED,
> +                                        "mic-x200-dma-intr", mic_dma_dev);
> +}
> +
> +static inline void mic_dma_free_irq(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct pci_dev *pdev = mic_dma_dev->pdev;
> +       struct device *dev = &pdev->dev;
> +
> +       devm_free_irq(dev, pdev->irq, mic_dma_dev);

> +       if (pci_dev_msi_enabled(pdev))
> +               pci_disable_msi(pdev);

pcim_enable_device() will take care of this one.

> +}
> +
> +static void mic_dma_hw_init(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> +       u32 intr_reg;
> +
> +       dev_dbg(mic_dma_dev->dma_dev.dev, "Ctrl reg 0x%0x, Intr reg 0x%0x\n",
> +               mic_dma_reg_read(mic_dma_dev, MIC_DMA_CTRL_STATUS),
> +               mic_dma_reg_read(mic_dma_dev, MIC_DMA_INTR_CTRL_STATUS));
> +
> +       intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +       if (intr_reg & MIC_DMA_ALL_INTR_EN) {
> +               /*
> +                * Interrupts are enabled - that means card has been reset
> +                * without resetting DMA HW.
> +                * Disable interrupts and issue abort to stop DMA HW.
> +                */
> +               intr_reg &= ~(MIC_DMA_ALL_INTR_EN);

Redundant parens.

> +               mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +               mic_dma_disable_chan(ch);
> +       }
> +
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_CTRL_STATUS,
> +                         MIC_DMA_CTRL_STATUS_INIT);
> +
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_CHAN_BANDWIDTH,
> +                         MIC_DMA_MAX_DST_MEM_WRITE);
> +
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA_RAM_THRESH,
> +                         MIC_DMA_STA_RAM_THRESH_INIT);
> +
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_HDR_RAM,
> +                         MIC_DMA_STA_HDR_THRESH_INIT);
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_HDR_RAM,
> +                         MIC_DMA_STA_HDR_THRESH_INIT);
> +
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_PLD_RAM,
> +                         MIC_DMA_STA_PLD_THRESH_INIT);
> +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_PLD_RAM,
> +                         MIC_DMA_STA_PLD_THRESH_INIT);
> +}
> +
> +static int mic_dma_init(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct mic_dma_chan *ch;

> +       int rc = 0;

Useless assignment.

> +
> +       mic_dma_hw_init(mic_dma_dev);
> +
> +       rc = mic_dma_setup_irq(mic_dma_dev);
> +       if (rc) {
> +               dev_err(mic_dma_dev->dma_dev.dev,
> +                       "%s %d func error line %d\n", __func__, __LINE__, rc);
> +               goto error;
> +       }
> +       ch = &mic_dma_dev->mic_chan;
> +       spin_lock_init(&ch->cleanup_lock);
> +       spin_lock_init(&ch->prep_lock);
> +       spin_lock_init(&ch->issue_lock);

> +error:

Useless label.

> +       return rc;
> +}
> +
> +static void mic_dma_uninit(struct mic_dma_device *mic_dma_dev)
> +{
> +       mic_dma_free_irq(mic_dma_dev);
> +}
> +
> +static int mic_register_dma_device(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct dma_device *dma_dev = &mic_dma_dev->dma_dev;
> +       struct mic_dma_chan *ch;
> +
> +       dma_cap_zero(dma_dev->cap_mask);
> +
> +       dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
> +       dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
> +
> +       dma_dev->device_alloc_chan_resources = mic_dma_alloc_chan_resources;
> +       dma_dev->device_free_chan_resources = mic_dma_free_chan_resources;
> +       dma_dev->device_tx_status = mic_dma_tx_status;
> +       dma_dev->device_prep_dma_memcpy = mic_dma_prep_memcpy_lock;
> +       dma_dev->device_prep_dma_interrupt = mic_dma_prep_interrupt_lock;
> +       dma_dev->device_issue_pending = mic_dma_issue_pending;
> +       INIT_LIST_HEAD(&dma_dev->channels);
> +
> +       /* Associate dma_dev with dma_chan */
> +       ch = &mic_dma_dev->mic_chan;
> +       ch->chan.device = dma_dev;
> +       dma_cookie_init(&ch->chan);
> +       list_add_tail(&ch->chan.device_node, &dma_dev->channels);
> +       return dma_async_device_register(dma_dev);
> +}
> +
> +static int mic_dma_probe(struct mic_dma_device *mic_dma_dev)
> +{
> +       struct dma_device *dma_dev;
> +       int rc = -EINVAL;
> +
> +       dma_dev = &mic_dma_dev->dma_dev;
> +       dma_dev->dev = &mic_dma_dev->pdev->dev;
> +
> +       /* MIC X200 has one DMA channel per function */
> +       dma_dev->chancnt = 1;
> +
> +       mic_dma_dev->max_xfer_size = MIC_DMA_DESC_SIZE_MASK;
> +
> +       rc = mic_dma_init(mic_dma_dev);
> +       if (rc) {
> +               dev_err(&mic_dma_dev->pdev->dev,
> +                       "%s %d rc %d\n", __func__, __LINE__, rc);
> +               goto init_error;
> +       }
> +
> +       rc = mic_register_dma_device(mic_dma_dev);
> +       if (rc) {
> +               dev_err(&mic_dma_dev->pdev->dev,
> +                       "%s %d rc %d\n", __func__, __LINE__, rc);
> +               goto reg_error;
> +       }
> +       return rc;
> +reg_error:
> +       mic_dma_uninit(mic_dma_dev);

> +init_error:

Ditto.

> +       return rc;
> +}
> +
> +static void mic_dma_remove(struct mic_dma_device *mic_dma_dev)
> +{
> +       dma_async_device_unregister(&mic_dma_dev->dma_dev);
> +       mic_dma_uninit(mic_dma_dev);
> +}
> +
> +static struct mic_dma_device *
> +mic_x200_alloc_dma(struct pci_dev *pdev, void __iomem *iobase)
> +{
> +       struct mic_dma_device *d = kzalloc(sizeof(*d), GFP_KERNEL);

devm_

> +
> +       if (!d)
> +               return NULL;
> +       d->pdev = pdev;
> +       d->reg_base = iobase;
> +       return d;
> +}
> +
> +static int
> +mic_x200_dma_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
> +{
> +       int rc;
> +       struct mic_dma_device *dma_dev;
> +       void __iomem * const *iomap;
> +
> +       rc = pcim_enable_device(pdev);
> +       if (rc)
> +               goto done;
> +       rc = pcim_iomap_regions(pdev, 0x1, MIC_DMA_DRV_NAME);
> +       if (rc)
> +               goto done;

> +       iomap = pcim_iomap_table(pdev);

> +       if (!iomap) {
> +               rc = -ENOMEM;
> +               goto done;
> +       }

Redundant check. Previous one fail otherwise this is valid.

> +       rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
> +       if (rc)
> +               goto done;
> +       rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
> +       if (rc)
> +               goto done;
> +       dma_dev = mic_x200_alloc_dma(pdev, iomap[0]);
> +       if (!dma_dev) {
> +               rc = -ENOMEM;
> +               goto done;
> +       }
> +       pci_set_master(pdev);
> +       pci_set_drvdata(pdev, dma_dev);
> +       rc = mic_dma_probe(dma_dev);
> +       if (rc)
> +               goto probe_err;
> +       return 0;

> +probe_err:
> +       kfree(dma_dev);

devm_ takes care of.

> +done:
> +       dev_err(&pdev->dev, "probe error rc %d\n", rc);

Useless. You may learn "initcall_debug" parameter.

And thus label is useless.

> +       return rc;
> +}
> +
> +static void  mic_x200_dma_remove(struct pci_dev *pdev)
> +{
> +       struct mic_dma_device *dma_dev = pci_get_drvdata(pdev);
> +
> +       mic_dma_remove(dma_dev);

> +       kfree(dma_dev);

devm_

> +}
> +
> +static struct pci_device_id mic_x200_dma_pci_id_table[] = {
> +       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2264)},
> +       {}
> +};
> +MODULE_DEVICE_TABLE(pci, mic_x200_dma_pci_id_table);
> +
> +static struct pci_driver mic_x200_dma_driver = {
> +       .name     = MIC_DMA_DRV_NAME,
> +       .id_table = mic_x200_dma_pci_id_table,
> +       .probe    = mic_x200_dma_probe,
> +       .remove   = mic_x200_dma_remove

> +};
> +
> +static int __init mic_x200_dma_init(void)
> +{
> +       int rc = pci_register_driver(&mic_x200_dma_driver);
> +
> +       if (rc)
> +               pr_err("%s %d rc %x\n", __func__, __LINE__, rc);
> +       return rc;
> +}
> +
> +static void __exit mic_x200_dma_exit(void)
> +{
> +       pci_unregister_driver(&mic_x200_dma_driver);
> +}
> +
> +module_init(mic_x200_dma_init);
> +module_exit(mic_x200_dma_exit);

module_pci_driver();

> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_DESCRIPTION("Intel(R) MIC X200 DMA Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.4
>



-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver
  2016-02-04 15:10 ` [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver Jacek Lawrynowicz
  2016-02-04 17:50   ` Andy Shevchenko
@ 2016-02-08  4:21   ` Vinod Koul
  1 sibling, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2016-02-08  4:21 UTC (permalink / raw)
  To: Jacek Lawrynowicz
  Cc: dan.j.williams, dmaengine, linux-kernel, ashutosh.dixit,
	sudeep.dutt, andrzej.kacprowski

On Thu, Feb 04, 2016 at 04:10:49PM +0100, Jacek Lawrynowicz wrote:
> This patch implements the DMA Engine driver for the DMA controller on
> MIC X200 Coprocessors which are PCIe cards running Linux. Separate but
> identical DMA channels are available for the host and card. DMA
> transfers can be done from both the host and card in host<->card and
> host<->host directions.
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> Signed-off-by: Sudeep Dutt <sudeep.dutt@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@intel.com>
> ---
>  drivers/dma/Kconfig        |   18 +
>  drivers/dma/Makefile       |    1 +
>  drivers/dma/mic_x200_dma.c | 1114 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1133 insertions(+)
>  create mode 100644 drivers/dma/mic_x200_dma.c
> 
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 79b1390..a3ea8b9 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -277,6 +277,24 @@ config INTEL_MIC_X100_DMA
>  	  OS and tools for MIC to use with this driver are available from
>  	  <http://software.intel.com/en-us/mic-developer>.
>  
> +config INTEL_MIC_X200_DMA
> +	tristate "Intel MIC X200 DMA Driver"
> +        depends on 64BIT && X86 && PCI

X86_64?

How different is this from x100 controller and why do we need a new driver?

> +	select DMAENGINE
> +	help
> +	  This enables DMA support for the Intel Many Integrated Core
> +	  (MIC) family of PCIe form factor coprocessor X200 devices that
> +	  run a 64 bit Linux OS. This driver will be used by both MIC
> +	  host and card drivers.
> +
> +	  If you are building host kernel with a MIC device or a card
> +	  kernel for a MIC device, then say M (recommended) or Y, else
> +	  say N. If unsure say N.
> +
> +	  More information about the Intel MIC family as well as the Linux
> +	  OS and tools for MIC to use with this driver are available from
> +	  <http://software.intel.com/en-us/mic-developer>.
> +
>  config K3_DMA
>  	tristate "Hisilicon K3 DMA support"
>  	depends on ARCH_HI3xxx
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 2dd0a067..a3756f5 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
>  obj-$(CONFIG_INTEL_IOATDMA) += ioat/
>  obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
>  obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
> +obj-$(CONFIG_INTEL_MIC_X200_DMA) += mic_x200_dma.o
>  obj-$(CONFIG_K3_DMA) += k3dma.o
>  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
>  obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
> diff --git a/drivers/dma/mic_x200_dma.c b/drivers/dma/mic_x200_dma.c
> new file mode 100644
> index 0000000..80c4959
> --- /dev/null
> +++ b/drivers/dma/mic_x200_dma.c
> @@ -0,0 +1,1114 @@
> +/*
> + * Intel MIC Platform Software Stack (MPSS)
> + *
> + * Copyright(c) 2016 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + *
> + * Intel MIC X200 DMA driver
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/vmalloc.h>
> +
> +#include "dmaengine.h"
> +
> +#define MIC_DMA_DRV_NAME	"mic_x200_dma"
> +
> +#define MIC_DMA_DESC_RX_SIZE	(128 * 1024)
> +#define MIC_DMA_ABORT_TO_MS	3000
> +#define MIC_DMA_RING_TO_MS	20
> +#define MIC_DMA_PENDING_LEVEL	16
> +#define MIC_DMA_ABORT_LIMIT	5
> +
> +/* DMA Descriptor related flags */
> +#define MIC_DMA_DESC_VALID		BIT(31)
> +#define MIC_DMA_DESC_INTR_ENABLE	BIT(30)
> +#define MIC_DMA_DESC_SRC_LINK_ERR	BIT(29)
> +#define MIC_DMA_DESC_DST_LINK_ERR	BIT(28)
> +#define MIC_DMA_DESC_LOW_MASK		(0xffffffffUL)
> +#define MIC_DMA_DESC_SRC_LOW_SHIFT	32
> +#define MIC_DMA_DESC_BITS_32_TO_47	(0xffffUL << 32)
> +#define MIC_DMA_DESC_SIZE_MASK		((1UL << 27) - 1)
> +
> +#define MIC_DMA_DESC_RING_ADDR_LOW		0x214
> +#define MIC_DMA_DESC_RING_ADDR_HIGH		0x218
> +#define MIC_DMA_NEXT_DESC_ADDR_LOW		0x21C
> +#define MIC_DMA_DESC_RING_SIZE			0x220
> +#define MIC_DMA_LAST_DESC_ADDR_LOW		0x224
> +#define MIC_DMA_LAST_DESC_XFER_SIZE		0x228
> +
> +#define MIC_DMA_CTRL_STATUS			0x238
> +#define MIC_DMA_CTRL_PAUSE			BIT(0)
> +#define MIC_DMA_CTRL_ABORT			BIT(1)
> +#define MIC_DMA_CTRL_WB_ENABLE			BIT(2)
> +#define MIC_DMA_CTRL_START			BIT(3)
> +#define MIC_DMA_CTRL_RING_STOP			BIT(4)
> +#define MIC_DMA_CTRL_ONCHIP_MODE		BIT(5)
> +#define MIC_DMA_CTRL_OFFCHIP_MODE		(2UL << 5)
> +#define MIC_DMA_CTRL_DESC_INVLD_STATUS		BIT(8)
> +#define MIC_DMA_CTRL_PAUSE_DONE_STATUS		BIT(9)
> +#define MIC_DMA_CTRL_ABORT_DONE_STATUS		BIT(10)
> +#define MIC_DMA_CTRL_FACTORY_TEST		BIT(11)
> +#define MIC_DMA_CTRL_IMM_PAUSE_DONE_STATUS	BIT(12)
> +#define MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE	(3UL << 16)
> +#define MIC_DMA_CTRL_RELAXED_DATA_WRITE		BIT(25)
> +#define MIC_DMA_CTRL_NO_SNOOP_DESC_READ		BIT(26)
> +#define MIC_DMA_CTRL_NO_SNOOP_DATA_READ		BIT(27)
> +#define MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE	BIT(28)
> +#define MIC_DMA_CTRL_IN_PROGRESS		BIT(30)
> +#define MIC_DMA_CTRL_HEADER_LOG			BIT(31)
> +#define MIC_DMA_CTRL_MODE_BITS			(3UL << 5)
> +#define MIC_DMA_CTRL_STATUS_BITS		(0x1FUL << 8)
> +#define MIC_DMA_CTRL_STATUS_INIT  (MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE |	\
> +				   MIC_DMA_CTRL_RELAXED_DATA_WRITE |	\
> +				   MIC_DMA_CTRL_NO_SNOOP_DESC_READ |	\
> +				   MIC_DMA_CTRL_NO_SNOOP_DATA_READ |	\
> +				   MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE)
> +
> +#define MIC_DMA_INTR_CTRL_STATUS		0x23C
> +#define MIC_DMA_ERROR_INTR_EN			BIT(0)
> +#define MIC_DMA_INVLD_DESC_INTR_EN		BIT(1)
> +#define MIC_DMA_ABORT_DONE_INTR_EN		BIT(3)
> +#define MIC_DMA_GRACE_PAUSE_INTR_EN		BIT(4)
> +#define MIC_DMA_IMM_PAUSE_INTR_EN		BIT(5)
> +#define MIC_DMA_IRQ_PIN_INTR_EN			BIT(15)
> +#define MIC_DMA_ERROR_INTR_STATUS		BIT(16)
> +#define MIC_DMA_INVLD_DESC_INTR_STATUS		BIT(17)
> +#define MIC_DMA_DESC_DONE_INTR_STATUS		BIT(18)
> +#define MIC_DMA_ABORT_DONE_INTR_STATUS		BIT(19)
> +#define MIC_DMA_GRACE_PAUSE_INTR_STATUS		BIT(20)
> +#define MIC_DMA_IMM_PAUSE_INTR_STATUS		BIT(21)
> +#define MIC_DMA_ALL_INTR_EN	(MIC_DMA_ERROR_INTR_EN |		\
> +				 MIC_DMA_INVLD_DESC_INTR_EN |		\
> +				 MIC_DMA_ABORT_DONE_INTR_EN |		\
> +				 MIC_DMA_GRACE_PAUSE_INTR_EN |		\
> +				 MIC_DMA_IMM_PAUSE_INTR_EN)
> +
> +#define MIC_DMA_ERROR_STATUS	(MIC_DMA_ERROR_INTR_STATUS |		\
> +				 MIC_DMA_ABORT_DONE_INTR_STATUS |	\
> +				 MIC_DMA_GRACE_PAUSE_INTR_STATUS |	\
> +				 MIC_DMA_IMM_PAUSE_INTR_STATUS)
> +
> +#define MIC_DMA_ALL_INTR_STATUS	(MIC_DMA_ERROR_INTR_STATUS |		\
> +				 MIC_DMA_INVLD_DESC_INTR_STATUS |	\
> +				 MIC_DMA_DESC_DONE_INTR_STATUS |	\
> +				 MIC_DMA_ABORT_DONE_INTR_STATUS |	\
> +				 MIC_DMA_GRACE_PAUSE_INTR_STATUS |	\
> +				 MIC_DMA_IMM_PAUSE_INTR_STATUS)
> +/*
> + * Performance tuning registers. For explanation of the values
> + * used please refer to mic x200 dma hardware specification.
> + */
> +#define MIC_DMA_CHAN_BANDWIDTH			0x22C
> +#define MIC_DMA_MAX_DST_MEM_WRITE		(2 << 24)
> +
> +#define MIC_DMA_STA_RAM_THRESH			0x258
> +#define MIC_DMA_HDR_RAM_USAGE_THRESH		46UL
> +#define MIC_DMA_PLD_RAM_USAGE_THRESH		(61UL << 16)
> +#define MIC_DMA_STA_RAM_THRESH_INIT (MIC_DMA_HDR_RAM_USAGE_THRESH |	\
> +				     MIC_DMA_PLD_RAM_USAGE_THRESH)
> +
> +#define MIC_DMA_STA0_HDR_RAM			0x25C
> +#define MIC_DMA_STA1_HDR_RAM			0x2A4
> +#define MIC_DMA_HDR_RAM_UPPER_THRESH		160UL
> +#define MIC_DMA_HDR_RAM_LOWER_THRESH		(152UL << 16)
> +#define MIC_DMA_STA_HDR_THRESH_INIT (MIC_DMA_HDR_RAM_UPPER_THRESH |	\
> +				     MIC_DMA_HDR_RAM_LOWER_THRESH)
> +
> +#define MIC_DMA_STA0_PLD_RAM			0x260
> +#define MIC_DMA_STA1_PLD_RAM			0x2A8
> +#define MIC_DMA_PLD_RAM_UPPER_THRESH		256UL
> +#define MIC_DMA_PLD_RAM_LOWER_THRESH		(248UL << 16)
> +#define MIC_DMA_STA_PLD_THRESH_INIT (MIC_DMA_PLD_RAM_UPPER_THRESH |	\
> +				     MIC_DMA_PLD_RAM_LOWER_THRESH)
> +
> +/* HW DMA descriptor */
> +struct mic_dma_desc {
> +	u64 qw0;
> +	u64 qw1;
> +};
> +
> +/*
> + * mic_dma_chan - MIC X200 DMA channel specific data structures
> + *
> + * @ch_num: channel number
> + * @last_tail: cached value of descriptor ring tail
> + * @head: index of next descriptor in desc_ring
> + * @chan: dma engine api channel
> + * @desc_ring: dma descriptor ring
> + * @desc_ring_da: DMA address of desc_ring
> + * @tx_array: array of async_tx
> + * @cleanup_lock: used to synchronize descriptor cleanup
> + * @prep_lock: lock held in prep_memcpy & released in tx_submit
> + * @issue_lock: lock used to synchronize writes to head
> + * @notify_hw: flag used to notify HW that new descriptors are available
> + * @abort_tail: descriptor ring position at which last abort occurred
> + * @abort_counter: number of aborts at the last position in desc ring
> + */
> +struct mic_dma_chan {
> +	int ch_num;
> +	u32 last_tail;
> +	u32 head;
> +	struct dma_chan chan;
> +	struct mic_dma_desc *desc_ring;
> +	dma_addr_t desc_ring_da;
> +	struct dma_async_tx_descriptor *tx_array;
> +	spinlock_t cleanup_lock;
> +	spinlock_t prep_lock;
> +	spinlock_t issue_lock;
> +	bool notify_hw;
> +	u32 abort_tail;
> +	u32 abort_counter;
> +};
> +
> +/*
> + * mic_dma_device - Per MIC X200 DMA device driver specific data structures
> + *
> + * @pdev: PCIe device
> + * @dma_dev: underlying dma device
> + * @reg_base: virtual address of the mmio space
> + * @mic_chan: Array of MIC X200 DMA channels
> + * @max_xfer_size: maximum transfer size per dma descriptor
> + */
> +struct mic_dma_device {
> +	struct pci_dev *pdev;
> +	struct dma_device dma_dev;
> +	void __iomem *reg_base;
> +	struct mic_dma_chan mic_chan;
> +	size_t max_xfer_size;
> +};
> +
> +static inline struct mic_dma_device *to_mic_dma_dev(struct mic_dma_chan *ch)
> +{
> +	return
> +	container_of((const typeof(((struct mic_dma_device *)0)->mic_chan) *)
> +			 (ch - ch->ch_num), struct mic_dma_device, mic_chan);
> +}
> +
> +static inline struct mic_dma_chan *to_mic_dma_chan(struct dma_chan *ch)
> +{
> +	return container_of(ch, struct mic_dma_chan, chan);
> +}
> +
> +static inline struct device *mic_dma_ch_to_device(struct mic_dma_chan *ch)
> +{
> +	return to_mic_dma_dev(ch)->dma_dev.dev;
> +}
> +
> +static inline u32 mic_dma_reg_read(struct mic_dma_device *pdma, u32 offset)
> +{
> +	return ioread32(pdma->reg_base + offset);
> +}
> +
> +static inline void mic_dma_reg_write(struct mic_dma_device *pdma,
> +				     u32 offset, u32 value)
> +{
> +	iowrite32(value, pdma->reg_base + offset);
> +}
> +
> +static inline u32 mic_dma_ch_reg_read(struct mic_dma_chan *ch, u32 offset)
> +{
> +	return mic_dma_reg_read(to_mic_dma_dev(ch), offset);
> +}
> +
> +static inline void mic_dma_ch_reg_write(struct mic_dma_chan *ch,
> +					u32 offset, u32 value)
> +{
> +	mic_dma_reg_write(to_mic_dma_dev(ch), offset, value);
> +}
> +
> +static inline u32 mic_dma_ring_inc(u32 val)
> +{
> +	return (val + 1) & (MIC_DMA_DESC_RX_SIZE - 1);
> +}
> +
> +static inline u32 mic_dma_ring_dec(u32 val)
> +{
> +	return val ? val - 1 : MIC_DMA_DESC_RX_SIZE - 1;
> +}
> +
> +static inline void mic_dma_inc_head(struct mic_dma_chan *ch)
> +{
> +	ch->head = mic_dma_ring_inc(ch->head);
> +}
> +
> +static int mic_dma_alloc_desc_ring(struct mic_dma_chan *ch)
> +{
> +	/* Desc size must be >= depth of ring + prefetch size + 1 */
> +	u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> +	struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> +
> +	ch->desc_ring = dma_alloc_coherent(dev, desc_ring_size,
> +					   &ch->desc_ring_da,
> +					   GFP_KERNEL | __GFP_ZERO);
> +	if (!ch->desc_ring)
> +		return -ENOMEM;
> +
> +	dev_dbg(dev, "DMA desc ring VA = %p DA 0x%llx size 0x%llx\n",
> +		ch->desc_ring, ch->desc_ring_da, desc_ring_size);
> +	ch->tx_array = vzalloc(MIC_DMA_DESC_RX_SIZE * sizeof(*ch->tx_array));
> +	if (!ch->tx_array)
> +		goto tx_error;
> +	return 0;
> +tx_error:
> +	dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
> +	return -ENOMEM;
> +}
> +
> +static inline void mic_dma_disable_chan(struct mic_dma_chan *ch)
> +{
> +	struct device *dev = mic_dma_ch_to_device(ch);
> +	u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +	int i;
> +
> +	/* set abort bit and clear start bit */
> +	ctrl_reg |= MIC_DMA_CTRL_ABORT;
> +	ctrl_reg &= ~MIC_DMA_CTRL_START;
> +	mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> +
> +	for (i = 0; i < MIC_DMA_ABORT_TO_MS; i++) {
> +		ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +
> +		if (ctrl_reg & MIC_DMA_CTRL_ABORT_DONE_STATUS)
> +			return;
> +
> +		mdelay(1);
> +	}
> +	dev_err(dev, "%s abort timed out 0x%x head %d tail %d\n",
> +		__func__, ctrl_reg, ch->head, ch->last_tail);
> +}
> +
> +static inline void mic_dma_enable_chan(struct mic_dma_chan *ch)
> +{
> +	u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +
> +	ctrl_reg |= MIC_DMA_CTRL_WB_ENABLE;
> +	/* Clear any active status bits ([31,12:8]) */
> +	ctrl_reg |= MIC_DMA_CTRL_HEADER_LOG | MIC_DMA_CTRL_STATUS_BITS;
> +	ctrl_reg &= ~MIC_DMA_CTRL_MODE_BITS;
> +	/* Enable SGL off-chip mode */
> +	ctrl_reg |= MIC_DMA_CTRL_OFFCHIP_MODE;
> +	/* Set start and clear abort and abort done bits */
> +	ctrl_reg |= MIC_DMA_CTRL_START;
> +	ctrl_reg &= ~MIC_DMA_CTRL_ABORT;
> +	mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> +}
> +
> +static inline void mic_dma_chan_mask_intr(struct mic_dma_chan *ch)
> +{
> +	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +	intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
> +	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +}
> +
> +static inline void mic_dma_chan_unmask_intr(struct mic_dma_chan *ch)
> +{
> +	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +	intr_reg |= (MIC_DMA_ALL_INTR_EN);
> +	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +}
> +
> +static void mic_dma_chan_set_desc_ring(struct mic_dma_chan *ch)
> +{
> +	/* Set up the descriptor ring base address and size */
> +	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_LOW,
> +			     ch->desc_ring_da & 0xffffffff);
> +	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_HIGH,
> +			     (ch->desc_ring_da >> 32) & 0xffffffff);
> +	mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_SIZE, MIC_DMA_DESC_RX_SIZE);
> +	/* Set up the next descriptor to the base of the descriptor ring */
> +	mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW,
> +			     ch->desc_ring_da & 0xffffffff);
> +}
> +
> +static void mic_dma_cleanup(struct mic_dma_chan *ch)
> +{
> +	struct dma_async_tx_descriptor *tx;
> +	u32 last_tail;
> +	u32 cached_head;
> +
> +	spin_lock(&ch->cleanup_lock);
> +	/*
> +	 * Use a cached head rather than using ch->head directly, since a
> +	 * different thread can be updating ch->head potentially leading to an
> +	 * infinite loop below.
> +	 */
> +	cached_head = ACCESS_ONCE(ch->head);
> +	/*
> +	 * This is the barrier pair for smp_wmb() in fn.
> +	 * mic_dma_tx_submit_unlock. It's required so that we read the
> +	 * updated cookie value from tx->cookie.
> +	 */
> +	smp_rmb();
> +
> +	for (last_tail = ch->last_tail; last_tail != cached_head;) {
> +		/* Break out of the loop if this desc is not yet complete */
> +		if (ch->desc_ring[last_tail].qw0 & MIC_DMA_DESC_VALID)
> +			break;
> +
> +		tx = &ch->tx_array[last_tail];
> +		if (tx->cookie) {
> +			dma_cookie_complete(tx);
> +			if (tx->callback) {
> +				tx->callback(tx->callback_param);
> +				tx->callback = NULL;
> +			}
> +		}
> +		last_tail = mic_dma_ring_inc(last_tail);
> +	}
> +	/* finish all completion callbacks before incrementing tail */
> +	smp_mb();
> +	ch->last_tail = last_tail;
> +	spin_unlock(&ch->cleanup_lock);
> +}
> +
> +static int mic_dma_chan_setup(struct mic_dma_chan *ch)
> +{
> +	mic_dma_chan_set_desc_ring(ch);
> +	ch->last_tail = 0;
> +	ch->head = 0;
> +	ch->notify_hw = 1;
> +	mic_dma_chan_unmask_intr(ch);
> +	mic_dma_enable_chan(ch);
> +	return 0;
> +}
> +
> +static void mic_dma_free_desc_ring(struct mic_dma_chan *ch)
> +{
> +	u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> +	struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> +
> +	vfree(ch->tx_array);
> +	dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch->desc_ring_da);
> +	ch->desc_ring = NULL;
> +	ch->desc_ring_da = 0;
> +
> +	/* Reset description ring registers */
> +	mic_dma_chan_set_desc_ring(ch);
> +}
> +
> +static int mic_dma_chan_init(struct mic_dma_chan *ch)
> +{
> +	struct device *dev = mic_dma_ch_to_device(ch);
> +	int rc;
> +
> +	rc = mic_dma_alloc_desc_ring(ch);
> +	if (rc)
> +		goto ring_error;
> +	rc = mic_dma_chan_setup(ch);
> +	if (rc)
> +		goto chan_error;
> +	return rc;
> +chan_error:
> +	mic_dma_free_desc_ring(ch);
> +ring_error:
> +	dev_err(dev, "%s ret %d\n", __func__, rc);
> +	return rc;

some blank line will improve readablity..

> +}
> +
> +static int mic_dma_alloc_chan_resources(struct dma_chan *ch)
> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +	struct device *dev = mic_dma_ch_to_device(mic_ch);
> +	int rc = mic_dma_chan_init(mic_ch);
> +
> +	if (rc) {
> +		dev_err(dev, "%s ret %d\n", __func__, rc);
> +		return rc;
> +	}
> +	return MIC_DMA_DESC_RX_SIZE;

??

alloc is supposed to return the number of descriptors allocated, you are not
doing that, and returning size? Why do you need to implement this

> +static enum dma_status
> +mic_dma_tx_status(struct dma_chan *ch, dma_cookie_t cookie,
> +		  struct dma_tx_state *txstate)
> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +
> +	if (dma_cookie_status(ch, cookie, txstate) != DMA_COMPLETE)
> +		mic_dma_cleanup(mic_ch);

why do you want to do cleanup if it is not completed?

> +
> +	return dma_cookie_status(ch, cookie, txstate);
> +}
> +
> +static int mic_dma_ring_count(u32 head, u32 tail)
> +{
> +	int count;
> +
> +	if (head >= tail)
> +		count = (tail - 0) + (MIC_DMA_DESC_RX_SIZE - head);
> +	else
> +		count = tail - head;
> +	return count - 1;
> +}
> +
> +static int mic_dma_avail_desc_ring_space(struct mic_dma_chan *ch, int required)
> +{
> +	struct device *dev = mic_dma_ch_to_device(ch);
> +	int count;
> +	unsigned long timeout = jiffies +
> +			msecs_to_jiffies(MIC_DMA_RING_TO_MS);
> +
> +	count = mic_dma_ring_count(ch->head, ch->last_tail);
> +	while (count < required) {
> +		mic_dma_cleanup(ch);
> +		count = mic_dma_ring_count(ch->head, ch->last_tail);
> +		if ((count >= required) || time_after_eq(jiffies, timeout))
> +			break;
> +		usleep_range(50, 100);
> +	}
> +
> +	if (count < required) {
> +		dev_err(dev, "%s count %d reqd %d head %d tail %d\n",
> +			__func__, count, required, ch->head, ch->last_tail);
> +		return -ENOMEM;
> +	}
> +	return count;
> +}
> +
> +static void
> +mic_dma_memcpy_desc(struct mic_dma_desc *desc, dma_addr_t src_phys,
> +		    dma_addr_t dst_phys, u64 size, int flags)
> +{
> +	u64 qw0, qw1;
> +
> +	qw0 = (src_phys & MIC_DMA_DESC_BITS_32_TO_47) << 16;
> +	qw0 |= (dst_phys & MIC_DMA_DESC_BITS_32_TO_47);
> +	qw0 |= size & MIC_DMA_DESC_SIZE_MASK;
> +	qw0 |=  MIC_DMA_DESC_VALID;
> +	if (flags & DMA_PREP_INTERRUPT)
> +		qw0 |=  MIC_DMA_DESC_INTR_ENABLE;
> +
> +	qw1 = (src_phys & MIC_DMA_DESC_LOW_MASK) << MIC_DMA_DESC_SRC_LOW_SHIFT;
> +	qw1 |= dst_phys & MIC_DMA_DESC_LOW_MASK;
> +
> +	desc->qw1 = qw1;
> +	/*
> +	 * Update QW1 before QW0 since QW0 has the valid bit and the
> +	 * descriptor might be picked up by the hardware DMA engine
> +	 * immediately if it finds a valid descriptor.
> +	 */
> +	wmb();
> +	desc->qw0 = qw0;
> +	/*
> +	 * This is not smp_wmb() on purpose since we are also publishing the
> +	 * descriptor updates to a dma device
> +	 */
> +	wmb();
> +}
> +
> +static void mic_dma_fence_intr_desc(struct mic_dma_chan *ch, int flags)
> +{
> +	/*
> +	* Zero-length DMA memcpy needs valid source and destination addresses
> +	*/

this is not accepted comment style

> +	mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> +			    ch->desc_ring_da, ch->desc_ring_da, 0, flags);
> +	mic_dma_inc_head(ch);
> +}
> +
> +static int mic_dma_prog_memcpy_desc(struct mic_dma_chan *ch, dma_addr_t src,
> +				    dma_addr_t dst, size_t len, int flags)
> +{
> +	struct device *dev = mic_dma_ch_to_device(ch);
> +	size_t current_transfer_len;
> +	size_t max_xfer_size = to_mic_dma_dev(ch)->max_xfer_size;
> +	int num_desc = len / max_xfer_size;
> +	int ret;
> +
> +	if (len % max_xfer_size)
> +		num_desc++;
> +	/*
> +	 * A single additional descriptor is programmed for fence/interrupt
> +	 * during submit(..)
> +	 */
> +	if (flags)
> +		num_desc++;
> +
> +	if (!num_desc) {
> +		dev_err(dev, "%s nothing to do: rc %d\n", __func__, -EINVAL);
> +		return -EINVAL;
> +	}
> +
> +	ret = mic_dma_avail_desc_ring_space(ch, num_desc);
> +	if (ret <= 0) {
> +		dev_err(dev, "%s desc ring full ret %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	while (len > 0) {
> +		current_transfer_len = min(len, max_xfer_size);
> +		/*
> +		 * Set flags to 0 here, we don't want memcpy descriptors to
> +		 * generate interrupts, a separate descriptor will be programmed
> +		 * for fence/interrupt during submit, after a callback is
> +		 * guaranteed to have been assigned
> +		 */
> +		mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> +				    src, dst, current_transfer_len, 0);
> +		mic_dma_inc_head(ch);
> +		len -= current_transfer_len;
> +		dst = dst + current_transfer_len;
> +		src = src + current_transfer_len;
> +	}
> +	return 0;
> +}
> +
> +static int mic_dma_do_dma(struct mic_dma_chan *ch, int flags, dma_addr_t src,
> +			  dma_addr_t dst, size_t len)
> +{
> +	return mic_dma_prog_memcpy_desc(ch, src, dst, len, flags);
> +}
> +
> +static inline void mic_dma_issue_pending(struct dma_chan *ch)
> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +	unsigned long flags;
> +	u32 ctrl_reg;
> +
> +	spin_lock_irqsave(&mic_ch->issue_lock, flags);
> +	/*
> +	 * The notify_hw is introduced because occasionally we were seeing an
> +	 * issue where the HW tail seemed stuck, i.e. the DMA engine would not
> +	 * pick up new descriptors available in the ring. We found that
> +	 * serializing operations between SW and HW made this issue
> +	 * disappear. We believe this issue arises because of a race between SW
> +	 * and HW. SW adds new descriptors to the ring and clears
> +	 * DESC_INVLD_STATUS to ask HW to pick them up. However HW has
> +	 * prefetched descriptors and writes DESC_INVLD_STATUS to let SW know it
> +	 * has fetched an invalid descriptor. It appears that in this way HW can
> +	 * overwrite the clearing of DESC_INVLD_STATUS by SW (though we never
> +	 * saw DESC_INVLD_STATUS set to 1 when the DMA engine had stopped) and
> +	 * therefore HW never picks up these descriptors.
> +	 *
> +	 * Therefore it is necessary for SW to clear DESC_INVLD_STATUS only
> +	 * after HW has set it, and the notify_hw flag accomplishes this
> +	 * between issue_pending(..) and the invld_desc interrupt handler.
> +	 */
> +	if (mic_ch->notify_hw) {
> +		ctrl_reg = mic_dma_ch_reg_read(mic_ch, MIC_DMA_CTRL_STATUS);
> +		if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
> +			mic_ch->notify_hw = 0;
> +			/* Clear DESC_INVLD_STATUS flag */
> +			mic_dma_ch_reg_write(mic_ch, MIC_DMA_CTRL_STATUS,
> +					     ctrl_reg);
> +		} else {
> +			dev_err(mic_dma_ch_to_device(mic_ch),
> +				"%s: DESC_INVLD_STATUS not set\n", __func__);
> +		}
> +	}
> +	spin_unlock_irqrestore(&mic_ch->issue_lock, flags);
> +}
> +
> +static inline void mic_dma_update_pending(struct mic_dma_chan *ch)
> +{
> +	if (mic_dma_ring_count(ch->head, ch->last_tail)
> +		> MIC_DMA_PENDING_LEVEL)
> +		mic_dma_issue_pending(&ch->chan);
> +}
> +
> +static dma_cookie_t mic_dma_tx_submit_unlock(struct dma_async_tx_descriptor *tx)

80 lines

> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(tx->chan);
> +	dma_cookie_t cookie;
> +
> +	dma_cookie_assign(tx);
> +	cookie = tx->cookie;
> +	/*
> +	 * smp_wmb pairs with smb_rmb in mic_dma_cleanup to ensure
> +	 * that tx->cookie is visible before updating ch->head
> +	 */
> +	smp_wmb();
> +
> +	/*
> +	 * The final fence/interrupt desc is now programmed in submit after the
> +	 * cookie has been assigned
> +	 */
> +	if (tx->flags) {
> +		mic_dma_fence_intr_desc(mic_ch, tx->flags);
> +		tx->flags = 0;
> +	}
> +
> +	spin_unlock(&mic_ch->prep_lock);
> +	mic_dma_update_pending(mic_ch);
> +	return cookie;
> +}
> +
> +static inline struct dma_async_tx_descriptor *
> +allocate_tx(struct mic_dma_chan *ch, unsigned long flags)
> +{
> +	/* index of the final desc which is or will be programmed */
> +	u32 idx = flags ? ch->head : mic_dma_ring_dec(ch->head);
> +	struct dma_async_tx_descriptor *tx = &ch->tx_array[idx];
> +
> +	dma_async_tx_descriptor_init(tx, &ch->chan);
> +	tx->tx_submit = mic_dma_tx_submit_unlock;
> +	tx->flags = flags;
> +	return tx;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
> +			 dma_addr_t dma_src, size_t len, unsigned long flags)
> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +	struct device *dev = mic_dma_ch_to_device(mic_ch);
> +	int result;
> +
> +	/*
> +	 * return holding prep_lock which will be released in submit
> +	 */

why do you want to do that. I have already falgged this for x100, so plese
fix it

> +	spin_lock(&mic_ch->prep_lock);
> +	result = mic_dma_do_dma(mic_ch, flags, dma_src, dma_dest, len);
> +	if (result >= 0)
> +		return allocate_tx(mic_ch, flags);
> +	dev_err(dev, "Error enqueueing dma, error=%d\n", result);
> +	spin_unlock(&mic_ch->prep_lock);
> +	return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +mic_dma_prep_interrupt_lock(struct dma_chan *ch, unsigned long flags)
> +{
> +	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> +	/*
> +	 * Program 0 length DMA. Interrupt desc to be programmed in submit.
> +	 * All DMA transfers need a valid source and destination.
> +	 */
> +	return mic_dma_prep_memcpy_lock(ch, mic_ch->desc_ring_da,
> +					mic_ch->desc_ring_da, 0, flags);
> +}
> +
> +static irqreturn_t mic_dma_thread_fn(int irq, void *data)
> +{
> +	struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
> +
> +	mic_dma_cleanup(&mic_dma_dev->mic_chan);
> +	return IRQ_HANDLED;
> +}
> +
> +static void mic_dma_invld_desc_interrupt(struct mic_dma_chan *ch)
> +{
> +	u32 ctrl_reg;
> +
> +	spin_lock(&ch->issue_lock);
> +	if (!ch->notify_hw) {
> +		ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> +		if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
> +			/*
> +			 * Check the descriptor ring for valid descriptors. If
> +			 * valid descriptors are available clear the
> +			 * DESC_INVLD_STATUS bit to signal to the hardware to
> +			 * pick them up. If no valid descriptors are available,
> +			 * when issue_pending is called, it will clear the
> +			 * INVLD_STATUS bit to signal to the hardware to pick up
> +			 * the posted descriptors.
> +			 */
> +			if (ch->desc_ring[mic_dma_ring_dec(ch->head)].qw0 &
> +				MIC_DMA_DESC_VALID) {
> +				mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS,
> +						     ctrl_reg);
> +			} else {
> +				ch->notify_hw = 1;
> +			}
> +		} else {
> +			dev_err(mic_dma_ch_to_device(ch),
> +				"%s: DESC_INVLD_STATUS not set\n", __func__);
> +		}
> +	}
> +	spin_unlock(&ch->issue_lock);
> +}
> +
> +static void mic_dma_abort_interrupt(struct mic_dma_chan *ch, u32 intr_reg)
> +{
> +	u32 reg;
> +	u32 ring_pos;
> +
> +	spin_lock(&ch->issue_lock);
> +	reg = mic_dma_ch_reg_read(ch, MIC_DMA_LAST_DESC_ADDR_LOW);
> +	ring_pos  = desc_ring_da_to_index(ch, reg);
> +
> +	if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID) {
> +		/*
> +		* Move back the HW next pointer so it points to the tail of
> +		* the ring descriptor.
> +		*/

style issue here too

> +		mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, reg);
> +	} else {
> +		/* Last descriptor is not valid, find a valid one or head */
> +		u32 tail = ch->last_tail;
> +		u32 new_next;
> +
> +		while (tail != ch->head) {
> +			tail = mic_dma_ring_inc(tail);
> +			if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID)
> +				break;
> +		}
> +		new_next = desc_ring_index_to_da(ch, tail);
> +		mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, new_next);
> +
> +		dev_err(mic_dma_ch_to_device(ch),
> +			"Abort at %d with invalid descriptor, new descr %d, head = %d, tail %d\n",
> +			ring_pos, tail, ch->head, ch->last_tail);
> +	}
> +
> +	/*
> +	 * If abort was caused by hw error then restart DMA engine.
> +	 * If abort was requested by driver then leave it stopped.
> +	 */
> +	if (intr_reg & MIC_DMA_ERROR_INTR_STATUS) {
> +		if (ch->abort_tail != desc_ring_da_to_index(ch, reg)) {
> +			ch->abort_tail = desc_ring_da_to_index(ch, reg);
> +			ch->abort_counter = 0;
> +		} else {
> +			ch->abort_counter++;
> +		}
> +
> +		if (ch->abort_counter < MIC_DMA_ABORT_LIMIT) {
> +			dev_err(mic_dma_ch_to_device(ch),
> +				"Abort/Error at %d (retry %d), reenabling DMA engine.\n",
> +				 ch->abort_tail, ch->abort_counter);
> +			mic_dma_enable_chan(ch);
> +		} else {
> +			/*
> +			 * We have tried same descriptor several times and it is
> +			 * still generating abort.
> +			 */
> +			dev_err(mic_dma_ch_to_device(ch),
> +				"Abort Error at %d (retry %d), give up.\n",
> +				ch->abort_tail, ch->abort_counter);
> +		}
> +	}
> +	spin_unlock(&ch->issue_lock);
> +}
> +
> +static u32 mic_dma_ack_interrupt(struct mic_dma_chan *ch)
> +{
> +	u32 intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +
> +	mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +	return intr_reg;
> +}
> +
> +static irqreturn_t mic_dma_intr_handler(int irq, void *data)
> +{
> +	struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device *)data);
> +	bool wake_thread = false, error = false;
> +	struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> +	struct device *dev = mic_dma_ch_to_device(ch);
> +	u32 reg;
> +
> +	reg = mic_dma_ack_interrupt(&mic_dma_dev->mic_chan);
> +	wake_thread = !!(reg & MIC_DMA_DESC_DONE_INTR_STATUS);
> +	error = !!(reg & MIC_DMA_ERROR_STATUS);
> +	if (error)
> +		dev_err(dev, "%s error status 0x%x\n", __func__, reg);
> +
> +	if (reg & MIC_DMA_INVLD_DESC_INTR_STATUS)
> +		mic_dma_invld_desc_interrupt(&mic_dma_dev->mic_chan);
> +
> +	if (reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> +		mic_dma_abort_interrupt(&mic_dma_dev->mic_chan, reg);
> +
> +	if (wake_thread)
> +		return IRQ_WAKE_THREAD;

any reason to use threads and not tasklets per dmaengine expectations. All
callback are expected to be tasklet context

> +
> +	if (error || reg & MIC_DMA_INVLD_DESC_INTR_STATUS ||
> +	    reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> +		return IRQ_HANDLED;
> +
> +	return IRQ_NONE;
> +}
> +
> +static int mic_dma_setup_irq(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct pci_dev *pdev = mic_dma_dev->pdev;
> +	struct device *dev = &pdev->dev;
> +	int rc = pci_enable_msi(pdev);
> +
> +	if (rc)
> +		pci_intx(pdev, 1);
> +
> +	return devm_request_threaded_irq(dev, pdev->irq, mic_dma_intr_handler,
> +					 mic_dma_thread_fn, IRQF_SHARED,
> +					 "mic-x200-dma-intr", mic_dma_dev);
> +}
> +
> +static inline void mic_dma_free_irq(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct pci_dev *pdev = mic_dma_dev->pdev;
> +	struct device *dev = &pdev->dev;
> +
> +	devm_free_irq(dev, pdev->irq, mic_dma_dev);
> +	if (pci_dev_msi_enabled(pdev))
> +		pci_disable_msi(pdev);
> +}
> +
> +static void mic_dma_hw_init(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> +	u32 intr_reg;
> +
> +	dev_dbg(mic_dma_dev->dma_dev.dev, "Ctrl reg 0x%0x, Intr reg 0x%0x\n",
> +		mic_dma_reg_read(mic_dma_dev, MIC_DMA_CTRL_STATUS),
> +		mic_dma_reg_read(mic_dma_dev, MIC_DMA_INTR_CTRL_STATUS));
> +
> +	intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> +	if (intr_reg & MIC_DMA_ALL_INTR_EN) {
> +		/*
> +		 * Interrupts are enabled - that means card has been reset
> +		 * without resetting DMA HW.
> +		 * Disable interrupts and issue abort to stop DMA HW.
> +		 */
> +		intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
> +		mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> +		mic_dma_disable_chan(ch);
> +	}
> +
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_CTRL_STATUS,
> +			  MIC_DMA_CTRL_STATUS_INIT);
> +
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_CHAN_BANDWIDTH,
> +			  MIC_DMA_MAX_DST_MEM_WRITE);
> +
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA_RAM_THRESH,
> +			  MIC_DMA_STA_RAM_THRESH_INIT);
> +
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_HDR_RAM,
> +			  MIC_DMA_STA_HDR_THRESH_INIT);
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_HDR_RAM,
> +			  MIC_DMA_STA_HDR_THRESH_INIT);
> +
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_PLD_RAM,
> +			  MIC_DMA_STA_PLD_THRESH_INIT);
> +	mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_PLD_RAM,
> +			  MIC_DMA_STA_PLD_THRESH_INIT);
> +}
> +
> +static int mic_dma_init(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct mic_dma_chan *ch;
> +	int rc = 0;
> +
> +	mic_dma_hw_init(mic_dma_dev);
> +
> +	rc = mic_dma_setup_irq(mic_dma_dev);
> +	if (rc) {
> +		dev_err(mic_dma_dev->dma_dev.dev,
> +			"%s %d func error line %d\n", __func__, __LINE__, rc);
> +		goto error;
> +	}
> +	ch = &mic_dma_dev->mic_chan;
> +	spin_lock_init(&ch->cleanup_lock);
> +	spin_lock_init(&ch->prep_lock);
> +	spin_lock_init(&ch->issue_lock);
> +error:
> +	return rc;
> +}
> +
> +static void mic_dma_uninit(struct mic_dma_device *mic_dma_dev)
> +{
> +	mic_dma_free_irq(mic_dma_dev);
> +}
> +
> +static int mic_register_dma_device(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct dma_device *dma_dev = &mic_dma_dev->dma_dev;
> +	struct mic_dma_chan *ch;
> +
> +	dma_cap_zero(dma_dev->cap_mask);
> +
> +	dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
> +	dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
> +
> +	dma_dev->device_alloc_chan_resources = mic_dma_alloc_chan_resources;
> +	dma_dev->device_free_chan_resources = mic_dma_free_chan_resources;
> +	dma_dev->device_tx_status = mic_dma_tx_status;
> +	dma_dev->device_prep_dma_memcpy = mic_dma_prep_memcpy_lock;
> +	dma_dev->device_prep_dma_interrupt = mic_dma_prep_interrupt_lock;
> +	dma_dev->device_issue_pending = mic_dma_issue_pending;
> +	INIT_LIST_HEAD(&dma_dev->channels);
> +
> +	/* Associate dma_dev with dma_chan */
> +	ch = &mic_dma_dev->mic_chan;
> +	ch->chan.device = dma_dev;
> +	dma_cookie_init(&ch->chan);
> +	list_add_tail(&ch->chan.device_node, &dma_dev->channels);
> +	return dma_async_device_register(dma_dev);
> +}
> +
> +static int mic_dma_probe(struct mic_dma_device *mic_dma_dev)
> +{
> +	struct dma_device *dma_dev;
> +	int rc = -EINVAL;
> +
> +	dma_dev = &mic_dma_dev->dma_dev;
> +	dma_dev->dev = &mic_dma_dev->pdev->dev;
> +
> +	/* MIC X200 has one DMA channel per function */
> +	dma_dev->chancnt = 1;
> +
> +	mic_dma_dev->max_xfer_size = MIC_DMA_DESC_SIZE_MASK;
> +
> +	rc = mic_dma_init(mic_dma_dev);
> +	if (rc) {
> +		dev_err(&mic_dma_dev->pdev->dev,
> +			"%s %d rc %d\n", __func__, __LINE__, rc);

why do you need func line, a unique print can be grepped

> +		goto init_error;
> +	}
> +
> +	rc = mic_register_dma_device(mic_dma_dev);
> +	if (rc) {
> +		dev_err(&mic_dma_dev->pdev->dev,
> +			"%s %d rc %d\n", __func__, __LINE__, rc);
> +		goto reg_error;
> +	}
> +	return rc;
> +reg_error:
> +	mic_dma_uninit(mic_dma_dev);
> +init_error:
> +	return rc;
> +}
> +
> +static void mic_dma_remove(struct mic_dma_device *mic_dma_dev)
> +{
> +	dma_async_device_unregister(&mic_dma_dev->dma_dev);
> +	mic_dma_uninit(mic_dma_dev);
> +}
> +
> +static struct mic_dma_device *
> +mic_x200_alloc_dma(struct pci_dev *pdev, void __iomem *iobase)
> +{
> +	struct mic_dma_device *d = kzalloc(sizeof(*d), GFP_KERNEL);
> +
> +	if (!d)
> +		return NULL;
> +	d->pdev = pdev;
> +	d->reg_base = iobase;
> +	return d;
> +}
> +
> +static int
> +mic_x200_dma_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
> +{
> +	int rc;
> +	struct mic_dma_device *dma_dev;
> +	void __iomem * const *iomap;
> +
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		goto done;
> +	rc = pcim_iomap_regions(pdev, 0x1, MIC_DMA_DRV_NAME);
> +	if (rc)
> +		goto done;
> +	iomap = pcim_iomap_table(pdev);
> +	if (!iomap) {
> +		rc = -ENOMEM;
> +		goto done;
> +	}
> +	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
> +	if (rc)
> +		goto done;
> +	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
> +	if (rc)
> +		goto done;
> +	dma_dev = mic_x200_alloc_dma(pdev, iomap[0]);
> +	if (!dma_dev) {
> +		rc = -ENOMEM;
> +		goto done;
> +	}
> +	pci_set_master(pdev);
> +	pci_set_drvdata(pdev, dma_dev);
> +	rc = mic_dma_probe(dma_dev);
> +	if (rc)
> +		goto probe_err;
> +	return 0;
> +probe_err:
> +	kfree(dma_dev);
> +done:
> +	dev_err(&pdev->dev, "probe error rc %d\n", rc);
> +	return rc;
> +}
> +
> +static void  mic_x200_dma_remove(struct pci_dev *pdev)
> +{
> +	struct mic_dma_device *dma_dev = pci_get_drvdata(pdev);
> +
> +	mic_dma_remove(dma_dev);
> +	kfree(dma_dev);
> +}
> +
> +static struct pci_device_id mic_x200_dma_pci_id_table[] = {
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2264)},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(pci, mic_x200_dma_pci_id_table);
> +
> +static struct pci_driver mic_x200_dma_driver = {
> +	.name     = MIC_DMA_DRV_NAME,
> +	.id_table = mic_x200_dma_pci_id_table,
> +	.probe    = mic_x200_dma_probe,
> +	.remove   = mic_x200_dma_remove
> +};
> +
> +static int __init mic_x200_dma_init(void)
> +{
> +	int rc = pci_register_driver(&mic_x200_dma_driver);
> +
> +	if (rc)
> +		pr_err("%s %d rc %x\n", __func__, __LINE__, rc);
> +	return rc;
> +}
> +
> +static void __exit mic_x200_dma_exit(void)
> +{
> +	pci_unregister_driver(&mic_x200_dma_driver);
> +}
> +
> +module_init(mic_x200_dma_init);
> +module_exit(mic_x200_dma_exit);

module_pci_driver ?

-- 
~Vinod

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

* RE: [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver
  2016-02-04 17:50   ` Andy Shevchenko
@ 2016-02-09 12:25     ` Lawrynowicz, Jacek
  0 siblings, 0 replies; 6+ messages in thread
From: Lawrynowicz, Jacek @ 2016-02-09 12:25 UTC (permalink / raw)
  To: Andy Shevchenko, Koul, Vinod
  Cc: Williams, Dan J, dmaengine, linux-kernel, Dixit, Ashutosh, Dutt,
	Sudeep, Kacprowski, Andrzej

Hi Andy, Vinod,

Thanks for your reviews. I will go with Andy's suggestion and rewrite 
the driver using virt-dma api. This will also solve the locking problem that 
Vinod has pointed out. I will post the next version after the rewrite and 
it will also include fixes for the rest of your comments.

Regarding x100_dma reuse: it would be really hard because x200 is not an 
evolution of x100, it's a different HW. X200 has its own PCIID, has different
MMIO registers, descriptor format and so on.

Regards,
Jacek

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Thursday, February 4, 2016 6:51 PM
> To: Lawrynowicz, Jacek <jacek.lawrynowicz@intel.com>
> Cc: Williams, Dan J <dan.j.williams@intel.com>; Koul, Vinod
> <vinod.koul@intel.com>; dmaengine <dmaengine@vger.kernel.org>; linux-
> kernel@vger.kernel.org; Dixit, Ashutosh <ashutosh.dixit@intel.com>; Dutt,
> Sudeep <sudeep.dutt@intel.com>; Kacprowski, Andrzej
> <Andrzej.Kacprowski@intel.com>
> Subject: Re: [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA
> driver
> 
> On Thu, Feb 4, 2016 at 5:10 PM, Jacek Lawrynowicz
> <jacek.lawrynowicz@intel.com> wrote:
> > This patch implements the DMA Engine driver for the DMA controller on
> > MIC X200 Coprocessors which are PCIe cards running Linux. Separate but
> > identical DMA channels are available for the host and card. DMA
> > transfers can be done from both the host and card in host<->card and
> > host<->host directions.
> >
> 
> Can it share code with 100 series?
> 
> > Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> > Signed-off-by: Sudeep Dutt <sudeep.dutt@intel.com>
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@intel.com>
> > ---
> >  drivers/dma/Kconfig        |   18 +
> >  drivers/dma/Makefile       |    1 +
> >  drivers/dma/mic_x200_dma.c | 1114
> ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 1133 insertions(+)
> >  create mode 100644 drivers/dma/mic_x200_dma.c
> >
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index 79b1390..a3ea8b9 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -277,6 +277,24 @@ config INTEL_MIC_X100_DMA
> >           OS and tools for MIC to use with this driver are available from
> >           <http://software.intel.com/en-us/mic-developer>.
> >
> > +config INTEL_MIC_X200_DMA
> > +       tristate "Intel MIC X200 DMA Driver"
> > +        depends on 64BIT && X86 && PCI
> 
> Doesn't it the same like
>         depends on X86_64 && PCI
> ?
> 
> > +       select DMAENGINE
> > +       help
> > +         This enables DMA support for the Intel Many Integrated Core
> > +         (MIC) family of PCIe form factor coprocessor X200 devices that
> > +         run a 64 bit Linux OS. This driver will be used by both MIC
> > +         host and card drivers.
> > +
> > +         If you are building host kernel with a MIC device or a card
> > +         kernel for a MIC device, then say M (recommended) or Y, else
> > +         say N. If unsure say N.
> > +
> > +         More information about the Intel MIC family as well as the Linux
> > +         OS and tools for MIC to use with this driver are available from
> > +         <http://software.intel.com/en-us/mic-developer>.
> > +
> >  config K3_DMA
> >         tristate "Hisilicon K3 DMA support"
> >         depends on ARCH_HI3xxx
> > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > index 2dd0a067..a3756f5 100644
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
> >  obj-$(CONFIG_INTEL_IOATDMA) += ioat/
> >  obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
> >  obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
> > +obj-$(CONFIG_INTEL_MIC_X200_DMA) += mic_x200_dma.o
> >  obj-$(CONFIG_K3_DMA) += k3dma.o
> >  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
> >  obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
> > diff --git a/drivers/dma/mic_x200_dma.c b/drivers/dma/mic_x200_dma.c
> > new file mode 100644
> > index 0000000..80c4959
> > --- /dev/null
> > +++ b/drivers/dma/mic_x200_dma.c
> > @@ -0,0 +1,1114 @@
> > +/*
> > + * Intel MIC Platform Software Stack (MPSS)
> > + *
> > + * Copyright(c) 2016 Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License, version 2, as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + * The full GNU General Public License is included in this distribution in
> > + * the file called "COPYING".
> > + *
> > + * Intel MIC X200 DMA driver
> > + */
> > +#include <linux/module.h>
> > +#include <linux/delay.h>
> > +#include <linux/pci.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/vmalloc.h>
> > +
> > +#include "dmaengine.h"
> > +
> > +#define MIC_DMA_DRV_NAME       "mic_x200_dma"
> > +
> > +#define MIC_DMA_DESC_RX_SIZE   (128 * 1024)
> > +#define MIC_DMA_ABORT_TO_MS    3000
> > +#define MIC_DMA_RING_TO_MS     20
> > +#define MIC_DMA_PENDING_LEVEL  16
> > +#define MIC_DMA_ABORT_LIMIT    5
> > +
> > +/* DMA Descriptor related flags */
> > +#define MIC_DMA_DESC_VALID             BIT(31)
> > +#define MIC_DMA_DESC_INTR_ENABLE       BIT(30)
> > +#define MIC_DMA_DESC_SRC_LINK_ERR      BIT(29)
> > +#define MIC_DMA_DESC_DST_LINK_ERR      BIT(28)
> > +#define MIC_DMA_DESC_LOW_MASK          (0xffffffffUL)
> > +#define MIC_DMA_DESC_SRC_LOW_SHIFT     32
> > +#define MIC_DMA_DESC_BITS_32_TO_47     (0xffffUL << 32)
> > +#define MIC_DMA_DESC_SIZE_MASK         ((1UL << 27) - 1)
> > +
> > +#define MIC_DMA_DESC_RING_ADDR_LOW             0x214
> > +#define MIC_DMA_DESC_RING_ADDR_HIGH            0x218
> > +#define MIC_DMA_NEXT_DESC_ADDR_LOW             0x21C
> > +#define MIC_DMA_DESC_RING_SIZE                 0x220
> > +#define MIC_DMA_LAST_DESC_ADDR_LOW             0x224
> > +#define MIC_DMA_LAST_DESC_XFER_SIZE            0x228
> > +
> > +#define MIC_DMA_CTRL_STATUS                    0x238
> > +#define MIC_DMA_CTRL_PAUSE                     BIT(0)
> > +#define MIC_DMA_CTRL_ABORT                     BIT(1)
> > +#define MIC_DMA_CTRL_WB_ENABLE                 BIT(2)
> > +#define MIC_DMA_CTRL_START                     BIT(3)
> > +#define MIC_DMA_CTRL_RING_STOP                 BIT(4)
> > +#define MIC_DMA_CTRL_ONCHIP_MODE               BIT(5)
> > +#define MIC_DMA_CTRL_OFFCHIP_MODE              (2UL << 5)
> > +#define MIC_DMA_CTRL_DESC_INVLD_STATUS         BIT(8)
> > +#define MIC_DMA_CTRL_PAUSE_DONE_STATUS         BIT(9)
> > +#define MIC_DMA_CTRL_ABORT_DONE_STATUS         BIT(10)
> > +#define MIC_DMA_CTRL_FACTORY_TEST              BIT(11)
> > +#define MIC_DMA_CTRL_IMM_PAUSE_DONE_STATUS     BIT(12)
> > +#define MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE     (3UL << 16)
> > +#define MIC_DMA_CTRL_RELAXED_DATA_WRITE                BIT(25)
> > +#define MIC_DMA_CTRL_NO_SNOOP_DESC_READ                BIT(26)
> > +#define MIC_DMA_CTRL_NO_SNOOP_DATA_READ                BIT(27)
> > +#define MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE       BIT(28)
> > +#define MIC_DMA_CTRL_IN_PROGRESS               BIT(30)
> > +#define MIC_DMA_CTRL_HEADER_LOG                        BIT(31)
> > +#define MIC_DMA_CTRL_MODE_BITS                 (3UL << 5)
> > +#define MIC_DMA_CTRL_STATUS_BITS               (0x1FUL << 8)
> > +#define MIC_DMA_CTRL_STATUS_INIT
> (MIC_DMA_CTRL_SRC_MAX_TRANSFER_SIZE |        \
> > +                                  MIC_DMA_CTRL_RELAXED_DATA_WRITE |    \
> > +                                  MIC_DMA_CTRL_NO_SNOOP_DESC_READ |    \
> > +                                  MIC_DMA_CTRL_NO_SNOOP_DATA_READ |    \
> > +                                  MIC_DMA_CTRL_NO_SNOOP_DESC_WRITE)
> > +
> > +#define MIC_DMA_INTR_CTRL_STATUS               0x23C
> > +#define MIC_DMA_ERROR_INTR_EN                  BIT(0)
> > +#define MIC_DMA_INVLD_DESC_INTR_EN             BIT(1)
> > +#define MIC_DMA_ABORT_DONE_INTR_EN             BIT(3)
> > +#define MIC_DMA_GRACE_PAUSE_INTR_EN            BIT(4)
> > +#define MIC_DMA_IMM_PAUSE_INTR_EN              BIT(5)
> > +#define MIC_DMA_IRQ_PIN_INTR_EN                        BIT(15)
> > +#define MIC_DMA_ERROR_INTR_STATUS              BIT(16)
> > +#define MIC_DMA_INVLD_DESC_INTR_STATUS         BIT(17)
> > +#define MIC_DMA_DESC_DONE_INTR_STATUS          BIT(18)
> > +#define MIC_DMA_ABORT_DONE_INTR_STATUS         BIT(19)
> > +#define MIC_DMA_GRACE_PAUSE_INTR_STATUS                BIT(20)
> > +#define MIC_DMA_IMM_PAUSE_INTR_STATUS          BIT(21)
> > +#define MIC_DMA_ALL_INTR_EN    (MIC_DMA_ERROR_INTR_EN |                \
> > +                                MIC_DMA_INVLD_DESC_INTR_EN |           \
> > +                                MIC_DMA_ABORT_DONE_INTR_EN |           \
> > +                                MIC_DMA_GRACE_PAUSE_INTR_EN |          \
> > +                                MIC_DMA_IMM_PAUSE_INTR_EN)
> > +
> > +#define MIC_DMA_ERROR_STATUS   (MIC_DMA_ERROR_INTR_STATUS |
> \
> > +                                MIC_DMA_ABORT_DONE_INTR_STATUS |       \
> > +                                MIC_DMA_GRACE_PAUSE_INTR_STATUS |      \
> > +                                MIC_DMA_IMM_PAUSE_INTR_STATUS)
> > +
> > +#define MIC_DMA_ALL_INTR_STATUS        (MIC_DMA_ERROR_INTR_STATUS
> |            \
> > +                                MIC_DMA_INVLD_DESC_INTR_STATUS |       \
> > +                                MIC_DMA_DESC_DONE_INTR_STATUS |        \
> > +                                MIC_DMA_ABORT_DONE_INTR_STATUS |       \
> > +                                MIC_DMA_GRACE_PAUSE_INTR_STATUS |      \
> > +                                MIC_DMA_IMM_PAUSE_INTR_STATUS)
> > +/*
> > + * Performance tuning registers. For explanation of the values
> > + * used please refer to mic x200 dma hardware specification.
> > + */
> > +#define MIC_DMA_CHAN_BANDWIDTH                 0x22C
> > +#define MIC_DMA_MAX_DST_MEM_WRITE              (2 << 24)
> > +
> > +#define MIC_DMA_STA_RAM_THRESH                 0x258
> > +#define MIC_DMA_HDR_RAM_USAGE_THRESH           46UL
> > +#define MIC_DMA_PLD_RAM_USAGE_THRESH           (61UL << 16)
> > +#define MIC_DMA_STA_RAM_THRESH_INIT
> (MIC_DMA_HDR_RAM_USAGE_THRESH |    \
> > +                                    MIC_DMA_PLD_RAM_USAGE_THRESH)
> > +
> > +#define MIC_DMA_STA0_HDR_RAM                   0x25C
> > +#define MIC_DMA_STA1_HDR_RAM                   0x2A4
> > +#define MIC_DMA_HDR_RAM_UPPER_THRESH           160UL
> > +#define MIC_DMA_HDR_RAM_LOWER_THRESH           (152UL << 16)
> > +#define MIC_DMA_STA_HDR_THRESH_INIT
> (MIC_DMA_HDR_RAM_UPPER_THRESH |    \
> > +                                    MIC_DMA_HDR_RAM_LOWER_THRESH)
> > +
> > +#define MIC_DMA_STA0_PLD_RAM                   0x260
> > +#define MIC_DMA_STA1_PLD_RAM                   0x2A8
> > +#define MIC_DMA_PLD_RAM_UPPER_THRESH           256UL
> > +#define MIC_DMA_PLD_RAM_LOWER_THRESH           (248UL << 16)
> > +#define MIC_DMA_STA_PLD_THRESH_INIT
> (MIC_DMA_PLD_RAM_UPPER_THRESH |    \
> > +                                    MIC_DMA_PLD_RAM_LOWER_THRESH)
> > +
> > +/* HW DMA descriptor */
> > +struct mic_dma_desc {
> > +       u64 qw0;
> > +       u64 qw1;
> > +};
> > +
> > +/*
> > + * mic_dma_chan - MIC X200 DMA channel specific data structures
> > + *
> > + * @ch_num: channel number
> > + * @last_tail: cached value of descriptor ring tail
> > + * @head: index of next descriptor in desc_ring
> > + * @chan: dma engine api channel
> > + * @desc_ring: dma descriptor ring
> > + * @desc_ring_da: DMA address of desc_ring
> > + * @tx_array: array of async_tx
> > + * @cleanup_lock: used to synchronize descriptor cleanup
> > + * @prep_lock: lock held in prep_memcpy & released in tx_submit
> > + * @issue_lock: lock used to synchronize writes to head
> > + * @notify_hw: flag used to notify HW that new descriptors are available
> > + * @abort_tail: descriptor ring position at which last abort occurred
> > + * @abort_counter: number of aborts at the last position in desc ring
> > + */
> > +struct mic_dma_chan {
> > +       int ch_num;
> > +       u32 last_tail;
> > +       u32 head;
> 
> > +       struct dma_chan chan;
> > +       struct mic_dma_desc *desc_ring;
> > +       dma_addr_t desc_ring_da;
> > +       struct dma_async_tx_descriptor *tx_array;
> 
> Btw, can you use virt-chan API?
> 
> > +       spinlock_t cleanup_lock;
> > +       spinlock_t prep_lock;
> > +       spinlock_t issue_lock;
> > +       bool notify_hw;
> > +       u32 abort_tail;
> > +       u32 abort_counter;
> > +};
> > +
> > +/*
> > + * mic_dma_device - Per MIC X200 DMA device driver specific data structures
> > + *
> > + * @pdev: PCIe device
> > + * @dma_dev: underlying dma device
> > + * @reg_base: virtual address of the mmio space
> > + * @mic_chan: Array of MIC X200 DMA channels
> > + * @max_xfer_size: maximum transfer size per dma descriptor
> > + */
> > +struct mic_dma_device {
> > +       struct pci_dev *pdev;
> > +       struct dma_device dma_dev;
> > +       void __iomem *reg_base;
> > +       struct mic_dma_chan mic_chan;
> > +       size_t max_xfer_size;
> > +};
> > +
> > +static inline struct mic_dma_device *to_mic_dma_dev(struct mic_dma_chan
> *ch)
> > +{
> > +       return
> > +       container_of((const typeof(((struct mic_dma_device *)0)->mic_chan) *)
> > +                        (ch - ch->ch_num), struct mic_dma_device, mic_chan);
> > +}
> > +
> > +static inline struct mic_dma_chan *to_mic_dma_chan(struct dma_chan *ch)
> > +{
> > +       return container_of(ch, struct mic_dma_chan, chan);
> > +}
> > +
> > +static inline struct device *mic_dma_ch_to_device(struct mic_dma_chan *ch)
> > +{
> > +       return to_mic_dma_dev(ch)->dma_dev.dev;
> > +}
> > +
> > +static inline u32 mic_dma_reg_read(struct mic_dma_device *pdma, u32
> offset)
> > +{
> > +       return ioread32(pdma->reg_base + offset);
> > +}
> > +
> > +static inline void mic_dma_reg_write(struct mic_dma_device *pdma,
> > +                                    u32 offset, u32 value)
> > +{
> > +       iowrite32(value, pdma->reg_base + offset);
> > +}
> > +
> > +static inline u32 mic_dma_ch_reg_read(struct mic_dma_chan *ch, u32 offset)
> > +{
> > +       return mic_dma_reg_read(to_mic_dma_dev(ch), offset);
> > +}
> > +
> > +static inline void mic_dma_ch_reg_write(struct mic_dma_chan *ch,
> > +                                       u32 offset, u32 value)
> > +{
> > +       mic_dma_reg_write(to_mic_dma_dev(ch), offset, value);
> > +}
> > +
> > +static inline u32 mic_dma_ring_inc(u32 val)
> > +{
> > +       return (val + 1) & (MIC_DMA_DESC_RX_SIZE - 1);
> 
>     return (val + 1) % MIC_DMA_DESC_RX_SIZE;
> 
> More flexible, if the size is power of 2 it will be optimized by
> compiler to the same you had previously.
> 
> > +}
> > +
> > +static inline u32 mic_dma_ring_dec(u32 val)
> > +{
> > +       return val ? val - 1 : MIC_DMA_DESC_RX_SIZE - 1;
> 
>    return (val - 1) % MIC_DMA_DESC_RX_SIZE;
> 
> Would it be issues with that?
> 
> > +}
> > +
> > +static inline void mic_dma_inc_head(struct mic_dma_chan *ch)
> > +{
> > +       ch->head = mic_dma_ring_inc(ch->head);
> > +}
> > +
> > +static int mic_dma_alloc_desc_ring(struct mic_dma_chan *ch)
> > +{
> > +       /* Desc size must be >= depth of ring + prefetch size + 1 */
> > +       u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> > +       struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> > +
> > +       ch->desc_ring = dma_alloc_coherent(dev, desc_ring_size,
> > +                                          &ch->desc_ring_da,
> > +                                          GFP_KERNEL | __GFP_ZERO);
> > +       if (!ch->desc_ring)
> > +               return -ENOMEM;
> > +
> > +       dev_dbg(dev, "DMA desc ring VA = %p DA 0x%llx size 0x%llx\n",
> > +               ch->desc_ring, ch->desc_ring_da, desc_ring_size);
> 
> %pad for DMA addresses.
> 
> > +       ch->tx_array = vzalloc(MIC_DMA_DESC_RX_SIZE * sizeof(*ch->tx_array));
> 
> > +       if (!ch->tx_array)
> > +               goto tx_error;
> > +       return 0;
> 
> > +tx_error:
> > +       dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch-
> >desc_ring_da);
> > +       return -ENOMEM;
> 
> Only one user, may be you can move it there?
> 
> > +}
> > +
> > +static inline void mic_dma_disable_chan(struct mic_dma_chan *ch)
> > +{
> > +       struct device *dev = mic_dma_ch_to_device(ch);
> > +       u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> 
> > +       int i;
> 
> unsigned int i;
> 
> > +
> > +       /* set abort bit and clear start bit */
> > +       ctrl_reg |= MIC_DMA_CTRL_ABORT;
> > +       ctrl_reg &= ~MIC_DMA_CTRL_START;
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> > +
> > +       for (i = 0; i < MIC_DMA_ABORT_TO_MS; i++) {
> > +               ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> > +
> > +               if (ctrl_reg & MIC_DMA_CTRL_ABORT_DONE_STATUS)
> > +                       return;
> > +
> > +               mdelay(1);
> > +       }
> > +       dev_err(dev, "%s abort timed out 0x%x head %d tail %d\n",
> > +               __func__, ctrl_reg, ch->head, ch->last_tail);
> > +}
> > +
> > +static inline void mic_dma_enable_chan(struct mic_dma_chan *ch)
> > +{
> > +       u32 ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> > +
> > +       ctrl_reg |= MIC_DMA_CTRL_WB_ENABLE;
> > +       /* Clear any active status bits ([31,12:8]) */
> > +       ctrl_reg |= MIC_DMA_CTRL_HEADER_LOG |
> MIC_DMA_CTRL_STATUS_BITS;
> > +       ctrl_reg &= ~MIC_DMA_CTRL_MODE_BITS;
> > +       /* Enable SGL off-chip mode */
> > +       ctrl_reg |= MIC_DMA_CTRL_OFFCHIP_MODE;
> > +       /* Set start and clear abort and abort done bits */
> > +       ctrl_reg |= MIC_DMA_CTRL_START;
> > +       ctrl_reg &= ~MIC_DMA_CTRL_ABORT;
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS, ctrl_reg);
> > +}
> > +
> > +static inline void mic_dma_chan_mask_intr(struct mic_dma_chan *ch)
> > +{
> > +       u32 intr_reg = mic_dma_ch_reg_read(ch,
> MIC_DMA_INTR_CTRL_STATUS);
> > +
> > +       intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
> 
> Redundant parens.
> 
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> > +}
> > +
> > +static inline void mic_dma_chan_unmask_intr(struct mic_dma_chan *ch)
> > +{
> > +       u32 intr_reg = mic_dma_ch_reg_read(ch,
> MIC_DMA_INTR_CTRL_STATUS);
> > +
> > +       intr_reg |= (MIC_DMA_ALL_INTR_EN);
> 
> Ditto
> 
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> > +}
> > +
> > +static void mic_dma_chan_set_desc_ring(struct mic_dma_chan *ch)
> > +{
> > +       /* Set up the descriptor ring base address and size */
> 
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_LOW,
> > +                            ch->desc_ring_da & 0xffffffff);
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_ADDR_HIGH,
> > +                            (ch->desc_ring_da >> 32) & 0xffffffff);
> 
> Better to provide mic_dma_ch_reg_writeq() based on lo_hi_writel().
> 
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_DESC_RING_SIZE,
> MIC_DMA_DESC_RX_SIZE);
> 
> > +       /* Set up the next descriptor to the base of the descriptor ring */
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW,
> > +                            ch->desc_ring_da & 0xffffffff);
> 
> Only low?
> 
> > +}
> > +
> > +static void mic_dma_cleanup(struct mic_dma_chan *ch)
> > +{
> > +       struct dma_async_tx_descriptor *tx;
> > +       u32 last_tail;
> > +       u32 cached_head;
> > +
> > +       spin_lock(&ch->cleanup_lock);
> > +       /*
> > +        * Use a cached head rather than using ch->head directly, since a
> > +        * different thread can be updating ch->head potentially leading to an
> > +        * infinite loop below.
> > +        */
> > +       cached_head = ACCESS_ONCE(ch->head);
> > +       /*
> > +        * This is the barrier pair for smp_wmb() in fn.
> > +        * mic_dma_tx_submit_unlock. It's required so that we read the
> > +        * updated cookie value from tx->cookie.
> > +        */
> > +       smp_rmb();
> > +
> > +       for (last_tail = ch->last_tail; last_tail != cached_head;) {
> > +               /* Break out of the loop if this desc is not yet complete */
> > +               if (ch->desc_ring[last_tail].qw0 & MIC_DMA_DESC_VALID)
> > +                       break;
> > +
> > +               tx = &ch->tx_array[last_tail];
> > +               if (tx->cookie) {
> > +                       dma_cookie_complete(tx);
> > +                       if (tx->callback) {
> > +                               tx->callback(tx->callback_param);
> > +                               tx->callback = NULL;
> > +                       }
> > +               }
> > +               last_tail = mic_dma_ring_inc(last_tail);
> > +       }
> > +       /* finish all completion callbacks before incrementing tail */
> > +       smp_mb();
> > +       ch->last_tail = last_tail;
> > +       spin_unlock(&ch->cleanup_lock);
> > +}
> > +
> > +static int mic_dma_chan_setup(struct mic_dma_chan *ch)
> > +{
> > +       mic_dma_chan_set_desc_ring(ch);
> > +       ch->last_tail = 0;
> > +       ch->head = 0;
> > +       ch->notify_hw = 1;
> 
> > +       mic_dma_chan_unmask_intr(ch);
> > +       mic_dma_enable_chan(ch);
> 
> Seems alogical. Perhaps first is to enable channel and then enable interrupts?
> 
> > +       return 0;
> > +}
> > +
> > +static void mic_dma_free_desc_ring(struct mic_dma_chan *ch)
> > +{
> > +       u64 desc_ring_size = MIC_DMA_DESC_RX_SIZE * sizeof(*ch->desc_ring);
> > +       struct device *dev = to_mic_dma_dev(ch)->dma_dev.dev;
> > +
> > +       vfree(ch->tx_array);
> > +       dma_free_coherent(dev, desc_ring_size, ch->desc_ring, ch-
> >desc_ring_da);
> > +       ch->desc_ring = NULL;
> > +       ch->desc_ring_da = 0;
> > +
> > +       /* Reset description ring registers */
> > +       mic_dma_chan_set_desc_ring(ch);
> > +}
> > +
> > +static int mic_dma_chan_init(struct mic_dma_chan *ch)
> > +{
> > +       struct device *dev = mic_dma_ch_to_device(ch);
> > +       int rc;
> > +
> > +       rc = mic_dma_alloc_desc_ring(ch);
> > +       if (rc)
> > +               goto ring_error;
> > +       rc = mic_dma_chan_setup(ch);
> > +       if (rc)
> > +               goto chan_error;
> > +       return rc;
> > +chan_error:
> > +       mic_dma_free_desc_ring(ch);
> 
> > +ring_error:
> > +       dev_err(dev, "%s ret %d\n", __func__, rc);
> 
> Is it useful?
> 
> > +       return rc;
> > +}
> > +
> > +static int mic_dma_alloc_chan_resources(struct dma_chan *ch)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +       struct device *dev = mic_dma_ch_to_device(mic_ch);
> > +       int rc = mic_dma_chan_init(mic_ch);
> > +
> > +       if (rc) {
> > +               dev_err(dev, "%s ret %d\n", __func__, rc);
> 
> Ditto.
> 
> > +               return rc;
> > +       }
> > +       return MIC_DMA_DESC_RX_SIZE;
> > +}
> > +
> > +static inline u32 desc_ring_da_to_index(struct mic_dma_chan *ch, u32 descr)
> > +{
> > +       dma_addr_t last_dma_off = descr - (ch->desc_ring_da & ULONG_MAX);
> 
> Hmm… Why do you need that strange masking?
> 
> > +
> > +       return (last_dma_off / sizeof(struct mic_dma_desc)) &
> > +               (MIC_DMA_DESC_RX_SIZE - 1);
> 
> % _RX_SIZE
> 
> > +}
> > +
> > +static inline u32 desc_ring_index_to_da(struct mic_dma_chan *ch, u32 index)
> > +{
> > +       dma_addr_t addr = ch->desc_ring_da +
> > +                       sizeof(struct mic_dma_desc) * index;
> > +
> > +       return (addr & ULONG_MAX);
> > +}
> > +
> > +static void mic_dma_free_chan_resources(struct dma_chan *ch)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +
> > +       mic_dma_disable_chan(mic_ch);
> > +       mic_dma_chan_mask_intr(mic_ch);
> > +       mic_dma_cleanup(mic_ch);
> > +       mic_dma_free_desc_ring(mic_ch);
> > +}
> > +
> > +static enum dma_status
> > +mic_dma_tx_status(struct dma_chan *ch, dma_cookie_t cookie,
> > +                 struct dma_tx_state *txstate)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +
> > +       if (dma_cookie_status(ch, cookie, txstate) != DMA_COMPLETE)
> > +               mic_dma_cleanup(mic_ch);
> > +
> > +       return dma_cookie_status(ch, cookie, txstate);
> > +}
> > +
> > +static int mic_dma_ring_count(u32 head, u32 tail)
> > +{
> > +       int count;
> > +
> > +       if (head >= tail)
> > +               count = (tail - 0) + (MIC_DMA_DESC_RX_SIZE - head);
> > +       else
> > +               count = tail - head;
> > +       return count - 1;
> > +}
> > +
> > +static int mic_dma_avail_desc_ring_space(struct mic_dma_chan *ch, int
> required)
> > +{
> > +       struct device *dev = mic_dma_ch_to_device(ch);
> > +       int count;
> > +       unsigned long timeout = jiffies +
> > +                       msecs_to_jiffies(MIC_DMA_RING_TO_MS);
> > +
> > +       count = mic_dma_ring_count(ch->head, ch->last_tail);
> > +       while (count < required) {
> > +               mic_dma_cleanup(ch);
> > +               count = mic_dma_ring_count(ch->head, ch->last_tail);
> > +               if ((count >= required) || time_after_eq(jiffies, timeout))
> > +                       break;
> > +               usleep_range(50, 100);
> > +       }
> > +
> > +       if (count < required) {
> > +               dev_err(dev, "%s count %d reqd %d head %d tail %d\n",
> > +                       __func__, count, required, ch->head, ch->last_tail);
> > +               return -ENOMEM;
> > +       }
> > +       return count;
> > +}
> > +
> > +static void
> > +mic_dma_memcpy_desc(struct mic_dma_desc *desc, dma_addr_t src_phys,
> > +                   dma_addr_t dst_phys, u64 size, int flags)
> > +{
> > +       u64 qw0, qw1;
> > +
> > +       qw0 = (src_phys & MIC_DMA_DESC_BITS_32_TO_47) << 16;
> > +       qw0 |= (dst_phys & MIC_DMA_DESC_BITS_32_TO_47);
> > +       qw0 |= size & MIC_DMA_DESC_SIZE_MASK;
> > +       qw0 |=  MIC_DMA_DESC_VALID;
> > +       if (flags & DMA_PREP_INTERRUPT)
> > +               qw0 |=  MIC_DMA_DESC_INTR_ENABLE;
> > +
> > +       qw1 = (src_phys & MIC_DMA_DESC_LOW_MASK) <<
> MIC_DMA_DESC_SRC_LOW_SHIFT;
> > +       qw1 |= dst_phys & MIC_DMA_DESC_LOW_MASK;
> > +
> > +       desc->qw1 = qw1;
> > +       /*
> > +        * Update QW1 before QW0 since QW0 has the valid bit and the
> > +        * descriptor might be picked up by the hardware DMA engine
> > +        * immediately if it finds a valid descriptor.
> > +        */
> > +       wmb();
> > +       desc->qw0 = qw0;
> > +       /*
> > +        * This is not smp_wmb() on purpose since we are also publishing the
> > +        * descriptor updates to a dma device
> > +        */
> > +       wmb();
> > +}
> > +
> > +static void mic_dma_fence_intr_desc(struct mic_dma_chan *ch, int flags)
> > +{
> > +       /*
> > +       * Zero-length DMA memcpy needs valid source and destination addresses
> > +       */
> > +       mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> > +                           ch->desc_ring_da, ch->desc_ring_da, 0, flags);
> > +       mic_dma_inc_head(ch);
> > +}
> > +
> > +static int mic_dma_prog_memcpy_desc(struct mic_dma_chan *ch,
> dma_addr_t src,
> > +                                   dma_addr_t dst, size_t len, int flags)
> > +{
> > +       struct device *dev = mic_dma_ch_to_device(ch);
> > +       size_t current_transfer_len;
> > +       size_t max_xfer_size = to_mic_dma_dev(ch)->max_xfer_size;
> > +       int num_desc = len / max_xfer_size;
> > +       int ret;
> > +
> > +       if (len % max_xfer_size)
> > +               num_desc++;
> > +       /*
> > +        * A single additional descriptor is programmed for fence/interrupt
> > +        * during submit(..)
> > +        */
> > +       if (flags)
> > +               num_desc++;
> > +
> > +       if (!num_desc) {
> > +               dev_err(dev, "%s nothing to do: rc %d\n", __func__, -EINVAL);
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = mic_dma_avail_desc_ring_space(ch, num_desc);
> > +       if (ret <= 0) {
> > +               dev_err(dev, "%s desc ring full ret %d\n", __func__, ret);
> > +               return ret;
> > +       }
> > +
> > +       while (len > 0) {
> > +               current_transfer_len = min(len, max_xfer_size);
> > +               /*
> > +                * Set flags to 0 here, we don't want memcpy descriptors to
> > +                * generate interrupts, a separate descriptor will be programmed
> > +                * for fence/interrupt during submit, after a callback is
> > +                * guaranteed to have been assigned
> > +                */
> > +               mic_dma_memcpy_desc(&ch->desc_ring[ch->head],
> > +                                   src, dst, current_transfer_len, 0);
> > +               mic_dma_inc_head(ch);
> > +               len -= current_transfer_len;
> > +               dst = dst + current_transfer_len;
> > +               src = src + current_transfer_len;
> > +       }
> > +       return 0;
> > +}
> > +
> > +static int mic_dma_do_dma(struct mic_dma_chan *ch, int flags, dma_addr_t
> src,
> > +                         dma_addr_t dst, size_t len)
> > +{
> > +       return mic_dma_prog_memcpy_desc(ch, src, dst, len, flags);
> > +}
> > +
> > +static inline void mic_dma_issue_pending(struct dma_chan *ch)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +       unsigned long flags;
> > +       u32 ctrl_reg;
> > +
> > +       spin_lock_irqsave(&mic_ch->issue_lock, flags);
> > +       /*
> > +        * The notify_hw is introduced because occasionally we were seeing an
> > +        * issue where the HW tail seemed stuck, i.e. the DMA engine would not
> > +        * pick up new descriptors available in the ring. We found that
> > +        * serializing operations between SW and HW made this issue
> > +        * disappear. We believe this issue arises because of a race between SW
> > +        * and HW. SW adds new descriptors to the ring and clears
> > +        * DESC_INVLD_STATUS to ask HW to pick them up. However HW has
> > +        * prefetched descriptors and writes DESC_INVLD_STATUS to let SW know
> it
> > +        * has fetched an invalid descriptor. It appears that in this way HW can
> > +        * overwrite the clearing of DESC_INVLD_STATUS by SW (though we never
> > +        * saw DESC_INVLD_STATUS set to 1 when the DMA engine had stopped)
> and
> > +        * therefore HW never picks up these descriptors.
> > +        *
> > +        * Therefore it is necessary for SW to clear DESC_INVLD_STATUS only
> > +        * after HW has set it, and the notify_hw flag accomplishes this
> > +        * between issue_pending(..) and the invld_desc interrupt handler.
> > +        */
> > +       if (mic_ch->notify_hw) {
> > +               ctrl_reg = mic_dma_ch_reg_read(mic_ch, MIC_DMA_CTRL_STATUS);
> > +               if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
> > +                       mic_ch->notify_hw = 0;
> > +                       /* Clear DESC_INVLD_STATUS flag */
> > +                       mic_dma_ch_reg_write(mic_ch, MIC_DMA_CTRL_STATUS,
> > +                                            ctrl_reg);
> > +               } else {
> > +                       dev_err(mic_dma_ch_to_device(mic_ch),
> > +                               "%s: DESC_INVLD_STATUS not set\n", __func__);
> > +               }
> > +       }
> > +       spin_unlock_irqrestore(&mic_ch->issue_lock, flags);
> > +}
> > +
> > +static inline void mic_dma_update_pending(struct mic_dma_chan *ch)
> > +{
> > +       if (mic_dma_ring_count(ch->head, ch->last_tail)
> > +               > MIC_DMA_PENDING_LEVEL)
> 
> One line?
> 
> > +               mic_dma_issue_pending(&ch->chan);
> > +}
> > +
> > +static dma_cookie_t mic_dma_tx_submit_unlock(struct
> dma_async_tx_descriptor *tx)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(tx->chan);
> > +       dma_cookie_t cookie;
> > +
> > +       dma_cookie_assign(tx);
> > +       cookie = tx->cookie;
> > +       /*
> > +        * smp_wmb pairs with smb_rmb in mic_dma_cleanup to ensure
> > +        * that tx->cookie is visible before updating ch->head
> > +        */
> > +       smp_wmb();
> > +
> > +       /*
> > +        * The final fence/interrupt desc is now programmed in submit after the
> > +        * cookie has been assigned
> > +        */
> > +       if (tx->flags) {
> > +               mic_dma_fence_intr_desc(mic_ch, tx->flags);
> > +               tx->flags = 0;
> > +       }
> > +
> > +       spin_unlock(&mic_ch->prep_lock);
> > +       mic_dma_update_pending(mic_ch);
> > +       return cookie;
> > +}
> > +
> > +static inline struct dma_async_tx_descriptor *
> > +allocate_tx(struct mic_dma_chan *ch, unsigned long flags)
> > +{
> > +       /* index of the final desc which is or will be programmed */
> > +       u32 idx = flags ? ch->head : mic_dma_ring_dec(ch->head);
> > +       struct dma_async_tx_descriptor *tx = &ch->tx_array[idx];
> > +
> > +       dma_async_tx_descriptor_init(tx, &ch->chan);
> > +       tx->tx_submit = mic_dma_tx_submit_unlock;
> > +       tx->flags = flags;
> > +       return tx;
> > +}
> > +
> > +static struct dma_async_tx_descriptor *
> > +mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
> > +                        dma_addr_t dma_src, size_t len, unsigned long flags)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +       struct device *dev = mic_dma_ch_to_device(mic_ch);
> > +       int result;
> > +
> > +       /*
> > +        * return holding prep_lock which will be released in submit
> > +        */
> > +       spin_lock(&mic_ch->prep_lock);
> > +       result = mic_dma_do_dma(mic_ch, flags, dma_src, dma_dest, len);
> 
> > +       if (result >= 0)
> > +               return allocate_tx(mic_ch, flags);
> 
> Hmm… Usual pattern is
> if (err)
>  do error path();
> 
> > +       dev_err(dev, "Error enqueueing dma, error=%d\n", result);
> > +       spin_unlock(&mic_ch->prep_lock);
> > +       return NULL;
> > +}
> > +
> > +static struct dma_async_tx_descriptor *
> > +mic_dma_prep_interrupt_lock(struct dma_chan *ch, unsigned long flags)
> > +{
> > +       struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > +       /*
> > +        * Program 0 length DMA. Interrupt desc to be programmed in submit.
> > +        * All DMA transfers need a valid source and destination.
> > +        */
> > +       return mic_dma_prep_memcpy_lock(ch, mic_ch->desc_ring_da,
> > +                                       mic_ch->desc_ring_da, 0, flags);
> > +}
> > +
> > +static irqreturn_t mic_dma_thread_fn(int irq, void *data)
> > +{
> > +       struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device
> *)data);
> > +
> > +       mic_dma_cleanup(&mic_dma_dev->mic_chan);
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static void mic_dma_invld_desc_interrupt(struct mic_dma_chan *ch)
> > +{
> > +       u32 ctrl_reg;
> > +
> > +       spin_lock(&ch->issue_lock);
> > +       if (!ch->notify_hw) {
> > +               ctrl_reg = mic_dma_ch_reg_read(ch, MIC_DMA_CTRL_STATUS);
> > +               if (ctrl_reg & MIC_DMA_CTRL_DESC_INVLD_STATUS) {
> 
> Isn't too long?
> MIC_DMA_CTRL_DESC_INVLD_STATUS -> MIC_DMA_CTRL_DESC_INVAL ?
> 
> > +                       /*
> > +                        * Check the descriptor ring for valid descriptors. If
> > +                        * valid descriptors are available clear the
> > +                        * DESC_INVLD_STATUS bit to signal to the hardware to
> > +                        * pick them up. If no valid descriptors are available,
> > +                        * when issue_pending is called, it will clear the
> > +                        * INVLD_STATUS bit to signal to the hardware to pick up
> > +                        * the posted descriptors.
> > +                        */
> > +                       if (ch->desc_ring[mic_dma_ring_dec(ch->head)].qw0 &
> > +                               MIC_DMA_DESC_VALID) {
> > +                               mic_dma_ch_reg_write(ch, MIC_DMA_CTRL_STATUS,
> > +                                                    ctrl_reg);
> > +                       } else {
> > +                               ch->notify_hw = 1;
> > +                       }
> > +               } else {
> > +                       dev_err(mic_dma_ch_to_device(ch),
> > +                               "%s: DESC_INVLD_STATUS not set\n", __func__);
> > +               }
> > +       }
> > +       spin_unlock(&ch->issue_lock);
> > +}
> > +
> > +static void mic_dma_abort_interrupt(struct mic_dma_chan *ch, u32 intr_reg)
> > +{
> > +       u32 reg;
> > +       u32 ring_pos;
> > +
> > +       spin_lock(&ch->issue_lock);
> > +       reg = mic_dma_ch_reg_read(ch, MIC_DMA_LAST_DESC_ADDR_LOW);
> > +       ring_pos  = desc_ring_da_to_index(ch, reg);
> > +
> > +       if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID) {
> > +               /*
> > +               * Move back the HW next pointer so it points to the tail of
> > +               * the ring descriptor.
> > +               */
> > +               mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW, reg);
> > +       } else {
> > +               /* Last descriptor is not valid, find a valid one or head */
> > +               u32 tail = ch->last_tail;
> > +               u32 new_next;
> > +
> > +               while (tail != ch->head) {
> > +                       tail = mic_dma_ring_inc(tail);
> > +                       if (ch->desc_ring[ring_pos].qw0 & MIC_DMA_DESC_VALID)
> > +                               break;
> > +               }
> > +               new_next = desc_ring_index_to_da(ch, tail);
> > +               mic_dma_ch_reg_write(ch, MIC_DMA_NEXT_DESC_ADDR_LOW,
> new_next);
> > +
> > +               dev_err(mic_dma_ch_to_device(ch),
> > +                       "Abort at %d with invalid descriptor, new descr %d, head = %d, tail
> %d\n",
> > +                       ring_pos, tail, ch->head, ch->last_tail);
> > +       }
> > +
> > +       /*
> > +        * If abort was caused by hw error then restart DMA engine.
> > +        * If abort was requested by driver then leave it stopped.
> > +        */
> > +       if (intr_reg & MIC_DMA_ERROR_INTR_STATUS) {
> > +               if (ch->abort_tail != desc_ring_da_to_index(ch, reg)) {
> > +                       ch->abort_tail = desc_ring_da_to_index(ch, reg);
> > +                       ch->abort_counter = 0;
> > +               } else {
> > +                       ch->abort_counter++;
> > +               }
> > +
> > +               if (ch->abort_counter < MIC_DMA_ABORT_LIMIT) {
> > +                       dev_err(mic_dma_ch_to_device(ch),
> > +                               "Abort/Error at %d (retry %d), reenabling DMA engine.\n",
> > +                                ch->abort_tail, ch->abort_counter);
> > +                       mic_dma_enable_chan(ch);
> > +               } else {
> > +                       /*
> > +                        * We have tried same descriptor several times and it is
> > +                        * still generating abort.
> > +                        */
> > +                       dev_err(mic_dma_ch_to_device(ch),
> > +                               "Abort Error at %d (retry %d), give up.\n",
> > +                               ch->abort_tail, ch->abort_counter);
> > +               }
> > +       }
> > +       spin_unlock(&ch->issue_lock);
> > +}
> > +
> > +static u32 mic_dma_ack_interrupt(struct mic_dma_chan *ch)
> > +{
> > +       u32 intr_reg = mic_dma_ch_reg_read(ch,
> MIC_DMA_INTR_CTRL_STATUS);
> > +
> > +       mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> > +       return intr_reg;
> > +}
> > +
> > +static irqreturn_t mic_dma_intr_handler(int irq, void *data)
> > +{
> > +       struct mic_dma_device *mic_dma_dev = ((struct mic_dma_device
> *)data);
> > +       bool wake_thread = false, error = false;
> > +       struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> > +       struct device *dev = mic_dma_ch_to_device(ch);
> > +       u32 reg;
> > +
> > +       reg = mic_dma_ack_interrupt(&mic_dma_dev->mic_chan);
> > +       wake_thread = !!(reg & MIC_DMA_DESC_DONE_INTR_STATUS);
> > +       error = !!(reg & MIC_DMA_ERROR_STATUS);
> > +       if (error)
> > +               dev_err(dev, "%s error status 0x%x\n", __func__, reg);
> > +
> > +       if (reg & MIC_DMA_INVLD_DESC_INTR_STATUS)
> > +               mic_dma_invld_desc_interrupt(&mic_dma_dev->mic_chan);
> > +
> > +       if (reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> > +               mic_dma_abort_interrupt(&mic_dma_dev->mic_chan, reg);
> > +
> > +       if (wake_thread)
> > +               return IRQ_WAKE_THREAD;
> > +
> > +       if (error || reg & MIC_DMA_INVLD_DESC_INTR_STATUS ||
> > +           reg & MIC_DMA_ABORT_DONE_INTR_STATUS)
> > +               return IRQ_HANDLED;
> > +
> > +       return IRQ_NONE;
> > +}
> > +
> > +static int mic_dma_setup_irq(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct pci_dev *pdev = mic_dma_dev->pdev;
> > +       struct device *dev = &pdev->dev;
> > +       int rc = pci_enable_msi(pdev);
> > +
> > +       if (rc)
> > +               pci_intx(pdev, 1);
> > +
> > +       return devm_request_threaded_irq(dev, pdev->irq,
> mic_dma_intr_handler,
> > +                                        mic_dma_thread_fn, IRQF_SHARED,
> > +                                        "mic-x200-dma-intr", mic_dma_dev);
> > +}
> > +
> > +static inline void mic_dma_free_irq(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct pci_dev *pdev = mic_dma_dev->pdev;
> > +       struct device *dev = &pdev->dev;
> > +
> > +       devm_free_irq(dev, pdev->irq, mic_dma_dev);
> 
> > +       if (pci_dev_msi_enabled(pdev))
> > +               pci_disable_msi(pdev);
> 
> pcim_enable_device() will take care of this one.
> 
> > +}
> > +
> > +static void mic_dma_hw_init(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct mic_dma_chan *ch = &mic_dma_dev->mic_chan;
> > +       u32 intr_reg;
> > +
> > +       dev_dbg(mic_dma_dev->dma_dev.dev, "Ctrl reg 0x%0x, Intr reg
> 0x%0x\n",
> > +               mic_dma_reg_read(mic_dma_dev, MIC_DMA_CTRL_STATUS),
> > +               mic_dma_reg_read(mic_dma_dev, MIC_DMA_INTR_CTRL_STATUS));
> > +
> > +       intr_reg = mic_dma_ch_reg_read(ch, MIC_DMA_INTR_CTRL_STATUS);
> > +       if (intr_reg & MIC_DMA_ALL_INTR_EN) {
> > +               /*
> > +                * Interrupts are enabled - that means card has been reset
> > +                * without resetting DMA HW.
> > +                * Disable interrupts and issue abort to stop DMA HW.
> > +                */
> > +               intr_reg &= ~(MIC_DMA_ALL_INTR_EN);
> 
> Redundant parens.
> 
> > +               mic_dma_ch_reg_write(ch, MIC_DMA_INTR_CTRL_STATUS, intr_reg);
> > +               mic_dma_disable_chan(ch);
> > +       }
> > +
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_CTRL_STATUS,
> > +                         MIC_DMA_CTRL_STATUS_INIT);
> > +
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_CHAN_BANDWIDTH,
> > +                         MIC_DMA_MAX_DST_MEM_WRITE);
> > +
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA_RAM_THRESH,
> > +                         MIC_DMA_STA_RAM_THRESH_INIT);
> > +
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_HDR_RAM,
> > +                         MIC_DMA_STA_HDR_THRESH_INIT);
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_HDR_RAM,
> > +                         MIC_DMA_STA_HDR_THRESH_INIT);
> > +
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA0_PLD_RAM,
> > +                         MIC_DMA_STA_PLD_THRESH_INIT);
> > +       mic_dma_reg_write(mic_dma_dev, MIC_DMA_STA1_PLD_RAM,
> > +                         MIC_DMA_STA_PLD_THRESH_INIT);
> > +}
> > +
> > +static int mic_dma_init(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct mic_dma_chan *ch;
> 
> > +       int rc = 0;
> 
> Useless assignment.
> 
> > +
> > +       mic_dma_hw_init(mic_dma_dev);
> > +
> > +       rc = mic_dma_setup_irq(mic_dma_dev);
> > +       if (rc) {
> > +               dev_err(mic_dma_dev->dma_dev.dev,
> > +                       "%s %d func error line %d\n", __func__, __LINE__, rc);
> > +               goto error;
> > +       }
> > +       ch = &mic_dma_dev->mic_chan;
> > +       spin_lock_init(&ch->cleanup_lock);
> > +       spin_lock_init(&ch->prep_lock);
> > +       spin_lock_init(&ch->issue_lock);
> 
> > +error:
> 
> Useless label.
> 
> > +       return rc;
> > +}
> > +
> > +static void mic_dma_uninit(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       mic_dma_free_irq(mic_dma_dev);
> > +}
> > +
> > +static int mic_register_dma_device(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct dma_device *dma_dev = &mic_dma_dev->dma_dev;
> > +       struct mic_dma_chan *ch;
> > +
> > +       dma_cap_zero(dma_dev->cap_mask);
> > +
> > +       dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
> > +       dma_cap_set(DMA_MEMCPY, dma_dev->cap_mask);
> > +
> > +       dma_dev->device_alloc_chan_resources =
> mic_dma_alloc_chan_resources;
> > +       dma_dev->device_free_chan_resources =
> mic_dma_free_chan_resources;
> > +       dma_dev->device_tx_status = mic_dma_tx_status;
> > +       dma_dev->device_prep_dma_memcpy = mic_dma_prep_memcpy_lock;
> > +       dma_dev->device_prep_dma_interrupt = mic_dma_prep_interrupt_lock;
> > +       dma_dev->device_issue_pending = mic_dma_issue_pending;
> > +       INIT_LIST_HEAD(&dma_dev->channels);
> > +
> > +       /* Associate dma_dev with dma_chan */
> > +       ch = &mic_dma_dev->mic_chan;
> > +       ch->chan.device = dma_dev;
> > +       dma_cookie_init(&ch->chan);
> > +       list_add_tail(&ch->chan.device_node, &dma_dev->channels);
> > +       return dma_async_device_register(dma_dev);
> > +}
> > +
> > +static int mic_dma_probe(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       struct dma_device *dma_dev;
> > +       int rc = -EINVAL;
> > +
> > +       dma_dev = &mic_dma_dev->dma_dev;
> > +       dma_dev->dev = &mic_dma_dev->pdev->dev;
> > +
> > +       /* MIC X200 has one DMA channel per function */
> > +       dma_dev->chancnt = 1;
> > +
> > +       mic_dma_dev->max_xfer_size = MIC_DMA_DESC_SIZE_MASK;
> > +
> > +       rc = mic_dma_init(mic_dma_dev);
> > +       if (rc) {
> > +               dev_err(&mic_dma_dev->pdev->dev,
> > +                       "%s %d rc %d\n", __func__, __LINE__, rc);
> > +               goto init_error;
> > +       }
> > +
> > +       rc = mic_register_dma_device(mic_dma_dev);
> > +       if (rc) {
> > +               dev_err(&mic_dma_dev->pdev->dev,
> > +                       "%s %d rc %d\n", __func__, __LINE__, rc);
> > +               goto reg_error;
> > +       }
> > +       return rc;
> > +reg_error:
> > +       mic_dma_uninit(mic_dma_dev);
> 
> > +init_error:
> 
> Ditto.
> 
> > +       return rc;
> > +}
> > +
> > +static void mic_dma_remove(struct mic_dma_device *mic_dma_dev)
> > +{
> > +       dma_async_device_unregister(&mic_dma_dev->dma_dev);
> > +       mic_dma_uninit(mic_dma_dev);
> > +}
> > +
> > +static struct mic_dma_device *
> > +mic_x200_alloc_dma(struct pci_dev *pdev, void __iomem *iobase)
> > +{
> > +       struct mic_dma_device *d = kzalloc(sizeof(*d), GFP_KERNEL);
> 
> devm_
> 
> > +
> > +       if (!d)
> > +               return NULL;
> > +       d->pdev = pdev;
> > +       d->reg_base = iobase;
> > +       return d;
> > +}
> > +
> > +static int
> > +mic_x200_dma_probe(struct pci_dev *pdev, const struct pci_device_id
> *pdev_id)
> > +{
> > +       int rc;
> > +       struct mic_dma_device *dma_dev;
> > +       void __iomem * const *iomap;
> > +
> > +       rc = pcim_enable_device(pdev);
> > +       if (rc)
> > +               goto done;
> > +       rc = pcim_iomap_regions(pdev, 0x1, MIC_DMA_DRV_NAME);
> > +       if (rc)
> > +               goto done;
> 
> > +       iomap = pcim_iomap_table(pdev);
> 
> > +       if (!iomap) {
> > +               rc = -ENOMEM;
> > +               goto done;
> > +       }
> 
> Redundant check. Previous one fail otherwise this is valid.
> 
> > +       rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
> > +       if (rc)
> > +               goto done;
> > +       rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
> > +       if (rc)
> > +               goto done;
> > +       dma_dev = mic_x200_alloc_dma(pdev, iomap[0]);
> > +       if (!dma_dev) {
> > +               rc = -ENOMEM;
> > +               goto done;
> > +       }
> > +       pci_set_master(pdev);
> > +       pci_set_drvdata(pdev, dma_dev);
> > +       rc = mic_dma_probe(dma_dev);
> > +       if (rc)
> > +               goto probe_err;
> > +       return 0;
> 
> > +probe_err:
> > +       kfree(dma_dev);
> 
> devm_ takes care of.
> 
> > +done:
> > +       dev_err(&pdev->dev, "probe error rc %d\n", rc);
> 
> Useless. You may learn "initcall_debug" parameter.
> 
> And thus label is useless.
> 
> > +       return rc;
> > +}
> > +
> > +static void  mic_x200_dma_remove(struct pci_dev *pdev)
> > +{
> > +       struct mic_dma_device *dma_dev = pci_get_drvdata(pdev);
> > +
> > +       mic_dma_remove(dma_dev);
> 
> > +       kfree(dma_dev);
> 
> devm_
> 
> > +}
> > +
> > +static struct pci_device_id mic_x200_dma_pci_id_table[] = {
> > +       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2264)},
> > +       {}
> > +};
> > +MODULE_DEVICE_TABLE(pci, mic_x200_dma_pci_id_table);
> > +
> > +static struct pci_driver mic_x200_dma_driver = {
> > +       .name     = MIC_DMA_DRV_NAME,
> > +       .id_table = mic_x200_dma_pci_id_table,
> > +       .probe    = mic_x200_dma_probe,
> > +       .remove   = mic_x200_dma_remove
> 
> > +};
> > +
> > +static int __init mic_x200_dma_init(void)
> > +{
> > +       int rc = pci_register_driver(&mic_x200_dma_driver);
> > +
> > +       if (rc)
> > +               pr_err("%s %d rc %x\n", __func__, __LINE__, rc);
> > +       return rc;
> > +}
> > +
> > +static void __exit mic_x200_dma_exit(void)
> > +{
> > +       pci_unregister_driver(&mic_x200_dma_driver);
> > +}
> > +
> > +module_init(mic_x200_dma_init);
> > +module_exit(mic_x200_dma_exit);
> 
> module_pci_driver();
> 
> > +
> > +MODULE_AUTHOR("Intel Corporation");
> > +MODULE_DESCRIPTION("Intel(R) MIC X200 DMA Driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.1.4
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

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

end of thread, other threads:[~2016-02-09 12:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-04 15:10 [PATCH RESEND 0/2] Enable DMA driver for MIC X200 Coprocessor Jacek Lawrynowicz
2016-02-04 15:10 ` [PATCH RESEND 1/2] dmaengine: added MIC X200 Coprocessor DMA driver Jacek Lawrynowicz
2016-02-04 17:50   ` Andy Shevchenko
2016-02-09 12:25     ` Lawrynowicz, Jacek
2016-02-08  4:21   ` Vinod Koul
2016-02-04 15:10 ` [PATCH RESEND 2/2] MAINTAINERS: update for mic Jacek Lawrynowicz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.