linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] i2c: add DMA support for freescale i2c driver
@ 2014-04-04  2:41 Yuan Yao
  2014-04-04  2:41 ` [PATCH v4 1/2] " Yuan Yao
  2014-04-04  2:41 ` [PATCH v4 2/2] Documentation:add " Yuan Yao
  0 siblings, 2 replies; 13+ messages in thread
From: Yuan Yao @ 2014-04-04  2:41 UTC (permalink / raw)
  To: linux-arm-kernel


Changed in v4:
- cancelled "i2c_imx->use_dma".
- changed "Dma" to "DMA".
- add timeout handling  for DMA transfer complete.

Changed in v3:
- fix a bug when request the DMA faild.
- some minor fixes for coding style.
- other minor fixes.

Changed in v2:
- remove has_dma_support property
- unify i2c_imx_dma_rx and i2c_imx_dma_tx
- unify i2c_imx_dma_read and i2c_imx_pio_read
- unify i2c_imx_dma_write and i2c_imx_pio_write

Added in v1:
- Enable DMA if it's support DMA and transfer size bigger than the threshold.
- Add device tree bindings for i2c eDMA support.
- Add eDMA support for i2c driver.

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04  2:41 [PATCH v4 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
@ 2014-04-04  2:41 ` Yuan Yao
  2014-04-04  9:49   ` sourav
  2014-04-04 14:28   ` Marek Vasut
  2014-04-04  2:41 ` [PATCH v4 2/2] Documentation:add " Yuan Yao
  1 sibling, 2 replies; 13+ messages in thread
From: Yuan Yao @ 2014-04-04  2:41 UTC (permalink / raw)
  To: linux-arm-kernel

Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts node.

Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
---
 drivers/i2c/busses/i2c-imx.c | 372 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 319 insertions(+), 53 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index db895fb..3d63b35 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -37,22 +37,27 @@
 /** Includes *******************************************************************
 *******************************************************************************/
 
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/dmapool.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
-#include <linux/delay.h>
 #include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/io.h>
-#include <linux/sched.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 #include <linux/platform_data/i2c-imx.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
 
 /** Defines ********************************************************************
 *******************************************************************************/
@@ -63,6 +68,10 @@
 /* Default value */
 #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
 
+/* enable DMA if transfer byte size is bigger than this threshold */
+#define IMX_I2C_DMA_THRESHOLD	16
+#define IMX_I2C_DMA_TIMEOUT	1000
+
 /* IMX I2C registers:
  * the I2C register offset is different between SoCs,
  * to provid support for all these chips, split the
@@ -88,6 +97,7 @@
 #define I2SR_IBB	0x20
 #define I2SR_IAAS	0x40
 #define I2SR_ICF	0x80
+#define I2CR_DMAEN	0x02
 #define I2CR_RSTA	0x04
 #define I2CR_TXAK	0x08
 #define I2CR_MTX	0x10
@@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
 	unsigned		i2cr_ien_opcode;
 };
 
+struct imx_i2c_dma {
+	struct dma_chan		*chan_tx;
+	struct dma_chan		*chan_rx;
+	struct dma_chan		*chan_using;
+	struct completion	cmd_complete;
+	dma_addr_t		dma_buf;
+	unsigned int		dma_len;
+	unsigned int		dma_transfer_dir;
+	unsigned int		dma_data_dir;
+};
+
 struct imx_i2c_struct {
 	struct i2c_adapter	adapter;
 	struct clk		*clk;
@@ -184,6 +205,8 @@ struct imx_i2c_struct {
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
 	const struct imx_i2c_hwdata	*hwdata;
+
+	struct imx_i2c_dma	*dma;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
@@ -254,6 +277,132 @@ static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
 	return readb(i2c_imx->base + (reg << i2c_imx->hwdata->regshift));
 }
 
+/* Functions for DMA support */
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+						dma_addr_t phy_addr)
+{
+	struct imx_i2c_dma *dma;
+	struct dma_slave_config dma_sconfig;
+	struct device *dev = &i2c_imx->adapter.dev;
+	int ret;
+
+	dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
+	if (!dma) {
+		dev_info(dev, "can't allocate DMA struct\n");
+		return -ENOMEM;
+	}
+
+	dma->chan_tx = dma_request_slave_channel(dev, "tx");
+	return 0;
+	if (!dma->chan_tx) {
+		dev_info(dev, "DMA tx channel request failed\n");
+		ret = -ENODEV;
+		goto fail_al;
+	}
+
+	dma_sconfig.dst_addr = phy_addr +
+				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_maxburst = 1;
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig);
+	if (ret < 0) {
+		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+		goto fail_tx;
+	}
+
+	dma->chan_rx = dma_request_slave_channel(dev, "rx");
+	if (!dma->chan_rx) {
+		dev_info(dev, "DMA rx channel request failed\n");
+		ret = -ENODEV;
+		goto fail_tx;
+	}
+
+	dma_sconfig.src_addr = phy_addr +
+				(IMX_I2C_I2DR << i2c_imx->hwdata->regshift);
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig);
+	if (ret < 0) {
+		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+		goto fail_rx;
+	}
+
+	i2c_imx->dma = dma;
+
+	init_completion(&dma->cmd_complete);
+
+	return 0;
+
+fail_rx:
+	dma_release_channel(dma->chan_rx);
+fail_tx:
+	dma_release_channel(dma->chan_tx);
+fail_al:
+	devm_kfree(dev, dma);
+
+	return ret;
+}
+
+static void i2c_imx_dma_callback(void *arg)
+{
+	struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+
+	dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
+			dma->dma_len, dma->dma_data_dir);
+	complete(&dma->cmd_complete);
+}
+
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+					struct i2c_msg *msgs)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_async_tx_descriptor *txdesc;
+	struct device *dev = &i2c_imx->adapter.dev;
+
+	dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,
+					dma->dma_len, dma->dma_data_dir);
+	if (dma_mapping_error(dma->chan_using->device->dev, dma->dma_buf)) {
+		dev_err(dev, "DMA mapping failed\n");
+		return -EINVAL;
+	}
+
+	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
+					dma->dma_len, dma->dma_transfer_dir,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!txdesc) {
+		dev_err(dev, "Not able to get desc for DMA xfer\n");
+		dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
+					dma->dma_len, dma->dma_data_dir);
+		return -EINVAL;
+	}
+
+	txdesc->callback = i2c_imx_dma_callback;
+	txdesc->callback_param = i2c_imx;
+	dmaengine_submit(txdesc);
+	dma_async_issue_pending(dma->chan_using);
+
+	return 0;
+}
+
+static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+
+	dma->dma_buf = 0;
+	dma->dma_len = 0;
+
+	dma_release_channel(dma->chan_tx);
+	dma->chan_tx = NULL;
+
+	dma_release_channel(dma->chan_rx);
+	dma->chan_rx = NULL;
+
+	dma->chan_using = NULL;
+}
+
 /** Functions for IMX I2C adapter driver ***************************************
 *******************************************************************************/
 
