[U-Boot] [PATCH] Nand: Implement raw read/write and biterr

New commands nand read.raw and write.raw read/write main and oob area.
Implement previously stubbed nand biterr command.
Document the above and also the previously undocumented read.oob and write.oob.
Signed-off-by: John Rigby jrigby@control4.com --- common/cmd_nand.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 158a55f..a488038 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -90,6 +90,95 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) return 0; }
+#define NAND_RW_RAW_READ 0 +#define NAND_RW_RAW_WRITE 1 + +static int nand_rdwr_raw(int rdwr, nand_info_t *nand, ulong off, u_char *buf, + size_t size) +{ + struct mtd_oob_ops ops = { + .len = nand->writesize, + .ooblen = nand->oobsize, + .mode = MTD_OOB_RAW, + }; + int i; + int nrblocks = size / nand->writesize; + loff_t addr = (loff_t)(off & ~(nand->writesize - 1)); + + while (nrblocks--) { + ops.datbuf = buf; + /* + * for read oobbuf must be set, but oob data + * will be appended to ops.datbuf + * for write oobbuf is actually used + */ + ops.oobbuf = buf + nand->writesize; + if (rdwr == NAND_RW_RAW_READ) + i = nand->read_oob(nand, addr, &ops); + else + i = nand->write_oob(nand, addr, &ops); + if (i < 0) { + printf("Error (%d) %s page %08lx\n", i, + rdwr == NAND_RW_RAW_READ ? + "reading" : "writing", + (unsigned long)addr); + return 1; + } + + addr += nand->writesize; + buf += (nand->writesize + nand->oobsize); + } + return 0; +} + +static int nand_read_raw(nand_info_t *nand, ulong off, u_char *buf, + size_t size) +{ + return nand_rdwr_raw(NAND_RW_RAW_READ, nand, off, buf, size); +} + +static int nand_write_raw(nand_info_t *nand, ulong off, u_char *buf, + size_t size) +{ + return nand_rdwr_raw(NAND_RW_RAW_WRITE, nand, off, buf, size); +} + +static int nand_biterr(nand_info_t *nand, ulong off, int bit) +{ + int ret = 0; + u_char *buf; + ulong blockoff = off & ~(nand->erasesize - 1); + int byteoff = off & (nand->erasesize - 1); + nand_erase_options_t opts = { + .offset = blockoff, + .length = nand->erasesize, + }; + + buf = malloc(nand->erasesize + + nand->oobsize * (nand->erasesize / nand->writesize)); + if (!buf) { + puts("No memory for page buffer\n"); + return 1; + } + nand_read_raw(nand, blockoff, buf, nand->erasesize); + + ret = nand_erase_opts(nand, &opts); + if (ret) { + puts("Failed to erase block at %x\n"); + return ret; + } + + printf("toggling bit %x in byte %x in block %x %02x ->", + bit, byteoff, blockoff, buf[byteoff]); + + buf[byteoff] ^= (1 << bit); + + printf("%02x\n", buf[byteoff]); + + nand_write_raw(nand, blockoff, buf, nand->erasesize); + free(buf); + return 0; +} /* ------------------------------------------------------------------------- */
static inline int str2long(char *p, ulong *num) @@ -401,6 +490,13 @@ 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 (!strcmp(s, ".raw")) { + if (read) + ret = nand_read_raw(nand, off, + (u_char *)addr, size); + else + ret = nand_write_raw(nand, off, + (u_char *)addr, size); } else { printf("Unknown nand command suffix '%s'.\n", s); return 1; @@ -437,9 +533,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) } return ret; } - if (strcmp(cmd, "biterr") == 0) { - /* todo */ + off = (ulong)simple_strtoul(argv[2], NULL, 16); + i = (int)simple_strtoul(argv[3], NULL, 16); + + int ret = nand_biterr(nand, off, i); + if (ret == 0) { + printf("byte offset 0x%08lx toggled bit %d\n", + (ulong) off, i); + return 0; + } else { + printf("byte offset 0x%08lx could not toggle bit %d\n", + (ulong) addr, i); + } return 1; }
@@ -494,13 +600,16 @@ U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand, "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" + " .oob - reads/writes oob only.\n" + " .raw - reads/writes both main and oob with no error\n" + " detection or correction\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(s) at offset (UNSAFE)\n" - "nand biterr off - make a bit error at offset (UNSAFE)" + "nand biterr off bitno - toggle bit bitno in byte off (UNSAFE)\n" #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK "\n" "nand lock [tight] [status]\n"

