[U-Boot] [PATCH] mtd: nand: fix the written length when nand_write_skip_bad failed

[htbegin]

Hi, Scott

On Thu, Mar 7, 2013 at 2:22 AM, Scott Wood <scottwood at freescale.com> wrote:
> On 03/06/2013 08:56:56 AM, htbegin wrote:
>>
>> Hi, Scott
>>
>> Thanks for your review.
>>
>> On Tue, Mar 5, 2013 at 9:58 AM, Scott Wood <scottwood at freescale.com>
>> wrote:
>> > On 03/02/2013 03:01:10 AM, Tao Hou wrote:
>> >>
>> >> When the data has been partially written into the NAND Flash,
>> >> returning the written length instead of 0. The written length
>> >> may be useful when the upper level decides to continue the writing
>> >> after skipping the block causing the write failure.
>> >
>> >
>> > We already do that in some code paths.
>> >
>> >
>> >> Signed-off-by: Tao Hou <hotforest at gmail.com>
>> >> Cc: Scott Wood <scottwood at freescale.com>
>> >> Cc: Ben Gardiner <bengardiner at nanometrics.ca>
>> >> Cc: Lei Wen <leiwen at marvell.com>
>> >> ---
>> >>  drivers/mtd/nand/nand_util.c |   22 +++++++++++++++-------
>> >>  1 file changed, 15 insertions(+), 7 deletions(-)
>> >
>> >
>> > Could you rebase this on top of this patch:
>> > http://patchwork.ozlabs.org/patch/224842/
>> Do you mean a V2 patch ?
>
>
> Yes.
I will send a V2 patch once we reach an agreement on the
"written_length" problem.

>
>
>> > BTW, are you actually using WITH_YAFFS_OOB?  I think there are some
>> > other
>> > things wrong with it at the moment, as mentioned here:
>> > http://lists.denx.de/pipermail/u-boot/2013-March/148378.html
>> No, I don't use it.
>
>
> Changes to that code should be tested by someone...
Sorry, I can't help.

>
>
>> >>                 if (rval != 0) {
>> >>                         printf("NAND write to offset %llx failed %d\n",
>> >>                                 offset, rval);
>> >> -                       *length -= left_to_write;
>> >> +                       *length -= left_to_write - written_size;
>> >>                         return rval;
>> >>                 }
>> >
>> >
>> > ...but I don't see why this part is needed (or correct).  Why doesn't
>> > "*length -= left_to_write" already get you what you want?
>> >
>> > -Scott
>> I just use "*length -= left_to_write - written_size" to tell the upper
>> level that what
>> had been actually happened. For the current block, "written_size" has
>> been written to the NAND Flash yet, so left_to_write should be
>> subtracted by "written_size".
>
>
> But left_to_write isn't decreased until after this error return, so that's
> already the case.  Subtracting written_size from left_to_write has the
> effect of increasing length by written_size, so the return value will now
> look like the error page has been written.
>
> -Scott
No, the returned value doesn't include the length of the error page.
In no-WITH_YAFFS_OOB case,  when nand_write failed,
truncated_write_size has been
updated by nand_write to the length which has been successfully
written , so it's OK to subtract written_size from left_to_write.
In WITH_YAFFS_OOB case, when nand->write_oob failed, written_size is
also the length which  has been successfully written.

Cheers,
Hou