* [PATCH][OneNAND] Write-while-program support
@ 2008-11-14 0:28 Kyungmin Park
2008-11-14 10:32 ` Adrian Hunter
0 siblings, 1 reply; 8+ messages in thread
From: Kyungmin Park @ 2008-11-14 0:28 UTC (permalink / raw)
To: linux-mtd; +Cc: ext-adrian.hunter
This is from Adrian hunter and modified for write_ops operation.
It's similar with load-while-read method.
but it's write side performance improvement.
Here's brief performance results.
Note: Measured from OMAP3 with async OneNAND mode. (not sync mode)
=================================================================
speedtest: dev = 3
speedtest: Size=16777216 EB size=131072 Write size=2048 EB count=128
Pages per EB=64 Page size=2048
speedtest: scanning for bad blocks
speedtest: scanned 0
speedtest: scanned 128, found 0 bad
speedtest: erasing
speedtest: erased 0
speedtest: erased 128
speedtest: Testing eraseblock write speed
eraseblock write speed is 2424 KiB/s
speedtest: Testing eraseblock read speed
eraseblock read speed is 11985 KiB/s
speedtest: Testing page write speed
page write speed is 2493 KiB/s
speedtest: Testing page read speed
page read speed is 11586 KiB/s
speedtest: Testing 2 page write speed
2 page write speed is 2681 KiB/s
speedtest: Testing 2 page read speed
2 page read speed is 10922 KiB/s
speedtest: Testing erase speed
erase speed is 192752 KiB/s
speedtest: speedtest finished
=================================================================
Also it's passed all nand-tests.
Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index aaf3504..bc9b420 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1455,7 +1455,8 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
struct mtd_oob_ops *ops)
{
struct onenand_chip *this = mtd->priv;
- int written = 0, column, thislen, subpage;
+ int written = 0, column, thislen = 0, subpage = 0;
+ int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
int oobwritten = 0, oobcolumn, thisooblen, oobsize;
size_t len = ops->len;
size_t ooblen = ops->ooblen;
@@ -1492,76 +1493,90 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
column = to & (mtd->writesize - 1);
/* Loop until all data write */
- while (written < len) {
- u_char *wbuf = (u_char *) buf;
+ while (1) {
+ if (written < len) {
+ u_char *wbuf = (u_char *) buf;
- thislen = min_t(int, mtd->writesize - column, len - written);
- thisooblen = min_t(int, oobsize - oobcolumn, ooblen - oobwritten);
+ thislen = min_t(int, mtd->writesize - column, len - written);
+ thisooblen = min_t(int, oobsize - oobcolumn, ooblen - oobwritten);
- cond_resched();
+ cond_resched();
- this->command(mtd, ONENAND_CMD_BUFFERRAM, to, thislen);
+ this->command(mtd, ONENAND_CMD_BUFFERRAM, to, thislen);
- /* Partial page write */
- subpage = thislen < mtd->writesize;
- if (subpage) {
- memset(this->page_buf, 0xff, mtd->writesize);
- memcpy(this->page_buf + column, buf, thislen);
- wbuf = this->page_buf;
- }
+ /* Partial page write */
+ subpage = thislen < mtd->writesize;
+ if (subpage) {
+ memset(this->page_buf, 0xff, mtd->writesize);
+ memcpy(this->page_buf + column, buf, thislen);
+ wbuf = this->page_buf;
+ }
- this->write_bufferram(mtd, ONENAND_DATARAM, wbuf, 0, mtd->writesize);
+ this->write_bufferram(mtd, ONENAND_DATARAM, wbuf, 0, mtd->writesize);
- if (oob) {
- oobbuf = this->oob_buf;
+ if (oob) {
+ oobbuf = this->oob_buf;
- /* We send data to spare ram with oobsize
- * to prevent byte access */
- memset(oobbuf, 0xff, mtd->oobsize);
- if (ops->mode == MTD_OOB_AUTO)
- onenand_fill_auto_oob(mtd, oobbuf, oob, oobcolumn, thisooblen);
- else
- memcpy(oobbuf + oobcolumn, oob, thisooblen);
+ /* We send data to spare ram with oobsize
+ * to prevent byte access */
+ memset(oobbuf, 0xff, mtd->oobsize);
+ if (ops->mode == MTD_OOB_AUTO)
+ onenand_fill_auto_oob(mtd, oobbuf, oob, oobcolumn, thisooblen);
+ else
+ memcpy(oobbuf + oobcolumn, oob, thisooblen);
- oobwritten += thisooblen;
- oob += thisooblen;
- oobcolumn = 0;
+ oobwritten += thisooblen;
+ oob += thisooblen;
+ oobcolumn = 0;
+ } else
+ oobbuf = (u_char *) ffchars;
+
+ this->write_bufferram(mtd, ONENAND_SPARERAM, oobbuf, 0, mtd->oobsize);
} else
- oobbuf = (u_char *) ffchars;
+ ONENAND_SET_NEXT_BUFFERRAM(this);
- this->write_bufferram(mtd, ONENAND_SPARERAM, oobbuf, 0, mtd->oobsize);
+ if (!first) {
+ ONENAND_SET_PREV_BUFFERRAM(this);
- this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
+ ret = this->wait(mtd, FL_WRITING);
- ret = this->wait(mtd, FL_WRITING);
+ /* In partial page write we don't update bufferram */
+ onenand_update_bufferram(mtd, prev, !ret && !prev_subpage);
+ if (ONENAND_IS_2PLANE(this)) {
+ ONENAND_SET_BUFFERRAM1(this);
+ onenand_update_bufferram(mtd, prev + this->writesize, !ret && !prev_subpage);
+ }
- /* In partial page write we don't update bufferram */
- onenand_update_bufferram(mtd, to, !ret && !subpage);
- if (ONENAND_IS_2PLANE(this)) {
- ONENAND_SET_BUFFERRAM1(this);
- onenand_update_bufferram(mtd, to + this->writesize, !ret && !subpage);
- }
+ if (ret) {
+ printk(KERN_ERR "onenand_write_ops_nolock: write filaed %d\n", ret);
+ break;
+ }
- if (ret) {
- printk(KERN_ERR "onenand_write_ops_nolock: write filaed %d\n", ret);
- break;
- }
+ if (written == len) {
+ /* Only check verify write turn on */
+ ret = onenand_verify(mtd, buf - len, to - len, len);
+ if (ret) {
+ printk(KERN_ERR "onenand_write_ops_nolock: verify failed %d\n", ret);
+ break;
+ }
+ }
+ ONENAND_SET_NEXT_BUFFERRAM(this);
- /* Only check verify write turn on */
- ret = onenand_verify(mtd, buf, to, thislen);
- if (ret) {
- printk(KERN_ERR "onenand_write_ops_nolock: verify failed %d\n", ret);
- break;
+ if (written == len)
+ break;
}
- written += thislen;
+ this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
- if (written == len)
- break;
+ written += thislen;
column = 0;
+ prev_subpage = subpage;
+ prev = to;
+ prevlen = thislen;
to += thislen;
buf += thislen;
+ first = 0;
}
ops->retlen = written;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-14 0:28 [PATCH][OneNAND] Write-while-program support Kyungmin Park
@ 2008-11-14 10:32 ` Adrian Hunter
2008-11-14 12:12 ` Amit Kumar Sharma
2008-11-17 2:56 ` Kyungmin Park
0 siblings, 2 replies; 8+ messages in thread
From: Adrian Hunter @ 2008-11-14 10:32 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mtd
Kyungmin Park wrote:
> This is from Adrian hunter and modified for write_ops operation.
Thanks for looking at write-while-program. I have a few questions though.
I have copied the whole function below because the diff is too confusing:
> /**
> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or out-of-band
> * @param mtd MTD device structure
> * @param to offset to write to
> * @param ops oob operation description structure
> *
> * Write main and/or oob with ECC
> */
> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
> struct mtd_oob_ops *ops)
> {
> struct onenand_chip *this = mtd->priv;
> int written = 0, column, thislen = 0, subpage = 0;
> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
> size_t len = ops->len;
> size_t ooblen = ops->ooblen;
> const u_char *buf = ops->datbuf;
> const u_char *oob = ops->oobbuf;
> u_char *oobbuf;
> int ret = 0;
>
> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x, len = %i\n", (unsigned int) to, (int) len);
>
> /* Initialize retlen, in case of early exit */
> ops->retlen = 0;
> ops->oobretlen = 0;
>
> /* Do not allow writes past end of device */
> if (unlikely((to + len) > mtd->size)) {
> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write to past end of device\n");
> return -EINVAL;
> }
>
> /* Reject writes, which are not page aligned */
> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to write not page aligned data\n");
> return -EINVAL;
> }
>
The loop cannot handle the case when len is zero so I had:
if (!len)
return 0;
> if (ops->mode == MTD_OOB_AUTO)
> oobsize = this->ecclayout->oobavail;
> else
> oobsize = mtd->oobsize;
>
> oobcolumn = to & (mtd->oobsize - 1);
>
> column = to & (mtd->writesize - 1);
>
> /* Loop until all data write */
> while (1) {
> if (written < len) {
> u_char *wbuf = (u_char *) buf;
>
> thislen = min_t(int, mtd->writesize - column, len - written);
> thisooblen = min_t(int, oobsize - oobcolumn, ooblen - oobwritten);
>
> cond_resched();
>
> this->command(mtd, ONENAND_CMD_BUFFERRAM, to, thislen);
>
> /* Partial page write */
> subpage = thislen < mtd->writesize;
> if (subpage) {
> memset(this->page_buf, 0xff, mtd->writesize);
> memcpy(this->page_buf + column, buf, thislen);
> wbuf = this->page_buf;
> }
>
> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf, 0, mtd->writesize);
>
> if (oob) {
> oobbuf = this->oob_buf;
>
> /* We send data to spare ram with oobsize
> * to prevent byte access */
> memset(oobbuf, 0xff, mtd->oobsize);
> if (ops->mode == MTD_OOB_AUTO)
> onenand_fill_auto_oob(mtd, oobbuf, oob, oobcolumn, thisooblen);
> else
> memcpy(oobbuf + oobcolumn, oob, thisooblen);
>
> oobwritten += thisooblen;
> oob += thisooblen;
> oobcolumn = 0;
> } else
> oobbuf = (u_char *) ffchars;
>
> this->write_bufferram(mtd, ONENAND_SPARERAM, oobbuf, 0, mtd->oobsize);
> } else
> ONENAND_SET_NEXT_BUFFERRAM(this);
>
> if (!first) {
> ONENAND_SET_PREV_BUFFERRAM(this);
>
> ret = this->wait(mtd, FL_WRITING);
>
> /* In partial page write we don't update bufferram */
> onenand_update_bufferram(mtd, prev, !ret && !prev_subpage);
> if (ONENAND_IS_2PLANE(this)) {
I do not understand how 2PLANE is compatible with write-while-program because
2PLANE uses both buffers so we cannot start transferring to the second buffer
while the program is ongoing. Does it work?
Won't MLC and FlexOneNAND have that problem too?
> ONENAND_SET_BUFFERRAM1(this);
> onenand_update_bufferram(mtd, prev + this->writesize, !ret && !prev_subpage);
> }
>
> if (ret) {
My original patch had "written -= prevlen" here.
> printk(KERN_ERR "onenand_write_ops_nolock: write filaed %d\n", ret);
> break;
> }
>
> if (written == len) {
> /* Only check verify write turn on */
> ret = onenand_verify(mtd, buf - len, to - len, len);
> if (ret) {
> printk(KERN_ERR "onenand_write_ops_nolock: verify failed %d\n", ret);
> break;
> }
Why not just break here?
> }
> ONENAND_SET_NEXT_BUFFERRAM(this);
>
> if (written == len)
> break;
> }
>
> this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
>
> written += thislen;
>
> column = 0;
> prev_subpage = subpage;
> prev = to;
> prevlen = thislen;
> to += thislen;
> buf += thislen;
> first = 0;
> }
>
> ops->retlen = written;
Is ops->oobretlen needed here also?
>
> return ret;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-14 10:32 ` Adrian Hunter
@ 2008-11-14 12:12 ` Amit Kumar Sharma
2008-11-17 2:56 ` Kyungmin Park
1 sibling, 0 replies; 8+ messages in thread
From: Amit Kumar Sharma @ 2008-11-14 12:12 UTC (permalink / raw)
To: Adrian Hunter, Kyungmin Park; +Cc: linux-mtd
Hi Adrian
FlexOneNand read dual data ram buffer and so we can not use
----- Original Message -----
From: "Adrian Hunter" <ext-adrian.hunter@nokia.com>
To: "Kyungmin Park" <kmpark@infradead.org>
Cc: <linux-mtd@lists.infradead.org>
Sent: Friday, November 14, 2008 4:02 PM
Subject: Re: [PATCH][OneNAND] Write-while-program support
> Kyungmin Park wrote:
>> This is from Adrian hunter and modified for write_ops
>> operation.
>
> Thanks for looking at write-while-program. I have a few
> questions though.
> I have copied the whole function below because the diff is
> too confusing:
>
>> /**
>> * onenand_write_ops_nolock - [OneNAND Interface] write
>> main and/or out-of-band
>> * @param mtd MTD device structure
>> * @param to offset to write to
>> * @param ops oob operation description structure
>> *
>> * Write main and/or oob with ECC
>> */
>> static int onenand_write_ops_nolock(struct mtd_info *mtd,
>> loff_t to,
>> struct mtd_oob_ops *ops)
>> {
>> struct onenand_chip *this = mtd->priv;
>> int written = 0, column, thislen = 0, subpage = 0;
>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>> size_t len = ops->len;
>> size_t ooblen = ops->ooblen;
>> const u_char *buf = ops->datbuf;
>> const u_char *oob = ops->oobbuf;
>> u_char *oobbuf;
>> int ret = 0;
>>
>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to =
>> 0x%08x, len = %i\n", (unsigned int) to, (int) len);
>>
>> /* Initialize retlen, in case of early exit */
>> ops->retlen = 0;
>> ops->oobretlen = 0;
>>
>> /* Do not allow writes past end of device */
>> if (unlikely((to + len) > mtd->size)) {
>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write
>> to past end of device\n");
>> return -EINVAL;
>> }
>>
>> /* Reject writes, which are not page aligned */
>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len)))
>> {
>> printk(KERN_ERR
>> "onenand_write_ops_nolock: Attempt to write not page
>> aligned data\n");
>> return -EINVAL;
>> }
>>
>
> The loop cannot handle the case when len is zero so I had:
>
> if (!len)
> return 0;
>
>> if (ops->mode == MTD_OOB_AUTO)
>> oobsize = this->ecclayout->oobavail;
>> else
>> oobsize = mtd->oobsize;
>>
>> oobcolumn = to & (mtd->oobsize - 1);
>>
>> column = to & (mtd->writesize - 1);
>>
>> /* Loop until all data write */
>> while (1) {
>> if (written < len) {
>> u_char *wbuf = (u_char *) buf;
>>
>> thislen = min_t(int, mtd->writesize - column, len -
>> written);
>> thisooblen = min_t(int, oobsize - oobcolumn, ooblen -
>> oobwritten);
>>
>> cond_resched();
>>
>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to, thislen);
>>
>> /* Partial page write */
>> subpage = thislen < mtd->writesize;
>> if (subpage) {
>> memset(this->page_buf, 0xff, mtd->writesize);
>> memcpy(this->page_buf + column, buf, thislen);
>> wbuf = this->page_buf;
>> }
>>
>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf, 0,
>> mtd->writesize);
>>
>> if (oob) {
>> oobbuf = this->oob_buf;
>>
>> /* We send data to spare ram with oobsize
>> * to prevent byte access */
>> memset(oobbuf, 0xff, mtd->oobsize);
>> if (ops->mode == MTD_OOB_AUTO)
>> onenand_fill_auto_oob(mtd, oobbuf, oob, oobcolumn,
>> thisooblen);
>> else
>> memcpy(oobbuf + oobcolumn, oob, thisooblen);
>>
>> oobwritten += thisooblen;
>> oob += thisooblen;
>> oobcolumn = 0;
>> } else
>> oobbuf = (u_char *) ffchars;
>>
>> this->write_bufferram(mtd, ONENAND_SPARERAM, oobbuf, 0,
>> mtd->oobsize);
>> } else
>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>
>> if (!first) {
>> ONENAND_SET_PREV_BUFFERRAM(this);
>>
>> ret = this->wait(mtd, FL_WRITING);
>>
>> /* In partial page write we don't update bufferram */
>> onenand_update_bufferram(mtd, prev, !ret &&
>> !prev_subpage);
>> if (ONENAND_IS_2PLANE(this)) {
>
> I do not understand how 2PLANE is compatible with
> write-while-program because
> 2PLANE uses both buffers so we cannot start transferring
> to the second buffer
> while the program is ongoing. Does it work?
>
> Won't MLC and FlexOneNAND have that problem too?
Amit : FlexOneNand use dual data ram buffer and so we can
not use it and other point is for OneNand we have a plan to
use dual buufer
because OneNand page size will increase to 4k.
>
>> ONENAND_SET_BUFFERRAM1(this);
>> onenand_update_bufferram(mtd, prev + this->writesize,
>> !ret && !prev_subpage);
>> }
>>
>> if (ret) {
>
> My original patch had "written -= prevlen" here.
>
>> printk(KERN_ERR "onenand_write_ops_nolock: write filaed
>> %d\n", ret);
>> break;
>> }
>>
>> if (written == len) {
>> /* Only check verify write turn on */
>> ret = onenand_verify(mtd, buf - len, to - len, len);
>> if (ret) {
>> printk(KERN_ERR "onenand_write_ops_nolock: verify failed
>> %d\n", ret);
>> break;
>> }
>
> Why not just break here?
>
Amit : same as kyungmin comment.
>> }
>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>
>> if (written == len)
>> break;
>> }
>>
>> this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
>>
>> written += thislen;
>>
>> column = 0;
>> prev_subpage = subpage;
>> prev = to;
>> prevlen = thislen;
>> to += thislen;
>> buf += thislen;
>> first = 0;
>> }
>>
>> ops->retlen = written;
>
> Is ops->oobretlen needed here also?
>
>>
>> return ret;
>> }
>
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-14 10:32 ` Adrian Hunter
2008-11-14 12:12 ` Amit Kumar Sharma
@ 2008-11-17 2:56 ` Kyungmin Park
2008-11-17 7:57 ` Adrian Hunter
1 sibling, 1 reply; 8+ messages in thread
From: Kyungmin Park @ 2008-11-17 2:56 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mtd
HI,
On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
<ext-adrian.hunter@nokia.com> wrote:
> Kyungmin Park wrote:
>>
>> This is from Adrian hunter and modified for write_ops operation.
>
> Thanks for looking at write-while-program. I have a few questions though.
> I have copied the whole function below because the diff is too confusing:
>
>> /**
>> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or
>> out-of-band
>> * @param mtd MTD device structure
>> * @param to offset to write to
>> * @param ops oob operation description structure
>> *
>> * Write main and/or oob with ECC
>> */
>> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>> struct mtd_oob_ops *ops)
>> {
>> struct onenand_chip *this = mtd->priv;
>> int written = 0, column, thislen = 0, subpage = 0;
>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>> size_t len = ops->len;
>> size_t ooblen = ops->ooblen;
>> const u_char *buf = ops->datbuf;
>> const u_char *oob = ops->oobbuf;
>> u_char *oobbuf;
>> int ret = 0;
>>
>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x, len
>> = %i\n", (unsigned int) to, (int) len);
>>
>> /* Initialize retlen, in case of early exit */
>> ops->retlen = 0;
>> ops->oobretlen = 0;
>>
>> /* Do not allow writes past end of device */
>> if (unlikely((to + len) > mtd->size)) {
>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write to
>> past end of device\n");
>> return -EINVAL;
>> }
>>
>> /* Reject writes, which are not page aligned */
>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to write
>> not page aligned data\n");
>> return -EINVAL;
>> }
>>
>
> The loop cannot handle the case when len is zero so I had:
>
> if (!len)
> return 0;
Yes there's length check. I wonder is it possible to write with zero?
Is it handle at driver level or upper level such as filesystem?
But no problem to just add length check here at this time.
>
>> if (ops->mode == MTD_OOB_AUTO)
>> oobsize = this->ecclayout->oobavail;
>> else
>> oobsize = mtd->oobsize;
>>
>> oobcolumn = to & (mtd->oobsize - 1);
>>
>> column = to & (mtd->writesize - 1);
>>
>> /* Loop until all data write */
>> while (1) {
>> if (written < len) {
>> u_char *wbuf = (u_char *) buf;
>>
>> thislen = min_t(int, mtd->writesize - column, len -
>> written);
>> thisooblen = min_t(int, oobsize - oobcolumn, ooblen
>> - oobwritten);
>>
>> cond_resched();
>>
>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to,
>> thislen);
>>
>> /* Partial page write */
>> subpage = thislen < mtd->writesize;
>> if (subpage) {
>> memset(this->page_buf, 0xff,
>> mtd->writesize);
>> memcpy(this->page_buf + column, buf,
>> thislen);
>> wbuf = this->page_buf;
>> }
>>
>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf,
>> 0, mtd->writesize);
>>
>> if (oob) {
>> oobbuf = this->oob_buf;
>>
>> /* We send data to spare ram with oobsize
>> * to prevent byte access */
>> memset(oobbuf, 0xff, mtd->oobsize);
>> if (ops->mode == MTD_OOB_AUTO)
>> onenand_fill_auto_oob(mtd, oobbuf,
>> oob, oobcolumn, thisooblen);
>> else
>> memcpy(oobbuf + oobcolumn, oob,
>> thisooblen);
>>
>> oobwritten += thisooblen;
>> oob += thisooblen;
>> oobcolumn = 0;
>> } else
>> oobbuf = (u_char *) ffchars;
>>
>> this->write_bufferram(mtd, ONENAND_SPARERAM,
>> oobbuf, 0, mtd->oobsize);
>> } else
>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>
>> if (!first) {
>> ONENAND_SET_PREV_BUFFERRAM(this);
>>
>> ret = this->wait(mtd, FL_WRITING);
>>
>> /* In partial page write we don't update bufferram
>> */
>> onenand_update_bufferram(mtd, prev, !ret &&
>> !prev_subpage);
>> if (ONENAND_IS_2PLANE(this)) {
>
> I do not understand how 2PLANE is compatible with write-while-program
> because
> 2PLANE uses both buffers so we cannot start transferring to the second
> buffer
> while the program is ongoing. Does it work?
>
> Won't MLC and FlexOneNAND have that problem too?
Agree, I'm not yet tested the 2PLANE features and it doesn't support
write-while-program.
If the 2plane and flex-onenand, it used the previous one.
>
>> ONENAND_SET_BUFFERRAM1(this);
>> onenand_update_bufferram(mtd, prev +
>> this->writesize, !ret && !prev_subpage);
>> }
>>
>> if (ret) {
>
> My original patch had "written -= prevlen" here.
Agreed.
>
>> printk(KERN_ERR "onenand_write_ops_nolock:
>> write filaed %d\n", ret);
>> break;
>> }
>>
>> if (written == len) {
>> /* Only check verify write turn on */
>> ret = onenand_verify(mtd, buf - len, to -
>> len, len);
>> if (ret) {
>> printk(KERN_ERR
>> "onenand_write_ops_nolock: verify failed %d\n", ret);
>> break;
>> }
>
> Why not just break here?
E.g., In original version, write bufferram 0 and it changes bufferam 1
automatically.
Same as if break here, it points out the previous bufferram.
>
>> }
>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>
>> if (written == len)
>> break;
>> }
>>
>> this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
>>
>> written += thislen;
>>
>> column = 0;
>> prev_subpage = subpage;
>> prev = to;
>> prevlen = thislen;
>> to += thislen;
>> buf += thislen;
>> first = 0;
>> }
>>
>> ops->retlen = written;
>
> Is ops->oobretlen needed here also?
Okay I will added. I'm not sure YAFFS handle this one correctly. Maybe
it used the retlen in ops operation (data + spare).
>
>>
>> return ret;
>> }
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-17 2:56 ` Kyungmin Park
@ 2008-11-17 7:57 ` Adrian Hunter
2008-11-17 8:23 ` Kyungmin Park
0 siblings, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2008-11-17 7:57 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mtd
Kyungmin Park wrote:
> On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
> <ext-adrian.hunter@nokia.com> wrote:
>> Kyungmin Park wrote:
>>> This is from Adrian hunter and modified for write_ops operation.
>> Thanks for looking at write-while-program. I have a few questions though.
>> I have copied the whole function below because the diff is too confusing:
>>
>>> /**
>>> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or
>>> out-of-band
>>> * @param mtd MTD device structure
>>> * @param to offset to write to
>>> * @param ops oob operation description structure
>>> *
>>> * Write main and/or oob with ECC
>>> */
>>> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>>> struct mtd_oob_ops *ops)
>>> {
>>> struct onenand_chip *this = mtd->priv;
>>> int written = 0, column, thislen = 0, subpage = 0;
>>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>>> size_t len = ops->len;
>>> size_t ooblen = ops->ooblen;
>>> const u_char *buf = ops->datbuf;
>>> const u_char *oob = ops->oobbuf;
>>> u_char *oobbuf;
>>> int ret = 0;
>>>
>>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x, len
>>> = %i\n", (unsigned int) to, (int) len);
>>>
>>> /* Initialize retlen, in case of early exit */
>>> ops->retlen = 0;
>>> ops->oobretlen = 0;
>>>
>>> /* Do not allow writes past end of device */
>>> if (unlikely((to + len) > mtd->size)) {
>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write to
>>> past end of device\n");
>>> return -EINVAL;
>>> }
>>>
>>> /* Reject writes, which are not page aligned */
>>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to write
>>> not page aligned data\n");
>>> return -EINVAL;
>>> }
>>>
>> The loop cannot handle the case when len is zero so I had:
>>
>> if (!len)
>> return 0;
>
> Yes there's length check. I wonder is it possible to write with zero?
> Is it handle at driver level or upper level such as filesystem?
> But no problem to just add length check here at this time.
>
>>> if (ops->mode == MTD_OOB_AUTO)
>>> oobsize = this->ecclayout->oobavail;
>>> else
>>> oobsize = mtd->oobsize;
>>>
>>> oobcolumn = to & (mtd->oobsize - 1);
>>>
>>> column = to & (mtd->writesize - 1);
>>>
>>> /* Loop until all data write */
>>> while (1) {
>>> if (written < len) {
>>> u_char *wbuf = (u_char *) buf;
>>>
>>> thislen = min_t(int, mtd->writesize - column, len -
>>> written);
>>> thisooblen = min_t(int, oobsize - oobcolumn, ooblen
>>> - oobwritten);
>>>
>>> cond_resched();
>>>
>>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to,
>>> thislen);
>>>
>>> /* Partial page write */
>>> subpage = thislen < mtd->writesize;
>>> if (subpage) {
>>> memset(this->page_buf, 0xff,
>>> mtd->writesize);
>>> memcpy(this->page_buf + column, buf,
>>> thislen);
>>> wbuf = this->page_buf;
>>> }
>>>
>>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf,
>>> 0, mtd->writesize);
>>>
>>> if (oob) {
>>> oobbuf = this->oob_buf;
>>>
>>> /* We send data to spare ram with oobsize
>>> * to prevent byte access */
>>> memset(oobbuf, 0xff, mtd->oobsize);
>>> if (ops->mode == MTD_OOB_AUTO)
>>> onenand_fill_auto_oob(mtd, oobbuf,
>>> oob, oobcolumn, thisooblen);
>>> else
>>> memcpy(oobbuf + oobcolumn, oob,
>>> thisooblen);
>>>
>>> oobwritten += thisooblen;
>>> oob += thisooblen;
>>> oobcolumn = 0;
>>> } else
>>> oobbuf = (u_char *) ffchars;
>>>
>>> this->write_bufferram(mtd, ONENAND_SPARERAM,
>>> oobbuf, 0, mtd->oobsize);
>>> } else
>>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>>
>>> if (!first) {
>>> ONENAND_SET_PREV_BUFFERRAM(this);
>>>
>>> ret = this->wait(mtd, FL_WRITING);
>>>
>>> /* In partial page write we don't update bufferram
>>> */
>>> onenand_update_bufferram(mtd, prev, !ret &&
>>> !prev_subpage);
>>> if (ONENAND_IS_2PLANE(this)) {
>> I do not understand how 2PLANE is compatible with write-while-program
>> because
>> 2PLANE uses both buffers so we cannot start transferring to the second
>> buffer
>> while the program is ongoing. Does it work?
>>
>> Won't MLC and FlexOneNAND have that problem too?
>
> Agree, I'm not yet tested the 2PLANE features and it doesn't support
> write-while-program.
> If the 2plane and flex-onenand, it used the previous one.
>
>>> ONENAND_SET_BUFFERRAM1(this);
>>> onenand_update_bufferram(mtd, prev +
>>> this->writesize, !ret && !prev_subpage);
>>> }
>>>
>>> if (ret) {
>> My original patch had "written -= prevlen" here.
>
> Agreed.
>
Also we need to invalidate the other bufferram because we transferred the
data but did not write it. i.e.
if (written < len)
onenand_update_bufferram(mtd, to, 0);
>>> printk(KERN_ERR "onenand_write_ops_nolock:
>>> write filaed %d\n", ret);
>>> break;
>>> }
>>>
>>> if (written == len) {
>>> /* Only check verify write turn on */
>>> ret = onenand_verify(mtd, buf - len, to -
>>> len, len);
>>> if (ret) {
>>> printk(KERN_ERR
>>> "onenand_write_ops_nolock: verify failed %d\n", ret);
>>> break;
>>> }
>> Why not just break here?
>
> E.g., In original version, write bufferram 0 and it changes bufferam 1
> automatically.
> Same as if break here, it points out the previous bufferram.
The previous bufferram is the last one that was used, so it should
point to the previous one.
>>> }
>>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>>
>>> if (written == len)
>>> break;
>>> }
>>>
>>> this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize);
>>>
>>> written += thislen;
>>>
>>> column = 0;
>>> prev_subpage = subpage;
>>> prev = to;
>>> prevlen = thislen;
>>> to += thislen;
>>> buf += thislen;
>>> first = 0;
>>> }
>>>
>>> ops->retlen = written;
>> Is ops->oobretlen needed here also?
>
> Okay I will added. I'm not sure YAFFS handle this one correctly. Maybe
> it used the retlen in ops operation (data + spare).
>
>>> return ret;
>>> }
>
> Thank you,
> Kyungmin Park
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-17 7:57 ` Adrian Hunter
@ 2008-11-17 8:23 ` Kyungmin Park
2008-11-17 8:41 ` Adrian Hunter
0 siblings, 1 reply; 8+ messages in thread
From: Kyungmin Park @ 2008-11-17 8:23 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mtd
On Mon, Nov 17, 2008 at 4:57 PM, Adrian Hunter
<ext-adrian.hunter@nokia.com> wrote:
> Kyungmin Park wrote:
>>
>> On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
>> <ext-adrian.hunter@nokia.com> wrote:
>>>
>>> Kyungmin Park wrote:
>>>>
>>>> This is from Adrian hunter and modified for write_ops operation.
>>>
>>> Thanks for looking at write-while-program. I have a few questions
>>> though.
>>> I have copied the whole function below because the diff is too confusing:
>>>
>>>> /**
>>>> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or
>>>> out-of-band
>>>> * @param mtd MTD device structure
>>>> * @param to offset to write to
>>>> * @param ops oob operation description structure
>>>> *
>>>> * Write main and/or oob with ECC
>>>> */
>>>> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>>>> struct mtd_oob_ops *ops)
>>>> {
>>>> struct onenand_chip *this = mtd->priv;
>>>> int written = 0, column, thislen = 0, subpage = 0;
>>>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>>>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>>>> size_t len = ops->len;
>>>> size_t ooblen = ops->ooblen;
>>>> const u_char *buf = ops->datbuf;
>>>> const u_char *oob = ops->oobbuf;
>>>> u_char *oobbuf;
>>>> int ret = 0;
>>>>
>>>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x,
>>>> len
>>>> = %i\n", (unsigned int) to, (int) len);
>>>>
>>>> /* Initialize retlen, in case of early exit */
>>>> ops->retlen = 0;
>>>> ops->oobretlen = 0;
>>>>
>>>> /* Do not allow writes past end of device */
>>>> if (unlikely((to + len) > mtd->size)) {
>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write
>>>> to
>>>> past end of device\n");
>>>> return -EINVAL;
>>>> }
>>>>
>>>> /* Reject writes, which are not page aligned */
>>>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to
>>>> write
>>>> not page aligned data\n");
>>>> return -EINVAL;
>>>> }
>>>>
>>> The loop cannot handle the case when len is zero so I had:
>>>
>>> if (!len)
>>> return 0;
>>
>> Yes there's length check. I wonder is it possible to write with zero?
>> Is it handle at driver level or upper level such as filesystem?
>> But no problem to just add length check here at this time.
>>
>>>> if (ops->mode == MTD_OOB_AUTO)
>>>> oobsize = this->ecclayout->oobavail;
>>>> else
>>>> oobsize = mtd->oobsize;
>>>>
>>>> oobcolumn = to & (mtd->oobsize - 1);
>>>>
>>>> column = to & (mtd->writesize - 1);
>>>>
>>>> /* Loop until all data write */
>>>> while (1) {
>>>> if (written < len) {
>>>> u_char *wbuf = (u_char *) buf;
>>>>
>>>> thislen = min_t(int, mtd->writesize - column, len
>>>> -
>>>> written);
>>>> thisooblen = min_t(int, oobsize - oobcolumn,
>>>> ooblen
>>>> - oobwritten);
>>>>
>>>> cond_resched();
>>>>
>>>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to,
>>>> thislen);
>>>>
>>>> /* Partial page write */
>>>> subpage = thislen < mtd->writesize;
>>>> if (subpage) {
>>>> memset(this->page_buf, 0xff,
>>>> mtd->writesize);
>>>> memcpy(this->page_buf + column, buf,
>>>> thislen);
>>>> wbuf = this->page_buf;
>>>> }
>>>>
>>>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf,
>>>> 0, mtd->writesize);
>>>>
>>>> if (oob) {
>>>> oobbuf = this->oob_buf;
>>>>
>>>> /* We send data to spare ram with oobsize
>>>> * to prevent byte access */
>>>> memset(oobbuf, 0xff, mtd->oobsize);
>>>> if (ops->mode == MTD_OOB_AUTO)
>>>> onenand_fill_auto_oob(mtd, oobbuf,
>>>> oob, oobcolumn, thisooblen);
>>>> else
>>>> memcpy(oobbuf + oobcolumn, oob,
>>>> thisooblen);
>>>>
>>>> oobwritten += thisooblen;
>>>> oob += thisooblen;
>>>> oobcolumn = 0;
>>>> } else
>>>> oobbuf = (u_char *) ffchars;
>>>>
>>>> this->write_bufferram(mtd, ONENAND_SPARERAM,
>>>> oobbuf, 0, mtd->oobsize);
>>>> } else
>>>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>>>
>>>> if (!first) {
>>>> ONENAND_SET_PREV_BUFFERRAM(this);
>>>>
>>>> ret = this->wait(mtd, FL_WRITING);
>>>>
>>>> /* In partial page write we don't update bufferram
>>>> */
>>>> onenand_update_bufferram(mtd, prev, !ret &&
>>>> !prev_subpage);
>>>> if (ONENAND_IS_2PLANE(this)) {
>>>
>>> I do not understand how 2PLANE is compatible with write-while-program
>>> because
>>> 2PLANE uses both buffers so we cannot start transferring to the second
>>> buffer
>>> while the program is ongoing. Does it work?
>>>
>>> Won't MLC and FlexOneNAND have that problem too?
>>
>> Agree, I'm not yet tested the 2PLANE features and it doesn't support
>> write-while-program.
>> If the 2plane and flex-onenand, it used the previous one.
>>
>>>> ONENAND_SET_BUFFERRAM1(this);
>>>> onenand_update_bufferram(mtd, prev +
>>>> this->writesize, !ret && !prev_subpage);
>>>> }
>>>>
>>>> if (ret) {
>>>
>>> My original patch had "written -= prevlen" here.
>>
>> Agreed.
>>
>
> Also we need to invalidate the other bufferram because we transferred the
> data but did not write it. i.e.
>
> if (written < len)
> onenand_update_bufferram(mtd, to, 0);
For clarity, clear all bufferram at error case.
>
>>>> printk(KERN_ERR "onenand_write_ops_nolock:
>>>> write filaed %d\n", ret);
>>>> break;
>>>> }
>>>>
>>>> if (written == len) {
>>>> /* Only check verify write turn on */
>>>> ret = onenand_verify(mtd, buf - len, to -
>>>> len, len);
>>>> if (ret) {
>>>> printk(KERN_ERR
>>>> "onenand_write_ops_nolock: verify failed %d\n", ret);
>>>> break;
>>>> }
>>>
>>> Why not just break here?
>>
>> E.g., In original version, write bufferram 0 and it changes bufferam 1
>> automatically.
>> Same as if break here, it points out the previous bufferram.
>
> The previous bufferram is the last one that was used, so it should
> point to the previous one.
>
then next time, it overwrite the previous bufferram, but it expects it
write next bufferram.
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-17 8:41 ` Adrian Hunter
@ 2008-11-17 8:37 ` Kyungmin Park
0 siblings, 0 replies; 8+ messages in thread
From: Kyungmin Park @ 2008-11-17 8:37 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mtd
On Mon, Nov 17, 2008 at 5:41 PM, Adrian Hunter
<ext-adrian.hunter@nokia.com> wrote:
> Kyungmin Park wrote:
>>
>> On Mon, Nov 17, 2008 at 4:57 PM, Adrian Hunter
>> <ext-adrian.hunter@nokia.com> wrote:
>>>
>>> Kyungmin Park wrote:
>>>>
>>>> On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
>>>> <ext-adrian.hunter@nokia.com> wrote:
>>>>>
>>>>> Kyungmin Park wrote:
>>>>>>
>>>>>> This is from Adrian hunter and modified for write_ops operation.
>>>>>
>>>>> Thanks for looking at write-while-program. I have a few questions
>>>>> though.
>>>>> I have copied the whole function below because the diff is too
>>>>> confusing:
>>>>>
>>>>>> /**
>>>>>> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or
>>>>>> out-of-band
>>>>>> * @param mtd MTD device structure
>>>>>> * @param to offset to write to
>>>>>> * @param ops oob operation description structure
>>>>>> *
>>>>>> * Write main and/or oob with ECC
>>>>>> */
>>>>>> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>>>>>> struct mtd_oob_ops *ops)
>>>>>> {
>>>>>> struct onenand_chip *this = mtd->priv;
>>>>>> int written = 0, column, thislen = 0, subpage = 0;
>>>>>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>>>>>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>>>>>> size_t len = ops->len;
>>>>>> size_t ooblen = ops->ooblen;
>>>>>> const u_char *buf = ops->datbuf;
>>>>>> const u_char *oob = ops->oobbuf;
>>>>>> u_char *oobbuf;
>>>>>> int ret = 0;
>>>>>>
>>>>>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x,
>>>>>> len
>>>>>> = %i\n", (unsigned int) to, (int) len);
>>>>>>
>>>>>> /* Initialize retlen, in case of early exit */
>>>>>> ops->retlen = 0;
>>>>>> ops->oobretlen = 0;
>>>>>>
>>>>>> /* Do not allow writes past end of device */
>>>>>> if (unlikely((to + len) > mtd->size)) {
>>>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write
>>>>>> to
>>>>>> past end of device\n");
>>>>>> return -EINVAL;
>>>>>> }
>>>>>>
>>>>>> /* Reject writes, which are not page aligned */
>>>>>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
>>>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to
>>>>>> write
>>>>>> not page aligned data\n");
>>>>>> return -EINVAL;
>>>>>> }
>>>>>>
>>>>> The loop cannot handle the case when len is zero so I had:
>>>>>
>>>>> if (!len)
>>>>> return 0;
>>>>
>>>> Yes there's length check. I wonder is it possible to write with zero?
>>>> Is it handle at driver level or upper level such as filesystem?
>>>> But no problem to just add length check here at this time.
>>>>
>>>>>> if (ops->mode == MTD_OOB_AUTO)
>>>>>> oobsize = this->ecclayout->oobavail;
>>>>>> else
>>>>>> oobsize = mtd->oobsize;
>>>>>>
>>>>>> oobcolumn = to & (mtd->oobsize - 1);
>>>>>>
>>>>>> column = to & (mtd->writesize - 1);
>>>>>>
>>>>>> /* Loop until all data write */
>>>>>> while (1) {
>>>>>> if (written < len) {
>>>>>> u_char *wbuf = (u_char *) buf;
>>>>>>
>>>>>> thislen = min_t(int, mtd->writesize - column, len
>>>>>> -
>>>>>> written);
>>>>>> thisooblen = min_t(int, oobsize - oobcolumn,
>>>>>> ooblen
>>>>>> - oobwritten);
>>>>>>
>>>>>> cond_resched();
>>>>>>
>>>>>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to,
>>>>>> thislen);
>>>>>>
>>>>>> /* Partial page write */
>>>>>> subpage = thislen < mtd->writesize;
>>>>>> if (subpage) {
>>>>>> memset(this->page_buf, 0xff,
>>>>>> mtd->writesize);
>>>>>> memcpy(this->page_buf + column, buf,
>>>>>> thislen);
>>>>>> wbuf = this->page_buf;
>>>>>> }
>>>>>>
>>>>>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf,
>>>>>> 0, mtd->writesize);
>>>>>>
>>>>>> if (oob) {
>>>>>> oobbuf = this->oob_buf;
>>>>>>
>>>>>> /* We send data to spare ram with oobsize
>>>>>> * to prevent byte access */
>>>>>> memset(oobbuf, 0xff, mtd->oobsize);
>>>>>> if (ops->mode == MTD_OOB_AUTO)
>>>>>> onenand_fill_auto_oob(mtd,
>>>>>> oobbuf,
>>>>>> oob, oobcolumn, thisooblen);
>>>>>> else
>>>>>> memcpy(oobbuf + oobcolumn, oob,
>>>>>> thisooblen);
>>>>>>
>>>>>> oobwritten += thisooblen;
>>>>>> oob += thisooblen;
>>>>>> oobcolumn = 0;
>>>>>> } else
>>>>>> oobbuf = (u_char *) ffchars;
>>>>>>
>>>>>> this->write_bufferram(mtd, ONENAND_SPARERAM,
>>>>>> oobbuf, 0, mtd->oobsize);
>>>>>> } else
>>>>>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>>>>>
>>>>>> if (!first) {
>>>>>> ONENAND_SET_PREV_BUFFERRAM(this);
>>>>>>
>>>>>> ret = this->wait(mtd, FL_WRITING);
>>>>>>
>>>>>> /* In partial page write we don't update
>>>>>> bufferram
>>>>>> */
>>>>>> onenand_update_bufferram(mtd, prev, !ret &&
>>>>>> !prev_subpage);
>>>>>> if (ONENAND_IS_2PLANE(this)) {
>>>>>
>>>>> I do not understand how 2PLANE is compatible with write-while-program
>>>>> because
>>>>> 2PLANE uses both buffers so we cannot start transferring to the second
>>>>> buffer
>>>>> while the program is ongoing. Does it work?
>>>>>
>>>>> Won't MLC and FlexOneNAND have that problem too?
>>>>
>>>> Agree, I'm not yet tested the 2PLANE features and it doesn't support
>>>> write-while-program.
>>>> If the 2plane and flex-onenand, it used the previous one.
>>>>
>>>>>> ONENAND_SET_BUFFERRAM1(this);
>>>>>> onenand_update_bufferram(mtd, prev +
>>>>>> this->writesize, !ret && !prev_subpage);
>>>>>> }
>>>>>>
>>>>>> if (ret) {
>>>>>
>>>>> My original patch had "written -= prevlen" here.
>>>>
>>>> Agreed.
>>>>
>>> Also we need to invalidate the other bufferram because we transferred the
>>> data but did not write it. i.e.
>>>
>>> if (written < len)
>>> onenand_update_bufferram(mtd, to, 0);
>>
>> For clarity, clear all bufferram at error case.
>
> OK
>
>>>>>> printk(KERN_ERR
>>>>>> "onenand_write_ops_nolock:
>>>>>> write filaed %d\n", ret);
>>>>>> break;
>>>>>> }
>>>>>>
>>>>>> if (written == len) {
>>>>>> /* Only check verify write turn on */
>>>>>> ret = onenand_verify(mtd, buf - len, to -
>>>>>> len, len);
>>>>>> if (ret) {
>>>>>> printk(KERN_ERR
>>>>>> "onenand_write_ops_nolock: verify failed %d\n", ret);
>>>>>> break;
>>>>>> }
>>>>>
>>>>> Why not just break here?
>>>>
>>>> E.g., In original version, write bufferram 0 and it changes bufferam 1
>>>> automatically.
>>>> Same as if break here, it points out the previous bufferram.
>>>
>>> The previous bufferram is the last one that was used, so it should
>>> point to the previous one.
>>>
>>
>> then next time, it overwrite the previous bufferram, but it expects it
>> write next bufferram.
>
> The current MTD write is finished because written == len. The next one
> starts by changing the bufferram with this->command(mtd,
> ONENAND_CMD_BUFFERRAM, ...)
>
Ah sorry, you're right. I'm confused.
I will repost it.
Thank you,
Kyungmin Park
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][OneNAND] Write-while-program support
2008-11-17 8:23 ` Kyungmin Park
@ 2008-11-17 8:41 ` Adrian Hunter
2008-11-17 8:37 ` Kyungmin Park
0 siblings, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2008-11-17 8:41 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mtd
Kyungmin Park wrote:
> On Mon, Nov 17, 2008 at 4:57 PM, Adrian Hunter
> <ext-adrian.hunter@nokia.com> wrote:
>> Kyungmin Park wrote:
>>> On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
>>> <ext-adrian.hunter@nokia.com> wrote:
>>>> Kyungmin Park wrote:
>>>>> This is from Adrian hunter and modified for write_ops operation.
>>>> Thanks for looking at write-while-program. I have a few questions
>>>> though.
>>>> I have copied the whole function below because the diff is too confusing:
>>>>
>>>>> /**
>>>>> * onenand_write_ops_nolock - [OneNAND Interface] write main and/or
>>>>> out-of-band
>>>>> * @param mtd MTD device structure
>>>>> * @param to offset to write to
>>>>> * @param ops oob operation description structure
>>>>> *
>>>>> * Write main and/or oob with ECC
>>>>> */
>>>>> static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
>>>>> struct mtd_oob_ops *ops)
>>>>> {
>>>>> struct onenand_chip *this = mtd->priv;
>>>>> int written = 0, column, thislen = 0, subpage = 0;
>>>>> int prev = 0, prevlen = 0, prev_subpage = 0, first = 1;
>>>>> int oobwritten = 0, oobcolumn, thisooblen, oobsize;
>>>>> size_t len = ops->len;
>>>>> size_t ooblen = ops->ooblen;
>>>>> const u_char *buf = ops->datbuf;
>>>>> const u_char *oob = ops->oobbuf;
>>>>> u_char *oobbuf;
>>>>> int ret = 0;
>>>>>
>>>>> DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_ops_nolock: to = 0x%08x,
>>>>> len
>>>>> = %i\n", (unsigned int) to, (int) len);
>>>>>
>>>>> /* Initialize retlen, in case of early exit */
>>>>> ops->retlen = 0;
>>>>> ops->oobretlen = 0;
>>>>>
>>>>> /* Do not allow writes past end of device */
>>>>> if (unlikely((to + len) > mtd->size)) {
>>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt write
>>>>> to
>>>>> past end of device\n");
>>>>> return -EINVAL;
>>>>> }
>>>>>
>>>>> /* Reject writes, which are not page aligned */
>>>>> if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
>>>>> printk(KERN_ERR "onenand_write_ops_nolock: Attempt to
>>>>> write
>>>>> not page aligned data\n");
>>>>> return -EINVAL;
>>>>> }
>>>>>
>>>> The loop cannot handle the case when len is zero so I had:
>>>>
>>>> if (!len)
>>>> return 0;
>>> Yes there's length check. I wonder is it possible to write with zero?
>>> Is it handle at driver level or upper level such as filesystem?
>>> But no problem to just add length check here at this time.
>>>
>>>>> if (ops->mode == MTD_OOB_AUTO)
>>>>> oobsize = this->ecclayout->oobavail;
>>>>> else
>>>>> oobsize = mtd->oobsize;
>>>>>
>>>>> oobcolumn = to & (mtd->oobsize - 1);
>>>>>
>>>>> column = to & (mtd->writesize - 1);
>>>>>
>>>>> /* Loop until all data write */
>>>>> while (1) {
>>>>> if (written < len) {
>>>>> u_char *wbuf = (u_char *) buf;
>>>>>
>>>>> thislen = min_t(int, mtd->writesize - column, len
>>>>> -
>>>>> written);
>>>>> thisooblen = min_t(int, oobsize - oobcolumn,
>>>>> ooblen
>>>>> - oobwritten);
>>>>>
>>>>> cond_resched();
>>>>>
>>>>> this->command(mtd, ONENAND_CMD_BUFFERRAM, to,
>>>>> thislen);
>>>>>
>>>>> /* Partial page write */
>>>>> subpage = thislen < mtd->writesize;
>>>>> if (subpage) {
>>>>> memset(this->page_buf, 0xff,
>>>>> mtd->writesize);
>>>>> memcpy(this->page_buf + column, buf,
>>>>> thislen);
>>>>> wbuf = this->page_buf;
>>>>> }
>>>>>
>>>>> this->write_bufferram(mtd, ONENAND_DATARAM, wbuf,
>>>>> 0, mtd->writesize);
>>>>>
>>>>> if (oob) {
>>>>> oobbuf = this->oob_buf;
>>>>>
>>>>> /* We send data to spare ram with oobsize
>>>>> * to prevent byte access */
>>>>> memset(oobbuf, 0xff, mtd->oobsize);
>>>>> if (ops->mode == MTD_OOB_AUTO)
>>>>> onenand_fill_auto_oob(mtd, oobbuf,
>>>>> oob, oobcolumn, thisooblen);
>>>>> else
>>>>> memcpy(oobbuf + oobcolumn, oob,
>>>>> thisooblen);
>>>>>
>>>>> oobwritten += thisooblen;
>>>>> oob += thisooblen;
>>>>> oobcolumn = 0;
>>>>> } else
>>>>> oobbuf = (u_char *) ffchars;
>>>>>
>>>>> this->write_bufferram(mtd, ONENAND_SPARERAM,
>>>>> oobbuf, 0, mtd->oobsize);
>>>>> } else
>>>>> ONENAND_SET_NEXT_BUFFERRAM(this);
>>>>>
>>>>> if (!first) {
>>>>> ONENAND_SET_PREV_BUFFERRAM(this);
>>>>>
>>>>> ret = this->wait(mtd, FL_WRITING);
>>>>>
>>>>> /* In partial page write we don't update bufferram
>>>>> */
>>>>> onenand_update_bufferram(mtd, prev, !ret &&
>>>>> !prev_subpage);
>>>>> if (ONENAND_IS_2PLANE(this)) {
>>>> I do not understand how 2PLANE is compatible with write-while-program
>>>> because
>>>> 2PLANE uses both buffers so we cannot start transferring to the second
>>>> buffer
>>>> while the program is ongoing. Does it work?
>>>>
>>>> Won't MLC and FlexOneNAND have that problem too?
>>> Agree, I'm not yet tested the 2PLANE features and it doesn't support
>>> write-while-program.
>>> If the 2plane and flex-onenand, it used the previous one.
>>>
>>>>> ONENAND_SET_BUFFERRAM1(this);
>>>>> onenand_update_bufferram(mtd, prev +
>>>>> this->writesize, !ret && !prev_subpage);
>>>>> }
>>>>>
>>>>> if (ret) {
>>>> My original patch had "written -= prevlen" here.
>>> Agreed.
>>>
>> Also we need to invalidate the other bufferram because we transferred the
>> data but did not write it. i.e.
>>
>> if (written < len)
>> onenand_update_bufferram(mtd, to, 0);
>
> For clarity, clear all bufferram at error case.
OK
>>>>> printk(KERN_ERR "onenand_write_ops_nolock:
>>>>> write filaed %d\n", ret);
>>>>> break;
>>>>> }
>>>>>
>>>>> if (written == len) {
>>>>> /* Only check verify write turn on */
>>>>> ret = onenand_verify(mtd, buf - len, to -
>>>>> len, len);
>>>>> if (ret) {
>>>>> printk(KERN_ERR
>>>>> "onenand_write_ops_nolock: verify failed %d\n", ret);
>>>>> break;
>>>>> }
>>>> Why not just break here?
>>> E.g., In original version, write bufferram 0 and it changes bufferam 1
>>> automatically.
>>> Same as if break here, it points out the previous bufferram.
>> The previous bufferram is the last one that was used, so it should
>> point to the previous one.
>>
>
> then next time, it overwrite the previous bufferram, but it expects it
> write next bufferram.
The current MTD write is finished because written == len. The next one
starts by changing the bufferram with this->command(mtd, ONENAND_CMD_BUFFERRAM, ...)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-11-17 8:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-14 0:28 [PATCH][OneNAND] Write-while-program support Kyungmin Park
2008-11-14 10:32 ` Adrian Hunter
2008-11-14 12:12 ` Amit Kumar Sharma
2008-11-17 2:56 ` Kyungmin Park
2008-11-17 7:57 ` Adrian Hunter
2008-11-17 8:23 ` Kyungmin Park
2008-11-17 8:41 ` Adrian Hunter
2008-11-17 8:37 ` Kyungmin Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox