Re: [PATCH mmc/next v2 3/4] mmc: renesas-sdhi: add support for R-Car Gen3 SDHI DMAC

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]

On Fri, Jun 16, 2017 at 11:58:53AM +0200, Simon Horman wrote:
> Add a new variant of the SDHI driver to support R-Car Gen3 with DMA via
> on-chip bus mastering.  Since the DMAC is in a part of the SDHI module it
> is not suitable to be used via DMA Engine.
> 
> Clearing of DM_CM_INFO1 after DMA thanks to Dirk Behme
> 
> Cc: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>

Your Sob is missing!

> + * - Since this SDHI DMAC register set has actual 32-bit and "bus_shift" is 2,
> + *   this driver cannot use original sd_ctrl_{write,read}32 functions.

Easier to understand, I'd think, would be:

Since the SDHI DMAC register set has not 16 but 32-bit width, we need a
custom accessor.

> +	ret = dma_map_sg(&host->pdev->dev, sg, host->sg_len, dir);
> +	if (ret < 0) {
> +		dev_err(&host->pdev->dev, "%s: dma_map_sg failed\n", __func__);

__func__?

> +	/* start the DMAC */

Not very useful comment in my book, but no strong opinion.

> +	renesas_sdhi_internal_dmac_dm_write(host, DM_CM_DTRAN_CTRL,
> +					    DTRAN_CTRL_DM_START);
> +}
> +	dma_unmap_sg(&host->pdev->dev, host->sg_ptr, host->sg_len, dir);
> +
> +	spin_unlock_irq(&host->lock);
> +
> +	spin_lock_irq(&host->lock);

Just keep the lock?

Rest looks good so far...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]