[PATCH][OneNAND] Write-while-program support

[Kyungmin Park]

On Mon, Nov 17, 2008 at 4:57 PM, Adrian Hunter
<ext-adrian.hunter at nokia.com> wrote:
> Kyungmin Park wrote:
>>
>> On Fri, Nov 14, 2008 at 7:32 PM, Adrian Hunter
>> <ext-adrian.hunter at 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