possible MXS-i2c bug

possible MXS-i2c bug

[Marek Vasut]

Hi,

I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected.

Apparently, there might be some timing issue, when I put printk() into the at24 
driver eeprom write function, it "fixed" itself. Though, replacing the i2c 
driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some 
flaw in it.

Further investigation turned out, that the complete() isn't called in the mxs-
i2c driver irq handler unless I put the printk(). Putting the printk() in the 
xfer function of the mxs-i2c driver instead of eeprom write function has the 
same effect -- "fixing" it.

I didn't have time to properly investigate this matter, so consider this as a 
quick and dirty bugreport (maybe?). But thie behavior I saw it quite easily 
replicated, try:

dd if=/dev/zero of=/sys/bus/devices/0-00XY/eeprom bs=<sizeof eeprom> count=1

And the write will timeout.

Has anyone recently tested the driver with such eeprom? Anything from the 24xxx 
series with 16 bit addresses has this problem. I tested and saw this on more 
than one board, so this is not hardware issue.

If noone has any fixes already, I'll debug this tonight or so.

Thanks!

Best regards,
Marek Vasut

possible MXS-i2c bug

[Fabio Estevam]

On Thu, Apr 26, 2012 at 8:26 AM, Marek Vasut <marex@denx.de> wrote:
> Hi,
>
> I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected.
>
> Apparently, there might be some timing issue, when I put printk() into the at24
> driver eeprom write function, it "fixed" itself. Though, replacing the i2c
> driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some
> flaw in it.

Does Wolfram's patch help?
http://www.spinics.net/lists/linux-i2c/msg08002.html

I don't have a I2C eeprom soldered on my mx28evk to test it.

Re: possible MXS-i2c bug

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]

Hi Marek,

you forgot the most important list, linux-i2c ;)

> I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected.

Yes, we had the same this week/last week.

> Apparently, there might be some timing issue, when I put printk() into the at24 
> driver eeprom write function, it "fixed" itself. Though, replacing the i2c 
> driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some 
> flaw in it.

Try removing the printk and use the module paramter "io_limit=24" for
at24. Does this help? (Or bs<=24 with dd)

> If noone has any fixes already, I'll debug this tonight or so.

The write FIFO overflows, since it only can carry 8 words. There is
currently no check for that. One solution would be to check the FIFO
status and fill them as needed. This will probably require additional
locking, so I think the proper solution would be to use DMA for
transfers bigger than the write FIFO size (we would get MX23 support
then, too). Sadly, we don't have DMA support yet and I don't have time
to implement it.

I am thinking if such transfers which would fail anyway, should
currently be rejected or if the driver should depend on BROKEN or both.

I am still undecided, though.

Please also check the patch I sent yesterday, there is also another one
coming after I get confirmation from a customer.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

possible MXS-i2c bug

[Wolfram Sang]

Hi Marek,

you forgot the most important list, linux-i2c ;)

> I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected.

Yes, we had the same this week/last week.

> Apparently, there might be some timing issue, when I put printk() into the at24 
> driver eeprom write function, it "fixed" itself. Though, replacing the i2c 
> driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some 
> flaw in it.

Try removing the printk and use the module paramter "io_limit=24" for
at24. Does this help? (Or bs<=24 with dd)

> If noone has any fixes already, I'll debug this tonight or so.

The write FIFO overflows, since it only can carry 8 words. There is
currently no check for that. One solution would be to check the FIFO
status and fill them as needed. This will probably require additional
locking, so I think the proper solution would be to use DMA for
transfers bigger than the write FIFO size (we would get MX23 support
then, too). Sadly, we don't have DMA support yet and I don't have time
to implement it.

I am thinking if such transfers which would fail anyway, should
currently be rejected or if the driver should depend on BROKEN or both.

I am still undecided, though.

Please also check the patch I sent yesterday, there is also another one
coming after I get confirmation from a customer.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120426/e69a67b6/attachment.sig>

