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 linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