@@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 
 	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~I2CR_DMAEN;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
 	return result;
 }
 
@@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 
 static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 {
-	int i, result;
+	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
+	unsigned int temp = 0;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct device *dev = &i2c_imx->adapter.dev;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
+	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
 		__func__, msgs->addr << 1);
+	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
+		reinit_completion(&i2c_imx->dma->cmd_complete);
+		dma->chan_using = dma->chan_tx;
+		dma->dma_transfer_dir = DMA_MEM_TO_DEV;
+		dma->dma_data_dir = DMA_TO_DEVICE;
+		dma->dma_len = msgs->len - 1;
+		result = i2c_imx_dma_xfer(i2c_imx, msgs);
+		if (result)
+			return result;
 
-	/* write slave address */
-	imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
-	result = i2c_imx_trx_complete(i2c_imx);
-	if (result)
-		return result;
-	result = i2c_imx_acked(i2c_imx);
-	if (result)
-		return result;
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> write data\n", __func__);
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 
-	/* write data */
-	for (i = 0; i < msgs->len; i++) {
-		dev_dbg(&i2c_imx->adapter.dev,
-			"<%s> write byte: B%d=0x%X\n",
-			__func__, i, msgs->buf[i]);
-		imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
+		/* write slave address */
+		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
+		result = wait_for_completion_interruptible_timeout(
+					&i2c_imx->dma->cmd_complete,
+					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
+		if (result <= 0) {
+			dmaengine_terminate_all(dma->chan_using);
+			if (result)
+				return result;
+			else
+				return -ETIMEDOUT;
+		}
+
+		/* waiting for Transfer complete. */
+		while (timeout--) {
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+			if (temp & I2SR_ICF)
+				break;
+			udelay(10);
+		}
+
+		if (!timeout)
+			return -ETIMEDOUT;
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp &= ~I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		/* write the last byte */
+		imx_i2c_write_reg(msgs->buf[msgs->len-1],
+					i2c_imx, IMX_I2C_I2DR);
 		result = i2c_imx_trx_complete(i2c_imx);
 		if (result)
 			return result;
+
+		result = i2c_imx_acked(i2c_imx);
+		if (result)
+			return result;
+	} else {
+		/* write slave address */
+		imx_i2c_write_reg(msgs->addr << 1, i2c_imx, IMX_I2C_I2DR);
+		result = i2c_imx_trx_complete(i2c_imx);
+		if (result)
+			return result;
+
 		result = i2c_imx_acked(i2c_imx);
 		if (result)
 			return result;
+
+		dev_dbg(dev, "<%s> write data\n", __func__);
+
+		/* write data */
+		for (i = 0; i < msgs->len; i++) {
+			dev_dbg(dev, "<%s> write byte: B%d=0x%X\n",
+				__func__, i, msgs->buf[i]);
+			imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
+			result = i2c_imx_trx_complete(i2c_imx);
+			if (result)
+				return result;
+			result = i2c_imx_acked(i2c_imx);
+			if (result)
+				return result;
+		}
 	}
 	return 0;
 }
 
 static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 {
-	int i, result;
+	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
 	unsigned int temp;
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct device *dev = &i2c_imx->adapter.dev;
 
-	dev_dbg(&i2c_imx->adapter.dev,
-		"<%s> write slave address: addr=0x%x\n",
+	dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
 		__func__, (msgs->addr << 1) | 0x01);
 
 	/* write slave address */
@@ -476,7 +687,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	if (result)
 		return result;
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
+	dev_dbg(dev, "<%s> setup bus\n", __func__);
 
 	/* setup bus to read data */
 	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
@@ -486,35 +697,81 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
 	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
 
-	dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
+	dev_dbg(dev, "<%s> read data\n", __func__);
 
-	/* read data */
-	for (i = 0; i < msgs->len; i++) {
-		result = i2c_imx_trx_complete(i2c_imx);
+	if (dma && msgs->len >= IMX_I2C_DMA_THRESHOLD) {
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp |= I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		reinit_completion(&i2c_imx->dma->cmd_complete);
+		dma->chan_using = dma->chan_rx;
+		dma->dma_transfer_dir = DMA_DEV_TO_MEM;
+		dma->dma_data_dir = DMA_FROM_DEVICE;
+		dma->dma_len = msgs->len - 2;
+		result = i2c_imx_dma_xfer(i2c_imx, msgs);
 		if (result)
 			return result;
-		if (i == (msgs->len - 1)) {
-			/* It must generate STOP before read I2DR to prevent
-			   controller from generating another clock cycle */
-			dev_dbg(&i2c_imx->adapter.dev,
-				"<%s> clear MSTA\n", __func__);
-			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-			temp &= ~(I2CR_MSTA | I2CR_MTX);
-			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
-			i2c_imx_bus_busy(i2c_imx, 0);
-			i2c_imx->stopped = 1;
-		} else if (i == (msgs->len - 2)) {
-			dev_dbg(&i2c_imx->adapter.dev,
-				"<%s> set TXAK\n", __func__);
-			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-			temp |= I2CR_TXAK;
-			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+		result = wait_for_completion_interruptible_timeout(
+					&i2c_imx->dma->cmd_complete,
+					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
+		if (result <= 0) {
+			dmaengine_terminate_all(dma->chan_using);
+			if (result)
+				return result;
+			else
+				return -ETIMEDOUT;
 		}
-		msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
-		dev_dbg(&i2c_imx->adapter.dev,
-			"<%s> read byte: B%d=0x%X\n",
-			__func__, i, msgs->buf[i]);
+
+		/* waiting for Transfer complete. */
+		while (timeout--) {
+			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+			if (temp & I2SR_ICF)
+				break;
+			udelay(10);
+		}
+
+		if(!timeout)
+			return -ETIMEDOUT;
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+		temp &= ~I2CR_DMAEN;
+		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	} else {
+		/* read data */
+		for (i = 0; i < msgs->len - 2; i++) {
+			result = i2c_imx_trx_complete(i2c_imx);
+			if (result)
+				return result;
+			msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+			dev_dbg(dev, "<%s> read byte: B%d=0x%X\n",
+				__func__, i, msgs->buf[i]);
+		}
+		result = i2c_imx_trx_complete(i2c_imx);
 	}
+
+	/* read n-1 byte data */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp |= I2CR_TXAK;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+	/* read n byte data */
+	result = i2c_imx_trx_complete(i2c_imx);
+	if (result)
+		return result;
+
+	/*
+	 * It must generate STOP before read I2DR to prevent
+	 * controller from generating another clock cycle
+	 */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~(I2CR_MSTA | I2CR_MTX);
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	i2c_imx_bus_busy(i2c_imx, 0);
+	i2c_imx->stopped = 1;
+	msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+
 	return 0;
 }
 
@@ -601,6 +858,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 	u32 bitrate;
+	u32 phy_addr;
 
 	dev_dbg(&pdev->dev, "<%s>\n", __func__);
 
@@ -611,6 +869,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	phy_addr = res->start;
 	base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
@@ -696,6 +955,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
 		i2c_imx->adapter.name);
 	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
 
+	/* Init DMA config if support*/
+	if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
+		dev_warn(&pdev->dev, "can't request DMA\n");
+
 	return 0;   /* Return OK */
 }
 
@@ -707,6 +970,9 @@ static int i2c_imx_remove(struct platform_device *pdev)
 	dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
 	i2c_del_adapter(&i2c_imx->adapter);
 
+	if (i2c_imx->dma)
+		i2c_imx_dma_free(i2c_imx);
+
 	/* setup chip registers to defaults */
 	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
 	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
-- 
1.8.4

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

* [PATCH v4 2/2] Documentation:add DMA support for freescale i2c driver
  2014-04-04  2:41 [PATCH v4 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
  2014-04-04  2:41 ` [PATCH v4 1/2] " Yuan Yao
@ 2014-04-04  2:41 ` Yuan Yao
  1 sibling, 0 replies; 13+ messages in thread
From: Yuan Yao @ 2014-04-04  2:41 UTC (permalink / raw)
  To: linux-arm-kernel

Add i2c dts node properties for eDMA support, them depend on the eDMA driver.

Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
---
 Documentation/devicetree/bindings/i2c/i2c-imx.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
index 4a8513e..52d37fd 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
@@ -11,6 +11,8 @@ Required properties:
 Optional properties:
 - clock-frequency : Constains desired I2C/HS-I2C bus clock frequency in Hz.
   The absence of the propoerty indicates the default frequency 100 kHz.
+- dmas: A list of two dma specifiers, one for each entry in dma-names.
+- dma-names: should contain "tx" and "rx".
 
 Examples:
 
@@ -26,3 +28,12 @@ i2c at 70038000 { /* HS-I2C on i.MX51 */
 	interrupts = <64>;
 	clock-frequency = <400000>;
 };
+
+i2c0: i2c at 40066000 { /* i2c0 on vf610 */
+	compatible = "fsl,vf610-i2c";
+	reg = <0x40066000 0x1000>;
+	interrupts =<0 71 0x04>;
+	dmas = <&edma0 0 50>,
+		<&edma0 0 51>;
+	dma-names = "rx","tx";
+};
-- 
1.8.4

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04  2:41 ` [PATCH v4 1/2] " Yuan Yao
@ 2014-04-04  9:49   ` sourav
  2014-04-04 10:04     ` sourav
  2014-04-04 11:49     ` Marek Vasut
  2014-04-04 14:28   ` Marek Vasut
  1 sibling, 2 replies; 13+ messages in thread
From: sourav @ 2014-04-04  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,
On Friday 04 April 2014 08:11 AM, Yuan Yao wrote:
> Add dma support for i2c. This function depend on DMA driver.
> You can turn on it by write both the dmas and dma-name properties in dts node.
>
> Signed-off-by: Yuan Yao<yao.yuan@freescale.com>
> ---
>   drivers/i2c/busses/i2c-imx.c | 372 +++++++++++++++++++++++++++++++++++++------
>   1 file changed, 319 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index db895fb..3d63b35 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -37,22 +37,27 @@
>   /** Includes *******************************************************************
>   *******************************************************************************/
>
> -#include<linux/init.h>
> -#include<linux/kernel.h>
> -#include<linux/module.h>
> +#include<linux/clk.h>
> +#include<linux/completion.h>
> +#include<linux/delay.h>
> +#include<linux/dma-mapping.h>
> +#include<linux/dmaengine.h>
> +#include<linux/dmapool.h>
>   #include<linux/errno.h>
>   #include<linux/err.h>
>   #include<linux/interrupt.h>
> -#include<linux/delay.h>
>   #include<linux/i2c.h>
> +#include<linux/init.h>
>   #include<linux/io.h>
> -#include<linux/sched.h>
> -#include<linux/platform_device.h>
> -#include<linux/clk.h>
> -#include<linux/slab.h>
> +#include<linux/kernel.h>
> +#include<linux/module.h>
>   #include<linux/of.h>
>   #include<linux/of_device.h>
> +#include<linux/of_dma.h>
>   #include<linux/platform_data/i2c-imx.h>
> +#include<linux/platform_device.h>
> +#include<linux/sched.h>
> +#include<linux/slab.h>
>
>   /** Defines ********************************************************************
>   *******************************************************************************/
> @@ -63,6 +68,10 @@
>   /* Default value */
>   #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
>
> +/* enable DMA if transfer byte size is bigger than this threshold */
> +#define IMX_I2C_DMA_THRESHOLD	16
> +#define IMX_I2C_DMA_TIMEOUT	1000
> +
>   /* IMX I2C registers:
>    * the I2C register offset is different between SoCs,
>    * to provid support for all these chips, split the
> @@ -88,6 +97,7 @@
>   #define I2SR_IBB	0x20
>   #define I2SR_IAAS	0x40
>   #define I2SR_ICF	0x80
> +#define I2CR_DMAEN	0x02
>   #define I2CR_RSTA	0x04
>   #define I2CR_TXAK	0x08
>   #define I2CR_MTX	0x10
> @@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
>   	unsigned		i2cr_ien_opcode;
>   };
>
> +struct imx_i2c_dma {
> +	struct dma_chan		*chan_tx;
> +	struct dma_chan		*chan_rx;
> +	struct dma_chan		*chan_using;
> +	struct completion	cmd_complete;
> +	dma_addr_t		dma_buf;
> +	unsigned int		dma_len;
> +	unsigned int		dma_transfer_dir;
> +	unsigned int		dma_data_dir;
> +};
> +
>   struct imx_i2c_struct {
>   	struct i2c_adapter	adapter;
>   	struct clk		*clk;
> @@ -184,6 +205,8 @@ struct imx_i2c_struct {
>   	int			stopped;
>   	unsigned int		ifdr; /* IMX_I2C_IFDR */
>   	const struct imx_i2c_hwdata	*hwdata;
> +
> +	struct imx_i2c_dma	*dma;
>   };
>
>   static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> @@ -254,6 +277,132 @@ static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
>   	return readb(i2c_imx->base + (reg<<  i2c_imx->hwdata->regshift));
>   }
>
> +/* Functions for DMA support */
> +static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> +						dma_addr_t phy_addr)
> +{
> +	struct imx_i2c_dma *dma;
> +	struct dma_slave_config dma_sconfig;
> +	struct device *dev =&i2c_imx->adapter.dev;
> +	int ret;
> +
> +	dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
> +	if (!dma) {
> +		dev_info(dev, "can't allocate DMA struct\n");
> +		return -ENOMEM;
> +	}
> +
> +	dma->chan_tx = dma_request_slave_channel(dev, "tx");
> +	return 0;
?? Looks to be some leftover?
> +	if (!dma->chan_tx) {
> +		dev_info(dev, "DMA tx channel request failed\n");
> +		ret = -ENODEV;
> +		goto fail_al;
> +	}
> +
> +	dma_sconfig.dst_addr = phy_addr +
> +				(IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
> +	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> +	dma_sconfig.dst_maxburst = 1;
> +	dma_sconfig.direction = DMA_MEM_TO_DEV;
> +	ret = dmaengine_slave_config(dma->chan_tx,&dma_sconfig);
> +	if (ret<  0) {
> +		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
> +		goto fail_tx;
> +	}
> +
> +	dma->chan_rx = dma_request_slave_channel(dev, "rx");
> +	if (!dma->chan_rx) {
> +		dev_info(dev, "DMA rx channel request failed\n");
> +		ret = -ENODEV;
> +		goto fail_tx;
> +	}
> +
> +	dma_sconfig.src_addr = phy_addr +
> +				(IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
> +	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> +	dma_sconfig.src_maxburst = 1;
> +	dma_sconfig.direction = DMA_DEV_TO_MEM;
> +	ret = dmaengine_slave_config(dma->chan_rx,&dma_sconfig);
> +	if (ret<  0) {
> +		dev_info(dev, "DMA slave config failed, err = %d\n", ret);
> +		goto fail_rx;
> +	}
> +
> +	i2c_imx->dma = dma;
> +
> +	init_completion(&dma->cmd_complete);
> +
> +	return 0;
> +
> +fail_rx:
> +	dma_release_channel(dma->chan_rx);
> +fail_tx:
> +	dma_release_channel(dma->chan_tx);
> +fail_al:
> +	devm_kfree(dev, dma);
> +
> +	return ret;
> +}
> +
> +static void i2c_imx_dma_callback(void *arg)
> +{
> +	struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +
> +	dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
> +			dma->dma_len, dma->dma_data_dir);
> +	complete(&dma->cmd_complete);
> +}
> +
> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> +					struct i2c_msg *msgs)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_async_tx_descriptor *txdesc;
> +	struct device *dev =&i2c_imx->adapter.dev;
> +
> +	dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,
> +					dma->dma_len, dma->dma_data_dir);
> +	if (dma_mapping_error(dma->chan_using->device->dev, dma->dma_buf)) {
> +		dev_err(dev, "DMA mapping failed\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
> +					dma->dma_len, dma->dma_transfer_dir,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!txdesc) {
> +		dev_err(dev, "Not able to get desc for DMA xfer\n");
> +		dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
> +					dma->dma_len, dma->dma_data_dir);
> +		return -EINVAL;
> +	}
> +
> +	txdesc->callback = i2c_imx_dma_callback;
> +	txdesc->callback_param = i2c_imx;
> +	dmaengine_submit(txdesc);
> +	dma_async_issue_pending(dma->chan_using);
> +
> +	return 0;
> +}
> +
> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +
> +	dma->dma_buf = 0;
> +	dma->dma_len = 0;
> +
> +	dma_release_channel(dma->chan_tx);
> +	dma->chan_tx = NULL;
> +
> +	dma_release_channel(dma->chan_rx);
> +	dma->chan_rx = NULL;
> +
> +	dma->chan_using = NULL;
> +}
> +
>   /** Functions for IMX I2C adapter driver ***************************************
>   *******************************************************************************/
>
> @@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>
>   	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>   	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp&= ~I2CR_DMAEN;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
>   	return result;
>   }
>
> @@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
>
>   static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>   {
> -	int i, result;
> +	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
> +	unsigned int temp = 0;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct device *dev =&i2c_imx->adapter.dev;
>
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s>  write slave address: addr=0x%x\n",
> +	dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>   		__func__, msgs->addr<<  1);
> +	if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
> +		reinit_completion(&i2c_imx->dma->cmd_complete);
> +		dma->chan_using = dma->chan_tx;
> +		dma->dma_transfer_dir = DMA_MEM_TO_DEV;
> +		dma->dma_data_dir = DMA_TO_DEVICE;
> +		dma->dma_len = msgs->len - 1;
> +		result = i2c_imx_dma_xfer(i2c_imx, msgs);
> +		if (result)
> +			return result;
>
> -	/* write slave address */
> -	imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
> -	result = i2c_imx_trx_complete(i2c_imx);
> -	if (result)
> -		return result;
> -	result = i2c_imx_acked(i2c_imx);
> -	if (result)
> -		return result;
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s>  write data\n", __func__);
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp |= I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>
> -	/* write data */
> -	for (i = 0; i<  msgs->len; i++) {
> -		dev_dbg(&i2c_imx->adapter.dev,
> -			"<%s>  write byte: B%d=0x%X\n",
> -			__func__, i, msgs->buf[i]);
> -		imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
> +		/* write slave address */
> +		imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
> +		result = wait_for_completion_interruptible_timeout(
> +					&i2c_imx->dma->cmd_complete,
> +					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
> +		if (result<= 0) {
> +			dmaengine_terminate_all(dma->chan_using);
> +			if (result)
> +				return result;
> +			else
> +				return -ETIMEDOUT;
> +		}
> +
> +		/* waiting for Transfer complete. */
> +		while (timeout--) {
> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +			if (temp&  I2SR_ICF)
> +				break;
> +			udelay(10);
> +		}
> +
> +		if (!timeout)
> +			return -ETIMEDOUT;
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp&= ~I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		/* write the last byte */
> +		imx_i2c_write_reg(msgs->buf[msgs->len-1],
> +					i2c_imx, IMX_I2C_I2DR);
>   		result = i2c_imx_trx_complete(i2c_imx);
>   		if (result)
>   			return result;
> +
> +		result = i2c_imx_acked(i2c_imx);
> +		if (result)
> +			return result;
> +	} else {
> +		/* write slave address */
> +		imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
> +		result = i2c_imx_trx_complete(i2c_imx);
> +		if (result)
> +			return result;
> +
>   		result = i2c_imx_acked(i2c_imx);
>   		if (result)
>   			return result;
> +
> +		dev_dbg(dev, "<%s>  write data\n", __func__);
> +
> +		/* write data */
> +		for (i = 0; i<  msgs->len; i++) {
> +			dev_dbg(dev, "<%s>  write byte: B%d=0x%X\n",
> +				__func__, i, msgs->buf[i]);
> +			imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
> +			result = i2c_imx_trx_complete(i2c_imx);
> +			if (result)
> +				return result;
> +			result = i2c_imx_acked(i2c_imx);
> +			if (result)
> +				return result;
> +		}
>   	}
>   	return 0;
>   }
>
>   static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>   {
> -	int i, result;
> +	int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>   	unsigned int temp;
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct device *dev =&i2c_imx->adapter.dev;
>
> -	dev_dbg(&i2c_imx->adapter.dev,
> -		"<%s>  write slave address: addr=0x%x\n",
> +	dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>   		__func__, (msgs->addr<<  1) | 0x01);
>
>   	/* write slave address */
> @@ -476,7 +687,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>   	if (result)
>   		return result;
>
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s>  setup bus\n", __func__);
> +	dev_dbg(dev, "<%s>  setup bus\n", __func__);
>
>   	/* setup bus to read data */
>   	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> @@ -486,35 +697,81 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>   	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>   	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
>
> -	dev_dbg(&i2c_imx->adapter.dev, "<%s>  read data\n", __func__);
> +	dev_dbg(dev, "<%s>  read data\n", __func__);
>
> -	/* read data */
> -	for (i = 0; i<  msgs->len; i++) {
> -		result = i2c_imx_trx_complete(i2c_imx);
> +	if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp |= I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		reinit_completion(&i2c_imx->dma->cmd_complete);
> +		dma->chan_using = dma->chan_rx;
> +		dma->dma_transfer_dir = DMA_DEV_TO_MEM;
> +		dma->dma_data_dir = DMA_FROM_DEVICE;
> +		dma->dma_len = msgs->len - 2;
> +		result = i2c_imx_dma_xfer(i2c_imx, msgs);
>   		if (result)
>   			return result;
> -		if (i == (msgs->len - 1)) {
> -			/* It must generate STOP before read I2DR to prevent
> -			   controller from generating another clock cycle */
> -			dev_dbg(&i2c_imx->adapter.dev,
> -				"<%s>  clear MSTA\n", __func__);
> -			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -			temp&= ~(I2CR_MSTA | I2CR_MTX);
> -			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> -			i2c_imx_bus_busy(i2c_imx, 0);
> -			i2c_imx->stopped = 1;
> -		} else if (i == (msgs->len - 2)) {
> -			dev_dbg(&i2c_imx->adapter.dev,
> -				"<%s>  set TXAK\n", __func__);
> -			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -			temp |= I2CR_TXAK;
> -			imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +		result = wait_for_completion_interruptible_timeout(
> +					&i2c_imx->dma->cmd_complete,
> +					msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
> +		if (result<= 0) {
> +			dmaengine_terminate_all(dma->chan_using);
> +			if (result)
> +				return result;
> +			else
> +				return -ETIMEDOUT;
>   		}
> -		msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> -		dev_dbg(&i2c_imx->adapter.dev,
> -			"<%s>  read byte: B%d=0x%X\n",
> -			__func__, i, msgs->buf[i]);
> +
> +		/* waiting for Transfer complete. */
> +		while (timeout--) {
> +			temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +			if (temp&  I2SR_ICF)
> +				break;
> +			udelay(10);
> +		}
> +
> +		if(!timeout)
> +			return -ETIMEDOUT;
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +		temp&= ~I2CR_DMAEN;
> +		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +	} else {
> +		/* read data */
> +		for (i = 0; i<  msgs->len - 2; i++) {
> +			result = i2c_imx_trx_complete(i2c_imx);
> +			if (result)
> +				return result;
> +			msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +			dev_dbg(dev, "<%s>  read byte: B%d=0x%X\n",
> +				__func__, i, msgs->buf[i]);
> +		}
> +		result = i2c_imx_trx_complete(i2c_imx);
>   	}
> +
> +	/* read n-1 byte data */
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp |= I2CR_TXAK;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +	/* read n byte data */
> +	result = i2c_imx_trx_complete(i2c_imx);
> +	if (result)
> +		return result;
> +
> +	/*
> +	 * It must generate STOP before read I2DR to prevent
> +	 * controller from generating another clock cycle
> +	 */
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp&= ~(I2CR_MSTA | I2CR_MTX);
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +	i2c_imx_bus_busy(i2c_imx, 0);
> +	i2c_imx->stopped = 1;
> +	msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> +
>   	return 0;
>   }
>
> @@ -601,6 +858,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
>   	void __iomem *base;
>   	int irq, ret;
>   	u32 bitrate;
> +	u32 phy_addr;
>
>   	dev_dbg(&pdev->dev, "<%s>\n", __func__);
>
> @@ -611,6 +869,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
>   	}
>
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	phy_addr = res->start;
>   	base = devm_ioremap_resource(&pdev->dev, res);
>   	if (IS_ERR(base))
>   		return PTR_ERR(base);
> @@ -696,6 +955,10 @@ static int i2c_imx_probe(struct platform_device *pdev)
>   		i2c_imx->adapter.name);
>   	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>
> +	/* Init DMA config if support*/
> +	if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
> +		dev_warn(&pdev->dev, "can't request DMA\n");
> +
>   	return 0;   /* Return OK */
>   }
>
> @@ -707,6 +970,9 @@ static int i2c_imx_remove(struct platform_device *pdev)
>   	dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
>   	i2c_del_adapter(&i2c_imx->adapter);
>
> +	if (i2c_imx->dma)
> +		i2c_imx_dma_free(i2c_imx);
> +
>   	/* setup chip registers to defaults */
>   	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
>   	imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04  9:49   ` sourav
@ 2014-04-04 10:04     ` sourav
  2014-04-04 11:49     ` Marek Vasut
  1 sibling, 0 replies; 13+ messages in thread
From: sourav @ 2014-04-04 10:04 UTC (permalink / raw)
  To: linux-arm-kernel


>> +
>> +    dma->chan_tx = dma_request_slave_channel(dev, "tx");
>> +    return 0;

To be more clear,
return here looks to be some leftover.

> ?? Looks to be some leftover?
>> +    if (!dma->chan_tx) {
>> +        dev_info(dev, "DMA tx channel request failed\n");
>> +        ret = -ENODEV;
>> +        goto fail_al;
>> +    }
>> +
>> +    dma_sconfig.dst_addr = phy_addr +
>> +                (IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
>> +    dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> +    dma_sconfig.dst_maxburst = 1;
>> +    dma_sconfig.direction = DMA_MEM_TO_DEV;
>> +    ret = dmaengine_slave_config(dma->chan_tx,&dma_sconfig);
>> +    if (ret<  0) {
>> +        dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>> +        goto fail_tx;
>> +    }
>> +
>> +    dma->chan_rx = dma_request_slave_channel(dev, "rx");
>> +    if (!dma->chan_rx) {
>> +        dev_info(dev, "DMA rx channel request failed\n");
>> +        ret = -ENODEV;
>> +        goto fail_tx;
>> +    }
>> +
>> +    dma_sconfig.src_addr = phy_addr +
>> +                (IMX_I2C_I2DR<<  i2c_imx->hwdata->regshift);
>> +    dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> +    dma_sconfig.src_maxburst = 1;
>> +    dma_sconfig.direction = DMA_DEV_TO_MEM;
>> +    ret = dmaengine_slave_config(dma->chan_rx,&dma_sconfig);
>> +    if (ret<  0) {
>> +        dev_info(dev, "DMA slave config failed, err = %d\n", ret);
>> +        goto fail_rx;
>> +    }
>> +
>> +    i2c_imx->dma = dma;
>> +
>> +    init_completion(&dma->cmd_complete);
>> +
>> +    return 0;
>> +
>> +fail_rx:
>> +    dma_release_channel(dma->chan_rx);
>> +fail_tx:
>> +    dma_release_channel(dma->chan_tx);
>> +fail_al:
>> +    devm_kfree(dev, dma);
>> +
>> +    return ret;
>> +}
>> +
>> +static void i2c_imx_dma_callback(void *arg)
>> +{
>> +    struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +
>> +    dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
>> +            dma->dma_len, dma->dma_data_dir);
>> +    complete(&dma->cmd_complete);
>> +}
>> +
>> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
>> +                    struct i2c_msg *msgs)
>> +{
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct dma_async_tx_descriptor *txdesc;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>> +
>> +    dma->dma_buf = dma_map_single(dma->chan_using->device->dev, 
>> msgs->buf,
>> +                    dma->dma_len, dma->dma_data_dir);
>> +    if (dma_mapping_error(dma->chan_using->device->dev, 
>> dma->dma_buf)) {
>> +        dev_err(dev, "DMA mapping failed\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
>> +                    dma->dma_len, dma->dma_transfer_dir,
>> +                    DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>> +    if (!txdesc) {
>> +        dev_err(dev, "Not able to get desc for DMA xfer\n");
>> +        dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
>> +                    dma->dma_len, dma->dma_data_dir);
>> +        return -EINVAL;
>> +    }
>> +
>> +    txdesc->callback = i2c_imx_dma_callback;
>> +    txdesc->callback_param = i2c_imx;
>> +    dmaengine_submit(txdesc);
>> +    dma_async_issue_pending(dma->chan_using);
>> +
>> +    return 0;
>> +}
>> +
>> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
>> +{
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +
>> +    dma->dma_buf = 0;
>> +    dma->dma_len = 0;
>> +
>> +    dma_release_channel(dma->chan_tx);
>> +    dma->chan_tx = NULL;
>> +
>> +    dma_release_channel(dma->chan_rx);
>> +    dma->chan_rx = NULL;
>> +
>> +    dma->chan_using = NULL;
>> +}
>> +
>>   /** Functions for IMX I2C adapter driver 
>> ***************************************
>>   
>> *******************************************************************************/
>>
>> @@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct 
>> *i2c_imx)
>>
>>       temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>>       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp&= ~I2CR_DMAEN;
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>>       return result;
>>   }
>>
>> @@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void 
>> *dev_id)
>>
>>   static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct 
>> i2c_msg *msgs)
>>   {
>> -    int i, result;
>> +    int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>> +    unsigned int temp = 0;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  write slave address: 
>> addr=0x%x\n",
>> +    dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>>           __func__, msgs->addr<<  1);
>> +    if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
>> +        reinit_completion(&i2c_imx->dma->cmd_complete);
>> +        dma->chan_using = dma->chan_tx;
>> +        dma->dma_transfer_dir = DMA_MEM_TO_DEV;
>> +        dma->dma_data_dir = DMA_TO_DEVICE;
>> +        dma->dma_len = msgs->len - 1;
>> +        result = i2c_imx_dma_xfer(i2c_imx, msgs);
>> +        if (result)
>> +            return result;
>>
>> -    /* write slave address */
>> -    imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> -    result = i2c_imx_trx_complete(i2c_imx);
>> -    if (result)
>> -        return result;
>> -    result = i2c_imx_acked(i2c_imx);
>> -    if (result)
>> -        return result;
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  write data\n", __func__);
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp |= I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>>
>> -    /* write data */
>> -    for (i = 0; i<  msgs->len; i++) {
>> -        dev_dbg(&i2c_imx->adapter.dev,
>> -            "<%s>  write byte: B%d=0x%X\n",
>> -            __func__, i, msgs->buf[i]);
>> -        imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
>> +        /* write slave address */
>> +        imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> +        result = wait_for_completion_interruptible_timeout(
>> + &i2c_imx->dma->cmd_complete,
>> +                    msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
>> +        if (result<= 0) {
>> +            dmaengine_terminate_all(dma->chan_using);
>> +            if (result)
>> +                return result;
>> +            else
>> +                return -ETIMEDOUT;
>> +        }
>> +
>> +        /* waiting for Transfer complete. */
>> +        while (timeout--) {
>> +            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>> +            if (temp&  I2SR_ICF)
>> +                break;
>> +            udelay(10);
>> +        }
>> +
>> +        if (!timeout)
>> +            return -ETIMEDOUT;
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp&= ~I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        /* write the last byte */
>> +        imx_i2c_write_reg(msgs->buf[msgs->len-1],
>> +                    i2c_imx, IMX_I2C_I2DR);
>>           result = i2c_imx_trx_complete(i2c_imx);
>>           if (result)
>>               return result;
>> +
>> +        result = i2c_imx_acked(i2c_imx);
>> +        if (result)
>> +            return result;
>> +    } else {
>> +        /* write slave address */
>> +        imx_i2c_write_reg(msgs->addr<<  1, i2c_imx, IMX_I2C_I2DR);
>> +        result = i2c_imx_trx_complete(i2c_imx);
>> +        if (result)
>> +            return result;
>> +
>>           result = i2c_imx_acked(i2c_imx);
>>           if (result)
>>               return result;
>> +
>> +        dev_dbg(dev, "<%s>  write data\n", __func__);
>> +
>> +        /* write data */
>> +        for (i = 0; i<  msgs->len; i++) {
>> +            dev_dbg(dev, "<%s>  write byte: B%d=0x%X\n",
>> +                __func__, i, msgs->buf[i]);
>> +            imx_i2c_write_reg(msgs->buf[i], i2c_imx, IMX_I2C_I2DR);
>> +            result = i2c_imx_trx_complete(i2c_imx);
>> +            if (result)
>> +                return result;
>> +            result = i2c_imx_acked(i2c_imx);
>> +            if (result)
>> +                return result;
>> +        }
>>       }
>>       return 0;
>>   }
>>
>>   static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct 
>> i2c_msg *msgs)
>>   {
>> -    int i, result;
>> +    int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
>>       unsigned int temp;
>> +    struct imx_i2c_dma *dma = i2c_imx->dma;
>> +    struct device *dev =&i2c_imx->adapter.dev;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev,
>> -        "<%s>  write slave address: addr=0x%x\n",
>> +    dev_dbg(dev, "<%s>  write slave address: addr=0x%x\n",
>>           __func__, (msgs->addr<<  1) | 0x01);
>>
>>       /* write slave address */
>> @@ -476,7 +687,7 @@ static int i2c_imx_read(struct imx_i2c_struct 
>> *i2c_imx, struct i2c_msg *msgs)
>>       if (result)
>>           return result;
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  setup bus\n", __func__);
>> +    dev_dbg(dev, "<%s>  setup bus\n", __func__);
>>
>>       /* setup bus to read data */
>>       temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> @@ -486,35 +697,81 @@ static int i2c_imx_read(struct imx_i2c_struct 
>> *i2c_imx, struct i2c_msg *msgs)
>>       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>>       imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
>>
>> -    dev_dbg(&i2c_imx->adapter.dev, "<%s>  read data\n", __func__);
>> +    dev_dbg(dev, "<%s>  read data\n", __func__);
>>
>> -    /* read data */
>> -    for (i = 0; i<  msgs->len; i++) {
>> -        result = i2c_imx_trx_complete(i2c_imx);
>> +    if (dma&&  msgs->len>= IMX_I2C_DMA_THRESHOLD) {
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp |= I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        reinit_completion(&i2c_imx->dma->cmd_complete);
>> +        dma->chan_using = dma->chan_rx;
>> +        dma->dma_transfer_dir = DMA_DEV_TO_MEM;
>> +        dma->dma_data_dir = DMA_FROM_DEVICE;
>> +        dma->dma_len = msgs->len - 2;
>> +        result = i2c_imx_dma_xfer(i2c_imx, msgs);
>>           if (result)
>>               return result;
>> -        if (i == (msgs->len - 1)) {
>> -            /* It must generate STOP before read I2DR to prevent
>> -               controller from generating another clock cycle */
>> -            dev_dbg(&i2c_imx->adapter.dev,
>> -                "<%s>  clear MSTA\n", __func__);
>> -            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> -            temp&= ~(I2CR_MSTA | I2CR_MTX);
>> -            imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> -            i2c_imx_bus_busy(i2c_imx, 0);
>> -            i2c_imx->stopped = 1;
>> -        } else if (i == (msgs->len - 2)) {
>> -            dev_dbg(&i2c_imx->adapter.dev,
>> -                "<%s>  set TXAK\n", __func__);
>> -            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> -            temp |= I2CR_TXAK;
>> -            imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +        result = wait_for_completion_interruptible_timeout(
>> + &i2c_imx->dma->cmd_complete,
>> +                    msecs_to_jiffies(IMX_I2C_DMA_TIMEOUT));
>> +        if (result<= 0) {
>> +            dmaengine_terminate_all(dma->chan_using);
>> +            if (result)
>> +                return result;
>> +            else
>> +                return -ETIMEDOUT;
>>           }
>> -        msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> -        dev_dbg(&i2c_imx->adapter.dev,
>> -            "<%s>  read byte: B%d=0x%X\n",
>> -            __func__, i, msgs->buf[i]);
>> +
>> +        /* waiting for Transfer complete. */
>> +        while (timeout--) {
>> +            temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>> +            if (temp&  I2SR_ICF)
>> +                break;
>> +            udelay(10);
>> +        }
>> +
>> +        if(!timeout)
>> +            return -ETIMEDOUT;
>> +        temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +        temp&= ~I2CR_DMAEN;
>> +        imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +    } else {
>> +        /* read data */
>> +        for (i = 0; i<  msgs->len - 2; i++) {
>> +            result = i2c_imx_trx_complete(i2c_imx);
>> +            if (result)
>> +                return result;
>> +            msgs->buf[i] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +            dev_dbg(dev, "<%s>  read byte: B%d=0x%X\n",
>> +                __func__, i, msgs->buf[i]);
>> +        }
>> +        result = i2c_imx_trx_complete(i2c_imx);
>>       }
>> +
>> +    /* read n-1 byte data */
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp |= I2CR_TXAK;
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +
>> +    msgs->buf[msgs->len-2] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +    /* read n byte data */
>> +    result = i2c_imx_trx_complete(i2c_imx);
>> +    if (result)
>> +        return result;
>> +
>> +    /*
>> +     * It must generate STOP before read I2DR to prevent
>> +     * controller from generating another clock cycle
>> +     */
>> +    temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
>> +    temp&= ~(I2CR_MSTA | I2CR_MTX);
>> +    imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
>> +    i2c_imx_bus_busy(i2c_imx, 0);
>> +    i2c_imx->stopped = 1;
>> +    msgs->buf[msgs->len-1] = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
>> +
>>       return 0;
>>   }
>>
>> @@ -601,6 +858,7 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>       void __iomem *base;
>>       int irq, ret;
>>       u32 bitrate;
>> +    u32 phy_addr;
>>
>>       dev_dbg(&pdev->dev, "<%s>\n", __func__);
>>
>> @@ -611,6 +869,7 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>       }
>>
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    phy_addr = res->start;
>>       base = devm_ioremap_resource(&pdev->dev, res);
>>       if (IS_ERR(base))
>>           return PTR_ERR(base);
>> @@ -696,6 +955,10 @@ static int i2c_imx_probe(struct platform_device 
>> *pdev)
>>           i2c_imx->adapter.name);
>>       dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>>
>> +    /* Init DMA config if support*/
>> +    if (i2c_imx_dma_request(i2c_imx, (dma_addr_t)phy_addr))
>> +        dev_warn(&pdev->dev, "can't request DMA\n");
>> +
>>       return 0;   /* Return OK */
>>   }
>>
>> @@ -707,6 +970,9 @@ static int i2c_imx_remove(struct platform_device 
>> *pdev)
>>       dev_dbg(&i2c_imx->adapter.dev, "adapter removed\n");
>>       i2c_del_adapter(&i2c_imx->adapter);
>>
>> +    if (i2c_imx->dma)
>> +        i2c_imx_dma_free(i2c_imx);
>> +
>>       /* setup chip registers to defaults */
>>       imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
>>       imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04  9:49   ` sourav
  2014-04-04 10:04     ` sourav
