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 <[email protected]> > > Cc: Detlev Zundel <[email protected]> > > Cc: Fabio Estevam <[email protected]> > > Cc: Stefano Babic <[email protected]> > > Cc: Wolfgang Denk <[email protected]> > > Cc: Wolfram Sang <[email protected]> > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
