| Message ID | f9a94a1f4b1aadc2a94821cf4148d6bf4273758f.1306246651.git.bengardiner@nanometrics.ca |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Ben, > Add another nand write. variant, trimffs. This command will request of > nand_write_skip_bad() that all trailing all-0xff pages will be > dropped from eraseblocks when they are written to flash as-per the > reccommended behaviour of the UBI FAQ [1]. > > The function that implements this timming is the drop_ffs() function > by Artem Bityutskiy, ported from the mtd-utils tree. > > [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo > > Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca> > CC: Artem Bityutskiy <dedekind1@gmail.com> > CC: Detlev Zundel <dzu@denx.de> Thanks - looks good now. Acked-by: Detlev Zundel <dzu@denx.de> Cheers Detlev
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? > + 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. -Scott
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
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")) { if (read) { @@ -689,6 +699,12 @@ U_BOOT_CMD( "nand write - addr off|partition size\n" " read/write 'size' bytes starting at offset 'off'\n" " to/from memory address 'addr', skipping bad blocks.\n" +#ifdef CONFIG_CMD_NAND_TRIMFFS + "nand write.trimffs - addr off|partition size\n" + " write 'size' bytes starting at offset 'off' from memory address\n" + " 'addr', skipping bad blocks and dropping any pages at the end\n" + " of eraseblocks that contain only 0xFF\n" +#endif #ifdef CONFIG_CMD_NAND_YAFFS "nand write.yaffs - addr off|partition size\n" " write 'size' bytes starting at offset 'off' with yaffs format\n" diff --git a/doc/README.nand b/doc/README.nand index 8eedb6c..ca62f00 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -78,6 +78,16 @@ Commands: should work well, but loading an image copied from another flash is going to be trouble if there are any bad blocks. + 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]. + + [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo + nand write.oob addr ofs|partition size Write `size' bytes from `addr' to the out-of-band data area corresponding to `ofs' in NAND flash. This is limited to the 16 bytes
Add another nand write. variant, trimffs. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks when they are written to flash as-per the reccommended behaviour of the UBI FAQ [1]. The function that implements this timming is the drop_ffs() function by Artem Bityutskiy, ported from the mtd-utils tree. [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca> CC: Artem Bityutskiy <dedekind1@gmail.com> CC: Detlev Zundel <dzu@denx.de> --- Changes since v2: * added nand write.trimffs to the README.nand file * moved the nand_util copyright header addition to patch 'nand_util: drop trailing all-0xff pages if requested' Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * renamed the command variant to '.trimffs' from '.ubi' (Detlev Zundel) * added attribution to mtd-utils and Artem Bityutskiy in both the source comments and commit message * wrapped the new command in a new ifdef, CONFIG_CMD_NAND_TRIMFFS, to reduce the size impact of this new feature --- common/cmd_nand.c | 16 ++++++++++++++++ doc/README.nand | 10 ++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)