Hi Scott,
Thank you for picking up the 'mtdparts spread' patch series into u-boot-nand-flash next by applying the v6 series and doing this fixup on-top. It is greatly appreciated.
On Thu, Sep 9, 2010 at 4:50 PM, Scott Wood scottwood@freescale.com wrote:
Consolidate some code in mtd_get_len_incl_bad(), and fix a condition where a valid partition could be reported as truncated if it has a good block at the end of the device (unlikely, since the BBT is usually there).
Fix mid-block declarations in net_part_size().
Signed-off-by: Scott Wood scottwood@freescale.com
common/cmd_mtdparts.c | 5 +++-- drivers/mtd/mtdcore.c | 15 +++++---------- 2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c index 17865b7..5481c88 100644 --- a/common/cmd_mtdparts.c +++ b/common/cmd_mtdparts.c @@ -1228,15 +1228,16 @@ static int generate_mtdparts_save(char *buf, u32 buflen) */ static uint64_t net_part_size(struct mtd_info *mtd, struct part_info *part) {
- uint64_t i, net_size = 0;
if (!mtd->block_isbad) return part->size;
- uint64_t i, net_size = 0;
for (i = 0; i < part->size; i += mtd->erasesize) { if (!mtd->block_isbad(mtd, part->offset + i)) net_size += mtd->erasesize; }
return net_size; } #endif
It's fine by me but I'm curious: why move the declaration of the stack variables i and net_size to before the check of the block_isbad function pointer? Before this change, I think there would be no allocation of these variables on the stack if the block_isbad function pointer is null.
I double-checked and there were no warnings from 'make' 'MAKEALL' (I'm using (Sourcery G++ Lite 2009q1-203) 4.3.3 here) or checkpatch.pl (from 2.6.36-rc3) before this change. Just curious -- I am interested in learning so I can produce and submit patchsets that meet the standards of this list in the future.
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 78f2a08..a195dda 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -162,11 +162,6 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset, *truncated = 0; *len_incl_bad = 0;
- if (offset >= mtd->size) {
- *truncated = 1;
- return;
- }
if (!mtd->block_isbad) { *len_incl_bad = length; return; @@ -176,6 +171,11 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset, uint64_t block_len;
while (len_excl_bad < length) {
- if (offset >= mtd->size) {
- *truncated = 1;
- return;
- }
block_len = mtd->erasesize - (offset & (mtd->erasesize - 1));
if (!mtd->block_isbad(mtd, offset & ~(mtd->erasesize - 1))) @@ -183,11 +183,6 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
*len_incl_bad += block_len; offset += block_len;
- if (offset >= mtd->size) {
- *truncated = 1;
- break;
- }
} } #endif /* defined(CONFIG_CMD_MTDPARTS_SPREAD) */
Awesome! Thank you for fixing that edge case.
Reviewed-by: Ben Gardiner bengardiner@nanometrics.ca
Best Regards, Ben Gardiner
--- Nanometrics Inc. http://www.nanometrics.ca