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

Kyungmin Park kmpark at infradead.org
Mon Nov 17 03:37:45 EST 2008


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



More information about the linux-mtd mailing list