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