| Message ID | 1393481115-22136-2-git-send-email-yao.yuan@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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);
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(-)