Re: possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> Hi Marek,
> 
> you forgot the most important list, linux-i2c ;)
> 
> > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as
> > expected.
> 
> Yes, we had the same this week/last week.

Ah ok ;-)
> 
> > Apparently, there might be some timing issue, when I put printk() into
> > the at24 driver eeprom write function, it "fixed" itself. Though,
> > replacing the i2c driver with i2c gpio also fixed the issue, so I
> > suspect the i2c driver has some flaw in it.
> 
> Try removing the printk and use the module paramter "io_limit=24" for
> at24. Does this help? (Or bs<=24 with dd)
> 
> > If noone has any fixes already, I'll debug this tonight or so.
> 
> The write FIFO overflows, since it only can carry 8 words. There is
> currently no check for that. One solution would be to check the FIFO
> status and fill them as needed. This will probably require additional
> locking, so I think the proper solution would be to use DMA for
> transfers bigger than the write FIFO size (we would get MX23 support
> then, too). Sadly, we don't have DMA support yet and I don't have time
> to implement it.
> 
> I am thinking if such transfers which would fail anyway, should
> currently be rejected or if the driver should depend on BROKEN or both.
> 
> I am still undecided, though.

I've been thinking about the DMA approach. The problem I found out is that we 
need to transfer all messages we're given at time in one DMA chain ... at least 
that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 
manual).

Therefore we'd have to prepare scatterlist for each message that we're given to 
transfer. In case we'd be given huge set of messages, we'd have to allocate 
these scatterlists at runtime. And maybe we'd even have to allocate buffers (to 
resize the ones in msg.buf for the additional one byte for i2c address).

I don't like the idea of calling kmalloc() in mxs_i2c_xfer_message() at all, 
does anyone have any hint how to handle these?

> 
> Please also check the patch I sent yesterday, there is also another one
> coming after I get confirmation from a customer.

Thanks

> Regards,
> 
>    Wolfram

Best regards,
Marek Vasut

possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> Hi Marek,
> 
> you forgot the most important list, linux-i2c ;)
> 
> > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as
> > expected.
> 
> Yes, we had the same this week/last week.

Ah ok ;-)
> 
> > Apparently, there might be some timing issue, when I put printk() into
> > the at24 driver eeprom write function, it "fixed" itself. Though,
> > replacing the i2c driver with i2c gpio also fixed the issue, so I
> > suspect the i2c driver has some flaw in it.
> 
> Try removing the printk and use the module paramter "io_limit=24" for
> at24. Does this help? (Or bs<=24 with dd)
> 
> > If noone has any fixes already, I'll debug this tonight or so.
> 
> The write FIFO overflows, since it only can carry 8 words. There is
> currently no check for that. One solution would be to check the FIFO
> status and fill them as needed. This will probably require additional
> locking, so I think the proper solution would be to use DMA for
> transfers bigger than the write FIFO size (we would get MX23 support
> then, too). Sadly, we don't have DMA support yet and I don't have time
> to implement it.
> 
> I am thinking if such transfers which would fail anyway, should
> currently be rejected or if the driver should depend on BROKEN or both.
> 
> I am still undecided, though.

I've been thinking about the DMA approach. The problem I found out is that we 
need to transfer all messages we're given at time in one DMA chain ... at least 
that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 
manual).

Therefore we'd have to prepare scatterlist for each message that we're given to 
transfer. In case we'd be given huge set of messages, we'd have to allocate 
these scatterlists at runtime. And maybe we'd even have to allocate buffers (to 
resize the ones in msg.buf for the additional one byte for i2c address).

I don't like the idea of calling kmalloc() in mxs_i2c_xfer_message() at all, 
does anyone have any hint how to handle these?

> 
> Please also check the patch I sent yesterday, there is also another one
> coming after I get confirmation from a customer.

Thanks

> Regards,
> 
>    Wolfram

Best regards,
Marek Vasut

Re: possible MXS-i2c bug

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

Hi,

