| Message ID | 20171109091529.x3pqqvbiywj5aulo@lenoch |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | OMAP2+ OneNAND driver update | expand |
On 09/11/17 11:15, Ladislav Michl wrote: > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > Therefore replace open coded timeout waiting for completion in OMAP3 > functions with API call and merge those two implementations. > > Note that currently the is no single DMA user, so this change is basically > no-op. Those interested will have to find DMA configuration in the > linux-omap.git history. > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> I'll be happy if there are no OMAP2 vs OMAP3 differences in code but we need to be absolutely sure that we don't break DMA functionality if it ever worked. Can we somehow test OneNAND DMA on OMAP2 and OMAP3? Tony? > --- > Changes: > -v3: new patch > > drivers/mtd/onenand/omap2.c | 165 +++----------------------------------------- > 1 file changed, 10 insertions(+), 155 deletions(-) > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > index 883993bbe40b..5760e40be008 100644 > --- a/drivers/mtd/onenand/omap2.c > +++ b/drivers/mtd/onenand/omap2.c > @@ -289,9 +289,7 @@ static inline int omap2_onenand_bufferram_offset(struct mtd_info *mtd, int area) > return 0; > } > > -#if defined(CONFIG_ARCH_OMAP3) || defined(MULTI_OMAP2) > - > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > +static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > unsigned char *buffer, int offset, > size_t count) > { > @@ -299,10 +297,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; > + unsigned long t; > void *buf = (void *)buffer; > size_t xtra; > - volatile unsigned *done; > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > @@ -349,15 +346,11 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > 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; > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > > - if (!*done) { > + if (!t) { > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > goto out_copy; > } > @@ -369,7 +362,7 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > return 0; > } > > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > +static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > const unsigned char *buffer, > int offset, size_t count) > { > @@ -377,9 +370,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; > + unsigned long t; > void *buf = (void *)buffer; > - volatile unsigned *done; > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > @@ -420,15 +412,11 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > 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; > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > > - if (!*done) { > + if (!t) { > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > goto out_copy; > } > @@ -440,134 +428,6 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > return 0; > } > > -#else > - > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - return -ENOSYS; > -} > - > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - return -ENOSYS; > -} > - > -#endif > - > -#if defined(CONFIG_ARCH_OMAP2) || defined(MULTI_OMAP2) > - > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - 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; > - > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > - /* DMA is not used. Revisit PM requirements before enabling it. */ > - if (1 || (c->dma_channel < 0) || > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > - memcpy(buffer, (__force void *)(this->base + bram_offset), > - count); > - return 0; > - } > - > - dma_src = c->phys_base + bram_offset; > - dma_dst = dma_map_single(&c->pdev->dev, buffer, count, > - DMA_FROM_DEVICE); > - if (dma_mapping_error(&c->pdev->dev, dma_dst)) { > - dev_err(&c->pdev->dev, > - "Couldn't DMA map a %d byte buffer\n", > - count); > - 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); > - > - dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > - > - return 0; > -} > - > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - 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; > - > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > - /* DMA is not used. Revisit PM requirements before enabling it. */ > - if (1 || (c->dma_channel < 0) || > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > - memcpy((__force void *)(this->base + bram_offset), buffer, > - count); > - return 0; > - } > - > - dma_src = dma_map_single(&c->pdev->dev, (void *) buffer, count, > - DMA_TO_DEVICE); > - dma_dst = c->phys_base + bram_offset; > - if (dma_mapping_error(&c->pdev->dev, dma_src)) { > - dev_err(&c->pdev->dev, > - "Couldn't DMA map a %d byte buffer\n", > - count); > - 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); > - > - dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > - > - return 0; > -} > - > -#else > - > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - return -ENOSYS; > -} > - > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - return -ENOSYS; > -} > - > -#endif > - > static struct platform_driver omap2_onenand_driver; > > static void omap2_onenand_shutdown(struct platform_device *pdev) > @@ -691,13 +551,8 @@ static int omap2_onenand_probe(struct platform_device *pdev) > this = &c->onenand; > if (c->dma_channel >= 0) { > this->wait = omap2_onenand_wait; > - if (c->flags & ONENAND_IN_OMAP34XX) { > - this->read_bufferram = omap3_onenand_read_bufferram; > - this->write_bufferram = omap3_onenand_write_bufferram; > - } else { > - this->read_bufferram = omap2_onenand_read_bufferram; > - this->write_bufferram = omap2_onenand_write_bufferram; > - } > + this->read_bufferram = omap2_onenand_read_bufferram; > + this->write_bufferram = omap2_onenand_write_bufferram; > } > > if ((r = onenand_scan(&c->mtd, 1)) < 0) >
Hi, On 2017-11-09 11:15, Ladislav Michl wrote: > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > Therefore replace open coded timeout waiting for completion in OMAP3 > functions with API call and merge those two implementations. > > Note that currently the is no single DMA user, so this change is basically > no-op. Those interested will have to find DMA configuration in the > linux-omap.git history. fwiw I have two patches for some time to convert the omap2 onenand driver to DMAengine: https://github.com/omap-audio/linux-audio/commits/peter/linux-next-wip/drivers/mtd/onenand/omap2.c I can wait for your series and rebase mine or if you have time can test them and see if they can be included in your update series. - Péter > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > --- > Changes: > -v3: new patch > > drivers/mtd/onenand/omap2.c | 165 +++----------------------------------------- > 1 file changed, 10 insertions(+), 155 deletions(-) > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > index 883993bbe40b..5760e40be008 100644 > --- a/drivers/mtd/onenand/omap2.c > +++ b/drivers/mtd/onenand/omap2.c > @@ -289,9 +289,7 @@ static inline int omap2_onenand_bufferram_offset(struct mtd_info *mtd, int area) > return 0; > } > > -#if defined(CONFIG_ARCH_OMAP3) || defined(MULTI_OMAP2) > - > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > +static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > unsigned char *buffer, int offset, > size_t count) > { > @@ -299,10 +297,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; > + unsigned long t; > void *buf = (void *)buffer; > size_t xtra; > - volatile unsigned *done; > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > @@ -349,15 +346,11 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > 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; > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > > - if (!*done) { > + if (!t) { > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > goto out_copy; > } > @@ -369,7 +362,7 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > return 0; > } > > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > +static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > const unsigned char *buffer, > int offset, size_t count) > { > @@ -377,9 +370,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; > + unsigned long t; > void *buf = (void *)buffer; > - volatile unsigned *done; > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > @@ -420,15 +412,11 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > 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; > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > > - if (!*done) { > + if (!t) { > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > goto out_copy; > } > @@ -440,134 +428,6 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > return 0; > } > > -#else > - > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - return -ENOSYS; > -} > - > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - return -ENOSYS; > -} > - > -#endif > - > -#if defined(CONFIG_ARCH_OMAP2) || defined(MULTI_OMAP2) > - > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - 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; > - > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > - /* DMA is not used. Revisit PM requirements before enabling it. */ > - if (1 || (c->dma_channel < 0) || > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > - memcpy(buffer, (__force void *)(this->base + bram_offset), > - count); > - return 0; > - } > - > - dma_src = c->phys_base + bram_offset; > - dma_dst = dma_map_single(&c->pdev->dev, buffer, count, > - DMA_FROM_DEVICE); > - if (dma_mapping_error(&c->pdev->dev, dma_dst)) { > - dev_err(&c->pdev->dev, > - "Couldn't DMA map a %d byte buffer\n", > - count); > - 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); > - > - dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > - > - return 0; > -} > - > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - 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; > - > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > - /* DMA is not used. Revisit PM requirements before enabling it. */ > - if (1 || (c->dma_channel < 0) || > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > - memcpy((__force void *)(this->base + bram_offset), buffer, > - count); > - return 0; > - } > - > - dma_src = dma_map_single(&c->pdev->dev, (void *) buffer, count, > - DMA_TO_DEVICE); > - dma_dst = c->phys_base + bram_offset; > - if (dma_mapping_error(&c->pdev->dev, dma_src)) { > - dev_err(&c->pdev->dev, > - "Couldn't DMA map a %d byte buffer\n", > - count); > - 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); > - > - dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > - > - return 0; > -} > - > -#else > - > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > - unsigned char *buffer, int offset, > - size_t count) > -{ > - return -ENOSYS; > -} > - > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > - const unsigned char *buffer, > - int offset, size_t count) > -{ > - return -ENOSYS; > -} > - > -#endif > - > static struct platform_driver omap2_onenand_driver; > > static void omap2_onenand_shutdown(struct platform_device *pdev) > @@ -691,13 +551,8 @@ static int omap2_onenand_probe(struct platform_device *pdev) > this = &c->onenand; > if (c->dma_channel >= 0) { > this->wait = omap2_onenand_wait; > - if (c->flags & ONENAND_IN_OMAP34XX) { > - this->read_bufferram = omap3_onenand_read_bufferram; > - this->write_bufferram = omap3_onenand_write_bufferram; > - } else { > - this->read_bufferram = omap2_onenand_read_bufferram; > - this->write_bufferram = omap2_onenand_write_bufferram; > - } > + this->read_bufferram = omap2_onenand_read_bufferram; > + this->write_bufferram = omap2_onenand_write_bufferram; > } > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Fri, Nov 10, 2017 at 10:21:51AM +0200, Roger Quadros wrote: > On 09/11/17 11:15, Ladislav Michl wrote: > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > Therefore replace open coded timeout waiting for completion in OMAP3 > > functions with API call and merge those two implementations. > > > > Note that currently the is no single DMA user, so this change is basically > > no-op. Those interested will have to find DMA configuration in the > > linux-omap.git history. > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > I'll be happy if there are no OMAP2 vs OMAP3 differences in code but we need to > be absolutely sure that we don't break DMA functionality if it ever worked. It probably worked at least for OMAP3 as it was enabled in legacy kernels. > Can we somehow test OneNAND DMA on OMAP2 and OMAP3? > Tony? I'll prepare next version patch serie for easier testing. Of course I would prefer to be able to test changes myself, but I do not have any Nokias :-) ladis
On Fri, Nov 10, 2017 at 10:25:24AM +0200, Peter Ujfalusi wrote: > Hi, > > On 2017-11-09 11:15, Ladislav Michl wrote: > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > Therefore replace open coded timeout waiting for completion in OMAP3 > > functions with API call and merge those two implementations. > > > > Note that currently the is no single DMA user, so this change is basically > > no-op. Those interested will have to find DMA configuration in the > > linux-omap.git history. > > fwiw I have two patches for some time to convert the omap2 onenand > driver to DMAengine: > https://github.com/omap-audio/linux-audio/commits/peter/linux-next-wip/drivers/mtd/onenand/omap2.c That is incredible! I was thinking about it as a next step and you already did all the work. > I can wait for your series and rebase mine or if you have time can test > them and see if they can be included in your update series. I run out of time few days ago already, but this looks so good that it is worth to delay other work. I'll include it in next version of patch serie. Also it will allow us to dump 'dma-channel' and use DMA if gpio pin is configured which was intended logic of original driver. 'dma-channel' was introduced during mechanical conversion to DT and fortunately it is not used yet, so we can safely remove it again. Thanks, ladis > - Péter > > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > --- > > Changes: > > -v3: new patch > > > > drivers/mtd/onenand/omap2.c | 165 +++----------------------------------------- > > 1 file changed, 10 insertions(+), 155 deletions(-) > > > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > > index 883993bbe40b..5760e40be008 100644 > > --- a/drivers/mtd/onenand/omap2.c > > +++ b/drivers/mtd/onenand/omap2.c > > @@ -289,9 +289,7 @@ static inline int omap2_onenand_bufferram_offset(struct mtd_info *mtd, int area) > > return 0; > > } > > > > -#if defined(CONFIG_ARCH_OMAP3) || defined(MULTI_OMAP2) > > - > > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > > +static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > > unsigned char *buffer, int offset, > > size_t count) > > { > > @@ -299,10 +297,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; > > + unsigned long t; > > void *buf = (void *)buffer; > > size_t xtra; > > - volatile unsigned *done; > > > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > > @@ -349,15 +346,11 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > > 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; > > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > > > dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > > > > - if (!*done) { > > + if (!t) { > > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > > goto out_copy; > > } > > @@ -369,7 +362,7 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > > return 0; > > } > > > > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > > +static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > > const unsigned char *buffer, > > int offset, size_t count) > > { > > @@ -377,9 +370,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; > > + unsigned long t; > > void *buf = (void *)buffer; > > - volatile unsigned *done; > > > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > > @@ -420,15 +412,11 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > > 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; > > + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); > > > > dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > > > > - if (!*done) { > > + if (!t) { > > dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > > goto out_copy; > > } > > @@ -440,134 +428,6 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > > return 0; > > } > > > > -#else > > - > > -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, > > - unsigned char *buffer, int offset, > > - size_t count) > > -{ > > - return -ENOSYS; > > -} > > - > > -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, > > - const unsigned char *buffer, > > - int offset, size_t count) > > -{ > > - return -ENOSYS; > > -} > > - > > -#endif > > - > > -#if defined(CONFIG_ARCH_OMAP2) || defined(MULTI_OMAP2) > > - > > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > > - unsigned char *buffer, int offset, > > - size_t count) > > -{ > > - 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; > > - > > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > > - /* DMA is not used. Revisit PM requirements before enabling it. */ > > - if (1 || (c->dma_channel < 0) || > > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > > - memcpy(buffer, (__force void *)(this->base + bram_offset), > > - count); > > - return 0; > > - } > > - > > - dma_src = c->phys_base + bram_offset; > > - dma_dst = dma_map_single(&c->pdev->dev, buffer, count, > > - DMA_FROM_DEVICE); > > - if (dma_mapping_error(&c->pdev->dev, dma_dst)) { > > - dev_err(&c->pdev->dev, > > - "Couldn't DMA map a %d byte buffer\n", > > - count); > > - 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); > > - > > - dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > > - > > - return 0; > > -} > > - > > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > > - const unsigned char *buffer, > > - int offset, size_t count) > > -{ > > - 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; > > - > > - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > > - /* DMA is not used. Revisit PM requirements before enabling it. */ > > - if (1 || (c->dma_channel < 0) || > > - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || > > - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { > > - memcpy((__force void *)(this->base + bram_offset), buffer, > > - count); > > - return 0; > > - } > > - > > - dma_src = dma_map_single(&c->pdev->dev, (void *) buffer, count, > > - DMA_TO_DEVICE); > > - dma_dst = c->phys_base + bram_offset; > > - if (dma_mapping_error(&c->pdev->dev, dma_src)) { > > - dev_err(&c->pdev->dev, > > - "Couldn't DMA map a %d byte buffer\n", > > - count); > > - 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); > > - > > - dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); > > - > > - return 0; > > -} > > - > > -#else > > - > > -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > > - unsigned char *buffer, int offset, > > - size_t count) > > -{ > > - return -ENOSYS; > > -} > > - > > -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, > > - const unsigned char *buffer, > > - int offset, size_t count) > > -{ > > - return -ENOSYS; > > -} > > - > > -#endif > > - > > static struct platform_driver omap2_onenand_driver; > > > > static void omap2_onenand_shutdown(struct platform_device *pdev) > > @@ -691,13 +551,8 @@ static int omap2_onenand_probe(struct platform_device *pdev) > > this = &c->onenand; > > if (c->dma_channel >= 0) { > > this->wait = omap2_onenand_wait; > > - if (c->flags & ONENAND_IN_OMAP34XX) { > > - this->read_bufferram = omap3_onenand_read_bufferram; > > - this->write_bufferram = omap3_onenand_write_bufferram; > > - } else { > > - this->read_bufferram = omap2_onenand_read_bufferram; > > - this->write_bufferram = omap2_onenand_write_bufferram; > > - } > > + this->read_bufferram = omap2_onenand_read_bufferram; > > + this->write_bufferram = omap2_onenand_write_bufferram; > > } > > > > if ((r = onenand_scan(&c->mtd, 1)) < 0) > > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Ladislav Michl <ladis@linux-mips.org> [171110 10:06]: > On Fri, Nov 10, 2017 at 10:25:24AM +0200, Peter Ujfalusi wrote: > > Hi, > > > > On 2017-11-09 11:15, Ladislav Michl wrote: > > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > > Therefore replace open coded timeout waiting for completion in OMAP3 > > > functions with API call and merge those two implementations. > > > > > > Note that currently the is no single DMA user, so this change is basically > > > no-op. Those interested will have to find DMA configuration in the > > > linux-omap.git history. > > > > fwiw I have two patches for some time to convert the omap2 onenand > > driver to DMAengine: > > https://github.com/omap-audio/linux-audio/commits/peter/linux-next-wip/drivers/mtd/onenand/omap2.c > > That is incredible! I was thinking about it as a next step and you already > did all the work. > > > I can wait for your series and rebase mine or if you have time can test > > them and see if they can be included in your update series. Yeah that's great, one step closer to removing the legacy dma support! > I run out of time few days ago already, but this looks so good that it is > worth to delay other work. I'll include it in next version of patch serie. > > Also it will allow us to dump 'dma-channel' and use DMA if gpio pin > is configured which was intended logic of original driver. 'dma-channel' > was introduced during mechanical conversion to DT and fortunately it is > not used yet, so we can safely remove it again. FYI, the gpio pin for onenand should not be in gpio mode. It should be used as external dma request line to automatically trigger new transfers like we do for tusb6010 dma. But of course it's possible that onenand has other issues too preventing the dma usage. Regards, Tony
* Ladislav Michl <ladis@linux-mips.org> [171110 09:53]: > On Fri, Nov 10, 2017 at 10:21:51AM +0200, Roger Quadros wrote: > > On 09/11/17 11:15, Ladislav Michl wrote: > > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > > Therefore replace open coded timeout waiting for completion in OMAP3 > > > functions with API call and merge those two implementations. > > > > > > Note that currently the is no single DMA user, so this change is basically > > > no-op. Those interested will have to find DMA configuration in the > > > linux-omap.git history. > > > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > > > I'll be happy if there are no OMAP2 vs OMAP3 differences in code but we need to > > be absolutely sure that we don't break DMA functionality if it ever worked. > > It probably worked at least for OMAP3 as it was enabled in legacy kernels. > > > Can we somehow test OneNAND DMA on OMAP2 and OMAP3? > > Tony? > > I'll prepare next version patch serie for easier testing. Of course I would > prefer to be able to test changes myself, but I do not have any Nokias :-) I tested Peter's dma changes yesterday and they worked. Yeah if you want me to test something a complete series or a git branch makes it easier for me to follow what all is needed. Regards, Tony
On Fri, Nov 10, 2017 at 07:26:10AM -0800, Tony Lindgren wrote: > * Ladislav Michl <ladis@linux-mips.org> [171110 09:53]: > > On Fri, Nov 10, 2017 at 10:21:51AM +0200, Roger Quadros wrote: > > > On 09/11/17 11:15, Ladislav Michl wrote: > > > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > > > Therefore replace open coded timeout waiting for completion in OMAP3 > > > > functions with API call and merge those two implementations. > > > > > > > > Note that currently the is no single DMA user, so this change is basically > > > > no-op. Those interested will have to find DMA configuration in the > > > > linux-omap.git history. > > > > > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > > > > > I'll be happy if there are no OMAP2 vs OMAP3 differences in code but we need to > > > be absolutely sure that we don't break DMA functionality if it ever worked. > > > > It probably worked at least for OMAP3 as it was enabled in legacy kernels. > > > > > Can we somehow test OneNAND DMA on OMAP2 and OMAP3? > > > Tony? > > > > I'll prepare next version patch serie for easier testing. Of course I would > > prefer to be able to test changes myself, but I do not have any Nokias :-) > > I tested Peter's dma changes yesterday and they worked. Yeah if you want > me to test something a complete series or a git branch makes it easier > for me to follow what all is needed. How did you configure DMA? Is there any additional patch? So far I prepared complete patchset including Peter's changes, but I might be good idea to start with something what's working for you. Thanks, ladis
On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > FYI, the gpio pin for onenand should not be in gpio mode. It should > be used as external dma request line to automatically trigger new > transfers like we do for tusb6010 dma. But of course it's possible > that onenand has other issues too preventing the dma usage. Hmm, that's probably what comment in omap3-n900.dts is saying: "sys_ndmareq1 could be used by the driver, not as gpio65 though" However board-rx51 (is that n900?) is using it as gpio pin. Also omap2420-n8x0-common.dtsi comment says: "gpio-irq for dma: 26" and it matches board-n8x0.c platform data configuration. But I still missing pinmux configuration as we need to add it into DT as well. I guess this was originally done in bootloader. Any hint where can I get this info from? Thanks, ladis
* Ladislav Michl <ladis@linux-mips.org> [171110 18:21]: > On Fri, Nov 10, 2017 at 07:26:10AM -0800, Tony Lindgren wrote: > > * Ladislav Michl <ladis@linux-mips.org> [171110 09:53]: > > > On Fri, Nov 10, 2017 at 10:21:51AM +0200, Roger Quadros wrote: > > > > On 09/11/17 11:15, Ladislav Michl wrote: > > > > > Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 > > > > > OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into > > > > > OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. > > > > > Therefore replace open coded timeout waiting for completion in OMAP3 > > > > > functions with API call and merge those two implementations. > > > > > > > > > > Note that currently the is no single DMA user, so this change is basically > > > > > no-op. Those interested will have to find DMA configuration in the > > > > > linux-omap.git history. > > > > > > > > > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org> > > > > > > > > I'll be happy if there are no OMAP2 vs OMAP3 differences in code but we need to > > > > be absolutely sure that we don't break DMA functionality if it ever worked. > > > > > > It probably worked at least for OMAP3 as it was enabled in legacy kernels. > > > > > > > Can we somehow test OneNAND DMA on OMAP2 and OMAP3? > > > > Tony? > > > > > > I'll prepare next version patch serie for easier testing. Of course I would > > > prefer to be able to test changes myself, but I do not have any Nokias :-) > > > > I tested Peter's dma changes yesterday and they worked. Yeah if you want > > me to test something a complete series or a git branch makes it easier > > for me to follow what all is needed. > > How did you configure DMA? Is there any additional patch? > So far I prepared complete patchset including Peter's changes, but I might > be good idea to start with something what's working for you. I just made sure things keep working with Peter's changes, no additional patches. So the dma is barely used at all. Regards, Tony
* Ladislav Michl <ladis@linux-mips.org> [171110 18:28]: > On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > > FYI, the gpio pin for onenand should not be in gpio mode. It should > > be used as external dma request line to automatically trigger new > > transfers like we do for tusb6010 dma. But of course it's possible > > that onenand has other issues too preventing the dma usage. > > Hmm, that's probably what comment in omap3-n900.dts is saying: > "sys_ndmareq1 could be used by the driver, not as gpio65 though" > However board-rx51 (is that n900?) is using it as gpio pin. > Also omap2420-n8x0-common.dtsi comment says: "gpio-irq for dma: 26" > and it matches board-n8x0.c platform data configuration. > But I still missing pinmux configuration as we need to add it into > DT as well. I guess this was originally done in bootloader. Any hint > where can I get this info from? Well I have a utility called padconftodts that uses the old kernel pinmux data :) I'll try to update a bit and push out over next few days. It can read the /sys/kernel/debug/pinctrl data and convert it to a dts file with some nice comments. So I should be able to also dump out the pins for n8x0 and n900. Regards, Tony
On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > FYI, the gpio pin for onenand should not be in gpio mode. It should > be used as external dma request line to automatically trigger new > transfers like we do for tusb6010 dma. But of course it's possible > that onenand has other issues too preventing the dma usage. Now, after reading dma and interrupt related code again, I still do not see how the driver could be using hardware synchronized transfer. DMA seems to be used as a memcpy and gpio pin a ready/busy. Care to elaborate a bit more? Thank you, ladis
On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote: > I just made sure things keep working with Peter's changes, no additional > patches. So the dma is barely used at all. It seemed suspicious to me looking here: https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312 as wait_for_completion_timeout is returning time remaining and zero on timeout :-) Fixed in next version and also changed to wait_for_completion_io_timeout to get proper sched stats. Btw, I think we should stop DMA on timeout as we are unmapping DMA region later and it might not behave nicely if we do so while DMA is eventually running (that's fixed too). I made Peter's patches part of next version and enabled DMA unconditionally. It works nicely on: OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x) OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk) ... omap-gpmc 6e000000.gpmc: GPMC revision 5.0 gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58) OneNAND version = 0x0031 Scanning device for bad blocks OneNAND eraseblock 597 is an initial bad block OneNAND eraseblock 1159 is an initial bad block OneNAND eraseblock 2812 is an initial bad block omap2-onenand 30000000.onenand: optimized timings for 83 MHz 2 ofpart partitions found on MTD device 30000000.onenand Creating 2 MTD partitions on "30000000.onenand": 0x000000000000-0x000000080000 : "SPL" 0x000000080000-0x000020000000 : "UBI" ... Based on this experiment, I'll delay v4 a bit and leave DMA enabled by default. We are brave enough and want some testing, right? It would be nice if someone could provide some benchmarks using PIO and DMA mode. I do not expect any dramatic change, but kernel latencies should decrease when doing flash I/O. Based on above I also think R/B pin should be in ordinary GPIO mode. Perhaps using external DMA request is somehow posible, but it is not clear to me how after yet another reading of SDMA, GPMC and OneNAND documentation, so I'll happily leave it for others. Btw, using R/B pin would elimitate latencies caused by busy looping while waiting for onenand command to complete. Nice todo for someone with a lot of spare time :-) Best regards, ladis
On 2017-11-10 17:24, Tony Lindgren wrote: >> I run out of time few days ago already, but this looks so good that it is >> worth to delay other work. I'll include it in next version of patch serie. >> >> Also it will allow us to dump 'dma-channel' and use DMA if gpio pin >> is configured which was intended logic of original driver. 'dma-channel' >> was introduced during mechanical conversion to DT and fortunately it is >> not used yet, so we can safely remove it again. > > FYI, the gpio pin for onenand should not be in gpio mode. It should > be used as external dma request line to automatically trigger new > transfers like we do for tusb6010 dma. But of course it's possible > that onenand has other issues too preventing the dma usage. My conversion to DMAengine is drop in replacement of the existing implementation: memcpy w/o hardware synchronization event. But I think it should be possible to use HW sync (slave DMA) with the src/dst_port_window in a similar way we do with the tusb6010. But that can be done in a followup series, but what to do in case of old DT where the dmas/dma-names properties are no there? Hmm, extending the dma_slave_map in mach-omap1/2/dma.c might work just fine. Having said that, there might have been a reason why the original implementation was not using DMA request to trigger the memcpy... The legacy omap-dma API would have allowed that as you kind of open code things with much flexibility. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Mon, Nov 13, 2017 at 10:22:12AM +0200, Peter Ujfalusi wrote: > > On 2017-11-10 17:24, Tony Lindgren wrote: > > FYI, the gpio pin for onenand should not be in gpio mode. It should > > be used as external dma request line to automatically trigger new > > transfers like we do for tusb6010 dma. But of course it's possible > > that onenand has other issues too preventing the dma usage. > > My conversion to DMAengine is drop in replacement of the existing > implementation: memcpy w/o hardware synchronization event. > > But I think it should be possible to use HW sync (slave DMA) with the > src/dst_port_window in a similar way we do with the tusb6010. How do you want to synchronize it from OneNAND side? > But that can be done in a followup series, but what to do in case of old > DT where the dmas/dma-names properties are no there? These will not work anyway as they do not have compatible property. Also note, that DMA is currently not used, yet nobody complained. > Hmm, extending the dma_slave_map in mach-omap1/2/dma.c might work just fine. > > Having said that, there might have been a reason why the original > implementation was not using DMA request to trigger the memcpy... The > legacy omap-dma API would have allowed that as you kind of open code > things with much flexibility. That's mainly problem of OneNAND driver itself, not oma-dma. But do we really want to invest more time to this obsolete technology? Of course, I would love to see my 10+ years old boards running faster, but it seems unrealistic to me to get enough manpower to finish this task. ladis
On 2017-11-13 14:15, Ladislav Michl wrote: > On Mon, Nov 13, 2017 at 10:22:12AM +0200, Peter Ujfalusi wrote: >> >> On 2017-11-10 17:24, Tony Lindgren wrote: >>> FYI, the gpio pin for onenand should not be in gpio mode. It should >>> be used as external dma request line to automatically trigger new >>> transfers like we do for tusb6010 dma. But of course it's possible >>> that onenand has other issues too preventing the dma usage. >> >> My conversion to DMAengine is drop in replacement of the existing >> implementation: memcpy w/o hardware synchronization event. >> >> But I think it should be possible to use HW sync (slave DMA) with the >> src/dst_port_window in a similar way we do with the tusb6010. > > How do you want to synchronize it from OneNAND side? _if_ the pin used as GPIO interrupt has ext DMA request mode then we can switch it, in the DT bindings we give the extDMA request as sync event and convert the driver: remove GPIO irq handling and have slave DMA setup with port_window to do kind of memcpy with HW synchronization. It might be possible... It was possible with tusb6010, but that one was already using hw sync with the legacy omap-dma API. >> But that can be done in a followup series, but what to do in case of old >> DT where the dmas/dma-names properties are no there? > > These will not work anyway as they do not have compatible property. > Also note, that DMA is currently not used, yet nobody complained. ;) >> Hmm, extending the dma_slave_map in mach-omap1/2/dma.c might work just fine. >> >> Having said that, there might have been a reason why the original >> implementation was not using DMA request to trigger the memcpy... The >> legacy omap-dma API would have allowed that as you kind of open code >> things with much flexibility. > > That's mainly problem of OneNAND driver itself, not oma-dma. But do we > really want to invest more time to this obsolete technology? > > Of course, I would love to see my 10+ years old boards running faster, > but it seems unrealistic to me to get enough manpower to finish this task. Valid point. I would - for now - only do the DMAengine memcpy conversion with the GPIO irq and investigate the slave DMA support after that. > > ladis > -- > 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 > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Peter Ujfalusi <peter.ujfalusi@ti.com> [171113 14:37]: > On 2017-11-13 14:15, Ladislav Michl wrote: > > Of course, I would love to see my 10+ years old boards running faster, > > but it seems unrealistic to me to get enough manpower to finish this task. > > Valid point. I would - for now - only do the DMAengine memcpy conversion > with the GPIO irq and investigate the slave DMA support after that. Yeah the use of the dmareq line is up to somebody who needs it naturally, just something we've ben speculating for a while :) Regards, Tony
On Mon, Nov 13, 2017 at 04:36:41PM +0200, Peter Ujfalusi wrote: > > > On 2017-11-13 14:15, Ladislav Michl wrote: > > On Mon, Nov 13, 2017 at 10:22:12AM +0200, Peter Ujfalusi wrote: > >> > >> On 2017-11-10 17:24, Tony Lindgren wrote: > >>> FYI, the gpio pin for onenand should not be in gpio mode. It should > >>> be used as external dma request line to automatically trigger new > >>> transfers like we do for tusb6010 dma. But of course it's possible > >>> that onenand has other issues too preventing the dma usage. > >> > >> My conversion to DMAengine is drop in replacement of the existing > >> implementation: memcpy w/o hardware synchronization event. > >> > >> But I think it should be possible to use HW sync (slave DMA) with the > >> src/dst_port_window in a similar way we do with the tusb6010. > > > > How do you want to synchronize it from OneNAND side? > > _if_ the pin used as GPIO interrupt has ext DMA request mode then we can > switch it, in the DT bindings we give the extDMA request as sync event > and convert the driver: remove GPIO irq handling and have slave DMA > setup with port_window to do kind of memcpy with HW synchronization. Well, the pin used as GPIO interrupt has this purpose, according to specs: "The OneNAND INT pin is an output pin function used to notify the Host when a command has been completed. This provides a hardware method of signaling the completion of a program, erase, or load operation". See for example section 3.9 (and later) of specs: https://www.abcelectronique.com/composants/telechargement_datasheet.php?id=1089396&part-number=KFH2G16Q2A-DEBx > It might be possible... It was possible with tusb6010, but that one was > already using hw sync with the legacy omap-dma API. Based on above, it seems there's long and hard way :) > >> But that can be done in a followup series, but what to do in case of old > >> DT where the dmas/dma-names properties are no there? > > > > These will not work anyway as they do not have compatible property. > > Also note, that DMA is currently not used, yet nobody complained. > > ;) > > >> Hmm, extending the dma_slave_map in mach-omap1/2/dma.c might work just fine. > >> > >> Having said that, there might have been a reason why the original > >> implementation was not using DMA request to trigger the memcpy... The > >> legacy omap-dma API would have allowed that as you kind of open code > >> things with much flexibility. > > > > That's mainly problem of OneNAND driver itself, not oma-dma. But do we > > really want to invest more time to this obsolete technology? > > > > Of course, I would love to see my 10+ years old boards running faster, > > but it seems unrealistic to me to get enough manpower to finish this task. > > Valid point. I would - for now - only do the DMAengine memcpy conversion > with the GPIO irq and investigate the slave DMA support after that. Here it would be nice to have the same documentation Nokia had at the time writing support for n8x0 as they clearly used few undocumented features. Please see section 3.9.5 of above mentioned specs. Seems DMA slave should be triggered by RDY not INT pin. Best regards, ladis
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 11/11/2017 02:50 PM, Ladislav Michl wrote: > On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote: >> I just made sure things keep working with Peter's changes, no additional >> patches. So the dma is barely used at all. > > It seemed suspicious to me looking here: > https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312 > as wait_for_completion_timeout is returning time remaining and zero on > timeout :-) aargh, yes, you are right... > Fixed in next version and also changed to wait_for_completion_io_timeout > to get proper sched stats. > Btw, I think we should stop DMA on timeout as we are unmapping DMA region > later and it might not behave nicely if we do so while DMA is eventually > running (that's fixed too). > > I made Peter's patches part of next version and enabled DMA unconditionally. > It works nicely on: Pretty cool! Thanks for doing it! > OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x) > OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk) > ... > omap-gpmc 6e000000.gpmc: GPMC revision 5.0 > gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 > omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode > Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58) > OneNAND version = 0x0031 > Scanning device for bad blocks > OneNAND eraseblock 597 is an initial bad block > OneNAND eraseblock 1159 is an initial bad block > OneNAND eraseblock 2812 is an initial bad block > omap2-onenand 30000000.onenand: optimized timings for 83 MHz > 2 ofpart partitions found on MTD device 30000000.onenand > Creating 2 MTD partitions on "30000000.onenand": > 0x000000000000-0x000000080000 : "SPL" > 0x000000080000-0x000020000000 : "UBI" > ... > > Based on this experiment, I'll delay v4 a bit and leave DMA enabled by > default. We are brave enough and want some testing, right? > It would be nice if someone could provide some benchmarks using PIO > and DMA mode. I do not expect any dramatic change, but kernel latencies > should decrease when doing flash I/O. > > Based on above I also think R/B pin should be in ordinary GPIO mode. > Perhaps using external DMA request is somehow posible, but it is not clear > to me how after yet another reading of SDMA, GPMC and OneNAND documentation, > so I'll happily leave it for others. > > Btw, using R/B pin would elimitate latencies caused by busy looping while > waiting for onenand command to complete. Nice todo for someone with a lot > of spare time :-) > > Best regards, > ladis >
On 11/11/17 14:50, Ladislav Michl wrote: > On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote: >> I just made sure things keep working with Peter's changes, no additional >> patches. So the dma is barely used at all. > > It seemed suspicious to me looking here: > https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312 > as wait_for_completion_timeout is returning time remaining and zero on > timeout :-) > Fixed in next version and also changed to wait_for_completion_io_timeout > to get proper sched stats. > Btw, I think we should stop DMA on timeout as we are unmapping DMA region > later and it might not behave nicely if we do so while DMA is eventually > running (that's fixed too). > > I made Peter's patches part of next version and enabled DMA unconditionally. That's cool, but I'd still hold from making it default in first go. It could have more testing and benchmarking. > It works nicely on: > OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x) > OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk) > ... > omap-gpmc 6e000000.gpmc: GPMC revision 5.0 > gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 > omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode > Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58) > OneNAND version = 0x0031 > Scanning device for bad blocks > OneNAND eraseblock 597 is an initial bad block > OneNAND eraseblock 1159 is an initial bad block > OneNAND eraseblock 2812 is an initial bad block > omap2-onenand 30000000.onenand: optimized timings for 83 MHz > 2 ofpart partitions found on MTD device 30000000.onenand > Creating 2 MTD partitions on "30000000.onenand": > 0x000000000000-0x000000080000 : "SPL" > 0x000000080000-0x000020000000 : "UBI" > ... > > Based on this experiment, I'll delay v4 a bit and leave DMA enabled by > default. We are brave enough and want some testing, right? > It would be nice if someone could provide some benchmarks using PIO > and DMA mode. I do not expect any dramatic change, but kernel latencies > should decrease when doing flash I/O. There should be noticeable difference between both when the CPU is loaded. > > Based on above I also think R/B pin should be in ordinary GPIO mode. > Perhaps using external DMA request is somehow posible, but it is not clear > to me how after yet another reading of SDMA, GPMC and OneNAND documentation, > so I'll happily leave it for others. At least on NAND we use R/B as a plain GPIO that is polled till it shows ready during NAND erase/program cycles. e.g. see nand_wait() in nand/nand_base.c and omap_dev_ready() in nand/omap2.c So ready checking could be done similarly in onenand core? > > Btw, using R/B pin would elimitate latencies caused by busy looping while > waiting for onenand command to complete. Nice todo for someone with a lot > of spare time :-) > > Best regards, > ladis >
On Tue, Nov 14, 2017 at 04:47:46PM +0200, Roger Quadros wrote: > On 11/11/17 14:50, Ladislav Michl wrote: > > On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote: > >> I just made sure things keep working with Peter's changes, no additional > >> patches. So the dma is barely used at all. > > > > It seemed suspicious to me looking here: > > https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312 > > as wait_for_completion_timeout is returning time remaining and zero on > > timeout :-) > > Fixed in next version and also changed to wait_for_completion_io_timeout > > to get proper sched stats. > > Btw, I think we should stop DMA on timeout as we are unmapping DMA region > > later and it might not behave nicely if we do so while DMA is eventually > > running (that's fixed too). > > > > I made Peter's patches part of next version and enabled DMA unconditionally. > > That's cool, but I'd still hold from making it default in first go. > It could have more testing and benchmarking. I do not expect v4 to be the last version of patchset and I enabled it to get as much testing as possible. > > It works nicely on: > > OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x) > > OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk) > > ... > > omap-gpmc 6e000000.gpmc: GPMC revision 5.0 > > gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 > > omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode > > Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58) > > OneNAND version = 0x0031 > > Scanning device for bad blocks > > OneNAND eraseblock 597 is an initial bad block > > OneNAND eraseblock 1159 is an initial bad block > > OneNAND eraseblock 2812 is an initial bad block > > omap2-onenand 30000000.onenand: optimized timings for 83 MHz > > 2 ofpart partitions found on MTD device 30000000.onenand > > Creating 2 MTD partitions on "30000000.onenand": > > 0x000000000000-0x000000080000 : "SPL" > > 0x000000080000-0x000020000000 : "UBI" > > ... > > > > Based on this experiment, I'll delay v4 a bit and leave DMA enabled by > > default. We are brave enough and want some testing, right? > > It would be nice if someone could provide some benchmarks using PIO > > and DMA mode. I do not expect any dramatic change, but kernel latencies > > should decrease when doing flash I/O. > > There should be noticeable difference between both when the CPU is loaded. > > > > > Based on above I also think R/B pin should be in ordinary GPIO mode. > > Perhaps using external DMA request is somehow posible, but it is not clear > > to me how after yet another reading of SDMA, GPMC and OneNAND documentation, > > so I'll happily leave it for others. > > At least on NAND we use R/B as a plain GPIO that is polled till it shows > ready during NAND erase/program cycles. > e.g. see nand_wait() in nand/nand_base.c > and omap_dev_ready() in nand/omap2.c As answered previously, OneNAND R/B pin is indeed used the same (*) way, it is just done at OMAP2+ driver level rather than in OneNAND base driver. (*) it is actually a bit better as R/B is configured as interrupt source and we are just waiting for completion. > So ready checking could be done similarly in onenand core? Yes, but that could wait for another round of driver updates. I'd like to sort GPMC interface first. There's more to be moved to OneNAND base driver (chip frequency probing, latency settings, etc). > > > > Btw, using R/B pin would elimitate latencies caused by busy looping while > > waiting for onenand command to complete. Nice todo for someone with a lot > > of spare time :-) > > > > Best regards, > > ladis > > > > -- > cheers, > -roger > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 13/11/17 17:27, Ladislav Michl wrote: > On Mon, Nov 13, 2017 at 04:36:41PM +0200, Peter Ujfalusi wrote: >> >> >> On 2017-11-13 14:15, Ladislav Michl wrote: >>> On Mon, Nov 13, 2017 at 10:22:12AM +0200, Peter Ujfalusi wrote: >>>> >>>> On 2017-11-10 17:24, Tony Lindgren wrote: >>>>> FYI, the gpio pin for onenand should not be in gpio mode. It should >>>>> be used as external dma request line to automatically trigger new >>>>> transfers like we do for tusb6010 dma. But of course it's possible >>>>> that onenand has other issues too preventing the dma usage. >>>> >>>> My conversion to DMAengine is drop in replacement of the existing >>>> implementation: memcpy w/o hardware synchronization event. >>>> >>>> But I think it should be possible to use HW sync (slave DMA) with the >>>> src/dst_port_window in a similar way we do with the tusb6010. >>> >>> How do you want to synchronize it from OneNAND side? >> >> _if_ the pin used as GPIO interrupt has ext DMA request mode then we can >> switch it, in the DT bindings we give the extDMA request as sync event >> and convert the driver: remove GPIO irq handling and have slave DMA >> setup with port_window to do kind of memcpy with HW synchronization. > > Well, the pin used as GPIO interrupt has this purpose, according to specs: > "The OneNAND INT pin is an output pin function used to notify the Host when > a command has been completed. This provides a hardware method of signaling > the completion of a program, erase, or load operation". > > See for example section 3.9 (and later) of specs: > https://www.abcelectronique.com/composants/telechargement_datasheet.php?id=1089396&part-number=KFH2G16Q2A-DEBx Also see section 7.1 Methods of Determining Interrupt Status. Based on that INT pin is analogous to NAND Read/Busy pin and has nothing to do with DMA. > >> It might be possible... It was possible with tusb6010, but that one was >> already using hw sync with the legacy omap-dma API. > > Based on above, it seems there's long and hard way :) > >>>> But that can be done in a followup series, but what to do in case of old >>>> DT where the dmas/dma-names properties are no there? >>> >>> These will not work anyway as they do not have compatible property. >>> Also note, that DMA is currently not used, yet nobody complained. >> >> ;) >> >>>> Hmm, extending the dma_slave_map in mach-omap1/2/dma.c might work just fine. >>>> >>>> Having said that, there might have been a reason why the original >>>> implementation was not using DMA request to trigger the memcpy... The >>>> legacy omap-dma API would have allowed that as you kind of open code >>>> things with much flexibility. >>> >>> That's mainly problem of OneNAND driver itself, not oma-dma. But do we >>> really want to invest more time to this obsolete technology? >>> >>> Of course, I would love to see my 10+ years old boards running faster, >>> but it seems unrealistic to me to get enough manpower to finish this task. >> >> Valid point. I would - for now - only do the DMAengine memcpy conversion >> with the GPIO irq and investigate the slave DMA support after that. > > Here it would be nice to have the same documentation Nokia had at the time > writing support for n8x0 as they clearly used few undocumented features. > Please see section 3.9.5 of above mentioned specs. Seems DMA slave should > be triggered by RDY not INT pin. Correct. For RDY pin usage See 3.7.3 Handshaking Operation "The handshaking feature allows the host system to simply monitor the RDY signal from the device to determine when the initial word of burst data is ready to be read." > > Best regards, > ladis >
On Tue, Nov 14, 2017 at 05:05:20PM +0200, Roger Quadros wrote: > On 13/11/17 17:27, Ladislav Michl wrote: > > Here it would be nice to have the same documentation Nokia had at the time > > writing support for n8x0 as they clearly used few undocumented features. > > Please see section 3.9.5 of above mentioned specs. Seems DMA slave should > > be triggered by RDY not INT pin. > > Correct. For RDY pin usage See 3.7.3 Handshaking Operation > > "The handshaking feature allows the host system to simply monitor the RDY signal from the device to determine > when the initial word of burst data is ready to be read." To collect more documentation from Samsung, this appnote is also relevant: http://trendol.free.fr/SMDK6410/842440S3C6410_ApplicationNotes_rev10.pdf Section 7. OneNand, especially 7.5.1.3-6.
* Ladislav Michl <ladis@linux-mips.org> [171110 21:41]: > On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > > FYI, the gpio pin for onenand should not be in gpio mode. It should > > be used as external dma request line to automatically trigger new > > transfers like we do for tusb6010 dma. But of course it's possible > > that onenand has other issues too preventing the dma usage. > > Now, after reading dma and interrupt related code again, I still do > not see how the driver could be using hardware synchronized transfer. > DMA seems to be used as a memcpy and gpio pin a ready/busy. > Care to elaborate a bit more? Well take a look at mux options for the onenand connected pins, and if there is one that has a mux option for ext_dmareq or similar, chances are it can be used to retrigger dma transfers. I could be wrong of course, and it could be it won't even work with onenand. Something to look later on for sure if anybody is interested. Regards, Tony
On Tue, Nov 14, 2017 at 01:53:12PM -0800, Tony Lindgren wrote: > * Ladislav Michl <ladis@linux-mips.org> [171110 21:41]: > > On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > > > FYI, the gpio pin for onenand should not be in gpio mode. It should > > > be used as external dma request line to automatically trigger new > > > transfers like we do for tusb6010 dma. But of course it's possible > > > that onenand has other issues too preventing the dma usage. > > > > Now, after reading dma and interrupt related code again, I still do > > not see how the driver could be using hardware synchronized transfer. > > DMA seems to be used as a memcpy and gpio pin a ready/busy. > > Care to elaborate a bit more? > > Well take a look at mux options for the onenand connected pins, > and if there is one that has a mux option for ext_dmareq or similar, > chances are it can be used to retrigger dma transfers. > > I could be wrong of course, and it could be it won't even work with > onenand. As previously discussed, slave DMA should be triggered by RDY pin and it has nothing to do with code already in place. That also means someone should remove misleding comment line from e2c5eb78a3cc "ARM: dts: Fix wrong GPMC size mappings for omaps" ;-) I'll do it later in v5 of "ARM: dts: Nokia: Use R/B pin". > Something to look later on for sure if anybody is interested. While there, we should also benchmark DMA memcpy as OMAP3 version was using 384 bytes as limit to trigger DMA while OMAP2 was using 1024. For smaller chunks, memcpy was used. I decided to leave 384 in place, but I do not have OMAP2 hardware to do any verification. ladis
* Ladislav Michl <ladis@linux-mips.org> [171114 22:34]: > On Tue, Nov 14, 2017 at 01:53:12PM -0800, Tony Lindgren wrote: > > * Ladislav Michl <ladis@linux-mips.org> [171110 21:41]: > > > On Fri, Nov 10, 2017 at 07:24:23AM -0800, Tony Lindgren wrote: > > > > FYI, the gpio pin for onenand should not be in gpio mode. It should > > > > be used as external dma request line to automatically trigger new > > > > transfers like we do for tusb6010 dma. But of course it's possible > > > > that onenand has other issues too preventing the dma usage. > > > > > > Now, after reading dma and interrupt related code again, I still do > > > not see how the driver could be using hardware synchronized transfer. > > > DMA seems to be used as a memcpy and gpio pin a ready/busy. > > > Care to elaborate a bit more? > > > > Well take a look at mux options for the onenand connected pins, > > and if there is one that has a mux option for ext_dmareq or similar, > > chances are it can be used to retrigger dma transfers. > > > > I could be wrong of course, and it could be it won't even work with > > onenand. > > As previously discussed, slave DMA should be triggered by RDY pin > and it has nothing to do with code already in place. > > That also means someone should remove misleding comment line from > e2c5eb78a3cc "ARM: dts: Fix wrong GPMC size mappings for omaps" ;-) > > I'll do it later in v5 of "ARM: dts: Nokia: Use R/B pin". Well gpio_65 has sys_ndmareq1 mode.. And if it's RDY pin, it to me sounds like it really could be used for loading more data to the fifo. But then again, I have not looked at the datasheet so you probably know better :) > > Something to look later on for sure if anybody is interested. > > While there, we should also benchmark DMA memcpy as OMAP3 version > was using 384 bytes as limit to trigger DMA while OMAP2 was using > 1024. For smaller chunks, memcpy was used. I decided to leave 384 > in place, but I do not have OMAP2 hardware to do any verification. Yeah testing on n8x0 really should be done to avoid regressions. Regards, Tony
diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c index 883993bbe40b..5760e40be008 100644 --- a/drivers/mtd/onenand/omap2.c +++ b/drivers/mtd/onenand/omap2.c @@ -289,9 +289,7 @@ static inline int omap2_onenand_bufferram_offset(struct mtd_info *mtd, int area) return 0; } -#if defined(CONFIG_ARCH_OMAP3) || defined(MULTI_OMAP2) - -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, +static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, unsigned char *buffer, int offset, size_t count) { @@ -299,10 +297,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; + unsigned long t; void *buf = (void *)buffer; size_t xtra; - volatile unsigned *done; bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; if (bram_offset & 3 || (size_t)buf & 3 || count < 384) @@ -349,15 +346,11 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, 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; + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); - if (!*done) { + if (!t) { dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); goto out_copy; } @@ -369,7 +362,7 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, return 0; } -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, +static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, const unsigned char *buffer, int offset, size_t count) { @@ -377,9 +370,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; + unsigned long t; void *buf = (void *)buffer; - volatile unsigned *done; bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; if (bram_offset & 3 || (size_t)buf & 3 || count < 384) @@ -420,15 +412,11 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, 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; + t = wait_for_completion_io_timeout(&c->dma_done, msecs_to_jiffies(20)); dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); - if (!*done) { + if (!t) { dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); goto out_copy; } @@ -440,134 +428,6 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, return 0; } -#else - -static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area, - unsigned char *buffer, int offset, - size_t count) -{ - return -ENOSYS; -} - -static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area, - const unsigned char *buffer, - int offset, size_t count) -{ - return -ENOSYS; -} - -#endif - -#if defined(CONFIG_ARCH_OMAP2) || defined(MULTI_OMAP2) - -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, - unsigned char *buffer, int offset, - size_t count) -{ - 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; - - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; - /* DMA is not used. Revisit PM requirements before enabling it. */ - if (1 || (c->dma_channel < 0) || - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { - memcpy(buffer, (__force void *)(this->base + bram_offset), - count); - return 0; - } - - dma_src = c->phys_base + bram_offset; - dma_dst = dma_map_single(&c->pdev->dev, buffer, count, - DMA_FROM_DEVICE); - if (dma_mapping_error(&c->pdev->dev, dma_dst)) { - dev_err(&c->pdev->dev, - "Couldn't DMA map a %d byte buffer\n", - count); - 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); - - dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); - - return 0; -} - -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, - const unsigned char *buffer, - int offset, size_t count) -{ - 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; - - bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; - /* DMA is not used. Revisit PM requirements before enabling it. */ - if (1 || (c->dma_channel < 0) || - ((void *) buffer >= (void *) high_memory) || (bram_offset & 3) || - (((unsigned int) buffer) & 3) || (count < 1024) || (count & 3)) { - memcpy((__force void *)(this->base + bram_offset), buffer, - count); - return 0; - } - - dma_src = dma_map_single(&c->pdev->dev, (void *) buffer, count, - DMA_TO_DEVICE); - dma_dst = c->phys_base + bram_offset; - if (dma_mapping_error(&c->pdev->dev, dma_src)) { - dev_err(&c->pdev->dev, - "Couldn't DMA map a %d byte buffer\n", - count); - 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); - - dma_unmap_single(&c->pdev->dev, dma_src, count, DMA_TO_DEVICE); - - return 0; -} - -#else - -static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, - unsigned char *buffer, int offset, - size_t count) -{ - return -ENOSYS; -} - -static int omap2_onenand_write_bufferram(struct mtd_info *mtd, int area, - const unsigned char *buffer, - int offset, size_t count) -{ - return -ENOSYS; -} - -#endif - static struct platform_driver omap2_onenand_driver; static void omap2_onenand_shutdown(struct platform_device *pdev) @@ -691,13 +551,8 @@ static int omap2_onenand_probe(struct platform_device *pdev) this = &c->onenand; if (c->dma_channel >= 0) { this->wait = omap2_onenand_wait; - if (c->flags & ONENAND_IN_OMAP34XX) { - this->read_bufferram = omap3_onenand_read_bufferram; - this->write_bufferram = omap3_onenand_write_bufferram; - } else { - this->read_bufferram = omap2_onenand_read_bufferram; - this->write_bufferram = omap2_onenand_write_bufferram; - } + this->read_bufferram = omap2_onenand_read_bufferram; + this->write_bufferram = omap2_onenand_write_bufferram; } if ((r = onenand_scan(&c->mtd, 1)) < 0)
Since the very first commit (36cd4fb5d277: "[MTD] [OneNAND] Add OMAP2 / OMAP3 OneNAND driver") DMA is disabled for OMAP2. Later fixes thus went only into OMAP3 specific DMA functions which turned out not to be so OMAP3 specific. Therefore replace open coded timeout waiting for completion in OMAP3 functions with API call and merge those two implementations. Note that currently the is no single DMA user, so this change is basically no-op. Those interested will have to find DMA configuration in the linux-omap.git history. Signed-off-by: Ladislav Michl <ladis@linux-mips.org> --- Changes: -v3: new patch drivers/mtd/onenand/omap2.c | 165 +++----------------------------------------- 1 file changed, 10 insertions(+), 155 deletions(-)