@ 2014-04-04 11:49     ` Marek Vasut
  1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2014-04-04 11:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, April 04, 2014 at 11:49:39 AM, sourav wrote:

[...]

> > +/* Functions for DMA support */
> > +static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> > +						dma_addr_t phy_addr)
> > +{
> > +	struct imx_i2c_dma *dma;
> > +	struct dma_slave_config dma_sconfig;
> > +	struct device *dev =&i2c_imx->adapter.dev;
> > +	int ret;
> > +
> > +	dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
> > +	if (!dma) {
> > +		dev_info(dev, "can't allocate DMA struct\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	dma->chan_tx = dma_request_slave_channel(dev, "tx");
> > +	return 0;
> 
> ?? Looks to be some leftover?

Nice find.

btw. you might want to trim the email only to the relevant parts when replying 
so it's easier to find your commments in the entire body of text.
 
> > +	if (!dma->chan_tx) {
> > +		dev_info(dev, "DMA tx channel request failed\n");
> > +		ret = -ENODEV;
> > +		goto fail_al;
> > +	}

[...]

Best regards,
Marek Vasut

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04  2:41 ` [PATCH v4 1/2] " Yuan Yao
  2014-04-04  9:49   ` sourav
@ 2014-04-04 14:28   ` Marek Vasut
  2014-04-04 14:40     ` Wolfram Sang
  2014-04-04 15:45     ` Yao Yuan
  1 sibling, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2014-04-04 14:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
