[U-Boot-Users] [PATCH] NAND read/write.jffs2 fix

patch for branch mtd-2.6.22.1 on git://git.denx.de/u-boot-nand-flash.git
nand read(.jffs2|.e|.i) skips bad blocks during read. write(.jffs2|.e|.i) skips bad blocks during write nand read will read 0xff for bad block. Update documentation.
Signed-off-by: Morten Ebbell Hestnes morten.hestnes@tandberg.com --- common/cmd_nand.c | 77 ++--- doc/README.nand | 16 +- drivers/mtd/nand/nand_util.c | 813 ++++++++++++------------------------------ include/nand.h | 6 +- 4 files changed, 272 insertions(+), 640 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index eff9173..237f4c9 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -39,6 +39,7 @@ int find_dev_and_part(const char *id, struct mtd_device **dev, #endif
extern nand_info_t nand_info[]; /* info for NAND chips */ +extern struct nand_chip nand_chip[]; /* extra info for NAND chips */
static int nand_dump(nand_info_t *nand, ulong off, int only_oob) { @@ -73,9 +74,9 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) while (i--) { if (!only_oob) { printf( "\t%02x %02x %02x %02x %02x %02x %02x %02x" - " %02x %02x %02x %02x %02x %02x %02x %02x\n", - p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7], - p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); + " %02x %02x %02x %02x %02x %02x %02x %02x\n", + p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7], + p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); } p += 16; } @@ -319,7 +320,6 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
}
- /* read write */ if (strncmp(cmd, "read", 4) == 0 || strncmp(cmd, "write", 5) == 0) { int read;
@@ -327,40 +327,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) goto usage;
addr = (ulong)simple_strtoul(argv[2], NULL, 16); - read = strncmp(cmd, "read", 4) == 0; /* 1 = read, 0 = write */ printf("\nNAND %s: ", read ? "read" : "write"); if (arg_off_size(argc - 3, argv + 3, nand, &off, &size) != 0) return 1;
s = strchr(cmd, '.'); - if (s != NULL && - (!strcmp(s, ".jffs2") || !strcmp(s, ".e") || !strcmp(s, ".i"))) { - if (read) { - /* read */ - nand_read_options_t opts; - memset(&opts, 0, sizeof(opts)); - opts.buffer = (u_char*) addr; - opts.length = size; - opts.offset = off; - opts.quiet = quiet; -/* - * ! BROKEN ! - * - * TODO: Function must be implemented - * - * ret = nand_read_opts(nand, &opts); - */ - } else { - /* write */ - mtd_oob_ops_t opts; - memset(&opts, 0, sizeof(opts)); - opts.datbuf = (u_char*) addr; - opts.len = size; - opts.ooblen = 64; - opts.mode = MTD_OOB_AUTO; - ret = nand_write_opts(nand, off, &opts); - } + if (s != NULL && (!strcmp(s, ".jffs2") || + !strcmp(s, ".e") || !strcmp(s, ".i"))) { + if (read) + ret = nand_read_skip_bad(nand, off, &size, (u_char *)addr); + else + ret = nand_write_skip_bad(nand, off, &size, (u_char *)addr); + } else if (s != NULL && !strcmp(s, ".oob")) { struct mtd_oob_ops ops;
@@ -372,9 +351,10 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) ret = nand->read_oob(nand, off, &ops); else ret = nand->write_oob(nand, off, &ops); + } else { if (read) - ret = nand_read(nand, off, &size, (u_char *)addr); + ret = nand_read_ff_upon_bad(nand, off, &size,(u_char *)addr); else ret = nand_write(nand, off, &size, (u_char *)addr); } @@ -399,6 +379,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) } return 1; } + if (strcmp(cmd, "biterr") == 0) { /* todo */ return 1; @@ -490,21 +471,21 @@ usage: }
U_BOOT_CMD(nand, 5, 1, do_nand, - "nand - NAND sub-system\n", - "info - show available NAND devices\n" - "nand device [dev] - show or set current device\n" - "nand read[.jffs2] - addr off|partition size\n" - "nand write[.jffs2] - addr off|partition size - read/write `size' bytes starting\n" - " at offset `off' to/from memory address `addr'\n" - "nand erase [clean] [off size] - erase `size' bytes from\n" - " offset `off' (entire device if not specified)\n" - "nand bad - show bad blocks\n" - "nand dump[.oob] off - dump page\n" - "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n" - "nand markbad off - mark bad block at offset (UNSAFE)\n" - "nand biterr off - make a bit error at offset (UNSAFE)\n" - "nand lock [tight] [status] - bring nand to lock state or display locked pages\n" - "nand unlock [offset] [size] - unlock section\n"); + "nand - NAND sub-system\n", + "info - show available NAND devices\n" + "nand device [dev] - show or set current device\n" + "nand read[.jffs2, .i] addr off|partition size\n" + "nand write[.jffs2, .i] addr off|partition size\n" + " - read/write `size' bytes starting at offset `off' to/from memory address `addr'\n" + "nand erase [clean] [off size]\n" + " - erase `size' bytes from offset `off' (entire device if not specified)\n" + "nand bad - show bad blocks\n" + "nand dump[.oob] off - dump page\n" + "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n" + "nand markbad off - mark bad block at offset (UNSAFE)\n" + "nand biterr off - make a bit error at offset (UNSAFE)\n" + "nand lock [tight] [status] - bring nand to lock state or display locked pages\n" + "nand unlock [offset] [size] - unlock section\n");
static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand, ulong offset, ulong addr, char *cmd) diff --git a/doc/README.nand b/doc/README.nand index 647a6b8..56f791f 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -57,14 +57,14 @@ Commands: Print information about all of the NAND devices found.
nand read addr ofs|partition size - Read `size' bytes from `ofs' in NAND flash to `addr'. If a page - cannot be read because it is marked bad or an uncorrectable data - error is found the command stops with an error. + Read `size' bytes from `ofs' in NAND flash to `addr'. Data for + blocks that are marked bad is read as 0xff. If an uncorrectable + data error is found the command stops with an error.
- nand read.jffs2 addr ofs|partition size - Like `read', but the data for blocks that are marked bad is read as - 0xff. This gives a readable JFFS2 image that can be processed by - the JFFS2 commands such as ls and fsload. + nand read.jffs2|.i addr ofs|partition size + Like `read', but the data for blocks that are marked bad are skipped + and it read the next block instead. This gives a readable JFFS2 image + that can be processed by the JFFS2 commands such as ls and fsload.
nand read.oob addr ofs|partition size Read `size' bytes from the out-of-band data area corresponding to @@ -77,7 +77,7 @@ Commands: cannot be written because it is marked bad or the write fails the command stops with an error.
- nand write.jffs2 addr ofs|partition size + nand write.jffs2|.i addr ofs|partition size Like `write', but blocks that are marked bad are skipped and the data is written to the next block instead. This allows writing a JFFS2 image, as long as the image is short enough to fit even diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 1916aca..dd4ed6b 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -52,6 +52,14 @@ typedef struct mtd_info mtd_info_t; #define cpu_to_je16(x) (x) #define cpu_to_je32(x) (x)
+#if 0 +#define MARK printf("Was here: %s, %s, %d\n",__FILE__, __FUNCTION__, __LINE__); +#define DEBUGP(fmt,args...) printf("%s, %s, %d: "fmt,__FILE__, __FUNCTION__, __LINE__,##args) +#else +#define MARK +#define DEBUGP(fmt,args...) +#endif + /*****************************************************************************/ static int nand_block_bad_scrub(struct mtd_info *mtd, loff_t ofs, int getchip) { @@ -248,586 +256,6 @@ static struct nand_ecclayout autoplace_ecclayout = { }; #endif
- -/** - * nand_fill_oob - [Internal] Transfer client buffer to oob - * @chip: nand chip structure - * @oob: oob data buffer - * @ops: oob ops structure - * - * Copied from nand_base.c - */ -static uint8_t *nand_fill_oob(struct nand_chip *chip, uint8_t *oob, - struct mtd_oob_ops *ops) -{ - size_t len = ops->ooblen; - - switch(ops->mode) { - - case MTD_OOB_PLACE: - case MTD_OOB_RAW: - memcpy(chip->oob_poi + ops->ooboffs, oob, len); - return oob + len; - - case MTD_OOB_AUTO: { - struct nand_oobfree *free = chip->ecc.layout->oobfree; - uint32_t boffs = 0, woffs = ops->ooboffs; - size_t bytes = 0; - - for(; free->length && len; free++, len -= bytes) { - /* Write request not from offset 0 ? */ - if (unlikely(woffs)) { - if (woffs >= free->length) { - woffs -= free->length; - continue; - } - boffs = free->offset + woffs; - bytes = min_t(size_t, len, - (free->length - woffs)); - woffs = 0; - } else { - bytes = min_t(size_t, len, free->length); - boffs = free->offset; - } - memcpy(chip->oob_poi + boffs, oob, bytes); - oob += bytes; - } - return oob; - } - default: - BUG(); - } - return NULL; -} - -#define NOTALIGNED(x) (x & (chip->subpagesize - 1)) != 0 - - -/* copied from nand_base.c: nand_do_write_ops() - * Only very small changes - */ -int nand_write_opts(nand_info_t *mtd, loff_t to, mtd_oob_ops_t *ops) -{ - int chipnr, realpage, page, blockmask, column; - struct nand_chip *chip = mtd->priv; - uint32_t writelen = ops->len; - uint8_t *oob = ops->oobbuf; - uint8_t *buf = ops->datbuf; - int ret, subpage; - - ops->retlen = 0; - if (!writelen) - return 0; - - printk("nand_write_opts: to: 0x%08x, ops->len: 0x%08x\n", to, ops->len); - - /* reject writes, which are not page aligned */ - if (NOTALIGNED(to) || NOTALIGNED(ops->len)) { - printk(KERN_NOTICE "nand_write: " - "Attempt to write not page aligned data\n"); - return -EINVAL; - } - - column = to & (mtd->writesize - 1); - subpage = column || (writelen & (mtd->writesize - 1)); - - if (subpage && oob) { - printk(KERN_NOTICE "nand_write: " - "Attempt to write oob to subpage\n"); - return -EINVAL; - } - - chipnr = (int)(to >> chip->chip_shift); - chip->select_chip(mtd, chipnr); - - /* XXX U-BOOT XXX */ -#if 0 - /* Check, if it is write protected */ - if (nand_check_wp(mtd)) - return -EIO; -#endif - - realpage = (int)(to >> chip->page_shift); - page = realpage & chip->pagemask; - blockmask = (1 << (chip->phys_erase_shift - chip->page_shift)) - 1; - - /* Invalidate the page cache, when we write to the cached page */ - if (to <= (chip->pagebuf << chip->page_shift) && - (chip->pagebuf << chip->page_shift) < (to + ops->len)) - chip->pagebuf = -1; - - /* If we're not given explicit OOB data, let it be 0xFF */ - if (likely(!oob)) { - printf("!oob, writing %d bytes with 0xff to chip->oob_poi (0x%08x)\n", mtd->oobsize, chip->oob_poi); - memset(chip->oob_poi, 0xff, mtd->oobsize); - } - - while(1) { - int bytes = mtd->writesize; - int cached = writelen > bytes && page != blockmask; - uint8_t *wbuf = buf; - - /* Partial page write ? */ - if (unlikely(column || writelen < (mtd->writesize - 1))) { - cached = 0; - bytes = min_t(int, bytes - column, (int) writelen); - chip->pagebuf = -1; - memset(chip->buffers->databuf, 0xff, mtd->writesize); - memcpy(&chip->buffers->databuf[column], buf, bytes); - wbuf = chip->buffers->databuf; - } - - if (unlikely(oob)) - oob = nand_fill_oob(chip, oob, ops); - - ret = chip->write_page(mtd, chip, wbuf, page, cached, - (ops->mode == MTD_OOB_RAW)); - if (ret) - break; - - writelen -= bytes; - if (!writelen) - break; - - column = 0; - buf += bytes; - realpage++; - - page = realpage & chip->pagemask; - /* Check, if we cross a chip boundary */ - if (!page) { - chipnr++; - chip->select_chip(mtd, -1); - chip->select_chip(mtd, chipnr); - } - } - - ops->retlen = ops->len - writelen; - if (unlikely(oob)) - ops->oobretlen = ops->ooblen; - return ret; -} - -/* XXX U-BOOT XXX */ -#if 0 -/** - * nand_write_opts: - write image to NAND flash with support for various options - * - * @param meminfo NAND device to erase - * @param opts write options (@see nand_write_options) - * @return 0 in case of success - * - * This code is ported from nandwrite.c from Linux mtd utils by - * Steven J. Hill and Thomas Gleixner. - */ -int nand_write_opts(nand_info_t *meminfo, const nand_write_options_t *opts) -{ - int imglen = 0; - int pagelen; - int baderaseblock; - int blockstart = -1; - loff_t offs; - int readlen; - int ecclayoutchanged = 0; - int percent_complete = -1; - struct nand_ecclayout old_ecclayout; - ulong mtdoffset = opts->offset; - ulong erasesize_blockalign; - u_char *buffer = opts->buffer; - size_t written; - int result; - - if (opts->pad && opts->writeoob) { - printf("Can't pad when oob data is present.\n"); - return -1; - } - - /* set erasesize to specified number of blocks - to match - * jffs2 (virtual) block size */ - if (opts->blockalign == 0) { - erasesize_blockalign = meminfo->erasesize; - } else { - erasesize_blockalign = meminfo->erasesize * opts->blockalign; - } - - /* make sure device page sizes are valid */ - if (!(meminfo->oobsize == 16 && meminfo->writesize == 512) - && !(meminfo->oobsize == 8 && meminfo->writesize == 256) - && !(meminfo->oobsize == 64 && meminfo->writesize == 2048)) { - printf("Unknown flash (not normal NAND)\n"); - return -1; - } - - /* read the current oob info */ - memcpy(&old_ecclayout, &meminfo->ecclayout, sizeof(old_ecclayout)); - - /* write without ecc? */ - if (opts->noecc) { - memcpy(&meminfo->ecclayout, &none_ecclayout, - sizeof(meminfo->ecclayout)); - ecclayoutchanged = 1; - } - - /* autoplace ECC? */ - if (opts->autoplace && (old_ecclayout.useecc != MTD_NANDECC_AUTOPLACE)) { - - memcpy(&meminfo->ecclayout, &autoplace_ecclayout, - sizeof(meminfo->ecclayout)); - ecclayoutchanged = 1; - } - - /* force OOB layout for jffs2 or yaffs? */ - if (opts->forcejffs2 || opts->forceyaffs) { - struct nand_ecclayout *oobsel = - opts->forcejffs2 ? &jffs2_ecclayout : &yaffs_ecclayout; - - if (meminfo->oobsize == 8) { - if (opts->forceyaffs) { - printf("YAFSS cannot operate on " - "256 Byte page size\n"); - goto restoreoob; - } - /* Adjust number of ecc bytes */ - jffs2_ecclayout.eccbytes = 3; - } - - memcpy(&meminfo->ecclayout, oobsel, sizeof(meminfo->ecclayout)); - } - - /* get image length */ - imglen = opts->length; - pagelen = meminfo->writesize - + ((opts->writeoob != 0) ? meminfo->oobsize : 0); - - /* check, if file is pagealigned */ - if ((!opts->pad) && ((imglen % pagelen) != 0)) { - printf("Input block length is not page aligned\n"); - goto restoreoob; - } - - /* check, if length fits into device */ - if (((imglen / pagelen) * meminfo->writesize) - > (meminfo->size - opts->offset)) { - printf("Image %d bytes, NAND page %d bytes, " - "OOB area %u bytes, device size %u bytes\n", - imglen, pagelen, meminfo->writesize, meminfo->size); - printf("Input block does not fit into device\n"); - goto restoreoob; - } - - if (!opts->quiet) - printf("\n"); - - /* get data from input and write to the device */ - while (imglen && (mtdoffset < meminfo->size)) { - - WATCHDOG_RESET (); - - /* - * new eraseblock, check for bad block(s). Stay in the - * loop to be sure if the offset changes because of - * a bad block, that the next block that will be - * written to is also checked. Thus avoiding errors if - * the block(s) after the skipped block(s) is also bad - * (number of blocks depending on the blockalign - */ - while (blockstart != (mtdoffset & (~erasesize_blockalign+1))) { - blockstart = mtdoffset & (~erasesize_blockalign+1); - offs = blockstart; - baderaseblock = 0; - - /* check all the blocks in an erase block for - * bad blocks */ - do { - int ret = meminfo->block_isbad(meminfo, offs); - - if (ret < 0) { - printf("Bad block check failed\n"); - goto restoreoob; - } - if (ret == 1) { - baderaseblock = 1; - if (!opts->quiet) - printf("\rBad block at 0x%lx " - "in erase block from " - "0x%x will be skipped\n", - (long) offs, - blockstart); - } - - if (baderaseblock) { - mtdoffset = blockstart - + erasesize_blockalign; - } - offs += erasesize_blockalign - / opts->blockalign; - } while (offs < blockstart + erasesize_blockalign); - } - - readlen = meminfo->writesize; - if (opts->pad && (imglen < readlen)) { - readlen = imglen; - memset(data_buf + readlen, 0xff, - meminfo->writesize - readlen); - } - - /* read page data from input memory buffer */ - memcpy(data_buf, buffer, readlen); - buffer += readlen; - - if (opts->writeoob) { - /* read OOB data from input memory block, exit - * on failure */ - memcpy(oob_buf, buffer, meminfo->oobsize); - buffer += meminfo->oobsize; - - /* write OOB data first, as ecc will be placed - * in there*/ - result = meminfo->write_oob(meminfo, - mtdoffset, - meminfo->oobsize, - &written, - (unsigned char *) - &oob_buf); - - if (result != 0) { - printf("\nMTD writeoob failure: %d\n", - result); - goto restoreoob; - } - imglen -= meminfo->oobsize; - } - - /* write out the page data */ - result = meminfo->write(meminfo, - mtdoffset, - meminfo->writesize, - &written, - (unsigned char *) &data_buf); - - if (result != 0) { - printf("writing NAND page at offset 0x%lx failed\n", - mtdoffset); - goto restoreoob; - } - imglen -= readlen; - - if (!opts->quiet) { - unsigned long long n = (unsigned long long) - (opts->length-imglen) * 100; - int percent; - - do_div(n, opts->length); - percent = (int)n; - - /* output progress message only at whole percent - * steps to reduce the number of messages printed - * on (slow) serial consoles - */ - if (percent != percent_complete) { - printf("\rWriting data at 0x%x " - "-- %3d%% complete.", - mtdoffset, percent); - percent_complete = percent; - } - } - - mtdoffset += meminfo->writesize; - } - - if (!opts->quiet) - printf("\n"); - -restoreoob: - if (ecclayoutchanged) { - memcpy(&meminfo->ecclayout, &old_ecclayout, - sizeof(meminfo->ecclayout)); - } - - if (imglen > 0) { - printf("Data did not fit into device, due to bad blocks\n"); - return -1; - } - - /* return happy */ - return 0; -} - -/** - * nand_read_opts: - read image from NAND flash with support for various options - * - * @param meminfo NAND device to erase - * @param opts read options (@see struct nand_read_options) - * @return 0 in case of success - * - */ -int nand_read_opts(nand_info_t *meminfo, const nand_read_options_t *opts) -{ - int imglen = opts->length; - int pagelen; - int baderaseblock; - int blockstart = -1; - int percent_complete = -1; - loff_t offs; - size_t readlen; - ulong mtdoffset = opts->offset; - u_char *buffer = opts->buffer; - int result; - - /* make sure device page sizes are valid */ - if (!(meminfo->oobsize == 16 && meminfo->writesize == 512) - && !(meminfo->oobsize == 8 && meminfo->writesize == 256) - && !(meminfo->oobsize == 64 && meminfo->writesize == 2048)) { - printf("Unknown flash (not normal NAND)\n"); - return -1; - } - - pagelen = meminfo->writesize - + ((opts->readoob != 0) ? meminfo->oobsize : 0); - - /* check, if length is not larger than device */ - if (((imglen / pagelen) * meminfo->writesize) - > (meminfo->size - opts->offset)) { - printf("Image %d bytes, NAND page %d bytes, " - "OOB area %u bytes, device size %u bytes\n", - imglen, pagelen, meminfo->writesize, meminfo->size); - printf("Input block is larger than device\n"); - return -1; - } - - if (!opts->quiet) - printf("\n"); - - /* get data from input and write to the device */ - while (imglen && (mtdoffset < meminfo->size)) { - - WATCHDOG_RESET (); - - /* - * new eraseblock, check for bad block(s). Stay in the - * loop to be sure if the offset changes because of - * a bad block, that the next block that will be - * written to is also checked. Thus avoiding errors if - * the block(s) after the skipped block(s) is also bad - * (number of blocks depending on the blockalign - */ - while (blockstart != (mtdoffset & (~meminfo->erasesize+1))) { - blockstart = mtdoffset & (~meminfo->erasesize+1); - offs = blockstart; - baderaseblock = 0; - - /* check all the blocks in an erase block for - * bad blocks */ - do { - int ret = meminfo->block_isbad(meminfo, offs); - - if (ret < 0) { - printf("Bad block check failed\n"); - return -1; - } - if (ret == 1) { - baderaseblock = 1; - if (!opts->quiet) - printf("\rBad block at 0x%lx " - "in erase block from " - "0x%x will be skipped\n", - (long) offs, - blockstart); - } - - if (baderaseblock) { - mtdoffset = blockstart - + meminfo->erasesize; - } - offs += meminfo->erasesize; - - } while (offs < blockstart + meminfo->erasesize); - } - - - /* read page data to memory buffer */ - result = meminfo->read(meminfo, - mtdoffset, - meminfo->writesize, - &readlen, - (unsigned char *) &data_buf); - - if (result != 0) { - printf("reading NAND page at offset 0x%lx failed\n", - mtdoffset); - return -1; - } - - if (imglen < readlen) { - readlen = imglen; - } - - memcpy(buffer, data_buf, readlen); - buffer += readlen; - imglen -= readlen; - - if (opts->readoob) { - result = meminfo->read_oob(meminfo, - mtdoffset, - meminfo->oobsize, - &readlen, - (unsigned char *) - &oob_buf); - - if (result != 0) { - printf("\nMTD readoob failure: %d\n", - result); - return -1; - } - - - if (imglen < readlen) { - readlen = imglen; - } - - memcpy(buffer, oob_buf, readlen); - - buffer += readlen; - imglen -= readlen; - } - - if (!opts->quiet) { - unsigned long long n = (unsigned long long) - (opts->length-imglen) * 100; - int percent; - - do_div(n, opts->length); - percent = (int)n; - - /* output progress message only at whole percent - * steps to reduce the number of messages printed - * on (slow) serial consoles - */ - if (percent != percent_complete) { - if (!opts->quiet) - printf("\rReading data from 0x%x " - "-- %3d%% complete.", - mtdoffset, percent); - percent_complete = percent; - } - } - - mtdoffset += meminfo->writesize; - } - - if (!opts->quiet) - printf("\n"); - - if (imglen > 0) { - printf("Could not read entire image due to bad blocks\n"); - return -1; - } - - /* return happy */ - return 0; -} -#endif - /* XXX U-BOOT XXX */ #if 0 /****************************************************************************** @@ -1007,4 +435,227 @@ int nand_unlock(nand_info_t *meminfo, ulong start, ulong length) } #endif
-#endif +/** + * get_len_incl_bad + * + * Check if length including bad blocks fits into device. + * + * @param nand NAND device + * @param offset offset in flash + * @param len image lenght + * @return image length including bad blocks + */ +static ulong get_len_incl_bad (nand_info_t *nand, ulong offset, ulong *len) +{ + ulong len_incl_bad; + ulong len_excl_bad; + ulong block_len; + + DEBUGP ("*len %05x, nand->erasesize %05x\n", *len, nand->erasesize); + + for (len_excl_bad = 0, len_incl_bad = 0; len_excl_bad < *len; ) + { + block_len = nand->erasesize - (offset & (nand->erasesize - 1)); + + if (!nand_block_isbad (nand, offset)) + len_excl_bad += block_len; + + len_incl_bad += block_len; + offset += block_len; + + DEBUGP ("excl %05x, incl %05x, offset %08x, block_len %05x\n", + len_excl_bad, len_incl_bad, offset, block_len); + + if ((offset + len_incl_bad) > nand->size) + break; + } + return len_incl_bad; +} + +/** + * nand_write_skip_bad: + * + * Write image to NAND flash. + * Blocks that are marked bad are skipped and the is written to the next + * block instead as long as the image is short enough to fit even after + * skipping the bad blocks. + * + * @param nand NAND device + * @param offset offset in flash + * @param len buffer lenght + * @param buf buffer to read from + * @return 0 in case of success + */ +int nand_write_skip_bad(nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{ + int result; + u_char *p_buffer = buf; + ulong left_to_write = *len; + ulong len_incl_bad; + + DEBUGP ("offset %08x, len 0x%05x\n", offset, *len); + + /* Reject writes, which are not page aligned */ + if (((offset & (nand->writesize - 1)) != 0) || + ((*len & (nand->writesize - 1)) != 0)) { + printf ("Attempt to write not page aligned data\n"); + return -EINVAL; + } + + len_incl_bad = get_len_incl_bad (nand, offset, len); + + if ((offset + len_incl_bad) > nand->size) { + printf ("Attempt to write outside the flash area\n"); + return -EINVAL; + } + + if (len_incl_bad == *len) + return (nand_write (nand, offset, len, buf)); + + while (left_to_write > 0) { + ulong block_offset = offset & (nand->erasesize - 1); + ulong write_size; + + DEBUGP ("left %05x, offset %08x, block_off %05x, p_buf %08x\n", + left_to_write, offset, block_offset, p_buffer); + + if (nand_block_isbad (nand, offset)) { + printf ("Skip bad block 0x%08x\n", + offset & ~(nand->erasesize - 1)); + offset += nand->erasesize - block_offset; + continue; + } + + if (left_to_write < (nand->erasesize - block_offset)) + write_size = left_to_write; + else + write_size = nand->erasesize - block_offset; + + result = nand_write (nand, offset, &write_size, p_buffer); + if (result != 0) { + printf ("Write failed %d\n", result); + return result; + } + left_to_write -= write_size; + offset += write_size; + p_buffer = (u_char *)((ulong)p_buffer + write_size); + } + return 0; +} + +/** + * nand_read_skip_bad: + * + * Read image from NAND flash. + * Blocks that are marked bad are skipped and the next block is readen + * instead as long as the image is short enough to fit even after skipping the + * bad blocks. + * + * @param nand NAND device + * @param offset offset in flash + * @param len buffer length + * @param buf buffer to write to + * @return 0 in case of success + */ +int nand_read_skip_bad(nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{ + int result; + u_char *p_buffer = buf; + ulong left_to_read = *len; + ulong len_incl_bad; + + DEBUGP ("offset %08x, len 0x%05x\n", offset, *len); + + len_incl_bad = get_len_incl_bad (nand, offset, len); + + if ((offset + len_incl_bad) > nand->size) { + printf ("Attempt to read outside the flash area\n"); + return -EINVAL; + } + + if (len_incl_bad == *len) + return (nand_read (nand, offset, len, buf)); + + while (left_to_read > 0) { + ulong block_offset = offset & (nand->erasesize - 1); + ulong read_length; + + DEBUGP ("left %05x, offset %08x, block_off %05x, p_buf %08x\n", + left_to_read, offset, block_offset, p_buffer); + + if (nand_block_isbad (nand, offset)) { + printf ("Skip bad block 0x%08x\n", + offset & ~(nand->erasesize - 1)); + offset += nand->erasesize - block_offset; + continue; + } + + if (left_to_read < (nand->erasesize - block_offset)) + read_length = left_to_read; + else + read_length = nand->erasesize - block_offset; + + result = nand_read (nand, offset, &read_length, p_buffer); + if (result != 0) { + printf ("Read failed %d\n", result); + return result; + } + left_to_read -= read_length; + offset += read_length; + p_buffer = (u_char *)((ulong)p_buffer + read_length); + } + return 0; +} + +/** + * nand_read_ff_upon_bad: + * + * Read image from NAND flash. + * Data from blocks that are marked bad is read as 0xff. + * + * @param nand NAND device + * @param offset offset in flash + * @param len buffer length + * @param buf buffer to write to + * @return 0 in case of success + */ +int nand_read_ff_upon_bad (nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{ + int result; + u_char *p_buffer = buf; + ulong left_to_read = *len; + + while (left_to_read > 0) { + ulong block_offset = offset & (nand->erasesize - 1); + ulong read_length; + + DEBUGP ("left %05x, offset %08x, block_off %04x, p_buf %08x\n", + left_to_read, offset, block_offset, p_buffer); + + if (left_to_read < (nand->erasesize - block_offset)) + read_length = left_to_read; + else + read_length = nand->erasesize - block_offset; + + if (nand_block_isbad (nand, offset)) { + int i; + printf ("Bad block 0x%08x. Read this block as 0xff\n", + offset & ~(nand->erasesize - 1)); + for (i = 0; i < read_length; i++) + p_buffer[i] = 0xff; + + } else { + result = nand_read (nand, offset, &read_length, p_buffer); + if (result != 0) { + printf ("Read failed %d\n", result); + return result; + } + } + left_to_read -= read_length; + offset += read_length; + p_buffer = (u_char *)((ulong)p_buffer + read_length); + } + return 0; +} + +#endif /* defined(CONFIG_CMD_NAND) && !defined(CFG_NAND_LEGACY) */ diff --git a/include/nand.h b/include/nand.h index 67dedbe..2388e7e 100644 --- a/include/nand.h +++ b/include/nand.h @@ -106,9 +106,9 @@ struct nand_erase_options {
typedef struct nand_erase_options nand_erase_options_t;
-int nand_write_opts(nand_info_t *mtd, loff_t to, mtd_oob_ops_t *ops); - -int nand_read_opts(nand_info_t *meminfo, const nand_read_options_t *opts); +int nand_read_ff_upon_bad(nand_info_t *nand, ulong offset, ulong *length, u_char *buffer); +int nand_read_skip_bad(nand_info_t *nand, ulong offset, ulong *length, u_char *buffer); +int nand_write_skip_bad(nand_info_t *nand, ulong offset, ulong *length, u_char *buffer); int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
#define NAND_LOCK_STATUS_TIGHT 0x01

