> > @@ -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!
Attachment:
signature.asc
Description: Digital signature