> Add dma support for i2c. This function depend on DMA driver.
> You can turn on it by write both the dmas and dma-name properties in dts
> node.
> 
> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>

[...]

Since you will be fixing that superfluous return 0; (I actually wonder, did you 
really test the driver at all before submitting it?) ...

> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> +					struct i2c_msg *msgs)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_async_tx_descriptor *txdesc;
> +	struct device *dev = &i2c_imx->adapter.dev;
> +
> +	dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,

Please fix this "noodle" here too, the chain of pointers is quite long, you can 
just define a variable for that.

> +					dma->dma_len, dma->dma_data_dir);
> +	if (dma_mapping_error(dma->chan_using->device->dev, dma->dma_buf)) {
> +		dev_err(dev, "DMA mapping failed\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
> +					dma->dma_len, dma->dma_transfer_dir,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!txdesc) {
> +		dev_err(dev, "Not able to get desc for DMA xfer\n");
> +		dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
> +					dma->dma_len, dma->dma_data_dir);
> +		return -EINVAL;
> +	}
> +
> +	txdesc->callback = i2c_imx_dma_callback;
> +	txdesc->callback_param = i2c_imx;
> +	dmaengine_submit(txdesc);
> +	dma_async_issue_pending(dma->chan_using);
> +
> +	return 0;
> +}
[...]