> I've been thinking about the DMA approach. The problem I found out is that we 
> need to transfer all messages we're given at time in one DMA chain ... at least 
> that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 
> manual).

I can't follow you:  mxs_i2c_xfer() gets a list of messages, yet they
are iterated over in mxs_i2c_xfer_msg(). A single I2C message is
unscattered and can't be bigger than 64KB, so that should be doable?

Am I missing something by already being in weekend-mode? :)

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

possible MXS-i2c bug

[Wolfram Sang]

Hi,

> I've been thinking about the DMA approach. The problem I found out is that we 
> need to transfer all messages we're given at time in one DMA chain ... at least 
> that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 
> manual).

I can't follow you:  mxs_i2c_xfer() gets a list of messages, yet they
are iterated over in mxs_i2c_xfer_msg(). A single I2C message is
unscattered and can't be bigger than 64KB, so that should be doable?

Am I missing something by already being in weekend-mode? :)

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/115b71e8/attachment.sig>

Re: possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> Hi,
> 
> > I've been thinking about the DMA approach. The problem I found out is
> > that we need to transfer all messages we're given at time in one DMA
> > chain ... at least that's how I understand it from the FSL manual (see
> > Fig. 27-10 in the mx28 manual).
> 
> I can't follow you:  mxs_i2c_xfer() gets a list of messages, yet they
> are iterated over in mxs_i2c_xfer_msg(). A single I2C message is
> unscattered and can't be bigger than 64KB, so that should be doable?

You can get a large list of i2c messages. In the current implementation, yes, 
they're iterated in mxs_i2c_xfer_msg. Correct.

If you want to do DMA transfer do/from the i2c controller, you have to take all 
these messages and create the chain of DMA transfers according to these 
messages, correct?

So either I missed something, or you need to do something like this to do the 
transfer:

for_each_message {
 sg_init_one()
 dma_map_sg()
 dmaengine_prep_slave_sg()
}

and then dmaengine_submit(). Is that correct?

And for each iteration of the cycle above, you need one scatterlist, you can't 
recycle one, correct?

So to create the chain. we'd need one scatterlist per message, correct?

> Am I missing something by already being in weekend-mode? :)

Best regards,
Marek Vasut

possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> Hi,
> 
> > I've been thinking about the DMA approach. The problem I found out is
> > that we need to transfer all messages we're given at time in one DMA
> > chain ... at least that's how I understand it from the FSL manual (see
> > Fig. 27-10 in the mx28 manual).
> 
> I can't follow you:  mxs_i2c_xfer() gets a list of messages, yet they
> are iterated over in mxs_i2c_xfer_msg(). A single I2C message is
> unscattered and can't be bigger than 64KB, so that should be doable?

You can get a large list of i2c messages. In the current implementation, yes, 
they're iterated in mxs_i2c_xfer_msg. Correct.

If you want to do DMA transfer do/from the i2c controller, you have to take all 
these messages and create the chain of DMA transfers according to these 
messages, correct?

So either I missed something, or you need to do something like this to do the 
transfer:

for_each_message {
 sg_init_one()
 dma_map_sg()
 dmaengine_prep_slave_sg()
}

and then dmaengine_submit(). Is that correct?

And for each iteration of the cycle above, you need one scatterlist, you can't 
recycle one, correct?

So to create the chain. we'd need one scatterlist per message, correct?

> Am I missing something by already being in weekend-mode? :)

Best regards,
Marek Vasut

Re: possible MXS-i2c bug

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]


> You can get a large list of i2c messages. In the current implementation, yes, 
> they're iterated in mxs_i2c_xfer_msg. Correct.
> 
> If you want to do DMA transfer do/from the i2c controller, you have to take all 
> these messages and create the chain of DMA transfers according to these 
> messages, correct?

This is what I wonder. I'd think one could work on a per message basis.
Regarding Figure 27-10, the first I2C write command could be sent
seperately (probably even via PIOQUEUE). The only thing to be chained is
the I2C read command and the actual reading of the data.

Just checked, the FSL driver does it basically this way, too.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