John Rigby wrote:
New commands nand read.raw and write.raw read/write main and oob area.
Implement previously stubbed nand biterr command.
Document the above and also the previously undocumented read.oob and write.oob.
Signed-off-by: John Rigby jrigby@control4.com
common/cmd_nand.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 158a55f..a488038 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -90,6 +90,95 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) return 0; }
+#define NAND_RW_RAW_READ 0 +#define NAND_RW_RAW_WRITE 1
+static int nand_rdwr_raw(int rdwr, nand_info_t *nand, ulong off, u_char *buf,
size_t size)
+{
- struct mtd_oob_ops ops = {
.len = nand->writesize,
.ooblen = nand->oobsize,
.mode = MTD_OOB_RAW,
- };
- int i;
- int nrblocks = size / nand->writesize;
- loff_t addr = (loff_t)(off & ~(nand->writesize - 1));
Silently dropping bytes. Would it be better to require the size to be a multiple of the block size ? Or at least warn of the dropped bytes.
- while (nrblocks--) {
ops.datbuf = buf;
<snip>
}
@@ -494,13 +600,16 @@ U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand, "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"
- " .oob - reads/writes oob only.\n"
- " .raw - reads/writes both main and oob with no error\n"
- " detection or correction\n"
Writing the oob area directly is UNSAFE. Having done to myself, it was a pain to undo. You should put at least document that.
Tom

Tom,
Thanks for the comments. I'll wait a couple of days for others then resubmit with fixes for these problems and any others that come up.
John
On Wed, Sep 30, 2009 at 1:22 PM, Tom Tom.Rix@windriver.com wrote:
John Rigby wrote:
New commands nand read.raw and write.raw read/write main and oob area.
Implement previously stubbed nand biterr command.
Document the above and also the previously undocumented read.oob and write.oob.
Signed-off-by: John Rigby jrigby@control4.com
common/cmd_nand.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 158a55f..a488038 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -90,6 +90,95 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob) return 0; } +#define NAND_RW_RAW_READ 0 +#define NAND_RW_RAW_WRITE 1
+static int nand_rdwr_raw(int rdwr, nand_info_t *nand, ulong off, u_char *buf,
size_t size)
+{
struct mtd_oob_ops ops = {
.len = nand->writesize,
.ooblen = nand->oobsize,
.mode = MTD_OOB_RAW,
};
int i;
int nrblocks = size / nand->writesize;
loff_t addr = (loff_t)(off & ~(nand->writesize - 1));
Silently dropping bytes. Would it be better to require the size to be a multiple of the block size ? Or at least warn of the dropped bytes.
while (nrblocks--) {
ops.datbuf = buf;
<snip>
}
@@ -494,13 +600,16 @@ U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand, "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"
" .oob - reads/writes oob only.\n"
" .raw - reads/writes both main and oob with no error\n"
" detection or correction\n"
Writing the oob area directly is UNSAFE. Having done to myself, it was a pain to undo. You should put at least document that.
Tom

On Wed, Sep 30, 2009 at 02:22:09PM -0500, Tom wrote:
John Rigby wrote:
@@ -494,13 +600,16 @@ U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand, "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"
- " .oob - reads/writes oob only.\n"
- " .raw - reads/writes both main and oob with no error\n"
- " detection or correction\n"
Writing the oob area directly is UNSAFE. Having done to myself, it was a pain to undo. You should put at least document that.
If we warn about oob being unsafe, the same warning should be made for raw.
-Scott

On Wed, Sep 30, 2009 at 12:11:39PM -0600, John Rigby wrote:
+static int nand_rdwr_raw(int rdwr, nand_info_t *nand, ulong off, u_char *buf,
size_t size)
Offset should be loff_t, here and elsewhere.
+{
- struct mtd_oob_ops ops = {
.len = nand->writesize,
.ooblen = nand->oobsize,
.mode = MTD_OOB_RAW,
- };
- int i;
- int nrblocks = size / nand->writesize;
- loff_t addr = (loff_t)(off & ~(nand->writesize - 1));
- while (nrblocks--) {
ops.datbuf = buf;
/*
* for read oobbuf must be set, but oob data
* will be appended to ops.datbuf
Hmm, that sounds like a bug in nand_do_read_ops().
* for write oobbuf is actually used
*/
ops.oobbuf = buf + nand->writesize;
if (rdwr == NAND_RW_RAW_READ)
i = nand->read_oob(nand, addr, &ops);
else
i = nand->write_oob(nand, addr, &ops);
if (i < 0) {
printf("Error (%d) %s page %08lx\n", i,
rdwr == NAND_RW_RAW_READ ?
"reading" : "writing",
(unsigned long)addr);
Don't cast to unsigned long; cast to unsigned long long and use %llx instead.
Change "page" to "offset"; IMHO the former implies "addr / nand->writesize".
buf += (nand->writesize + nand->oobsize);
Unnecessary parens.
+static int nand_biterr(nand_info_t *nand, ulong off, int bit) +{
- int ret = 0;
- u_char *buf;
- ulong blockoff = off & ~(nand->erasesize - 1);
- int byteoff = off & (nand->erasesize - 1);
- nand_erase_options_t opts = {
.offset = blockoff,
.length = nand->erasesize,
- };
- buf = malloc(nand->erasesize +
nand->oobsize * (nand->erasesize / nand->writesize));
- if (!buf) {
puts("No memory for page buffer\n");
s/page buffer/block buffer/
Also include the name of the function.
return 1;
- }
- nand_read_raw(nand, blockoff, buf, nand->erasesize);
Check whether it succeeded -- don't erase if you couldn't read the data.
- ret = nand_erase_opts(nand, &opts);
- if (ret) {
puts("Failed to erase block at %x\n");
return ret;
- }
- printf("toggling bit %x in byte %x in block %x %02x ->",
bit, byteoff, blockoff, buf[byteoff]);
- buf[byteoff] ^= (1 << bit);
Is there any way to flip a bit in the OOB using this command?
Also, unnecessary parens.
- printf("%02x\n", buf[byteoff]);
Put a space between -> and the number.
} else if (!strcmp(s, ".raw")) {
if (read)
ret = nand_read_raw(nand, off,
(u_char *)addr, size);
else
ret = nand_write_raw(nand, off,
(u_char *)addr, size);
Maybe just pass "!read" into nand_rdwr_raw?
} else { printf("Unknown nand command suffix '%s'.\n", s); return 1;
@@ -437,9 +533,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) } return ret; }
- if (strcmp(cmd, "biterr") == 0) {
/* todo */
off = (ulong)simple_strtoul(argv[2], NULL, 16);
i = (int)simple_strtoul(argv[3], NULL, 16);
int ret = nand_biterr(nand, off, i);
if (ret == 0) {
printf("byte offset 0x%08lx toggled bit %d\n",
(ulong) off, i);
return 0;
} else {
printf("byte offset 0x%08lx could not toggle bit %d\n",
(ulong) addr, i);
}
Why "addr" on failure but "off" on success? Looks like addr is used uninitialized.
As NAND is already on the heavy side, perhaps we should make non-core functionality like raw accesses and bit errors be configurable?
-Scott
participants (3)
-
John Rigby
-
Scott Wood
-
Tom