Other than those two things,

Reviewed-by: Marek Vasut <marex@denx.de>

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04 14:28   ` Marek Vasut
@ 2014-04-04 14:40     ` Wolfram Sang
  2014-04-04 15:45     ` Yao Yuan
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2014-04-04 14:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Apr 04, 2014 at 04:28:55PM +0200, Marek Vasut wrote:
> On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
> > Add dma support for i2c. This function depend on DMA driver.
> > You can turn on it by write both the dmas and dma-name properties in dts
> > node.
> > 
> > Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> 
> [...]
> 
> Since you will be fixing that superfluous return 0; (I actually wonder, did you 
> really test the driver at all before submitting it?) ...

Good question. @Yuan: Next time, please state what tests you did with
the new version of the driver.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140404/9d59872d/attachment.sig>

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04 14:28   ` Marek Vasut
  2014-04-04 14:40     ` Wolfram Sang
@ 2014-04-04 15:45     ` Yao Yuan
  2014-04-04 17:40       ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Yao Yuan @ 2014-04-04 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, April 04, 2014 at 22:29:32 PM, Marek Vasut wrote:
> On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
> > Add dma support for i2c. This function depend on DMA driver.
> > You can turn on it by write both the dmas and dma-name properties in dts
> > node.
> >
> > Signed-off-by: Yuan Yao <yao.yuan@freescale.com>

