[U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command

[Ben Gardiner]

Hi Scott,

Thanks for the reviews.

On Mon, Jun 6, 2011 at 5:00 PM, Scott Wood <scottwood at freescale.com> wrote:
> On Tue, May 24, 2011 at 10:18:37AM -0400, Ben Gardiner wrote:
>> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
>> index e7db4c9..786f179 100644
>> --- a/common/cmd_nand.c
>> +++ b/common/cmd_nand.c
>> @@ -575,6 +575,16 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>>                       else
>>                               ret = nand_write_skip_bad(nand, off, &rwsize,
>>                                                         (u_char *)addr, 0);
>> +#ifdef CONFIG_CMD_NAND_TRIMFFS
>> +             } else if (!strcmp(s, ".trimffs")) {
>> +                     if (read) {
>> +                             printf("Unknown nand command suffix '%s'\n", s);
>> +                             return 1;
>> +                     }
>> +                     ret = nand_write_skip_bad(nand, off, &rwsize,
>> +                                             (u_char *)addr,
>> +                                             WITH_DROP_FFS);
>> +#endif
>>  #ifdef CONFIG_CMD_NAND_YAFFS
>>               } else if (!strcmp(s, ".yaffs")) {
>
> Should these really be modes rather than flags?

For the two currently possible values of 'int flags' -- yes. But
that's because the WITH_YAFFS_OOB causes a specific exuction path
which is mutually exclusive to the usual path; whereas the
WITH_DROP_FFS option does an operation in addition to the usual
execution. So the latter is (to me at least) a flag whereas the former
is a mode.

I see you have already applied the patches which convert to a flag --
so I will leave as is in v4. I will, as noted in the previous email --
add a return -EINVAL if WITH_YAFFS_OOB is used with any other flags.

>> +   nand write.trimffs addr ofs|partition size
>> +      Enabled by the CONFIG_CMD_NAND_TRIMFFS macro. This command will write to
>> +      the NAND flash in a manner identical to the 'nand write' command described
>> +      above -- with the additional check that all pages at the end of
>> +      eraseblocks which contain only 0xff data will not be written to the NAND
>> +      flash. This behaviour is required when flashing UBI images containing
>> +       UBIFS volumes as per the UBI FAQ[1].
>
> Please wrap at 80 columns.

Ok. Will do in v4.

checkpatch.pl did not complain about "     the NAND flash in a manner
identical to the 'nand write' command described" -- which is 81
characters including the \n but it did not complain about a line over
85 characters in that README either so I guess READMEs aren't enforced
by that script.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca