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@xxxxxxxxxxxxxxx>
> > Cc: Detlev Zundel <dzu@xxxxxxx>
> > Cc: Fabio Estevam <festevam@xxxxxxxxx>
> > Cc: Stefano Babic <sbabic@xxxxxxx>
> > Cc: Wolfgang Denk <wd@xxxxxxx>
> > Cc: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