possible MXS-i2c bug

[Wolfram Sang]

> You can get a large list of i2c messages. In the current implementation, yes, 
> they're iterated in mxs_i2c_xfer_msg. Correct.
> 
> If you want to do DMA transfer do/from the i2c controller, you have to take all 
> these messages and create the chain of DMA transfers according to these 
> messages, correct?

This is what I wonder. I'd think one could work on a per message basis.
Regarding Figure 27-10, the first I2C write command could be sent
seperately (probably even via PIOQUEUE). The only thing to be chained is
the I2C read command and the actual reading of the data.

Just checked, the FSL driver does it basically this way, too.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/db3b5ffd/attachment.sig>

Re: possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> > You can get a large list of i2c messages. In the current implementation,
> > yes, they're iterated in mxs_i2c_xfer_msg. Correct.
> > 
> > If you want to do DMA transfer do/from the i2c controller, you have to
> > take all these messages and create the chain of DMA transfers according
> > to these messages, correct?
> 
> This is what I wonder. I'd think one could work on a per message basis.

But then you don't have the DMA chain linked. Which I wonder if the controller 
has any problem with or not. I tried yesterday, got wrotes working perfectly, 
but still had issues with reads, which is exactly what needs to be chained.

I'll poke further eventually.

> Regarding Figure 27-10, the first I2C write command could be sent
> seperately (probably even via PIOQUEUE).

I wonder if we want to combine pioqueue and DMA, that might create quite some 
franken-driver.

> The only thing to be chained is
> the I2C read command and the actual reading of the data.
> 
> Just checked, the FSL driver does it basically this way, too.

Which doesn't mean FSL driver does it correctly, but it probably worked for them 
and there was some bug in my DMA tinkering.

Best regards,
Marek Vasut

possible MXS-i2c bug

[Marek Vasut]

Dear Wolfram Sang,

> > You can get a large list of i2c messages. In the current implementation,
> > yes, they're iterated in mxs_i2c_xfer_msg. Correct.
> > 
> > If you want to do DMA transfer do/from the i2c controller, you have to
> > take all these messages and create the chain of DMA transfers according
> > to these messages, correct?
> 
> This is what I wonder. I'd think one could work on a per message basis.

But then you don't have the DMA chain linked. Which I wonder if the controller 
has any problem with or not. I tried yesterday, got wrotes working perfectly, 
but still had issues with reads, which is exactly what needs to be chained.

I'll poke further eventually.

> Regarding Figure 27-10, the first I2C write command could be sent
> seperately (probably even via PIOQUEUE).

I wonder if we want to combine pioqueue and DMA, that might create quite some 
franken-driver.

> The only thing to be chained is
> the I2C read command and the actual reading of the data.
> 
> Just checked, the FSL driver does it basically this way, too.

Which doesn't mean FSL driver does it correctly, but it probably worked for them 
and there was some bug in my DMA tinkering.

Best regards,
Marek Vasut

Re: possible MXS-i2c bug

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]


> But then you don't have the DMA chain linked. Which I wonder if the controller 
> has any problem with or not. I tried yesterday, got wrotes working perfectly, 
> but still had issues with reads, which is exactly what needs to be chained.

Yes, the read needs chaining of two "DMA command blocks". That should be
the only chain needed, because of how the driver handles reads. It is
still one I2C message, though.

> I'll poke further eventually.

I really hope it works out! That would be great.

> > Regarding Figure 27-10, the first I2C write command could be sent
> > seperately (probably even via PIOQUEUE).
> 
> I wonder if we want to combine pioqueue and DMA, that might create quite some 
> franken-driver.

Might be true, yet I hope it won't. Most I2C transfers tend to be very
small, so PIOQEUE would have some advantage here (less overhead).

> > The only thing to be chained is the I2C read command and the actual
> > reading of the data.
> > 
> > Just checked, the FSL driver does it basically this way, too.
> 
> Which doesn't mean FSL driver does it correctly, but it probably
> worked for them and there was some bug in my DMA tinkering.

