Patchwork [2/2] I2C: Implement DMA support into mxs-i2c

login
register
mail settings
Submitter Marek Vasut
Date April 29, 2012, 10:36 p.m.
Message ID <1335738969-27445-2-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/155776/
State New
Headers show

Comments

Marek Vasut - April 29, 2012, 10:36 p.m.
This patch implements DMA support into mxs-i2c. DMA transfers are now enabled by
default, though it is possible for example for debugging purposes, to disable
them via i2c->dma_mode.

Cc: Linux I2C <linux-i2c@vger.kernel.org>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 arch/arm/mach-mxs/devices/platform-mxs-i2c.c    |    5 +
 arch/arm/mach-mxs/include/mach/devices-common.h |    1 +
 drivers/i2c/busses/i2c-mxs.c                    |  261 +++++++++++++++++++++--
 3 files changed, 245 insertions(+), 22 deletions(-)
Wolfram Sang - April 30, 2012, 6:05 a.m.
On Mon, Apr 30, 2012 at 12:36:09AM +0200, Marek Vasut wrote:
> This patch implements DMA support into mxs-i2c. DMA transfers are now enabled by
> default, though it is possible for example for debugging purposes, to disable
> them via i2c->dma_mode.
> 
> Cc: Linux I2C <linux-i2c@vger.kernel.org>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>

Looks promising, will give it a shot this week! Thanks.
Very quick review.

