Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Thursday, July 24, 2014 at 05:19:27 AM, Yao Yuan wrote: > On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote: > > Ping. Any updates? I think this was pretty close to inclusion? > > Hi, Wolfram > Thanks for your concern. Sorry for my reply so late. I had on a business > trip for months. At that time I have no device to debug it. Now, I'm come > back and I will try my best to finish it. I have sent the patch for V5. > Thanks for your review. So you have no means to test this ? Can this be tested on MX5 ? MX6 ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote: > > Ping. Any updates? I think this was pretty close to inclusion? Hi, Wolfram Thanks for your concern. Sorry for my reply so late. I had on a business trip for months. At that time I have no device to debug it. Now, I'm come back and I will try my best to finish it. I have sent the patch for V5. Thanks for your review. Best regards, Yuan Yao -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote: Ping. Any updates? I think this was pretty close to inclusion? Hi, Wolfram Thanks for your concern. Sorry for my reply so late. I had on a business trip for months. At that time I have no device to debug it. Now, I'm come back and I will try my best to finish it. I have sent the patch for V5. Thanks for your review. Best regards, Yuan Yao -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Thursday, July 24, 2014 at 05:19:27 AM, Yao Yuan wrote: On Fri, May 21, 2014 at 4:01 PM +0200, Wolfram Sang wrote: Ping. Any updates? I think this was pretty close to inclusion? Hi, Wolfram Thanks for your concern. Sorry for my reply so late. I had on a business trip for months. At that time I have no device to debug it. Now, I'm come back and I will try my best to finish it. I have sent the patch for V5. Thanks for your review. So you have no means to test this ? Can this be tested on MX5 ? MX6 ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, Apr 04, 2014 at 07:40:11PM +0200, Wolfram Sang wrote: > > The ugly is my work or even the ability but not the attitude, I think. > > Explaining which tests were done is what I wanted to request more often > from submitters anyhow. Your leftover (yes, this can happen) just > reminded me to actually do that :) Ping. Any updates? I think this was pretty close to inclusion? signature.asc Description: Digital signature
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, Apr 04, 2014 at 07:40:11PM +0200, Wolfram Sang wrote: The ugly is my work or even the ability but not the attitude, I think. Explaining which tests were done is what I wanted to request more often from submitters anyhow. Your leftover (yes, this can happen) just reminded me to actually do that :) Ping. Any updates? I think this was pretty close to inclusion? signature.asc Description: Digital signature
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
> The ugly is my work or even the ability but not the attitude, I think. Explaining which tests were done is what I wanted to request more often from submitters anyhow. Your leftover (yes, this can happen) just reminded me to actually do that :) signature.asc Description: Digital signature
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 22:29:32 PM, Marek Vasut wrote:
> On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
> > Add dma support for i2c. This function depend on DMA driver.
> > You can turn on it by write both the dmas and dma-name properties in dts
> > node.
> >
> > Signed-off-by: Yuan Yao
[...]
> Since you will be fixing that superfluous return 0; (I actually wonder, did
> you
> really test the driver at all before submitting it?) ...
I'm sorry for the leftover. It's a serious bug. The support by DMA will be
disabled always.
Thanks for your consideration. But subjective assume may not suitable. Testing
is a essential link .
But the bug can't be found by normal test . The testing may not be imperfect,
it's my
serious fault, but please don't doubt my work attitude.
The ugly is my work or even the ability but not the attitude, I think.
At last, Thank you for giving me a lot of help, it's very important for me.
> > +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> > + struct i2c_msg *msgs)
> > +{
> > + struct imx_i2c_dma *dma = i2c_imx->dma;
> > + struct dma_async_tx_descriptor *txdesc;
> > + struct device *dev = _imx->adapter.dev;
> +
> + dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,
[...]
> Reviewed-by: Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, Apr 04, 2014 at 04:28:55PM +0200, Marek Vasut wrote: > On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote: > > Add dma support for i2c. This function depend on DMA driver. > > You can turn on it by write both the dmas and dma-name properties in dts > > node. > > > > Signed-off-by: Yuan Yao > > [...] > > Since you will be fixing that superfluous return 0; (I actually wonder, did > you > really test the driver at all before submitting it?) ... Good question. @Yuan: Next time, please state what tests you did with the new version of the driver. signature.asc Description: Digital signature
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
> Add dma support for i2c. This function depend on DMA driver.
> You can turn on it by write both the dmas and dma-name properties in dts
> node.
>
> Signed-off-by: Yuan Yao
[...]
Since you will be fixing that superfluous return 0; (I actually wonder, did you
really test the driver at all before submitting it?) ...
> +static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> + struct i2c_msg *msgs)
> +{
> + struct imx_i2c_dma *dma = i2c_imx->dma;
> + struct dma_async_tx_descriptor *txdesc;
> + struct device *dev = _imx->adapter.dev;
> +
> + dma->dma_buf = dma_map_single(dma->chan_using->device->dev, msgs->buf,
Please fix this "noodle" here too, the chain of pointers is quite long, you can
just define a variable for that.
> + dma->dma_len, dma->dma_data_dir);
> + if (dma_mapping_error(dma->chan_using->device->dev, dma->dma_buf)) {
> + dev_err(dev, "DMA mapping failed\n");
> + return -EINVAL;
> + }
> +
> + txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
> + dma->dma_len, dma->dma_transfer_dir,
> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> + if (!txdesc) {
> + dev_err(dev, "Not able to get desc for DMA xfer\n");
> + dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
> + dma->dma_len, dma->dma_data_dir);
> + return -EINVAL;
> + }
> +
> + txdesc->callback = i2c_imx_dma_callback;
> + txdesc->callback_param = i2c_imx;
> + dmaengine_submit(txdesc);
> + dma_async_issue_pending(dma->chan_using);
> +
> + return 0;
> +}
[...]
Other than those two things,
Reviewed-by: Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 11:49:39 AM, sourav wrote:
[...]
> > +/* Functions for DMA support */
> > +static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> > + dma_addr_t phy_addr)
> > +{
> > + struct imx_i2c_dma *dma;
> > + struct dma_slave_config dma_sconfig;
> > + struct device *dev =_imx->adapter.dev;
> > + int ret;
> > +
> > + dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
> > + if (!dma) {
> > + dev_info(dev, "can't allocate DMA struct\n");
> > + return -ENOMEM;
> > + }
> > +
> > + dma->chan_tx = dma_request_slave_channel(dev, "tx");
> > + return 0;
>
> ?? Looks to be some leftover?
Nice find.
btw. you might want to trim the email only to the relevant parts when replying
so it's easier to find your commments in the entire body of text.
> > + if (!dma->chan_tx) {
> > + dev_info(dev, "DMA tx channel request failed\n");
> > + ret = -ENODEV;
> > + goto fail_al;
> > + }
[...]
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
+
+dma->chan_tx = dma_request_slave_channel(dev, "tx");
+return 0;
To be more clear,
return here looks to be some leftover.
?? Looks to be some leftover?
+if (!dma->chan_tx) {
+dev_info(dev, "DMA tx channel request failed\n");
+ret = -ENODEV;
+goto fail_al;
+}
+
+dma_sconfig.dst_addr = phy_addr +
+(IMX_I2C_I2DR<< i2c_imx->hwdata->regshift);
+dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+dma_sconfig.dst_maxburst = 1;
+dma_sconfig.direction = DMA_MEM_TO_DEV;
+ret = dmaengine_slave_config(dma->chan_tx,_sconfig);
+if (ret< 0) {
+dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+goto fail_tx;
+}
+
+dma->chan_rx = dma_request_slave_channel(dev, "rx");
+if (!dma->chan_rx) {
+dev_info(dev, "DMA rx channel request failed\n");
+ret = -ENODEV;
+goto fail_tx;
+}
+
+dma_sconfig.src_addr = phy_addr +
+(IMX_I2C_I2DR<< i2c_imx->hwdata->regshift);
+dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+dma_sconfig.src_maxburst = 1;
+dma_sconfig.direction = DMA_DEV_TO_MEM;
+ret = dmaengine_slave_config(dma->chan_rx,_sconfig);
+if (ret< 0) {
+dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+goto fail_rx;
+}
+
+i2c_imx->dma = dma;
+
+init_completion(>cmd_complete);
+
+return 0;
+
+fail_rx:
+dma_release_channel(dma->chan_rx);
+fail_tx:
+dma_release_channel(dma->chan_tx);
+fail_al:
+devm_kfree(dev, dma);
+
+return ret;
+}
+
+static void i2c_imx_dma_callback(void *arg)
+{
+struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
+struct imx_i2c_dma *dma = i2c_imx->dma;
+
+dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
+dma->dma_len, dma->dma_data_dir);
+complete(>cmd_complete);
+}
+
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+struct i2c_msg *msgs)
+{
+struct imx_i2c_dma *dma = i2c_imx->dma;
+struct dma_async_tx_descriptor *txdesc;
+struct device *dev =_imx->adapter.dev;
+
+dma->dma_buf = dma_map_single(dma->chan_using->device->dev,
msgs->buf,
+dma->dma_len, dma->dma_data_dir);
+if (dma_mapping_error(dma->chan_using->device->dev,
dma->dma_buf)) {
+dev_err(dev, "DMA mapping failed\n");
+return -EINVAL;
+}
+
+txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf,
+dma->dma_len, dma->dma_transfer_dir,
+DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+if (!txdesc) {
+dev_err(dev, "Not able to get desc for DMA xfer\n");
+dma_unmap_single(dma->chan_using->device->dev, dma->dma_buf,
+dma->dma_len, dma->dma_data_dir);
+return -EINVAL;
+}
+
+txdesc->callback = i2c_imx_dma_callback;
+txdesc->callback_param = i2c_imx;
+dmaengine_submit(txdesc);
+dma_async_issue_pending(dma->chan_using);
+
+return 0;
+}
+
+static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
+{
+struct imx_i2c_dma *dma = i2c_imx->dma;
+
+dma->dma_buf = 0;
+dma->dma_len = 0;
+
+dma_release_channel(dma->chan_tx);
+dma->chan_tx = NULL;
+
+dma_release_channel(dma->chan_rx);
+dma->chan_rx = NULL;
+
+dma->chan_using = NULL;
+}
+
/** Functions for IMX I2C adapter driver
***
***/
@@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct
*i2c_imx)
temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+temp&= ~I2CR_DMAEN;
+imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
return result;
}
@@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void
*dev_id)
static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct
i2c_msg *msgs)
{
-int i, result;
+int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
+unsigned int temp = 0;
+struct imx_i2c_dma *dma = i2c_imx->dma;
+struct device *dev =_imx->adapter.dev;
-dev_dbg(_imx->adapter.dev, "<%s> write slave address:
addr=0x%x\n",
+dev_dbg(dev, "<%s> write slave address: addr=0x%x\n",
__func__, msgs->addr<< 1);
+if (dma&& msgs->len>= IMX_I2C_DMA_THRESHOLD) {
+reinit_completion(_imx->dma->cmd_complete);
+dma->chan_using = dma->chan_tx;
+dma->dma_transfer_dir = DMA_MEM_TO_DEV;
+dma->dma_data_dir = DMA_TO_DEVICE;
+dma->dma_len = msgs->len - 1;
+result = i2c_imx_dma_xfer(i2c_imx, msgs);
+if (result)
+return result;
-/* write slave address */
-imx_i2c_write_reg(msgs->addr<< 1, i2c_imx, IMX_I2C_I2DR);
-result = i2c_imx_trx_complete(i2c_imx);
-if (result)
-return result;
-result
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
Hi,
On Friday 04 April 2014 08:11 AM, Yuan Yao wrote:
Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts node.
Signed-off-by: Yuan Yao
---
drivers/i2c/busses/i2c-imx.c | 372 +--
1 file changed, 319 insertions(+), 53 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index db895fb..3d63b35 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -37,22 +37,27 @@
/** Includes
***
***/
-#include
-#include
-#include
+#include
+#include
+#include
+#include
+#include
+#include
#include
#include
#include
-#include
#include
+#include
#include
-#include
-#include
-#include
-#include
+#include
+#include
#include
#include
+#include
#include
+#include
+#include
+#include
/** Defines
***/
@@ -63,6 +68,10 @@
/* Default value */
#define IMX_I2C_BIT_RATE 10 /* 100kHz */
+/* enable DMA if transfer byte size is bigger than this threshold */
+#define IMX_I2C_DMA_THRESHOLD 16
+#define IMX_I2C_DMA_TIMEOUT1000
+
/* IMX I2C registers:
* the I2C register offset is different between SoCs,
* to provid support for all these chips, split the
@@ -88,6 +97,7 @@
#define I2SR_IBB 0x20
#define I2SR_IAAS 0x40
#define I2SR_ICF 0x80
+#define I2CR_DMAEN 0x02
#define I2CR_RSTA 0x04
#define I2CR_TXAK 0x08
#define I2CR_MTX 0x10
@@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
unsignedi2cr_ien_opcode;
};
+struct imx_i2c_dma {
+ struct dma_chan *chan_tx;
+ struct dma_chan *chan_rx;
+ struct dma_chan *chan_using;
+ struct completion cmd_complete;
+ dma_addr_t dma_buf;
+ unsigned intdma_len;
+ unsigned intdma_transfer_dir;
+ unsigned intdma_data_dir;
+};
+
struct imx_i2c_struct {
struct i2c_adapter adapter;
struct clk *clk;
@@ -184,6 +205,8 @@ struct imx_i2c_struct {
int stopped;
unsigned intifdr; /* IMX_I2C_IFDR */
const struct imx_i2c_hwdata *hwdata;
+
+ struct imx_i2c_dma *dma;
};
static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -254,6 +277,132 @@ static inline unsigned char imx_i2c_read_reg(struct
imx_i2c_struct *i2c_imx,
return readb(i2c_imx->base + (reg<< i2c_imx->hwdata->regshift));
}
+/* Functions for DMA support */
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+ dma_addr_t phy_addr)
+{
+ struct imx_i2c_dma *dma;
+ struct dma_slave_config dma_sconfig;
+ struct device *dev =_imx->adapter.dev;
+ int ret;
+
+ dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
+ if (!dma) {
+ dev_info(dev, "can't allocate DMA struct\n");
+ return -ENOMEM;
+ }
+
+ dma->chan_tx = dma_request_slave_channel(dev, "tx");
+ return 0;
?? Looks to be some leftover?
+ if (!dma->chan_tx) {
+ dev_info(dev, "DMA tx channel request failed\n");
+ ret = -ENODEV;
+ goto fail_al;
+ }
+
+ dma_sconfig.dst_addr = phy_addr +
+ (IMX_I2C_I2DR<< i2c_imx->hwdata->regshift);
+ dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.dst_maxburst = 1;
+ dma_sconfig.direction = DMA_MEM_TO_DEV;
+ ret = dmaengine_slave_config(dma->chan_tx,_sconfig);
+ if (ret< 0) {
+ dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+ goto fail_tx;
+ }
+
+ dma->chan_rx = dma_request_slave_channel(dev, "rx");
+ if (!dma->chan_rx) {
+ dev_info(dev, "DMA rx channel request failed\n");
+ ret = -ENODEV;
+ goto fail_tx;
+ }
+
+ dma_sconfig.src_addr = phy_addr +
+ (IMX_I2C_I2DR<< i2c_imx->hwdata->regshift);
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 1;
+ dma_sconfig.direction = DMA_DEV_TO_MEM;
+ ret = dmaengine_slave_config(dma->chan_rx,_sconfig);
+ if (ret< 0) {
+ dev_info(dev, "DMA slave config failed, err = %d\n", ret);
+ goto fail_rx;
+ }
+
+ i2c_imx->dma = dma;
+
+ init_completion(>cmd_complete);
+
+ return 0;
+
+fail_rx:
+ dma_release_channel(dma->chan_rx);
+fail_tx:
+
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
Hi,
On Friday 04 April 2014 08:11 AM, Yuan Yao wrote:
Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts node.
Signed-off-by: Yuan Yaoyao.y...@freescale.com
---
drivers/i2c/busses/i2c-imx.c | 372 +--
1 file changed, 319 insertions(+), 53 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index db895fb..3d63b35 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -37,22 +37,27 @@
/** Includes
***
***/
-#includelinux/init.h
-#includelinux/kernel.h
-#includelinux/module.h
+#includelinux/clk.h
+#includelinux/completion.h
+#includelinux/delay.h
+#includelinux/dma-mapping.h
+#includelinux/dmaengine.h
+#includelinux/dmapool.h
#includelinux/errno.h
#includelinux/err.h
#includelinux/interrupt.h
-#includelinux/delay.h
#includelinux/i2c.h
+#includelinux/init.h
#includelinux/io.h
-#includelinux/sched.h
-#includelinux/platform_device.h
-#includelinux/clk.h
-#includelinux/slab.h
+#includelinux/kernel.h
+#includelinux/module.h
#includelinux/of.h
#includelinux/of_device.h
+#includelinux/of_dma.h
#includelinux/platform_data/i2c-imx.h
+#includelinux/platform_device.h
+#includelinux/sched.h
+#includelinux/slab.h
/** Defines
***/
@@ -63,6 +68,10 @@
/* Default value */
#define IMX_I2C_BIT_RATE 10 /* 100kHz */
+/* enable DMA if transfer byte size is bigger than this threshold */
+#define IMX_I2C_DMA_THRESHOLD 16
+#define IMX_I2C_DMA_TIMEOUT1000
+
/* IMX I2C registers:
* the I2C register offset is different between SoCs,
* to provid support for all these chips, split the
@@ -88,6 +97,7 @@
#define I2SR_IBB 0x20
#define I2SR_IAAS 0x40
#define I2SR_ICF 0x80
+#define I2CR_DMAEN 0x02
#define I2CR_RSTA 0x04
#define I2CR_TXAK 0x08
#define I2CR_MTX 0x10
@@ -174,6 +184,17 @@ struct imx_i2c_hwdata {
unsignedi2cr_ien_opcode;
};
+struct imx_i2c_dma {
+ struct dma_chan *chan_tx;
+ struct dma_chan *chan_rx;
+ struct dma_chan *chan_using;
+ struct completion cmd_complete;
+ dma_addr_t dma_buf;
+ unsigned intdma_len;
+ unsigned intdma_transfer_dir;
+ unsigned intdma_data_dir;
+};
+
struct imx_i2c_struct {
struct i2c_adapter adapter;
struct clk *clk;
@@ -184,6 +205,8 @@ struct imx_i2c_struct {
int stopped;
unsigned intifdr; /* IMX_I2C_IFDR */
const struct imx_i2c_hwdata *hwdata;
+
+ struct imx_i2c_dma *dma;
};
static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -254,6 +277,132 @@ static inline unsigned char imx_i2c_read_reg(struct
imx_i2c_struct *i2c_imx,
return readb(i2c_imx-base + (reg i2c_imx-hwdata-regshift));
}
+/* Functions for DMA support */
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+ dma_addr_t phy_addr)
+{
+ struct imx_i2c_dma *dma;
+ struct dma_slave_config dma_sconfig;
+ struct device *dev =i2c_imx-adapter.dev;
+ int ret;
+
+ dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
+ if (!dma) {
+ dev_info(dev, can't allocate DMA struct\n);
+ return -ENOMEM;
+ }
+
+ dma-chan_tx = dma_request_slave_channel(dev, tx);
+ return 0;
?? Looks to be some leftover?
+ if (!dma-chan_tx) {
+ dev_info(dev, DMA tx channel request failed\n);
+ ret = -ENODEV;
+ goto fail_al;
+ }
+
+ dma_sconfig.dst_addr = phy_addr +
+ (IMX_I2C_I2DR i2c_imx-hwdata-regshift);
+ dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.dst_maxburst = 1;
+ dma_sconfig.direction = DMA_MEM_TO_DEV;
+ ret = dmaengine_slave_config(dma-chan_tx,dma_sconfig);
+ if (ret 0) {
+ dev_info(dev, DMA slave config failed, err = %d\n, ret);
+ goto fail_tx;
+ }
+
+ dma-chan_rx = dma_request_slave_channel(dev, rx);
+ if (!dma-chan_rx) {
+ dev_info(dev, DMA rx channel request failed\n);
+ ret = -ENODEV;
+ goto fail_tx;
+ }
+
+ dma_sconfig.src_addr = phy_addr +
+ (IMX_I2C_I2DR i2c_imx-hwdata-regshift);
+ dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+ dma_sconfig.src_maxburst = 1;
+
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
+
+dma-chan_tx = dma_request_slave_channel(dev, tx);
+return 0;
To be more clear,
return here looks to be some leftover.
?? Looks to be some leftover?
+if (!dma-chan_tx) {
+dev_info(dev, DMA tx channel request failed\n);
+ret = -ENODEV;
+goto fail_al;
+}
+
+dma_sconfig.dst_addr = phy_addr +
+(IMX_I2C_I2DR i2c_imx-hwdata-regshift);
+dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+dma_sconfig.dst_maxburst = 1;
+dma_sconfig.direction = DMA_MEM_TO_DEV;
+ret = dmaengine_slave_config(dma-chan_tx,dma_sconfig);
+if (ret 0) {
+dev_info(dev, DMA slave config failed, err = %d\n, ret);
+goto fail_tx;
+}
+
+dma-chan_rx = dma_request_slave_channel(dev, rx);
+if (!dma-chan_rx) {
+dev_info(dev, DMA rx channel request failed\n);
+ret = -ENODEV;
+goto fail_tx;
+}
+
+dma_sconfig.src_addr = phy_addr +
+(IMX_I2C_I2DR i2c_imx-hwdata-regshift);
+dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+dma_sconfig.src_maxburst = 1;
+dma_sconfig.direction = DMA_DEV_TO_MEM;
+ret = dmaengine_slave_config(dma-chan_rx,dma_sconfig);
+if (ret 0) {
+dev_info(dev, DMA slave config failed, err = %d\n, ret);
+goto fail_rx;
+}
+
+i2c_imx-dma = dma;
+
+init_completion(dma-cmd_complete);
+
+return 0;
+
+fail_rx:
+dma_release_channel(dma-chan_rx);
+fail_tx:
+dma_release_channel(dma-chan_tx);
+fail_al:
+devm_kfree(dev, dma);
+
+return ret;
+}
+
+static void i2c_imx_dma_callback(void *arg)
+{
+struct imx_i2c_struct *i2c_imx = (struct imx_i2c_struct *)arg;
+struct imx_i2c_dma *dma = i2c_imx-dma;
+
+dma_unmap_single(dma-chan_using-device-dev, dma-dma_buf,
+dma-dma_len, dma-dma_data_dir);
+complete(dma-cmd_complete);
+}
+
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+struct i2c_msg *msgs)
+{
+struct imx_i2c_dma *dma = i2c_imx-dma;
+struct dma_async_tx_descriptor *txdesc;
+struct device *dev =i2c_imx-adapter.dev;
+
+dma-dma_buf = dma_map_single(dma-chan_using-device-dev,
msgs-buf,
+dma-dma_len, dma-dma_data_dir);
+if (dma_mapping_error(dma-chan_using-device-dev,
dma-dma_buf)) {
+dev_err(dev, DMA mapping failed\n);
+return -EINVAL;
+}
+
+txdesc = dmaengine_prep_slave_single(dma-chan_using, dma-dma_buf,
+dma-dma_len, dma-dma_transfer_dir,
+DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+if (!txdesc) {
+dev_err(dev, Not able to get desc for DMA xfer\n);
+dma_unmap_single(dma-chan_using-device-dev, dma-dma_buf,
+dma-dma_len, dma-dma_data_dir);
+return -EINVAL;
+}
+
+txdesc-callback = i2c_imx_dma_callback;
+txdesc-callback_param = i2c_imx;
+dmaengine_submit(txdesc);
+dma_async_issue_pending(dma-chan_using);
+
+return 0;
+}
+
+static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx)
+{
+struct imx_i2c_dma *dma = i2c_imx-dma;
+
+dma-dma_buf = 0;
+dma-dma_len = 0;
+
+dma_release_channel(dma-chan_tx);
+dma-chan_tx = NULL;
+
+dma_release_channel(dma-chan_rx);
+dma-chan_rx = NULL;
+
+dma-chan_using = NULL;
+}
+
/** Functions for IMX I2C adapter driver
***
***/
@@ -334,6 +483,11 @@ static int i2c_imx_start(struct imx_i2c_struct
*i2c_imx)
temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+temp= ~I2CR_DMAEN;
+imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
return result;
}
@@ -427,44 +581,101 @@ static irqreturn_t i2c_imx_isr(int irq, void
*dev_id)
static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct
i2c_msg *msgs)
{
-int i, result;
+int i, result, timeout = IMX_I2C_DMA_TIMEOUT;
+unsigned int temp = 0;
+struct imx_i2c_dma *dma = i2c_imx-dma;
+struct device *dev =i2c_imx-adapter.dev;
-dev_dbg(i2c_imx-adapter.dev, %s write slave address:
addr=0x%x\n,
+dev_dbg(dev, %s write slave address: addr=0x%x\n,
__func__, msgs-addr 1);
+if (dma msgs-len= IMX_I2C_DMA_THRESHOLD) {
+reinit_completion(i2c_imx-dma-cmd_complete);
+dma-chan_using = dma-chan_tx;
+dma-dma_transfer_dir = DMA_MEM_TO_DEV;
+dma-dma_data_dir = DMA_TO_DEVICE;
+dma-dma_len = msgs-len - 1;
+result = i2c_imx_dma_xfer(i2c_imx, msgs);
+if (result)
+return result;
-/* write slave address */
-imx_i2c_write_reg(msgs-addr 1, i2c_imx, IMX_I2C_I2DR);
-result = i2c_imx_trx_complete(i2c_imx);
-if (result)
-return result;
-result = i2c_imx_acked(i2c_imx);
-if (result)
-return result;
-
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 11:49:39 AM, sourav wrote:
[...]
+/* Functions for DMA support */
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+ dma_addr_t phy_addr)
+{
+ struct imx_i2c_dma *dma;
+ struct dma_slave_config dma_sconfig;
+ struct device *dev =i2c_imx-adapter.dev;
+ int ret;
+
+ dma = devm_kzalloc(dev, sizeof(struct imx_i2c_dma), GFP_KERNEL);
+ if (!dma) {
+ dev_info(dev, can't allocate DMA struct\n);
+ return -ENOMEM;
+ }
+
+ dma-chan_tx = dma_request_slave_channel(dev, tx);
+ return 0;
?? Looks to be some leftover?
Nice find.
btw. you might want to trim the email only to the relevant parts when replying
so it's easier to find your commments in the entire body of text.
+ if (!dma-chan_tx) {
+ dev_info(dev, DMA tx channel request failed\n);
+ ret = -ENODEV;
+ goto fail_al;
+ }
[...]
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts
node.
Signed-off-by: Yuan Yao yao.y...@freescale.com
[...]
Since you will be fixing that superfluous return 0; (I actually wonder, did you
really test the driver at all before submitting it?) ...
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+ struct i2c_msg *msgs)
+{
+ struct imx_i2c_dma *dma = i2c_imx-dma;
+ struct dma_async_tx_descriptor *txdesc;
+ struct device *dev = i2c_imx-adapter.dev;
+
+ dma-dma_buf = dma_map_single(dma-chan_using-device-dev, msgs-buf,
Please fix this noodle here too, the chain of pointers is quite long, you can
just define a variable for that.
+ dma-dma_len, dma-dma_data_dir);
+ if (dma_mapping_error(dma-chan_using-device-dev, dma-dma_buf)) {
+ dev_err(dev, DMA mapping failed\n);
+ return -EINVAL;
+ }
+
+ txdesc = dmaengine_prep_slave_single(dma-chan_using, dma-dma_buf,
+ dma-dma_len, dma-dma_transfer_dir,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!txdesc) {
+ dev_err(dev, Not able to get desc for DMA xfer\n);
+ dma_unmap_single(dma-chan_using-device-dev, dma-dma_buf,
+ dma-dma_len, dma-dma_data_dir);
+ return -EINVAL;
+ }
+
+ txdesc-callback = i2c_imx_dma_callback;
+ txdesc-callback_param = i2c_imx;
+ dmaengine_submit(txdesc);
+ dma_async_issue_pending(dma-chan_using);
+
+ return 0;
+}
[...]
Other than those two things,
Reviewed-by: Marek Vasut ma...@denx.de
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Fri, Apr 04, 2014 at 04:28:55PM +0200, Marek Vasut wrote: On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote: Add dma support for i2c. This function depend on DMA driver. You can turn on it by write both the dmas and dma-name properties in dts node. Signed-off-by: Yuan Yao yao.y...@freescale.com [...] Since you will be fixing that superfluous return 0; (I actually wonder, did you really test the driver at all before submitting it?) ... Good question. @Yuan: Next time, please state what tests you did with the new version of the driver. signature.asc Description: Digital signature
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
On Friday, April 04, 2014 at 22:29:32 PM, Marek Vasut wrote:
On Friday, April 04, 2014 at 04:41:11 AM, Yuan Yao wrote:
Add dma support for i2c. This function depend on DMA driver.
You can turn on it by write both the dmas and dma-name properties in dts
node.
Signed-off-by: Yuan Yao yao.y...@freescale.com
[...]
Since you will be fixing that superfluous return 0; (I actually wonder, did
you
really test the driver at all before submitting it?) ...
I'm sorry for the leftover. It's a serious bug. The support by DMA will be
disabled always.
Thanks for your consideration. But subjective assume may not suitable. Testing
is a essential link .
But the bug can't be found by normal test . The testing may not be imperfect,
it's my
serious fault, but please don't doubt my work attitude.
The ugly is my work or even the ability but not the attitude, I think.
At last, Thank you for giving me a lot of help, it's very important for me.
+static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
+ struct i2c_msg *msgs)
+{
+ struct imx_i2c_dma *dma = i2c_imx-dma;
+ struct dma_async_tx_descriptor *txdesc;
+ struct device *dev = i2c_imx-adapter.dev;
+
+ dma-dma_buf = dma_map_single(dma-chan_using-device-dev, msgs-buf,
[...]
Reviewed-by: Marek Vasut ma...@denx.de
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] i2c: add DMA support for freescale i2c driver
The ugly is my work or even the ability but not the attitude, I think. Explaining which tests were done is what I wanted to request more often from submitters anyhow. Your leftover (yes, this can happen) just reminded me to actually do that :) signature.asc Description: Digital signature