It's only a proof-of-concept. We both know that :) (If it works, that
is, AFAICT that one will fail for transfers bigger than PAGE_SIZE, too).

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

possible MXS-i2c bug

[Wolfram Sang]

> But then you don't have the DMA chain linked. Which I wonder if the controller 
> has any problem with or not. I tried yesterday, got wrotes working perfectly, 
> but still had issues with reads, which is exactly what needs to be chained.

Yes, the read needs chaining of two "DMA command blocks". That should be
the only chain needed, because of how the driver handles reads. It is
still one I2C message, though.

> I'll poke further eventually.

I really hope it works out! That would be great.

> > Regarding Figure 27-10, the first I2C write command could be sent
> > seperately (probably even via PIOQUEUE).
> 
> I wonder if we want to combine pioqueue and DMA, that might create quite some 
> franken-driver.

Might be true, yet I hope it won't. Most I2C transfers tend to be very
small, so PIOQEUE would have some advantage here (less overhead).

> > The only thing to be chained is the I2C read command and the actual
> > reading of the data.
> > 
> > Just checked, the FSL driver does it basically this way, too.
> 
> Which doesn't mean FSL driver does it correctly, but it probably
> worked for them and there was some bug in my DMA tinkering.

It's only a proof-of-concept. We both know that :) (If it works, that
is, AFAICT that one will fail for transfers bigger than PAGE_SIZE, too).

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/10286e8f/attachment.sig>

[TEST PATCH] mxs-i2c DMA support

[Marek Vasut]

This patch is by no means complete. This patch does have issues, see below.

Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
Cc: Fabio Estevam <festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-mxs.c |  257 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 238 insertions(+), 19 deletions(-)

NOTE: It's still crappy and in-the-works, but I wanted to push it out ASAP so
others can play around (so don't bash me for coding style etc.). I still need
to fix various things here and there until I make this official:

1) The printk("%s[%i]\n", __func__, __LINE__); in mxs_i2c_xfer_msg(), without
   this it craps out for some reason, I'll have to check it after I get some
   sleep.
2) The coexistence of DMA and PIO transfers.

NOTE2: Wolfram, I merged our patch into this as I hoped it might fix my DMA/PIO
       coexistance issue, but obviously I'd like to see your patch go in
       separately.

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 3d471d5..eea2d8a 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -26,6 +26,9 @@
 #include <linux/platform_device.h>
 #include <linux/jiffies.h>
 #include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/fsl/mxs-dma.h>
 
 #include <mach/common.h>
 
@@ -109,6 +112,19 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+
+	bool				dma;
+	struct resource			*dmares;
+	struct dma_chan         	*dmach;
+	struct mxs_dma_data		dma_data;
+
+	struct mutex			mutex;
+
+	uint32_t			pio_data[2];
+	uint32_t			addr_data;
+	struct scatterlist		sg_io[2];
+
+	bool				read;
 };
 
 /*
@@ -194,7 +210,7 @@ static int mxs_i2c_wait_for_data(struct mxs_i2c_dev *i2c)
 
 static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 {
-	u32 data;
+	u32 data = 0;
 	int i;
 
 	for (i = 0; i < len; i++) {
@@ -210,6 +226,148 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 	return 0;
 }
 
+static void mxs_i2c_dma_irq_callback(void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+	struct i2c_adapter *adap = &i2c->adapter;
+
+	if (i2c->read) {
+		dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+	} else {
+		dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+	}
+
+	complete(&i2c->cmd_complete);
+}
+
+static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
+			struct i2c_msg *msg, uint32_t flags)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	if (msg->flags & I2C_M_RD) {
+		i2c->read = 1;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ;
+
+		/*
+		 * SELECT command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = MXS_CMD_I2C_SELECT;
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1);
+		dma_map_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/*
+		 * READ command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[1] = flags | MXS_I2C_CTRL0_MASTER_MODE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[1],
+					1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1,
+					DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto read_init_dma_fail;
+		}
+	} else {
+		i2c->read = 0;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE;
+
+		/*
+		 * WRITE command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto write_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_table(i2c->sg_io, 2);
+		sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1);
+		sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto write_init_dma_fail;
+		}
+	}
+
+	/*
+	 * The last descriptor must have this callback,
+	 * to finish the DMA transaction.
+	 */
+	desc->callback = mxs_i2c_dma_irq_callback;
+	desc->callback_param = i2c;
+
+	/* Start the transfer. */
+	dmaengine_submit(desc);
+
+	return 0;
+
+/* Read failpath. */
+read_init_dma_fail:
+	dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+select_init_dma_fail:
+	dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+select_init_pio_fail:
+	return 1;
+
+
+/* Write failpath. */
+write_init_dma_fail:
+	dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+write_init_pio_fail:
+	return 1;
+}
+
 /*
  * Low level master read/write transaction.
  */
