> > @@ -33,6 +35,9 @@ > > #include <linux/io.h> > > #include <linux/slab.h> > > #include <linux/of_device.h> > > +#include <linux/dmaengine.h> > > +#include <linux/dma-mapping.h> > > +#include <linux/sh_dma.h> > > I would have tried to keep the headers alphabetically sorted, if they had > been > sorted in the first place :-)
Sure, I can do that as a seperate patch.
> > +static void sh_mobile_i2c_cleanup_dma(struct sh_mobile_i2c_data *pd)
> > +{
> > + if (pd->dma_direction == DMA_FROM_DEVICE)
> > + dmaengine_terminate_all(pd->dma_rx);
> > + if (pd->dma_direction == DMA_TO_DEVICE)
>
> else ?
Yes, why not.
> > + pd->buf_mapped = true;
>
> Can't you use dma_direction != DMA_NONE to detect whether the buffer has been
> mapped, instead of adding a new field to struct sh_mobile_i2c_data ? You
> could
> then simplify sh_mobile_i2c_cleanup_dma() by returning immediately at the
> beginning of the function if dma_direction == DMA_NONE.
I thought having this explicit might be more readable. Will try your
idea though and check.
>
> Extra blank line here.
Yes.
Thanks for the review!
signature.asc
Description: Digital signature