> ---
>  arch/arm/mach-mxs/devices/platform-mxs-i2c.c    |    5 +
>  arch/arm/mach-mxs/include/mach/devices-common.h |    1 +
>  drivers/i2c/busses/i2c-mxs.c                    |  261 +++++++++++++++++++++--
>  3 files changed, 245 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> index 79222ec..a94f308 100644
> --- a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> +++ b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> @@ -14,6 +14,7 @@
>  	{								\
>  		.id = _id,						\
>  		.iobase = soc ## _I2C ## _id ## _BASE_ADDR,		\
> +		.dma	= soc ## _DMA_I2C ## _id,			\
>  		.errirq = soc ## _INT_I2C ## _id ## _ERROR,		\
>  		.dmairq = soc ## _INT_I2C ## _id ## _DMA,		\
>  	}
> @@ -37,6 +38,10 @@ struct platform_device *__init mxs_add_mxs_i2c(
>  			.end = data->iobase + SZ_8K - 1,
>  			.flags = IORESOURCE_MEM,
>  		}, {
> +			.start	= data->dma,
> +			.end	= data->dma,
> +			.flags	= IORESOURCE_DMA,
> +		}, {
>  			.start = data->errirq,
>  			.end = data->errirq,
>  			.flags = IORESOURCE_IRQ,
> diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h
> index f2e3839..791d99c 100644
> --- a/arch/arm/mach-mxs/include/mach/devices-common.h
> +++ b/arch/arm/mach-mxs/include/mach/devices-common.h
> @@ -80,6 +80,7 @@ mxs_add_gpmi_nand(const struct gpmi_nand_platform_data *pdata,
>  struct mxs_mxs_i2c_data {
>  	int id;
>  	resource_size_t iobase;
> +	resource_size_t dma;
>  	resource_size_t errirq;
>  	resource_size_t dmairq;
>  };

You need to either split this and send via alkml or I need an ACK from
Shawn if it shall go in via I2C. You don't have Shawn on CC, please use
scripts/get_maintainer.pl on your patchesr; you have been missing
people/lists at least for the third time now.

>  static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
>  {
> -	u32 data;
> +	u32 data = 0;

Unrelated, useless change.

>  	int i;
> +
> +	/*
> +	 * The MXS I2C DMA mode is prefered and enabled by default.
> +	 * The PIO mode is still supported, but should be used only
> +	 * for debuging purposes etc.
> +	 */

For exactness, should be "PIOQUEUE" instead of PIO. There is a PIO mode,
but we don't use it.

> +	i2c->dma_mode = 1;

Have you measured the overhead of DMA? I'd still think that 

	if (MX23 || msg->len > 24)
		do_dma
	else
		do_pioqueue

might be worth considered.

Regards,

   Wolfram
Marek Vasut - April 30, 2012, 12:10 p.m.
Dear Wolfram Sang,

> On Mon, Apr 30, 2012 at 12:36:09AM +0200, Marek Vasut wrote:
> > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > enabled by default, though it is possible for example for debugging
> > purposes, to disable them via i2c->dma_mode.
> > 
> > Cc: Linux I2C <linux-i2c@vger.kernel.org>
> > Cc: Detlev Zundel <dzu@denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> 
> Looks promising, will give it a shot this week! Thanks.
> Very quick review.
> 
> > ---
> > 
> >  arch/arm/mach-mxs/devices/platform-mxs-i2c.c    |    5 +
> >  arch/arm/mach-mxs/include/mach/devices-common.h |    1 +
> >  drivers/i2c/busses/i2c-mxs.c                    |  261
> >  +++++++++++++++++++++-- 3 files changed, 245 insertions(+), 22
> >  deletions(-)
> > 
> > diff --git a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> > b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c index 79222ec..a94f308
> > 100644
> > --- a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> > +++ b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
> > @@ -14,6 +14,7 @@
> > 
> >  	{								\
> >  	
> >  		.id = _id,						\
> >  		.iobase = soc ## _I2C ## _id ## _BASE_ADDR,		\
> > 
> > +		.dma	= soc ## _DMA_I2C ## _id,			\
> > 
> >  		.errirq = soc ## _INT_I2C ## _id ## _ERROR,		\
> >  		.dmairq = soc ## _INT_I2C ## _id ## _DMA,		\
> >  	
> >  	}
> > 
> > @@ -37,6 +38,10 @@ struct platform_device *__init mxs_add_mxs_i2c(
> > 
> >  			.end = data->iobase + SZ_8K - 1,
> >  			.flags = IORESOURCE_MEM,
> >  		
> >  		}, {
> > 
> > +			.start	= data->dma,
> > +			.end	= data->dma,
> > +			.flags	= IORESOURCE_DMA,
> > +		}, {
> > 
> >  			.start = data->errirq,
> >  			.end = data->errirq,
> >  			.flags = IORESOURCE_IRQ,
> > 
> > diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h
> > b/arch/arm/mach-mxs/include/mach/devices-common.h index f2e3839..791d99c
> > 100644
> > --- a/arch/arm/mach-mxs/include/mach/devices-common.h
> > +++ b/arch/arm/mach-mxs/include/mach/devices-common.h
> > @@ -80,6 +80,7 @@ mxs_add_gpmi_nand(const struct gpmi_nand_platform_data
> > *pdata,
> > 
> >  struct mxs_mxs_i2c_data {
> >  
> >  	int id;
> >  	resource_size_t iobase;
> > 
> > +	resource_size_t dma;
> > 
> >  	resource_size_t errirq;
> >  	resource_size_t dmairq;
> >  
> >  };
> 
> You need to either split this and send via alkml or I need an ACK from
> Shawn if it shall go in via I2C. You don't have Shawn on CC, please use
> scripts/get_maintainer.pl on your patchesr; you have been missing
> people/lists at least for the third time now.
> 
> >  static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int
> >  len) {
> > 
> > -	u32 data;
> > +	u32 data = 0;
> 
> Unrelated, useless change.

Have you tried compiling the code with gcc 4.7?

> 
> >  	int i;
> > 
> > +
> > +	/*
> > +	 * The MXS I2C DMA mode is prefered and enabled by default.
> > +	 * The PIO mode is still supported, but should be used only
> > +	 * for debuging purposes etc.
> > +	 */
> 
> For exactness, should be "PIOQUEUE" instead of PIO. There is a PIO mode,
> but we don't use it.

Ok, it indeed should.

> 
> > +	i2c->dma_mode = 1;
> 
> Have you measured the overhead of DMA? I'd still think that
> 
> 	if (MX23 || msg->len > 24)
> 		do_dma
> 	else
> 		do_pioqueue
> 
> might be worth considered.

It might, but in the 2.6.35.3 I have from FSL, it only does:

WARN_ONCE(len > 24, "choose DMA mode if xfer len > 24 bytes\n");

Also, I tried switching PIOQUEUE/DMA, but didn't get it working. We might as 
well make this dma/pioqueue mode configurable as well as speed via platform data 
until someone comes up with a patch that can mix pioqueue and dma?
> 
> Regards,
> 
>    Wolfram
Wolfram Sang - April 30, 2012, 7:52 p.m.
> > >  static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int
> > >  len) {
> > > 
> > > -	u32 data;
> > > +	u32 data = 0;
> > 
> > Unrelated, useless change.
> 
> Have you tried compiling the code with gcc 4.7?

What has the compiler version to do with the relevance? :) This has
nothing to do with adding DMA support. Plus, nobody so far could tell me
what path gcc is seeing there.

> > > +	i2c->dma_mode = 1;
> > 
> > Have you measured the overhead of DMA? I'd still think that
> > 
> > 	if (MX23 || msg->len > 24)
> > 		do_dma
> > 	else
> > 		do_pioqueue
> > 
> > might be worth considered.
> 
> It might, but in the 2.6.35.3 I have from FSL, it only does:
> 
> WARN_ONCE(len > 24, "choose DMA mode if xfer len > 24 bytes\n");

Well, we are back to the "FSL code is just a reference" topic. I truly
hope that part was implemented because of laziness and not because of
hardware limitations.

> Also, I tried switching PIOQUEUE/DMA, but didn't get it working. We might as 
> well make this dma/pioqueue mode configurable as well as speed via platform data 
> until someone comes up with a patch that can mix pioqueue and dma?

User configuration is not an option here IMO, because PIOQUEUE would still
fail for bigger transfers. "Always DMA" might be better then. But I will
try to switch modes this week, that's just too tempting.

Regards,

   Wolfram
Marek Vasut - April 30, 2012, 8:07 p.m.
Dear Wolfram Sang,

> > > >  static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int
> > > >  len) {
> > > > 
> > > > -	u32 data;
> > > > +	u32 data = 0;
> > > 
> > > Unrelated, useless change.
> > 
> > Have you tried compiling the code with gcc 4.7?
> 
> What has the compiler version to do with the relevance? :) This has
> nothing to do with adding DMA support. Plus, nobody so far could tell me
> what path gcc is seeing there.

It complains that data might be undefined, do you want it in a separate patch 
then ?

> 
> > > > +	i2c->dma_mode = 1;
> > > 
> > > Have you measured the overhead of DMA? I'd still think that
> > > 
> > > 	if (MX23 || msg->len > 24)
> > > 	
> > > 		do_dma
> > > 	
> > > 	else
> > > 	
> > > 		do_pioqueue
> > > 
> > > might be worth considered.
> > 
> > It might, but in the 2.6.35.3 I have from FSL, it only does:
> > 
> > WARN_ONCE(len > 24, "choose DMA mode if xfer len > 24 bytes\n");
> 
> Well, we are back to the "FSL code is just a reference" topic.

Obviously.

> I truly
> hope that part was implemented because of laziness and not because of
> hardware limitations.

I still havent figured out how to flip between pioqueue mode and dma mode 
without the controller getting confused, so I prefer to postpone it into 
subsequent patch. Eventually, I agree it'd be cool to be able to do this 
switching.

> > Also, I tried switching PIOQUEUE/DMA, but didn't get it working. We might
> > as well make this dma/pioqueue mode configurable as well as speed via
> > platform data until someone comes up with a patch that can mix pioqueue
> > and dma?
> 
> User configuration is not an option here IMO, because PIOQUEUE would still
> fail for bigger transfers. "Always DMA" might be better then.

Yes, that's what I have in V2.

> But I will
> try to switch modes this week, that's just too tempting.

I'll have a V2 patch for you by ... in a bit ;-) Also, you can grab latest 
version here (will be updated before I submit V2): 
http://git.kernel.org/?p=linux/kernel/git/marex/linux-2.6.git;a=shortlog;h=refs/heads/mxs-
i2c

> 
> Regards,
> 
>    Wolfram

Best regards,
Marek Vasut
Wolfram Sang - April 30, 2012, 8:30 p.m.
> > What has the compiler version to do with the relevance? :) This has
> > nothing to do with adding DMA support. Plus, nobody so far could tell me
> > what path gcc is seeing there.
> 
> It complains that data might be undefined, do you want it in a separate patch 
> then ?

Yes, but only if you can prove that the compiler is right.

> I still havent figured out how to flip between pioqueue mode and dma mode 
> without the controller getting confused, so I prefer to postpone it into 
> subsequent patch. Eventually, I agree it'd be cool to be able to do this 
> switching.

Yes, send V2 with DMA by default, and I will give a try on the
mode-switching later, too.

Thanks,

   Wolfram
Marek Vasut - April 30, 2012, 8:45 p.m.
Dear Wolfram Sang,

> > > What has the compiler version to do with the relevance? :) This has
> > > nothing to do with adding DMA support. Plus, nobody so far could tell
> > > me what path gcc is seeing there.
> > 
> > It complains that data might be undefined, do you want it in a separate
> > patch then ?
> 
> Yes, but only if you can prove that the compiler is right.

It's not right, because that variable is always initialized, but this at least 
squashes the compile warning.

> > I still havent figured out how to flip between pioqueue mode and dma mode
> > without the controller getting confused, so I prefer to postpone it into
> > subsequent patch. Eventually, I agree it'd be cool to be able to do this
> > switching.
> 
> Yes, send V2 with DMA by default, and I will give a try on the
> mode-switching later, too.

I'd prefer to have PIOQ by default, to avoid breaking some of the boards. Or can 
you test on all of them?

> 
> Thanks,
> 
>    Wolfram

Best regards,
Marek Vasut
Wolfram Sang - April 30, 2012, 9:01 p.m.
> > > It complains that data might be undefined, do you want it in a separate
> > > patch then ?
> > 
> > Yes, but only if you can prove that the compiler is right.
> 
> It's not right, because that variable is always initialized, but this at least 
> squashes the compile warning.

The compiler needs to be fixed, not the kernel.

> > Yes, send V2 with DMA by default, and I will give a try on the
> > mode-switching later, too.
> 
> I'd prefer to have PIOQ by default, to avoid breaking some of the boards. Or can 
> you test on all of them?

? What kind of breakage do you expect? I think it is okay if we pull it
in during the merge window; we can fix issues then till the relase (and
maybe mode switching works, after all).
Marek Vasut - April 30, 2012, 9:19 p.m.
Dear Wolfram Sang,

> > > > It complains that data might be undefined, do you want it in a
> > > > separate patch then ?
> > > 
> > > Yes, but only if you can prove that the compiler is right.
> > 
> > It's not right, because that variable is always initialized, but this at
> > least squashes the compile warning.
> 
> The compiler needs to be fixed, not the kernel.
> 
> > > Yes, send V2 with DMA by default, and I will give a try on the
> > > mode-switching later, too.
> > 
> > I'd prefer to have PIOQ by default, to avoid breaking some of the boards.
> > Or can you test on all of them?
> 
> ? What kind of breakage do you expect? I think it is okay if we pull it
> in during the merge window; we can fix issues then till the relase (and
> maybe mode switching works, after all).

Ok, so be it then.

Best regards,
Marek Vasut
Shawn Guo - May 1, 2012, 1:58 p.m.
On Mon, Apr 30, 2012 at 11:01:08PM +0200, Wolfram Sang wrote:
> 
> > > > It complains that data might be undefined, do you want it in a separate
> > > > patch then ?
> > > 
> > > Yes, but only if you can prove that the compiler is right.
> > 
> > It's not right, because that variable is always initialized, but this at least 
> > squashes the compile warning.
> 
> The compiler needs to be fixed, not the kernel.
> 
Heh, this is coming up again.  I'm not convincing you to take the
change, but just curious what if this is not a compile warning but
an error for the same cause that compiler is not right.
Marek Vasut - May 1, 2012, 2:02 p.m.
Dear Shawn Guo,

> On Mon, Apr 30, 2012 at 11:01:08PM +0200, Wolfram Sang wrote:
> > > > > It complains that data might be undefined, do you want it in a
> > > > > separate patch then ?
> > > > 
> > > > Yes, but only if you can prove that the compiler is right.
> > > 
> > > It's not right, because that variable is always initialized, but this
> > > at least squashes the compile warning.
> > 
> > The compiler needs to be fixed, not the kernel.

I second this.

> Heh, this is coming up again.  I'm not convincing you to take the
> change, but just curious what if this is not a compile warning but
> an error for the same cause that compiler is not right.

Can you elaborate?

Best regards,
Marek Vasut
Shawn Guo - May 1, 2012, 2:09 p.m.
On Tue, May 01, 2012 at 04:02:56PM +0200, Marek Vasut wrote:
> Dear Shawn Guo,
> 
> > On Mon, Apr 30, 2012 at 11:01:08PM +0200, Wolfram Sang wrote:
> > > > > > It complains that data might be undefined, do you want it in a
> > > > > > separate patch then ?
> > > > > 
> > > > > Yes, but only if you can prove that the compiler is right.
> > > > 
> > > > It's not right, because that variable is always initialized, but this
> > > > at least squashes the compile warning.
> > > 
> > > The compiler needs to be fixed, not the kernel.
> 
> I second this.
> 
I agree on this too.

> > Heh, this is coming up again.  I'm not convincing you to take the
> > change, but just curious what if this is not a compile warning but
> > an error for the same cause that compiler is not right.
> 
> Can you elaborate?
> 
Will we have the kernel not compilable for a few cycles until the
compiler fixing is available, or we work it around to keep kernel
compiling and working before we have the compiler fixed?
Marek Vasut - May 1, 2012, 2:14 p.m.
Dear Shawn Guo,

> On Tue, May 01, 2012 at 04:02:56PM +0200, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> > > On Mon, Apr 30, 2012 at 11:01:08PM +0200, Wolfram Sang wrote:
> > > > > > > It complains that data might be undefined, do you want it in a
> > > > > > > separate patch then ?
> > > > > > 
> > > > > > Yes, but only if you can prove that the compiler is right.
> > > > > 
> > > > > It's not right, because that variable is always initialized, but
> > > > > this at least squashes the compile warning.
> > > > 
> > > > The compiler needs to be fixed, not the kernel.
> > 
> > I second this.
> 
> I agree on this too.
> 
> > > Heh, this is coming up again.  I'm not convincing you to take the
> > > change, but just curious what if this is not a compile warning but
> > > an error for the same cause that compiler is not right.
> > 
> > Can you elaborate?
> 
> Will we have the kernel not compilable for a few cycles until the
> compiler fixing is available, or we work it around to keep kernel
> compiling and working before we have the compiler fixed?

This is kind of lacmus paper showing if the compiler was fixed. On the other 
hand, many people won't have a fixed compiler for a long time, so I'd be for 
applying this patch.

Best regards,
Marek Vasut
Russell King - ARM Linux - May 1, 2012, 2:37 p.m.
On Tue, May 01, 2012 at 09:58:03PM +0800, Shawn Guo wrote:
> On Mon, Apr 30, 2012 at 11:01:08PM +0200, Wolfram Sang wrote:
> > 
> > > > > It complains that data might be undefined, do you want it in a separate
> > > > > patch then ?
> > > > 
> > > > Yes, but only if you can prove that the compiler is right.
> > > 
> > > It's not right, because that variable is always initialized, but this at least 
> > > squashes the compile warning.
> > 
> > The compiler needs to be fixed, not the kernel.
> > 
> Heh, this is coming up again.  I'm not convincing you to take the
> change, but just curious what if this is not a compile warning but
> an error for the same cause that compiler is not right.

It's a compiler warning.

Please look at the wording of the warning.  The compiler uses "may" not
"is".  That means it can't come to a conclusion about it.

But we, as humans with our biological inference engines, can see that
the code is correct, and we _can_ choose to ignore the warning -
just like I do with:

drivers/video/omap2/displays/panel-taal.c: In function 'taal_num_errors_show':
drivers/video/omap2/displays/panel-taal.c:605: warning: 'errors' may be used uninitialized in this function

static int taal_dcs_read_1(struct taal_data *td, u8 dcs_cmd, u8 *data)
{
        int r;
        u8 buf[1];
        r = dsi_vc_dcs_read(td->dssdev, td->channel, dcs_cmd, buf, 1);
        if (r < 0)
                return r;
        *data = buf[0];  
        return 0;
}

        u8 errors;

        mutex_lock(&td->lock);
        if (td->enabled) {
                dsi_bus_lock(dssdev);
                r = taal_wake_up(dssdev);
                if (!r) 
                        r = taal_dcs_read_1(td, DCS_READ_NUM_ERRORS, &errors);
                dsi_bus_unlock(dssdev);
        } else {
                r = -ENODEV;
        }
        mutex_unlock(&td->lock);

        if (r)
                return r;

        return snprintf(buf, PAGE_SIZE, "%d\n", errors);

See?  We can infer from the above that that 'errors' will always be set
if r is zero - but the compiler is saying it's not clever enough to do
that automatically.

This doesn't mean the code _has_ to be changed - we can see that the
above warning is false.  Though, if there is a way to rewrite it to make
it easier for us humans to read and that makes the warning go away, it
_might_ be a good idea to make the change - but only for a maintainability
reason.

The reverse is also true - if trying to fix a warning like this makes
the code more difficult to understand, then the warning _should_ stay.

That's actually the point of programming languages - the ability to
express our instructions to a computer in a way that both we and the
compiler can readily understand.  The most important part of that is
that _we_ understand what's been written.

Patch

diff --git a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
index 79222ec..a94f308 100644
--- a/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
+++ b/arch/arm/mach-mxs/devices/platform-mxs-i2c.c
@@ -14,6 +14,7 @@ 
 	{								\
 		.id = _id,						\
 		.iobase = soc ## _I2C ## _id ## _BASE_ADDR,		\
+		.dma	= soc ## _DMA_I2C ## _id,			\
 		.errirq = soc ## _INT_I2C ## _id ## _ERROR,		\
 		.dmairq = soc ## _INT_I2C ## _id ## _DMA,		\
 	}
@@ -37,6 +38,10 @@  struct platform_device *__init mxs_add_mxs_i2c(
 			.end = data->iobase + SZ_8K - 1,
 			.flags = IORESOURCE_MEM,
 		}, {
+			.start	= data->dma,
+			.end	= data->dma,
+			.flags	= IORESOURCE_DMA,
+		}, {
 			.start = data->errirq,
 			.end = data->errirq,
 			.flags = IORESOURCE_IRQ,
diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h
index f2e3839..791d99c 100644
--- a/arch/arm/mach-mxs/include/mach/devices-common.h
+++ b/arch/arm/mach-mxs/include/mach/devices-common.h
@@ -80,6 +80,7 @@  mxs_add_gpmi_nand(const struct gpmi_nand_platform_data *pdata,
 struct mxs_mxs_i2c_data {
 	int id;
 	resource_size_t iobase;
+	resource_size_t dma;
 	resource_size_t errirq;
 	resource_size_t dmairq;
 };
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 0d8e485..b364b8b 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>
 
@@ -113,6 +116,16 @@  struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+
+	/* DMA support components */
+	bool				dma_mode;
+	struct resource			*dmares;
+	struct dma_chan         	*dmach;
+	struct mxs_dma_data		dma_data;
+	uint32_t			pio_data[2];
+	uint32_t			addr_data;
+	struct scatterlist		sg_io[2];
+	bool				dma_read;
 };
 
 /*
@@ -129,7 +142,12 @@  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 	writel(0x0015000d, i2c->regs + MXS_I2C_TIMING2);
 
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
-	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+
+	if (i2c->dma_mode)
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
+	else
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
 }
 
@@ -204,7 +222,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++) {
@@ -220,9 +238,155 @@  static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 	return 0;
 }
 
+static void mxs_i2c_dma_finish(struct mxs_i2c_dev *i2c)
+{
+	if (i2c->dma_read) {
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+	} else {
+		dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+	}
+}
+
+static void mxs_i2c_dma_irq_callback(void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	complete(&i2c->cmd_complete);
+	mxs_i2c_dma_finish(i2c);
+}
+
+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->dma_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(i2c->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(i2c->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(i2c->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_CMD_I2C_READ |
+				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(i2c->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(i2c->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(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto read_init_dma_fail;
+		}
+	} else {
+		i2c->dma_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(i2c->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(i2c->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(i2c->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);
+	dma_async_issue_pending(i2c->dmach);
+	return 0;
+
+/* Read failpath. */
+read_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+select_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+select_init_pio_fail:
+	return 1;
+
+
+/* Write failpath. */
+write_init_dma_fail:
+	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+write_init_pio_fail:
+	return 1;
+}
+
 /*
  * Low level master read/write transaction.
  */
+
 static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 				int stop)
 {
@@ -230,6 +394,8 @@  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);
 
