On Mon, Mar 23, 2009 at 12:36:32PM +0530, Amul Kumar Saha wrote: > diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c > index 5832ff8..ed6681a 100644 > --- a/common/cmd_onenand.c > +++ b/common/cmd_onenand.c > @@ -65,36 +65,49 @@ static int arg_off_size(int argc, char *argv[], ulong > *off, size_t *size) > return 0; > } > > +static inline int onenand_blocksize(loff_t ofs) > +{ > + struct onenand_chip *this = mtd->priv; > + int i;
Patch is whitespace-damaged. Please send the patch separately (not as a reply; it's easy to miss patches at the end of a reply, and I have to manually strip the discussion from the changelog) using something that will preserve the patch text intact (not Outlook Express). > + int blocks = (int) onenand_block(this, from + len) > + - onenand_block(this, from); Why the (int) cast? onenand_block() already returns int. > - if (end_block > (mtd->size >> this->erase_shift)) > - end_block = mtd->size >> this->erase_shift; > + if (end_block > onenand_block(this, mtd->size)) > + end_block = onenand_block(this, mtd->size); This should probably be: if (end_block >= onenand_block(this, mtd->size - 1)) ...to avoid asking for a block that doesn't exist. > blocks = start_block; > ofs = start; > while (blocks < end_block) { > - printf("\rTesting block %d at 0x%x", (u32)(ofs >> this->erase_shift), > (u32)ofs); > + printf("\rTesting block %d at 0x%x", (u32) onenand_block(this, ofs), > (u32)ofs); No (u32) cast on return from onenand_block(). Likewise elsewhere. > @@ -359,9 +378,11 @@ int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, > char *argv[]) > if (strcmp(cmd, "bad") == 0) { > /* Currently only one OneNAND device is supported */ > printf("\nDevice %d bad blocks:\n", 0); > - for (ofs = 0; ofs < mtd->size; ofs += mtd->erasesize) { > + for (ofs = 0; ofs < mtd->size; ofs += blocksize) { > if (mtd->block_isbad(mtd, ofs)) > printf(" %08x\n", (u32)ofs); > + > + blocksize = onenand_blocksize(ofs); > } Hmm, not sure I like blocksize being used in the for loop when it's first initialized in the body of the loop. It's valid code, but it might be more readable as a while loop. > + die = (int) simple_strtoul(argv[2], NULL, 0); > + bdry = (int) simple_strtoul(argv[3], NULL, 0); Casts should not be necessary. > + if (argc == 5 && strncmp(argv[4], "LOCK", 4) == 0) > + lock = 1; Please have commands be lower-case, just like all the other ones. -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot