Hi Phil, Thanks for the comments. On Mon, 28 Apr 2014, Phil Edworthy wrote: > Hi Guennadi, > > On 26 April 2014 13:06, Guennadi wrote: > > Subject: [PATCH v2] mmc: add a driver for the Renesas usdhi6rol0 SD/SDIO > > host controller > > > > This patch adds a driver for the Renesas usdhi6rol0 SD/SDIO host controller > > in both PIO and DMA modes. > > ... > > +static void usdhi6_dma_stop_unmap(struct usdhi6_host *host) > > +{ > > + struct mmc_data *data = host->mrq->data; > > + > > + if (!host->dma_active) > > + return; > > + > > + usdhi6_write(host, USDHI6_CC_EXT_MODE, 0); > > + host->dma_active = false; > > + > > + if (data->flags & MMC_DATA_READ) > > + /* TODO: do we have to synchronise? */ > > + dma_unmap_sg(host->chan_rx->device->dev, data->sg, > > + data->sg_len, DMA_FROM_DEVICE); > Yes, you have to sync, so you can remove this TODO comment. Why do you think so? If we really do, we'd have to add it. And I did think synchronisation was needed, that's why I posted this patch series: http://thread.gmane.org/gmane.linux.kernel.mmc/24969 but it never got applied, even though it got 2 acks. And actually now I think that synchronisation isn't needed. I think dma_map_sg() / dma_unmap_sg() do that for us already. E.g. dma_map_sg_attrs() arm_dma_map_page() __dma_page_cpu_to_dev() dmac_map_area() cpu_cache.dma_map_area() v7_dma_inv_range() v7_dma_clean_range() Similar for unmapping. So, I think it would be best to remove the comment without adding synchronisation. Let me do this: I'll prepare a separate patch for adding dma syncs, but I won't submit it for now, or I can just provide it for reference. > ... > > +static int usdhi6_probe(struct platform_device *pdev) > > +{ > ... > > + host = mmc_priv(mmc); > > + host->mmc = mmc; > > + host->wait = USDHI6_WAIT_FOR_REQUEST; > > + host->timeout = msecs_to_jiffies(1000); > In all places you use host->timeout, the code uses host->timeout * 4. > Wouldn't it better to just set it here to 4 seconds? ok, I'll do that for v3. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html