Morten Ebbell Hestens wrote:
extern nand_info_t nand_info[]; /* info for NAND chips */ +extern struct nand_chip nand_chip[]; /* extra info for NAND chips */
Where is this defined or used?
static int nand_dump(nand_info_t *nand, ulong off, int only_oob) { @@ -73,9 +74,9 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) while (i--) { if (!only_oob) { printf( "\t%02x %02x %02x %02x %02x %02x %02x %02x"
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
Still exceeds 80 characters on the last line.
@@ -327,40 +327,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) goto usage;
addr = (ulong)simple_strtoul(argv[2], NULL, 16);
- read = strncmp(cmd, "read", 4) == 0; /* 1 = read, 0 = write */ printf("\nNAND %s: ", read ? "read" : "write"); if (arg_off_size(argc - 3, argv + 3, nand, &off, &size) != 0) return 1;
What's wrong with that newline?
@@ -372,9 +351,10 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) ret = nand->read_oob(nand, off, &ops); else ret = nand->write_oob(nand, off, &ops);
- } else {
Why add a blank line here?
U_BOOT_CMD(nand, 5, 1, do_nand,
- "nand - NAND sub-system\n",
- "info - show available NAND devices\n"
- "nand device [dev] - show or set current device\n"
- "nand read[.jffs2] - addr off|partition size\n"
- "nand write[.jffs2] - addr off|partition size - read/write `size' bytes starting\n"
- " at offset `off' to/from memory address `addr'\n"
- "nand erase [clean] [off size] - erase `size' bytes from\n"
- " offset `off' (entire device if not specified)\n"
- "nand bad - show bad blocks\n"
- "nand dump[.oob] off - dump page\n"
- "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n"
- "nand markbad off - mark bad block at offset (UNSAFE)\n"
- "nand biterr off - make a bit error at offset (UNSAFE)\n"
- "nand lock [tight] [status] - bring nand to lock state or display locked pages\n"
- "nand unlock [offset] [size] - unlock section\n");
"nand - NAND sub-system\n",
"info - show available NAND devices\n"
"nand device [dev] - show or set current device\n"
Should either keep these lined up or consistently have only one character before the dash.
"nand read[.jffs2, .i] addr off|partition size\n"
"nand write[.jffs2, .i] addr off|partition size\n"
What about .e? Is it just for backwards compatibility that we have three commands that mean the same thing? Do we want to document all three?
" - read/write `size' bytes starting at offset `off' to/from memory address `addr'\n"
Maybe mention the effect of .jffs2/.i/.e here in addition to the offline documentation.
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 1916aca..dd4ed6b 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -52,6 +52,14 @@ typedef struct mtd_info mtd_info_t; #define cpu_to_je16(x) (x) #define cpu_to_je32(x) (x)
+#if 0 +#define MARK printf("Was here: %s, %s, %d\n",__FILE__, __FUNCTION__, __LINE__); +#define DEBUGP(fmt,args...) printf("%s, %s, %d: "fmt,__FILE__, __FUNCTION__, __LINE__,##args) +#else +#define MARK +#define DEBUGP(fmt,args...) +#endif
We should probably use the existing MTD DEBUG() facility.
+/**
- get_len_incl_bad
- Check if length including bad blocks fits into device.
- @param nand NAND device
- @param offset offset in flash
- @param len image lenght
length
- @return image length including bad blocks
- */
+static ulong get_len_incl_bad (nand_info_t *nand, ulong offset, ulong *len)
Why is len a pointer? It doesn't seem to be modified.
+{
- ulong len_incl_bad;
- ulong len_excl_bad;
- ulong block_len;
- DEBUGP ("*len %05x, nand->erasesize %05x\n", *len, nand->erasesize);
- for (len_excl_bad = 0, len_incl_bad = 0; len_excl_bad < *len; )
- {
Brace on preceding line. Might be clearer as a while statement.
block_len = nand->erasesize - (offset & (nand->erasesize - 1));
if (!nand_block_isbad (nand, offset))
len_excl_bad += block_len;
Hmm, is it safe to call nand_block_isbad with an offset that isn't the start of the block? The BBT implementation seems OK with it, but what about block_bad callbacks?
+/**
- nand_write_skip_bad:
- Write image to NAND flash.
- Blocks that are marked bad are skipped and the is written to the next
- block instead as long as the image is short enough to fit even after
- skipping the bad blocks.
- @param nand NAND device
- @param offset offset in flash
- @param len buffer lenght
length
- @param buf buffer to read from
- @return 0 in case of success
- */
+int nand_write_skip_bad(nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{
- int result;
- u_char *p_buffer = buf;
- ulong left_to_write = *len;
- ulong len_incl_bad;
- DEBUGP ("offset %08x, len 0x%05x\n", offset, *len);
- /* Reject writes, which are not page aligned */
- if (((offset & (nand->writesize - 1)) != 0) ||
((*len & (nand->writesize - 1)) != 0)) {
printf ("Attempt to write not page aligned data\n");
return -EINVAL;
- }
- len_incl_bad = get_len_incl_bad (nand, offset, len);
- if ((offset + len_incl_bad) > nand->size) {
printf ("Attempt to write outside the flash area\n");
return -EINVAL;
- }
- if (len_incl_bad == *len)
return (nand_write (nand, offset, len, buf));
Unnecessary parentheses.
Here, len will be modified to show how much was written on an error, but in the loop below, it won't. In the loop below, we'll print an error if the write fails, but here we won't.
- while (left_to_write > 0) {
ulong block_offset = offset & (nand->erasesize - 1);
ulong write_size;
DEBUGP ("left %05x, offset %08x, block_off %05x, p_buf %08x\n",
left_to_write, offset, block_offset, p_buffer);
if (nand_block_isbad (nand, offset)) {
printf ("Skip bad block 0x%08x\n",
offset & ~(nand->erasesize - 1));
offset += nand->erasesize - block_offset;
continue;
}
if (left_to_write < (nand->erasesize - block_offset))
write_size = left_to_write;
else
write_size = nand->erasesize - block_offset;
result = nand_write (nand, offset, &write_size, p_buffer);
if (result != 0) {
printf ("Write failed %d\n", result);
return result;
We should probably be more detailed about exactly what failed. It's obvious (except for the exact block that failed) if the user typed in the command directly, but not if it's from a script.
}
left_to_write -= write_size;
offset += write_size;
p_buffer = (u_char *)((ulong)p_buffer + write_size);
Why not just p_buffer += write_size?
+/**
- nand_read_ff_upon_bad:
- Read image from NAND flash.
- Data from blocks that are marked bad is read as 0xff.
- @param nand NAND device
- @param offset offset in flash
- @param len buffer length
- @param buf buffer to write to
- @return 0 in case of success
- */
What was the reason for changing the behavior of non-jffs2 reads, BTW?
+int nand_read_ff_upon_bad (nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{
- int result;
- u_char *p_buffer = buf;
- ulong left_to_read = *len;
The aligned variable declaration style can be annoying to maintain if a new variable with a longer-named type has to be added in the future...
- while (left_to_read > 0) {
ulong block_offset = offset & (nand->erasesize - 1);
ulong read_length;
DEBUGP ("left %05x, offset %08x, block_off %04x, p_buf %08x\n",
left_to_read, offset, block_offset, p_buffer);
if (left_to_read < (nand->erasesize - block_offset))
read_length = left_to_read;
else
read_length = nand->erasesize - block_offset;
if (nand_block_isbad (nand, offset)) {
int i;
printf ("Bad block 0x%08x. Read this block as 0xff\n",
offset & ~(nand->erasesize - 1));
for (i = 0; i < read_length; i++)
p_buffer[i] = 0xff;
} else {
No blank line at the end of the block.
result = nand_read (nand, offset, &read_length, p_buffer);
if (result != 0) {
printf ("Read failed %d\n", result);
return result;
}
}
left_to_read -= read_length;
A blank line after the end of a block would be nice, though.
-Scott

On Tue, 13 May 2008 15:41:06 -0500 Scott Wood scottwood@freescale.com wrote:
static int nand_dump(nand_info_t *nand, ulong off, int only_oob) { @@ -73,9 +74,9 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) while (i--) { if (!only_oob) { printf( "\t%02x %02x %02x %02x %02x %02x %02x %02x"
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
Still exceeds 80 characters on the last line.
or use lib_generic/display_options.c:print_buffer()
Kim

Hi Kim,
Kim Phillips wrote:
On Tue, 13 May 2008 15:41:06 -0500 Scott Wood scottwood@freescale.com wrote:
static int nand_dump(nand_info_t *nand, ulong off, int only_oob) { @@ -73,9 +74,9 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) while (i--) { if (!only_oob) { printf( "\t%02x %02x %02x %02x %02x %02x %02x %02x"
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
" %02x %02x %02x %02x %02x %02x %02x %02x\n",
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7],
p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
Still exceeds 80 characters on the last line.
or use lib_generic/display_options.c:print_buffer()
806b4da8: ff ff 01 10 00 00 05 01 ........ or ff ff 01 10 00 00 05 01
The 806b4da8 is a address RAM address and has nothing to do with the nand offset. I think the last printout is better.
Morten

Hi Scott,
Thanks for a good review ! See my comment below. I will later make av new version of this patch.
Scott Wood wrote:
extern nand_info_t nand_info[]; /* info for NAND chips */ +extern struct nand_chip nand_chip[]; /* extra info for NAND chips */
Where is this defined or used?
No, it's for a other patch. I will remove it.
@@ -327,40 +327,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) goto usage;
addr = (ulong)simple_strtoul(argv[2], NULL, 16);
read = strncmp(cmd, "read", 4) == 0; /* 1 = read, 0 = write */ printf("\nNAND %s: ", read ? "read" : "write"); if (arg_off_size(argc - 3, argv + 3, nand, &off, &size) != 0) return 1;
What's wrong with that newline?
I will revert to get a less noisy patch.
@@ -372,9 +351,10 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) ret = nand->read_oob(nand, off, &ops); else ret = nand->write_oob(nand, off, &ops);
} else {
Why add a blank line here?
I will revert.
U_BOOT_CMD(nand, 5, 1, do_nand,
- "nand - NAND sub-system\n",
- "info - show available NAND devices\n"
- "nand device [dev] - show or set current device\n"
- "nand read[.jffs2] - addr off|partition size\n"
- "nand write[.jffs2] - addr off|partition size - read/write
`size' bytes starting\n"
- " at offset `off' to/from memory address `addr'\n"
- "nand erase [clean] [off size] - erase `size' bytes from\n"
- " offset `off' (entire device if not specified)\n"
- "nand bad - show bad blocks\n"
- "nand dump[.oob] off - dump page\n"
- "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n"
- "nand markbad off - mark bad block at offset (UNSAFE)\n"
- "nand biterr off - make a bit error at offset (UNSAFE)\n"
- "nand lock [tight] [status] - bring nand to lock state or display
locked pages\n"
- "nand unlock [offset] [size] - unlock section\n");
"nand - NAND sub-system\n",
"info - show available NAND devices\n"
"nand device [dev] - show or set current device\n"
Should either keep these lined up or consistently have only one character before the dash.
Ok.
"nand read[.jffs2, .i] addr off|partition size\n"
"nand write[.jffs2, .i] addr off|partition size\n"
What about .e? Is it just for backwards compatibility that we have three commands that mean the same thing? Do we want to document all three?
The doc/README.nand only mention .jffs2 and .i The option .jffs2 only has skipping bad blocks (and ECC) in common with the jffs2 file system. If we want to be backwards compatible we should use [.jffs2|.i|.e] otherwise [.i|.e] or [.skip] would be better. I leave this choice to you Scott.
" - read/write `size' bytes starting at offset `off' to/from
memory address `addr'\n"
Maybe mention the effect of .jffs2/.i/.e here in addition to the offline documentation.
Agree. Something like this ? "nand - NAND sub-system\n", "info - show available NAND devices\n" "nand device [dev] - show or set current device\n" "nand read[.jffs2|.i|.e] addr off|partition size - read `size' bytes\n" " starting at offset `off' to memory address `addr'. Using one of\n" " the `.' options bad blocks is skipped otherwise read as 0xff\n" "nand write[.jffs2|.i|.e] addr off|partition size - write `size'\n" " bytes starting at offset `off' from memory address `addr'. Using\n" " one of `.' options bad blocks is skipped otherwise write fails\n" "nand erase [clean] [off size] - erase `size' bytes from offset\n" " `off' (entire device if not specified)\n" "nand bad - show bad blocks\n" "nand dump[.oob] off - dump page\n" "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n" "nand markbad off - mark bad block at offset (UNSAFE)\n" "nand biterr off - make a bit error at offset (UNSAFE)\n" "nand lock [tight] [status] - bring nand to lock state or display\n" " locked pages\n" "nand unlock [offset] [size] - unlock section\n");
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 1916aca..dd4ed6b 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -52,6 +52,14 @@ typedef struct mtd_info mtd_info_t; #define cpu_to_je16(x) (x) #define cpu_to_je32(x) (x)
+#if 0 +#define MARK printf("Was here: %s, %s, %d\n",__FILE__, __FUNCTION__, __LINE__); +#define DEBUGP(fmt,args...) printf("%s, %s, %d: "fmt,__FILE__, __FUNCTION__, __LINE__,##args) +#else +#define MARK +#define DEBUGP(fmt,args...) +#endif
We should probably use the existing MTD DEBUG() facility.
I will remove it.
- @return image length including bad blocks
- */
+static ulong get_len_incl_bad (nand_info_t *nand, ulong offset, ulong *len)
Why is len a pointer? It doesn't seem to be modified.
Will change it to a const ulong length.
+{
- ulong len_incl_bad;
- ulong len_excl_bad;
- ulong block_len;
- DEBUGP ("*len %05x, nand->erasesize %05x\n", *len, nand->erasesize);
- for (len_excl_bad = 0, len_incl_bad = 0; len_excl_bad < *len; )
- {
Brace on preceding line. Might be clearer as a while statement.
Agree.
block_len = nand->erasesize - (offset & (nand->erasesize - 1));
if (!nand_block_isbad (nand, offset))
len_excl_bad += block_len;
Hmm, is it safe to call nand_block_isbad with an offset that isn't the start of the block? The BBT implementation seems OK with it, but what about block_bad callbacks?
Both nand_block_bad and nand_isbad_bbt seems ok. But just in case it could be changed to "if (!nand_block_isbad (nand, offset & (~nand->erasesize + 1))"
I do not understand what you mean about block_bad callbacks.
- @param buf buffer to read from
- @return 0 in case of success
- */
+int nand_write_skip_bad(nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{
- int result;
- u_char *p_buffer = buf;
- ulong left_to_write = *len;
- ulong len_incl_bad;
- DEBUGP ("offset %08x, len 0x%05x\n", offset, *len);
- /* Reject writes, which are not page aligned */
- if (((offset & (nand->writesize - 1)) != 0) || + ((*len &
(nand->writesize - 1)) != 0)) {
printf ("Attempt to write not page aligned data\n");
return -EINVAL;
- }
- len_incl_bad = get_len_incl_bad (nand, offset, len); +
- if ((offset + len_incl_bad) > nand->size) {
printf ("Attempt to write outside the flash area\n");
return -EINVAL;
- }
- if (len_incl_bad == *len)
return (nand_write (nand, offset, len, buf));
Unnecessary parentheses.
Here, len will be modified to show how much was written on an error, but in the loop below, it won't. In the loop below, we'll print an error if the write fails, but here we won't.
I will fix this.
- while (left_to_write > 0) {
ulong block_offset = offset & (nand->erasesize - 1);
ulong write_size;
DEBUGP ("left %05x, offset %08x, block_off %05x, p_buf %08x\n",
left_to_write, offset, block_offset, p_buffer);
if (nand_block_isbad (nand, offset)) {
printf ("Skip bad block 0x%08x\n",
offset & ~(nand->erasesize - 1));
offset += nand->erasesize - block_offset;
continue;
}
if (left_to_write < (nand->erasesize - block_offset))
write_size = left_to_write;
else
write_size = nand->erasesize - block_offset;
result = nand_write (nand, offset, &write_size, p_buffer);
if (result != 0) {
printf ("Write failed %d\n", result);
return result;
We should probably be more detailed about exactly what failed. It's obvious (except for the exact block that failed) if the user typed in the command directly, but not if it's from a script.
Agree.
}
left_to_write -= write_size;
offset += write_size;
p_buffer = (u_char *)((ulong)p_buffer + write_size);
Why not just p_buffer += write_size?
Agree.
+/**
- nand_read_ff_upon_bad: + *
- Read image from NAND flash. + * Data from blocks that are marked
bad is read as 0xff.
- @param nand NAND device
- @param offset offset in flash
- @param len buffer length
- @param buf buffer to write to
- @return 0 in case of success
- */
What was the reason for changing the behavior of non-jffs2 reads, BTW?
Quoting from a conversation with Stefan Roese:
In the README.nand: " nand read.jffs2 addr ofs|partition size Like `read', but the data for blocks that are marked bad is read as 0xff. This gives a readable JFFS2 image that can be processed by the JFFS2 commands such as ls and fsload. " Is this correct or should read skip the bad blocks and read the next instead ?
No. This seems to be incorrect. IIRC the "normal" nand read read 0xff's upon bad blocks. Please fix this when you submit a new patch.
+int nand_read_ff_upon_bad (nand_info_t *nand, ulong offset, ulong *len, u_char *buf) +{
- int result;
- u_char *p_buffer = buf;
- ulong left_to_read = *len;
The aligned variable declaration style can be annoying to maintain if a new variable with a longer-named type has to be added in the future...
Ok.
offset & ~(nand->erasesize - 1));
for (i = 0; i < read_length; i++)
p_buffer[i] = 0xff;
} else {
No blank line at the end of the block.
Ok
}
left_to_read -= read_length;
A blank line after the end of a block would be nice, though.
Ok
Best regards, Morten

Morten Ebbell Hestnes wrote:
"nand read[.jffs2, .i] addr off|partition size\n"
"nand write[.jffs2, .i] addr off|partition size\n"
What about .e? Is it just for backwards compatibility that we have three commands that mean the same thing? Do we want to document all three?
The doc/README.nand only mention .jffs2 and .i The option .jffs2 only has skipping bad blocks (and ECC) in common with the jffs2 file system. If we want to be backwards compatible we should use [.jffs2|.i|.e] otherwise [.i|.e] or [.skip] would be better. I leave this choice to you Scott.
I think we should stay compatible; I was just curious why some were documented and others weren't.
If we *weren't* maintaining backward compatibility, I'd make the block-skipping mode the default...
" - read/write `size' bytes starting at offset `off' to/from
memory address `addr'\n"
Maybe mention the effect of .jffs2/.i/.e here in addition to the offline documentation.
Agree. Something like this ? "nand - NAND sub-system\n", "info - show available NAND devices\n" "nand device [dev] - show or set current device\n" "nand read[.jffs2|.i|.e] addr off|partition size - read `size' bytes\n" " starting at offset `off' to memory address `addr'. Using one of\n" " the `.' options bad blocks is skipped otherwise read as 0xff\n" "nand write[.jffs2|.i|.e] addr off|partition size - write `size'\n" " bytes starting at offset `off' from memory address `addr'. Using\n" " one of `.' options bad blocks is skipped otherwise write fails\n" "nand erase [clean] [off size] - erase `size' bytes from offset\n" " `off' (entire device if not specified)\n" "nand bad - show bad blocks\n" "nand dump[.oob] off - dump page\n" "nand scrub - really clean NAND erasing bad blocks (UNSAFE)\n" "nand markbad off - mark bad block at offset (UNSAFE)\n" "nand biterr off - make a bit error at offset (UNSAFE)\n" "nand lock [tight] [status] - bring nand to lock state or display\n" " locked pages\n" "nand unlock [offset] [size] - unlock section\n");
s/blocks is skipped otherwise read/blocks are skipped, otherwise they are read/
Looks OK otherwise.
block_len = nand->erasesize - (offset & (nand->erasesize - 1));
if (!nand_block_isbad (nand, offset))
len_excl_bad += block_len;
Hmm, is it safe to call nand_block_isbad with an offset that isn't the start of the block? The BBT implementation seems OK with it, but what about block_bad callbacks?
Both nand_block_bad and nand_isbad_bbt seems ok. But just in case it could be changed to "if (!nand_block_isbad (nand, offset & (~nand->erasesize + 1))"
I do not understand what you mean about block_bad callbacks.
An individual NAND driver can provide its own block_bad() method in the nand_chip struct, and the semantics of the "ofs" parameter aren't described.
There's no code currently in-tree that would have a problem with it, but it might be better to mask it anyway.
-Scott
participants (4)
-
Kim Phillips
-
Morten Ebbell Hestens
-
Morten Ebbell Hestnes
-
Scott Wood