@@ -220,31 +378,50 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	int ret;
 	int flags;
 
+	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+
 	dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
 		msg->addr, msg->len, msg->flags, stop);
 
 	if (msg->len == 0)
 		return -EINVAL;
 
+	i2c->dma = 1;//(msg->len > 16);
+
 	init_completion(&i2c->cmd_complete);
 
-	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+	if (i2c->dma) {
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
 
-	if (msg->flags & I2C_M_RD)
-		mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags);
-	else
-		mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len,
-					flags);
+		ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
+		if (ret)
+			return -EINVAL;
+	} else {
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_SET);
 
-	writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+		if (msg->flags & I2C_M_RD) {
+			mxs_i2c_pioq_setup_read(i2c, msg->addr,
+						msg->len, flags);
+		} else {
+			mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf,
+						msg->len, flags);
+		}
+
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
+	}
+
+	printk("%s[%i]\n", __func__, __LINE__);
 
 	ret = wait_for_completion_timeout(&i2c->cmd_complete,
 						msecs_to_jiffies(1000));
+
 	if (ret == 0)
 		goto timeout;
 
-	if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
+	if (!i2c->dma && (!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
 		ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len);
 		if (ret)
 			goto timeout;
@@ -252,6 +429,9 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 	if (i2c->cmd_err == -ENXIO)
 		mxs_i2c_reset(i2c);
+	else
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
 
 	dev_dbg(i2c->dev, "Done with err=%d\n", i2c->cmd_err);
 
@@ -268,13 +448,20 @@ static int mxs_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 {
 	int i;
 	int err;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	mutex_lock(&i2c->mutex);
 
 	for (i = 0; i < num; i++) {
 		err = mxs_i2c_xfer_msg(adap, &msgs[i], i == (num - 1));
-		if (err)
+		if (err) {
+			mutex_unlock(&i2c->mutex);
 			return err;
+		}
 	}
 
+	mutex_unlock(&i2c->mutex);
+
 	return num;
 }
 
@@ -305,7 +494,7 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id)
 	is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
 		MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
 
-	if (is_last_cmd || i2c->cmd_err)
+	if ((is_last_cmd || i2c->cmd_err) && !i2c->dma)
 		complete(&i2c->cmd_complete);
 
 	writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR);
@@ -318,23 +507,45 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	if (!mxs_dma_is_apbx(chan))
+		return false;
+
+	if (chan->chan_id != i2c->dmares->start)
+		return false;
+
+	chan->private = &i2c->dma_data;
+
+	return true;
+}
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mxs_i2c_dev *i2c;
 	struct i2c_adapter *adap;
-	struct resource *res;
+	struct resource *res, *dmares;
 	resource_size_t res_size;
