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

Sebastian Reichel
2017-11-15T15:05:29Z

[-- 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 --]