[...]

> Since you will be fixing that superfluous return 0; (I actually wonder, did you
> really test the driver at all before submitting it?) ...

I'm sorry for the leftover. It's a serious bug. The support by DMA will be disabled always.
Thanks for your consideration. But subjective assume may not suitable. Testing is a essential link . 
But the bug can't be found by normal test . The testing may not be  imperfect, it's my 
serious fault, but please don't doubt my work attitude.
The ugly is my work or even the ability but not the attitude, I think.

At last, Thank you for giving me a lot of help, it's very important for me.

> > +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> > +                                     struct i2c_msg *msgs)
> > +{
> > +     struct imx_i2c_dma *dma = i2c_imx->dma;
> > +     struct dma_async_tx_descriptor *txdesc;
> > +     struct device *dev = &i2c_imx->adapter.dev;
> +
> +     dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,
[...]

>  Reviewed-by: Marek Vasut <marex@denx.de>

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04 15:45     ` Yao Yuan
@ 2014-04-04 17:40       ` Wolfram Sang
  2014-05-21  8:00         ` Wolfram Sang
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2014-04-04 17:40 UTC (permalink / raw)
  To: linux-arm-kernel

> The ugly is my work or even the ability but not the attitude, I think.

Explaining which tests were done is what I wanted to request more often
from submitters anyhow. Your leftover (yes, this can happen) just
reminded me to actually do that :)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140404/0cac4c18/attachment.sig>

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-04-04 17:40       ` Wolfram Sang
@ 2014-05-21  8:00         ` Wolfram Sang
  2014-07-24  3:19           ` Yao Yuan
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2014-05-21  8:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Apr 04, 2014 at 07:40:11PM +0200, Wolfram Sang wrote:
> > The ugly is my work or even the ability but not the attitude, I think.
> 
> Explaining which tests were done is what I wanted to request more often
> from submitters anyhow. Your leftover (yes, this can happen) just
> reminded me to actually do that :)

