Re: [PATCH] mmc: host: renesas_internal_dmac: add pre_req and post_req support

From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: ulf.hansson@linaro.org, linux-mmc@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH] mmc: host: renesas_internal_dmac: add pre_req and post_req support
Date: Mon, 14 Dec 2020 16:50:01 +0100	[thread overview]
Message-ID: <20201214155001.GA950@ninjato> (raw)
In-Reply-To: <1607087853-6570-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

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

Hi Shimoda-san,

On Fri, Dec 04, 2020 at 10:17:33PM +0900, Yoshihiro Shimoda wrote:
> Add pre_req and post_req support to improve performance.
> 
> Inspired by a patch in the BSP by Masaharu Hayakawa.

Thank you for upporting this!

>  /*
>   * Specification of this driver:
>   * - host->chan_{rx,tx} will be used as a flag of enabling/disabling the dma
> @@ -172,6 +178,47 @@ renesas_sdhi_internal_dmac_dataend_dma(struct tmio_mmc_host *host) {
>  	tasklet_schedule(&priv->dma_priv.dma_complete);
>  }
>  
> +/* Should not use host->sg_ptr/sg_len in the following function */

Maybe a short explanation why we shouldn't use the functions?

> +static void
> +renesas_sdhi_internal_dmac_unmap(struct tmio_mmc_host *host,
> +				 struct mmc_data *data,
> +				 enum renesas_sdhi_dma_cookie cookie,
> +				 bool expected_unmatch)

Can we maybe skip "expected_unmatch"? It is always true for
COOKIE_UNMAPPED and always false for the COOKIE_*MAPPED values, or?

> +{
> +	bool unmap = expected_unmatch ? (data->host_cookie != cookie) :
> +					(data->host_cookie == cookie);

Then, we could do:
 +	bool unmap = cookie == COOKIE_UNMAPPED ? (data->host_cookie != cookie) :
 +					(data->host_cookie == cookie);

> +
> +	if (unmap) {
> +		dma_unmap_sg(&host->pdev->dev, data->sg, data->sg_len,
> +			     mmc_get_dma_dir(data));
> +		data->host_cookie = COOKIE_UNMAPPED;
> +	}

Is it maybe worth a warning if the expected condition was not found?

Rest looks good!

All the best,

   Wolfram

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