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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