@@ -238,23 +404,30 @@  static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 	init_completion(&i2c->cmd_complete);
 
-	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
-
-	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);
+	if (i2c->dma_mode) {
+		ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
+		if (ret)
+			return -EINVAL;
+	} else {
+		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,
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
+	}
 
 	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_mode && (!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
 		ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len);
 		if (ret)
 			goto timeout;
@@ -269,7 +442,9 @@  static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 timeout:
 	dev_dbg(i2c->dev, "Timeout!\n");
+	mxs_i2c_dma_finish(i2c);
 	mxs_i2c_reset(i2c);
+
 	return -ETIMEDOUT;
 }
 
@@ -312,11 +487,13 @@  static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id)
 	else
 		i2c->cmd_err = 0;
 
-	is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
-		MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
+	if (!i2c->dma_mode) {
+		is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
+			MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
 
-	if (is_last_cmd || i2c->cmd_err)
-		complete(&i2c->cmd_complete);
+		if (!i2c->dma_mode && (is_last_cmd || i2c->cmd_err))
+			complete(&i2c->cmd_complete);
+	}
 
 	writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR);
 
@@ -328,21 +505,41 @@  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;
 
 	res_size = resource_size(res);
@@ -353,15 +550,32 @@  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->dev = dev;
+
+	/*
+	 * The MXS I2C DMA mode is prefered and enabled by default.
+	 * The PIO mode is still supported, but should be used only
+	 * for debuging purposes etc.
+	 */
+	i2c->dma_mode = 1;
+
+	/* Setup the DMA */
+	if (i2c->dma_mode) {
+		i2c->dmares = dmares;
+		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 */
@@ -394,6 +608,9 @@  static int __devexit mxs_i2c_remove(struct platform_device *pdev)
 	if (ret)
 		return -EBUSY;
 
+	if (i2c->dmach)
+		dma_release_channel(i2c->dmach);
+
 	writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
 	writel(MXS_I2C_CTRL0_SFTRST, i2c->regs + MXS_I2C_CTRL0_SET);