Re: [PATCH v4 08/16] mtd: onenand: omap2: Simplify the DMA setup for various paths

[Sebastian Reichel <sre@kernel.org>]

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

Hi,

On Sat, Nov 11, 2017 at 10:21:53PM +0100, Ladislav Michl wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> We have 4 functions containing almost identical DMA setup code. Create one
> function which can set up the DMA for both read and write and use this in
> place for the setup code in the driver.
> The new function will use wait_for_completion_io_timeout() and it will
> figure out the best data_type to be used for the transfer instead of
> hardwiring 32 or 16 bit data.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

>  Changes:
>  -v4: new patch
> 
>  drivers/mtd/onenand/omap2.c | 109 ++++++++++++++++++--------------------------
>  1 file changed, 45 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
> index 0e7772e16d75..d22163271dc9 100644
> --- a/drivers/mtd/onenand/omap2.c
> +++ b/drivers/mtd/onenand/omap2.c
> @@ -288,6 +288,33 @@ static inline int omap2_onenand_bufferram_offset(struct mtd_info *mtd, int area)
>  	return 0;
>  }
>  
> +static inline int omap2_onenand_dma_transfer(struct omap2_onenand *c,
> +					     dma_addr_t src, dma_addr_t dst,
> +					     size_t count)
> +{
> +	int data_type = __ffs((src | dst | count));
> +
> +	if (data_type > OMAP_DMA_DATA_TYPE_S32)
> +		data_type = OMAP_DMA_DATA_TYPE_S32;
> +
> +	omap_set_dma_transfer_params(c->dma_channel, data_type,
> +				     count / BIT(data_type), 1, 0, 0, 0);
> +	omap_set_dma_src_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> +				src, 0, 0);
> +	omap_set_dma_dest_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> +				 dst, 0, 0);
> +
> +	reinit_completion(&c->dma_done);
> +	omap_start_dma(c->dma_channel);
> +	if (!wait_for_completion_io_timeout(&c->dma_done,
> +					    msecs_to_jiffies(20))) {
> +		omap_stop_dma(c->dma_channel);
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
>  #if defined(CONFIG_ARCH_OMAP3) || defined(MULTI_OMAP2)
>  
>  static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area,
> @@ -298,10 +325,9 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area,
>  	struct onenand_chip *this = mtd->priv;
>  	dma_addr_t dma_src, dma_dst;
>  	int bram_offset;
> -	unsigned long timeout;
>  	void *buf = (void *)buffer;
>  	size_t xtra;
> -	volatile unsigned *done;
> +	int ret;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
>  	if (bram_offset & 3 || (size_t)buf & 3 || count < 384)
> @@ -338,25 +364,10 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area,
>  		goto out_copy;
>  	}
>  
> -	omap_set_dma_transfer_params(c->dma_channel, OMAP_DMA_DATA_TYPE_S32,
> -				     count >> 2, 1, 0, 0, 0);
> -	omap_set_dma_src_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				dma_src, 0, 0);
> -	omap_set_dma_dest_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				 dma_dst, 0, 0);
> -
> -	reinit_completion(&c->dma_done);
> -	omap_start_dma(c->dma_channel);
> -
> -	timeout = jiffies + msecs_to_jiffies(20);
> -	done = &c->dma_done.done;
> -	while (time_before(jiffies, timeout))
> -		if (*done)
> -			break;
> -
> +	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
>  	dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE);
>  
> -	if (!*done) {
> +	if (ret) {
>  		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
>  		goto out_copy;
>  	}
> @@ -376,9 +387,8 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area,
>  	struct onenand_chip *this = mtd->priv;
>  	dma_addr_t dma_src, dma_dst;
>  	int bram_offset;
> -	unsigned long timeout;
>  	void *buf = (void *)buffer;
> -	volatile unsigned *done;
> +	int ret;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
>  	if (bram_offset & 3 || (size_t)buf & 3 || count < 384)
> @@ -409,25 +419,10 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area,
>  		return -1;
>  	}
>  
> -	omap_set_dma_transfer_params(c->dma_channel, OMAP_DMA_DATA_TYPE_S32,
> -				     count >> 2, 1, 0, 0, 0);
> -	omap_set_dma_src_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				dma_src, 0, 0);
> -	omap_set_dma_dest_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				 dma_dst, 0, 0);
> -
> -	reinit_completion(&c->dma_done);
> -	omap_start_dma(c->dma_channel);
> -
> -	timeout = jiffies + msecs_to_jiffies(20);
> -	done = &c->dma_done.done;
> -	while (time_before(jiffies, timeout))
> -		if (*done)
> -			break;
> -
> +	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
>  	dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE);
>  
> -	if (!*done) {
> +	if (ret) {
>  		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
>  		goto out_copy;
>  	}
> @@ -466,7 +461,7 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area,
>  	struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd);
>  	struct onenand_chip *this = mtd->priv;
>  	dma_addr_t dma_src, dma_dst;
> -	int bram_offset;
> +	int bram_offset, ret;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
>  	/* DMA is not used.  Revisit PM requirements before enabling it. */
> @@ -488,20 +483,13 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area,
>  		return -1;
>  	}
>  
> -	omap_set_dma_transfer_params(c->dma_channel, OMAP_DMA_DATA_TYPE_S32,
> -				     count / 4, 1, 0, 0, 0);
> -	omap_set_dma_src_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				dma_src, 0, 0);
> -	omap_set_dma_dest_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				 dma_dst, 0, 0);
> -
> -	reinit_completion(&c->dma_done);
> -	omap_start_dma(c->dma_channel);
> -	wait_for_completion(&c->dma_done);
> -
> +	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
>  	dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE);
>  
> -	return 0;
> +	if (ret)
> +		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
> +
> +	return ret;
>  }
>  
>  static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area,
> @@ -511,7 +499,7 @@ static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area,
>  	struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd);
>  	struct onenand_chip *this = mtd->priv;
>  	dma_addr_t dma_src, dma_dst;
> -	int bram_offset;
> +	int bram_offset, ret;
>  
>  	bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset;
>  	/* DMA is not used.  Revisit PM requirements before enabling it. */
> @@ -533,20 +521,13 @@ static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area,
>  		return -1;
>  	}
>  
> -	omap_set_dma_transfer_params(c->dma_channel, OMAP_DMA_DATA_TYPE_S16,
> -				     count / 2, 1, 0, 0, 0);
> -	omap_set_dma_src_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				dma_src, 0, 0);
> -	omap_set_dma_dest_params(c->dma_channel, 0, OMAP_DMA_AMODE_POST_INC,
> -				 dma_dst, 0, 0);
> -
> -	reinit_completion(&c->dma_done);
> -	omap_start_dma(c->dma_channel);
> -	wait_for_completion(&c->dma_done);
> -
> +	ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count);
>  	dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE);
>  
> -	return 0;
> +	if (ret)
> +		dev_err(&c->pdev->dev, "timeout waiting for DMA\n");
> +
> +	return ret;
>  }
>  
>  #else
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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