Hi Scott,
Thanks for the reviews.
On Mon, Jun 6, 2011 at 5:00 PM, Scott Wood scottwood@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