-	int err, irq;
+	int err, irq, dmairq;
+	dma_cap_mask_t mask;
 
 	i2c = devm_kzalloc(dev, sizeof(struct mxs_i2c_dev), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
+	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	irq = platform_get_irq(pdev, 0);
+	dmairq = platform_get_irq(pdev, 1);
+
+	if (!res || !dmares || irq < 0 || dmairq < 0)
 		return -ENOENT;
 
+	mutex_init(&i2c->mutex);
+
 	res_size = resource_size(res);
 	if (!devm_request_mem_region(dev, res->start, res_size, res->name))
 		return -EBUSY;
@@ -343,15 +554,23 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (!i2c->regs)
 		return -EBUSY;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
 	if (err)
 		return err;
 
+	i2c->dmares = dmares;
 	i2c->dev = dev;
+
+	/* Setup the DMA */
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	i2c->dma_data.chan_irq = dmairq;
+	i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c);
+	if (!i2c->dmach) {
+		dev_err(dev, "Failed to request dma\n");
+		return -ENODEV;
+	}
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
-- 
1.7.10

[TEST PATCH] mxs-i2c DMA support

[Marek Vasut]

This patch is by no means complete. This patch does have issues, see below.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-i2c at vger.kernel.org
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/i2c/busses/i2c-mxs.c |  257 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 238 insertions(+), 19 deletions(-)

NOTE: It's still crappy and in-the-works, but I wanted to push it out ASAP so
others can play around (so don't bash me for coding style etc.). I still need
to fix various things here and there until I make this official:

1) The printk("%s[%i]\n", __func__, __LINE__); in mxs_i2c_xfer_msg(), without
   this it craps out for some reason, I'll have to check it after I get some
   sleep.
2) The coexistence of DMA and PIO transfers.

NOTE2: Wolfram, I merged our patch into this as I hoped it might fix my DMA/PIO
       coexistance issue, but obviously I'd like to see your patch go in
       separately.

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 3d471d5..eea2d8a 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -26,6 +26,9 @@
 #include <linux/platform_device.h>
 #include <linux/jiffies.h>
 #include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/fsl/mxs-dma.h>
 
 #include <mach/common.h>
 
@@ -109,6 +112,19 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+
+	bool				dma;
+	struct resource			*dmares;
+	struct dma_chan         	*dmach;
+	struct mxs_dma_data		dma_data;
+
+	struct mutex			mutex;
+
+	uint32_t			pio_data[2];
+	uint32_t			addr_data;
+	struct scatterlist		sg_io[2];
+
+	bool				read;
 };
 
 /*
@@ -194,7 +210,7 @@ static int mxs_i2c_wait_for_data(struct mxs_i2c_dev *i2c)
 
 static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 {
-	u32 data;
+	u32 data = 0;
 	int i;
 
 	for (i = 0; i < len; i++) {
@@ -210,6 +226,148 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 	return 0;
 }
 
+static void mxs_i2c_dma_irq_callback(void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+	struct i2c_adapter *adap = &i2c->adapter;
+
+	if (i2c->read) {
+		dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+	} else {
+		dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+	}
+
+	complete(&i2c->cmd_complete);
+}
+
+static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
+			struct i2c_msg *msg, uint32_t flags)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	if (msg->flags & I2C_M_RD) {
+		i2c->read = 1;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ;
+
+		/*
+		 * SELECT command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = MXS_CMD_I2C_SELECT;
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1);
+		dma_map_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/*
+		 * READ command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[1] = flags | MXS_I2C_CTRL0_MASTER_MODE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[1],
+					1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1,
+					DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto read_init_dma_fail;
+		}
+	} else {
+		i2c->read = 0;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE;
+
+		/*
+		 * WRITE command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto write_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_table(i2c->sg_io, 2);
+		sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1);
+		sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(&adap->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto write_init_dma_fail;
+		}
+	}
+
+	/*
+	 * The last descriptor must have this callback,
+	 * to finish the DMA transaction.
+	 */
+	desc->callback = mxs_i2c_dma_irq_callback;
+	desc->callback_param = i2c;
+
+	/* Start the transfer. */
+	dmaengine_submit(desc);
+
+	return 0;
+
+/* Read failpath. */
+read_init_dma_fail:
+	dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+select_init_dma_fail:
+	dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+select_init_pio_fail:
+	return 1;
+
+
+/* Write failpath. */
+write_init_dma_fail:
+	dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+write_init_pio_fail:
+	return 1;
+}
+
 /*
  * Low level master read/write transaction.
  */
