Patchwork [1/3] i2c: add DMA support for freescale i2c driver

login
register
mail settings
Submitter Yao Yuan
Date Feb. 27, 2014, 6:05 a.m.
Message ID <1393481115-22136-2-git-send-email-yao.yuan@freescale.com>
Download mbox | patch
Permalink /patch/324703/
State Superseded
Headers show

Comments

Yao Yuan - Feb. 27, 2014, 6:05 a.m.
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.
And you should set ".has_dma_support" as true for dma support in imx_i2c_hwdata struct.

Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
---
 drivers/i2c/busses/i2c-imx.c | 358 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 344 insertions(+), 14 deletions(-)
Shawn Guo - Feb. 27, 2014, 12:55 p.m.
On Thu, Feb 27, 2014 at 02:05:14PM +0800, 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.
> And you should set ".has_dma_support" as true for dma support in imx_i2c_hwdata struct.
> 
> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>

Just added a few people who might be interested.

Shawn

> ---
>  drivers/i2c/busses/i2c-imx.c | 358 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 344 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index db895fb..6ec392b 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,9 @@
>  /* Default value */
>  #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
>  
> +/* enable DMA if transfer size is bigger than this threshold */
> +#define IMX_I2C_DMA_THRESHOLD	16
> +
>  /* IMX I2C registers:
>   * the I2C register offset is different between SoCs,
>   * to provid support for all these chips, split the
> @@ -88,6 +96,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
> @@ -172,6 +181,17 @@ struct imx_i2c_hwdata {
>  	unsigned		ndivs;
>  	unsigned		i2sr_clr_opcode;
>  	unsigned		i2cr_ien_opcode;
> +	bool			has_dma_support;
> +};
> +
> +struct imx_i2c_dma {
> +	struct dma_chan		*chan_tx;
> +	struct dma_chan		*chan_rx;
> +	dma_addr_t		buf_tx;
> +	dma_addr_t		buf_rx;
> +	unsigned int		len_tx;
> +	unsigned int		len_rx;
> +	struct completion	cmd_complete;
>  };
>  
>  struct imx_i2c_struct {
> @@ -184,6 +204,9 @@ struct imx_i2c_struct {
>  	int			stopped;
>  	unsigned int		ifdr; /* IMX_I2C_IFDR */
>  	const struct imx_i2c_hwdata	*hwdata;
> +
> +	bool			use_dma;
> +	struct imx_i2c_dma	*dma;
>  };
>  
>  static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
>  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> +	.has_dma_support	= false,
>  
>  };
>  
> @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  = {
>  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> +	.has_dma_support	= false,
>  
>  };
>  
> @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
>  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> +	.has_dma_support	= true,
>  
>  };
>  
> @@ -254,6 +280,155 @@ 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, u32 phy_addr)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_slave_config dma_sconfig;
> +	int ret;
> +
> +	dma->chan_tx = dma_request_slave_channel(&i2c_imx->adapter.dev, "tx");
> +	if (!dma->chan_tx) {
> +		dev_err(&i2c_imx->adapter.dev,
> +				"Dma tx channel request failed!\n");
> +		return -ENODEV;
> +	}
> +
> +	dma_sconfig.dst_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
> +				"Dma slave config failed, err = %d\n", ret);
> +		dma_release_channel(dma->chan_tx);
> +		return ret;
> +	}
> +
> +	dma->chan_rx = dma_request_slave_channel(&i2c_imx->adapter.dev, "rx");
> +	if (!dma->chan_rx) {
> +		dev_err(&i2c_imx->adapter.dev,
> +				"Dma rx channel request failed!\n");
> +		return -ENODEV;
> +	}
> +
> +	dma_sconfig.src_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
> +				"Dma slave config failed, err = %d\n", ret);
> +		dma_release_channel(dma->chan_rx);
> +		return ret;
> +	}
> +
> +	init_completion(&dma->cmd_complete);
> +
> +	return 0;
> +}
> +
> +static void i2c_imx_dma_tx_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_tx->device->dev, dma->buf_tx,
> +			dma->len_tx, DMA_TO_DEVICE);
> +	complete(&dma->cmd_complete);
> +}
> +
> +static int i2c_imx_dma_tx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_async_tx_descriptor *txdesc;
> +
> +	dma->len_tx = msgs->len-1;
> +	dma->buf_tx = dma_map_single(dma->chan_tx->device->dev, msgs->buf,
> +					dma->len_tx, DMA_TO_DEVICE);
> +	if (dma_mapping_error(dma->chan_tx->device->dev, dma->buf_tx)) {
> +		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc = dmaengine_prep_slave_single(dma->chan_tx, dma->buf_tx,
> +					dma->len_tx, DMA_MEM_TO_DEV,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!txdesc) {
> +		dev_err(&i2c_imx->adapter.dev,
> +				"Not able to get desc for tx\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc->callback = i2c_imx_dma_tx_callback;
> +	txdesc->callback_param = i2c_imx;
> +	dmaengine_submit(txdesc);
> +	dma_async_issue_pending(dma->chan_tx);
> +	
> +	return 0;
> +}
> +
> +static void i2c_imx_dma_rx_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_rx->device->dev, dma->buf_rx,
> +				dma->len_rx, DMA_FROM_DEVICE);
> +	complete(&dma->cmd_complete);
> +}
> +
> +static int i2c_imx_dma_rx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_async_tx_descriptor *txdesc;
> +
> +	dma->len_rx = msgs->len - 2;
> +	dma->buf_rx = dma_map_single(dma->chan_rx->device->dev, msgs->buf,
> +					dma->len_rx, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dma->chan_rx->device->dev, dma->buf_rx)) {
> +		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc = dmaengine_prep_slave_single(dma->chan_rx, dma->buf_rx,
> +					dma->len_rx, DMA_DEV_TO_MEM,
> +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (!txdesc) {
> +		dev_err(&i2c_imx->adapter.dev,
> +				"Not able to get desc for rx\n");
> +		return -EINVAL;
> +	}
> +
> +	txdesc->callback = i2c_imx_dma_rx_callback;
> +	txdesc->callback_param = i2c_imx;
> +	dmaengine_submit(txdesc);
> +	dma_async_issue_pending(dma->chan_rx);
> +	
> +	return 0;
> +}
> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_chan *dma_chan;
> +
> +	dma_chan = dma->chan_tx;
> +	dma->chan_tx = NULL;
> +	dma->buf_tx = 0;
> +	dma->len_tx = 0;
> +	dma_release_channel(dma_chan);
> +
> +	dma_chan = dma->chan_rx;
> +	dma->chan_tx = NULL;
> +	dma->buf_rx = 0;
> +	dma->len_rx = 0;
> +	dma_release_channel(dma_chan);
> +}
>  /** Functions for IMX I2C adapter driver ***************************************
>  *******************************************************************************/
>  
> @@ -425,7 +600,8 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
>  	return IRQ_NONE;
>  }
>  
> -static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> +static int i2c_imx_pio_write(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
>  {
>  	int i, result;
>  
> @@ -458,7 +634,56 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  	return 0;
>  }
>  
> -static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> +static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
> +{
> +	int result, timeout=1000;
> +	unsigned int temp = 0;
> +
> +	reinit_completion(&i2c_imx->dma->cmd_complete);
> +	result = i2c_imx_dma_tx(i2c_imx, msgs);
> +	if(result)
> +		return result;
> +
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp |= I2CR_DMAEN;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	/* 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(1000));
> +	if (result == 0)
> +		return  -ETIMEDOUT;
> +
> +	/* waiting for Transfer complete. */
> +	while(timeout--) {
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +		if (temp & 0x80)
> +			break;
> +		udelay(10);
> +	}
> +
> +	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;
> +
> +	return 0;
> +}
> +
> +static int i2c_imx_pio_read(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
>  {
>  	int i, result;
>  	unsigned int temp;
> @@ -518,6 +743,80 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  	return 0;
>  }
>  
> +static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
> +{
> +	int result, timeout=1000;
> +	unsigned int temp;
> +
> +	/* write slave address */
> +	imx_i2c_write_reg((msgs->addr << 1) | 0x01, 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;
> +
> +	/* setup bus to read data */
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp &= ~I2CR_MTX;
> +	if (msgs->len - 1)
> +		temp &= ~I2CR_TXAK;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
> +
> +	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);
> +	result = i2c_imx_dma_rx(i2c_imx, msgs);
> +	if(result)
> +		return result;
> +
> +	result = wait_for_completion_interruptible_timeout(
> +						&i2c_imx->dma->cmd_complete,
> +						msecs_to_jiffies(1000));
> +	if (result == 0)
> +		return  -ETIMEDOUT;
> +
> +	/* waiting for Transfer complete. */
> +	while(timeout--) {
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +		if (temp & 0x80)
> +			break;
> +		udelay(10);
> +	}
> +
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp &= ~I2CR_DMAEN;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	/* 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;
> +}
> +
>  static int i2c_imx_xfer(struct i2c_adapter *adapter,
>  						struct i2c_msg *msgs, int num)
>  {
> @@ -563,10 +862,18 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>  			(temp & I2SR_SRW ? 1 : 0), (temp & I2SR_IIF ? 1 : 0),
>  			(temp & I2SR_RXAK ? 1 : 0));
>  #endif
> -		if (msgs[i].flags & I2C_M_RD)
> -			result = i2c_imx_read(i2c_imx, &msgs[i]);
> -		else
> -			result = i2c_imx_write(i2c_imx, &msgs[i]);
> +		if(i2c_imx->use_dma && msgs[i].len >= IMX_I2C_DMA_THRESHOLD) {
> +			if (msgs[i].flags & I2C_M_RD)
> +				result = i2c_imx_dma_read(i2c_imx, &msgs[i]);
> +			else
> +				result = i2c_imx_dma_write(i2c_imx, &msgs[i]);
> +		} else {
> +			if (msgs[i].flags & I2C_M_RD)
> +				result = i2c_imx_pio_read(i2c_imx, &msgs[i]);
> +			else
> +				result = i2c_imx_pio_write(i2c_imx, &msgs[i]);
> +		}
> +
>  		if (result)
>  			goto fail0;
>  	}
> @@ -601,6 +908,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 +919,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 +1005,24 @@ 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->hwdata->has_dma_support) {
> +		dev_info(&pdev->dev, "has_dma_support_begin.\n");
> +		i2c_imx->dma = devm_kzalloc(&pdev->dev, sizeof(struct imx_i2c_dma),
> +				GFP_KERNEL);
> +		if (!i2c_imx->dma) {
> +			dev_info(&pdev->dev, "can't allocate dma struct faild use dma.\n");
> +			i2c_imx->use_dma = false;
> +		}
> +		else if (i2c_imx_dma_request(i2c_imx, phy_addr)) {
> +			dev_info(&pdev->dev, "can't request dma chan, faild use dma.\n");
> +			i2c_imx->use_dma = false;
> +		}
> +		else {
> +			i2c_imx->use_dma = true;
> +		}
> +	}
> +
>  	return 0;   /* Return OK */
>  }
>  
> @@ -707,6 +1034,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->use_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
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo - Feb. 27, 2014, 1:21 p.m.
On Thu, Feb 27, 2014 at 08:55:45PM +0800, Shawn Guo wrote:
> On Thu, Feb 27, 2014 at 02:05:14PM +0800, 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.
> > And you should set ".has_dma_support" as true for dma support in imx_i2c_hwdata struct.
> > 
> > Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> 
> Just added a few people who might be interested.

Still missed one - Marek.  Sorry.

Shawn

> 
> > ---
> >  drivers/i2c/busses/i2c-imx.c | 358 +++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 344 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> > index db895fb..6ec392b 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,9 @@
> >  /* Default value */
> >  #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
> >  
> > +/* enable DMA if transfer size is bigger than this threshold */
> > +#define IMX_I2C_DMA_THRESHOLD	16
> > +
> >  /* IMX I2C registers:
> >   * the I2C register offset is different between SoCs,
> >   * to provid support for all these chips, split the
> > @@ -88,6 +96,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
> > @@ -172,6 +181,17 @@ struct imx_i2c_hwdata {
> >  	unsigned		ndivs;
> >  	unsigned		i2sr_clr_opcode;
> >  	unsigned		i2cr_ien_opcode;
> > +	bool			has_dma_support;
> > +};
> > +
> > +struct imx_i2c_dma {
> > +	struct dma_chan		*chan_tx;
> > +	struct dma_chan		*chan_rx;
> > +	dma_addr_t		buf_tx;
> > +	dma_addr_t		buf_rx;
> > +	unsigned int		len_tx;
> > +	unsigned int		len_rx;
> > +	struct completion	cmd_complete;
> >  };
> >  
> >  struct imx_i2c_struct {
> > @@ -184,6 +204,9 @@ struct imx_i2c_struct {
> >  	int			stopped;
> >  	unsigned int		ifdr; /* IMX_I2C_IFDR */
> >  	const struct imx_i2c_hwdata	*hwdata;
> > +
> > +	bool			use_dma;
> > +	struct imx_i2c_dma	*dma;
> >  };
> >  
> >  static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> > @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> >  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > +	.has_dma_support	= false,
> >  
> >  };
> >  
> > @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  = {
> >  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > +	.has_dma_support	= false,
> >  
> >  };
> >  
> > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
> >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > +	.has_dma_support	= true,
> >  
> >  };
> >  
> > @@ -254,6 +280,155 @@ 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, u32 phy_addr)
> > +{
> > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > +	struct dma_slave_config dma_sconfig;
> > +	int ret;
> > +
> > +	dma->chan_tx = dma_request_slave_channel(&i2c_imx->adapter.dev, "tx");
> > +	if (!dma->chan_tx) {
> > +		dev_err(&i2c_imx->adapter.dev,
> > +				"Dma tx channel request failed!\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	dma_sconfig.dst_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
> > +				"Dma slave config failed, err = %d\n", ret);
> > +		dma_release_channel(dma->chan_tx);
> > +		return ret;
> > +	}
> > +
> > +	dma->chan_rx = dma_request_slave_channel(&i2c_imx->adapter.dev, "rx");
> > +	if (!dma->chan_rx) {
> > +		dev_err(&i2c_imx->adapter.dev,
> > +				"Dma rx channel request failed!\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	dma_sconfig.src_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
> > +				"Dma slave config failed, err = %d\n", ret);
> > +		dma_release_channel(dma->chan_rx);
> > +		return ret;
> > +	}
> > +
> > +	init_completion(&dma->cmd_complete);
> > +
> > +	return 0;
> > +}
> > +
> > +static void i2c_imx_dma_tx_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_tx->device->dev, dma->buf_tx,
> > +			dma->len_tx, DMA_TO_DEVICE);
> > +	complete(&dma->cmd_complete);
> > +}
> > +
> > +static int i2c_imx_dma_tx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> > +{
> > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > +	struct dma_async_tx_descriptor *txdesc;
> > +
> > +	dma->len_tx = msgs->len-1;
> > +	dma->buf_tx = dma_map_single(dma->chan_tx->device->dev, msgs->buf,
> > +					dma->len_tx, DMA_TO_DEVICE);
> > +	if (dma_mapping_error(dma->chan_tx->device->dev, dma->buf_tx)) {
> > +		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	txdesc = dmaengine_prep_slave_single(dma->chan_tx, dma->buf_tx,
> > +					dma->len_tx, DMA_MEM_TO_DEV,
> > +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> > +	if (!txdesc) {
> > +		dev_err(&i2c_imx->adapter.dev,
> > +				"Not able to get desc for tx\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	txdesc->callback = i2c_imx_dma_tx_callback;
> > +	txdesc->callback_param = i2c_imx;
> > +	dmaengine_submit(txdesc);
> > +	dma_async_issue_pending(dma->chan_tx);
> > +	
> > +	return 0;
> > +}
> > +
> > +static void i2c_imx_dma_rx_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_rx->device->dev, dma->buf_rx,
> > +				dma->len_rx, DMA_FROM_DEVICE);
> > +	complete(&dma->cmd_complete);
> > +}
> > +
> > +static int i2c_imx_dma_rx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> > +{
> > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > +	struct dma_async_tx_descriptor *txdesc;
> > +
> > +	dma->len_rx = msgs->len - 2;
> > +	dma->buf_rx = dma_map_single(dma->chan_rx->device->dev, msgs->buf,
> > +					dma->len_rx, DMA_FROM_DEVICE);
> > +	if (dma_mapping_error(dma->chan_rx->device->dev, dma->buf_rx)) {
> > +		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	txdesc = dmaengine_prep_slave_single(dma->chan_rx, dma->buf_rx,
> > +					dma->len_rx, DMA_DEV_TO_MEM,
> > +					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> > +	if (!txdesc) {
> > +		dev_err(&i2c_imx->adapter.dev,
> > +				"Not able to get desc for rx\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	txdesc->callback = i2c_imx_dma_rx_callback;
> > +	txdesc->callback_param = i2c_imx;
> > +	dmaengine_submit(txdesc);
> > +	dma_async_issue_pending(dma->chan_rx);
> > +	
> > +	return 0;
> > +}
> > +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
> > +{
> > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > +	struct dma_chan *dma_chan;
> > +
> > +	dma_chan = dma->chan_tx;
> > +	dma->chan_tx = NULL;
> > +	dma->buf_tx = 0;
> > +	dma->len_tx = 0;
> > +	dma_release_channel(dma_chan);
> > +
> > +	dma_chan = dma->chan_rx;
> > +	dma->chan_tx = NULL;
> > +	dma->buf_rx = 0;
> > +	dma->len_rx = 0;
> > +	dma_release_channel(dma_chan);
> > +}
> >  /** Functions for IMX I2C adapter driver ***************************************
> >  *******************************************************************************/
> >  
> > @@ -425,7 +600,8 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> >  	return IRQ_NONE;
> >  }
> >  
> > -static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> > +static int i2c_imx_pio_write(struct imx_i2c_struct *i2c_imx,
> > +							struct i2c_msg *msgs)
> >  {
> >  	int i, result;
> >  
> > @@ -458,7 +634,56 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> >  	return 0;
> >  }
> >  
> > -static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> > +static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
> > +							struct i2c_msg *msgs)
> > +{
> > +	int result, timeout=1000;
> > +	unsigned int temp = 0;
> > +
> > +	reinit_completion(&i2c_imx->dma->cmd_complete);
> > +	result = i2c_imx_dma_tx(i2c_imx, msgs);
> > +	if(result)
> > +		return result;
> > +
> > +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +	temp |= I2CR_DMAEN;
> > +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +	/* 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(1000));
> > +	if (result == 0)
> > +		return  -ETIMEDOUT;
> > +
> > +	/* waiting for Transfer complete. */
> > +	while(timeout--) {
> > +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +		if (temp & 0x80)
> > +			break;
> > +		udelay(10);
> > +	}
> > +
> > +	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;
> > +
> > +	return 0;
> > +}
> > +
> > +static int i2c_imx_pio_read(struct imx_i2c_struct *i2c_imx,
> > +							struct i2c_msg *msgs)
> >  {
> >  	int i, result;
> >  	unsigned int temp;
> > @@ -518,6 +743,80 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> >  	return 0;
> >  }
> >  
> > +static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> > +							struct i2c_msg *msgs)
> > +{
> > +	int result, timeout=1000;
> > +	unsigned int temp;
> > +
> > +	/* write slave address */
> > +	imx_i2c_write_reg((msgs->addr << 1) | 0x01, 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;
> > +
> > +	/* setup bus to read data */
> > +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +	temp &= ~I2CR_MTX;
> > +	if (msgs->len - 1)
> > +		temp &= ~I2CR_TXAK;
> > +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
> > +
> > +	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);
> > +	result = i2c_imx_dma_rx(i2c_imx, msgs);
> > +	if(result)
> > +		return result;
> > +
> > +	result = wait_for_completion_interruptible_timeout(
> > +						&i2c_imx->dma->cmd_complete,
> > +						msecs_to_jiffies(1000));
> > +	if (result == 0)
> > +		return  -ETIMEDOUT;
> > +
> > +	/* waiting for Transfer complete. */
> > +	while(timeout--) {
> > +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +		if (temp & 0x80)
> > +			break;
> > +		udelay(10);
> > +	}
> > +
> > +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +	temp &= ~I2CR_DMAEN;
> > +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +	/* 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;
> > +}
> > +
> >  static int i2c_imx_xfer(struct i2c_adapter *adapter,
> >  						struct i2c_msg *msgs, int num)
> >  {
> > @@ -563,10 +862,18 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
> >  			(temp & I2SR_SRW ? 1 : 0), (temp & I2SR_IIF ? 1 : 0),
> >  			(temp & I2SR_RXAK ? 1 : 0));
> >  #endif
> > -		if (msgs[i].flags & I2C_M_RD)
> > -			result = i2c_imx_read(i2c_imx, &msgs[i]);
> > -		else
> > -			result = i2c_imx_write(i2c_imx, &msgs[i]);
> > +		if(i2c_imx->use_dma && msgs[i].len >= IMX_I2C_DMA_THRESHOLD) {
> > +			if (msgs[i].flags & I2C_M_RD)
> > +				result = i2c_imx_dma_read(i2c_imx, &msgs[i]);
> > +			else
> > +				result = i2c_imx_dma_write(i2c_imx, &msgs[i]);
> > +		} else {
> > +			if (msgs[i].flags & I2C_M_RD)
> > +				result = i2c_imx_pio_read(i2c_imx, &msgs[i]);
> > +			else
> > +				result = i2c_imx_pio_write(i2c_imx, &msgs[i]);
> > +		}
> > +
> >  		if (result)
> >  			goto fail0;
> >  	}
> > @@ -601,6 +908,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 +919,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 +1005,24 @@ 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->hwdata->has_dma_support) {
> > +		dev_info(&pdev->dev, "has_dma_support_begin.\n");
> > +		i2c_imx->dma = devm_kzalloc(&pdev->dev, sizeof(struct imx_i2c_dma),
> > +				GFP_KERNEL);
> > +		if (!i2c_imx->dma) {
> > +			dev_info(&pdev->dev, "can't allocate dma struct faild use dma.\n");
> > +			i2c_imx->use_dma = false;
> > +		}
> > +		else if (i2c_imx_dma_request(i2c_imx, phy_addr)) {
> > +			dev_info(&pdev->dev, "can't request dma chan, faild use dma.\n");
> > +			i2c_imx->use_dma = false;
> > +		}
> > +		else {
> > +			i2c_imx->use_dma = true;
> > +		}
> > +	}
> > +
> >  	return 0;   /* Return OK */
> >  }
> >  
> > @@ -707,6 +1034,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->use_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
> > 
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - Feb. 27, 2014, 8:39 p.m.
On Thursday, February 27, 2014 at 07:05:14 AM, Yuan Yao wrote:

[...]

> *****/ @@ -63,6 +68,9 @@
>  /* Default value */
>  #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
> 
> +/* enable DMA if transfer size is bigger than this threshold */
> +#define IMX_I2C_DMA_THRESHOLD	16

So what's the unit here , potatoes or beers or what ? I suppose it's bytes , but 
please make it explicit in the comment ...

[...]

>  static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
>  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> +	.has_dma_support	= false,
> 
>  };
> 
> @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  =
> { .ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> +	.has_dma_support	= false,
> 
>  };
> 
> @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
>  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
>  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
>  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> +	.has_dma_support	= true,

So why exactly don't we have a DT prop for determining whether the controller 
has DMA support ?

What about the other controllers, do they not support DMA for some specific 
reason? Please elaborate on that, thank you !
[...]

> +static void i2c_imx_dma_tx_callback(void *arg)
[...]
> +static int i2c_imx_dma_tx(struct imx_i2c_struct *i2c_imx, struct i2c_msg
> *msgs) +{
[...]
> +static void i2c_imx_dma_rx_callback(void *arg)
[...]
> +static int i2c_imx_dma_rx(struct imx_i2c_struct *i2c_imx, struct i2c_msg
> *msgs) +{
[...]

Looks like there's quite a bit of code duplication in the four functions above, 
can you not unify them ?

Also, can the DMA not do full-duplex operation ? What I see here is just half-
duplex operations , one for RX and the other one for TX .

> +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
> +{
> +	struct imx_i2c_dma *dma = i2c_imx->dma;
> +	struct dma_chan *dma_chan;
> +
> +	dma_chan = dma->chan_tx;
> +	dma->chan_tx = NULL;
> +	dma->buf_tx = 0;
> +	dma->len_tx = 0;
> +	dma_release_channel(dma_chan);
> +
> +	dma_chan = dma->chan_rx;
> +	dma->chan_tx = NULL;
> +	dma->buf_rx = 0;
> +	dma->len_rx = 0;
> +	dma_release_channel(dma_chan);

You must make _DEAD_ _SURE_ this function is not ever called while the DMA is 
still active. In your case, I have a feeling that's not handled.

> +}
>  /** Functions for IMX I2C adapter driver
> ***************************************
> **************************************************************************
> *****/
> 
> @@ -425,7 +600,8 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
>  	return IRQ_NONE;
>  }
> 
> -static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg
> *msgs) +static int i2c_imx_pio_write(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
>  {
>  	int i, result;
> 
> @@ -458,7 +634,56 @@ static int i2c_imx_write(struct imx_i2c_struct
> *i2c_imx, struct i2c_msg *msgs) return 0;
>  }
> 
> -static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg
> *msgs) +static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
> +{
> +	int result, timeout=1000;
> +	unsigned int temp = 0;
> +
> +	reinit_completion(&i2c_imx->dma->cmd_complete);
> +	result = i2c_imx_dma_tx(i2c_imx, msgs);
> +	if(result)
> +		return result;
> +
> +	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +	temp |= I2CR_DMAEN;
> +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> +
> +	/* 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(1000));

Pull the magic constant of 1000 out and #define it as some I2C_IMX_DMA_TIMEOUT 
please .

> +	if (result == 0)
> +		return  -ETIMEDOUT;
> +
> +	/* waiting for Transfer complete. */
> +	while(timeout--) {
> +		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> +		if (temp & 0x80)
> +			break;
> +		udelay(10);
> +	}
> +
> +	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;
> +
> +	return 0;
> +}
> +
> +static int i2c_imx_pio_read(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
>  {
>  	int i, result;
>  	unsigned int temp;
> @@ -518,6 +743,80 @@ static int i2c_imx_read(struct imx_i2c_struct
> *i2c_imx, struct i2c_msg *msgs) return 0;
>  }
> 
> +static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> +							struct i2c_msg *msgs)
> +{

Looks like almost an duplication as well...

Besides, full-duplex DMA operation is missing, please explain why.

THanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo - Feb. 28, 2014, 2:13 a.m.
On Thu, Feb 27, 2014 at 09:39:35PM +0100, Marek Vasut wrote:
> > @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> >  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > +	.has_dma_support	= false,
> > 
> >  };
> > 
> > @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  =
> > { .ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > +	.has_dma_support	= false,
> > 
> >  };
> > 
> > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
> >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > +	.has_dma_support	= true,
> 
> So why exactly don't we have a DT prop for determining whether the controller 
> has DMA support ?

Using DMA or PIO is a decision that should be made by driver on its own,
not device tree.

Shawn

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo - Feb. 28, 2014, 2:23 a.m.
On Fri, Feb 28, 2014 at 10:13:02AM +0800, Shawn Guo wrote:
> On Thu, Feb 27, 2014 at 09:39:35PM +0100, Marek Vasut wrote:
> > > @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
> > >  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > > +	.has_dma_support	= false,
> > > 
> > >  };
> > > 
> > > @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata  =
> > > { .ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > > +	.has_dma_support	= false,
> > > 
> > >  };
> > > 
> > > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
> > >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > > +	.has_dma_support	= true,
> > 
> > So why exactly don't we have a DT prop for determining whether the controller 
> > has DMA support ?
> 
> Using DMA or PIO is a decision that should be made by driver on its own,
> not device tree.

Sorry.  I misunderstood it.  Yes, we can look at the DT property 'dmas'
to know if the controller has DMA capability.

Shawn

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yao Yuan - Feb. 28, 2014, 5:19 a.m.
SGkgTWFyZWssDQoNClRoYW5rIHlvdSB2ZXJ5IG11Y2ggZm9yIHlvdXIgc3VnZ2VzdGlvbi4NCg0K
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBNYXJlayBWYXN1dCBbbWFpbHRv
Om1hcmV4QGRlbnguZGVdDQo+IFNlbnQ6IEZyaWRheSwgRmVicnVhcnkgMjgsIDIwMTQgNDo0MCBB
TQ0KPiBUbzogbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnDQo+IENjOiBZdWFu
IFlhby1CNDY2ODM7IHdzYUB0aGUtZHJlYW1zLmRlOyBtYXJrLnJ1dGxhbmRAYXJtLmNvbTsNCj4g
c2hhd24uZ3VvQGxpbmFyby5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4
LQ0KPiBpMmNAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMS8zXSBpMmM6
IGFkZCBETUEgc3VwcG9ydCBmb3IgZnJlZXNjYWxlIGkyYyBkcml2ZXINCj4gDQo+IE9uIFRodXJz
ZGF5LCBGZWJydWFyeSAyNywgMjAxNCBhdCAwNzowNToxNCBBTSwgWXVhbiBZYW8gd3JvdGU6DQo+
IA0KPiBbLi4uXQ0KPiANCj4gPiAqKioqKi8gQEAgLTYzLDYgKzY4LDkgQEANCj4gPiAgLyogRGVm
YXVsdCB2YWx1ZSAqLw0KPiA+ICAjZGVmaW5lIElNWF9JMkNfQklUX1JBVEUJMTAwMDAwCS8qIDEw
MGtIeiAqLw0KPiA+DQo+ID4gKy8qIGVuYWJsZSBETUEgaWYgdHJhbnNmZXIgc2l6ZSBpcyBiaWdn
ZXIgdGhhbiB0aGlzIHRocmVzaG9sZCAqLw0KPiA+ICsjZGVmaW5lIElNWF9JMkNfRE1BX1RIUkVT
SE9MRAkxNg0KPiANCj4gU28gd2hhdCdzIHRoZSB1bml0IGhlcmUgLCBwb3RhdG9lcyBvciBiZWVy
cyBvciB3aGF0ID8gSSBzdXBwb3NlIGl0J3MNCj4gYnl0ZXMgLCBidXQgcGxlYXNlIG1ha2UgaXQg
ZXhwbGljaXQgaW4gdGhlIGNvbW1lbnQgLi4uDQo+IA0KDQpZZXMgaXQncyBieXRlcy4gSSB3aWxs
IG1ha2UgaXQgZXhwbGljaXQgaW4gdGhlIGNvbW1lbnQuDQoNCj4gWy4uLl0NCj4gDQo+ID4gIHN0
YXRpYyBjb25zdCBzdHJ1Y3QgaW14X2kyY19od2RhdGEgaW14MV9pMmNfaHdkYXRhICA9IHsgQEAg
LTE5Myw2DQo+ID4gKzIxNiw3IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3QgaW14X2kyY19od2RhdGEg
aW14MV9pMmNfaHdkYXRhICA9IHsNCj4gPiAgCS5uZGl2cwkJCT0gQVJSQVlfU0laRShpbXhfaTJj
X2Nsa19kaXYpLA0KPiA+ICAJLmkyc3JfY2xyX29wY29kZQk9IEkyU1JfQ0xSX09QQ09ERV9XMEMs
DQo+ID4gIAkuaTJjcl9pZW5fb3Bjb2RlCT0gSTJDUl9JRU5fT1BDT0RFXzEsDQo+ID4gKwkuaGFz
X2RtYV9zdXBwb3J0CT0gZmFsc2UsDQo+ID4NCj4gPiAgfTsNCj4gPg0KPiA+IEBAIC0yMDMsNiAr
MjI3LDcgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBpbXhfaTJjX2h3ZGF0YSBpbXgyMV9pMmNfaHdk
YXRhDQo+ID0NCj4gPiB7IC5uZGl2cwkJCT0gQVJSQVlfU0laRShpbXhfaTJjX2Nsa19kaXYpLA0K
PiA+ICAJLmkyc3JfY2xyX29wY29kZQk9IEkyU1JfQ0xSX09QQ09ERV9XMEMsDQo+ID4gIAkuaTJj
cl9pZW5fb3Bjb2RlCT0gSTJDUl9JRU5fT1BDT0RFXzEsDQo+ID4gKwkuaGFzX2RtYV9zdXBwb3J0
CT0gZmFsc2UsDQo+ID4NCj4gPiAgfTsNCj4gPg0KPiA+IEBAIC0yMTMsNiArMjM4LDcgQEAgc3Rh
dGljIHN0cnVjdCBpbXhfaTJjX2h3ZGF0YSB2ZjYxMF9pMmNfaHdkYXRhID0gew0KPiA+ICAJLm5k
aXZzCQkJPSBBUlJBWV9TSVpFKHZmNjEwX2kyY19jbGtfZGl2KSwNCj4gPiAgCS5pMnNyX2Nscl9v
cGNvZGUJPSBJMlNSX0NMUl9PUENPREVfVzFDLA0KPiA+ICAJLmkyY3JfaWVuX29wY29kZQk9IEky
Q1JfSUVOX09QQ09ERV8wLA0KPiA+ICsJLmhhc19kbWFfc3VwcG9ydAk9IHRydWUsDQo+IA0KPiBT
byB3aHkgZXhhY3RseSBkb24ndCB3ZSBoYXZlIGEgRFQgcHJvcCBmb3IgZGV0ZXJtaW5pbmcgd2hl
dGhlciB0aGUNCj4gY29udHJvbGxlciBoYXMgRE1BIHN1cHBvcnQgPw0KPiANCj4gV2hhdCBhYm91
dCB0aGUgb3RoZXIgY29udHJvbGxlcnMsIGRvIHRoZXkgbm90IHN1cHBvcnQgRE1BIGZvciBzb21l
DQo+IHNwZWNpZmljIHJlYXNvbj8gUGxlYXNlIGVsYWJvcmF0ZSBvbiB0aGF0LCB0aGFuayB5b3Ug
IQ0KDQpTb3JyeSBmb3IgbXkgZmF1bHQuIEkgd2lsbCBtb2RpZnkgaXQuDQoNCj4gWy4uLl0NCj4g
DQo+ID4gK3N0YXRpYyB2b2lkIGkyY19pbXhfZG1hX3R4X2NhbGxiYWNrKHZvaWQgKmFyZykNCj4g
Wy4uLl0NCj4gPiArc3RhdGljIGludCBpMmNfaW14X2RtYV90eChzdHJ1Y3QgaW14X2kyY19zdHJ1
Y3QgKmkyY19pbXgsIHN0cnVjdA0KPiA+ICtpMmNfbXNnDQo+ID4gKm1zZ3MpICt7DQo+IFsuLi5d
DQo+ID4gK3N0YXRpYyB2b2lkIGkyY19pbXhfZG1hX3J4X2NhbGxiYWNrKHZvaWQgKmFyZykNCj4g
Wy4uLl0NCj4gPiArc3RhdGljIGludCBpMmNfaW14X2RtYV9yeChzdHJ1Y3QgaW14X2kyY19zdHJ1
Y3QgKmkyY19pbXgsIHN0cnVjdA0KPiA+ICtpMmNfbXNnDQo+ID4gKm1zZ3MpICt7DQo+IFsuLi5d
DQo+IA0KPiBMb29rcyBsaWtlIHRoZXJlJ3MgcXVpdGUgYSBiaXQgb2YgY29kZSBkdXBsaWNhdGlv
biBpbiB0aGUgZm91ciBmdW5jdGlvbnMNCj4gYWJvdmUsIGNhbiB5b3Ugbm90IHVuaWZ5IHRoZW0g
Pw0KPiANCg0KWWVzLCBUaGVyZSdzIGxvb2tzIGxpa2UgcXVpdGUgYSBiaXQgb2YgY29kZSBkdXBs
aWNhdGlvbiBpbiB0aGUgZm91ciBmdW5jdGlvbnMgYWJvdmUuDQpJIGFsc28gaGF0ZSBxdWl0ZSBh
IGJpdCBvZiBjb2RlIGR1cGxpY2F0aW9uLg0KQnV0IHRoZXJlIGFyZSBtYW55IGRpZmZlcmVuY2Vz
IGluIGZhY3QuDQpJZiB1bmlmeSB0aGVtIHdlIHNob3VsZCBhZGQgbWFueSBjb25kaXRpb25hbCBz
dGF0ZW1lbnRzIGFuZCBhdXhpbGlhcnkgdmFyaWFibGUuDQpJIHRoaW5rIGl0J3Mgc3VwZXJmbHVv
dXMgYW5kIHdpbGwgZGFtYWdlIHRoZSByZWFkYWJpbGl0eS4NClNvLCBJIGFtIHZlcnkgY29uZnVz
ZWQuIEFuZCBpZiB5b3UgdGhpbmsgdW5pZnkgdGhlbSB3aWxsIGJlIGJldHRlciBJIHdpbGwgbW9k
aWZ5IGl0LiANClRoYW5rcyBmb3IgeW91ciBzdWdnZXN0aW9uIGFuZCBsb29raW5nIGZvcndhcmQg
dG8gaGVhcmluZyBmcm9tIHlvdS4NCg0KDQo+IEFsc28sIGNhbiB0aGUgRE1BIG5vdCBkbyBmdWxs
LWR1cGxleCBvcGVyYXRpb24gPyBXaGF0IEkgc2VlIGhlcmUgaXMganVzdA0KPiBoYWxmLSBkdXBs
ZXggb3BlcmF0aW9ucyAsIG9uZSBmb3IgUlggYW5kIHRoZSBvdGhlciBvbmUgZm9yIFRYIC4NCj4g
DQoNClllcywgaGVyZSBoYXZlIHR3byBkbWEgY2hhbm5lbHMsIG9uZSBmb3IgUlggYW5kIHRoZSBv
dGhlciBvbmUgZm9yIFRYLg0KV2hlbiB3ZSByZXF1ZXN0IHRoZSBjaGFubmVsIHdlIHNob3VsZCBk
ZXRlcm1pbmUgaXQgZm9yIFRYIG9yIFJYLg0KDQo+ID4gK3N0YXRpYyB2b2lkIGkyY19pbXhfZG1h
X2ZyZWUoc3RydWN0IGlteF9pMmNfc3RydWN0ICppMmNfaW14KSB7DQo+ID4gKwlzdHJ1Y3QgaW14
X2kyY19kbWEgKmRtYSA9IGkyY19pbXgtPmRtYTsNCj4gPiArCXN0cnVjdCBkbWFfY2hhbiAqZG1h
X2NoYW47DQo+ID4gKw0KPiA+ICsJZG1hX2NoYW4gPSBkbWEtPmNoYW5fdHg7DQo+ID4gKwlkbWEt
PmNoYW5fdHggPSBOVUxMOw0KPiA+ICsJZG1hLT5idWZfdHggPSAwOw0KPiA+ICsJZG1hLT5sZW5f
dHggPSAwOw0KPiA+ICsJZG1hX3JlbGVhc2VfY2hhbm5lbChkbWFfY2hhbik7DQo+ID4gKw0KPiA+
ICsJZG1hX2NoYW4gPSBkbWEtPmNoYW5fcng7DQo+ID4gKwlkbWEtPmNoYW5fdHggPSBOVUxMOw0K
PiA+ICsJZG1hLT5idWZfcnggPSAwOw0KPiA+ICsJZG1hLT5sZW5fcnggPSAwOw0KPiA+ICsJZG1h
X3JlbGVhc2VfY2hhbm5lbChkbWFfY2hhbik7DQo+IA0KPiBZb3UgbXVzdCBtYWtlIF9ERUFEXyBf
U1VSRV8gdGhpcyBmdW5jdGlvbiBpcyBub3QgZXZlciBjYWxsZWQgd2hpbGUgdGhlDQo+IERNQSBp
cyBzdGlsbCBhY3RpdmUuIEluIHlvdXIgY2FzZSwgSSBoYXZlIGEgZmVlbGluZyB0aGF0J3Mgbm90
IGhhbmRsZWQuDQo+DQoNCkkgdGhpbmsgdGhpcyBmdW5jdGlvbiB3aWxsIG5vdCBjYWxsZWQgd2hp
bGUgdGhlIERNQSBpcyBzdGlsbCANCmFjdGl2ZSBiZWNhdXNlIG9mIHRoZSBMaW51eCBzeW5jaHJv
bml6YXRpb24gbWVjaGFuaXNtIC0gY29tcGxldGlvbi4gDQpJIHVzZWQgaXQgaW4gdGhlIGRtYSBm
dW5jdGlvbi4NCg0KPiA+ICt9DQo+ID4gIC8qKiBGdW5jdGlvbnMgZm9yIElNWCBJMkMgYWRhcHRl
ciBkcml2ZXINCj4gPiAqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioNCj4g
PiAqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqDQo+ID4gKioqKg0KPiA+ICoqKioqLw0KPiA+DQo+ID4gQEAgLTQyNSw3
ICs2MDAsOCBAQCBzdGF0aWMgaXJxcmV0dXJuX3QgaTJjX2lteF9pc3IoaW50IGlycSwgdm9pZA0K
PiAqZGV2X2lkKQ0KPiA+ICAJcmV0dXJuIElSUV9OT05FOw0KPiA+ICB9DQo+ID4NCj4gPiAtc3Rh
dGljIGludCBpMmNfaW14X3dyaXRlKHN0cnVjdCBpbXhfaTJjX3N0cnVjdCAqaTJjX2lteCwgc3Ry
dWN0DQo+ID4gaTJjX21zZw0KPiA+ICptc2dzKSArc3RhdGljIGludCBpMmNfaW14X3Bpb193cml0
ZShzdHJ1Y3QgaW14X2kyY19zdHJ1Y3QgKmkyY19pbXgsDQo+ID4gKwkJCQkJCQlzdHJ1Y3QgaTJj
X21zZyAqbXNncykNCj4gPiAgew0KPiA+ICAJaW50IGksIHJlc3VsdDsNCj4gPg0KPiA+IEBAIC00
NTgsNyArNjM0LDU2IEBAIHN0YXRpYyBpbnQgaTJjX2lteF93cml0ZShzdHJ1Y3QgaW14X2kyY19z
dHJ1Y3QNCj4gPiAqaTJjX2lteCwgc3RydWN0IGkyY19tc2cgKm1zZ3MpIHJldHVybiAwOyAgfQ0K
PiA+DQo+ID4gLXN0YXRpYyBpbnQgaTJjX2lteF9yZWFkKHN0cnVjdCBpbXhfaTJjX3N0cnVjdCAq
aTJjX2lteCwgc3RydWN0DQo+ID4gaTJjX21zZw0KPiA+ICptc2dzKSArc3RhdGljIGludCBpMmNf
aW14X2RtYV93cml0ZShzdHJ1Y3QgaW14X2kyY19zdHJ1Y3QgKmkyY19pbXgsDQo+ID4gKwkJCQkJ
CQlzdHJ1Y3QgaTJjX21zZyAqbXNncykNCj4gPiArew0KPiA+ICsJaW50IHJlc3VsdCwgdGltZW91
dD0xMDAwOw0KPiA+ICsJdW5zaWduZWQgaW50IHRlbXAgPSAwOw0KPiA+ICsNCj4gPiArCXJlaW5p
dF9jb21wbGV0aW9uKCZpMmNfaW14LT5kbWEtPmNtZF9jb21wbGV0ZSk7DQo+ID4gKwlyZXN1bHQg
PSBpMmNfaW14X2RtYV90eChpMmNfaW14LCBtc2dzKTsNCj4gPiArCWlmKHJlc3VsdCkNCj4gPiAr
CQlyZXR1cm4gcmVzdWx0Ow0KPiA+ICsNCj4gPiArCXRlbXAgPSBpbXhfaTJjX3JlYWRfcmVnKGky
Y19pbXgsIElNWF9JMkNfSTJDUik7DQo+ID4gKwl0ZW1wIHw9IEkyQ1JfRE1BRU47DQo+ID4gKwlp
bXhfaTJjX3dyaXRlX3JlZyh0ZW1wLCBpMmNfaW14LCBJTVhfSTJDX0kyQ1IpOw0KPiA+ICsNCj4g
PiArCS8qIHdyaXRlIHNsYXZlIGFkZHJlc3MgKi8NCj4gPiArCWlteF9pMmNfd3JpdGVfcmVnKG1z
Z3MtPmFkZHIgPDwgMSwgaTJjX2lteCwgSU1YX0kyQ19JMkRSKTsNCj4gPiArCXJlc3VsdCA9IHdh
aXRfZm9yX2NvbXBsZXRpb25faW50ZXJydXB0aWJsZV90aW1lb3V0KA0KPiA+ICsJCQkJCQkmaTJj
X2lteC0+ZG1hLT5jbWRfY29tcGxldGUsDQo+ID4gKwkJCQkJCW1zZWNzX3RvX2ppZmZpZXMoMTAw
MCkpOw0KPiANCj4gUHVsbCB0aGUgbWFnaWMgY29uc3RhbnQgb2YgMTAwMCBvdXQgYW5kICNkZWZp
bmUgaXQgYXMgc29tZQ0KPiBJMkNfSU1YX0RNQV9USU1FT1VUIHBsZWFzZSAuDQo+IA0KDQpUaGFu
a3MsIEkgd2lsbCBtb2RpZnkgaXQuDQoNCj4gPiArCWlmIChyZXN1bHQgPT0gMCkNCj4gPiArCQly
ZXR1cm4gIC1FVElNRURPVVQ7DQo+ID4gKw0KPiA+ICsJLyogd2FpdGluZyBmb3IgVHJhbnNmZXIg
Y29tcGxldGUuICovDQo+ID4gKwl3aGlsZSh0aW1lb3V0LS0pIHsNCj4gPiArCQl0ZW1wID0gaW14
X2kyY19yZWFkX3JlZyhpMmNfaW14LCBJTVhfSTJDX0kyU1IpOw0KPiA+ICsJCWlmICh0ZW1wICYg
MHg4MCkNCj4gPiArCQkJYnJlYWs7DQo+ID4gKwkJdWRlbGF5KDEwKTsNCj4gPiArCX0NCj4gPiAr
DQo+ID4gKwl0ZW1wID0gaW14X2kyY19yZWFkX3JlZyhpMmNfaW14LCBJTVhfSTJDX0kyQ1IpOw0K
PiA+ICsJdGVtcCAmPSB+STJDUl9ETUFFTjsNCj4gPiArCWlteF9pMmNfd3JpdGVfcmVnKHRlbXAs
IGkyY19pbXgsIElNWF9JMkNfSTJDUik7DQo+ID4gKw0KPiA+ICsJLyogd3JpdGUgdGhlIGxhc3Qg
Ynl0ZSAqLw0KPiA+ICsJaW14X2kyY193cml0ZV9yZWcobXNncy0+YnVmW21zZ3MtPmxlbi0xXSwg
aTJjX2lteCwgSU1YX0kyQ19JMkRSKTsNCj4gPiArCXJlc3VsdCA9IGkyY19pbXhfdHJ4X2NvbXBs
ZXRlKGkyY19pbXgpOw0KPiA+ICsJaWYgKHJlc3VsdCkNCj4gPiArCQlyZXR1cm4gcmVzdWx0Ow0K
PiA+ICsNCj4gPiArCXJlc3VsdCA9IGkyY19pbXhfYWNrZWQoaTJjX2lteCk7DQo+ID4gKwlpZiAo
cmVzdWx0KQ0KPiA+ICsJCXJldHVybiByZXN1bHQ7DQo+ID4gKw0KPiA+ICsJcmV0dXJuIDA7DQo+
ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgaTJjX2lteF9waW9fcmVhZChzdHJ1Y3QgaW14
X2kyY19zdHJ1Y3QgKmkyY19pbXgsDQo+ID4gKwkJCQkJCQlzdHJ1Y3QgaTJjX21zZyAqbXNncykN
Cj4gPiAgew0KPiA+ICAJaW50IGksIHJlc3VsdDsNCj4gPiAgCXVuc2lnbmVkIGludCB0ZW1wOw0K
PiA+IEBAIC01MTgsNiArNzQzLDgwIEBAIHN0YXRpYyBpbnQgaTJjX2lteF9yZWFkKHN0cnVjdCBp
bXhfaTJjX3N0cnVjdA0KPiA+ICppMmNfaW14LCBzdHJ1Y3QgaTJjX21zZyAqbXNncykgcmV0dXJu
IDA7ICB9DQo+ID4NCj4gPiArc3RhdGljIGludCBpMmNfaW14X2RtYV9yZWFkKHN0cnVjdCBpbXhf
aTJjX3N0cnVjdCAqaTJjX2lteCwNCj4gPiArCQkJCQkJCXN0cnVjdCBpMmNfbXNnICptc2dzKQ0K
PiA+ICt7DQo+IA0KPiBMb29rcyBsaWtlIGFsbW9zdCBhbiBkdXBsaWNhdGlvbiBhcyB3ZWxsLi4u
DQo+IA0KDQpDb25zaWRlcmluZyB0aGUgc3ltbWV0cmljIHdpdGggdGhlbSBpMmNfaW14X2RtYV93
cml0ZS4NCmkyY19pbXhfZG1hX3dyaXRlIGFuZCBpMmNfaW14X3Bpb193cml0ZSBoYXZlIG1hbnkg
ZGlmZmVyZW5jZXMuIFNvIEkgc2VwYXJhdGUgdGhlbS4NCkJ1dCBpMmNfaW14X2RtYV9yZWFkIGFu
ZCBpMmNfaW14X3Bpb19yZWFkIGlzIHRoZSBzYW1lIGF0IGZpcnN0IHBhcnQuIEkgbWF5IHNob3Vs
ZCB1bmlmeSB0aGVtLg0KQnV0IGl0J3Mgd2lsbCBub3Qgc3ltbWV0cmljIHdpdGggdGhlbSBpMmNf
aW14X2RtYV93cml0ZSBpZiB1bmlmaWVkIHRoZW0uIFNvIEkgZG9uJ3Qga25vdyB3aGljaCB3aWxs
IGJlIGJldHRlcj8NCkxvb2tpbmcgZm9yd2FyZCB0byBoZWFyaW5nIGZyb20geW91Lg0KDQo+IEJl
c2lkZXMsIGZ1bGwtZHVwbGV4IERNQSBvcGVyYXRpb24gaXMgbWlzc2luZywgcGxlYXNlIGV4cGxh
aW4gd2h5Lg0KPiANCj4gVEhhbmtzIQ0KPiANCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - Feb. 28, 2014, 8:57 a.m.
On Friday, February 28, 2014 at 03:23:52 AM, Shawn Guo wrote:
> On Fri, Feb 28, 2014 at 10:13:02AM +0800, Shawn Guo wrote:
> > On Thu, Feb 27, 2014 at 09:39:35PM +0100, Marek Vasut wrote:
> > > > @@ -193,6 +216,7 @@ static const struct imx_i2c_hwdata
> > > > imx1_i2c_hwdata  = {
> > > > 
> > > >  	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
> > > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> > > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > > > 
> > > > +	.has_dma_support	= false,
> > > > 
> > > >  };
> > > > 
> > > > @@ -203,6 +227,7 @@ static const struct imx_i2c_hwdata
> > > > imx21_i2c_hwdata  = { .ndivs			= 
ARRAY_SIZE(imx_i2c_clk_div),
> > > > 
> > > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
> > > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
> > > > 
> > > > +	.has_dma_support	= false,
> > > > 
> > > >  };
> > > > 
> > > > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
> > > > 
> > > >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> > > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> > > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > > > 
> > > > +	.has_dma_support	= true,
> > > 
> > > So why exactly don't we have a DT prop for determining whether the
> > > controller has DMA support ?
> > 
> > Using DMA or PIO is a decision that should be made by driver on its own,
> > not device tree.
> 
> Sorry.  I misunderstood it.  Yes, we can look at the DT property 'dmas'
> to know if the controller has DMA capability.

You're right. Whether or not does the hardware support DMA transfers is a 
property of the hardware, that's why it should be in DT.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - Feb. 28, 2014, 9:04 a.m.
On Friday, February 28, 2014 at 06:19:18 AM, Yao Yuan wrote:

[...]

> > > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
> > > 
> > >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > > 
> > > +	.has_dma_support	= true,
> > 
> > So why exactly don't we have a DT prop for determining whether the
> > controller has DMA support ?
> > 
> > What about the other controllers, do they not support DMA for some
> > specific reason? Please elaborate on that, thank you !
> 
> Sorry for my fault. I will modify it.

I would prefer if you could explain why other controllers do have DMA disabled 
even if the hardware does support the DMA operation.
 
> > [...]
> > 
> > > +static void i2c_imx_dma_tx_callback(void *arg)
> > 
> > [...]
> > 
> > > +static int i2c_imx_dma_tx(struct imx_i2c_struct *i2c_imx, struct
> > > +i2c_msg
> > > *msgs) +{
> > 
> > [...]
> > 
> > > +static void i2c_imx_dma_rx_callback(void *arg)
> > 
> > [...]
> > 
> > > +static int i2c_imx_dma_rx(struct imx_i2c_struct *i2c_imx, struct
> > > +i2c_msg
> > > *msgs) +{
> > 
> > [...]
> > 
> > Looks like there's quite a bit of code duplication in the four functions
> > above, can you not unify them ?
> 
> Yes, There's looks like quite a bit of code duplication in the four
> functions above. I also hate quite a bit of code duplication.
> But there are many differences in fact.
> If unify them we should add many conditional statements and auxiliary
> variable. I think it's superfluous and will damage the readability.
> So, I am very confused. And if you think unify them will be better I will
> modify it. Thanks for your suggestion and looking forward to hearing from
> you.

I'd say try it, the RX and TX callback look almost the same. So does the 
i2c_imx_dma_rx() and i2c_imx_dma_tx() .

> > Also, can the DMA not do full-duplex operation ? What I see here is just
> > half- duplex operations , one for RX and the other one for TX .
> 
> Yes, here have two dma channels, one for RX and the other one for TX.
> When we request the channel we should determine it for TX or RX.

Sorry, I don't quite understand this. If you have two DMA channels, can you not 
use them both to do full-duplex SPI transfer ?

> > > +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx) {
> > > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > > +	struct dma_chan *dma_chan;
> > > +
> > > +	dma_chan = dma->chan_tx;
> > > +	dma->chan_tx = NULL;
> > > +	dma->buf_tx = 0;
> > > +	dma->len_tx = 0;
> > > +	dma_release_channel(dma_chan);
> > > +
> > > +	dma_chan = dma->chan_rx;
> > > +	dma->chan_tx = NULL;
> > > +	dma->buf_rx = 0;
> > > +	dma->len_rx = 0;
> > > +	dma_release_channel(dma_chan);
> > 
> > You must make _DEAD_ _SURE_ this function is not ever called while the
> > DMA is still active. In your case, I have a feeling that's not handled.
> 
> I think this function will not called while the DMA is still
> active because of the Linux synchronization mechanism - completion.
> I used it in the dma function.

This doesn't check whether the completion is actually finished anywhere. I don't 
quite understand how this is safe .

[...]

> > > +static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> > > +							struct i2c_msg *msgs)
> > > +{
> > 
> > Looks like almost an duplication as well...
> 
> Considering the symmetric with them i2c_imx_dma_write.
> i2c_imx_dma_write and i2c_imx_pio_write have many differences. So I
> separate them. But i2c_imx_dma_read and i2c_imx_pio_read is the same at
> first part. I may should unify them. But it's will not symmetric with them
> i2c_imx_dma_write if unified them. So I don't know which will be better?
> Looking forward to hearing from you.

The dma_read() looks almost like dma_write(), so I'd also try merging them 
together.

> > Besides, full-duplex DMA operation is missing, please explain why.
> > 
> > THanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lothar Waßmann - Feb. 28, 2014, 10:59 a.m.
Hi,

Marek Vasut wrote:
> On Friday, February 28, 2014 at 06:19:18 AM, Yao Yuan wrote:
> 
> [...]
> > Yes, here have two dma channels, one for RX and the other one for TX.
> > When we request the channel we should determine it for TX or RX.
> 
> Sorry, I don't quite understand this. If you have two DMA channels, can you not 
> use them both to do full-duplex SPI transfer ?
> 
SPI != I2C


Lothar Waßmann
Yao Yuan - Feb. 28, 2014, 11:36 a.m.
SGkgTWFyZWssDQo+IE9uIEZyaWRheSwgRmVicnVhcnkgMjgsIDIwMTQgYXQgMDY6MTk6MTggQU0s
IFlhbyBZdWFuIHdyb3RlOg0KPiANCj4gWy4uLl0NCj4gDQo+ID4gPiA+IEBAIC0yMTMsNiArMjM4
LDcgQEAgc3RhdGljIHN0cnVjdCBpbXhfaTJjX2h3ZGF0YSB2ZjYxMF9pMmNfaHdkYXRhDQo+ID4g
PiA+ID0gew0KPiA+ID4gPg0KPiA+ID4gPiAgCS5uZGl2cwkJCT0gQVJSQVlfU0laRSh2ZjYxMF9p
MmNfY2xrX2RpdiksDQo+ID4gPiA+ICAJLmkyc3JfY2xyX29wY29kZQk9IEkyU1JfQ0xSX09QQ09E
RV9XMUMsDQo+ID4gPiA+ICAJLmkyY3JfaWVuX29wY29kZQk9IEkyQ1JfSUVOX09QQ09ERV8wLA0K
PiA+ID4gPg0KPiA+ID4gPiArCS5oYXNfZG1hX3N1cHBvcnQJPSB0cnVlLA0KPiA+ID4NCj4gPiA+
IFNvIHdoeSBleGFjdGx5IGRvbid0IHdlIGhhdmUgYSBEVCBwcm9wIGZvciBkZXRlcm1pbmluZyB3
aGV0aGVyIHRoZQ0KPiA+ID4gY29udHJvbGxlciBoYXMgRE1BIHN1cHBvcnQgPw0KPiA+ID4NCj4g
PiA+IFdoYXQgYWJvdXQgdGhlIG90aGVyIGNvbnRyb2xsZXJzLCBkbyB0aGV5IG5vdCBzdXBwb3J0
IERNQSBmb3Igc29tZQ0KPiA+ID4gc3BlY2lmaWMgcmVhc29uPyBQbGVhc2UgZWxhYm9yYXRlIG9u
IHRoYXQsIHRoYW5rIHlvdSAhDQo+ID4NCj4gPiBTb3JyeSBmb3IgbXkgZmF1bHQuIEkgd2lsbCBt
b2RpZnkgaXQuDQo+IA0KPiBJIHdvdWxkIHByZWZlciBpZiB5b3UgY291bGQgZXhwbGFpbiB3aHkg
b3RoZXIgY29udHJvbGxlcnMgZG8gaGF2ZSBETUENCj4gZGlzYWJsZWQgZXZlbiBpZiB0aGUgaGFy
ZHdhcmUgZG9lcyBzdXBwb3J0IHRoZSBETUEgb3BlcmF0aW9uLg0KPiANCg0KV2VsbCwgQmVjYXVz
ZSBvZiB0aGUgSTJDIGluIEkuTVggaGFyZHdhcmUgZG9uJ3Qgc3VwcG9ydCB0aGUgRE1BIG9wZXJh
dGlvbi4gDQpCdXQgaGVyZSBJIGFsc28gdGhpbmsgaGFzX2RtYV9zdXBwb3J0IGlzbid0IG5lY2Vz
c2FyeS4NCg0KPiA+ID4gQWxzbywgY2FuIHRoZSBETUEgbm90IGRvIGZ1bGwtZHVwbGV4IG9wZXJh
dGlvbiA/IFdoYXQgSSBzZWUgaGVyZSBpcw0KPiA+ID4ganVzdA0KPiA+ID4gaGFsZi0gZHVwbGV4
IG9wZXJhdGlvbnMgLCBvbmUgZm9yIFJYIGFuZCB0aGUgb3RoZXIgb25lIGZvciBUWCAuDQo+ID4N
Cj4gPiBZZXMsIGhlcmUgaGF2ZSB0d28gZG1hIGNoYW5uZWxzLCBvbmUgZm9yIFJYIGFuZCB0aGUg
b3RoZXIgb25lIGZvciBUWC4NCj4gPiBXaGVuIHdlIHJlcXVlc3QgdGhlIGNoYW5uZWwgd2Ugc2hv
dWxkIGRldGVybWluZSBpdCBmb3IgVFggb3IgUlguDQo+IA0KPiBTb3JyeSwgSSBkb24ndCBxdWl0
ZSB1bmRlcnN0YW5kIHRoaXMuIElmIHlvdSBoYXZlIHR3byBETUEgY2hhbm5lbHMsIGNhbg0KPiB5
b3Ugbm90IHVzZSB0aGVtIGJvdGggdG8gZG8gZnVsbC1kdXBsZXggU1BJIHRyYW5zZmVyID8NCj4g
DQoNClNvcnJ5LCBUaGVyZSBhcmUgYWxzbyBoYXJkIGZvciBtZS4gSSBkb24ndCB1bmRlcnN0YW5k
IHdoYXQgaXMgZnVsbC1kdXBsZXggZm9yIGRtYT8gDQpBIERNQSBlbmdpbmUgY2FuIG9ubHkgcmVh
ZCBvciB3cml0ZSBhdCB0aGUgc2FtZSB0aW1lLiBBbmQgYSBkbWEgY2hhbm5lbCByZXF1ZXN0IGZv
ciBvbmx5IERNQV9NRU1fVE9fREVWIG9yIERNQV9ERVZfVE9fTUVNIGFsbW9zdC4NCkFsc28gaTJj
IGlzIGEgaGFsZi1kdXBsZXggYnVzLg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - Feb. 28, 2014, noon
On Friday, February 28, 2014 at 11:59:25 AM, Lothar Waßmann wrote:
> Hi,
> 
> Marek Vasut wrote:
> > On Friday, February 28, 2014 at 06:19:18 AM, Yao Yuan wrote:
> > 
> > [...]
> > 
> > > Yes, here have two dma channels, one for RX and the other one for TX.
> > > When we request the channel we should determine it for TX or RX.
> > 
> > Sorry, I don't quite understand this. If you have two DMA channels, can
> > you not use them both to do full-duplex SPI transfer ?
> 
> SPI != I2C

You got me, sorry.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - Feb. 28, 2014, 12:03 p.m.
On Friday, February 28, 2014 at 12:36:01 PM, Yao Yuan wrote:
> Hi Marek,
> 
> > On Friday, February 28, 2014 at 06:19:18 AM, Yao Yuan wrote:
> > 
> > [...]
> > 
> > > > > @@ -213,6 +238,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata
> > > > > = {
> > > > > 
> > > > >  	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
> > > > >  	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
> > > > >  	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
> > > > > 
> > > > > +	.has_dma_support	= true,
> > > > 
> > > > So why exactly don't we have a DT prop for determining whether the
> > > > controller has DMA support ?
> > > > 
> > > > What about the other controllers, do they not support DMA for some
> > > > specific reason? Please elaborate on that, thank you !
> > > 
> > > Sorry for my fault. I will modify it.
> > 
> > I would prefer if you could explain why other controllers do have DMA
> > disabled even if the hardware does support the DMA operation.
> 
> Well, Because of the I2C in I.MX hardware don't support the DMA operation.
> But here I also think has_dma_support isn't necessary.

OK, got it now. Thanks!

> > > > Also, can the DMA not do full-duplex operation ? What I see here is
> > > > just
> > > > half- duplex operations , one for RX and the other one for TX .
> > > 
> > > Yes, here have two dma channels, one for RX and the other one for TX.
> > > When we request the channel we should determine it for TX or RX.
> > 
> > Sorry, I don't quite understand this. If you have two DMA channels, can
> > you not use them both to do full-duplex SPI transfer ?
> 
> Sorry, There are also hard for me. I don't understand what is full-duplex
> for dma?

Sorry, nevermind. I was confused by this and some SPI patches review. Like 
Lothar (thanks!) pointed out, using only half-duplex operation is OK.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yao Yuan - March 3, 2014, 10:23 a.m.
SGksIE1hcmVrDQoNCk1hcmVrIFZhc3V0IHdyb3RlOg0KPiBPbiBUaHVyc2RheSwgRmVicnVhcnkg
MjcsIDIwMTQgYXQgMDc6MDU6MTQgQU0sIFl1YW4gWWFvIHdyb3RlOg0KPiANCj4gWy4uLl0NCj4g
DQo+ID4gK3N0YXRpYyB2b2lkIGkyY19pbXhfZG1hX2ZyZWUoc3RydWN0IGlteF9pMmNfc3RydWN0
ICppMmNfaW14KSB7DQo+ID4gKwlzdHJ1Y3QgaW14X2kyY19kbWEgKmRtYSA9IGkyY19pbXgtPmRt
YTsNCj4gPiArCXN0cnVjdCBkbWFfY2hhbiAqZG1hX2NoYW47DQo+ID4gKw0KPiA+ICsJZG1hX2No
YW4gPSBkbWEtPmNoYW5fdHg7DQo+ID4gKwlkbWEtPmNoYW5fdHggPSBOVUxMOw0KPiA+ICsJZG1h
LT5idWZfdHggPSAwOw0KPiA+ICsJZG1hLT5sZW5fdHggPSAwOw0KPiA+ICsJZG1hX3JlbGVhc2Vf
Y2hhbm5lbChkbWFfY2hhbik7DQo+ID4gKw0KPiA+ICsJZG1hX2NoYW4gPSBkbWEtPmNoYW5fcng7
DQo+ID4gKwlkbWEtPmNoYW5fdHggPSBOVUxMOw0KPiA+ICsJZG1hLT5idWZfcnggPSAwOw0KPiA+
ICsJZG1hLT5sZW5fcnggPSAwOw0KPiA+ICsJZG1hX3JlbGVhc2VfY2hhbm5lbChkbWFfY2hhbik7
DQo+IA0KPiBZb3UgbXVzdCBtYWtlIF9ERUFEXyBfU1VSRV8gdGhpcyBmdW5jdGlvbiBpcyBub3Qg
ZXZlciBjYWxsZWQgd2hpbGUgdGhlDQo+IERNQSBpcyBzdGlsbCBhY3RpdmUuIEluIHlvdXIgY2Fz
ZSwgSSBoYXZlIGEgZmVlbGluZyB0aGF0J3Mgbm90IGhhbmRsZWQuDQo+DQoNClRoYW5rcyBmb3Ig
eW91ciBhdHRlbnRpb24uDQpUaGlzIGZldyBkYXlzIEkgbG9vayB1cCB0aGUgY29kZSBmb3IgdGhl
IHJlYWxpemF0aW9uIG9mIGRtYV9yZWxlYXNlX2NoYW5uZWwoKS4gDQpJIGZvdW5kIHRoYXQgaXQg
d2lsbCBkaXNhYmxlIHRoZSBkbWEgcmVxdWVzdCBmaXJzdC4gU28gaXQncyBtYXkgc2FmZS4gDQpB
bmQgZHJpdmVycyBoYWRuJ3QgY2hlY2sgd2hldGhlciBkbWEgaXMgc3RpbGwgYWN0aXZlIGJlZm9y
ZSBkbWFfcmVsZWFzZV9jaGFubmVsKCkgYXMgYSB1c3VhbGx5IHVzYWdlLg0KQmVjYXVzZSBpdCB3
aWxsIGJlIGRpc2FibGVkIGF1dG9tYXRpYy4NCg0KVGhlIG9ubHkgcHJvYmxlbSBpcyB0aGF0LCBp
dCB3aWxsIGJlIGZvcmNlZCB0byBjYW5jZWwgdGhlIHRyYW5zZmVyIHdoaWNoIHdhcyBub3QgeWV0
IGNvbXBsZXRlZC4NCkxvb2tpbmcgZm9yd2FyZCB0byBoZWFyaW5nIGZyb20geW91Lg0KDQpCZXN0
IHJlZ2FyZHMsDQpZdWFuIFlhbw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - March 3, 2014, 11:14 a.m.
On Monday, March 03, 2014 at 11:23:33 AM, Yao Yuan wrote:
> Hi, Marek
> 
> Marek Vasut wrote:
> > On Thursday, February 27, 2014 at 07:05:14 AM, Yuan Yao wrote:
> > 
> > [...]
> > 
> > > +static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx) {
> > > +	struct imx_i2c_dma *dma = i2c_imx->dma;
> > > +	struct dma_chan *dma_chan;
> > > +
> > > +	dma_chan = dma->chan_tx;
> > > +	dma->chan_tx = NULL;
> > > +	dma->buf_tx = 0;
> > > +	dma->len_tx = 0;
> > > +	dma_release_channel(dma_chan);
> > > +
> > > +	dma_chan = dma->chan_rx;
> > > +	dma->chan_tx = NULL;
> > > +	dma->buf_rx = 0;
> > > +	dma->len_rx = 0;
> > > +	dma_release_channel(dma_chan);
> > 
> > You must make _DEAD_ _SURE_ this function is not ever called while the
> > DMA is still active. In your case, I have a feeling that's not handled.
> 
> Thanks for your attention.
> This few days I look up the code for the realization of
> dma_release_channel(). I found that it will disable the dma request first.
> So it's may safe. And drivers hadn't check whether dma is still active
> before dma_release_channel() as a usually usage. Because it will be
> disabled automatic.
> 
> The only problem is that, it will be forced to cancel the transfer which
> was not yet completed. Looking forward to hearing from you.

OK

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index db895fb..6ec392b 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,9 @@ 
 /* Default value */
 #define IMX_I2C_BIT_RATE	100000	/* 100kHz */
 
+/* enable DMA if transfer size is bigger than this threshold */
+#define IMX_I2C_DMA_THRESHOLD	16
+
 /* IMX I2C registers:
  * the I2C register offset is different between SoCs,
  * to provid support for all these chips, split the
@@ -88,6 +96,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
@@ -172,6 +181,17 @@  struct imx_i2c_hwdata {
 	unsigned		ndivs;
 	unsigned		i2sr_clr_opcode;
 	unsigned		i2cr_ien_opcode;
+	bool			has_dma_support;
+};
+
+struct imx_i2c_dma {
+	struct dma_chan		*chan_tx;
+	struct dma_chan		*chan_rx;
+	dma_addr_t		buf_tx;
+	dma_addr_t		buf_rx;
+	unsigned int		len_tx;
+	unsigned int		len_rx;
+	struct completion	cmd_complete;
 };
 
 struct imx_i2c_struct {
@@ -184,6 +204,9 @@  struct imx_i2c_struct {
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
 	const struct imx_i2c_hwdata	*hwdata;
+
+	bool			use_dma;
+	struct imx_i2c_dma	*dma;
 };
 
 static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
@@ -193,6 +216,7 @@  static const struct imx_i2c_hwdata imx1_i2c_hwdata  = {
 	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
 	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
 	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
+	.has_dma_support	= false,
 
 };
 
@@ -203,6 +227,7 @@  static const struct imx_i2c_hwdata imx21_i2c_hwdata  = {
 	.ndivs			= ARRAY_SIZE(imx_i2c_clk_div),
 	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W0C,
 	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_1,
+	.has_dma_support	= false,
 
 };
 
@@ -213,6 +238,7 @@  static struct imx_i2c_hwdata vf610_i2c_hwdata = {
 	.ndivs			= ARRAY_SIZE(vf610_i2c_clk_div),
 	.i2sr_clr_opcode	= I2SR_CLR_OPCODE_W1C,
 	.i2cr_ien_opcode	= I2CR_IEN_OPCODE_0,
+	.has_dma_support	= true,
 
 };
 
@@ -254,6 +280,155 @@  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, u32 phy_addr)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_slave_config dma_sconfig;
+	int ret;
+
+	dma->chan_tx = dma_request_slave_channel(&i2c_imx->adapter.dev, "tx");
+	if (!dma->chan_tx) {
+		dev_err(&i2c_imx->adapter.dev,
+				"Dma tx channel request failed!\n");
+		return -ENODEV;
+	}
+
+	dma_sconfig.dst_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
+				"Dma slave config failed, err = %d\n", ret);
+		dma_release_channel(dma->chan_tx);
+		return ret;
+	}
+
+	dma->chan_rx = dma_request_slave_channel(&i2c_imx->adapter.dev, "rx");
+	if (!dma->chan_rx) {
+		dev_err(&i2c_imx->adapter.dev,
+				"Dma rx channel request failed!\n");
+		return -ENODEV;
+	}
+
+	dma_sconfig.src_addr = (dma_addr_t)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_err(&i2c_imx->adapter.dev,
+				"Dma slave config failed, err = %d\n", ret);
+		dma_release_channel(dma->chan_rx);
+		return ret;
+	}
+
+	init_completion(&dma->cmd_complete);
+
+	return 0;
+}
+
+static void i2c_imx_dma_tx_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_tx->device->dev, dma->buf_tx,
+			dma->len_tx, DMA_TO_DEVICE);
+	complete(&dma->cmd_complete);
+}
+
+static int i2c_imx_dma_tx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_async_tx_descriptor *txdesc;
+
+	dma->len_tx = msgs->len-1;
+	dma->buf_tx = dma_map_single(dma->chan_tx->device->dev, msgs->buf,
+					dma->len_tx, DMA_TO_DEVICE);
+	if (dma_mapping_error(dma->chan_tx->device->dev, dma->buf_tx)) {
+		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
+		return -EINVAL;
+	}
+
+	txdesc = dmaengine_prep_slave_single(dma->chan_tx, dma->buf_tx,
+					dma->len_tx, DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!txdesc) {
+		dev_err(&i2c_imx->adapter.dev,
+				"Not able to get desc for tx\n");
+		return -EINVAL;
+	}
+
+	txdesc->callback = i2c_imx_dma_tx_callback;
+	txdesc->callback_param = i2c_imx;
+	dmaengine_submit(txdesc);
+	dma_async_issue_pending(dma->chan_tx);
+	
+	return 0;
+}
+
+static void i2c_imx_dma_rx_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_rx->device->dev, dma->buf_rx,
+				dma->len_rx, DMA_FROM_DEVICE);
+	complete(&dma->cmd_complete);
+}
+
+static int i2c_imx_dma_rx(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_async_tx_descriptor *txdesc;
+
+	dma->len_rx = msgs->len - 2;
+	dma->buf_rx = dma_map_single(dma->chan_rx->device->dev, msgs->buf,
+					dma->len_rx, DMA_FROM_DEVICE);
+	if (dma_mapping_error(dma->chan_rx->device->dev, dma->buf_rx)) {
+		dev_err(&i2c_imx->adapter.dev, "dma_map_single tx failed\n");
+		return -EINVAL;
+	}
+
+	txdesc = dmaengine_prep_slave_single(dma->chan_rx, dma->buf_rx,
+					dma->len_rx, DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!txdesc) {
+		dev_err(&i2c_imx->adapter.dev,
+				"Not able to get desc for rx\n");
+		return -EINVAL;
+	}
+
+	txdesc->callback = i2c_imx_dma_rx_callback;
+	txdesc->callback_param = i2c_imx;
+	dmaengine_submit(txdesc);
+	dma_async_issue_pending(dma->chan_rx);
+	
+	return 0;
+}
+static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
+{
+	struct imx_i2c_dma *dma = i2c_imx->dma;
+	struct dma_chan *dma_chan;
+
+	dma_chan = dma->chan_tx;
+	dma->chan_tx = NULL;
+	dma->buf_tx = 0;
+	dma->len_tx = 0;
+	dma_release_channel(dma_chan);
+
+	dma_chan = dma->chan_rx;
+	dma->chan_tx = NULL;
+	dma->buf_rx = 0;
+	dma->len_rx = 0;
+	dma_release_channel(dma_chan);
+}
 /** Functions for IMX I2C adapter driver ***************************************
 *******************************************************************************/
 
@@ -425,7 +600,8 @@  static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
+static int i2c_imx_pio_write(struct imx_i2c_struct *i2c_imx,
+							struct i2c_msg *msgs)
 {
 	int i, result;
 
@@ -458,7 +634,56 @@  static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	return 0;
 }
 
-static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
+static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
+							struct i2c_msg *msgs)
+{
+	int result, timeout=1000;
+	unsigned int temp = 0;
+
+	reinit_completion(&i2c_imx->dma->cmd_complete);
+	result = i2c_imx_dma_tx(i2c_imx, msgs);
+	if(result)
+		return result;
+
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp |= I2CR_DMAEN;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	/* 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(1000));
+	if (result == 0)
+		return  -ETIMEDOUT;
+
+	/* waiting for Transfer complete. */
+	while(timeout--) {
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+		if (temp & 0x80)
+			break;
+		udelay(10);
+	}
+
+	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;
+
+	return 0;
+}
+
+static int i2c_imx_pio_read(struct imx_i2c_struct *i2c_imx,
+							struct i2c_msg *msgs)
 {
 	int i, result;
 	unsigned int temp;
@@ -518,6 +743,80 @@  static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	return 0;
 }
 
+static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
+							struct i2c_msg *msgs)
+{
+	int result, timeout=1000;
+	unsigned int temp;
+
+	/* write slave address */
+	imx_i2c_write_reg((msgs->addr << 1) | 0x01, 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;
+
+	/* setup bus to read data */
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~I2CR_MTX;
+	if (msgs->len - 1)
+		temp &= ~I2CR_TXAK;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
+
+	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);
+	result = i2c_imx_dma_rx(i2c_imx, msgs);
+	if(result)
+		return result;
+
+	result = wait_for_completion_interruptible_timeout(
+						&i2c_imx->dma->cmd_complete,
+						msecs_to_jiffies(1000));
+	if (result == 0)
+		return  -ETIMEDOUT;
+
+	/* waiting for Transfer complete. */
+	while(timeout--) {
+		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
+		if (temp & 0x80)
+			break;
+		udelay(10);
+	}
+
+	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+	temp &= ~I2CR_DMAEN;
+	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+	/* 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;
+}
+
 static int i2c_imx_xfer(struct i2c_adapter *adapter,
 						struct i2c_msg *msgs, int num)
 {
@@ -563,10 +862,18 @@  static int i2c_imx_xfer(struct i2c_adapter *adapter,
 			(temp & I2SR_SRW ? 1 : 0), (temp & I2SR_IIF ? 1 : 0),
 			(temp & I2SR_RXAK ? 1 : 0));
 #endif
-		if (msgs[i].flags & I2C_M_RD)
-			result = i2c_imx_read(i2c_imx, &msgs[i]);
-		else
-			result = i2c_imx_write(i2c_imx, &msgs[i]);
+		if(i2c_imx->use_dma && msgs[i].len >= IMX_I2C_DMA_THRESHOLD) {
+			if (msgs[i].flags & I2C_M_RD)
+				result = i2c_imx_dma_read(i2c_imx, &msgs[i]);
+			else
+				result = i2c_imx_dma_write(i2c_imx, &msgs[i]);
+		} else {
+			if (msgs[i].flags & I2C_M_RD)
+				result = i2c_imx_pio_read(i2c_imx, &msgs[i]);
+			else
+				result = i2c_imx_pio_write(i2c_imx, &msgs[i]);
+		}
+
 		if (result)
 			goto fail0;
 	}
@@ -601,6 +908,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 +919,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 +1005,24 @@  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->hwdata->has_dma_support) {
+		dev_info(&pdev->dev, "has_dma_support_begin.\n");
+		i2c_imx->dma = devm_kzalloc(&pdev->dev, sizeof(struct imx_i2c_dma),
+				GFP_KERNEL);
+		if (!i2c_imx->dma) {
+			dev_info(&pdev->dev, "can't allocate dma struct faild use dma.\n");
+			i2c_imx->use_dma = false;
+		}
+		else if (i2c_imx_dma_request(i2c_imx, phy_addr)) {
+			dev_info(&pdev->dev, "can't request dma chan, faild use dma.\n");
+			i2c_imx->use_dma = false;
+		}
+		else {
+			i2c_imx->use_dma = true;
+		}
+	}
+
 	return 0;   /* Return OK */
 }
 
@@ -707,6 +1034,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->use_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);