Ping. Any updates? I think this was pretty close to inclusion?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140521/5aebd98c/attachment.sig>

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-05-21  8:00         ` Wolfram Sang
@ 2014-07-24  3:19           ` Yao Yuan
  2014-07-24  4:14             ` Marek Vasut
  0 siblings, 1 reply; 13+ messages in thread
From: Yao Yuan @ 2014-07-24  3:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote:
> 
> Ping. Any updates? I think this was pretty close to inclusion?

Hi, Wolfram
	Thanks for your concern. Sorry for my reply so late. I had on a business trip for months.
At that time I have no device to debug it. Now, I'm come back and I will try my best to finish it.
I have sent the patch for V5. Thanks for your review.

Best regards,
Yuan Yao

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

* [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
  2014-07-24  3:19           ` Yao Yuan
@ 2014-07-24  4:14             ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2014-07-24  4:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, July 24, 2014 at 05:19:27 AM, Yao Yuan wrote:
> On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote:
> > Ping. Any updates? I think this was pretty close to inclusion?
> 
> Hi, Wolfram
> 	Thanks for your concern. Sorry for my reply so late. I had on a business
> trip for months. At that time I have no device to debug it. Now, I'm come
> back and I will try my best to finish it. I have sent the patch for V5.
> Thanks for your review.

So you have no means to test this ? Can this be tested on MX5 ? MX6 ?

Best regards,
Marek Vasut

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

end of thread, other threads:[~2014-07-24  4:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-04  2:41 [PATCH v4 0/2] i2c: add DMA support for freescale i2c driver Yuan Yao
2014-04-04  2:41 ` [PATCH v4 1/2] " Yuan Yao
2014-04-04  9:49   ` sourav
2014-04-04 10:04     ` sourav
2014-04-04 11:49     ` Marek Vasut
2014-04-04 14:28   ` Marek Vasut
2014-04-04 14:40     ` Wolfram Sang
2014-04-04 15:45     ` Yao Yuan
2014-04-04 17:40       ` Wolfram Sang
2014-05-21  8:00         ` Wolfram Sang
2014-07-24  3:19           ` Yao Yuan
2014-07-24  4:14             ` Marek Vasut
2014-04-04  2:41 ` [PATCH v4 2/2] Documentation:add " Yuan Yao

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