@@ -220,31 +378,50 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	int ret;
 	int flags;
 
+	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+
 	dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
 		msg->addr, msg->len, msg->flags, stop);
 
 	if (msg->len == 0)
 		return -EINVAL;
 
+	i2c->dma = 1;//(msg->len > 16);
+
 	init_completion(&i2c->cmd_complete);
 
-	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+	if (i2c->dma) {
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
 
-	if (msg->flags & I2C_M_RD)
-		mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags);
-	else
-		mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len,
-					flags);
+		ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
+		if (ret)
+			return -EINVAL;
+	} else {
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_SET);
 
-	writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+		if (msg->flags & I2C_M_RD) {
+			mxs_i2c_pioq_setup_read(i2c, msg->addr,
+						msg->len, flags);
+		} else {
+			mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf,
+						msg->len, flags);
+		}
+
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
+	}
+
+	printk("%s[%i]\n", __func__, __LINE__);
 
 	ret = wait_for_completion_timeout(&i2c->cmd_complete,
 						msecs_to_jiffies(1000));
+
 	if (ret == 0)
 		goto timeout;
 
-	if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
+	if (!i2c->dma && (!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
 		ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len);
 		if (ret)
 			goto timeout;
@@ -252,6 +429,9 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 	if (i2c->cmd_err == -ENXIO)
 		mxs_i2c_reset(i2c);
+	else
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
 
 	dev_dbg(i2c->dev, "Done with err=%d\n", i2c->cmd_err);
 
@@ -268,13 +448,20 @@ static int mxs_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 {
 	int i;
 	int err;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	mutex_lock(&i2c->mutex);
 
 	for (i = 0; i < num; i++) {
 		err = mxs_i2c_xfer_msg(adap, &msgs[i], i == (num - 1));
-		if (err)
+		if (err) {
+			mutex_unlock(&i2c->mutex);
 			return err;
+		}
 	}
 
+	mutex_unlock(&i2c->mutex);
+
 	return num;
 }
 
@@ -305,7 +494,7 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id)
 	is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
 		MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
 
-	if (is_last_cmd || i2c->cmd_err)
+	if ((is_last_cmd || i2c->cmd_err) && !i2c->dma)
 		complete(&i2c->cmd_complete);
 
 	writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR);
@@ -318,23 +507,45 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	if (!mxs_dma_is_apbx(chan))
+		return false;
+
+	if (chan->chan_id != i2c->dmares->start)
+		return false;
+
+	chan->private = &i2c->dma_data;
+
+	return true;
+}
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mxs_i2c_dev *i2c;
 	struct i2c_adapter *adap;
-	struct resource *res;
+	struct resource *res, *dmares;
 	resource_size_t res_size;
-	int err, irq;
+	int err, irq, dmairq;
+	dma_cap_mask_t mask;
 
 	i2c = devm_kzalloc(dev, sizeof(struct mxs_i2c_dev), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
+	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	irq = platform_get_irq(pdev, 0);
+	dmairq = platform_get_irq(pdev, 1);
+
+	if (!res || !dmares || irq < 0 || dmairq < 0)
 		return -ENOENT;
 
+	mutex_init(&i2c->mutex);
+
 	res_size = resource_size(res);
 	if (!devm_request_mem_region(dev, res->start, res_size, res->name))
 		return -EBUSY;
@@ -343,15 +554,23 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (!i2c->regs)
 		return -EBUSY;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
 	if (err)
 		return err;
 
+	i2c->dmares = dmares;
 	i2c->dev = dev;
+
+	/* Setup the DMA */
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	i2c->dma_data.chan_irq = dmairq;
+	i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c);
+	if (!i2c->dmach) {
+		dev_err(dev, "Failed to request dma\n");
+		return -ENODEV;
+	}
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
-- 
1.7.10