Hi Scott,
>
>> + int blocks = (int) onenand_block(this, from + len)
>> + - onenand_block(this, from);
>
> Why the (int) cast? onenand_block() already returns int.
>
Resolved
>> - 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.
>
Changed from mtd->size to (mtd->size) - 1,
In the OP, end_block was understood to accomodate (last_block_no + 1).
But, as you have pointed it may be more conventional to have the exact
last_block_no
made necessary changes in some other places as well.
>> 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.
>
Accepted and changed.
>> + 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.
>
Modified it to a while loop. Thanks :)
ofs = 0;
while (ofs < mtd->size) {
if (mtd->block_isbad(mtd, ofs))
printf(" %08x\n", (u32)ofs);
ofs += onenand_blocksize(ofs);
}
>> + die = (int) simple_strtoul(argv[2], NULL, 0);
>> + bdry = (int) simple_strtoul(argv[3], NULL, 0);
>
> Casts should not be necessary.
>
I believe that typecasting a UL to an int, is OK.
Do let me know.
>> + if (argc == 5 && strncmp(argv[4], "LOCK", 4) == 0)
>> + lock = 1;
>
> Please have commands be lower-case, just like all the other ones.
>
Changed
Do let me know, your feedback on the changes made.
I'll send the updated patch, in the next post.
With regards,
Amul
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
